From eeac63b339226e7be44944c15652215ed5fbb1ba Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Tue, 14 Oct 2025 20:57:34 -0400 Subject: [PATCH] Fix exit code to return 0 when all findings are auto-fixable (#1242) Co-authored-by: William Woodruff --- crates/zizmor/src/main.rs | 18 +++++++++++--- crates/zizmor/src/output/fix.rs | 23 ++++++++++++++--- crates/zizmor/src/registry.rs | 24 ++++++++++++++++++ docs/release-notes.md | 10 ++++++++ docs/usage.md | 44 ++++++++++++++++++++++++--------- 5 files changed, 101 insertions(+), 18 deletions(-) diff --git a/crates/zizmor/src/main.rs b/crates/zizmor/src/main.rs index d4d7384c..795733ed 100644 --- a/crates/zizmor/src/main.rs +++ b/crates/zizmor/src/main.rs @@ -769,12 +769,24 @@ fn run(app: &mut App) -> Result { } }; - if let Some(fix_mode) = app.fix { - output::fix::apply_fixes(fix_mode, &results, ®istry).map_err(Error::Fix)?; - } + let all_fixed = if let Some(fix_mode) = app.fix { + let fix_result = + output::fix::apply_fixes(fix_mode, &results, ®istry).map_err(Error::Fix)?; + + // If all findings have applicable fixes and all were successfully applied, + // we should exit with success. + results.all_findings_have_applicable_fixes(fix_mode) + && fix_result.failed_count == 0 + && fix_result.applied_count > 0 + } else { + false + }; if app.no_exit_codes || matches!(app.format, OutputFormat::Sarif) { Ok(ExitCode::SUCCESS) + } else if all_fixed { + // All findings were auto-fixed, no manual intervention needed + Ok(ExitCode::SUCCESS) } else { Ok(results.exit_code()) } diff --git a/crates/zizmor/src/output/fix.rs b/crates/zizmor/src/output/fix.rs index bcea1ac0..f6a6a27a 100644 --- a/crates/zizmor/src/output/fix.rs +++ b/crates/zizmor/src/output/fix.rs @@ -13,12 +13,21 @@ use crate::{ registry::{FindingRegistry, input::InputKey, input::InputRegistry}, }; +/// Result of applying fixes. +#[derive(Debug)] +pub struct FixResult { + /// Number of fixes that were successfully applied. + pub applied_count: usize, + /// Number of fixes that failed to apply. + pub failed_count: usize, +} + /// Apply all fixes associated with findings, filtered by the specified fix mode. pub fn apply_fixes( fix_mode: FixMode, results: &FindingRegistry, registry: &InputRegistry, -) -> Result<()> { +) -> Result { let mut fixes_by_input: HashMap<&InputKey, Vec<(&Fix, &Finding)>> = HashMap::new(); let mut total_fixes = 0; for finding in results.fixable_findings() { @@ -46,12 +55,16 @@ pub fn apply_fixes( } else { anstream::eprintln!("No fixes available to apply."); } - return Ok(()); + return Ok(FixResult { + applied_count: 0, + failed_count: 0, + }); } // Process each file let mut applied_fixes = Vec::new(); let mut failed_fixes = Vec::new(); + let mut total_applied = 0; for (input_key, fixes) in &fixes_by_input { let InputKey::Local(local) = input_key else { @@ -72,6 +85,7 @@ pub fn apply_fixes( Ok(new_document) => { current_document = new_document; file_applied_fixes.push((finding.ident, fix, finding)); + total_applied += 1; } Err(e) => { // If the fix fails on modified content, it might be due to conflicts @@ -101,7 +115,10 @@ pub fn apply_fixes( print_summary(&applied_fixes, &failed_fixes); } - Ok(()) + Ok(FixResult { + applied_count: total_applied, + failed_count: failed_fixes.len(), + }) } fn print_summary( diff --git a/crates/zizmor/src/registry.rs b/crates/zizmor/src/registry.rs index bf055c1e..a12949fd 100644 --- a/crates/zizmor/src/registry.rs +++ b/crates/zizmor/src/registry.rs @@ -199,6 +199,30 @@ impl<'a> FindingRegistry<'a> { }) } + /// Checks if all findings have at least one fix matching the given fix mode. + /// + /// Returns true if every finding has at least one applicable fix based on the mode, + /// meaning no manual intervention would be required if all fixes are applied successfully. + pub(crate) fn all_findings_have_applicable_fixes(&self, fix_mode: crate::FixMode) -> bool { + use crate::finding::FixDisposition; + + if self.findings.is_empty() { + return true; + } + + self.findings.iter().all(|finding| { + finding.fixes.iter().any(|fix| { + let disposition_matches = match fix_mode { + crate::FixMode::Safe => matches!(fix.disposition, FixDisposition::Safe), + crate::FixMode::UnsafeOnly => matches!(fix.disposition, FixDisposition::Unsafe), + crate::FixMode::All => true, + }; + + disposition_matches && matches!(fix.key, InputKey::Local(_)) + }) + }) + } + /// All ignored findings. pub(crate) fn ignored(&self) -> &[Finding<'a>] { &self.ignored diff --git a/docs/release-notes.md b/docs/release-notes.md index ae900047..9d7cbf27 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -9,6 +9,14 @@ of `zizmor`. ## Next (UNRELEASED) +### Enhancements 🌱 + +* When running in `--fix` mode and all fixes are successfully applied, + `zizmor` now has similar [exit code] behavior as the `--no-exit-codes` + and `--format=sarif` flags (#1242) + + Many thanks to @cnaples79 for implementing this improvement! + ## 1.15.2 ### Bug Fixes 🐛 @@ -1161,3 +1169,5 @@ This is one of `zizmor`'s bigger recent releases! Key enhancements include: [ref-version-mismatch]: ./audits.md#ref-version-mismatch [dependabot-execution]: ./audits.md#dependabot-execution [dependabot-cooldown]: ./audits.md#dependabot-cooldown + +[exit code]: ./usage.md#exit-codes diff --git a/docs/usage.md b/docs/usage.md index d89dac3c..057fcd5e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -393,18 +393,8 @@ annotations. ## Exit codes -!!! note - - Exit codes 11 and above are **not used** if `--no-exit-codes` or - `--format sarif` is passed. - -!!! warning "Removal" - - Versions of zizmor prior to `v1.14.0` used exit code `10` to indicate - the highest finding having "unknown" severity. This exit code is - no longer used as of `v1.14.0`. - -`zizmor` uses various exit codes to summarize the results of a run: +`zizmor` will exit with a variety of exit codes, depending on how +you invoke it and what happens during the run. In general: | Code | Meaning | | ---- | ------- | @@ -418,6 +408,36 @@ annotations. All other exit codes are currently reserved. +!!! warning "Removal" + + Versions of zizmor prior to `v1.14.0` used exit code `10` to indicate + the highest finding having "unknown" severity. This exit code is + no longer used as of `v1.14.0`. + +Beyond this general table, the following modes of operation also +change the exit code behavior: + +* If you run with `--no-exit-codes`, `zizmor` will **not** use exit codes 11 + and above. +* If you use `--format=sarif`, `zizmor` will **not** use exit codes 11 and + above. + + !!! tip "Why?" + + Because SARIF consumers generally don't expect SARIF producers + (like `zizmor`) to exit with a non-zero code *except* in the case + of an internal failure. SARIF consumers expect semantic results + to be communicated within the SARIF itself. + +* If you run with `--fix` *and* all findings are successfully auto-fixed, + `zizmor` will **not** use exit codes 11 and above. + + !!! tip "Why?" + + Because the higher exit codes indicate that action needs to be taken, + but a successful application of all fixes means that no action + is required. + ## Using personas !!! tip