Repository: woodruffw/zizmor
Pull request #408 (2025-01-08 03:25:32)

feat: secrets-inherit: new audit
Adds a `secrets-inherit` audit, which detects and flags usage of `secrets: inherit` in reusable workflow calls.


Patch:
diff --git a/docs/audits.md b/docs/audits.md
index 3ee9db6..2ce0b8b 100644
--- a/docs/audits.md
+++ b/docs/audits.md
@@ -753,6 +753,48 @@ intended to publish build artifacts:
 * Set an action-specific input to disable cache restoration when appropriate,
   such as `lookup-only` in @Swatinem/rust-cache.
 
+## `secrets-inherit`
+
+| Type     | Examples                | Introduced in | Works offline  | Enabled by default |
+|----------|-------------------------|---------------|----------------|--------------------|
+| Workflow  | [secrets-inherit]   | v1.1.0      | ✅             | ✅                 |
+
+[secrets-inherit]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/secrets-inherit
+
+Detects excessive secret inheritance between calling workflows and reusable
+(called) workflows.
+
+[Reusable workflows] can be given secrets by their calling workflow either
+explicitly, or in a blanket fashion with `secrets: inherit`. The latter
+should almost never be used, as it makes it violates the
+[Principle of Least Authority] and makes it impossible to determine which exact
+secrets a reusable workflow was executed with.
+
+### Remediation
+
+In general, `secrets: inherit` should be replaced with a `secrets:` block
+that explicitly forwards each secret actually needed by the reusable workflow.
+
+=== "Before"
+
+    ```yaml title="reusable.yml" hl_lines="4"
+    jobs:
+      pass-secrets-to-workflow:
+        uses: ./.github/workflows/called-workflow.yml
+        secrets: inherit
+    ```
+
+=== "After"
+
+    ```yaml title="reusable.yml" hl_lines="4-6"
+    jobs:
+      pass-secrets-to-workflow:
+        uses: ./.github/workflows/called-workflow.yml
+        secrets:
+          forward-me: ${{ secrets.forward-me }}
+          me-too: ${{ secrets.me-too }}
+    ```
+
 [ArtiPACKED: Hacking Giants Through a Race Condition in GitHub Actions Artifacts]: https://unit42.paloaltonetworks.com/github-repo-artifacts-leak-tokens/
 [Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests]: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/
 [What the fork? Imposter commits in GitHub Actions and CI/CD]: https://www.chainguard.dev/unchained/what-the-fork-imposter-commits-in-github-actions-and-ci-cd
@@ -771,3 +813,5 @@ intended to publish build artifacts:
 [Google & Apache Found Vulnerable to GitHub Environment Injection]: https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability-0
 [Hacking with Environment Variables]: https://www.elttam.com/blog/env/
 [The Monsters in Your Build Cache – GitHub Actions Cache Poisoning]: https://adnanthekhan.com/2024/05/06/the-monsters-in-your-build-cache-github-actions-cache-poisoning/
+[reusable workflows]: https://docs.github.com/en/actions/sharing-automations/reusing-workflows
+[Principle of Least Authority]: https://en.wikipedia.org/wiki/Principle_of_least_privilege
diff --git a/docs/release-notes.md b/docs/release-notes.md
index f63cbab..43306b4 100644
--- a/docs/release-notes.md
+++ b/docs/release-notes.md
@@ -7,9 +7,12 @@ description: Abbreviated change notes about each zizmor release.
 This page contains _abbreviated_, user-focused release notes for each version
 of `zizmor`.
 
-## Upcoming (UNRELEASED)
+## v1.1.0 (UNRELEASED)
 
