From 10bc14e204fb4138900ed08fd6c39592cab96992 Mon Sep 17 00:00:00 2001 From: Brett Date: Thu, 8 Oct 2026 18:36:38 -0500 Subject: [PATCH 1/4] fix(audit): keep p7-quiet's warn for a --help that does not exit on its own Moving `p7-quiet` onto the shared help probe changed one more thing than how the flag is matched. The shared probe keeps whatever a `--help` printed before it crashed or timed out, so a tool whose `--help` printed `-q, --quiet` and then died on a signal passed, where the audit used to warn that it could not run `--help`. `run()` reads the `--help` call's status again, from the runner's cache, and grades the help only when the call exited on its own. --- src/audits/behavioral/quiet.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/audits/behavioral/quiet.rs b/src/audits/behavioral/quiet.rs index 712ce7a..832133e 100644 --- a/src/audits/behavioral/quiet.rs +++ b/src/audits/behavioral/quiet.rs @@ -1,7 +1,7 @@ use crate::audit::Audit; use crate::audits::behavioral::flag_presence::pass_or_warn; use crate::project::Project; -use crate::runner::HelpOutput; +use crate::runner::{HelpOutput, RunStatus}; use crate::types::{AuditGroup, AuditLayer, AuditResult, AuditStatus, Confidence}; const QUIET_FLAGS: &[&str] = &["--quiet", "-q"]; @@ -34,9 +34,10 @@ impl Audit for QuietAudit { } fn run(&self, project: &Project) -> anyhow::Result { - let status = match project.help_output() { - None => AuditStatus::Warn("could not run --help to detect quiet flag".into()), - Some(help) => audit_quiet(help), + let ran = project.runner_ref().run(&["--help"], &[]).status; + let status = match (ran, project.help_output()) { + (RunStatus::Ok, Some(help)) => audit_quiet(help), + _ => AuditStatus::Warn("could not run --help to detect quiet flag".into()), }; Ok(AuditResult { @@ -81,6 +82,18 @@ mod tests { assert!(matches!(result.status, AuditStatus::Warn(_))); } + #[test] + fn a_help_that_crashes_is_not_searched() { + // Prints a quiet flag, then dies on a signal. + let project = + test_project_with_sh_script("echo ' -q, --quiet Suppress output'\nkill -11 $$"); + let result = QuietAudit.run(&project).expect("audit should run"); + assert_eq!( + result.status, + AuditStatus::Warn("could not run --help to detect quiet flag".into()) + ); + } + /// One definition line per tool whose help carries `-q` inside another /// flag's name and declares no quiet flag. const NO_QUIET_FLAG: &[(&str, &str)] = &[ From 9462a60ca5565ada5842e4c1add2cd91e3678443 Mon Sep 17 00:00:00 2001 From: Brett Date: Thu, 8 Oct 2026 18:36:39 -0500 Subject: [PATCH 2/4] fix(audit): read a confirm_flags entry written without dashes as a flag A `[p5] confirm_flags` entry such as `"noconfirm"` or `"y"` matched `--noconfirm` or `-y` while flags were looked up through `Flag::matches`, which added the dashes. The definition query compares spellings, so those entries stopped matching: the subcommand failed `p5-must-force-yes` and the entry was still listed under the accepted flags. `force_yes.rs` adds the dashes before it asks the query, two for a word and one for a single character, and takes an entry that already leads with a dash as written. Evidence still quotes the entry as the file spells it. --- README.md | 5 ++-- src/audits/behavioral/force_yes.rs | 43 ++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 17f452a..08ced24 100644 --- a/README.md +++ b/README.md @@ -164,8 +164,9 @@ The built-in names are `--force`, `--yes`, `-y`, `-f`, `--auto-approve`, `--assu flag counts beside them, and only where the subcommand's `--help` lists it, so a declaration names the flag and cannot stand in for one. Built-in and declared names match whole: `-f` does not match `-force-copy`. In a help that declares no double-dash name, a single-dash name matches its double-dash spelling, so terraform's `-auto-approve` meets the built-in -`--auto-approve` without a declaration, and a declared `--noconfirm` meets a listed `-noconfirm`. A pass that needed a -declared flag says so in the row's evidence, naming the subcommand, the flag, and the file: +`--auto-approve` without a declaration, and a declared `--noconfirm` meets a listed `-noconfirm`. An entry written +without dashes is read as a flag: `noconfirm` is `--noconfirm`, and `y` is `-y`. A pass that needed a declared flag says +so in the row's evidence, naming the subcommand, the flag, and the file: `destroy accepts --noconfirm via .anc.toml [p5].confirm_flags`. `not_destructive`: `p5-must-force-yes` treats a subcommand as destructive by its name (`delete`, `rm`, `purge`, `clean`, diff --git a/src/audits/behavioral/force_yes.rs b/src/audits/behavioral/force_yes.rs index 054f6ae..12e35ab 100644 --- a/src/audits/behavioral/force_yes.rs +++ b/src/audits/behavioral/force_yes.rs @@ -12,6 +12,8 @@ //! subcommand lists none. Vacuous Skip when the binary has no destructive //! subcommands. +use std::borrow::Cow; + use crate::anc_toml::{CONFIRM_FLAGS_KEY, NOT_DESTRUCTIVE_KEY, Sourced}; use crate::audit::Audit; use crate::audits::behavioral::destructive_ops::destructive_subcommands; @@ -105,6 +107,18 @@ impl Audit for ForceYesAudit { } } +/// A declared flag as the lookup spells it. An entry written without dashes +/// is a flag all the same: `noconfirm` is `--noconfirm`, and `y` is `-y`. +fn as_flag_name(value: &str) -> Cow<'_, str> { + if value.starts_with('-') { + Cow::Borrowed(value) + } else if value.chars().count() == 1 { + Cow::Owned(format!("-{value}")) + } else { + Cow::Owned(format!("--{value}")) + } +} + /// Pass when every destructive subcommand's `--help` lists a built-in /// confirmation flag or one of `declared_flags`. A built-in match takes /// priority; a Pass that needed a declared flag names the subcommand, the @@ -126,7 +140,7 @@ pub(crate) fn audit_force_yes( } match declared_flags .iter() - .find(|flag| help.find_flag(&[&flag.value]).is_some()) + .find(|flag| help.find_flag(&[&as_flag_name(&flag.value)]).is_some()) { Some(flag) => confirmed_by_declaration.push(format!( "{verb} accepts {} via {}", @@ -143,10 +157,14 @@ pub(crate) fn audit_force_yes( .then(|| Mitigation::Config(confirmed_by_declaration.join("; "))), }; } + let declared_names: Vec> = declared_flags + .iter() + .map(|flag| as_flag_name(&flag.value)) + .collect(); let wanted: Vec<&str> = CONFIRM_FLAGS .iter() .copied() - .chain(declared_flags.iter().map(|flag| flag.value.as_str())) + .chain(declared_names.iter().map(AsRef::as_ref)) .collect(); AuditStatus::Fail(format!( "destructive subcommand(s) whose --help lists no confirmation flag: {}. \ @@ -388,6 +406,27 @@ mod tests { ); } + #[test] + fn a_declared_flag_written_without_dashes_is_a_flag() { + let long = confirms( + "Options:\n --noconfirm Do not ask.\n -h, --help Show help.\n", + "noconfirm", + ); + assert_eq!(long.status, AuditStatus::Pass); + assert_eq!( + long.mitigation, + Some(Mitigation::Config( + "destroy accepts noconfirm via .anc.toml [p5].confirm_flags".into() + )) + ); + + let short = confirms( + "Options:\n -k Do not ask.\n -h, --help Show help.\n", + "k", + ); + assert_eq!(short.status, AuditStatus::Pass); + } + #[test] fn a_declared_flag_missing_from_the_subcommand_help_still_fails_and_is_named() { let subhelp = vec![( From 14ca51a2478b424c241bd6777731c3cdb1bb113d Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 9 Oct 2026 12:05:31 -0500 Subject: [PATCH 3/4] fix(audit): match a confirm_flags entry that leads with + or / as written Adding dashes to a `[p5] confirm_flags` entry that had none treated every entry not starting with `-` as a bare word. An entry spelled `+n` became `--+n` and stopped matching a help that declares `+n`. Only an entry that starts with a letter or digit is a bare word; one that leads with `-`, `+` or `/` is a name as written. --- src/audits/behavioral/force_yes.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/audits/behavioral/force_yes.rs b/src/audits/behavioral/force_yes.rs index 12e35ab..e77589e 100644 --- a/src/audits/behavioral/force_yes.rs +++ b/src/audits/behavioral/force_yes.rs @@ -107,10 +107,11 @@ impl Audit for ForceYesAudit { } } -/// A declared flag as the lookup spells it. An entry written without dashes -/// is a flag all the same: `noconfirm` is `--noconfirm`, and `y` is `-y`. +/// A declared flag as the lookup spells it. An entry written as a bare word +/// is a flag all the same: `noconfirm` is `--noconfirm`, and `y` is `-y`. An +/// entry that leads with `-`, `+` or `/` is a name as written. fn as_flag_name(value: &str) -> Cow<'_, str> { - if value.starts_with('-') { + if !value.starts_with(char::is_alphanumeric) { Cow::Borrowed(value) } else if value.chars().count() == 1 { Cow::Owned(format!("-{value}")) @@ -427,6 +428,15 @@ mod tests { assert_eq!(short.status, AuditStatus::Pass); } + #[test] + fn a_declared_flag_that_leads_with_a_plus_is_matched_as_written() { + let plus = confirms( + "Options:\n +n, --no-ask Do not ask.\n -h, --help Show help.\n", + "+n", + ); + assert_eq!(plus.status, AuditStatus::Pass); + } + #[test] fn a_declared_flag_missing_from_the_subcommand_help_still_fails_and_is_named() { let subhelp = vec![( From 7e3ca56dfddd0fc5179603b6432706e257d03cb7 Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 9 Oct 2026 12:05:32 -0500 Subject: [PATCH 4/4] test(audit): cover p7-quiet's warn for a --help that timed out The gate that keeps a `--help` which did not exit on its own from being searched had a test for a crash and none for a timeout. The match moves into `status_after`, which takes the run status and the help, so the timeout case is tested without waiting out the runner's five seconds. Letting `RunStatus::Timeout` through the gate fails the new test. --- src/audits/behavioral/quiet.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/audits/behavioral/quiet.rs b/src/audits/behavioral/quiet.rs index 832133e..876c40b 100644 --- a/src/audits/behavioral/quiet.rs +++ b/src/audits/behavioral/quiet.rs @@ -35,10 +35,7 @@ impl Audit for QuietAudit { fn run(&self, project: &Project) -> anyhow::Result { let ran = project.runner_ref().run(&["--help"], &[]).status; - let status = match (ran, project.help_output()) { - (RunStatus::Ok, Some(help)) => audit_quiet(help), - _ => AuditStatus::Warn("could not run --help to detect quiet flag".into()), - }; + let status = status_after(&ran, project.help_output()); Ok(AuditResult { id: self.id().to_string(), @@ -54,6 +51,16 @@ impl Audit for QuietAudit { } } +/// The verdict for a `--help` run that ended with `ran`. A help that did not +/// exit on its own is not searched: what it printed before it timed out or +/// died may not be all of it. +fn status_after(ran: &RunStatus, help: Option<&HelpOutput>) -> AuditStatus { + match (ran, help) { + (RunStatus::Ok, Some(help)) => audit_quiet(help), + _ => AuditStatus::Warn("could not run --help to detect quiet flag".into()), + } +} + pub(crate) fn audit_quiet(help: &HelpOutput) -> AuditStatus { pass_or_warn( help, @@ -94,6 +101,20 @@ mod tests { ); } + #[test] + fn a_help_that_timed_out_is_not_searched() { + let help = HelpOutput::from_raw(" -q, --quiet Suppress output"); + assert_eq!( + status_after(&RunStatus::Timeout, Some(&help)), + AuditStatus::Warn("could not run --help to detect quiet flag".into()) + ); + assert_eq!(status_after(&RunStatus::Ok, Some(&help)), AuditStatus::Pass); + assert_eq!( + status_after(&RunStatus::Ok, None), + AuditStatus::Warn("could not run --help to detect quiet flag".into()) + ); + } + /// One definition line per tool whose help carries `-q` inside another /// flag's name and declares no quiet flag. const NO_QUIET_FLAG: &[(&str, &str)] = &[