zizmorcore · aldur · Mar 31, 2025 · Mar 31, 2025 · Mar 31, 2025 · Apr 9, 2025
diff --git a/src/audit/mod.rs b/src/audit/mod.rs
@@ -30,6 +30,7 @@ pub(crate) mod obfuscation;
 pub(crate) mod overprovisioned_secrets;
 pub(crate) mod ref_confusion;
 pub(crate) mod secrets_inherit;
+pub(crate) mod secret_without_env;
 pub(crate) mod self_hosted_runner;
 pub(crate) mod stale_action_refs;
 pub(crate) mod template_injection;

diff --git a/src/audit/secret_without_env.rs b/src/audit/secret_without_env.rs
@@ -0,0 +1,92 @@
+use github_actions_models::{
+    common::{
+        Env, EnvValue,
+        expr::{ExplicitExpr, LoE},
+    },
+    workflow::job::StepBody,
+};
+
+use super::{Audit, audit_meta};
+use crate::{AuditLoadError, AuditState, Persona, finding::Confidence, models::StepCommon};
+
+pub(crate) struct SecretWithoutEnv;
+
+audit_meta!(
+    SecretWithoutEnv,
+    "secret-without-env",
+    "secret used without an environment to gate it"
+);
+
+impl Audit for SecretWithoutEnv {
+    fn new(_state: &AuditState<'_>) -> Result<Self, AuditLoadError>
+    where
+        Self: Sized,
+    {
+        Ok(Self)
+    }
+
+    fn audit_step<'w>(
+        &self,
+        step: &crate::models::Step<'w>,
+    ) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
+        let mut findings = vec![];
+
+        if step.parent.environment().is_some() {
+            return Ok(findings);
+        }
+
+        let eenv: &Env;
+        match &step.body {
+            StepBody::Uses { uses: _, with } => {
+                eenv = with;
+            }
+            StepBody::Run {
+                run: _,
+                shell: _,
+                env,
+                working_directory: _,
+            } => match env {
+                LoE::Expr(e) => {
+                    Self::check_secrets_access(e.as_bare(), step, &mut findings)?;
+                    return Ok(findings);
+                }
+                LoE::Literal(env) => eenv = env,
+            },
+        }
+
+        for v in eenv.values() {
+            if let EnvValue::String(s) = v {
+                if let Some(expr) = ExplicitExpr::from_curly(s) {
+                    Self::check_secrets_access(expr.as_bare(), step, &mut findings)?;
+                } else {
+                    Self::check_secrets_access(s, step, &mut findings)?
+                }
+            }
+        }
+
+        Ok(findings)
+    }
+}
+
+impl SecretWithoutEnv {
+    fn check_secrets_access<'w>(
+        s: &str,
+        step: &crate::models::Step<'w>,
+        findings: &mut Vec<crate::finding::Finding<'w>>,
+    ) -> anyhow::Result<()> {
+        // See https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication
+        // for `secrets.github_token`
+        if s.contains("secrets") && s.trim() != "secrets.github_token" {
+            findings.push(
+                Self::finding()
+                    .add_location(step.location().primary())
+                    .confidence(Confidence::High)
+                    .severity(crate::finding::Severity::High)
+                    .persona(Persona::Pedantic)
+                    .build(step.workflow())?,
+            );
+        }
+
+        Ok(())
+    }
+}
diff --git a/src/main.rs b/src/main.rs
@@ -1,5 +1,5 @@
 use std::{
-    io::{Write, stdout},
+    io::{stdout, Write},
     process::ExitCode,
     str::FromStr,
 };
@@ -20,9 +20,9 @@ use indicatif::ProgressStyle;
 use owo_colors::OwoColorize;
 use registry::{AuditRegistry, FindingRegistry, InputKey, InputKind, InputRegistry};
 use state::AuditState;
-use tracing::{Span, info_span, instrument};
-use tracing_indicatif::{IndicatifLayer, span_ext::IndicatifSpanExt};
-use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _, util::SubscriberInitExt as _};
+use tracing::{info_span, instrument, Span};
+use tracing_indicatif::{span_ext::IndicatifSpanExt, IndicatifLayer};
+use tracing_subscriber::{layer::SubscriberExt as _, util::SubscriberInitExt as _, EnvFilter};
 
 mod audit;
 mod config;
