Add Fixes for dependabot-cooldown and dependabot-execution audit rules (#1229)

Co-authored-by: William Woodruff <william@yossarian.net>

crates/zizmor/src/audit/dependabot_cooldown.rs

@@ -1,7 +1,8 @@
 use crate::{
     audit::{Audit, audit_meta},
-    finding::{Confidence, Severity, location::Locatable as _},
+    finding::{Confidence, Fix, FixDisposition, Severity, location::Locatable as _},
 };
+use yamlpatch::{Op, Patch};
 
 audit_meta!(
     DependabotCooldown,
@@ -11,6 +12,67 @@ audit_meta!(
 
 pub(crate) struct DependabotCooldown;
 
+impl DependabotCooldown {
+    /// Creates a fix that adds default-days to an existing cooldown block
+    fn create_add_default_days_fix<'doc>(
+        update: crate::models::dependabot::Update<'doc>,
+    ) -> Fix<'doc> {
+        Fix {
+            title: "add default-days to cooldown".to_string(),
+            key: update.location().key,
+            disposition: FixDisposition::Safe,
+            patches: vec![Patch {
+                route: update.location().route.with_keys(["cooldown".into()]),
+                operation: Op::Add {
+                    key: "default-days".to_string(),
+                    value: serde_yaml::Value::Number(7.into()),
+                },
+            }],
+        }
+    }
+
+    /// Creates a fix that increases an insufficient default-days value
+    fn create_increase_default_days_fix<'doc>(
+        update: crate::models::dependabot::Update<'doc>,
+    ) -> Fix<'doc> {
+        Fix {
+            title: "increase default-days to 7".to_string(),
+            key: update.location().key,
+            disposition: FixDisposition::Safe,
+            patches: vec![Patch {
+                route: update
+                    .location()
+                    .route
+                    .with_keys(["cooldown".into(), "default-days".into()]),
+                operation: Op::Replace(serde_yaml::Value::Number(7.into())),
+            }],
+        }
+    }
+
+    /// Creates a fix that adds a cooldown block with default-days
+    fn create_add_cooldown_fix<'doc>(update: crate::models::dependabot::Update<'doc>) -> Fix<'doc> {
+        Fix {
+            title: "add cooldown configuration".to_string(),
+            key: update.location().key,
+            disposition: FixDisposition::Safe,
+            patches: vec![Patch {
+                route: update.location().route,
+                operation: Op::Add {
+                    key: "cooldown".to_string(),
+                    value: serde_yaml::Value::Mapping({
+                        let mut map = serde_yaml::Mapping::new();
+                        map.insert(
+                            serde_yaml::Value::String("default-days".to_string()),
+                            serde_yaml::Value::Number(7.into()),
+                        );
+                        map
+                    }),
+                },
+            }],
+        }
+    }
+}
+
 impl Audit for DependabotCooldown {
     fn new(_state: &crate::state::AuditState) -> Result<Self, super::AuditLoadError>
     where
@@ -45,6 +107,7 @@ impl Audit for DependabotCooldown {
                             )
                             .confidence(Confidence::High)
                             .severity(Severity::Medium)
+                            .fix(Self::create_add_default_days_fix(update))
                             .build(dependabot)?,
                     ),
                     // We currently (arbitrarily) consider cooldowns under 4 days
@@ -63,6 +126,7 @@ impl Audit for DependabotCooldown {
                             )
                             .confidence(Confidence::Medium)
                             .severity(Severity::Low)
+                            .fix(Self::create_increase_default_days_fix(update))
                             .build(dependabot)?,
                     ),
                     Some(_) => {}
@@ -77,6 +141,7 @@ impl Audit for DependabotCooldown {
                         )
                         .confidence(Confidence::High)
                         .severity(Severity::Medium)
+                        .fix(Self::create_add_cooldown_fix(update))
                         .build(dependabot)?,
                 ),
             }
