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..e77589e 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,19 @@ impl Audit for ForceYesAudit { } } +/// 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(char::is_alphanumeric) { + 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 +141,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 +158,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 +407,36 @@ 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_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![( diff --git a/src/audits/behavioral/quiet.rs b/src/audits/behavioral/quiet.rs index 712ce7a..876c40b 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,10 +34,8 @@ 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 = status_after(&ran, project.help_output()); Ok(AuditResult { id: self.id().to_string(), @@ -53,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, @@ -81,6 +89,32 @@ 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()) + ); + } + + #[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)] = &[