From 13465ab42fd78593f383cdd18854532d688da778 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sun, 14 Sep 2025 09:09:41 -0400 Subject: [PATCH] fix: handle another cache setting in setup-node (#1153) --- crates/zizmor/src/audit/cache_poisoning.rs | 16 ++++--- crates/zizmor/src/models/coordinate.rs | 5 ++- crates/zizmor/tests/integration/snapshot.rs | 6 +++ ...gration__snapshot__cache_poisoning-17.snap | 39 ++++++++++++++++ .../cache-poisoning/issue-1152-repro.yml | 45 +++++++++++++++++++ docs/release-notes.md | 6 +++ 6 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 crates/zizmor/tests/integration/snapshots/integration__snapshot__cache_poisoning-17.snap create mode 100644 crates/zizmor/tests/integration/test-data/cache-poisoning/issue-1152-repro.yml diff --git a/crates/zizmor/src/audit/cache_poisoning.rs b/crates/zizmor/src/audit/cache_poisoning.rs index 8b6de714..182501cb 100644 --- a/crates/zizmor/src/audit/cache_poisoning.rs +++ b/crates/zizmor/src/audit/cache_poisoning.rs @@ -50,12 +50,16 @@ static KNOWN_CACHE_AWARE_ACTIONS: LazyLock> = LazyLock::ne // https://github.com/actions/setup-node/blob/main/action.yml ActionCoordinate::Configurable { uses_pattern: "actions/setup-node".parse().unwrap(), - control: ControlExpr::single( - Toggle::OptIn, - "cache", - ControlFieldType::FreeString, - false, - ), + control: ControlExpr::any([ + ControlExpr::single(Toggle::OptIn, "cache", ControlFieldType::FreeString, false), + // NOTE: Added with `setup-node@v5`. + ControlExpr::single( + Toggle::OptIn, + "package-manager-cache", + ControlFieldType::Boolean, + true, + ), + ]), }, // https://github.com/actions/setup-python/blob/main/action.yml ActionCoordinate::Configurable { diff --git a/crates/zizmor/src/models/coordinate.rs b/crates/zizmor/src/models/coordinate.rs index f22270b9..48737f56 100644 --- a/crates/zizmor/src/models/coordinate.rs +++ b/crates/zizmor/src/models/coordinate.rs @@ -267,7 +267,6 @@ pub(crate) enum ControlExpr { /// Universal quantification: all of the fields must be satisfied. All(Vec), /// Existential quantification: any of the fields must be satisfied. - #[allow(dead_code)] Any(Vec), /// Negation: the "opposite" of the expression's satisfaction. Not(Box), @@ -292,6 +291,10 @@ impl ControlExpr { Self::All(exprs.into_iter().collect()) } + pub(crate) fn any(exprs: impl IntoIterator) -> Self { + Self::Any(exprs.into_iter().collect()) + } + pub(crate) fn not(expr: ControlExpr) -> Self { Self::Not(Box::new(expr)) } diff --git a/crates/zizmor/tests/integration/snapshot.rs b/crates/zizmor/tests/integration/snapshot.rs index 4b972940..754c41e3 100644 --- a/crates/zizmor/tests/integration/snapshot.rs +++ b/crates/zizmor/tests/integration/snapshot.rs @@ -543,6 +543,12 @@ fn cache_poisoning() -> Result<()> { .run()? ); + insta::assert_snapshot!( + zizmor() + .input(input_under_test("cache-poisoning/issue-1152-repro.yml")) + .run()? + ); + Ok(()) } diff --git a/crates/zizmor/tests/integration/snapshots/integration__snapshot__cache_poisoning-17.snap b/crates/zizmor/tests/integration/snapshots/integration__snapshot__cache_poisoning-17.snap new file mode 100644 index 00000000..d2c9c40d --- /dev/null +++ b/crates/zizmor/tests/integration/snapshots/integration__snapshot__cache_poisoning-17.snap @@ -0,0 +1,39 @@ +--- +source: crates/zizmor/tests/integration/snapshot.rs +expression: "zizmor().input(input_under_test(\"cache-poisoning/issue-1152-repro.yml\")).run()?" +--- +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:16:9 + | + 5 | on: release + | ----------- generally used when publishing artifacts generated at runtime +... +16 | uses: actions/setup-node@v5 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cache enabled by default here + | + = note: audit confidence → Low + +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:21:9 + | + 5 | on: release + | ----------- generally used when publishing artifacts generated at runtime +... +21 | / with: +22 | | package-manager-cache: true + | |_____________________________________^ opt-in for caching here + | + = note: audit confidence → Low + +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:42:9 + | + 5 | on: release + | ----------- generally used when publishing artifacts generated at runtime +... +42 | - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cache enabled by default here + | + = note: audit confidence → Low + +3 findings: 0 unknown, 0 informational, 0 low, 0 medium, 3 high diff --git a/crates/zizmor/tests/integration/test-data/cache-poisoning/issue-1152-repro.yml b/crates/zizmor/tests/integration/test-data/cache-poisoning/issue-1152-repro.yml new file mode 100644 index 00000000..0f131fd7 --- /dev/null +++ b/crates/zizmor/tests/integration/test-data/cache-poisoning/issue-1152-repro.yml @@ -0,0 +1,45 @@ +# repro case for https://github.com/zizmorcore/zizmor/issues/1152 + +name: issue-1152 + +on: release + +permissions: {} + +jobs: + issue-1152-true-positive: + name: issue-1152-true-positive + runs-on: ubuntu-latest + steps: + # TRUE POSITIVE: package-manager-cache enabled by default + - name: true-positive-default + uses: actions/setup-node@v5 + + # TRUE POSITIVE: enable-cache explicitly set to true + - name: true-positive-explicit + uses: actions/setup-node@v5 + with: + package-manager-cache: true + + issue-1152-true-negative: + name: issue-1152-true-negative + runs-on: ubuntu-latest + steps: + # TRUE NEGATIVE: package-manager-cache explicitly set to false + - name: true-negative + uses: actions/setup-node@v5 + with: + package-manager-cache: false + + publish: + name: user-repro-case-1 + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + with: + persist-credentials: false + - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + with: + node-version: 24 + registry-url: https://registry.npmjs.org diff --git a/docs/release-notes.md b/docs/release-notes.md index 458248c7..017beb3d 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -9,6 +9,12 @@ of `zizmor`. ## Next (UNRELEASED) +### Bug Fixes 🐛 + +* Fixed a bug where the [cache-poisoning] audit would fail to detect + some cache usage variants in newer versions of `actions/setup-node` + (#1152) + ## 1.13.0 ### New Features 🌈