@@ -546,6 +546,7 @@ fn run() -> Result<ExitCode> {
     register_audit!(audit::github_env::GitHubEnv);
     register_audit!(audit::cache_poisoning::CachePoisoning);
     register_audit!(audit::secrets_inherit::SecretsInherit);
+    register_audit!(audit::secret_without_env::SecretWithoutEnv);
     register_audit!(audit::bot_conditions::BotConditions);
     register_audit!(audit::overprovisioned_secrets::OverprovisionedSecrets);
     register_audit!(audit::unredacted_secrets::UnredactedSecrets);

diff --git a/src/models.rs b/src/models.rs
@@ -9,8 +9,8 @@ use anyhow::{Context, bail};
 use github_actions_models::common::Env;
 use github_actions_models::common::expr::LoE;
 use github_actions_models::workflow::event::{BareEvent, OptionalBody};
-use github_actions_models::workflow::job::{RunsOn, Strategy};
-use github_actions_models::workflow::{self, Trigger, job, job::StepBody};
+use github_actions_models::workflow::job::{DeploymentEnvironment, RunsOn, Strategy};
+use github_actions_models::workflow::{self, job, job::StepBody, Trigger};
 use github_actions_models::{action, common};
 use indexmap::IndexMap;
 use line_index::LineIndex;
@@ -225,6 +225,10 @@ impl<'doc> NormalJob<'doc> {
         Steps::new(self)
     }
 
+    pub(crate) fn environment(&self) -> Option<&DeploymentEnvironment> {
+        self.inner.environment.as_ref()
+    }
+
     /// Perform feats of heroism to figure of what this job's runner's
     /// default shell is.
     ///

diff --git a/tests/integration/snapshot.rs b/tests/integration/snapshot.rs
@@ -647,6 +647,18 @@ fn unredacted_secrets() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn secrets_outside_environment() -> Result<()> {
+    insta::assert_snapshot!(
+        zizmor()
+            .input(input_under_test("secret-without-env.yml"))
+            .args(["--persona=pedantic"])
+            .run()?
+    );
+
+    Ok(())
+}
+
 #[test]
 fn forbidden_uses() -> Result<()> {
     for config in ["allow-all", "deny-all", "allow-some", "deny-some"] {
@@ -662,6 +674,7 @@ fn forbidden_uses() -> Result<()> {
         );
     }
 
+
     Ok(())
 }
 

diff --git a/tests/integration/snapshots/integration__snapshot__cache_poisoning-10.snap b/tests/integration/snapshots/integration__snapshot__cache_poisoning-10.snap
@@ -3,14 +3,14 @@ source: tests/integration/snapshot.rs
 expression: "zizmor().input(input_under_test(\"cache-poisoning/publisher-step.yml\")).run()?"
 ---
 error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack
-  --> @@INPUT@@:25:9
+  --> @@INPUT@@:26:9
    |
-18 |         uses: Swatinem/rust-cache@82a92a6e8fbeee089604da2575dc567ae9ddeaab
+19 |         uses: Swatinem/rust-cache@82a92a6e8fbeee089604da2575dc567ae9ddeaab
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cache enabled by default here
-19 |
+20 |
 ...
-24 |       - name: Publish draft release on Github
-25 |         uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974
+25 |       - name: Publish draft release on Github
+26 |         uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ runtime artifacts usually published here
    |
    = note: audit confidence → Low

diff --git a/tests/integration/snapshots/integration__snapshot__cache_poisoning-4.snap b/tests/integration/snapshots/integration__snapshot__cache_poisoning-4.snap
@@ -9,10 +9,10 @@ error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache pois
    |   ^^^^^^^^^^^ generally used when publishing artifacts generated at runtime
  2 |
 ...
-15 |           uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a
-16 | /         with:
-17 | |           python-version: "3.12"
-18 | |           enable-cache: ${{ github.ref == 'refs/heads/main' }}
+16 |           uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a
+17 | /         with:
+18 | |           python-version: "3.12"
+19 | |           enable-cache: ${{ github.ref == 'refs/heads/main' }}
    | |______________________________________________________________^ opt-in for caching might happen here
    |
    = note: audit confidence → Low

diff --git a/tests/integration/snapshots/integration__snapshot__overprovisioned_secrets.snap b/tests/integration/snapshots/integration__snapshot__overprovisioned_secrets.snap
@@ -3,17 +3,17 @@ source: tests/integration/snapshot.rs
 expression: "zizmor().input(input_under_test(\"overprovisioned-secrets.yml\")).run()?"
 ---
 warning[overprovisioned-secrets]: excessively provisioned secrets
-  --> @@INPUT@@:12:18
+  --> @@INPUT@@:13:18
    |
-12 |           stuff: ${{ format('{0}', toJSON(secrets)) }}
+13 |           stuff: ${{ format('{0}', toJSON(secrets)) }}
    |                  ------------------------------------- injects the entire secrets context into the runner
    |
    = note: audit confidence → High
 
 warning[overprovisioned-secrets]: excessively provisioned secrets
-  --> @@INPUT@@:21:25
+  --> @@INPUT@@:22:25
    |
-21 |           secrets_json: ${{ toJSON(secrets) }}
+22 |           secrets_json: ${{ toJSON(secrets) }}
    |                         ---------------------- injects the entire secrets context into the runner
    |
    = note: audit confidence → High

diff --git a/tests/integration/snapshots/integration__snapshot__secrets_outside_environment.snap b/tests/integration/snapshots/integration__snapshot__secrets_outside_environment.snap
@@ -0,0 +1,17 @@
+---
+source: tests/integration/snapshot.rs
+expression: "zizmor().input(input_under_test(\"secret-without-env.yml\")).args([\"--persona=pedantic\"]).run()?"
+---
+error[secret-without-env]: secret used without an environment to gate it
+  --> @@INPUT@@:10:9
+   |
+10 |         - uses: actions_repo/actions/docker@13c8cf37e54dd9488afe8c067575444cc58bf155
+   |  _________^
+11 | |         with:
+12 | |           # NOT OK: Anyone with write access can exfiltrate this secret.
+13 | |           password: ${{ secrets.DOCKERHUB_PASSWORD }}
+   | |______________________________________________________^ this step
+   |
+   = note: audit confidence → High
+
+1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
diff --git a/tests/integration/snapshots/integration__snapshot__unredacted_secrets.snap b/tests/integration/snapshots/integration__snapshot__unredacted_secrets.snap
@@ -3,17 +3,17 @@ source: tests/integration/snapshot.rs
 expression: "zizmor().input(input_under_test(\"unredacted-secrets.yml\")).run()?"
 ---
 warning[unredacted-secrets]: leaked secret values
-  --> @@INPUT@@:14:18
+  --> @@INPUT@@:15:18
    |
-14 |           stuff: ${{ fromJSON(secrets.password) }}
+15 |           stuff: ${{ fromJSON(secrets.password) }}
    |                  --------------------------------- bypasses secret redaction
    |
    = note: audit confidence → High
 
 warning[unredacted-secrets]: leaked secret values
-  --> @@INPUT@@:17:23
+  --> @@INPUT@@:18:23
    |
-17 |           otherstuff: ${{ fromJson(secrets.otherstuff).field }}
+18 |           otherstuff: ${{ fromJson(secrets.otherstuff).field }}
    |                       ----------------------------------------- bypasses secret redaction
    |
    = note: audit confidence → High

diff --git a/tests/integration/test-data/cache-poisoning.yml b/tests/integration/test-data/cache-poisoning.yml
@@ -5,6 +5,7 @@ permissions: {}
 jobs:
   publish:
     runs-on: ubuntu-latest
+    environment: "Pypi"
     steps:
       - name: Setup uv
         uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a

diff --git a/tests/integration/test-data/cache-poisoning/caching-opt-in-expression.yml b/tests/integration/test-data/cache-poisoning/caching-opt-in-expression.yml
@@ -5,6 +5,7 @@ permissions: {}
 jobs:
   publish:
     runs-on: ubuntu-latest
+    environment: "Pypi"
     steps:
       - name: Project Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683

diff --git a/tests/integration/test-data/cache-poisoning/publisher-step.yml b/tests/integration/test-data/cache-poisoning/publisher-step.yml
@@ -8,6 +8,7 @@ permissions: {}
 jobs:
   publish:
     runs-on: macos-latest
+    environment: "GitHub release"
     steps:
       - name: Project Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683

diff --git a/tests/integration/test-data/overprovisioned-secrets.yml b/tests/integration/test-data/overprovisioned-secrets.yml
@@ -5,6 +5,7 @@ permissions: {}
 jobs:
   test:
     runs-on: ubuntu-latest
+    environment: "overprovisioned-secrets"
     steps:
       - run: echo "${stuff} ${otherstuff} ${morestuff}"
         env:

diff --git a/tests/integration/test-data/secret-without-env.yml b/tests/integration/test-data/secret-without-env.yml
@@ -0,0 +1,13 @@
+on: push
+permissions: {}
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions_repo/actions/docker-login@13c8cf37e54dd9488afe8c067575444cc58bf155
+        with:
+          password: ${{ secrets.github_token }}  # This is OK.
+      - uses: actions_repo/actions/docker@13c8cf37e54dd9488afe8c067575444cc58bf155
+        with:
+          # NOT OK: Anyone with write access can exfiltrate this secret.
+          password: ${{ secrets.DOCKERHUB_PASSWORD }}
diff --git a/tests/integration/test-data/unredacted-secrets.yml b/tests/integration/test-data/unredacted-secrets.yml
@@ -7,6 +7,7 @@ permissions: {}
 jobs:
   test:
     runs-on: ubuntu-latest
+    environment: "Unredacted secrets"
     steps:
       - run: echo "${stuff} ${otherstuff}"
         env:

diff --git a/tests/integration/test-data/use-trusted-publishing.yml b/tests/integration/test-data/use-trusted-publishing.yml
@@ -6,6 +6,7 @@ on: [push]
 jobs:
   publish:
     runs-on: ubuntu-latest
+    environment: "secret environment"
     permissions:
       id-token: write
     steps:
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ permissions: {} @@
     jobs:
       test:
         runs-on: ubuntu-latest
+        environment: "overprovisioned-secrets"
         steps:
           - run: echo "${stuff} ${otherstuff} ${morestuff}"
             env:
@@ Expand Down @@