feat: "raw" audit support + overprovisioned-secrets

.github/workflows/ci.yml

@@ -22,7 +22,7 @@ jobs:
       - uses: Swatinem/rust-cache@f0deed1e0edfc6a9be95417288c0e1099b1eeec3 # v2
 
       - name: Lint
-        run: cargo clippy -- -D warnings
+        run: cargo clippy -- -D warnings -D clippy::dbg_macro
 
   test:
     runs-on: ubuntu-latest

Cargo.toml

@@ -29,6 +29,7 @@ human-panic = "2.0.1"
 indexmap = "2.7.1"
 indicatif = "0.17.9"
 itertools = "0.14.0"
+line-index = "0.1.2"
 owo-colors = "4.1.0"
 pest = "2.7.15"
 pest_derive = "2.7.15"

docs/audits.md

@@ -873,6 +873,67 @@ not using `pull_request_target` for auto-merge workflows.
               GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     ```
 
+## `overprovisioned-secrets`
+
+| Type     | Examples                | Introduced in | Works offline  | Enabled by default |
+|----------|-------------------------|---------------|----------------|--------------------|
+| Workflow  | [overprovisioned-secrets.yml]   | v1.3.0      | ✅             | ✅                 |
+
+[overprovisioned-secrets.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/overprovisioned-secrets.yml
+
+Detects excessive sharing of the `secrets` context.
+
+Typically, users access the `secrets` context via its individual members:
+
+```yaml
+env:
+  SECRET_ONE: ${{ secrets.SECRET_ONE }}
+  SECRET_TWO: ${{ secrets.SECRET_TWO }}
+```
+
+This allows the Actions runner to only expose the secrets actually used by
+the workflow to the job environment.
+
+However, if the user instead accesses the *entire* `secrets` context:
+
+```yaml
+env:
+  SECRETS: ${{ toJson(secrets) }}
+```
+
+...then the entire `secrets` context is exposed to the runner, even if
+only a single secret is actually needed.
+
+### Remediation
+
+In general, users should avoid loading the entire `secrets` context.
+Secrets should be accessed individually by name.
+
+=== "Before :warning:"
+
+    ```yaml title="overprovisioned.yml" hl_lines="7"
+    jobs:
+      deploy:
+        runs-on: ubuntu-latest
+        steps:
+          - run: ./deploy.sh
+            env:
+              SECRETS: ${{ toJSON(secrets) }}
+    ```
+
+=== "After :white_check_mark:"
+
+    ```yaml title="overprovisioned.yml" hl_lines="7-8"
+    jobs:
+      deploy:
+        runs-on: ubuntu-latest
+        steps:
+          - run: ./deploy.sh
+            env:
+              SECRET_ONE: ${{ secrets.SECRET_ONE }}
+              SECRET_TWO: ${{ secrets.SECRET_TWO }}
+    ```
+
 [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

src/audit/mod.rs

@@ -2,10 +2,12 @@
 
 use anyhow::Result;
 use github_actions_models::action;
+use line_index::LineIndex;
 use tracing::instrument;
+use yamlpath::Document;
 
 use crate::{
-    finding::{Finding, FindingBuilder},
+    finding::{Finding, FindingBuilder, SymbolicLocation},
     models::{Action, CompositeStep, Job, NormalJob, ReusableWorkflowCallJob, Step, Workflow},
     registry::InputKey,
     state::AuditState,
@@ -21,6 +23,7 @@ pub(crate) mod hardcoded_container_credentials;
 pub(crate) mod impostor_commit;
 pub(crate) mod insecure_commands;
 pub(crate) mod known_vulnerable_actions;
+pub(crate) mod overprovisioned_secrets;
 pub(crate) mod ref_confusion;
 pub(crate) mod secrets_inherit;
 pub(crate) mod self_hosted_runner;
@@ -49,12 +52,32 @@ impl AuditInput {
         }
     }
 
+    pub(crate) fn line_index(&self) -> &LineIndex {
+        match self {
+            AuditInput::Workflow(workflow) => &workflow.line_index,
+            AuditInput::Action(action) => &action.line_index,
+        }
+    }
+
     pub(crate) fn link(&self) -> Option<&str> {
         match self {
             AuditInput::Workflow(workflow) => workflow.link.as_deref(),
             AuditInput::Action(action) => action.link.as_deref(),
         }
     }
+
+    pub(crate) fn location(&self) -> SymbolicLocation {
+        match self {
+            AuditInput::Workflow(workflow) => workflow.location(),
+            AuditInput::Action(action) => action.location(),
+        }
+    }
+}
+
+impl AsRef<Document> for AuditInput {
+    fn as_ref(&self) -> &Document {
+        self.document()
+    }
 }
 
 impl From<Workflow> for AuditInput {
@@ -147,6 +170,10 @@ pub(crate) use audit_meta;
 /// 2. [`Audit::audit_composite_step`]: runs on each composite step within the
 ///    action (most specific)
 ///
+/// For both:
+///
+/// 1. [`Audit::audit_raw`]: runs on the raw, unparsed YAML document source
+///
 /// Picking a higher specificity means that the lower methods are shadowed.
 /// In other words, if an audit chooses to implement [`Audit::audit`], it should implement
 /// **only** [`Audit::audit`] and not [`Audit::audit_normal_job`] or
@@ -208,15 +235,23 @@ pub(crate) trait Audit: AuditCore {
         Ok(results)
     }
 
+    fn audit_raw<'w>(&self, _input: &'w AuditInput) -> Result<Vec<Finding<'w>>> {
+        Ok(vec![])
+    }
+
     /// The top-level auditing function for both workflows and actions.
     ///
     /// Implementors **should not** override this blanket implementation,
     /// since it's marked with tracing instrumentation.
     #[instrument(skip(self))]
     fn audit<'w>(&self, input: &'w AuditInput) -> Result<Vec<Finding<'w>>> {
-        match input {
+        let mut results = match input {
             AuditInput::Workflow(workflow) => self.audit_workflow(workflow),
             AuditInput::Action(action) => self.audit_action(action),
-        }
+        }?;
+
+        results.extend(self.audit_raw(input)?);
+
+        Ok(results)
     }
 }

src/audit/overprovisioned_secrets.rs

@@ -0,0 +1,117 @@
+use crate::{
+    expr::Expr,
+    finding::{Confidence, Feature, Location, Severity},
+    utils::extract_expressions,
+};
+
+use super::{audit_meta, Audit, AuditInput};
+
+pub(crate) struct OverprovisionedSecrets;
+
+audit_meta!(
+    OverprovisionedSecrets,
+    "overprovisioned-secrets",
+    "excessively provisioned secrets"
+);
+
+impl Audit for OverprovisionedSecrets {
+    fn new(_state: super::AuditState) -> anyhow::Result<Self>
+    where
+        Self: Sized,
+    {
+        Ok(Self)
+    }
+
+    fn audit_raw<'w>(&self, input: &'w AuditInput) -> anyhow::Result<Vec<super::Finding<'w>>> {
+        let mut findings = vec![];
+        let raw = input.document().source();
+
+        for (expr, span) in extract_expressions(raw) {
+            let Ok(parsed) = Expr::parse(expr.as_bare()) else {
+                tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
+                continue;
+            };
+
+            expr.as_curly();
+
+            for _ in Self::secrets_expansions(&parsed) {
+                findings.push(
+                    Self::finding()
+                        .confidence(Confidence::High)
+                        .severity(Severity::Medium)
+                        .add_raw_location(Location::new(
+                            input
+                                .location()
+                                .annotated("injects the entire secrets context into the runner")
+                                .primary(),
+                            Feature::from_span(&span, input),
+                        ))
+                        .build(input)?,
+                );
+            }
+        }
+
+        findings.len();
+
+        Ok(findings)
+    }
+}
+
+impl OverprovisionedSecrets {
+    fn secrets_expansions(expr: &Expr) -> Vec<()> {
+        let mut results = vec![];
+
+        match expr {
+            Expr::Call { func, args } => {
+                // TODO: Consider any function call that accepts bare `secrets`
+                // to be a finding? Are there any other functions that users
+                // would plausible call with the entire `secrets` object?
+                if func.eq_ignore_ascii_case("toJSON")
+                    && args
+                        .iter()
+                        .any(|arg| matches!(arg, Expr::Context { raw, components: _ } if raw.eq_ignore_ascii_case("secrets")))
+                {
+                    results.push(());
+                } else {
+                    results.extend(args.iter().flat_map(Self::secrets_expansions));
+                }
+            }
+            Expr::Index(expr) => results.extend(Self::secrets_expansions(expr)),
+            Expr::Context { raw: _, components } => {
+                results.extend(components.iter().flat_map(Self::secrets_expansions))
+            }
+            Expr::BinOp { lhs, op: _, rhs } => {
+                results.extend(Self::secrets_expansions(lhs));
+                results.extend(Self::secrets_expansions(rhs));
+            }
+            Expr::UnOp { op: _, expr } => results.extend(Self::secrets_expansions(expr)),
+            _ => (),
+        }
+
+        results
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn test_secrets_expansions() {
+        for (expr, count) in &[
+            ("secrets", 0),
+            ("toJSON(secrets.foo)", 0),
+            ("toJSON(secrets)", 1),
+            ("tojson(secrets)", 1),
+            ("toJSON(SECRETS)", 1),
+            ("tOjSoN(sECrEtS)", 1),
+            ("false || toJSON(secrets)", 1),
+            ("toJSON(secrets) || toJSON(secrets)", 2),
+            ("format('{0}', toJSON(secrets))", 1),
+        ] {
+            let expr = crate::expr::Expr::parse(expr).unwrap();
+            assert_eq!(
+                super::OverprovisionedSecrets::secrets_expansions(&expr).len(),
+                *count
+            );
+        }
+    }
+}

src/audit/template_injection.rs

@@ -168,7 +168,7 @@ impl TemplateInjection {
         step: &impl StepCommon<'s>,
     ) -> Vec<(String, Severity, Confidence, Persona)> {
         let mut bad_expressions = vec![];
-        for expr in extract_expressions(run) {
+        for (expr, _) in extract_expressions(run) {
             let Ok(parsed) = Expr::parse(expr.as_bare()) else {
                 tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
                 continue;