@@ -85,3 +150,245 @@ impl Audit for DependabotCooldown {
         Ok(findings)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{
+        config::Config,
+        models::{AsDocument, dependabot::Dependabot},
+        registry::input::InputKey,
+        state::AuditState,
+    };
+
+    /// Macro for testing dependabot audits with common boilerplate
+    macro_rules! test_dependabot_audit {
+        ($audit_type:ty, $filename:expr, $dependabot_content:expr, $test_fn:expr) => {{
+            let key = InputKey::local("fakegroup".into(), $filename, None::<&str>).unwrap();
+            let dependabot = Dependabot::from_string($dependabot_content.to_string(), key).unwrap();
+            let audit_state = AuditState::default();
+            let audit = <$audit_type>::new(&audit_state).unwrap();
+            let findings = audit
+                .audit_dependabot(&dependabot, &Config::default())
+                .unwrap();
+
+            $test_fn(&dependabot, findings)
+        }};
+    }
+
+    #[test]
+    fn test_fix_missing_cooldown() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+    insecure-external-code-execution: deny
+"#;
+
+        test_dependabot_audit!(
+            DependabotCooldown,
+            "test_fix_missing_cooldown.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert!(!findings.is_empty(), "Expected findings but got none");
+                let finding = &findings[0];
+                assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+
+                let fix = &finding.fixes[0];
+                let fixed_document = fix.apply(dependabot.as_document()).unwrap();
+                insta::assert_snapshot!(fixed_document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+                    cooldown:
+                      default-days: 7
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_fix_missing_default_days() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    cooldown: {}
+    schedule:
+      interval: daily
+    insecure-external-code-execution: deny
+"#;
+
+        test_dependabot_audit!(
+            DependabotCooldown,
+            "test_fix_missing_default_days.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert!(!findings.is_empty(), "Expected findings but got none");
+                let finding = &findings[0];
+                assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+
+                let fix = &finding.fixes[0];
+                let fixed_document = fix.apply(dependabot.as_document()).unwrap();
+                insta::assert_snapshot!(fixed_document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    cooldown: { default-days: 7 }
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_fix_insufficient_default_days() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    cooldown:
+      default-days: 2
+    schedule:
+      interval: daily
+    insecure-external-code-execution: deny
+"#;
+
+        test_dependabot_audit!(
+            DependabotCooldown,
+            "test_fix_insufficient_default_days.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert!(!findings.is_empty(), "Expected findings but got none");
+                let finding = &findings[0];
+                assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+
+                let fix = &finding.fixes[0];
+                let fixed_document = fix.apply(dependabot.as_document()).unwrap();
+                insta::assert_snapshot!(fixed_document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    cooldown:
+                      default-days: 7
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_fix_multiple_updates() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+
+  - package-ecosystem: npm
+    directory: /
+    cooldown:
+      default-days: 1
+    schedule:
+      interval: weekly
+"#;
+
+        test_dependabot_audit!(
+            DependabotCooldown,
+            "test_fix_multiple_updates.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert_eq!(findings.len(), 2, "Expected 2 findings");
+
+                // Apply all fixes and snapshot the result
+                let mut document = dependabot.as_document().clone();
+                for finding in &findings {
+                    assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+                    for fix in &finding.fixes {
+                        document = fix.apply(&document).unwrap();
+                    }
+                }
+
+                insta::assert_snapshot!(document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                    cooldown:
+                      default-days: 7
+
+                  - package-ecosystem: npm
+                    directory: /
+                    cooldown:
+                      default-days: 7
+                    schedule:
+                      interval: weekly
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_no_fix_needed_for_sufficient_cooldown() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    cooldown:
+      default-days: 7
+    schedule:
+      interval: daily
+"#;
+
+        test_dependabot_audit!(
+            DependabotCooldown,
+            "test_no_fix_needed.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert_eq!(findings.len(), 0, "Expected no findings");
+
+                // Verify the document remains unchanged
+                insta::assert_snapshot!(dependabot.as_document().source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    cooldown:
+                      default-days: 7
+                    schedule:
+                      interval: daily
+                ");
+            }
+        );
+    }
+}

crates/zizmor/src/audit/dependabot_execution.rs

@@ -2,8 +2,9 @@ use github_actions_models::dependabot::v2::AllowDeny;
 
 use crate::{
     audit::{Audit, audit_meta},
-    finding::location::Locatable as _,
+    finding::{Fix, FixDisposition, location::Locatable as _},
 };
+use yamlpatch::{Op, Patch};
 
 audit_meta!(
     DependabotExecution,
@@ -13,6 +14,24 @@ audit_meta!(
 
 pub(crate) struct DependabotExecution;
 
+impl DependabotExecution {
+    /// Creates a fix that changes insecure-external-code-execution from allow to deny
+    fn create_set_deny_fix<'doc>(update: crate::models::dependabot::Update<'doc>) -> Fix<'doc> {
+        Fix {
+            title: "set insecure-external-code-execution to deny".to_string(),
+            key: update.location().key,
+            disposition: FixDisposition::Unsafe,
+            patches: vec![Patch {
+                route: update
+                    .location()
+                    .route
+                    .with_keys(["insecure-external-code-execution".into()]),
+                operation: Op::Replace(serde_yaml::Value::String("deny".to_string())),
+            }],
+        }
+    }
+}
+
 impl Audit for DependabotExecution {
     fn new(_state: &crate::state::AuditState) -> Result<Self, super::AuditLoadError>
     where
@@ -42,6 +61,7 @@ impl Audit for DependabotExecution {
                                 .annotated("enabled here"),
                         )
                         .add_location(update.location_with_name())
+                        .fix(Self::create_set_deny_fix(update))
                         .build(dependabot)?,
                 );
             }
@@ -50,3 +70,190 @@ impl Audit for DependabotExecution {
         Ok(findings)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{
+        config::Config,
+        models::{AsDocument, dependabot::Dependabot},
+        registry::input::InputKey,
+        state::AuditState,
+    };
+
+    /// Macro for testing dependabot audits with common boilerplate
+    macro_rules! test_dependabot_audit {
+        ($audit_type:ty, $filename:expr, $dependabot_content:expr, $test_fn:expr) => {{
+            let key = InputKey::local("fakegroup".into(), $filename, None::<&str>).unwrap();
+            let dependabot = Dependabot::from_string($dependabot_content.to_string(), key).unwrap();
+            let audit_state = AuditState::default();
+            let audit = <$audit_type>::new(&audit_state).unwrap();
+            let findings = audit
+                .audit_dependabot(&dependabot, &Config::default())
+                .unwrap();
+
+            $test_fn(&dependabot, findings)
+        }};
+    }
+
+    #[test]
+    fn test_fix_allow_to_deny() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+    insecure-external-code-execution: allow
+"#;
+
+        test_dependabot_audit!(
+            DependabotExecution,
+            "test_fix_allow_to_deny.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert!(!findings.is_empty(), "Expected findings but got none");
+                let finding = &findings[0];
+                assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+
+                let fix = &finding.fixes[0];
+                let fixed_document = fix.apply(dependabot.as_document()).unwrap();
+                insta::assert_snapshot!(fixed_document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_no_fix_needed_for_deny() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+    insecure-external-code-execution: deny
+"#;
+
+        test_dependabot_audit!(
+            DependabotExecution,
+            "test_no_fix_needed_for_deny.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert_eq!(findings.len(), 0, "Expected no findings");
+
+                // Verify the document remains unchanged
+                insta::assert_snapshot!(dependabot.as_document().source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_no_fix_needed_when_omitted() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+"#;
+
+        test_dependabot_audit!(
+            DependabotExecution,
+            "test_no_fix_needed_when_omitted.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert_eq!(findings.len(), 0, "Expected no findings");
+
+                // Verify the document remains unchanged
+                insta::assert_snapshot!(dependabot.as_document().source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                ");
+            }
+        );
+    }
+
+    #[test]
+    fn test_fix_multiple_updates() {
+        let dependabot_content = r#"
+version: 2
+
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: daily
+    insecure-external-code-execution: allow
+
+  - package-ecosystem: npm
+    directory: /
+    schedule:
+      interval: weekly
+    insecure-external-code-execution: allow
+"#;
+
+        test_dependabot_audit!(
+            DependabotExecution,
+            "test_fix_multiple_updates.yml",
+            dependabot_content,
+            |dependabot: &Dependabot, findings: Vec<crate::finding::Finding>| {
+                assert_eq!(findings.len(), 2, "Expected 2 findings");
+
+                // Apply all fixes and snapshot the result
+                let mut document = dependabot.as_document().clone();
+                for finding in &findings {
+                    assert!(!finding.fixes.is_empty(), "Expected fixes but got none");
+                    for fix in &finding.fixes {
+                        document = fix.apply(&document).unwrap();
+                    }
+                }
+
+                insta::assert_snapshot!(document.source(), @r"
+                version: 2
+
+                updates:
+                  - package-ecosystem: pip
+                    directory: /
+                    schedule:
+                      interval: daily
+                    insecure-external-code-execution: deny
+
+                  - package-ecosystem: npm
+                    directory: /
+                    schedule:
+                      interval: weekly
+                    insecure-external-code-execution: deny
+                ");
+            }
+        );
+    }
+}

crates/zizmor/tests/integration/snapshot.rs

@@ -979,8 +979,9 @@ fn dependabot_execution() -> Result<()> {
        |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ enabled here
        |
        = note: audit confidence → High
+       = note: this finding has an auto-fix
 
-    1 finding: 0 informational, 0 low, 0 medium, 1 high
+    1 findings (1 fixable): 0 informational, 0 low, 0 medium, 1 high
     "
     );
 
@@ -1003,8 +1004,9 @@ fn dependabot_cooldown() -> Result<()> {
       |     ^^^^^^^^^^^^^^^^^^^^^^ missing cooldown configuration
       |
       = note: audit confidence → High
+      = note: this finding has an auto-fix
 
-    1 finding: 0 informational, 0 low, 1 medium, 0 high
+    1 findings (1 fixable): 0 informational, 0 low, 1 medium, 0 high
     "
     );
 
@@ -1022,8 +1024,9 @@ fn dependabot_cooldown() -> Result<()> {
       |     ^^^^^^^^^^^^ no default-days configured
       |
       = note: audit confidence → High
+      = note: this finding has an auto-fix
 
-    1 finding: 0 informational, 0 low, 1 medium, 0 high
+    1 findings (1 fixable): 0 informational, 0 low, 1 medium, 0 high
     ");
 
     insta::assert_snapshot!(
@@ -1040,8 +1043,9 @@ fn dependabot_cooldown() -> Result<()> {
       |       ^^^^^^^^^^^^^^^ insufficient default-days configured
       |
       = note: audit confidence → Medium
+      = note: this finding has an auto-fix
 
-    1 finding: 0 informational, 1 low, 0 medium, 0 high
+    1 findings (1 fixable): 0 informational, 1 low, 0 medium, 0 high
     ");
 
     Ok(())

docs/audits.md

@@ -377,7 +377,7 @@ Some general pointers:
 
 | Type     | Examples                | Introduced in | Works offline  | Auto-fixes available | Configurable |
 |----------|-------------------------|---------------|----------------|--------------------| ---------------|
-| Dependabot  | [dependabot-cooldown/]       | v1.15.0       | ✅             | ❌                 | ❌  |
+| Dependabot  | [dependabot-cooldown/]       | v1.15.0       | ✅             | ✅                 | ❌  |
 
 [dependabot-cooldown/]: https://github.com/zizmorcore/zizmor/blob/main/crates/zizmor/tests/integration/test-data/dependabot-cooldown/
 
@@ -443,7 +443,7 @@ enforces the following minimums:
 
 | Type     | Examples                | Introduced in | Works offline  | Auto-fixes available | Configurable |
 |----------|-------------------------|---------------|----------------|--------------------| ---------------|
-| Dependabot  | [dependabot-execution/]       | v1.15.0       | ✅             | ❌                | ❌  |
+| Dependabot  | [dependabot-execution/]       | v1.15.0       | ✅             | ✅                | ❌  |
 
 [dependabot-execution/]: https://github.com/zizmorcore/zizmor/blob/main/crates/zizmor/tests/integration/test-data/dependabot-execution/
 

docs/release-notes.md

@@ -32,6 +32,16 @@ of `zizmor`.
 
     Many thanks to @cnaples79 for implementing this improvement!
 
+* The [dependabot-cooldown] audit now supports auto-fixes for many findings
+  (#1229)
+
+    Many thanks to @mostafa for implementing this improvement!
+
+* The [dependabot-execution] audit now supports auto-fixes for many findings
+  (#1229)
+
+    Many thanks to @mostafa for implementing this improvement!
+
 ## 1.15.2
 
 ### Bug Fixes 🐛