diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e976d..052d20b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,25 @@ Notable changes to Chrono Mock, newest first. The format follows ### Fixed +- **`--dry-run` approved sessions the real run refuses.** An impossible `--at` (month 13, 30 February, + 25:61) was printed as the plan and exited 0, while the real run was refused by the core with + exit 1. A file Windows will not start (a text file, an empty `.exe`, one cut short inside its + header, a library) was planned as "native injection" and exited 0, while the real run failed to + launch it with exit 2. The plan now refuses both with the code the real run gives, the moment in + the core's own words. It reads the header where the file says it is, so a program whose header + sits far into the file is still planned. What it leaves to the real run is whether the rest of the + file is intact. A batch script is still planned, because Windows starts it through the command + interpreter. `chronomock.plan/1` names that target state `not_a_program`. +- **A preset that dates from the application's file carried the time of day the file was written.** + `chrono run --preset date-before-install` without `--param` set the session to 23:00:35 the day + before installation, where the same preset given the date by hand sets midnight. A date parameter + is a date, so the file's creation date now arrives as midnight too. The trial presets were not + affected, because they set their own time. +- **Asking for help was answered as a mistake.** `chrono help` said "unknown command", and + `chrono calc --help` said "unknown flag" and exited 1, while `chrono --help` exited 0. `help`, + `run --help` and `calc --help` (and `-h`) now print the usage and exit 0. `--help` further along the + command line is still passed to the application. `--at` followed by another flag, or by `-h`, now + says that `--at` is missing its moment, instead of a sentence about a "shift". - **Ending a session sent the application's timers back, and left its web pages on the session date.** A session often ends while the application keeps running: `--ticks` ran out, or the session was stopped from the window. The application was then handed back the real value of every diff --git a/crates/cli/src/cli.rs b/crates/cli/src/cli.rs index 5811ffc..3731556 100644 --- a/crates/cli/src/cli.rs +++ b/crates/cli/src/cli.rs @@ -235,6 +235,28 @@ pub(crate) fn print_usage() { eprintln!("usage: chrono license [--components] (also --license) the licence, the warranty disclaimer, and every bundled component with its version"); } +/// Whether a word is the help flag, which every command answers. +pub(crate) fn is_help_flag(word: &str) -> bool { + matches!(word, "--help" | "-h") +} + +/// Whether the words after a command ask for its help: `run --help`, `calc -h`. Only the FIRST word +/// counts, because `--help` further along can be meant for the target (`--args --help`). +pub(crate) fn asks_for_help(rest: &[String]) -> bool { + rest.first().is_some_and(|word| is_help_flag(word)) +} + +/// The usage a command was asked for, and exit 0 - it is an answer, not a refusal. `calc --help` +/// used to say "unknown flag" and exit 1 while `chrono --help` beside it exited 0. +pub(crate) fn print_help_for(command: &str) -> i32 { + if command == "calc" { + print_calc_usage(); + } else { + print_usage(); + } + 0 +} + pub(crate) fn print_calc_usage() { eprintln!("usage: chrono calc [--base ] [--base-utc ] [--shift <±N>]... [--set-time ] [--snap ] [--nearest ] [--to-zone <+HH:MM>] [--zone <+HH:MM>] [--calendar ] [--format ] [--json]"); eprintln!(" or: chrono calc --preset [--param id=value]... (named moment, e.g. month-end, trial-first-day-after)"); diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index bf10b03..c1c10b3 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -63,7 +63,7 @@ mod zone; use calc::calc_run; use cdp_probe::{cdp_date_probe, cdp_launch_probe, cdp_probe, cdp_shim_probe}; -use cli::{print_license, print_usage, print_version}; +use cli::{asks_for_help, print_help_for, print_license, print_usage, print_version}; use core::core_mode; use run::driver_run; @@ -71,6 +71,7 @@ fn main() { let args: Vec = std::env::args().collect(); let code = match args.get(1).map(String::as_str) { Some("__core") => core_mode(), + Some(command @ ("run" | "calc")) if asks_for_help(&args[2..]) => print_help_for(command), Some("run") => driver_run(&args[2..]), Some("calc") => calc_run(&args[2..]), Some("__cdp-probe") => cdp_probe(&args[2..]), @@ -83,7 +84,7 @@ fn main() { 0 } Some("license") | Some("--license") => print_license(&args[2..]), - Some("--help") | Some("-h") | None => { + Some("--help") | Some("-h") | Some("help") | None => { print_usage(); 0 } diff --git a/crates/cli/src/pe.rs b/crates/cli/src/pe.rs index 078a117..0e5a451 100644 --- a/crates/cli/src/pe.rs +++ b/crates/cli/src/pe.rs @@ -1,6 +1,6 @@ //! A bounded, read-only look inside the target's own executable, for the facts the file names around -//! it cannot give: whether the Go toolchain linked it, and whether it is a .NET executable that leaves -//! no runtime file beside it to say so. +//! it cannot give: whether the Go toolchain linked it, whether it is a .NET executable that leaves +//! no runtime file beside it to say so, and whether its header describes a program at all. //! //! Every read is capped and every offset goes through `get`, so a truncated or hostile file answers //! "no" rather than panicking, and a half-gigabyte target costs a few kilobytes to ask. The one read @@ -14,10 +14,30 @@ use std::fs::File; use std::io::{Read, Seek, SeekFrom}; use std::path::Path; -/// How much of the PE headers to read. The section table sits right behind the optional header, and -/// four kilobytes covers it with room to spare on every binary we have measured. +/// How much of the PE header to read, counted from where the DOS header points. The section table sits +/// right behind the optional header, and four kilobytes covers it with room to spare on every binary +/// we have measured. const HEADER_WINDOW: u64 = 4096; +/// The DOS header, whose last field says where the PE header starts. +const DOS_HEADER: u64 = 0x40; + +/// The four bytes the PE header starts with. +const PE_SIGNATURE: &[u8; 4] = b"PE\0\0"; + +/// Where the optional header starts inside the PE header: behind the signature and the twenty-byte +/// file header. +const OPTIONAL_AT: usize = 24; + +/// How much of the PE header `is_pe_image` reads: the signature, the file header, and the optional +/// header's first field, its magic. +const PROGRAM_HEADER: u64 = 26; + +/// The two file header flags that say what kind of image this is, `IMAGE_FILE_EXECUTABLE_IMAGE` and +/// `IMAGE_FILE_DLL` in the PE format. +const FILE_IS_EXECUTABLE: u16 = 0x0002; +const FILE_IS_LIBRARY: u16 = 0x2000; + /// The size of one section header, and of one data directory entry, as the PE format fixes them. const SECTION_HEADER: usize = 40; const DATA_DIRECTORY_ENTRY: usize = 8; @@ -224,12 +244,86 @@ struct ImageSpan { pointer: usize, } -/// The first four kilobytes of a PE file, checked for the two signatures, plus the open handle so the -/// sections and directories they describe can be read. +/// Whether the file's header describes a program Windows can start. `Some(false)` only for a file that +/// was READ and whose header is missing, cut short, or a library's, `None` for one that could not be +/// read. +/// +/// Every refusal here is one Windows makes too, measured on synthetic images through the real run +/// (`CreateProcessW` failing with 0x800700C1 or 0x800700D8): no `MZ`, no `PE\0\0` where the DOS header +/// points, a file ending inside the file header, an optional header declared shorter than its magic, +/// a magic other than PE32 or PE32+, the executable flag missing, the library flag set, a section +/// table the file does not hold in full. The header is read where the DOS header points, however far +/// in: Windows starts an image whose header is 4 KiB, 32 KiB and 1 MiB into the file, which the first +/// version of this check, reading only the first four kilobytes, called "not a program". What it leaves +/// to the real run: the machine (`run::plan` explains why that field is not trusted), the subsystem, +/// and whether the sections behind the header are all there. +/// +/// The questions above lean to "no" on any doubt, because a false fingerprint accuses a target of +/// something it does not do. This one is asked in order to REFUSE a target, so its doubt leans the +/// other way: a file this could not read is not called "not a program" (untouchable rule 4). +pub(crate) fn is_pe_image(target_path: &Path) -> Option { + let mut file = File::open(target_path).ok()?; + let Some(pe) = pe_pointer(&read_bytes(&mut file, 0, DOS_HEADER)?) else { + return Some(false); + }; + let header = read_bytes(&mut file, pe, PROGRAM_HEADER)?; + let length = file.metadata().ok()?.len(); + Some(names_a_program(&header, pe, length)) +} + +/// Whether `header`, read at file offset `pe`, starts the PE header of a program: see `is_pe_image` +/// for what each condition is and that Windows refuses a file failing it. A header shorter than +/// `PROGRAM_HEADER` is a file ending inside it, which is a "no". +fn names_a_program(header: &[u8], pe: u64, length: u64) -> bool { + names_a_program_checked(header, pe, length) == Some(true) +} + +/// `names_a_program` with every read that can fail spelled `?`, so a short header is a "no". +fn names_a_program_checked(header: &[u8], pe: u64, length: u64) -> Option { + let sections = u64::from(u16_at(header, 6)?); + let optional_size = u16_at(header, 20)?; + let flags = u16_at(header, 22)?; + let magic = u16_at(header, OPTIONAL_AT)?; + let table_end = pe + OPTIONAL_AT as u64 + u64::from(optional_size) + sections * SECTION_HEADER as u64; + Some( + header.get(..4)? == PE_SIGNATURE + && optional_size >= 2 + && matches!(magic, 0x10B | 0x20B) + && flags & FILE_IS_EXECUTABLE != 0 + && flags & FILE_IS_LIBRARY == 0 + && table_end <= length, + ) +} + +/// Where the PE header starts, when `dos` begins with `MZ` and holds the DOS header's pointer. +fn pe_pointer(dos: &[u8]) -> Option { + if dos.get(..2)? != b"MZ" { + return None; + } + u32_at(dos, 0x3C).map(u64::from) +} + +/// Up to `limit` bytes of the file from `offset`: fewer where the file ends first, `None` when it +/// cannot be read. +fn read_bytes(file: &mut File, offset: u64, limit: u64) -> Option> { + let mut bytes: Vec = Vec::new(); + file.seek(SeekFrom::Start(offset)).ok()?; + // `take` + `read_to_end` rather than one `read`: a single read may return fewer bytes than + // asked for, and a short header would send the section-table offsets somewhere arbitrary. + file.by_ref().take(limit).read_to_end(&mut bytes).ok()?; + Some(bytes) +} + +/// The first four kilobytes of the PE header, read from where the DOS header points and checked for +/// its signature, plus the open handle so the sections and directories it describes can be read. +/// +/// Offsets into `head` count from the signature. The ones the header stores (a section's raw data) +/// count from the start of the file, and are read through the handle. The header is usually a few +/// hundred bytes in, but Windows starts an image whose header is a megabyte in (measured), and reading +/// from where it is rather than from the start of the file keeps such a target fingerprinted. struct PeFile { file: File, head: Vec, - optional: usize, optional_size: usize, section_count: usize, } @@ -237,24 +331,16 @@ struct PeFile { impl PeFile { fn open(path: &Path) -> Option { let mut file = File::open(path).ok()?; - let mut head: Vec = Vec::new(); - // `take` + `read_to_end` rather than one `read`: a single read may return fewer bytes than - // asked for, and a short header would send the section-table offsets somewhere arbitrary. - file.by_ref().take(HEADER_WINDOW).read_to_end(&mut head).ok()?; - if head.get(..2)? != b"MZ" { + let pe = pe_pointer(&read_bytes(&mut file, 0, DOS_HEADER)?)?; + let head = read_bytes(&mut file, pe, HEADER_WINDOW)?; + if head.get(..4)? != PE_SIGNATURE { return None; } - let pe = u32_at(&head, 0x3C)? as usize; - if head.get(pe..pe.checked_add(4)?)? != b"PE\0\0" { - return None; - } - let section_count = u16_at(&head, pe.checked_add(6)?)? as usize; - let optional_size = u16_at(&head, pe.checked_add(20)?)? as usize; - let optional = pe.checked_add(24)?; + let section_count = u16_at(&head, 6)? as usize; + let optional_size = u16_at(&head, 20)? as usize; Some(Self { file, head, - optional, optional_size, section_count, }) @@ -262,7 +348,7 @@ impl PeFile { /// The section headers in table order, up to the first one the header window does not hold. fn sections(&self) -> impl Iterator + '_ { - let table = self.optional.saturating_add(self.optional_size); + let table = OPTIONAL_AT.saturating_add(self.optional_size); (0..self.section_count).map_while(move |index| { let entry = table.checked_add(index.checked_mul(SECTION_HEADER)?)?; Some(Section { @@ -278,12 +364,12 @@ impl PeFile { /// The image base and size from the optional header. PE32 keeps a four-byte base at offset 28 and /// PE32+ an eight-byte one at 24, and both keep the size of the image at 56. fn image_span(&self) -> Option { - let (base, pointer) = match u16_at(&self.head, self.optional)? { - 0x10B => (u64::from(u32_at(&self.head, self.optional.checked_add(28)?)?), 4), - 0x20B => (u64_at(&self.head, self.optional.checked_add(24)?)?, 8), + let (base, pointer) = match u16_at(&self.head, OPTIONAL_AT)? { + 0x10B => (u64::from(u32_at(&self.head, OPTIONAL_AT + 28)?), 4), + 0x20B => (u64_at(&self.head, OPTIONAL_AT + 24)?, 8), _ => return None, }; - let size = u32_at(&self.head, self.optional.checked_add(56)?)?; + let size = u32_at(&self.head, OPTIONAL_AT + 56)?; Some(ImageSpan { base, end: base.checked_add(u64::from(size))?, @@ -303,20 +389,17 @@ impl PeFile { /// four stack and heap sizes before it are eight bytes wide in PE32+. Any other magic is a shape /// this module does not know, so it has no directories. fn directory(&self, index: usize) -> Option<(u32, u32)> { - let (count_at, table_at) = match u16_at(&self.head, self.optional)? { + let (count_at, table_at) = match u16_at(&self.head, OPTIONAL_AT)? { 0x10B => (92, 96), 0x20B => (108, 112), _ => return None, }; - if index >= u32_at(&self.head, self.optional.checked_add(count_at)?)? as usize { + if index >= u32_at(&self.head, OPTIONAL_AT + count_at)? as usize { return None; } - let entry = self - .optional - .checked_add(table_at)? - .checked_add(index.checked_mul(DATA_DIRECTORY_ENTRY)?)?; + let entry = (OPTIONAL_AT + table_at).checked_add(index.checked_mul(DATA_DIRECTORY_ENTRY)?)?; // The table lives inside the optional header, so an entry past its declared size is not one. - if entry.checked_add(DATA_DIRECTORY_ENTRY)? > self.optional.checked_add(self.optional_size)? { + if entry.checked_add(DATA_DIRECTORY_ENTRY)? > OPTIONAL_AT + self.optional_size { return None; } Some((u32_at(&self.head, entry)?, u32_at(&self.head, entry.checked_add(4)?)?)) @@ -335,10 +418,7 @@ impl PeFile { } fn read_at(&mut self, offset: u64, limit: usize) -> Option> { - let mut bytes: Vec = Vec::new(); - self.file.seek(SeekFrom::Start(offset)).ok()?; - self.file.by_ref().take(limit as u64).read_to_end(&mut bytes).ok()?; - Some(bytes) + read_bytes(&mut self.file, offset, limit as u64) } /// Up to `window` bytes from the start of the named section, or `None` when there is no such @@ -672,6 +752,95 @@ mod tests { } } + /// A synthetic image whose file header says what a linker says about a program: an executable + /// image, not a library. + fn program(magic: u16) -> Vec { + let mut pe = synthetic_pe(magic, b".text\0\0\0", b"\x31\xc0\xc3", None); + pe[0x80 + 22..0x80 + 24].copy_from_slice(&FILE_IS_EXECUTABLE.to_le_bytes()); + pe + } + + /// The same image with its PE header moved to `to`, where the DOS header then points. The section + /// table still names the section where it was, so only the header's place changes - the shape of + /// an image Windows starts with its header 4 KiB, 32 KiB and 1 MiB in (measured). + fn header_moved(pe: &[u8], to: usize) -> Vec { + let header = pe[0x80..RAW].to_vec(); + let mut moved = pe.to_vec(); + moved[0x80..RAW].fill(0); + moved.resize(moved.len().max(to + header.len()), 0); + moved[to..to + header.len()].copy_from_slice(&header); + moved[0x3C..0x40].copy_from_slice(&(to as u32).to_le_bytes()); + moved + } + + /// The one question here whose doubt leans the other way: a file read in full whose header is + /// missing, cut short or a library's is "not a program", every one of them a file Windows refuses + /// to start (measured), and a file that could not be read is "do not know" - never "not a program". + #[test] + fn only_a_file_whose_header_describes_a_program_is_called_one() { + let mut probes = Vec::new(); + let mut says = |name: &str, bytes: &[u8], expected: Option| { + let path = write_probe(name, bytes); + assert_eq!(is_pe_image(&path), expected, "{name}"); + probes.push(path); + }; + says("pe32plus", &program(0x20B), Some(true)); + says("pe32", &program(0x10B), Some(true)); + // Wherever the DOS header points: the first version read only the first four kilobytes, and + // called an image Windows starts "not a program". + says("header-at-4k", &header_moved(&program(0x20B), 0x1100), Some(true)); + says("header-at-1m", &header_moved(&program(0x20B), 0x10_0000), Some(true)); + + says("text", b"this is a note, not a program", Some(false)); + says("empty", b"", Some(false)); + // `MZ` alone is a DOS stub or a truncated file (measured: 0x800700D8). + says("mz-only", b"MZ\0\0", Some(false)); + let mut ne = program(0x20B); + ne[0x80..0x84].copy_from_slice(b"NE\0\0"); + says("mz-without-pe", &ne, Some(false)); + + // Cut short: right behind the signature, inside the file header, inside the section table. + let whole = program(0x20B); + says("cut-after-signature", &whole[..0x80 + 4], Some(false)); + says("cut-in-file-header", &whole[..0x80 + 20], Some(false)); + says("cut-in-section-table", &whole[..0x80 + 24 + 240 + 20], Some(false)); + + // Whole, and not a program. + let with_flags = |flags: u16| { + let mut pe = program(0x20B); + pe[0x80 + 22..0x80 + 24].copy_from_slice(&flags.to_le_bytes()); + pe + }; + says("library", &with_flags(FILE_IS_EXECUTABLE | FILE_IS_LIBRARY), Some(false)); + says("not-executable", &with_flags(0x0020), Some(false)); + for magic in [0u16, 0x107] { + let mut other = program(0x20B); + other[0x80 + 24..0x80 + 26].copy_from_slice(&magic.to_le_bytes()); + says(&format!("magic-{magic:x}"), &other, Some(false)); + } + let mut no_optional = program(0x20B); + no_optional[0x80 + 20..0x80 + 22].copy_from_slice(&0u16.to_le_bytes()); + says("optional-size-zero", &no_optional, Some(false)); + + assert_eq!(is_pe_image(Path::new("no such file anywhere.exe")), None); + for p in probes { + let _ = std::fs::remove_file(p); + } + } + + /// The fingerprints read the header where the DOS header points too. Reading only the first four + /// kilobytes left an image Windows starts, with its header further in, without its caution. + #[test] + fn a_header_far_into_the_file_is_still_fingerprinted() { + let go = write_probe("go-far", &header_moved(&synthetic_pe(0, b".data\0\0\0", &go_blob(), None), 0x1100)); + assert!(is_go_binary(&go), "the header 4 KiB in"); + let aot = write_probe("aot-far", &header_moved(&exporting(0x20B, &[b"DotNetRuntimeDebugHeader"]), 0x8000)); + assert!(is_dotnet_executable(&aot), "the header 32 KiB in"); + for p in [go, aot] { + let _ = std::fs::remove_file(p); + } + } + /// Every doubt answers "not Go". A target we cannot read must not be accused of a gap it may not /// have - an audit that invents one is no better than an audit that hides one (rule 4). #[test] diff --git a/crates/cli/src/preset.rs b/crates/cli/src/preset.rs index d8b289e..eaa45d0 100644 --- a/crates/cli/src/preset.rs +++ b/crates/cli/src/preset.rs @@ -624,7 +624,13 @@ pub(crate) fn read_target_creation_date( return None; } let wall = filetime_utc_to_wall(created as i64, tz_bias_min.unwrap_or(0)); - chrono_core::calc::parse_civil_datetime(&wall).ok() + // The DATE, at midnight. A `date` parameter is a bare date (docs/04 4.2), and the calculator's + // `--param install_date=...` is one. The time of day the file was written leaked through here into + // every preset without a `set_time` step: `date-before-install` put the session at 23:00:35 the + // day before, where the same preset with the same date given by hand gives midnight. + chrono_core::calc::parse_civil_datetime(&wall) + .ok() + .map(|d| chrono_core::calc::CivilDateTime { hour: 0, minute: 0, second: 0, ..d }) } /// Locate a preset file: next to the executable (portable layout), else in ./presets. @@ -1112,6 +1118,34 @@ mod tests { assert!(matches!(resolve_moment(p.moment, &values), Err(PresetError::BadFile(_)))); } + /// The file date is a DATE: midnight of the day the file was created, in the session zone, the + /// same value `--param install_date=YYYY-MM-DD` gives. It used to carry the time of day too. + /// + /// The file is given a creation time half an hour from midnight UTC on either side, so the day + /// itself moves with the zone: a date that ignored the zone, or read its sign backwards, lands on + /// the wrong day for at least one of the three. + #[test] + fn the_target_file_date_is_midnight_of_the_day_it_was_created() { + use std::os::windows::fs::FileTimesExt; + use std::time::{Duration, UNIX_EPOCH}; + let dir = crate::testutil::unique_temp_dir("chrono-preset-file-date"); + std::fs::create_dir_all(&dir).expect("scratch dir"); + let file = dir.join("app.exe"); + std::fs::write(&file, b"MZ").expect("file"); + // 2030-06-15T23:30:00Z and 2030-06-15T00:30:00Z. Bias is UTC minus local: -330 is UTC+05:30, + // 480 is UTC-08:00. + let late = 1_907_796_600; + for (created, days) in [(late, [(0, 15), (-330, 16), (480, 15)]), (late - 23 * 3600, [(0, 15), (-330, 15), (480, 14)])] { + let times = std::fs::FileTimes::new().set_created(UNIX_EPOCH + Duration::from_secs(created)); + std::fs::File::options().write(true).open(&file).expect("open").set_times(times).expect("set the creation time"); + for (bias, day) in days { + let d = read_target_creation_date(&file.display().to_string(), Some(bias)).expect("a creation date"); + assert_eq!((d.year, d.month, d.day, d.hour, d.minute, d.second), (2030, 6, day, 0, 0, 0), "created {created}, bias {bias}"); + } + } + let _ = std::fs::remove_dir_all(&dir); + } + /// The target_file_creation hint fills a date parameter from the target's file date (run only) - /// without a target it is the honest not-built - a duration slot or an unbuilt hint is refused. #[test] diff --git a/crates/cli/src/run/moment.rs b/crates/cli/src/run/moment.rs index fb2938a..c1cd3c4 100644 --- a/crates/cli/src/run/moment.rs +++ b/crates/cli/src/run/moment.rs @@ -201,16 +201,30 @@ pub(super) fn resolve_time_spec(ra: &RunArgs, now_bias: i32) -> Result) -> Result { + // `--at` takes the next word whatever it is, so `--at --dry-run` handed the flag over as the + // moment, and its leading '-' made that a relative one: the refusal then spoke of a "shift" + // nobody had written. No moment starts with two dashes, and none is spelled like the help flag + // (`-h` has no number), so the word is named as what it is. + if raw.starts_with("--") || crate::cli::is_help_flag(raw) { + return Err(format!( + "--at needs a moment after it, but the next word is the flag '{raw}' - write --at YYYY-MM-DDTHH:MM:SS, or a relative +N" + )); + } if raw.starts_with(['+', '-']) { let now = resolve_now_civil(tz_bias_min)?; resolve_relative_at(raw, now) } else { + // The core parses this same string with this same function before it starts anything, and + // refused 2038-13-45 there - AFTER the driver had already let a dry run approve it with exit + // 0 and a plan naming that moment. Docs/08 section 8 says a dry run's 0 means "this plan is + // sound", so a moment the run would refuse is refused here, in the core's own words. + chrono_core::calc::parse_civil_datetime(raw)?; Ok(raw.to_string()) } } @@ -264,6 +278,34 @@ mod tests { ); } + /// A moment the core would refuse is refused here, before anything starts, and in the core's own + /// words - the three impossible fields a dry run used to approve with exit 0. + #[test] + fn an_absolute_at_the_core_would_refuse_is_refused_before_anything_starts() { + for (raw, words) in [ + ("2038-13-45T00:00:00", "month out of range"), + ("2030-02-30T00:00:00", "day 30 out of range for month 2"), + ("2030-02-28T25:61:00", "hour 25 out of range"), + ] { + let e = resolve_at(raw, Some(0)).expect_err(raw); + assert!(e.contains(words), "{raw}: {e}"); + } + // The space the core accepts in place of the T stays accepted - one parser, one answer. + assert_eq!(resolve_at("2038-01-19 03:14:07", Some(0)).unwrap(), "2038-01-19 03:14:07"); + } + + /// `--at --dry-run` took the flag as the moment and answered with a sentence about a "shift", and + /// so did `--at -h`. A relative moment with a number stays one. + #[test] + fn a_flag_where_the_moment_should_be_is_named_as_a_flag() { + for flag in ["--dry-run", "-h", "--help"] { + let e = resolve_at(flag, Some(0)).unwrap_err(); + assert!(e.contains("--at needs a moment") && e.contains(&format!("'{flag}'")), "{flag}: {e}"); + assert!(!e.contains("shift"), "{flag}: {e}"); + } + assert!(resolve_at("-5h", Some(0)).is_ok(), "five hours back is a moment"); + } + #[test] fn at_relative_resolves_to_absolute_wall() { // Value is now-dependent, but a valid delta must produce a wall string. diff --git a/crates/cli/src/run/plan.rs b/crates/cli/src/run/plan.rs index eacb526..10156fe 100644 --- a/crates/cli/src/run/plan.rs +++ b/crates/cli/src/run/plan.rs @@ -56,6 +56,11 @@ const LABEL: usize = 12; enum TargetPath { /// A file is there, at the absolute path the session would use. Found(PathBuf), + /// A file is there, and Windows will not start it: its header is missing, cut short or a + /// library's, and it is not a batch script. Measured: the real run exits 2 with `CreateProcessW` + /// failing on a text file, an empty `.exe`, two bytes of `MZ` and this tool's own hook library, + /// while this plan used to call all four sound and exit 0. + NotAProgram(PathBuf), /// No file of that name, and the mechanism that would run it does not search anywhere else. Missing, /// A bare name on the Chromium path, which resolves it through PATH itself. @@ -67,6 +72,7 @@ impl TargetPath { fn key(&self) -> &'static str { match self { TargetPath::Found(_) => "found", + TargetPath::NotAProgram(_) => "not_a_program", TargetPath::Missing => "missing", TargetPath::Unchecked => "unchecked", } @@ -86,8 +92,8 @@ fn inspect_target(target: &str, chromium: bool) -> TargetPath { if path.is_file() { // The absolute path, so the plan names the file the session would open rather than whatever // the shell's current directory made of it. - let canonical = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); - return TargetPath::Found(readable(canonical)); + let canonical = readable(std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf())); + return if windows_would_start(path) { TargetPath::Found(canonical) } else { TargetPath::NotAProgram(canonical) }; } if chromium && is_bare_name(target) { TargetPath::Unchecked @@ -96,6 +102,27 @@ fn inspect_target(target: &str, chromium: bool) -> TargetPath { } } +/// Whether `CreateProcessW` would start this file, which both mechanisms end in: a program's PE image, +/// or a batch script. A file that could not be read gets the benefit of the doubt - the plan does not +/// know, so it does not refuse (untouchable rule 4). +/// +/// A batch script it starts through the command interpreter itself, although Microsoft Learn says the +/// caller must start `cmd.exe /c` for it. Measured with a script that writes down what it received: +/// it runs and gets its arguments, spaces in its path or name included. Except when an argument +/// carries quotes - the interpreter Windows starts then strips the first quote and the last one on +/// the line, and cannot find the script. That is a fact about the launch, not about the file, so the +/// plan does not refuse it here. +/// +/// Only WHETHER it is a program, never its bitness - see `mechanism_text` for why the header's +/// machine field is not trusted here. +fn windows_would_start(path: &Path) -> bool { + let batch = path + .extension() + .and_then(|e| e.to_str()) + .is_some_and(|e| e.eq_ignore_ascii_case("bat") || e.eq_ignore_ascii_case("cmd")); + batch || crate::pe::is_pe_image(path) != Some(false) +} + /// The canonical path without the extended-length prefix Windows answers `canonicalize` with. That /// prefix is correct and unreadable, and this line is read by a person first - `\\?\C:\app.exe` names /// the same file as `C:\app.exe`. @@ -129,8 +156,9 @@ struct Plan<'a> { zone_bias_min: i32, } -/// Print the plan and start nothing. Exit 0, except for a path that definitely leads to no file, -/// which exits 2 - the code a real run would give for the same fact (docs/08 section 8). +/// Print the plan and start nothing. Exit 0, except for a path that definitely leads to no file or +/// to a file Windows will not start, which exits 2 - the code a real run gives for the same fact +/// (docs/08 section 8). pub(super) fn dry_run(ra: &RunArgs, spec: &TimeSpec, origin: &TimeOrigin, now_bias: i32) -> i32 { // The same pure function the core calls, so the plan names the mechanism the core would choose // and not one worked out a second way (ADR-9). @@ -160,6 +188,13 @@ pub(super) fn dry_run(ra: &RunArgs, spec: &TimeSpec, origin: &TimeOrigin, now_bi ); return 2; } + if let TargetPath::NotAProgram(path) = &plan.target { + eprintln!( + "chrono: '{}' is not a program Windows can start - its header is missing, cut short or a library's, and it is not a batch script - so a real run would fail to launch it (exit 2)", + path.display() + ); + return 2; + } 0 } @@ -177,6 +212,10 @@ fn target_block(p: &Plan) -> String { let mut out = String::new(); match &p.target { TargetPath::Found(path) => out.push_str(&line("target", &path.display().to_string())), + TargetPath::NotAProgram(path) => { + out.push_str(&line("target", &path.display().to_string())); + out.push_str(¬e("not a program Windows can start - its header is missing, cut short or a library's, and it is not a batch script")); + } TargetPath::Missing => { out.push_str(&line("target", &p.ra.target)); out.push_str(¬e("there is no file here by that name")); @@ -208,6 +247,9 @@ fn mechanism_text(p: &Plan) -> String { if p.target == TargetPath::Missing { return "not decided - a mechanism is chosen from the target's own folder".to_string(); } + if matches!(p.target, TargetPath::NotAProgram(_)) { + return "none - neither mechanism can start this file".to_string(); + } if p.chromium { return "Chromium or Electron over CDP - the folder carries the Chromium runtime".to_string(); } @@ -428,7 +470,7 @@ fn render_json(p: &Plan) -> String { target: TargetJson { path: &p.ra.target, resolved: match &p.target { - TargetPath::Found(path) => Some(path.display().to_string()), + TargetPath::Found(path) | TargetPath::NotAProgram(path) => Some(path.display().to_string()), _ => None, }, state: p.target.key(), @@ -544,6 +586,27 @@ mod tests { /// name through PATH and so declined to judge one. It does not: the native mechanism passes the /// target to CreateProcessW as lpApplicationName, which Microsoft documents as never using the /// search path, and `chrono run notepad` exits 2 on this machine with notepad.exe on PATH twice. + /// A file that is there but that Windows will not start is its own state, and a batch script is + /// not one of them - `CreateProcessW` starts it through the command interpreter, measured. + #[test] + fn a_file_windows_will_not_start_is_not_a_program_and_a_batch_script_is() { + let dir = crate::testutil::unique_temp_dir("chrono-plan-not-a-program"); + std::fs::create_dir_all(&dir).expect("scratch dir"); + let note = dir.join("note.txt"); + std::fs::write(¬e, "not a program").expect("text file"); + assert!(matches!(inspect_target(¬e.display().to_string(), false), TargetPath::NotAProgram(_))); + assert!(matches!(inspect_target(¬e.display().to_string(), true), TargetPath::NotAProgram(_))); + for script in ["run.bat", "RUN.CMD"] { + let path = dir.join(script); + std::fs::write(&path, "@echo off\r\n").expect("batch file"); + assert!( + matches!(inspect_target(&path.display().to_string(), false), TargetPath::Found(_)), + "{script} is started by CreateProcessW, so the plan must not refuse it" + ); + } + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn a_name_that_is_not_a_file_here_is_missing_whether_or_not_it_looks_like_a_path() { assert_eq!(inspect_target(r"C:\definitely\not\here\nothing.exe", false), TargetPath::Missing); diff --git a/crates/cli/tests/dry_run.rs b/crates/cli/tests/dry_run.rs index edf59b7..bc574b8 100644 --- a/crates/cli/tests/dry_run.rs +++ b/crates/cli/tests/dry_run.rs @@ -15,6 +15,19 @@ use std::path::PathBuf; use std::process::Command; +use std::sync::{Mutex, MutexGuard}; + +/// Held by every test here that drives a REAL session. The core allows one session at a time, and +/// the tests of one file run on parallel threads, so two real runs side by side had the second +/// refused with "another session's core is running" - seen on the third run of this file after the +/// second real session came in, having passed the two before it. +static REAL_SESSION: Mutex<()> = Mutex::new(()); + +/// The lock, whether or not a test holding it before has failed - a failure there says nothing +/// about the next session, and a poisoned lock would turn one red test into several. +fn one_real_session_at_a_time() -> MutexGuard<'static, ()> { + REAL_SESSION.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} /// The command interpreter, by full path. fn command_interpreter() -> String { @@ -100,6 +113,7 @@ fn the_same_command_line_without_the_flag_does_start_the_target() { library.display() ); + let _session = one_real_session_at_a_time(); let dir = scratch("starts-something"); let marker = dir.join("the-target-ran"); @@ -194,6 +208,101 @@ fn every_committed_runner_builds_the_debug_artifacts_before_it_runs_the_tests() } } +/// A plan refuses what the run would refuse, with the code the run gives for it (docs/08 section 8): +/// a moment the core cannot read exits 1 in the core's own words, and a file Windows will not start +/// exits 2. Both used to exit 0 with a plan calling them sound - measured, while the real run refused +/// both. A batch script is the control: `CreateProcessW` starts one through the command interpreter, +/// so a plan that refused it would be wrong the other way. +#[test] +fn a_plan_refuses_what_the_run_would_refuse_and_nothing_else() { + let target = command_interpreter(); + for (at, words) in [ + ("2038-13-45T00:00:00", "month out of range"), + ("2030-02-30T00:00:00", "day 30 out of range for month 2"), + ("2030-02-28T25:61:00", "hour 25 out of range"), + ] { + let out = Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(["run", &target, "--at", at, "--dry-run"]) + .output() + .expect("the tool must run"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "{at}: {stderr}"); + assert!(stderr.contains(words), "{at}: the refusal must name the field: {stderr}"); + assert!( + !String::from_utf8_lossy(&out.stdout).contains("Nothing was started"), + "{at}: no plan is printed for a moment that cannot be" + ); + } + + let dir = scratch("not-a-program"); + let note = dir.join("note.txt"); + std::fs::write(¬e, "a note, not a program").expect("a text file"); + let out = Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(["run", ¬e.display().to_string(), "--at", "2038-01-19T03:14:07", "--dry-run"]) + .output() + .expect("the tool must run"); + assert_eq!(out.status.code(), Some(2), "{}", String::from_utf8_lossy(&out.stdout)); + assert!( + String::from_utf8_lossy(&out.stderr).contains("not a program Windows can start"), + "the reason must be on stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + + let script = dir.join("script.bat"); + std::fs::write(&script, "@echo off\r\n").expect("a batch script"); + let out = Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(["run", &script.display().to_string(), "--at", "2038-01-19T03:14:07", "--dry-run"]) + .output() + .expect("the tool must run"); + assert_eq!(out.status.code(), Some(0), "a batch script is started by Windows: {}", String::from_utf8_lossy(&out.stderr)); + + // A library is a whole PE image, and still not a program: its header says so, and Windows + // refuses it. The state, not only the code, because a missing file exits 2 as well. + let library = injected_library(); + assert!(library.is_file(), "this needs {}, which `cargo test` does not build", library.display()); + let out = Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(["run", &library.display().to_string(), "--at", "2038-01-19T03:14:07", "--dry-run", "--json"]) + .output() + .expect("the tool must run"); + let plan = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(2), "{plan}"); + assert!(plan.contains(r#""state":"not_a_program""#), "a library is not a program: {plan}"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The premise under the refusal above, checked on a real run: the text file and the library really +/// do fail to launch with exit 2. Should the core ever start such a file, the plan's refusal would +/// become the lie, and this is what would say so. +#[test] +fn a_real_run_of_a_file_windows_will_not_start_exits_two() { + let library = injected_library(); + assert!( + library.is_file(), + "this probe drives a real session and needs {}, which `cargo test` does not build. \ + Run `cargo build --workspace` first - CI and tools/gates.ps1 both do that now.", + library.display() + ); + let _session = one_real_session_at_a_time(); + let dir = scratch("real-not-a-program"); + let note = dir.join("note.txt"); + std::fs::write(¬e, "a note, not a program").expect("a text file"); + for target in [note.display().to_string(), library.display().to_string()] { + let out = Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(["run", &target, "--at", "2038-01-19T03:14:07", "--ticks", "1"]) + .output() + .expect("the tool must run"); + assert_eq!( + out.status.code(), + Some(2), + "{target}: stdout: {} stderr: {}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + } + let _ = std::fs::remove_dir_all(&dir); +} + /// A path with a directory component that holds no file is a fact the plan can establish without /// starting anything, and it exits 2 - the code a real run gives for the same fact. That is what /// makes `--dry-run` usable as a pre-flight check rather than a pretty printer. diff --git a/crates/cli/tests/network.rs b/crates/cli/tests/network.rs index 2dd7227..2f29e79 100644 --- a/crates/cli/tests/network.rs +++ b/crates/cli/tests/network.rs @@ -158,9 +158,16 @@ const ALLOWED: &[(&str, &str, &str)] = &[ ( "crates/cli/tests/dry_run.rs", "spawn", - "the dry-run guard runs the built binary twice over: once with --dry-run, which must start \ - nothing, and once without it, which must start the target - the second is what proves the \ - first can fail. Neither reaches past this machine", + "the dry-run guard runs the built binary: with --dry-run, which must start nothing and must \ + refuse what a real run refuses, and without it, which must start the target or fail to \ + launch a file Windows will not start - the real runs are what prove the plans can fail. \ + None reaches past this machine", + ), + ( + "crates/cli/tests/usage.rs", + "spawn", + "runs the built binary to ask it for help and to give it a flag without a value, and reads \ + what it answers. It starts no target and reaches nothing past this machine", ), ( "crates/cli/tests/session_clock.rs", diff --git a/crates/cli/tests/usage.rs b/crates/cli/tests/usage.rs new file mode 100644 index 0000000..1f38042 --- /dev/null +++ b/crates/cli/tests/usage.rs @@ -0,0 +1,69 @@ +//! What a person gets when they ask the tool for help, or leave a flag without its value: an answer +//! in words that match what they typed. +//! +//! Found by the release checks, which drive the packaged tool the way a person would: `chrono help` +//! said "unknown command", `chrono calc --help` said "unknown flag" and exited 1 while `chrono --help` +//! beside it exited 0, and `--at --dry-run` was answered with a sentence about a "shift" nobody wrote. + +use std::process::{Command, Output}; + +fn chrono(args: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_chrono")) + .args(args) + .output() + .expect("the tool must run") +} + +fn text(out: &Output) -> String { + format!("{}{}", String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr)) +} + +/// Asking is answered: usage, exit 0, no word about anything being unknown. +#[test] +fn every_way_of_asking_for_help_is_answered_with_usage_and_exit_zero() { + let asks: [&[&str]; 7] = [ + &["--help"], + &["-h"], + &["help"], + &["run", "--help"], + &["run", "-h"], + &["calc", "--help"], + &["calc", "-h"], + ]; + for args in asks { + let out = chrono(args); + let said = text(&out); + assert_eq!(out.status.code(), Some(0), "{args:?}: {said}"); + assert!(said.contains("usage:"), "{args:?} must print the usage: {said}"); + assert!(!said.contains("unknown"), "{args:?} is a question, not a mistake: {said}"); + } + // The calculator's help is the calculator's usage, not the whole tool's. + let calc = text(&chrono(&["calc", "--help"])); + assert!(calc.contains("chrono calc") && !calc.contains("chrono run "), "{calc}"); +} + +/// Only the first word after a command asks for its help. Further along, `--help` can belong to the +/// target, and taking it as a question would replace the session with a usage text. The plan names +/// the arguments the application would receive, so it shows `--help` arriving there - the tool itself +/// as the target, because the plan has to find a program to print one. +#[test] +fn a_help_flag_meant_for_the_target_is_passed_on_and_not_answered() { + let out = chrono(&["run", env!("CARGO_BIN_EXE_chrono"), "--args", "--help", "--dry-run", "--json"]); + let said = text(&out); + assert_eq!(out.status.code(), Some(0), "the plan, not the usage: {said}"); + assert!(said.contains(r#""args":["--help"]"#), "the application must be handed --help: {said}"); + assert!(!said.contains("usage:"), "{said}"); +} + +/// `--at` takes the next word, so `--at --dry-run` gave it a flag, and `--at -h` the help flag, whose +/// dash made it a relative moment. The refusal names the flag. +#[test] +fn a_flag_where_the_moment_should_be_is_named_as_a_flag() { + for flag in ["--dry-run", "-h"] { + let out = chrono(&["run", r"C:\definitely\not\here\app.exe", "--at", flag]); + let said = text(&out); + assert_eq!(out.status.code(), Some(1), "{flag}: {said}"); + assert!(said.contains("--at needs a moment") && said.contains(&format!("'{flag}'")), "{flag}: {said}"); + assert!(!said.contains("shift needs"), "{flag}: {said}"); + } +}