-Nothing to see here yet!
+### Added
+
+* **New audit**: [secrets-inherit] detects use of `secrets: inherit` with
+  reusable workflow calls (#408)
 
 ## v1.0.1
 
@@ -399,3 +402,4 @@ This is one of `zizmor`'s bigger recent releases! Key enhancements include:
 [cache-poisoning]: ./audits.md#cache-poisoning
 [github-env]: ./audits.md#github-env
 [template-injection]: ./audits.md#template-injection
+[secrets-inherit]: ./audits.md#secrets-inherit
diff --git a/src/audit/mod.rs b/src/audit/mod.rs
index df55963..d00ceab 100644
--- a/src/audit/mod.rs
+++ b/src/audit/mod.rs
@@ -21,6 +21,7 @@ pub(crate) mod impostor_commit;
 pub(crate) mod insecure_commands;
 pub(crate) mod known_vulnerable_actions;
 pub(crate) mod ref_confusion;
+pub(crate) mod secrets_inherit;
 pub(crate) mod self_hosted_runner;
 pub(crate) mod template_injection;
 pub(crate) mod unpinned_uses;
diff --git a/src/audit/secrets_inherit.rs b/src/audit/secrets_inherit.rs
new file mode 100644
index 0000000..64700fe
--- /dev/null
+++ b/src/audit/secrets_inherit.rs
@@ -0,0 +1,56 @@
+use std::ops::Deref;
+
+use github_actions_models::workflow::{job::Secrets, Job};
+
+use crate::finding::Confidence;
+
+use super::{audit_meta, Audit};
+
+pub(crate) struct SecretsInherit;
+
+audit_meta!(
+    SecretsInherit,
+    "secrets-inherit",
+    "secrets unconditionally inherited by called workflow"
+);
+
+impl Audit for SecretsInherit {
+    fn new(_state: super::AuditState) -> anyhow::Result<Self>
+    where
+        Self: Sized,
+    {
+        Ok(Self)
+    }
+
+    fn audit_reusable_job<'w>(
+        &self,
+        job: &super::Job<'w>,
+    ) -> anyhow::Result<Vec<super::Finding<'w>>> {
+        let mut findings = vec![];
+        let Job::ReusableWorkflowCallJob(reusable) = job.deref() else {
+            return Ok(findings);
+        };
+
+        if matches!(reusable.secrets, Some(Secrets::Inherit)) {
+            findings.push(
+                Self::finding()
+                    .add_location(
+                        job.location()
+                            .primary()
+                            .with_keys(&["uses".into()])
+                            .annotated("this reusable workflow"),
+                    )
+                    .add_location(
+                        job.location()
+                            .with_keys(&["secrets".into()])
+                            .annotated("inherits all parent secrets"),
+                    )
+                    .confidence(Confidence::High)
+                    .severity(crate::finding::Severity::Medium)
+                    .build(job.parent())?,
+            );
+        }
+
+        Ok(findings)
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index 7453847..cdeba71 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -363,6 +363,7 @@ fn run() -> Result<ExitCode> {
     register_audit!(audit::insecure_commands::InsecureCommands);
     register_audit!(audit::github_env::GitHubEnv);
     register_audit!(audit::cache_poisoning::CachePoisoning);
+    register_audit!(audit::secrets_inherit::SecretsInherit);
 
     let mut results = FindingRegistry::new(&app, &config);
     {
diff --git a/src/models.rs b/src/models.rs
index cacc598..bfb4dca 100644
--- a/src/models.rs
+++ b/src/models.rs
@@ -177,7 +177,10 @@ impl<'w> Job<'w> {
 
     /// This job's [`SymbolicLocation`].
     pub(crate) fn location(&self) -> SymbolicLocation<'w> {
-        self.parent().location().with_job(self)
+        self.parent()
+            .location()
+            .annotated("this job")
+            .with_job(self)
     }
 
     /// An iterator of this job's constituent [`Step`]s.


Test patch:
diff --git a/tests/snapshot.rs b/tests/snapshot.rs
index 975787b..e96606c 100644
--- a/tests/snapshot.rs
+++ b/tests/snapshot.rs
@@ -391,3 +391,12 @@ fn github_env() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn secrets_inherit() -> Result<()> {
+    insta::assert_snapshot!(zizmor()
+        .workflow(workflow_under_test("secrets-inherit.yml"))
+        .run()?);
+
+    Ok(())
+}
diff --git a/tests/snapshots/snapshot__secrets_inherit.snap b/tests/snapshots/snapshot__secrets_inherit.snap
new file mode 100644
index 0000000..9c22bae
--- /dev/null
+++ b/tests/snapshots/snapshot__secrets_inherit.snap
@@ -0,0 +1,17 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor().workflow(workflow_under_test(\"secrets-inherit.yml\")).run()?"
+snapshot_kind: text
+---
+warning[secrets-inherit]: secrets unconditionally inherited by called workflow
+ --> @@INPUT@@:5:5
+  |
+5 |     uses: octo-org/example-repo/.github/workflows/called-workflow.yml@main
+  |     ---------------------------------------------------------------------- this reusable workflow
+6 |     # NOT OK: unconditionally inherits
+7 |     secrets: inherit
+  |     ---------------- inherits all parent secrets
+  |
+  = note: audit confidence → High
+
+1 finding: 0 unknown, 0 informational, 0 low, 1 medium, 0 high
diff --git a/tests/test-data/secrets-inherit.yml b/tests/test-data/secrets-inherit.yml
new file mode 100644
index 0000000..925ec67
--- /dev/null
+++ b/tests/test-data/secrets-inherit.yml
@@ -0,0 +1,22 @@
+on: push
+
+jobs:
+  call-workflow-vulnerable-1:
+    uses: octo-org/example-repo/.github/workflows/called-workflow.yml@main
+    # NOT OK: unconditionally inherits
+    secrets: inherit
+
+  call-workflow-not-vulnerable-2:
+    uses: octo-org/example-repo/.github/workflows/called-workflow.yml@main
+    # OK: explicitly forwards intended secrets
+    secrets:
+      special-secret: ${{ secrets.special-secret }}
+
+  call-workflow-not-vulnerable-3:
+    uses: octo-org/example-repo/.github/workflows/called-workflow.yml@main
+    # OK: no secrets forwarded
+
+  call-workflow-not-vulnerable-4:
+    uses: octo-org/example-repo/.github/workflows/called-workflow.yml@main
+    # OK: no secrets forwarded
+    secrets: {}