diff --git a/.changeset/olive-moons-shave.md b/.changeset/olive-moons-shave.md new file mode 100644 index 000000000..876df2eb9 --- /dev/null +++ b/.changeset/olive-moons-shave.md @@ -0,0 +1,18 @@ +--- +'stash': patch +--- + +Correct the release-workflow section of the bundled `stash-supply-chain-security` +skill. It described the no-Actions-cache rule as a property of one file — "no +`cache:`, `package-manager-cache: false`, `pnpm/action-setup` with +`cache: false`" — which is no longer the whole rule. + +The gate now follows any local composite action or reusable workflow the job +reaches, so the constraint is on the whole call tree rather than the workflow +file. And every published `uses:` must appear in the script's `AUDITED_ACTIONS` +allowlist: the check cannot open a published action to prove it does not cache, +and caching actions are not reliably named — a `setup-` action that caches +by default has no `cache:` input and nothing in its name to match. The list is +therefore what is permitted, not what is forbidden, and adding a step to +`release.yml` or `tests-supply-chain.yml` means auditing the action and adding +it there in the same PR. diff --git a/.github/actions/integration-setup/action.yml b/.github/actions/integration-setup/action.yml index 171eddfc6..e248356bd 100644 --- a/.github/actions/integration-setup/action.yml +++ b/.github/actions/integration-setup/action.yml @@ -20,7 +20,7 @@ runs: - name: Checkout Repo uses: actions/checkout@v6 - - uses: pnpm/action-setup@v6.0.8 + - uses: pnpm/action-setup@v6.0.9 name: Install pnpm with: run_install: false diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/audited-actions.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/audited-actions.yml new file mode 100644 index 000000000..d801d96dc --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/audited-actions.yml @@ -0,0 +1,27 @@ +# Over-trigger guard. Every `uses:` here is on the allowlist and none of them +# caches, so the gate must stay silent — including on `changesets/action`, +# which is real (release.yml's publish step), third-party, and has nothing to +# do with caching. A rule that fired here would fire on the live release +# workflow. +name: Audited Actions +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: pnpm/action-setup@v6 + with: + run_install: false + cache: false + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 + package-manager-cache: false + - name: Publish to npm + uses: changesets/action@v1.9.0 + with: + publish: pnpm run release diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/cache-family.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/cache-family.yml new file mode 100644 index 000000000..e2512f423 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/cache-family.yml @@ -0,0 +1,17 @@ +# Three more vendors, none of them first-party, none with a `cache:` input. +# The point is that no rule keyed on a list of known cache actions would have +# had these on it — the list is open-ended and grows without this repo hearing +# about it. +name: Cache Family +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: buildjet/cache@v4 + - uses: runs-on/cache@v4 + - uses: tespkg/actions-cache@v1 + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cache-passthrough/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cache-passthrough/action.yml new file mode 100644 index 000000000..5f0b512d4 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cache-passthrough/action.yml @@ -0,0 +1,26 @@ +# The fail-closed half of the exemption `input-named-cache` earns. This +# composite forwards its `cache` input straight into a step that caches, which +# is what makes skipping the CALLER's `with.cache` safe rather than a hole: the +# justification for skipping is "the body is audited instead", and here the +# audit of the body is what produces the finding. +# +# `package-manager-cache: false` is set on purpose, and satisfies the +# explicit-`false` rule, so the single finding this fixture pins is the +# forwarded `cache:` — the very rule that is exempted at the caller, firing +# inside the composite where the caching actually happens. +name: Cache Passthrough +description: Forwards its `cache` input into actions/setup-node. +inputs: + cache: + description: Which package manager's cache to restore, or empty for none. + required: false + default: '' +runs: + using: composite + steps: + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 + cache: ${{ inputs.cache }} + package-manager-cache: false diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-restore/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-restore/action.yml new file mode 100644 index 000000000..03475b336 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-restore/action.yml @@ -0,0 +1,10 @@ +name: Cachey Restore +description: Restore-only half of the GitHub Actions cache. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache/restore@v4 + with: + path: index.node + key: binding-${{ runner.os }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-save/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-save/action.yml new file mode 100644 index 000000000..be9f469d3 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-save/action.yml @@ -0,0 +1,10 @@ +name: Cachey Save +description: Save-only half of the GitHub Actions cache. +runs: + using: composite + steps: + - name: Save the compiled binding + uses: actions/cache/save@v4 + with: + path: index.node + key: binding-${{ runner.os }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey/action.yml new file mode 100644 index 000000000..52105a5af --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey/action.yml @@ -0,0 +1,13 @@ +name: Cachey +description: Restores a build artifact from the GitHub Actions cache. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} + - name: Build the binding + shell: bash + run: echo build diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/clean-composite/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/clean-composite/action.yml new file mode 100644 index 000000000..44795cfcd --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/clean-composite/action.yml @@ -0,0 +1,16 @@ +name: Clean Composite +description: Sets up the toolchain with every cache explicitly disabled. +runs: + using: composite + steps: + - uses: pnpm/action-setup@v6 + with: + run_install: false + cache: false + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 + package-manager-cache: false + - shell: bash + run: pnpm install --frozen-lockfile diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/dir-action-yml/action.yml/.gitkeep b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/dir-action-yml/action.yml/.gitkeep new file mode 100644 index 000000000..2a48dff1d --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/dir-action-yml/action.yml/.gitkeep @@ -0,0 +1,3 @@ +This file exists only so git tracks the DIRECTORY it sits in, which is named +`action.yml` on purpose. Git cannot track an empty directory, and the directory +is the whole fixture: see composite-dir-action-yml.yml for what it pins. diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/input-named-cache/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/input-named-cache/action.yml new file mode 100644 index 000000000..bbc01411c --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/input-named-cache/action.yml @@ -0,0 +1,19 @@ +# Declares an input called `cache` and touches no cache at all. `with:` on a +# step that hands off to a local action is that action's declared inputs — an +# input named `cache` has no more to do with the GitHub Actions cache than one +# named `path` does, and a composite is free to declare either. +# +# The step-level twin of what `called-input-named-cache.yml` pins one level up. +name: Input Named Cache +description: Takes an input called `cache` and restores nothing. +inputs: + cache: + description: Reuse `index.node` from the working tree if it is already built. + required: false + default: 'false' +runs: + using: composite + steps: + - name: Build the binding + shell: bash + run: echo "build (reuse=${{ inputs.cache }})" diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/js-action/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/js-action/action.yml new file mode 100644 index 000000000..dec2ae413 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/js-action/action.yml @@ -0,0 +1,21 @@ +# A local action that is NOT a composite: `runs.using` names a Node runtime and +# there is no step list in this file or anywhere this gate can reach. Whatever +# `index.js` does with an input called `cache` is unreadable from here. +# +# So the `with.cache` heuristic stays on a caller of this action, and that is +# the whole reason the exemption is keyed on `runs.using: composite` rather than +# on "the `uses:` starts with `./`". A local `uses:` is already exempt from +# AUDITED_ACTIONS — exempt precisely because the gate audits its body instead — +# so on an action whose body cannot be audited, the caller's `with:` is the only +# signal left. Dropping it for every local `uses:` would make a two-line +# `action.yml` a supported way past the gate. +name: JS Action +description: A local JavaScript action, with no steps for this gate to audit. +inputs: + cache: + description: Passed straight to index.js, which this gate cannot read. + required: false + default: 'false' +runs: + using: node20 + main: index.js diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-a/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-a/action.yml new file mode 100644 index 000000000..47e63e1a4 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-a/action.yml @@ -0,0 +1,7 @@ +name: Loop A +description: Half of a cyclic composite reference. +runs: + using: composite + steps: + - name: Hand off to B + uses: ./.github/actions/loop-b diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-b/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-b/action.yml new file mode 100644 index 000000000..a74f8fa0e --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-b/action.yml @@ -0,0 +1,12 @@ +name: Loop B +description: Half of a cyclic composite reference, and it caches. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} + - name: Hand back to A + uses: ./.github/actions/loop-a diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/malformed/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/malformed/action.yml new file mode 100644 index 000000000..28de9741a --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/malformed/action.yml @@ -0,0 +1,19 @@ +# A composite manifest this gate cannot parse: the `steps:` list is mis-indented +# and js-yaml refuses the file. +# +# This is the unresolvable branch's twin — no step list comes out of it either +# way — so it is reported as un-auditable, and for the same reason it is NOT +# treated as a composite. `runs.using` is unreadable here, and assuming +# `composite` would suppress the caller's `with.cache` heuristic, which is the +# only signal left standing once the body cannot be read. +# +# The `actions/cache@v4` below is deliberate: it is what the caller would be +# hiding behind an unparseable file, and it must never be reported as found, +# because this gate never read it. +name: Malformed Composite +description: A composite action whose YAML does not parse. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/missing-explicit-false/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/missing-explicit-false/action.yml new file mode 100644 index 000000000..03119b4dd --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/missing-explicit-false/action.yml @@ -0,0 +1,12 @@ +name: Missing Explicit False +description: Sets up the toolchain but never says what it wants from the cache. +runs: + using: composite + steps: + - uses: pnpm/action-setup@v6 + with: + run_install: false + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/outer/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/outer/action.yml new file mode 100644 index 000000000..60bbd347d --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/outer/action.yml @@ -0,0 +1,7 @@ +name: Outer +description: A composite whose only job is to call another composite. +runs: + using: composite + steps: + - name: Delegate to the inner composite + uses: ./.github/actions/cachey diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/setup-node-cache/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/setup-node-cache/action.yml new file mode 100644 index 000000000..3735a0973 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/setup-node-cache/action.yml @@ -0,0 +1,10 @@ +name: Setup Node With Cache +description: Sets up Node with the package-manager cache switched on. +runs: + using: composite + steps: + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 + cache: 'pnpm' diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/thirdparty-cache/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/thirdparty-cache/action.yml new file mode 100644 index 000000000..44c476df0 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/thirdparty-cache/action.yml @@ -0,0 +1,12 @@ +name: Third Party Cache +description: Restores caches through actions that are not actions/cache. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: useblacksmith/cache@v5 + with: + path: index.node + key: binding-${{ runner.os }} + - name: Restore the Cargo build + uses: Swatinem/rust-cache@v2 diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/yaml-ext/action.yaml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/yaml-ext/action.yaml new file mode 100644 index 000000000..c3c051750 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/yaml-ext/action.yaml @@ -0,0 +1,10 @@ +name: Yaml Ext +description: Spelled `action.yaml`, which GitHub accepts alongside `action.yml`. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-passthrough.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-passthrough.yml new file mode 100644 index 000000000..8e21a9d0b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-passthrough.yml @@ -0,0 +1,19 @@ +# Same caller shape as `composite-input-named-cache.yml` — `with: {cache: true}` +# on a local composite — with the opposite verdict, because the composite hands +# the input to `actions/setup-node`. Exempting the caller must not lose the +# finding, and must not move it: the report names the step inside the composite, +# which is the step to edit. +name: Composite Cache Passthrough +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/cache-passthrough + with: + cache: true + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-restore.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-restore.yml new file mode 100644 index 000000000..485d22635 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-restore.yml @@ -0,0 +1,12 @@ +name: Composite Cache Restore +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/cachey-restore + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-save.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-save.yml new file mode 100644 index 000000000..3e2232b49 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-save.yml @@ -0,0 +1,12 @@ +name: Composite Cache Save +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/cachey-save + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache.yml new file mode 100644 index 000000000..8842e20e7 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache.yml @@ -0,0 +1,12 @@ +name: Composite Cache +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/cachey + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-clean.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-clean.yml new file mode 100644 index 000000000..2d5bb137b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-clean.yml @@ -0,0 +1,12 @@ +name: Composite Clean +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/clean-composite + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cyclic.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cyclic.yml new file mode 100644 index 000000000..7f484f0fc --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cyclic.yml @@ -0,0 +1,12 @@ +name: Composite Cyclic +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/loop-a + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-dir-action-yml.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-dir-action-yml.yml new file mode 100644 index 000000000..622af368e --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-dir-action-yml.yml @@ -0,0 +1,22 @@ +# `./.github/actions/dir-action-yml` holds a DIRECTORY named `action.yml`. +# +# `resolveActionFile` returned on a bare `existsSync`, so that directory passed +# as the manifest and `readFileSync` aborted the whole run with an unhandled +# EISDIR — while its sibling `resolveWorkflowFile` has guarded exactly this with +# `statSync(...).isFile()` since it was written. The asymmetry was the bug. +# +# Guarded, the directory is skipped, the next candidate name (`action.yaml`) is +# tried, no regular file is found, and this lands on the report that already +# exists for a local `uses:` pointing at nothing: exit 2, naming the path. +name: Composite Dir Action Yml +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/dir-action-yml + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-input-named-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-input-named-cache.yml new file mode 100644 index 000000000..ea317e149 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-input-named-cache.yml @@ -0,0 +1,23 @@ +# The reviewer's repro. A composite declaring a `cache` input, invoked with +# `cache: true`, was reported as ``with.cache: true` restores the GitHub Actions +# cache` — a finding about nothing, and the same false positive `walkJob` +# already refuses to make one level up by never reading a job-level `with:` as +# step inputs. +# +# The `cache` input here decides whether to reuse a binary already in the +# working tree. Nothing in the composite reaches the Actions cache, and this +# gate reads every one of its steps to know that. +name: Composite Input Named Cache +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/input-named-cache + with: + cache: true + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-leading-space.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-leading-space.yml new file mode 100644 index 000000000..e28f2d80e --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-leading-space.yml @@ -0,0 +1,16 @@ +# A quoted `uses:` with a leading space. GitHub trims it and resolves the +# composite; an untrimmed match against `^\.{1,2}/` does not, so the composite +# is never opened. Contrived on its own — but it was the one unfollowable local +# reference that exited 0 rather than 2, and silent-pass is the wrong direction +# for this gate. +name: Composite Leading Space +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - name: Build the protect-ffi binding + uses: ' ./.github/actions/cachey' + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-malformed.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-malformed.yml new file mode 100644 index 000000000..74daba491 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-malformed.yml @@ -0,0 +1,23 @@ +# The caller of a composite whose manifest does not parse, with +# `with: {cache: true}` on the step. +# +# Unguarded, the `yaml.load` of that manifest throws out of the whole run. Once +# it is caught, the null it returns has to fall through the same way the +# unresolvable branch above it does: no body was audited, so `checkStep` keeps +# every rule it has — including the `with.cache` heuristic — and that finding +# has to stay visible next to the exit-2 report rather than being swallowed by +# it. +name: Composite Malformed +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/malformed + with: + cache: true + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-missing-explicit-false.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-missing-explicit-false.yml new file mode 100644 index 000000000..d4f5ee922 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-missing-explicit-false.yml @@ -0,0 +1,12 @@ +name: Composite Missing Explicit False +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/missing-explicit-false + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-nested.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-nested.yml new file mode 100644 index 000000000..17a787a14 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-nested.yml @@ -0,0 +1,12 @@ +name: Composite Nested +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/outer + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-setup-node-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-setup-node-cache.yml new file mode 100644 index 000000000..fb0e3f8c6 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-setup-node-cache.yml @@ -0,0 +1,12 @@ +name: Composite Setup Node Cache +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/setup-node-cache + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-thirdparty-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-thirdparty-cache.yml new file mode 100644 index 000000000..9330e666e --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-thirdparty-cache.yml @@ -0,0 +1,11 @@ +name: Composite Third Party Cache +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - name: Build the protect-ffi binding + uses: ./.github/actions/thirdparty-cache + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable-with-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable-with-cache.yml new file mode 100644 index 000000000..e8ef65ca9 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable-with-cache.yml @@ -0,0 +1,19 @@ +# `composite-unresolvable.yml` with a `with: {cache: true}` on the broken +# reference. There is no manifest to open, so there is no body to audit and no +# way to know whether that input reaches a cache — the heuristic has to stay on, +# and its finding has to stay visible next to the exit-2 report rather than +# being swallowed by it. +name: Composite Unresolvable With Cache +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/no-such-action + with: + cache: true + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable.yml new file mode 100644 index 000000000..6052839a4 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable.yml @@ -0,0 +1,12 @@ +name: Composite Unresolvable +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/no-such-action + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-yaml-ext.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-yaml-ext.yml new file mode 100644 index 000000000..f3491fccc --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-yaml-ext.yml @@ -0,0 +1,12 @@ +name: Composite Yaml Ext +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/yaml-ext + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/local-js-action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/local-js-action.yml new file mode 100644 index 000000000..c7d8b9e6b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/local-js-action.yml @@ -0,0 +1,18 @@ +# A local `uses:` that resolves to a JavaScript action rather than a composite. +# The gate opens the manifest, finds no step list, and audits nothing — so it +# must keep the `with.cache` heuristic on this caller, unlike the composite +# callers next to it. +name: Local JS Action +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/js-action + with: + cache: true + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/mixed-unresolved-and-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/mixed-unresolved-and-cache.yml new file mode 100644 index 000000000..480d84e93 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/mixed-unresolved-and-cache.yml @@ -0,0 +1,22 @@ +# One run, both kinds of finding: an `actions/cache@v4` the gate read and +# judged, and a local reference it could not open at all. The un-auditable +# reference used to print and exit before the offender list was reached, so the +# actionable finding — the one with a step to delete — stayed hidden until the +# path was fixed, then arrived on the next run looking new. +name: Mixed Unresolved And Cache +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} + - name: Build the protect-ffi binding + uses: ./.github/actions/no-such-action + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/third-party-uses.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/third-party-uses.yml new file mode 100644 index 000000000..ca1f4a661 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/third-party-uses.yml @@ -0,0 +1,16 @@ +name: Third Party Uses +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + # None of these is a local path, so none of them has an `action.yml` on + # this filesystem to open. A traversal that treats every `uses:` as a + # path fails here rather than on the composite fixtures. + - uses: actions/checkout@v6 + - uses: jdx/mise-action@5228313ee0372e111a38da051671ca30fc5a96db # v3.6.3 + - uses: cipherstash/some-action/nested/path@v1 + - uses: docker://alpine:3.19 + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/malformed.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/malformed.yml new file mode 100644 index 000000000..624f4f4b7 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/malformed.yml @@ -0,0 +1,23 @@ +# A target this gate cannot parse. Unguarded, `yaml.load` throws straight out of +# the run: a stack trace instead of a finding, exit 1 — indistinguishable from +# "found a caching issue" — and every target listed after this one never scanned +# at all. +# +# An unparseable file is the same problem as a missing one, so it gets the same +# verdict: it hands the traversal no job list, so nothing below it is audited, +# and that is what the un-auditable list and exit 2 say. +# +# The error is a mis-indented `steps:`. js-yaml puts the position in the first +# line of its message and follows it with a multi-line source snippet, which is +# why only the first line reaches the report — the snippet carries its own +# indentation and would wreck the two-space bullets. +name: Malformed +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/parent-uses-step.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/parent-uses-step.yml new file mode 100644 index 000000000..7a5f0e67d --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/parent-uses-step.yml @@ -0,0 +1,22 @@ +# A step-level `uses: ../…`. GitHub does not accept one — a reference to a +# directory in the checkout must start with `./` — but `LOCAL_USES` was +# `^\.{1,2}/`, which contradicted the comment directly above it and read this as +# local. Local means two things here, and both were wrong for `../`: exempt from +# `AUDITED_ACTIONS`, and handed to `resolveActionFile`, which `resolve()`s it +# clean out of the workspace root. A file OUTSIDE the checkout could be opened +# and audited as though it were in it, with `../..` trails in the report. +# +# It is now reported and never resolved. That is the only fail-closed reading: +# the gate opens no step list here, so it cannot prove this step does not cache. +name: Parent Uses Step +on: + push: + branches: [main] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ../outside-action + - run: pnpm publish --no-git-checks diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/actions/cachey/action.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/actions/cachey/action.yml new file mode 100644 index 000000000..5dca85512 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/actions/cachey/action.yml @@ -0,0 +1,10 @@ +name: Cachey +description: Restores a build artifact from the GitHub Actions cache. +runs: + using: composite + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-cache.yml new file mode 100644 index 000000000..1cd4a1cde --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-cache.yml @@ -0,0 +1,14 @@ +name: Called Cache +on: + workflow_call: +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-clean.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-clean.yml new file mode 100644 index 000000000..b411534e0 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-clean.yml @@ -0,0 +1,18 @@ +name: Called Clean +on: + workflow_call: +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: pnpm/action-setup@v6 + with: + run_install: false + cache: false + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 + package-manager-cache: false + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-composite.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-composite.yml new file mode 100644 index 000000000..79a30b497 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-composite.yml @@ -0,0 +1,14 @@ +# The composed hop: a reusable workflow whose job reaches the cache through a +# local composite. Neither traversal alone sees this — the job-level one stops +# at the step list, the composite one is never reached. +name: Called Composite +on: + workflow_call: +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/cachey + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-input-named-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-input-named-cache.yml new file mode 100644 index 000000000..825b4f602 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-input-named-cache.yml @@ -0,0 +1,21 @@ +# Declares an input called `cache` and does not cache. A job-level `with:` is +# inputs to a reusable workflow, not `with:` on an action step — reading it +# with the step rules would flag this caller for passing `cache: true` to an +# input that switches something else on entirely. +name: Called Input Named Cache +on: + workflow_call: + inputs: + cache: + type: boolean + default: false +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: pnpm/action-setup@v6 + with: + run_install: false + cache: false + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-a.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-a.yml new file mode 100644 index 000000000..604c6684a --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-a.yml @@ -0,0 +1,7 @@ +name: Called Loop A +on: + workflow_call: +jobs: + a: + uses: ./.github/workflows/called-loop-b.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-b.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-b.yml new file mode 100644 index 000000000..4766fa1ea --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-b.yml @@ -0,0 +1,15 @@ +name: Called Loop B +on: + workflow_call: +jobs: + b-caches: + runs-on: ubuntu-latest + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 + with: + path: index.node + key: binding-${{ runner.os }} + b-back: + uses: ./.github/workflows/called-loop-a.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-malformed.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-malformed.yml new file mode 100644 index 000000000..7f459e5d3 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-malformed.yml @@ -0,0 +1,17 @@ +# A called workflow this gate cannot parse — the third unguarded `yaml.load`, +# and the same failure one node type over: unguarded it throws out of the run, +# guarded it hands the traversal no job list, so the hop stops here and the +# reference is reported as un-auditable. +# +# The `actions/cache@v4` below is deliberate. It is exactly what an unparseable +# called workflow would hide, and the gate must never claim to have found it — +# it never read this file. +name: Called Malformed +on: + workflow_call: +jobs: + release: + runs-on: ubuntu-latest + steps: + - name: Restore the compiled binding + uses: actions/cache@v4 diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-missing-explicit-false.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-missing-explicit-false.yml new file mode 100644 index 000000000..88c836c9b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-missing-explicit-false.yml @@ -0,0 +1,14 @@ +name: Called Missing Explicit False +on: + workflow_call: +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: pnpm/action-setup@v6 + with: + run_install: false + - name: Install Node.js + uses: actions/setup-node@v6 + with: + node-version: 22 diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-outer.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-outer.yml new file mode 100644 index 000000000..37d9c37f9 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-outer.yml @@ -0,0 +1,7 @@ +name: Called Outer +on: + workflow_call: +jobs: + forward: + uses: ./.github/workflows/called-cache.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-thirdparty-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-thirdparty-cache.yml new file mode 100644 index 000000000..341a8c1bd --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-thirdparty-cache.yml @@ -0,0 +1,11 @@ +name: Called Third Party Cache +on: + workflow_call: +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Restore the Cargo build + uses: Swatinem/rust-cache@v2 + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-both.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-both.yml new file mode 100644 index 000000000..f64c49d4e --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-both.yml @@ -0,0 +1,19 @@ +# `steps:` and `uses:` together is invalid to GitHub's schema, which rejects +# the file before any of it runs. This gate runs on files GitHub has not +# validated yet, so it checks both rather than picking one: treating either key +# as authoritative would let an invalid file hide a cache behind the key the +# gate chose to ignore, and report a pass. +name: Reusable Both +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-cache.yml + secrets: inherit + steps: + - name: Restore the toolchain + uses: actions/cache@v4 + with: + path: ~/.cache + key: toolchain-${{ runner.os }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cache.yml new file mode 100644 index 000000000..56706df9b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cache.yml @@ -0,0 +1,11 @@ +# The shape that matters: a credential-bearing publishing job that is nothing +# but a call into another workflow. It has no `steps:` at all, so the gate used +# to iterate an empty list and print OK over a cache restore. +name: Reusable Cache +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-cache.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-clean.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-clean.yml new file mode 100644 index 000000000..163309d25 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-clean.yml @@ -0,0 +1,8 @@ +name: Reusable Clean +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-clean.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-composite.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-composite.yml new file mode 100644 index 000000000..37ebea168 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-composite.yml @@ -0,0 +1,8 @@ +name: Reusable Composite +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-composite.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cyclic.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cyclic.yml new file mode 100644 index 000000000..8fe76d263 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cyclic.yml @@ -0,0 +1,8 @@ +name: Reusable Cyclic +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-loop-a.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-explicit-secrets.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-explicit-secrets.yml new file mode 100644 index 000000000..a853d45c4 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-explicit-secrets.yml @@ -0,0 +1,12 @@ +# Secrets named one by one rather than inherited. Same verdict as +# `secrets: inherit` and as no secrets at all — see the test that runs all +# three. +name: Reusable Explicit Secrets +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-cache.yml + secrets: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-input-named-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-input-named-cache.yml new file mode 100644 index 000000000..8aac9cbe8 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-input-named-cache.yml @@ -0,0 +1,10 @@ +name: Reusable Input Named Cache +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-input-named-cache.yml + secrets: inherit + with: + cache: true diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-malformed.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-malformed.yml new file mode 100644 index 000000000..05ceba95b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-malformed.yml @@ -0,0 +1,12 @@ +# A job that delegates its whole body to a called workflow whose YAML does not +# parse. The job has no `steps:` of its own, so the un-auditable report is the +# only thing between this file and a silent `OK` — and unguarded it was not a +# report at all but a stack trace. +name: Reusable Malformed +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-malformed.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-missing-explicit-false.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-missing-explicit-false.yml new file mode 100644 index 000000000..5838c0ed6 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-missing-explicit-false.yml @@ -0,0 +1,8 @@ +name: Reusable Missing Explicit False +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-missing-explicit-false.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-nested.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-nested.yml new file mode 100644 index 000000000..2508bc6fa --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-nested.yml @@ -0,0 +1,8 @@ +name: Reusable Nested +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-outer.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-no-secrets.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-no-secrets.yml new file mode 100644 index 000000000..d77ce26b4 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-no-secrets.yml @@ -0,0 +1,10 @@ +# No `secrets:` key. The called workflow still inherits `permissions:` — which +# is how npm trusted publishing mints its OIDC token — so "no secrets" is not +# "no credentials". +name: Reusable No Secrets +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-cache.yml diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-parent-uses.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-parent-uses.yml new file mode 100644 index 000000000..82e8dadf8 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-parent-uses.yml @@ -0,0 +1,17 @@ +# The job-level twin of `parent-uses-step.yml`. A `../` reference is not a +# workflow in this checkout, and GitHub does not accept one — so the job is +# reported as un-auditable rather than resolved, which under the old +# `^\.{1,2}/` reading would have pointed `resolveWorkflowFile` at a path outside +# the workspace root. +# +# The verdict has to name what this actually is. Falling through to the remote +# branch called it "a remote reusable workflow", which it is not: nothing here +# is remote, the reference is simply invalid and unfollowable. +name: Reusable Parent Uses +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ../outside-workflow.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-remote.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-remote.yml new file mode 100644 index 000000000..c2bae01fb --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-remote.yml @@ -0,0 +1,13 @@ +# The reference is the one live job-level `uses:` in this repo, lifted from +# .github/workflows/osv-scanner.yml, with `secrets: inherit` added to make the +# hazard shape explicit. osv-scanner.yml is not a target of this gate; this +# fixture is about what the gate should say if a *publishing* workflow ever +# delegates a whole job off this filesystem. +name: Reusable Remote +on: + push: + tags: ['v*'] +jobs: + publish: + uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@v2.3.8 + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-thirdparty-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-thirdparty-cache.yml new file mode 100644 index 000000000..a50463efa --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-thirdparty-cache.yml @@ -0,0 +1,8 @@ +name: Reusable Third Party Cache +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/called-thirdparty-cache.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-unresolvable.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-unresolvable.yml new file mode 100644 index 000000000..ad0ccae0b --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-unresolvable.yml @@ -0,0 +1,8 @@ +name: Reusable Unresolvable +on: + push: + tags: ['v*'] +jobs: + publish: + uses: ./.github/workflows/no-such-workflow.yml + secrets: inherit diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/thirdparty-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/thirdparty-cache.yml new file mode 100644 index 000000000..391079b25 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/thirdparty-cache.yml @@ -0,0 +1,27 @@ +# The gap this fixture pins: neither of these steps matches +# `^actions/cache(/(restore|save))?@`, and neither carries a `cache:` input for +# the `with.cache` rule to read. Both restore the GitHub Actions cache anyway. +# +# Not hypothetical. This repo runs eleven jobs on `blacksmith-*` runners, where +# `useblacksmith/cache` is the documented drop-in for `actions/cache`, and it +# has just absorbed a Cargo workspace at `packages/protect-ffi`, where +# `Swatinem/rust-cache` is the conventional choice. Either could be added by +# someone who reasonably believed this gate would catch the mistake. +name: Third Party Cache +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Restore the compiled binding + uses: useblacksmith/cache@v5 + with: + path: index.node + key: binding-${{ runner.os }} + # No `with:` at all — there is no input here for any rule to inspect. + - name: Restore the Cargo build + uses: Swatinem/rust-cache@v2 + - run: pnpm changeset:publish diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/unaudited-setup.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/unaudited-setup.yml new file mode 100644 index 000000000..c7f7e60cc --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/unaudited-setup.yml @@ -0,0 +1,19 @@ +# The case that decides the design. `gradle/actions/setup-gradle` restores the +# Actions cache BY DEFAULT, has no `cache:` input to read, and has no "cache" +# anywhere in its name — so a rule that enumerates cache actions, by exact name +# or by name shape, cannot see it. The allowlist can: it is not on the list. +# +# This class is already present in this file as two hand-maintained entries +# (`pnpm/action-setup`, `actions/setup-node`) that exist only because somebody +# happened to notice. This fixture is the third member nobody noticed. +name: Unaudited Setup +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: gradle/actions/setup-gradle@v4 + - run: pnpm changeset:publish diff --git a/scripts/__tests__/integration-workflow-paths.test.mjs b/scripts/__tests__/integration-workflow-paths.test.mjs index d47c67bba..76fd71ef7 100644 --- a/scripts/__tests__/integration-workflow-paths.test.mjs +++ b/scripts/__tests__/integration-workflow-paths.test.mjs @@ -255,6 +255,20 @@ describe('integration workflow paths filters', () => { for (const target of importedSourcePaths(file)) required.add(target) } + // The requirement is DERIVED from `@/`-aliased imports, so "no suite + // uses that alias" and "every import is covered" are the same green. + // Mutation-tested: rewriting the suites' `from '@/…'` to the public + // `@cipherstash/stack/…` entry — an ordinary "test the built package, + // not internals" refactor — empties this set, and the check then passed + // with `packages/stack/src/dynamodb/**` deleted from the filter, which + // is verbatim the #815 gap described at the top of this file. The + // sibling manifest check above already asserts its own premise; this one + // has to as well. + expect( + required.size, + `No @/-aliased import was resolved from ${files.length} suite file(s) in ${relPath}, so this check has nothing to verify and would pass no matter what the paths filter said. If the suites moved off the @/ alias, teach importedSourcePaths the new form.`, + ).toBeGreaterThan(0) + for (const block of blocks) { const uncovered = [...required].filter( (target) => !isCovered(target, block.paths), diff --git a/scripts/__tests__/lint-no-workflow-caching.test.mjs b/scripts/__tests__/lint-no-workflow-caching.test.mjs index 261fa228b..193bf3190 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -92,6 +92,615 @@ describe('lint-no-workflow-caching', () => { ).toBe(0) }) + // The gate read a step's own `uses:` and stopped there, so a workflow could + // reach `actions/cache` through one indirection — `uses: ./.github/actions/x` + // — and stay green. Verified against a copy of release.yml with a + // cache-restoring composite spliced in: exit 0, no output, while the + // composite it never opened restores two caches. + describe('local composite actions', () => { + const cfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/composites/.github/workflows/${name}`, + ) + + it('follows a local composite into its `actions/cache` step', () => { + const r = run(cfx('composite-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache@/) + }) + + // A message naming only the workflow step sends the reader to a file with + // no `actions/cache` anywhere in it. Both ends of the indirection, or the + // finding costs more to act on than it saves. + it('names the workflow step and the offending composite step', () => { + const r = run(cfx('composite-cache.yml')) + expect(r.output).toMatch(/step "Build the protect-ffi binding"/) + expect(r.output).toMatch( + /\.github\/actions\/cachey\/action\.yml step "Restore the compiled binding"/, + ) + }) + + it('follows a local composite into `actions/cache/restore`', () => { + const r = run(cfx('composite-cache-restore.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache\/restore@/) + }) + + it('follows a local composite into `actions/cache/save`', () => { + const r = run(cfx('composite-cache-save.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache\/save@/) + }) + + it('passes on a composite that disables caching explicitly', () => { + expect(run(cfx('composite-clean.yml')).exitCode).toBe(0) + }) + + it('flags a truthy `with.cache` inside a composite', () => { + const r = run(cfx('composite-setup-node-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/with\.cache/) + }) + + // The explicit-`false` rules apply through the indirection too: the action + // executes in the same job, with the same credentials, and defaults the + // same way. Exempting composites would make "move the step into a + // composite" a silent way out of the rule — the bug this whole block fixes. + it('applies the explicit-`false` rules inside a composite', () => { + const r = run(cfx('composite-missing-explicit-false.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/pnpm\/action-setup/) + expect(r.output).toMatch(/package-manager-cache/) + }) + + it('recurses into a composite reached from a composite', () => { + const r = run(cfx('composite-nested.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache@/) + // The whole chain, not just its ends — otherwise the reader has to guess + // which of `outer`'s steps led to the cache. + expect(r.output).toMatch( + /step "Build the protect-ffi binding".*outer\/action\.yml step "Delegate to the inner composite".*cachey\/action\.yml step "Restore the compiled binding"/, + ) + }) + + // `execFileSync` has no timeout here, and does not need one: the script is + // synchronous end to end, so it has no way to hang. Deleting the `visited` + // guard was tried — the recursion blows the stack and the child exits in + // well under a second, which a timeout would not improve on. + // + // The offender count is the real assertion, because that exit is a bare 1 + // and so is a genuine finding. A visited set that is per-branch rather than + // per-run terminates too, but reports `loop-b` once per path into it. + it('terminates on a cyclic composite reference, reporting once', () => { + const r = run(cfx('composite-cyclic.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/Found 1 caching issue/) + }) + + it('reads `action.yaml` as well as `action.yml`', () => { + const r = run(cfx('composite-yaml-ext.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache@/) + }) + + // Marketplace and `docker://` references have no `action.yml` on this + // filesystem, and the traversal must not try to open them — a traversal + // that treats every `uses:` as a path fails here rather than on the + // composite fixtures. + // + // The assertion used to be exit 0. It is no longer, because the allowlist + // below reports each of these as unaudited; what is asserted instead is the + // property this test was written for, and more directly than exit 0 did: + // no message says the path could not be opened. Exit 1, not 2, is the other + // half — the gate reached a verdict on every step, it did not fail to look. + it('never tries to open a `uses:` that is not a local path', () => { + const r = run(cfx('third-party-uses.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).not.toMatch(/no action\.yml or action\.yaml there/) + expect(r.output).toMatch(/docker:\/\/alpine:3\.19/) + }) + + // GitHub trims a `uses:` before resolving it. Untrimmed, `" ./x"` fails the + // `^\.{1,2}/` test, so the composite was never opened — and nothing was + // reported either, making this the one unfollowable local reference shape + // that exited 0 instead of 2. Trimming makes it followable, which is the + // right outcome: this is a valid reference GitHub runs. + it('follows a local composite whose `uses:` has leading whitespace', () => { + const r = run(cfx('composite-leading-space.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache@/) + expect(r.output).toMatch( + /\.github\/actions\/cachey\/action\.yml step "Restore the compiled binding"/, + ) + }) + + // Exit 2, not 1: nothing was found to be caching, the linter simply could + // not look — the same contract lint-no-hardcoded-runners.mjs uses for a + // target that does not exist. Silently skipping would turn a typo'd path + // into a permanent exemption. + it('exits 2 when a local `uses:` resolves to no action file', () => { + const r = run(cfx('composite-unresolvable.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/no-such-action/) + }) + + // The sibling asymmetry, and the bug in it. `resolveWorkflowFile` has + // guarded `isFile` since it was written; `resolveActionFile` returned on a + // bare `existsSync`, so a DIRECTORY named `action.yml` passed as the + // manifest and `readFileSync` aborted the run with an unhandled EISDIR. + // Guarded, the directory is skipped, `action.yaml` is tried, and this lands + // on the report that already exists for a local `uses:` pointing at + // nothing — the accurate message rather than a stack trace. + it('skips a directory named `action.yml` rather than reading it', () => { + const r = run(cfx('composite-dir-action-yml.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/dir-action-yml/) + expect(r.output).toMatch(/no action\.yml or action\.yaml there/) + }) + }) + + // The step-level twin of the false positive `reusable-input-named-cache` + // pins one level up. `with:` on a step that hands off to a LOCAL action is + // that action's declared inputs, and a composite is free to declare one + // called `cache` — so the step rules reported ``with.cache: true` restores + // the GitHub Actions cache` about an input that restores nothing. + // + // The exemption is narrow on purpose, because its justification is "the body + // is audited instead", not "local is trusted". It applies only where this + // gate actually opens the action and reads its steps — a resolved + // `runs.using: composite`. A local `uses:` that resolves to nothing, or to a + // JS or Docker action with no step list, keeps the heuristic, because there + // the heuristic is the only signal the gate has: a local `uses:` is already + // exempt from AUDITED_ACTIONS as well. + describe('`with.cache` on a step that hands off to a local action', () => { + const cfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/composites/.github/workflows/${name}`, + ) + + it("does not read a composite's declared inputs as step inputs", () => { + expect(run(cfx('composite-input-named-cache.yml')).exitCode).toBe(0) + }) + + // The guard that makes the exemption safe rather than a hole, and the + // reason it may only apply where the body is read: a composite that + // forwards its `cache` input into a step that caches is still a finding, + // and the finding names the step doing the caching rather than the caller + // that switched it on. One issue, not two — the caller is not a second + // problem to fix. + it('still flags a composite that forwards `cache` into a caching step', () => { + const r = run(cfx('composite-cache-passthrough.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/Found 1 caching issue/) + expect(r.output).toMatch( + /cache-passthrough\/action\.yml step "Install Node\.js": `with\.cache/, + ) + expect(r.output).not.toMatch( + /step "Build the protect-ffi binding": `with\.cache/, + ) + }) + + // `runs.using: node20`, `main: index.js` — the gate opens the manifest, + // finds no step list, and audits nothing. Since a local `uses:` is already + // exempt from AUDITED_ACTIONS, the caller's `with:` is all that is left + // standing here. + it('keeps the `with.cache` rule on a local JavaScript action', () => { + const r = run(cfx('local-js-action.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/with\.cache/) + }) + + // No manifest to open means no body to audit, so the heuristic stays on — + // and its finding has to stay visible next to the exit-2 report rather + // than being swallowed by it. + it('keeps the `with.cache` rule on a local `uses:` resolving to nothing', () => { + const r = run(cfx('composite-unresolvable-with-cache.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/with\.cache/) + expect(r.output).toMatch(/no action\.yml or action\.yaml there/) + }) + }) + + // The second indirection the composite traversal left open. A job that calls + // a reusable workflow has no `steps:` — it is `jobs..uses` plus `with:` / + // `secrets:` — so `Array.isArray(job?.steps) ? job.steps : []` yielded an + // empty list and skipped the job whole. Verified before the fix against a + // caller whose job was `uses: ./.github/workflows/reusable.yml` with + // `secrets: inherit`, the called workflow holding an `actions/cache@v4` step: + // exit 0, `OK`, nothing scanned. + describe('reusable workflows', () => { + const rfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/reusable/.github/workflows/${name}`, + ) + + it('follows a job-level `uses:` into the called workflow', () => { + const r = run(rfx('reusable-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/actions\/cache@/) + }) + + // The called workflow adds a job of its own between the caller's job and + // the step, so a message that names only the caller job sends the reader to + // a file with no `actions/cache` in it — and, worse, no `steps:` either. + it('names the caller job, the called workflow, and its job and step', () => { + const r = run(rfx('reusable-cache.yml')) + expect(r.output).toMatch( + /job "publish" -> \.github\/workflows\/called-cache\.yml job "release" step "Restore the compiled binding"/, + ) + }) + + // The case that proves the two traversals compose rather than each handling + // only its own shape: the workflow hop lands on a job whose step hands off + // to a local composite, and the cache is inside that. + it('composes with the composite traversal: workflow -> composite -> cache', () => { + const r = run(rfx('reusable-composite.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch( + /job "publish" -> \.github\/workflows\/called-composite\.yml job "build" step "Build the protect-ffi binding" -> \.github\/actions\/cachey\/action\.yml step "Restore the compiled binding"/, + ) + }) + + it('follows a called workflow that itself calls a called workflow', () => { + const r = run(rfx('reusable-nested.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch( + /job "publish" -> \.github\/workflows\/called-outer\.yml job "forward" -> \.github\/workflows\/called-cache\.yml job "release" step "Restore the compiled binding"/, + ) + }) + + // Same reasoning 402af3f3 recorded for composites: the called workflow's + // steps default the same way, so exempting them would make "move the step + // into a reusable workflow" a supported way out of the rule — one level up + // from the way out that commit closed. + it('applies the explicit-`false` rules inside a called workflow', () => { + const r = run(rfx('reusable-missing-explicit-false.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/pnpm\/action-setup/) + expect(r.output).toMatch(/package-manager-cache/) + }) + + it('passes on a called workflow that disables caching explicitly', () => { + expect(run(rfx('reusable-clean.yml')).exitCode).toBe(0) + }) + + // A job-level `with:` is inputs to the called workflow, not `with:` on an + // action step. Running the step rules over the job object would flag this + // caller for passing `cache: true` to an input named `cache` — a finding + // about nothing, in the file least able to act on it. + it('does not read a called workflow`s inputs as step inputs', () => { + expect(run(rfx('reusable-input-named-cache.yml')).exitCode).toBe(0) + }) + + // The verdict does not turn on how credentials reach the called workflow. + // `secrets:` is not the only channel — `permissions:` is inherited + // independently, and that is what mints the OIDC token npm trusted + // publishing signs with, so a call passing no secrets at all can still + // publish. A restore also does not need credentials in its own job to be + // the attack: poisoned bytes landing in a build job that hands an artifact + // to a publish job is the canonical shape. Conditioning on `secrets:` would + // buy nothing and hand an attacker a phrasing that evades the gate. + it('flags the cache however credentials are passed, or not passed', () => { + for (const name of [ + 'reusable-cache.yml', // secrets: inherit + 'reusable-explicit-secrets.yml', // secrets: {NPM_TOKEN: ...} + 'reusable-no-secrets.yml', // no secrets: key at all + ]) { + expect(run(rfx(name)).exitCode, name).toBe(1) + } + }) + + // `execFileSync` has no timeout here, so an unguarded cycle hangs the suite + // rather than failing it. The count is the real assertion — a visited set + // that does not span the workflow hops terminates but re-reports the cache + // once per path into it. + it('terminates on a cyclic reusable reference, reporting once', () => { + const r = run(rfx('reusable-cyclic.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/Found 1 caching issue/) + }) + + it('exits 2 when a job-level `uses:` resolves to no workflow file', () => { + const r = run(rfx('reusable-unresolvable.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/no-such-workflow/) + }) + + // A remote reusable workflow is reported, where a remote *step* action is + // skipped, and the difference is coverage rather than depth. A marketplace + // step sits inside a job whose step list the gate has read end to end; the + // residual risk is bounded, and reporting every `actions/checkout@v6` would + // make the gate exit 2 forever and mean nothing. A remote job-level `uses:` + // is the whole job — the gate reads no steps, reaches no verdict, and + // prints OK anyway. That is the exact failure this file exists to stop: a + // check that never ran reads like a check that passed. It is also + // actionable and rare (zero today), so the report is signal, not noise — + // inline the job, or point it at a workflow in this checkout. + it('reports a remote reusable workflow as un-auditable rather than passing it', () => { + const r = run(rfx('reusable-remote.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/osv-scanner-reusable\.yml@v2\.3\.8/) + expect(r.output).toMatch(/not in this checkout/) + }) + + // Never reached on GitHub — the schema rejects `steps:` and `uses:` on one + // job — but this gate runs on files GitHub has not validated yet. Treating + // either key as authoritative would let an invalid file hide a cache behind + // the key the gate chose to ignore and still report a pass, so both are + // checked and both are counted. + it('checks both halves of a job carrying `steps:` and `uses:`', () => { + const r = run(rfx('reusable-both.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/Found 2 caching issue/) + expect(r.output).toMatch(/step "Restore the toolchain"/) + expect(r.output).toMatch(/called-cache\.yml job "release"/) + }) + }) + + // Every `yaml.load` in the script was unguarded, so a file this gate cannot + // parse — malformed YAML, EISDIR, EACCES — threw straight out of the run: a + // stack trace instead of a finding, exit 1 (indistinguishable from "found a + // caching issue"), and every remaining target never scanned at all. + // + // An unparseable file is the same problem as a missing one. It hands the + // traversal no step list, so nothing below it is audited — which is exactly + // what the un-auditable list, exit 2, and the "this gate could not look" + // epilogue are for. + describe('a file this gate cannot parse', () => { + const cfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/composites/.github/workflows/${name}`, + ) + const rfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/reusable/.github/workflows/${name}`, + ) + + it('reports an unparseable target rather than crashing on it', () => { + const r = run(fx('malformed.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/Found 1 un-auditable reference/) + expect(r.output).toMatch( + /malformed\.yml — could not be read: bad indentation of a mapping entry \(\d+:\d+\)/, + ) + }) + + // js-yaml's message is a position line followed by a multi-line source + // snippet — numbered lines and a caret ruler, carrying their own + // indentation. Pasting all of it into a two-space bullet wrecks the report, + // so the first line is kept and the snippet dropped. The position is the + // half that lets someone fix the file. + it('keeps the parse error’s position and drops its source snippet', () => { + const r = run(fx('malformed.yml')) + expect(r.output).toMatch(/\(\d+:\d+\)/) + expect(r.output).not.toMatch(/-{4,}\^/) + expect(r.output).not.toMatch(/^\s*\d+ \| /m) + }) + + // The target loop died on the first unparseable file, so a malformed + // release.yml meant tests-supply-chain.yml was never scanned — the gate + // silently stopped covering the workflow it could still read. + it('keeps scanning the remaining targets after an unparseable one', () => { + const r = run(fx('malformed.yml'), fx('actions-cache.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/could not be read/) + expect(r.output).toMatch(/Found 1 caching issue/) + expect(r.output).toMatch(/actions\/cache@/) + }) + + // A manifest that does not parse is a NON-composite manifest, for the same + // reason the unresolvable branch beside it is: no body was audited, so + // every step rule stays on, `with.cache` included. The `actions/cache@v4` + // inside the unreadable file must not be reported as found — this gate + // never read it, and claiming otherwise would be a guess. + it('keeps every step rule on a caller whose composite does not parse', () => { + const r = run(cfx('composite-malformed.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch( + /\.github\/actions\/malformed\/action\.yml — could not be read:/, + ) + expect(r.output).toMatch(/with\.cache/) + expect(r.output).not.toMatch(/actions\/cache@v4/) + }) + + it('stops the workflow hop at a called workflow that does not parse', () => { + const r = run(rfx('reusable-malformed.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/Found 1 un-auditable reference/) + expect(r.output).toMatch( + /\.github\/workflows\/called-malformed\.yml — could not be read:/, + ) + expect(r.output).not.toMatch(/actions\/cache@v4/) + }) + }) + + // `LOCAL_USES` was `^\.{1,2}/`, which accepted `../` while the comment + // directly above it said GitHub requires the `./` prefix. GitHub does not + // accept `../`, and reading one as local meant two wrong things at once: + // exempt from `AUDITED_ACTIONS`, and handed to a resolver that `resolve()`s + // it clean out of the workspace root — so a file OUTSIDE the checkout could + // be opened and audited as though it were inside it. + describe('a `../` reference', () => { + const rfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/reusable/.github/workflows/${name}`, + ) + + // Falling through to the unaudited branch is fail-closed, so the exit code + // was never the problem — the wording was. "This gate cannot read a + // published action's steps" sends the reader to audit a published action + // that does not exist; the finding is that the reference itself is one + // GitHub will not run and this gate will not follow. + it('reports a step-level `../` instead of resolving it', () => { + const r = run(fx('parent-uses-step.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch( + /uses `\.\.\/outside-action` — a `\.\.\/` reference/, + ) + expect(r.output).not.toMatch( + /uses `\.\.\/outside-action` — not in AUDITED_ACTIONS/, + ) + // Reported, never resolved: nothing outside the workspace root is opened. + expect(r.output).not.toMatch(/no action\.yml or action\.yaml there/) + }) + + // Same at job level, where the fall-through called it "a remote reusable + // workflow". Nothing here is remote — the reference is simply invalid. + it('reports a job-level `../` instead of resolving it', () => { + const r = run(rfx('reusable-parent-uses.yml')) + expect(r.exitCode).toBe(2) + expect(r.output).toMatch( + /`uses: \.\.\/outside-workflow\.yml` — a `\.\.\/` reference/, + ) + expect(r.output).not.toMatch( + /outside-workflow\.yml` — a remote reusable workflow/, + ) + expect(r.output).not.toMatch(/no workflow file there/) + }) + }) + + // The gap both traversals ran straight past. `CACHE_ACTION` recognised only + // GitHub's first-party cache action, and the `with.cache` rule only fires on + // an action that takes a `cache:` input — so a third-party cache action with + // neither was invisible to every rule in the file. Reproduced before the fix + // against a composite reached from a targeted workflow holding + // `useblacksmith/cache@v5` and `Swatinem/rust-cache@v2`: `OK`, exit 0. + // + // The fix is not two more names on a regex. See the ALLOWLIST RATIONALE in + // the script: every remote `uses:` reachable from a targeted workflow must be + // on `AUDITED_ACTIONS`, so the gate fails CLOSED on an action it has never + // met — which is the only posture that survives the next vendor. + describe('third-party cache actions and unaudited `uses:`', () => { + const cfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/composites/.github/workflows/${name}`, + ) + const rfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/reusable/.github/workflows/${name}`, + ) + + it('flags a third-party cache action at workflow level', () => { + const r = run(fx('thirdparty-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/useblacksmith\/cache@v5/) + expect(r.output).toMatch(/Swatinem\/rust-cache@v2/) + }) + + // Two rules could fire on `useblacksmith/cache`: it is cache-shaped AND it + // is unaudited. The cache-shaped message wins, because "this restores a + // cache" is what the reader needs to act on — "this is not on a list" + // invites adding it to the list. + it('names a cache-shaped action as a cache, not merely as unaudited', () => { + const r = run(fx('thirdparty-cache.yml')) + expect(r.output).toMatch( + /useblacksmith\/cache@v5` — a third-party cache action/, + ) + }) + + // `Swatinem/rust-cache` is the check the shape heuristic had to survive: a + // segment-equality test (`owner/cache`) misses it, a substring test does + // not. Verified, not assumed. + it('flags a cache action whose name is not exactly `cache`', () => { + const r = run(fx('cache-family.yml')) + expect(r.exitCode).toBe(1) + for (const name of [ + 'buildjet/cache', + 'runs-on/cache', + 'tespkg/actions-cache', + ]) { + expect(r.output, name).toContain(name) + } + }) + + // The case the allowlist exists for, and the argument against a denylist of + // any kind. `gradle/actions/setup-gradle` caches by default, has no `cache:` + // input, and has no "cache" in its name — so no enumeration of cache + // actions can see it. The message must say unaudited, not cache: claiming + // it caches would be a guess, and the finding is true either way. + it('flags a caching setup action that no cache-name rule could see', () => { + const r = run(fx('unaudited-setup.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/gradle\/actions\/setup-gradle@v4/) + expect(r.output).toMatch(/AUDITED_ACTIONS/) + expect(r.output).not.toMatch(/setup-gradle@v4` — a third-party cache/) + }) + + // Over-trigger guard. Every `uses:` here is allowlisted and none caches, + // `changesets/action` most of all: it is third-party, it is real + // (release.yml's publish step), and its name is nowhere near "cache". A + // rule that fired here would fire on the live release workflow — which is + // what the two live-target assertions above independently confirm. + it('does not flag audited actions, including third-party ones', () => { + expect(run(fx('audited-actions.yml')).exitCode).toBe(0) + }) + + it('flags a third-party cache action inside a local composite', () => { + const r = run(cfx('composite-thirdparty-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch( + /step "Build the protect-ffi binding" -> \.github\/actions\/thirdparty-cache\/action\.yml step "Restore the compiled binding"/, + ) + expect(r.output).toMatch(/useblacksmith\/cache@v5/) + }) + + it('flags a third-party cache action inside a reusable workflow', () => { + const r = run(rfx('reusable-thirdparty-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch( + /job "publish" -> \.github\/workflows\/called-thirdparty-cache\.yml job "release" step "Restore the Cargo build"/, + ) + expect(r.output).toMatch(/Swatinem\/rust-cache@v2/) + }) + + // A local `uses:` is exempt from the allowlist because this gate opens it + // and reads every step — it is audited by construction, not trusted. Losing + // that exemption would report `./.github/actions/cachey` as unaudited *as + // well as* the `actions/cache@v4` inside it, burying the finding that + // matters under one about the wrapper. The count is the assertion; grepping + // the output for `AUDITED_ACTIONS` would only find the epilogue, which + // names it unconditionally. + it('does not report a local composite as unaudited', () => { + const r = run(cfx('composite-cache.yml')) + expect(r.output).toMatch(/Found 1 caching issue/) + expect(r.output).toMatch(/actions\/cache@/) + }) + }) + + // The un-auditable list printed and exited before the offender list was + // reached, so a run collecting both showed only the reference the gate could + // not open. The cache finding — the one with a step to delete — stayed + // hidden until the path was fixed, then arrived on the next run looking new. + // Both fail CI either way, so this is about what the failure tells you. + describe('a run that collects both kinds of finding', () => { + const cfx = (name) => + resolve( + fileURLToPath(import.meta.url), + `../fixtures/lint-no-workflow-caching/composites/.github/workflows/${name}`, + ) + + it('prints the cache offenders alongside the un-auditable references', () => { + const r = run(cfx('mixed-unresolved-and-cache.yml')) + // 2 beats 1 on a mixed run: something WAS found caching, but an + // incomplete scan is the more severe verdict — the list of what was + // found may be short. + expect(r.exitCode).toBe(2) + expect(r.output).toMatch(/Found 1 un-auditable reference/) + expect(r.output).toMatch(/no-such-action/) + expect(r.output).toMatch(/Found 1 caching issue/) + expect(r.output).toMatch(/actions\/cache@v4/) + }) + }) + it('the target workflows contain no `actions/cache` step', () => { for (const target of TARGET_WORKFLOWS) { const doc = yaml.load(readFileSync(resolve(REPO_ROOT, target), 'utf8')) diff --git a/scripts/lint-no-workflow-caching.mjs b/scripts/lint-no-workflow-caching.mjs index ffb89b2ff..d5a109b20 100644 --- a/scripts/lint-no-workflow-caching.mjs +++ b/scripts/lint-no-workflow-caching.mjs @@ -1,5 +1,5 @@ -import { readFileSync } from 'node:fs' -import { relative, resolve } from 'node:path' +import { existsSync, readFileSync, statSync } from 'node:fs' +import { dirname, join, relative, resolve, sep } from 'node:path' import yaml from 'js-yaml' const REPO_ROOT = resolve(import.meta.dirname, '..') @@ -21,8 +21,152 @@ const CACHE_ACTION = /^actions\/cache(\/(restore|save))?@/ const PNPM_ACTION_SETUP = /^pnpm\/action-setup(@|$)/ const SETUP_NODE = /^actions\/setup-node(@|$)/ +// A `uses:` naming a directory in this checkout rather than a published action. +// GitHub requires the `./` prefix for those, so anything without it is an +// `owner/repo@ref` or `docker://` reference with nothing here to open. +// +// `./` only, and the `{1,2}` this used to carry was not a harmless widening. It +// accepted `../`, which GitHub does not, and "local" means two things here: +// exempt from `AUDITED_ACTIONS`, and handed to a resolver that `resolve()`s the +// value against the workspace root. A `../` reference walks straight out of it, +// so a file OUTSIDE the checkout was opened and audited as though it were +// inside — confirmed against `uses: ../outside-workflow.yml`, which reported an +// `actions/cache@v4` it found one directory above the workspace root, `../` +// trail and all. +const LOCAL_USES = /^\.\// + +// The `../` shape LOCAL_USES no longer claims. It gets its own verdict rather +// than falling through to the remote branches, which would be fail-closed but +// misleading in both places: at step level "not in AUDITED_ACTIONS. This gate +// cannot read a published action's steps" sends the reader to audit a published +// action that does not exist, and at job level "a remote reusable workflow" +// describes something remote, which this is not. It is an invalid reference, +// and that is what the report should say. Reported either way — never resolved, +// so nothing outside the workspace root is opened. +const PARENT_USES = /^\.\.\// + +// ALLOWLIST RATIONALE — why this is a list of what is permitted, and not a +// longer list of cache actions. +// +// The rules above only see two things: an action literally named +// `actions/cache*`, and an action that takes a `cache:` input. A third-party +// cache action has neither, so it was invisible to the whole file. Reproduced +// against a composite reached from a targeted workflow holding +// `useblacksmith/cache@v5` and `Swatinem/rust-cache@v2`: `OK`, exit 0. Both are +// live-relevant here — eleven jobs in this repo run on `blacksmith-*` runners, +// where `useblacksmith/cache` is the documented drop-in for `actions/cache`, +// and an in-tree Cargo workspace is exactly where someone reaches for +// `Swatinem/rust-cache`. +// +// The obvious repair is to enumerate the cache actions — by name +// (`useblacksmith/cache`, `buildjet/cache`, `runs-on/cache`, `tespkg/actions- +// cache`, …) or by name shape (does the path contain "cache"?). Both were +// rejected, for the same reason: +// +// A denylist fails OPEN on the action nobody has met yet. It is silently +// correct until the day it is silently wrong, and it is wrong in the +// direction that prints `OK`. That is this repo's own stated criticism of +// its own checks — "a check that never ran reads exactly like a check that +// passed" — and it applies with full force to a list of vendor names that +// grows without this repo hearing about it. +// +// Worse, neither denylist form covers the class most likely to be added here by +// accident: a `setup-` action that caches BY DEFAULT, with no `cache:` +// input to inspect and no "cache" anywhere in its name. +// `gradle/actions/setup-gradle` is one; a runner vendor's drop-in `setup-node` +// is another. This file already carries two hand-maintained members of that +// class — `pnpm/action-setup` and `actions/setup-node` above — and they are +// here only because somebody happened to notice. Nothing would have caught the +// third. +// +// So the rule is inverted. Every REMOTE `uses:` reachable from a targeted +// workflow must appear below, or it is reported. This fails CLOSED: an action +// this gate has never seen is a finding by default, whatever it is called and +// whatever inputs it takes. +// +// This does not go stale silently, which is the whole point of choosing it. An +// allowlist's staleness is a build failure naming the exact action and the file +// it was added to — the person adding it is the person told to audit it, in the +// same PR. A denylist's staleness is an `OK`. +// +// The cost is bounded and was measured before choosing it, not assumed. The two +// targeted workflows are deliberately minimal and reach exactly four actions +// between them (all four are below); traversal is target-scoped, so nothing +// outside what release.yml and tests-supply-chain.yml reach is constrained; and +// they reach no local composite and no reusable workflow today. Adding an +// action to the npm-publishing workflow costs one line here plus a sentence +// saying why — which is the review that workflow warrants regardless. +// +// LOCAL `uses:` IS EXEMPT, and deliberately so: this gate opens a `./` action +// and reads every one of its steps, so it is audited by construction rather +// than trusted. Listing it here would report the composite and bury the +// `actions/cache` inside it — the finding that actually matters. +const AUDITED_ACTIONS = new Set([ + // First-party checkout. No cache of its own. + 'actions/checkout', + // Both cache on request only, and the explicit-`false` rules above assert + // that these two are told not to — they are on this list *and* separately + // constrained. + 'actions/setup-node', + 'pnpm/action-setup', + // release.yml's publish step. Runs `pnpm run release` and talks to npm over + // OIDC; no cache, no cache input. + 'changesets/action', +]) + +// A secondary, deliberately over-broad read of the action's name. It is NOT the +// defence — the allowlist above is, and it already rejects everything this +// matches. This exists for two narrower jobs: +// +// 1. Message quality. `useblacksmith/cache@v5` should read as "this restores a +// cache", not "this is not on a list" — the second invites adding it to the +// list. +// 2. Guarding the allowlist against itself. The one careless edit that could +// re-open this hole is someone appending a cache action to AUDITED_ACTIONS, +// so the assertion below makes that impossible: no entry may be +// cache-shaped, checked on every single invocation. +// +// Substring, not segment-equality, and that was verified rather than assumed: +// `Swatinem/rust-cache` has no path segment equal to `cache`, so +// `/(^|\/)cache(\/|$)/` misses it while a substring catches it — along with +// `tespkg/actions-cache` and any `*-cache` / `cache-*` naming a vendor picks. +// False positives (a hypothetical `foo/cache-warmer-status`) cost one line of +// review and cannot let anything through, because the allowlist has already +// done the fail-closed work. +const CACHE_SHAPED_ACTION = /cache/i + +for (const audited of AUDITED_ACTIONS) { + if (CACHE_SHAPED_ACTION.test(audited)) { + throw new Error( + `AUDITED_ACTIONS must not contain a cache action, found "${audited}". ` + + 'Allowlisting one would re-open the hole this list exists to close.', + ) + } +} + +// GitHub trims a `uses:` value before resolving it, and a quoted one can carry +// leading whitespace (`uses: " ./.github/actions/x"`). Untrimmed, that fails +// LOCAL_USES, so the composite is never opened — and before the allowlist above +// nothing was reported either, making it the one unfollowable local reference +// shape that exited 0 rather than 2. Normalising once here means every rule +// below reads the same string GitHub does. +function usesOf(step) { + if (typeof step?.uses !== 'string') return null + const trimmed = step.uses.trim() + return trimmed === '' ? null : trimmed +} + +// The `owner/repo[/subpath]` half of a `uses:`, without the `@ref`. Compared +// case-insensitively because GitHub repository names are: a `uses: Actions/ +// Checkout@v6` is the same action, and case-sensitivity would only ever make +// the allowlist reject something legitimate (fail-closed, so harmless) while +// letting `Actions/Cache@v4` past a case-sensitive denylist (fail-open, not). +function actionPath(uses) { + return uses.split('@')[0].toLowerCase() +} + function stepLabel(step, idx) { - return step?.name || step?.uses || `step #${idx + 1}` + return step?.name || usesOf(step) || `step #${idx + 1}` } // Returns a reason string if `inputName` is not explicitly set to boolean @@ -39,57 +183,418 @@ function explicitFalseReason(step, inputName) { } const offenders = [] +const unresolved = [] + +// Every `yaml.load` in this file goes through here. Unguarded — which all three +// call sites were — a file this gate cannot read throws straight out of the +// run: a stack trace instead of a finding, exit 1 (indistinguishable from +// "found a caching issue"), and every remaining target never scanned at all. +// +// A file that will not parse is the same problem as one that is not there. It +// hands the traversal no step list, so nothing below it is audited — which is +// exactly what `unresolved` is for, and why this belongs there rather than in +// `offenders`: exit 2, and the epilogue saying the list of findings may be +// short. The same reasoning `resolveWorkflowFile` records for a path that +// exists as a directory, one layer up. +// +// The catch is deliberately broad rather than YAMLException-only: EISDIR, +// EACCES and a vanished file all leave the caller with the same nothing, and +// all three are worth a report rather than a crash. +// +// Only the FIRST line of the error is kept. js-yaml puts the position there +// (`bad indentation of a mapping entry (3:5)`) and follows it with a multi-line +// source snippet — numbered lines and a caret ruler, carrying their own +// indentation — which would wreck the two-space bullets of the report below. +// The position is the half that lets someone fix the file. +// +// Callers get `null` and decide what it means for their own traversal. All of +// them stop descending; `walkSteps` additionally reads it as a NON-composite +// manifest, so `checkStep` keeps every rule it has. +function loadYaml(file, at) { + try { + return yaml.load(readFileSync(file, 'utf8')) + } catch (err) { + const [firstLine] = String(err?.message ?? err).split('\n') + unresolved.push(`${at} — could not be read: ${firstLine}`) + return null + } +} + +// Every rule that applies to a single step. Factored out of the job loop +// because the same rules have to hold for a step written inside a composite +// action: it runs in the same job, holding the same credentials, and caches by +// the same defaults. Exempting composites would make "move the step into a +// composite" a supported way out of the rule — which is the bug this file's +// traversal exists to close, one level up. +// +// `bodyAudited` says this step hands off to a local composite whose steps this +// gate reads for itself. It suppresses exactly one rule — see below. +function checkStep(step, at, bodyAudited = false) { + // `cache:` under a step's `with:` — covers actions/setup-node, + // actions/setup-python, etc. An explicit falsy value does not count. + // + // Skipped when the step's body is audited, and only then. `with:` on a step + // that hands off to a local composite is that composite's declared inputs, + // and a composite is free to declare one called `cache` that switches + // something else on entirely — the same false positive `walkJob` refuses to + // make by never running these rules over a job-level `with:`. The + // justification for skipping is "the body is read instead", so it may only + // apply where the body is actually read: a composite that forwards `cache` + // into a caching step is still reported, on the step doing the caching. + // + // A local `uses:` that resolves to nothing, or to a JS/Docker action, keeps + // the rule. There the gate opens no step list and reaches no verdict, and a + // local `uses:` is already exempt from AUDITED_ACTIONS — so this heuristic is + // the only thing standing. + if ( + !bodyAudited && + step?.with && + Object.hasOwn(step.with, 'cache') && + step.with.cache + ) { + offenders.push( + `${at}: \`with.cache: ${JSON.stringify(step.with.cache)}\` restores the GitHub Actions cache`, + ) + } + + const uses = usesOf(step) + if (uses === null) return + + // Explicit-disable assertions for the package-manager setup actions. Both are + // allowlisted, so these are the additional constraint on them, not a + // substitute for one. + if (PNPM_ACTION_SETUP.test(uses)) { + const reason = explicitFalseReason(step, 'cache') + if (reason) offenders.push(`${at}: pnpm/action-setup ${reason}`) + } + if (SETUP_NODE.test(uses)) { + const reason = explicitFalseReason(step, 'package-manager-cache') + if (reason) offenders.push(`${at}: actions/setup-node ${reason}`) + } + + // One verdict per `uses:`, most specific first — a step reported twice reads + // as two problems and gets fixed once. + if (CACHE_ACTION.test(uses)) { + // `uses: actions/cache...` + offenders.push(`${at}: uses \`${uses}\` (GitHub Actions cache)`) + } else if (LOCAL_USES.test(uses)) { + // Audited by construction: `walkSteps` opens it and checks every step. + } else if (PARENT_USES.test(uses)) { + // Ahead of the two remote branches because neither describes this: nothing + // published is being referenced, so neither "third-party cache action" nor + // "not in AUDITED_ACTIONS" would be true. The gate opens no step list here, + // which is why it is still a finding. + offenders.push( + `${at}: uses \`${uses}\` — a \`../\` reference. GitHub only resolves a ` + + 'local `uses:` that starts with `./`, and this gate will not follow one ' + + 'out of the workspace root, so these steps cannot be audited', + ) + } else if (CACHE_SHAPED_ACTION.test(actionPath(uses))) { + offenders.push( + `${at}: uses \`${uses}\` — a third-party cache action (GitHub Actions cache)`, + ) + } else if (!AUDITED_ACTIONS.has(actionPath(uses))) { + offenders.push( + `${at}: uses \`${uses}\` — not in AUDITED_ACTIONS. This gate cannot read ` + + 'a published action’s steps, so it cannot prove this one does not cache', + ) + } +} + +// GitHub resolves `uses: ./x` against the root of the CHECKOUT, not against the +// workflow file's own directory, so the traversal has to know where that root +// is. Any workflow GitHub will actually run sits in `/.github/workflows/`, +// which names the root exactly. The fallback covers a file handed to this +// script from somewhere else — a fixture, or an ad-hoc check — where the repo +// root is the only sensible reading of `./`. +function workspaceRootFor(workflowFile) { + const dir = dirname(workflowFile) + return dir.endsWith(`${sep}.github${sep}workflows`) + ? resolve(dir, '../..') + : REPO_ROOT +} + +// Both spellings are valid to GitHub, and a repo that mixes them is not doing +// anything wrong — so accepting only `action.yml` would silently stop +// traversing half the composites it was pointed at. +// +// `isFile` is what keeps this symmetrical with `resolveWorkflowFile` below, +// which has guarded it since it was written. On a bare `existsSync` a DIRECTORY +// named `action.yml` passed as the manifest and aborted the run with an +// unhandled EISDIR at `readFileSync`. Guarded, the directory is skipped, the +// next candidate name is tried, and a directory under both names returns null — +// which lands on the "no action.yml or action.yaml there" report the caller +// already has. `loadYaml` would catch the EISDIR too, but this is the precise +// fix and it is the one that names the actual problem. +function resolveActionFile(workspaceRoot, usesPath) { + const dir = resolve(workspaceRoot, usesPath) + for (const name of ['action.yml', 'action.yaml']) { + const file = join(dir, name) + if (existsSync(file) && statSync(file).isFile()) return file + } + return null +} + +// Walks a step list, following any step that hands off to a local composite +// action. +// +// WHY: a composite is one `uses:` of indirection and the checks above used to +// stop dead at it. `uses: ./.github/actions/build-ffi-binding` in release.yml +// restored two GitHub Actions caches into the credential-bearing publishing job +// while this script printed `OK` — confirmed against a copy of release.yml with +// that step spliced in, exit 0, no output. Neither target workflow uses a local +// composite today; this is the gate that keeps that true. +// +// `visited` is scoped per job, not per run: a job is the unit of credential +// exposure this rule protects, so one report per job is enough, while a +// composite shared between two jobs still gets named in each. It also breaks +// the `A uses B uses A` cycle, which otherwise recurses until the stack blows. +// +// `if:` is deliberately not evaluated. A conditional cache restore is still a +// cache restore, and its condition is only known at run time anyway. +function walkSteps(steps, prefix, workspaceRoot, visited) { + steps.forEach((step, idx) => { + const at = `${prefix} step "${stepLabel(step, idx)}"` + + // The action is resolved BEFORE the step is checked, not after, because + // `checkStep` needs to know whether this step's body is about to be + // audited. Resolving it twice — once for that answer, once to recurse — + // would let the two readings drift apart, which is the one way the + // suppression could outlive the audit that justifies it. + const uses = usesOf(step) + if (uses === null || !LOCAL_USES.test(uses)) { + checkStep(step, at) + return + } + + const file = resolveActionFile(workspaceRoot, uses) + if (file === null) { + // No manifest to open: nothing here is audited, so every step rule + // applies, and the offender it finds prints alongside the report below. + checkStep(step, at) + unresolved.push( + `${at}: \`uses: ${uses}\` — no action.yml or action.yaml there`, + ) + return + } + + // An action manifest puts its steps under `runs:`, not `jobs:` — and only + // when `runs.using` is `composite`. A JavaScript or Docker action has a + // `runs.main`/`runs.image` and no step list, which lands on the `[]` below. + // + // Read before the `visited` check rather than after: a second reference to + // an already-walked composite still needs the same verdict on its own + // `with:`, and its body has been audited by the first reference. Re-parsing + // a handful of small manifests is cheaper than the alternatives, and + // `visited` still guards the recursion, so nothing is reported twice. + // + // A manifest that will not parse yields null, which is a non-composite by + // every reading below: `runs.using` is not `composite`, so `checkStep` + // keeps all its rules — the same verdict the unresolvable branch above + // reaches, for the same reason — and `runs.steps` is undefined, so the + // traversal stops here rather than claiming to have audited a body it + // never read. + const doc = loadYaml(file, `${at} -> ${relative(workspaceRoot, file)}`) + + // Keyed on `runs.using`, not on "did we find a step list", so that a + // manifest declaring a JS runtime yet carrying steps — invalid to GitHub, + // but this gate runs on files GitHub has not validated — is audited AND + // still judged on its caller's `with:`. Both readings, fail-closed. + checkStep(step, at, doc?.runs?.using === 'composite') + + if (visited.has(file)) return + visited.add(file) + + const nested = Array.isArray(doc?.runs?.steps) ? doc.runs.steps : [] + + // The trail is the whole chain, not just its ends. A message naming only + // the workflow step sends the reader to a file with no `actions/cache` + // anywhere in it. + walkSteps( + nested, + `${at} -> ${relative(workspaceRoot, file)}`, + workspaceRoot, + visited, + ) + }) +} + +// Unlike an action, a reusable workflow is named by its file, extension and +// all (`./.github/workflows/x.yml`), so there is no `.yml`/`.yaml` probing to +// do here — the path is exact. `isFile` is the guard that matters instead: a +// path that exists as a directory would otherwise reach `readFileSync` and +// abort the run with an unhandled EISDIR, which is a worse outcome than the +// report below. +function resolveWorkflowFile(workspaceRoot, usesPath) { + const file = resolve(workspaceRoot, usesPath) + return existsSync(file) && statSync(file).isFile() ? file : null +} + +// Follows `jobs..uses:` — a job that delegates its whole body to another +// workflow. +// +// WHY: such a job has no `steps:` at all, so the loop below used to hand +// `walkSteps` an empty list and skip the job entire. Confirmed before this +// existed against a caller whose only job was +// `uses: ./.github/workflows/reusable.yml` with `secrets: inherit`, the called +// workflow holding an `actions/cache@v4` step: exit 0, `OK`, nothing scanned. +// Same failure the composite traversal closed, one shape over. +// +// The verdict deliberately ignores `secrets:`. It is not the only credential +// channel — `permissions:` is inherited independently, and that is what mints +// the OIDC token npm trusted publishing signs with, so a call passing no +// secrets can still publish. Nor does a restore need credentials in its own job +// to be the attack: poisoned bytes landing in a build job that hands an +// artifact to a publish job is the canonical shape. Conditioning on `secrets:` +// would prevent no failure and would hand an attacker a phrasing that evades +// the gate. +function followReusableWorkflow(uses, prefix, workspaceRoot, visited) { + // Checked before the remote branch below, which would otherwise call this "a + // remote reusable workflow" — it is not remote, it is unresolvable. Same + // list either way: no jobs are read, so the scan is incomplete here. + if (PARENT_USES.test(uses)) { + unresolved.push( + `${prefix}: \`uses: ${uses}\` — a \`../\` reference; GitHub only resolves a local \`uses:\` that starts with \`./\`, and this gate will not follow one out of the workspace root`, + ) + return + } + + // A remote reusable workflow is reported, where `walkSteps` skips a remote + // *step* action, and the difference is coverage rather than depth. A + // marketplace step sits inside a job whose step list this gate has read end + // to end; flagging every `actions/checkout@v6` would make it exit 2 forever + // and mean nothing. A remote job-level `uses:` is the whole job — no steps + // read, no verdict reached, `OK` printed anyway, which is precisely the + // "a check that never ran reads like a check that passed" failure this file + // exists to stop. It is rare (zero in this repo today) and actionable — + // inline the job, or point it at a workflow in this checkout — so the report + // is signal rather than noise. + if (!LOCAL_USES.test(uses)) { + unresolved.push( + `${prefix}: \`uses: ${uses}\` — a remote reusable workflow; its jobs are not in this checkout`, + ) + return + } + + const file = resolveWorkflowFile(workspaceRoot, uses) + if (file === null) { + unresolved.push(`${prefix}: \`uses: ${uses}\` — no workflow file there`) + return + } + if (visited.has(file)) return + visited.add(file) + + // One `visited` spans both node types, so `a.yml -> b.yml -> a.yml` + // terminates the same way `a -> b -> a` does between composites. + // + // A called workflow that will not parse yields null, so it contributes no + // jobs and the hop stops here — reported, not silently traversed past. + const doc = loadYaml(file, `${prefix} -> ${relative(workspaceRoot, file)}`) + for (const [jobName, job] of Object.entries(doc?.jobs ?? {})) { + walkJob( + job, + `${prefix} -> ${relative(workspaceRoot, file)} job "${jobName}"`, + workspaceRoot, + visited, + ) + } +} + +// A job is one of two shapes, and the recursion has to know both: `steps:` (a +// normal job, and the shape a composite's `runs.steps` also has) or `uses:` (a +// call into another workflow, whose own jobs are either shape again). +// +// Both are checked rather than one being chosen. Carrying both keys is invalid +// to GitHub's schema, which rejects the file outright — but this gate runs on +// files GitHub has not validated yet, and treating either key as authoritative +// would let such a file hide a cache behind the key the gate ignored and still +// report a pass. +// +// The job object itself is never passed to `checkStep`: at job level `with:` is +// inputs to the called workflow, not `with:` on an action step, so the step +// rules would flag a caller for passing `cache: true` to an input that +// switches something else on entirely. `walkSteps` makes the same distinction +// one level down, for a step whose `uses:` is a local composite — see +// `bodyAudited`. +function walkJob(job, prefix, workspaceRoot, visited) { + walkSteps( + Array.isArray(job?.steps) ? job.steps : [], + prefix, + workspaceRoot, + visited, + ) + + const uses = usesOf(job) + if (uses !== null) { + followReusableWorkflow(uses, prefix, workspaceRoot, visited) + } +} + for (const target of TARGETS) { const abs = resolve(REPO_ROOT, target) const rel = relative(REPO_ROOT, abs) - const doc = yaml.load(readFileSync(abs, 'utf8')) + const workspaceRoot = workspaceRootFor(abs) + // A target that will not parse yields null, so it contributes no jobs and + // this iteration finds nothing — and, the point of the loop continuing, the + // remaining targets are still scanned. A malformed release.yml used to mean + // tests-supply-chain.yml was never looked at either. + const doc = loadYaml(abs, rel) const jobs = doc?.jobs ?? {} for (const [jobName, job] of Object.entries(jobs)) { - const steps = Array.isArray(job?.steps) ? job.steps : [] - steps.forEach((step, idx) => { - const label = stepLabel(step, idx) - const at = `${rel}: job "${jobName}" step "${label}"` - - // `cache:` under a step's `with:` — covers actions/setup-node, - // actions/setup-python, etc. An explicit falsy value does not count. - if (step?.with && Object.hasOwn(step.with, 'cache') && step.with.cache) { - offenders.push( - `${at}: \`with.cache: ${JSON.stringify(step.with.cache)}\` restores the GitHub Actions cache`, - ) - } - - // `uses: actions/cache...` - if (typeof step?.uses === 'string' && CACHE_ACTION.test(step.uses)) { - offenders.push(`${at}: uses \`${step.uses}\` (GitHub Actions cache)`) - } - - // Explicit-disable assertions for the package-manager setup actions. - if (typeof step?.uses === 'string') { - if (PNPM_ACTION_SETUP.test(step.uses)) { - const reason = explicitFalseReason(step, 'cache') - if (reason) offenders.push(`${at}: pnpm/action-setup ${reason}`) - } - if (SETUP_NODE.test(step.uses)) { - const reason = explicitFalseReason(step, 'package-manager-cache') - if (reason) offenders.push(`${at}: actions/setup-node ${reason}`) - } - } - }) + walkJob(job, `${rel}: job "${jobName}"`, workspaceRoot, new Set()) } } +if (unresolved.length > 0) { + console.error(`Found ${unresolved.length} un-auditable reference(s):\n`) + for (const u of unresolved) console.error(` ${u}`) + console.error( + '\nEach of these hands this gate a step list it cannot open, so it cannot\n' + + 'prove those steps are cache-free. A local `uses:` pointing at nothing\n' + + 'fails the job on GitHub anyway; a remote reusable workflow runs fine and\n' + + 'audits nothing at all. Passing either silently would turn it into a\n' + + 'permanent exemption — fix the path, inline the job, or point it at a\n' + + 'workflow in this checkout.', + ) +} + if (offenders.length > 0) { + // Separated from the block above only when there is a block above, so the + // two epilogues do not run together on a mixed run and the common + // single-finding output does not open on a blank line. + if (unresolved.length > 0) console.error('') console.error(`Found ${offenders.length} caching issue(s) in workflow(s):\n`) for (const o of offenders) console.error(` ${o}`) console.error( '\nThese workflows must not restore the GitHub Actions cache — it is a\n' + 'cache-poisoning / supply-chain vector for credential-bearing jobs.\n' + 'Caching must be disabled explicitly (`cache: false`,\n' + - '`package-manager-cache: false`). See the "CI/CD Supply-Chain\n' + - 'Hardening" section of SECURITY.md.', + '`package-manager-cache: false`), including inside any local composite\n' + + 'action or reusable workflow they reach.\n' + + '\nA published `uses:` that is not in AUDITED_ACTIONS is reported for the\n' + + 'same reason rather than a different one: this gate cannot open a\n' + + 'published action, so it cannot prove that action does not cache — and\n' + + 'the ones that do are not all called "cache" (a `setup-` action\n' + + 'that caches by default has no `cache:` input and no telling name).\n' + + 'The list is what is permitted, not what is forbidden, so an action it\n' + + 'has never met fails by default. Review the action and add it there with\n' + + 'the reason, or drop the step.\n' + + '\nSee the "CI/CD Supply-Chain Hardening" section of SECURITY.md.', ) - process.exit(1) } +// Both lists print before either exit, because a run can collect both and each +// is acted on separately. Exiting inside the first block hid the cache finding +// — the one with a step to delete — until the broken path was fixed, at which +// point it arrived on the next run looking new. +// +// Exit 2 outranks 1 on a mixed run, and not because nothing was found caching: +// on a mixed run something was. An incomplete scan is simply the more severe +// verdict, since the exit 1 reports what this gate could see and the exit 2 +// says that list may be short. Same contract lint-no-hardcoded-runners.mjs uses +// for a missing scan target. +if (unresolved.length > 0) process.exit(2) +if (offenders.length > 0) process.exit(1) + console.log('OK — GitHub Actions caching is explicitly disabled in:\n') for (const target of TARGETS) console.log(target) diff --git a/skills/stash-supply-chain-security/SKILL.md b/skills/stash-supply-chain-security/SKILL.md index ec8491320..4ff1862c1 100644 --- a/skills/stash-supply-chain-security/SKILL.md +++ b/skills/stash-supply-chain-security/SKILL.md @@ -127,7 +127,8 @@ Constraints baked into that workflow — don't undo them: - **`runs-on: ubuntu-latest`, not a self-hosted/Blacksmith runner.** npm rejects provenance from non-GitHub-hosted runners with E422. - **Never set `NPM_TOKEN`.** `changesets/action` writes a token `.npmrc` when it sees one, which shadows OIDC and fails every publish with E404 (npm/cli#8976). - **npm ≥ 11.5.1 and Node ≥ 22.14.** Node 22 ships npm 10.x, so the workflow installs `npm@^11.5.1` explicitly before publishing. -- **No Actions cache in this workflow** (no `cache:`, `package-manager-cache: false`, `pnpm/action-setup` with `cache: false`). A poisoned cache entry would execute in a credential-bearing job. Enforced by `scripts/lint-no-workflow-caching.mjs`. +- **No Actions cache in this workflow** (no `cache:`, `package-manager-cache: false`, `pnpm/action-setup` with `cache: false`). A poisoned cache entry would execute in a credential-bearing job. Enforced by `scripts/lint-no-workflow-caching.mjs`, which also follows any local composite action or reusable workflow the job reaches — the rule is about the whole call tree, not the one file. +- **Every published `uses:` must be in that script's `AUDITED_ACTIONS` allowlist.** The gate cannot open a published action to check whether it caches, and the ones that do are not all named "cache" — a `setup-` action that caches by default has no `cache:` input and no telling name. So the list is what is *permitted*, and an action it has never met fails by default. Adding a step to `release.yml` or `tests-supply-chain.yml` means auditing the action and adding it there with the reason, in the same PR. Trusted publishing is configured **per package** on npmjs.com (package settings → Trusted publisher → GitHub Actions): owner/repo `cipherstash/stack`, workflow