feat: downgrade artipacked severity for checkout@v6+

crates/zizmor/src/audit/artipacked.rs

@@ -1,17 +1,25 @@
+use std::sync::LazyLock;
+
 use github_actions_models::common::{EnvValue, Uses, expr::ExplicitExpr};
 use itertools::Itertools as _;
 
 use super::{Audit, AuditLoadError, audit_meta};
 use crate::{
     audit::AuditError,
     finding::{Confidence, Finding, Fix, Persona, Severity, location::Routable as _},
-    models::{StepBodyCommon, StepCommon, uses::RepositoryUsesExt as _},
+    github::{Client, ClientError},
+    models::{StepBodyCommon, StepCommon, uses::RepositoryUsesExt, version::Version},
     state::AuditState,
     utils::split_patterns,
 };
 use yamlpatch::{Op, Patch};
 
-pub(crate) struct Artipacked;
+#[allow(clippy::unwrap_used)]
+static V6: LazyLock<Version> = LazyLock::new(|| Version::parse("v6").unwrap());
+
+pub(crate) struct Artipacked {
+    client: Option<Client>,
+}
 
 audit_meta!(
     Artipacked,
@@ -20,7 +28,68 @@ audit_meta!(
 );
 
 impl Artipacked {
-    fn process_steps<'doc>(
+    /// Determines if an `actions/checkout` usage is version 6 or higher.
+    ///
+    /// This takes two different paths:
+    /// 1. If the ref is a symbolic ref (tag/branch), we use it for a direct version comparison.
+    /// 2. If the ref is a commit SHA *and* we have a GitHub client, we match the commit
+    ///    to its longest tag. We then use that tag for the version comparison.
+    ///
+    /// If we can't determine the version (e.g., commit SHA without client),
+    /// we return `None`.
+    async fn is_checkout_v6_or_higher(
+        &self,
+        uses: &github_actions_models::common::RepositoryUses,
+    ) -> Result<Option<bool>, ClientError> {
+        let version = if !uses.ref_is_commit() {
+            uses.git_ref.clone()
+        } else {
+            match self.client {
+                Some(ref client) => {
+                    let tag = client
+                        .longest_tag_for_commit(&uses.owner, &uses.repo, &uses.git_ref)
+                        .await?;
+
+                    match tag {
+                        Some(tag) => tag.name,
+                        None => return Ok(None),
+                    }
+                }
+                None => return Ok(None),
+            }
+        };
+
+        // Try to parse the ref as a version
+        let Ok(version) = Version::parse(&version) else {
+            // If we can't parse it as a version, assume it's not v6+
+            return Ok(None);
+        };
+
+        Ok(Some(version >= *V6))
+    }
+
+    /// Determine the severity for an artipacked finding based on checkout version
+    /// and whether there are vulnerable uploads.
+    fn determine_severity(
+        is_v6_or_higher: Option<bool>,
+        has_no_vulnerable_uploads: bool,
+    ) -> Severity {
+        if is_v6_or_higher == Some(true) {
+            // For checkout@v6+, downgrade severity since credentials are stored
+            // in $RUNNER_TEMP instead of .git/config, reducing leakage risk.
+            if has_no_vulnerable_uploads {
+                Severity::Low
+            } else {
+                Severity::Medium
+            }
+        } else if has_no_vulnerable_uploads {
+            Severity::Medium
+        } else {
+            Severity::High
+        }
+    }
+
+    async fn process_steps<'doc>(
         &self,
         steps: impl Iterator<Item = impl StepCommon<'doc>>,
     ) -> Result<Vec<Finding<'doc>>, AuditError> {
@@ -39,6 +108,10 @@ impl Artipacked {
             };
 
             if uses.matches("actions/checkout") {
+                let is_v6_or_higher = self
+                    .is_checkout_v6_or_higher(uses)
+                    .await
+                    .map_err(Self::err)?;
                 match with
                     .get("persist-credentials")
                     .map(|v| v.to_string())
@@ -48,11 +121,11 @@ impl Artipacked {
                     Some("true") => {
                         // If a user explicitly sets `persist-credentials: true`,
                         // they probably mean it. Only report if in auditor mode.
-                        vulnerable_checkouts.push((step, Persona::Auditor))
+                        vulnerable_checkouts.push((step, Persona::Auditor, is_v6_or_higher))
                     }
                     // TODO: handle expressions here.
                     // persist-credentials is true by default.
-                    _ => vulnerable_checkouts.push((step, Persona::default())),
+                    _ => vulnerable_checkouts.push((step, Persona::default(), is_v6_or_higher)),
                 }
             } else if uses.matches("actions/upload-artifact") {
                 let Some(EnvValue::String(path)) = with.get("path") else {
@@ -70,10 +143,13 @@ impl Artipacked {
         if vulnerable_uploads.is_empty() {
             // If we have no vulnerable uploads, then emit lower-confidence
             // findings for just the checkout steps.
-            for (checkout, persona) in &vulnerable_checkouts {
+            for (checkout, persona, is_v6_or_higher) in &vulnerable_checkouts {
+                let severity =
+                    Self::determine_severity(*is_v6_or_higher, vulnerable_uploads.is_empty());
+
                 findings.push(
                     Self::finding()
-                        .severity(Severity::Medium)
+                        .severity(severity)
                         .confidence(Confidence::Low)
                         .persona(*persona)
                         .add_location(
@@ -90,14 +166,17 @@ impl Artipacked {
             // Select only pairs where the vulnerable checkout precedes the
             // vulnerable upload. There are more efficient ways to do this than
             // a cartesian product, but this way is simple.
-            for ((checkout, persona), upload) in vulnerable_checkouts
+            for ((checkout, persona, is_v6_or_higher), upload) in vulnerable_checkouts
                 .iter()
                 .cartesian_product(vulnerable_uploads.iter())
             {
                 if checkout.index() < upload.index() {
+                    let severity =
+                        Self::determine_severity(*is_v6_or_higher, vulnerable_uploads.is_empty());
+
                     findings.push(
                         Self::finding()
-                            .severity(Severity::High)
+                            .severity(severity)
                             .confidence(Confidence::High)
                             .persona(*persona)
                             .add_location(
@@ -163,8 +242,10 @@ impl Artipacked {
 
 #[async_trait::async_trait]
 impl Audit for Artipacked {
-    fn new(_state: &AuditState) -> Result<Self, AuditLoadError> {
-        Ok(Self)
+    fn new(state: &AuditState) -> Result<Self, AuditLoadError> {
+        Ok(Self {
+            client: state.gh_client.clone(),
+        })
     }
 
     async fn audit_action<'doc>(
@@ -176,20 +257,24 @@ impl Audit for Artipacked {
             return Ok(vec![]);
         };
 
-        self.process_steps(steps)
+        self.process_steps(steps).await
     }
 
     async fn audit_normal_job<'doc>(
         &self,
         job: &super::NormalJob<'doc>,
         _config: &crate::config::Config,
     ) -> Result<Vec<Finding<'doc>>, AuditError> {
-        self.process_steps(job.steps())
+        self.process_steps(job.steps()).await
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use std::str::FromStr;
+
+    use github_actions_models::common::RepositoryUses;
+
     use super::*;
     use crate::{
         config::Config,
@@ -237,6 +322,164 @@ mod tests {
         fix.apply(document).unwrap()
     }
 
+    #[tokio::test]
+    async fn test_is_checkout_v6_or_higher_offline() {
+        // Test v6 and higher versions
+        let v6 = RepositoryUses::from_str("actions/checkout@v6").unwrap();
+        let v6_0 = RepositoryUses::from_str("actions/checkout@v6.0").unwrap();
+        let v6_1_0 = RepositoryUses::from_str("actions/checkout@v6.1.0").unwrap();
+        let v7 = RepositoryUses::from_str("actions/checkout@v7").unwrap();
+        let v10 = RepositoryUses::from_str("actions/checkout@v10").unwrap();
+
+        let artipacked = Artipacked { client: None };
+
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v6).await.unwrap(),
+            Some(true)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v6_0).await.unwrap(),
+            Some(true)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v6_1_0).await.unwrap(),
+            Some(true)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v7).await.unwrap(),
+            Some(true)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v10).await.unwrap(),
+            Some(true)
+        );
+
+        // Test versions below v6
+        let v4 = RepositoryUses::from_str("actions/checkout@v4").unwrap();
+        let v5 = RepositoryUses::from_str("actions/checkout@v5").unwrap();
+        let v5_9 = RepositoryUses::from_str("actions/checkout@v5.9").unwrap();
+
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v4).await.unwrap(),
+            Some(false)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v5).await.unwrap(),
+            Some(false)
+        );
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&v5_9).await.unwrap(),
+            Some(false)
+        );
+
+        // Test commit SHA (should return None when offline)
+        let commit_sha =
+            RepositoryUses::from_str("actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683")
+                .unwrap();
+        assert_eq!(
+            artipacked
+                .is_checkout_v6_or_higher(&commit_sha)
+                .await
+                .unwrap(),
+            None
+        );
+
+        // Test invalid/unparseable refs (should return None)
+        let invalid = RepositoryUses::from_str("actions/checkout@main").unwrap();
+        assert_eq!(
+            artipacked.is_checkout_v6_or_higher(&invalid).await.unwrap(),
+            None
+        );
+    }
+
+    #[cfg(feature = "online-tests")]
+    #[tokio::test]
+    async fn test_is_checkout_v6_or_higher_online() {
+        use crate::github;
+
+        let artipacked = Artipacked {
+            client: Some(
+                Client::new(
+                    &github::GitHubHost::default(),
+                    &github::GitHubToken::new(&std::env::var("GH_TOKEN").unwrap()).unwrap(),
+                    "/tmp".into(),
+                )
+                .unwrap(),
+            ),
+        };
+
+        // Points to v6.0.0.
+        let commit_sha_v6 =
+            RepositoryUses::from_str("actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3")
+                .unwrap();
+
+        assert_eq!(
+            artipacked
+                .is_checkout_v6_or_higher(&commit_sha_v6)
+                .await
+                .unwrap(),
+            Some(true)
+        );
+
+        // Points to v5.0.1.
+        let commit_sha_v5 =
+            RepositoryUses::from_str("actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd")
+                .unwrap();
+
+        assert_eq!(
+            artipacked
+                .is_checkout_v6_or_higher(&commit_sha_v5)
+                .await
+                .unwrap(),
+            Some(false)
+        );
+    }
+
+    #[test]
+    fn test_determine_severity() {
+        const IS_V6_OR_HIGHER: Option<bool> = Some(true);
+        const IS_OLDER_VERSION: Option<bool> = Some(false);
+        const UNKNOWN_VERSION: Option<bool> = None;
+        const HAS_NO_VULNERABLE_UPLOADS: bool = true;
+        const HAS_VULNERABLE_UPLOADS: bool = false;
+
+        // checkout@v6+ with no vulnerable uploads -> Low
+        assert_eq!(
+            Artipacked::determine_severity(IS_V6_OR_HIGHER, HAS_NO_VULNERABLE_UPLOADS),
+            Severity::Low
+        );
+
+        // checkout@v6+ with vulnerable uploads -> Medium
+        assert_eq!(
+            Artipacked::determine_severity(IS_V6_OR_HIGHER, HAS_VULNERABLE_UPLOADS),
+            Severity::Medium
+        );
+
+        // Older checkout versions with no vulnerable uploads -> Medium
+        assert_eq!(
+            Artipacked::determine_severity(IS_OLDER_VERSION, HAS_NO_VULNERABLE_UPLOADS),
+            Severity::Medium
+        );
+
+        // Older checkout versions with vulnerable uploads -> High
+        assert_eq!(
+            Artipacked::determine_severity(IS_OLDER_VERSION, HAS_VULNERABLE_UPLOADS),
+            Severity::High
+        );
+
+        // Unknown version (None) with no vulnerable uploads -> Medium (treated as older)
+        assert_eq!(
+            Artipacked::determine_severity(UNKNOWN_VERSION, HAS_NO_VULNERABLE_UPLOADS),
+            Severity::Medium
+        );
+
+        // Unknown version (None) with vulnerable uploads -> High (treated as older)
+        assert_eq!(
+            Artipacked::determine_severity(UNKNOWN_VERSION, HAS_VULNERABLE_UPLOADS),
+            Severity::High
+        );
+    }
+
     #[test]
     fn test_fix_title_and_description() {
         // Test that the fix has the expected title and description format

docs/audits.md

@@ -71,15 +71,24 @@ Add a `name:` field to your workflow or action.
 Detects local filesystem `git` credential storage on GitHub Actions, as well as
 potential avenues for unintentional persistence of credentials in artifacts.
 
-By default, using @actions/checkout causes a credential to be persisted
-in the checked-out repo's `.git/config`, so that subsequent `git` operations
-can be authenticated.
+By default, using @actions/checkout causes a credential to be persisted on disk.
+Versions below v6.0.0 store the credential directly in the checked-out repo's
+`.git/config`, while v6.0.0 and later store it under `$RUNNER_TEMP`.
 
-Subsequent steps may accidentally publicly persist `.git/config`, e.g. by
+Subsequent steps may accidentally publicly persist the credential, e.g. by
 including it in a publicly accessible artifact via @actions/upload-artifact.
 
-However, even without this, persisting the credential in the `.git/config`
-is non-ideal unless actually needed.
+However, even without this, persisting the credential on disk is non-ideal
+unless actually needed.
+
+!!! note "Behavior change"
+
+    Starting with zizmor v1.17.0, this audit produces lower-severity findings
+    when v6.0.0 or higher of @actions/checkout is used. This reflects a
+    change in v6.0.0's credential persistence behavior towards a more
+    misuse-resistant location.
+
+    See orgs/community?179107 for additional information.
 
 Other resources: