From db594e2eb3f0616c267c7ca8deecfa55b7445e64 Mon Sep 17 00:00:00 2001 From: Jamison Lahman Date: Sat, 21 Dec 2024 11:41:25 -0800 Subject: [PATCH] fix: setup-go's cache-control is OptOut (#343) Co-authored-by: William Woodruff --- src/audit/cache_poisoning.rs | 2 +- tests/snapshot.rs | 4 ++ .../snapshot__cache_poisoning-11.snap | 60 +++++++++++++++++++ .../cache-poisoning/issue-343-repro.yml | 41 +++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 tests/snapshots/snapshot__cache_poisoning-11.snap create mode 100644 tests/test-data/cache-poisoning/issue-343-repro.yml diff --git a/src/audit/cache_poisoning.rs b/src/audit/cache_poisoning.rs index b50eddee..a6039371 100644 --- a/src/audit/cache_poisoning.rs +++ b/src/audit/cache_poisoning.rs @@ -55,7 +55,7 @@ static KNOWN_CACHE_AWARE_ACTIONS: LazyLock> = LazyLock::ne CacheAwareAction { uses: Uses::from_step("actions/setup-go").unwrap(), cache_control: CacheControl::OptIn("cache"), - control_value: ControlValue::String, + control_value: ControlValue::Boolean, caching_by_default: true, }, // https://github.com/actions/setup-node/blob/main/action.yml diff --git a/tests/snapshot.rs b/tests/snapshot.rs index 74af2a47..1d623126 100644 --- a/tests/snapshot.rs +++ b/tests/snapshot.rs @@ -324,6 +324,10 @@ fn cache_poisoning() -> Result<()> { .workflow(workflow_under_test("cache-poisoning/publisher-step.yml")) .run()?); + insta::assert_snapshot!(zizmor() + .workflow(workflow_under_test("cache-poisoning/issue-343-repro.yml")) + .run()?); + Ok(()) } diff --git a/tests/snapshots/snapshot__cache_poisoning-11.snap b/tests/snapshots/snapshot__cache_poisoning-11.snap new file mode 100644 index 00000000..b6c69f86 --- /dev/null +++ b/tests/snapshots/snapshot__cache_poisoning-11.snap @@ -0,0 +1,60 @@ +--- +source: tests/snapshot.rs +expression: "zizmor().workflow(workflow_under_test(\"cache-poisoning/issue-343-repro.yml\")).run()?" +snapshot_kind: text +--- +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:5:1 + | + 5 | / on: + 6 | | push: + 7 | | tags: + 8 | | - "v*.*.*" + | |________________^ generally used when publishing artifacts generated at runtime + 9 | +... +24 | - name: true-positive-2 +25 | uses: actions/setup-go@v5 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ cache enabled by default here + | + = note: audit confidence → Low + +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:5:1 + | + 5 | / on: + 6 | | push: + 7 | | tags: + 8 | | - "v*.*.*" + | |________________^ generally used when publishing artifacts generated at runtime + 9 | +... +31 | uses: actions/setup-go@v5 +32 | / with: +33 | | go-version: stable +34 | | cache: true +35 | | +36 | | # Finding because setup enables cache explicitly + | |______________________________________________________^ opt-in for caching here + | + = note: audit confidence → Low + +error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack + --> @@INPUT@@:5:1 + | + 5 | / on: + 6 | | push: + 7 | | tags: + 8 | | - "v*.*.*" + | |________________^ generally used when publishing artifacts generated at runtime + 9 | +... +38 | uses: actions/setup-go@v5 +39 | / with: +40 | | go-version: stable +41 | | cache: "true" + | |________________________^ opt-in for caching here + | + = note: audit confidence → Low + +7 findings (4 suppressed): 0 unknown, 0 informational, 0 low, 0 medium, 3 high diff --git a/tests/test-data/cache-poisoning/issue-343-repro.yml b/tests/test-data/cache-poisoning/issue-343-repro.yml new file mode 100644 index 00000000..3f8f1cfc --- /dev/null +++ b/tests/test-data/cache-poisoning/issue-343-repro.yml @@ -0,0 +1,41 @@ +# minimized from https://github.com/woodruffw/zizmor/pull/343 + +name: Release + +on: + push: + tags: + - "v*.*.*" + +jobs: + goreleaser: + runs-on: ubuntu-latest + permissions: + contents: write + steps: + # No finding, since cache is explicitly disabled. + - name: true-negative-1 + uses: actions/setup-go@v5 + with: + go-version: stable + cache: false + + # Finding because setup-go enables cache by default + - name: true-positive-2 + uses: actions/setup-go@v5 + with: + go-version: stable + + # Finding because setup enables cache explicitly + - name: true-positive-2 + uses: actions/setup-go@v5 + with: + go-version: stable + cache: true + + # Finding because setup enables cache explicitly + - name: true-positive-3 + uses: actions/setup-go@v5 + with: + go-version: stable + cache: "true"