Repository: woodruffw/zizmor (rust)

Issue:
Feature: specific references in forbidden-uses
### Pre-submission checks

- [x] I am **not** reporting a bug (crash, false positive/negative, etc). These must be filed via the bug report template.
- [x] I have looked through the open issues for a duplicate request.

### What's the problem this feature will solve?

Developers might update an action or add a new action without vetting that it's trustworthy.

### Describe the solution you'd like

Having an allow-list that includes the concrete SHA would help here.

### Additional context

_No response_

Pull request description:
feat: extend RepositoryUsesPattern to allow exact ref patterns
Closes #676.

Patch:
diff --git a/docs/release-notes.md b/docs/release-notes.md
index 4c6894b..976b969 100644
--- a/docs/release-notes.md
+++ b/docs/release-notes.md
@@ -41,6 +41,13 @@ of `zizmor`.
   which was to terminate the audit if any collected input could
   not be parsed (#734)
 
+### Enhancements 🌱
+
+* The [forbidden-uses] audit can now be configured with patterns that
+  match exact `uses:` clauses, including refs. For example,
+  exactly `actions/checkout@v4` can now be explicitly allowed or forbidden,
+  rather than every ref that matches `actions/checkout` (#750)
+
 ### Bug Fixes 🐛
 
 * The SARIF output format now uses `zizmor/{id}` for rule IDs instead
diff --git a/src/audit/unpinned_uses.rs b/src/audit/unpinned_uses.rs
index ddfd2e8..0d784d2 100644
--- a/src/audit/unpinned_uses.rs
+++ b/src/audit/unpinned_uses.rs
@@ -64,6 +64,8 @@ impl UnpinnedUses {
                     }) => {
                         format!("{owner}/{repo}/{subpath}")
                     }
+                    // Not allowed in this audit.
+                    Some(RepositoryUsesPattern::ExactWithRef { .. }) => unreachable!(),
                 };
 
                 match policy {
@@ -127,9 +129,11 @@ impl Audit for UnpinnedUses {
             .map_err(AuditLoadError::Fail)?
             .unwrap_or_default();
 
-        Ok(Self {
-            policies: config.into(),
-        })
+        let policies = UnpinnedUsesPolicies::try_from(config)
+            .context("invalid configuration")
+            .map_err(AuditLoadError::Fail)?;
+
+        Ok(Self { policies })
     }
 
     fn audit_step<'doc>(&self, step: &Step<'doc>) -> anyhow::Result<Vec<Finding<'doc>>> {
@@ -239,14 +243,21 @@ impl UnpinnedUsesPolicies {
     }
 }
 
