Repository: woodruffw/zizmor
Pull request #364 (2024-12-26 17:50:07)

feat: adds unpinned-uses support to composite actions
Contributes to #350 

I tried to dedup a bit the inner logic for this audit by extracting at least one common function. I do think there is room for more deduplication, but I did not want to push hard on that in the first take. 

Let me know your thoughts 🙂

woodruffw: Looks good to me! Feel free to dedupe here or in a follow-up, whatever you prefer 🙂 

ubiratansoares: Follow-up then, so I have room to get more used to the new CompositeStep APIs 🙂. 

I'll take a self-note to revisit this one!

Patch:
diff --git a/docs/audits.md b/docs/audits.md
index 7db0831..4df4b9c 100644
--- a/docs/audits.md
+++ b/docs/audits.md
@@ -547,9 +547,9 @@ or @rubygems/release-gem for canonical examples of using it.
 
 ## `unpinned-uses`
 
-| Type     | Examples                     | Introduced in | Works offline  | Enabled by default |
-|----------|------------------------------|---------------|----------------|--------------------|
-| Workflow  | [unpinned.yml]              | v0.4.0        | ✅             | ✅                 |
+| Type             | Examples                     | Introduced in | Works offline  | Enabled by default |
+|------------------|------------------------------|---------------|----------------|--------------------|
+| Workflow, Action | [unpinned.yml]              | v0.4.0        | ✅             | ✅                 |
 
 [unpinned.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/unpinned.yml
 
diff --git a/src/audit/unpinned_uses.rs b/src/audit/unpinned_uses.rs
index d2f0c25..25e8e86 100644
--- a/src/audit/unpinned_uses.rs
+++ b/src/audit/unpinned_uses.rs
@@ -1,11 +1,31 @@
-use crate::finding::{Confidence, Persona, Severity};
-
 use super::{audit_meta, Audit, AuditState, Finding, Step};
+use crate::finding::{Confidence, Persona, Severity};
+use crate::models::{CompositeStep, Uses};
 
 pub(crate) struct UnpinnedUses;
 
 audit_meta!(UnpinnedUses, "unpinned-uses", "unpinned action reference");
 
+impl UnpinnedUses {
+    pub fn evaluate_pinning<'u>(&self, uses: &Uses) -> Option<(&'u str, Severity, Persona)> {
+        if uses.unpinned() {
+            Some((
+                "action is not pinned to a tag, branch, or hash ref",
+                Severity::Medium,
+                Persona::default(),
+            ))
+        } else if uses.unhashed() {
+            Some((
+                "action is not pinned to a hash ref",
+                Severity::Low,
+                Persona::Pedantic,
+            ))
+        } else {
+            None
+        }
+    }
+}
+
 impl Audit for UnpinnedUses {
     fn new(_state: AuditState) -> anyhow::Result<Self>
     where
@@ -21,19 +41,7 @@ impl Audit for UnpinnedUses {
             return Ok(vec![]);
         };
 
-        let (annotation, severity, persona) = if uses.unpinned() {
-            (
-                "action is not pinned to a tag, branch, or hash ref",
-                Severity::Medium,
-                Persona::default(),
-            )
-        } else if uses.unhashed() {
-            (
-                "action is not pinned to a hash ref",
-                Severity::Low,
-                Persona::Pedantic,
-            )
-        } else {
+        let Some((annotation, severity, persona)) = self.evaluate_pinning(&uses) else {
             return Ok(vec![]);
         };
 
@@ -53,4 +61,35 @@ impl Audit for UnpinnedUses {
 
         Ok(findings)
     }
+
+    fn audit_composite_step<'a>(
+        &self,
+        step: &CompositeStep<'a>,
+    ) -> anyhow::Result<Vec<Finding<'a>>> {
+        let mut findings = vec![];
+
+        let Some(uses) = step.uses() else {
+            return Ok(vec![]);
+        };
+
+        let Some((annotation, severity, persona)) = self.evaluate_pinning(&uses) else {
+            return Ok(vec![]);
+        };
+
+        findings.push(
+            Self::finding()
+                .confidence(Confidence::High)
+                .severity(severity)
+                .persona(persona)
+                .add_location(
+                    step.location()
+                        .primary()
+                        .with_keys(&["uses".into()])
+                        .annotated(annotation),
+                )
+                .build(step.action())?,
+        );
+
+        Ok(findings)
+    }
 }


Test patch:
diff --git a/tests/snapshot.rs b/tests/snapshot.rs
index b06544f..f993bf7 100644
--- a/tests/snapshot.rs
+++ b/tests/snapshot.rs
@@ -212,6 +212,11 @@ fn unpinned_uses() -> Result<()> {
         .workflow(workflow_under_test("unpinned-uses.yml"))
         .run()?);
 
+    insta::assert_snapshot!(zizmor()
+        .workflow(workflow_under_test("unpinned-uses/action.yml"))
+        .args(["--pedantic"])
+        .run()?);
+
     Ok(())
 }
 
diff --git a/tests/snapshots/snapshot__unpinned_uses-3.snap b/tests/snapshots/snapshot__unpinned_uses-3.snap
new file mode 100644
index 0000000..372ecb1
--- /dev/null
+++ b/tests/snapshots/snapshot__unpinned_uses-3.snap
@@ -0,0 +1,22 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor().workflow(workflow_under_test(\"unpinned-uses/action.yml\")).args([\"--pedantic\"]).run()?"
+snapshot_kind: text
+---
+help[unpinned-uses]: unpinned action reference
+ --> @@INPUT@@:8:7
+  |
+8 |       uses: asdf-vm/actions/setup@v3
+  |       ------------------------------ help: action is not pinned to a hash ref
+  |
+  = note: audit confidence → High
+
+help[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:11:7
+   |
+11 |       uses: asdf-vm/actions/setup@main
+   |       -------------------------------- help: action is not pinned to a hash ref
+   |
+   = note: audit confidence → High
+
+2 findings: 0 unknown, 0 informational, 2 low, 0 medium, 0 high
diff --git a/tests/test-data/unpinned-uses/action.yml b/tests/test-data/unpinned-uses/action.yml
new file mode 100644
index 0000000..2fdaf6b
--- /dev/null
+++ b/tests/test-data/unpinned-uses/action.yml
@@ -0,0 +1,11 @@
+name: unpinned-uses-composite-action
+description: unpinned-uses-composite-action
+
+runs:
+  using: composite
+  steps:
+    - name: true-positive-1
+      uses: asdf-vm/actions/setup@v3
+
+    - name: true-positive-2
+      uses: asdf-vm/actions/setup@main