Repository: woodruffw/zizmor
Pull request #359 (2024-12-25 23:23:00)

feat: insecure-commands: support composite actions
See #350.


Patch:
diff --git a/docs/audits.md b/docs/audits.md
index 5be27e8..7db0831 100644
--- a/docs/audits.md
+++ b/docs/audits.md
@@ -623,7 +623,7 @@ A before/after example is shown below.
 
 | Type     | Examples                | Introduced in | Works offline  | Enabled by default |
 |----------|-------------------------|---------------|----------------|--------------------|
-| Workflow  | [insecure-commands.yml] | v0.5.0        | ✅             | ✅                 |
+| Workflow, Action  | [insecure-commands.yml] | v0.5.0        | ✅             | ✅                 |
 
 [insecure-commands.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/insecure-commands.yml
 
@@ -669,7 +669,7 @@ In general, users should use for [GitHub Actions environment files]
 
 | Type     | Examples           | Introduced in | Works offline  | Enabled by default |
 |----------|--------------------|---------------|----------------|--------------------|
-| Workflow  | [github-env.yml]   | v0.6.0        | ✅             | ✅                 |
+| Workflow, Action  | [github-env.yml]   | v0.6.0        | ✅             | ✅                 |
 
 [github-env.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/github-env.yml
 
diff --git a/src/audit/insecure_commands.rs b/src/audit/insecure_commands.rs
index ba0415b..75a3f45 100644
--- a/src/audit/insecure_commands.rs
+++ b/src/audit/insecure_commands.rs
@@ -3,6 +3,7 @@ use crate::finding::{Confidence, Finding, Persona, Severity, SymbolicLocation};
 use crate::models::{Steps, Workflow};
 use crate::state::AuditState;
 use anyhow::Result;
+use github_actions_models::action;
 use github_actions_models::common::expr::LoE;
 use github_actions_models::common::{Env, EnvValue};
 use github_actions_models::workflow::job::StepBody;
@@ -22,7 +23,7 @@ audit_meta!(
 impl InsecureCommands {
     fn insecure_commands_maybe_present<'w>(
         &self,
-        workflow: &'w Workflow,
+        doc: &'w impl AsRef<yamlpath::Document>,
         location: SymbolicLocation<'w>,
     ) -> Result<Finding<'w>> {
         Self::finding()
@@ -34,12 +35,12 @@ impl InsecureCommands {
                     "non-static environment may contain ACTIONS_ALLOW_UNSECURE_COMMANDS",
                 ),
             )
-            .build(workflow)
+            .build(doc)
     }
 
     fn insecure_commands_allowed<'w>(
         &self,
-        workflow: &'w Workflow,
+        doc: &'w impl AsRef<yamlpath::Document>,
         location: SymbolicLocation<'w>,
     ) -> Result<Finding<'w>> {
         Self::finding()
@@ -51,7 +52,7 @@ impl InsecureCommands {
                     .with_keys(&["env".into()])
                     .annotated("insecure commands enabled here"),
             )
-            .build(workflow)
+            .build(doc)
     }
 
     fn has_insecure_commands_enabled(&self, env: &Env) -> bool {
@@ -135,4 +136,28 @@ impl Audit for InsecureCommands {
 
         Ok(results)
     }
+
+    fn audit_composite_step<'a>(
+        &self,
+        step: &super::CompositeStep<'a>,
+    ) -> Result<Vec<Finding<'a>>> {
+        let mut findings = vec![];
+
+        let action::StepBody::Run { env, .. } = &step.body else {
+            return Ok(findings);
+        };
+
+        match env {
+            LoE::Expr(_) => {
+                findings.push(self.insecure_commands_maybe_present(step.action(), step.location())?)
+            }
+            LoE::Literal(env) => {
+                if self.has_insecure_commands_enabled(env) {
+                    findings.push(self.insecure_commands_allowed(step.action(), step.location())?);
+                }
+            }
+        }
+
+        Ok(findings)
+    }
 }


Test patch:
diff --git a/tests/snapshot.rs b/tests/snapshot.rs
index c2639b5..b06544f 100644
--- a/tests/snapshot.rs
+++ b/tests/snapshot.rs
@@ -226,6 +226,11 @@ fn insecure_commands() -> Result<()> {
         .workflow(workflow_under_test("insecure-commands.yml"))
         .run()?);
 
+    insta::assert_snapshot!(zizmor()
+        .workflow(workflow_under_test("insecure-commands/action.yml"))
+        .args(["--persona=auditor"])
+        .run()?);
+
     Ok(())
 }
 
diff --git a/tests/snapshots/snapshot__insecure_commands-3.snap b/tests/snapshots/snapshot__insecure_commands-3.snap
new file mode 100644
index 0000000..480d606
--- /dev/null
+++ b/tests/snapshots/snapshot__insecure_commands-3.snap
@@ -0,0 +1,32 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor().workflow(workflow_under_test(\"insecure-commands/action.yml\")).args([\"--persona=auditor\"]).run()?"
+snapshot_kind: text
+---
+error[insecure-commands]: execution of insecure workflow commands is enabled
+  --> @@INPUT@@:18:7
+   |
+18 | /       env:
+19 | |         ACTIONS_ALLOW_UNSECURE_COMMANDS: yes
+   | |____________________________________________^ insecure commands enabled here
+   |
+   = note: audit confidence → High
+
+error[insecure-commands]: execution of insecure workflow commands is enabled
+  --> @@INPUT@@:25:7
+   |
+25 | /       env:
+26 | |         ACTIONS_ALLOW_UNSECURE_COMMANDS: anything
+   | |_________________________________________________^ insecure commands enabled here
+   |
+   = note: audit confidence → High
+
+error[insecure-commands]: execution of insecure workflow commands is enabled
+  --> @@INPUT@@:32:7
+   |
+32 |       env: ${{ mystery }}
+   |       ^^^^^^^^^^^^^^^^^^^ non-static environment may contain ACTIONS_ALLOW_UNSECURE_COMMANDS
+   |
+   = note: audit confidence → Low
+
+3 findings: 0 unknown, 0 informational, 0 low, 0 medium, 3 high
diff --git a/tests/test-data/insecure-commands/action.yml b/tests/test-data/insecure-commands/action.yml
new file mode 100644
index 0000000..4a64b53
--- /dev/null
+++ b/tests/test-data/insecure-commands/action.yml
@@ -0,0 +1,32 @@
+# demo of a composite action being flagged by insecure-commands
+
+name: insecure-commands-composite-action
+description: insecure-commands-composite-action
+
+inputs:
+  insecure-commands:
+    default: "yes"
+    description: "do something insecure"
+
+runs:
+  using: composite
+  steps:
+    - name: true-positive-1
+      run: |
+        echo "hello!"
+      shell: bash
+      env:
+        ACTIONS_ALLOW_UNSECURE_COMMANDS: yes
+
+    - name: true-positive-2
+      run: |
+        echo "hello!"
+      shell: bash
+      env:
+        ACTIONS_ALLOW_UNSECURE_COMMANDS: anything
+
+    - name: true-positive-3
+      run: |
+        echo "hello!"
+      shell: bash
+      env: ${{ mystery }}