-impl From<UnpinnedUsesConfig> for UnpinnedUsesPolicies {
-    fn from(config: UnpinnedUsesConfig) -> Self {
+impl TryFrom<UnpinnedUsesConfig> for UnpinnedUsesPolicies {
+    type Error = anyhow::Error;
+
+    fn try_from(config: UnpinnedUsesConfig) -> Result<Self, Self::Error> {
         let mut policy_tree: HashMap<String, Vec<(RepositoryUsesPattern, UsesPolicy)>> =
             HashMap::new();
         let mut default_policy = UsesPolicy::HashPin;
 
         for (pattern, policy) in config.policies {
             match pattern {
+                // Patterns with refs don't make sense in this context, since
+                // we're establishing policies for the refs themselves.
+                RepositoryUsesPattern::ExactWithRef { .. } => {
+                    return Err(anyhow::anyhow!("can't use exact ref patterns here"));
+                }
                 RepositoryUsesPattern::ExactPath { ref owner, .. } => {
                     policy_tree
                         .entry(owner.clone())
@@ -282,9 +293,9 @@ impl From<UnpinnedUsesConfig> for UnpinnedUsesPolicies {
             policies.sort_by(|a, b| a.0.cmp(&b.0));
         }
 
-        Self {
+        Ok(Self {
             policy_tree,
             default_policy,
-        }
+        })
     }
 }
diff --git a/src/models/uses.rs b/src/models/uses.rs
index f6f39d5..5e6382b 100644
--- a/src/models/uses.rs
+++ b/src/models/uses.rs
@@ -7,14 +7,43 @@ use regex::Regex;
 use serde::Deserialize;
 
 /// Matches all variants of [`RepositoryUsesPattern`] except `*`.
-static REPOSITORY_USES_PATTERN: LazyLock<Regex> =
-    LazyLock::new(|| Regex::new(r#"(?mi)^([\w-]+)/([\w\.-]+|\*)(?:/(.+))?$"#).unwrap());
+static REPOSITORY_USES_PATTERN: LazyLock<Regex> = LazyLock::new(|| {
+    Regex::new(
+        r#"(?xmi)                   # verbose, multi-line mode, case-insensitive
+        ^                           # start of line
+        ([\w-]+)                    # (1) owner
+        /                           # /
+        ([\w\.-]+|\*)               # (2) repo or *
+        (?:                         # non-capturing group for optional subpath
+          /                         # /
+          (                         # (3) subpath
+            [[[:graph:]]&&[^@\*]]+  # any non-space, non-@, non-* characters
+            |                       # OR
+            \*                      # *
+          )                         # end of (3) subpath
+        )?                          # end of non-capturing group for optional subpath
+        (?:                         # non-capturing group for optional git ref
+          @                         # @
+          ([[[:graph:]]&&[^\*]]+)   # (4) git ref (any non-space, non-* characters)
+        )?                          # end of non-capturing group for optional git ref
+        $                           # end of line
+        "#,
+    )
+    .unwrap()
+});
 
 /// Represents a pattern for matching repository `uses` references.
 /// These patterns are ordered by specificity; more specific patterns
 /// should be listed first.
 #[derive(Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub(crate) enum RepositoryUsesPattern {
+    /// Matches exactly `owner/repo/subpath@ref`.
+    ExactWithRef {
+        owner: String,
+        repo: String,
+        subpath: Option<String>,
+        git_ref: String,
+    },
     /// Matches exactly `owner/repo/subpath`.
     ExactPath {
         owner: String,
@@ -35,6 +64,17 @@ pub(crate) enum RepositoryUsesPattern {
 impl RepositoryUsesPattern {
     pub(crate) fn matches(&self, uses: &RepositoryUses) -> bool {
         match self {
+            RepositoryUsesPattern::ExactWithRef {
+                owner,
+                repo,
+                subpath,
+                git_ref,
+            } => {
+                uses.owner.eq_ignore_ascii_case(owner)
+                    && uses.repo.eq_ignore_ascii_case(repo)
+                    && uses.subpath == *subpath
+                    && uses.git_ref.as_deref().is_some_and(|s| s == git_ref)
+            }
             RepositoryUsesPattern::ExactPath {
                 owner,
                 repo,
@@ -79,23 +119,31 @@ impl FromStr for RepositoryUsesPattern {
         let owner = &caps[1];
         let repo = &caps[2];
         let subpath = caps.get(3).map(|m| m.as_str());
+        let git_ref = caps.get(4).map(|m| m.as_str());
 
-        match (owner, repo, subpath) {
-            (owner, "*", None) => Ok(RepositoryUsesPattern::InOwner(owner.into())),
-            (owner, repo, None) => Ok(RepositoryUsesPattern::ExactRepo {
+        match (owner, repo, subpath, git_ref) {
+            (owner, "*", None, None) => Ok(RepositoryUsesPattern::InOwner(owner.into())),
+            (owner, repo, None, None) => Ok(RepositoryUsesPattern::ExactRepo {
                 owner: owner.into(),
                 repo: repo.into(),
             }),
-            (_, "*", Some(_)) => Err(anyhow::anyhow!("invalid pattern: {s}")),
-            (owner, repo, Some("*")) => Ok(RepositoryUsesPattern::InRepo {
+            (_, "*", Some(_), _) => Err(anyhow::anyhow!("invalid pattern: {s}")),
+            (owner, repo, Some("*"), None) => Ok(RepositoryUsesPattern::InRepo {
                 owner: owner.into(),
                 repo: repo.into(),
             }),
-            (owner, repo, Some(subpath)) => Ok(RepositoryUsesPattern::ExactPath {
+            (owner, repo, Some(subpath), None) => Ok(RepositoryUsesPattern::ExactPath {
                 owner: owner.into(),
                 repo: repo.into(),
                 subpath: subpath.into(),
             }),
+            (_, _, Some("*"), Some(_)) => Err(anyhow::anyhow!("invalid pattern: {s}")),
+            (owner, repo, subpath, Some(git_ref)) => Ok(RepositoryUsesPattern::ExactWithRef {
+                owner: owner.into(),
+                repo: repo.into(),
+                subpath: subpath.map(|s| s.into()),
+                git_ref: git_ref.into(),
+            }),
         }
     }
 }
@@ -267,16 +315,18 @@ mod tests {
             ("/", None),     // Invalid, not well formed
             ("//", None),    // Invalid, not well formed
             ("///", None),   // Invalid, not well formed
-            ("owner", None), // Invalid, should be owner/*
+            ("owner", None), // Invalid, sho,uld be owner/*
             ("**", None),    // Invalid, should be *
             ("*", Some(RepositoryUsesPattern::Any)),
             (
                 "owner/*",
                 Some(RepositoryUsesPattern::InOwner("owner".into())),
             ),
-            ("owner/*/", None),    // Invalid, should be owner/*
-            ("owner/*/foo", None), // Invalid, not well formed
-            ("owner/*/*", None),   // Invalid, not well formed
+            ("owner/*/", None),      // Invalid, should be owner/*
+            ("owner/*/foo", None),   // Invalid, not well formed
+            ("owner/*/*", None),     // Invalid, not well formed
+            ("*/foo", None),         // Invalid, not well formed
+            ("owner/repo/**", None), // Invalid, not well formed.
             (
                 "owner/repo/*",
                 Some(RepositoryUsesPattern::InRepo {
@@ -308,31 +358,70 @@ mod tests {
                     subpath: "/".into(),
                 }),
             ),
-            // Weird, but we allow it (for now).
             (
-                "owner/repo/**",
+                "owner/repo/subpath/",
                 Some(RepositoryUsesPattern::ExactPath {
                     owner: "owner".into(),
                     repo: "repo".into(),
-                    subpath: "**".into(),
+                    subpath: "subpath/".into(),
                 }),
             ),
             (
-                "owner/repo/subpath/",
+                "owner/repo/subpath/very/nested////and/literal",
                 Some(RepositoryUsesPattern::ExactPath {
                     owner: "owner".into(),
                     repo: "repo".into(),
-                    subpath: "subpath/".into(),
+                    subpath: "subpath/very/nested////and/literal".into(),
                 }),
             ),
             (
-                "owner/repo/subpath/very/nested////and/literal",
-                Some(RepositoryUsesPattern::ExactPath {
+                "owner/repo@v1",
+                Some(RepositoryUsesPattern::ExactWithRef {
                     owner: "owner".into(),
                     repo: "repo".into(),
-                    subpath: "subpath/very/nested////and/literal".into(),
+                    subpath: None,
+                    git_ref: "v1".into(),
+                }),
+            ),
+            (
+                "owner/repo/subpath@v1",
+                Some(RepositoryUsesPattern::ExactWithRef {
+                    owner: "owner".into(),
+                    repo: "repo".into(),
+                    subpath: Some("subpath".into()),
+                    git_ref: "v1".into(),
                 }),
             ),
+            (
+                "owner/repo@172239021f7ba04fe7327647b213799853a9eb89",
+                Some(RepositoryUsesPattern::ExactWithRef {
+                    owner: "owner".into(),
+                    repo: "repo".into(),
+                    subpath: None,
+                    git_ref: "172239021f7ba04fe7327647b213799853a9eb89".into(),
+                }),
+            ),
+            (
+                "pypa/gh-action-pypi-publish@release/v1",
+                Some(RepositoryUsesPattern::ExactWithRef {
+                    owner: "pypa".into(),
+                    repo: "gh-action-pypi-publish".into(),
+                    subpath: None,
+                    git_ref: "release/v1".into(),
+                }),
+            ),
+            // Invalid: no wildcards allowed when refs are present.
+            ("owner/repo/*@v1", None),
+            ("owner/repo/*/subpath@v1", None),
+            ("owner/*/subpath@v1", None),
+            ("*/*/subpath@v1", None),
+            // Ref also cannot be a wildcard.
+            ("owner/repo@*", None),
+            ("owner/repo@**", None),
+            ("owner/repo@***", None),
+            ("owner/repo/subpath@*", None),
+            ("owner/*@*", None),
+            ("*@*", None),
         ] {
             let pattern = RepositoryUsesPattern::from_str(pattern).ok();
             assert_eq!(pattern, expected);
@@ -410,6 +499,10 @@ mod tests {
             ("actions/checkout@v3", "*", true),
             ("actions/checkout/foo@v3", "*", true),
             ("whatever/checkout", "*", true),
+            // exact matches
+            ("actions/checkout@v3", "actions/checkout@v3", true),
+            ("actions/checkout/foo@v3", "actions/checkout/foo@v3", true),
+            ("actions/checkout/foo", "actions/checkout/foo@v3", false),
         ] {
             let Ok(Uses::Repository(uses)) = Uses::from_str(uses) else {
                 return Err(anyhow!("invalid uses: {uses}"));


Test patch:
diff --git a/tests/integration/snapshot.rs b/tests/integration/snapshot.rs
index 5ce3c69..91f3c47 100644
--- a/tests/integration/snapshot.rs
+++ b/tests/integration/snapshot.rs
@@ -226,6 +226,7 @@ fn unpinned_uses() -> Result<()> {
         "invalid-policy-syntax-3",
         "invalid-policy-syntax-4",
         "invalid-policy-syntax-5",
+        "invalid-policy-syntax-6",
     ] {
         insta::assert_snapshot!(
             zizmor()
@@ -649,7 +650,14 @@ fn unredacted_secrets() -> Result<()> {
 
 #[test]
 fn forbidden_uses() -> Result<()> {
-    for config in ["allow-all", "deny-all", "allow-some", "deny-some"] {
+    for config in [
+        "allow-all",
+        "deny-all",
+        "allow-some",
+        "deny-some",
+        "deny-some-refs",
+        "allow-some-refs",
+    ] {
         insta::assert_snapshot!(
             zizmor()
                 .config(input_under_test(&format!(
diff --git a/tests/integration/snapshots/integration__snapshot__forbidden_uses-5.snap b/tests/integration/snapshots/integration__snapshot__forbidden_uses-5.snap
new file mode 100644
index 0000000..dcf0a3b
--- /dev/null
+++ b/tests/integration/snapshots/integration__snapshot__forbidden_uses-5.snap
@@ -0,0 +1,22 @@
+---
+source: tests/integration/snapshot.rs
+expression: "zizmor().config(input_under_test(&format!(\"forbidden-uses/configs/{config}.yml\"))).input(input_under_test(\"forbidden-uses/forbidden-uses-menagerie.yml\")).run()?"
+snapshot_kind: text
+---
+error[forbidden-uses]: forbidden action used
+  --> @@INPUT@@:12:9
+   |
+12 |       - uses: actions/setup-python@v4
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of this action is forbidden
+   |
+   = note: audit confidence → High
+
+error[forbidden-uses]: forbidden action used
+  --> @@INPUT@@:13:9
+   |
+13 |       - uses: pypa/gh-action-pypi-publish@release/v1
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of this action is forbidden
+   |
+   = note: audit confidence → High
+
+2 findings: 0 unknown, 0 informational, 0 low, 0 medium, 2 high
diff --git a/tests/integration/snapshots/integration__snapshot__forbidden_uses-6.snap b/tests/integration/snapshots/integration__snapshot__forbidden_uses-6.snap
new file mode 100644
index 0000000..d05b399
--- /dev/null
+++ b/tests/integration/snapshots/integration__snapshot__forbidden_uses-6.snap
@@ -0,0 +1,14 @@
+---
+source: tests/integration/snapshot.rs
+expression: "zizmor().config(input_under_test(&format!(\"forbidden-uses/configs/{config}.yml\"))).input(input_under_test(\"forbidden-uses/forbidden-uses-menagerie.yml\")).run()?"
+snapshot_kind: text
+---
+error[forbidden-uses]: forbidden action used
+  --> @@INPUT@@:14:9
+   |
+14 |       - uses: actions/checkout@v4
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ use of this action is forbidden
+   |
+   = note: audit confidence → High
+
+1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
diff --git a/tests/integration/snapshots/integration__snapshot__unpinned_uses-12.snap b/tests/integration/snapshots/integration__snapshot__unpinned_uses-12.snap
new file mode 100644
index 0000000..f0955bb
--- /dev/null
+++ b/tests/integration/snapshots/integration__snapshot__unpinned_uses-12.snap
@@ -0,0 +1,12 @@
+---
+source: tests/integration/snapshot.rs
+expression: "zizmor().expects_failure(true).config(input_under_test(&format!(\"unpinned-uses/configs/{tc}.yml\",))).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
+snapshot_kind: text
+---
+ INFO zizmor: skipping impostor-commit: can't run without a GitHub API token
+ INFO zizmor: skipping ref-confusion: can't run without a GitHub API token
+ INFO zizmor: skipping known-vulnerable-actions: can't run without a GitHub API token
+fatal: no audit was performed
+error: failed to load audit: unpinned-uses
+ = note: invalid configuration: can't use exact ref patterns here
+ = note: see: https://woodruffw.github.io/zizmor/audits/#unpinned-uses
diff --git a/tests/integration/test-data/forbidden-uses/configs/allow-some-refs.yml b/tests/integration/test-data/forbidden-uses/configs/allow-some-refs.yml
new file mode 100644
index 0000000..0dc88a6
--- /dev/null
+++ b/tests/integration/test-data/forbidden-uses/configs/allow-some-refs.yml
@@ -0,0 +1,13 @@
+rules:
+  unpinned-uses:
+    config:
+      policies:
+        # just to make unrelated findings go away
+        "*": ref-pin
+
+  forbidden-uses:
+    config:
+      allow:
+        - actions/setup-python@v4
+        - actions/checkout@v3 # workflow actually uses v4
+        - pypa/gh-action-pypi-publish@release/v1
diff --git a/tests/integration/test-data/forbidden-uses/configs/deny-some-refs.yml b/tests/integration/test-data/forbidden-uses/configs/deny-some-refs.yml
new file mode 100644
index 0000000..ff72df4
--- /dev/null
+++ b/tests/integration/test-data/forbidden-uses/configs/deny-some-refs.yml
@@ -0,0 +1,13 @@
+rules:
+  unpinned-uses:
+    config:
+      policies:
+        # just to make unrelated findings go away
+        "*": ref-pin
+
+  forbidden-uses:
+    config:
+      deny:
+        - actions/setup-python@v4
+        - actions/checkout@v3 # workflow actually uses v4
+        - pypa/gh-action-pypi-publish@release/v1
diff --git a/tests/integration/test-data/unpinned-uses/configs/invalid-policy-syntax-6.yml b/tests/integration/test-data/unpinned-uses/configs/invalid-policy-syntax-6.yml
new file mode 100644
index 0000000..41a711a
--- /dev/null
+++ b/tests/integration/test-data/unpinned-uses/configs/invalid-policy-syntax-6.yml
@@ -0,0 +1,7 @@
+rules:
+  unpinned-uses:
+    config:
+      policies:
+        # valid pattern, but not allowed in this context
+        # because it makes no sense
+        "foo/bar@v1": "hash-pin"