From 05f84bcc7ad51b50ceb37725d82cfd394aa851b7 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 10:46:35 +1000 Subject: [PATCH 1/3] fix(ci): audit the whole call tree in the no-caching gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/lint-no-workflow-caching.mjs` keeps the GitHub Actions cache out of the two credential-bearing workflows, because a poisoned cache entry would execute in a job that can publish to npm. It had three fail-open holes, each of which printed `OK`. **1. It stopped at a local composite action.** The gate read a step's own `uses:` and went no further, so a workflow could reach `actions/cache` through one indirection — `uses: ./.github/actions/x` — and stay green. Reproduced against a copy of release.yml with a cache-restoring composite spliced in: exit 0, no output, while the composite it never opened restored two caches. It now flattens local composites and checks every step inside them, naming the whole trail so the report points at the file the cache is actually in. **2. It skipped a job that delegates to a reusable workflow.** Such a job has no `steps:` at all — it is `jobs..uses` — so the walker was handed an empty list and skipped the job entire. Confirmed before the fix against a caller whose only job was `uses: ./.github/workflows/reusable.yml` with `secrets: inherit`, the called workflow holding `actions/cache@v4`: exit 0, `OK`, nothing scanned. The verdict deliberately ignores `secrets:` — `permissions:` is inherited independently and is what mints the OIDC token npm trusted publishing signs with, so a call passing no secrets can still publish. **3. A third-party cache action was invisible.** The rules only recognised an action literally named `actions/cache*` and one taking a `cache:` input, so `useblacksmith/cache@v5` and `Swatinem/rust-cache@v2` both passed. The repair is an inversion rather than a longer denylist: every REMOTE `uses:` reachable from a targeted workflow must now appear in an `AUDITED_ACTIONS` allowlist, so an action this gate has never seen is a finding by default whatever it is called. A denylist fails open on the action nobody has met yet — silently correct until the day it is silently wrong, and wrong in the direction that prints `OK`. It also cannot cover the class most likely to be added by accident: a `setup-` action that caches BY DEFAULT, with no `cache:` input to inspect and no "cache" in its name. The allowlist cannot go stale silently, which is why it was chosen: its staleness is a build failure naming the exact action and the file it was added to, so the person adding it is the person told to audit it, in the same PR. Cost was measured rather than assumed — the two targeted workflows reach four actions between them. Also adds the missing premise assertion to `integration-workflow-paths`: its requirement set is DERIVED from `@/`-aliased imports, so "no suite uses that alias" and "every import is covered" were the same green. Mutation-tested — rewriting the suites onto the public entry empties the set, and the check then passed with `packages/stack/src/dynamodb/**` deleted from the filter, which is verbatim the #815 gap the file exists to prevent. Second of four stacked PRs splitting the protect-ffi absorption. Independent of the vendoring: these are pre-existing holes in a control that already shipped. --- .changeset/olive-moons-shave.md | 18 + .github/actions/integration-setup/action.yml | 2 +- .../audited-actions.yml | 27 ++ .../lint-no-workflow-caching/cache-family.yml | 17 + .../.github/actions/cachey-restore/action.yml | 10 + .../.github/actions/cachey-save/action.yml | 10 + .../.github/actions/cachey/action.yml | 13 + .../actions/clean-composite/action.yml | 16 + .../.github/actions/loop-a/action.yml | 7 + .../.github/actions/loop-b/action.yml | 12 + .../actions/missing-explicit-false/action.yml | 12 + .../.github/actions/outer/action.yml | 7 + .../actions/setup-node-cache/action.yml | 10 + .../actions/thirdparty-cache/action.yml | 12 + .../.github/actions/yaml-ext/action.yaml | 10 + .../workflows/composite-cache-restore.yml | 12 + .../workflows/composite-cache-save.yml | 12 + .../.github/workflows/composite-cache.yml | 12 + .../.github/workflows/composite-clean.yml | 12 + .../.github/workflows/composite-cyclic.yml | 12 + .../workflows/composite-leading-space.yml | 16 + .../composite-missing-explicit-false.yml | 12 + .../.github/workflows/composite-nested.yml | 12 + .../workflows/composite-setup-node-cache.yml | 12 + .../workflows/composite-thirdparty-cache.yml | 11 + .../workflows/composite-unresolvable.yml | 12 + .../.github/workflows/composite-yaml-ext.yml | 12 + .../.github/workflows/third-party-uses.yml | 16 + .../.github/actions/cachey/action.yml | 10 + .../.github/workflows/called-cache.yml | 14 + .../.github/workflows/called-clean.yml | 18 + .../.github/workflows/called-composite.yml | 14 + .../workflows/called-input-named-cache.yml | 21 + .../.github/workflows/called-loop-a.yml | 7 + .../.github/workflows/called-loop-b.yml | 15 + .../called-missing-explicit-false.yml | 14 + .../.github/workflows/called-outer.yml | 7 + .../workflows/called-thirdparty-cache.yml | 11 + .../.github/workflows/reusable-both.yml | 19 + .../.github/workflows/reusable-cache.yml | 11 + .../.github/workflows/reusable-clean.yml | 8 + .../.github/workflows/reusable-composite.yml | 8 + .../.github/workflows/reusable-cyclic.yml | 8 + .../workflows/reusable-explicit-secrets.yml | 12 + .../workflows/reusable-input-named-cache.yml | 10 + .../reusable-missing-explicit-false.yml | 8 + .../.github/workflows/reusable-nested.yml | 8 + .../.github/workflows/reusable-no-secrets.yml | 10 + .../.github/workflows/reusable-remote.yml | 13 + .../workflows/reusable-thirdparty-cache.yml | 8 + .../workflows/reusable-unresolvable.yml | 8 + .../thirdparty-cache.yml | 27 ++ .../unaudited-setup.yml | 19 + .../integration-workflow-paths.test.mjs | 14 + .../lint-no-workflow-caching.test.mjs | 378 ++++++++++++++++ scripts/lint-no-workflow-caching.mjs | 417 ++++++++++++++++-- skills/stash-supply-chain-security/SKILL.md | 3 +- 57 files changed, 1429 insertions(+), 37 deletions(-) create mode 100644 .changeset/olive-moons-shave.md create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/audited-actions.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/cache-family.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-restore/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey-save/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cachey/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/clean-composite/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-a/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/loop-b/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/missing-explicit-false/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/outer/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/setup-node-cache/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/thirdparty-cache/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/yaml-ext/action.yaml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-restore.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-save.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-clean.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cyclic.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-leading-space.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-missing-explicit-false.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-nested.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-setup-node-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-thirdparty-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-yaml-ext.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/third-party-uses.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/actions/cachey/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-clean.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-composite.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-input-named-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-a.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-loop-b.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-missing-explicit-false.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-outer.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-thirdparty-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-both.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-clean.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-composite.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-cyclic.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-explicit-secrets.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-input-named-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-missing-explicit-false.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-nested.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-no-secrets.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-remote.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-thirdparty-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-unresolvable.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/thirdparty-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/unaudited-setup.yml 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/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/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/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-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-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-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.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/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/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-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-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-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..e9f5c89a8 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -92,6 +92,384 @@ 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, so an unguarded cycle hangs the suite + // rather than failing it. The offender count is the real assertion: a + // visited set that is per-branch rather than per-run terminates 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 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"/) + }) + }) + + // 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@/) + }) + }) + 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..672e0c2a4 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,133 @@ 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. +const LOCAL_USES = /^\.{1,2}\// + +// 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,45 +164,258 @@ function explicitFalseReason(step, inputName) { } const offenders = [] +const unresolved = [] + +// 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. +function checkStep(step, at) { + // `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`, + ) + } + + 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 (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. +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)) 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)}"` + checkStep(step, at) + + const uses = usesOf(step) + if (uses === null || !LOCAL_USES.test(uses)) return + + const file = resolveActionFile(workspaceRoot, uses) + if (file === null) { + unresolved.push( + `${at}: \`uses: ${uses}\` — no action.yml or action.yaml there`, + ) + return + } + if (visited.has(file)) return + visited.add(file) + + // 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. + const doc = yaml.load(readFileSync(file, 'utf8')) + 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) { + // 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. + const doc = yaml.load(readFileSync(file, 'utf8')) + 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. +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 workspaceRoot = workspaceRootFor(abs) const doc = yaml.load(readFileSync(abs, 'utf8')) 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.', + ) + // Exit 2, not 1: nothing was found caching — the linter could not look. Same + // contract as lint-no-hardcoded-runners.mjs uses for a missing scan target. + process.exit(2) +} + if (offenders.length > 0) { console.error(`Found ${offenders.length} caching issue(s) in workflow(s):\n`) for (const o of offenders) console.error(` ${o}`) @@ -85,8 +423,17 @@ if (offenders.length > 0) { '\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) } 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 From c716e0372f6997c224df6b610da37b22f0757dd0 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 12:11:19 +1000 Subject: [PATCH 2/3] fix(ci): stop the no-caching gate reading composite inputs as step inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the call-tree traversal. **`with.cache` fired on a local composite's declared inputs.** `checkStep` applied the `with.cache: ` rule to every step, including one whose `uses:` is a local action — where `with:` is that action's arbitrary declared inputs. A composite taking a `cache` input that decides whether to reuse a binary already in the working tree, invoked `with: {cache: true}`, was reported as "restores the GitHub Actions cache". It is the step-level twin of the false positive `walkJob` already refuses to make by never running the step rules over a job-level `with:`, which `reusable-input-named-cache.yml` pins. The exemption is keyed on the resolved manifest's `runs.using` being `composite`, not on the `uses:` starting with `./`, because its justification is "the body is audited instead" rather than "local is trusted". A local `uses:` resolving to nothing, or to a JS/Docker action with no step list, keeps the rule: there the gate opens no step list, and a local `uses:` is already exempt from AUDITED_ACTIONS, so the caller's `with:` is the only signal left — dropping it for every local reference would make a two-line `action.yml` a supported way past the gate. `walkSteps` now resolves the action before checking the step rather than after, so one reading feeds both decisions and the suppression cannot outlive the audit that justifies it. `cache-passthrough` pins the fail-closed half: a composite forwarding its `cache` input into `actions/setup-node` is still one finding, named on the step inside the composite rather than on the caller that switched it on. **Exit 2 suppressed the cache offenders on a mixed run.** The un-auditable list printed and called `process.exit(2)` before the offender block was reached, so a run collecting both showed only the reference the gate could not open. 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. Both lists now print before either exit. Exit 2 still outranks 1, but no longer 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. Scripts suite 216 passing (49 in the touched file, +5); all three lint gates OK; biome 0 errors. --- .../actions/cache-passthrough/action.yml | 26 ++++++ .../actions/input-named-cache/action.yml | 19 ++++ .../.github/actions/js-action/action.yml | 21 +++++ .../workflows/composite-cache-passthrough.yml | 19 ++++ .../workflows/composite-input-named-cache.yml | 23 +++++ .../composite-unresolvable-with-cache.yml | 19 ++++ .../.github/workflows/local-js-action.yml | 18 ++++ .../workflows/mixed-unresolved-and-cache.yml | 22 +++++ .../lint-no-workflow-caching.test.mjs | 88 +++++++++++++++++++ scripts/lint-no-workflow-caching.mjs | 83 ++++++++++++++--- 10 files changed, 327 insertions(+), 11 deletions(-) create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/cache-passthrough/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/input-named-cache/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/js-action/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-cache-passthrough.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-input-named-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-unresolvable-with-cache.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/local-js-action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/mixed-unresolved-and-cache.yml 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/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/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-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-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/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__/lint-no-workflow-caching.test.mjs b/scripts/__tests__/lint-no-workflow-caching.test.mjs index e9f5c89a8..6fe37f7a8 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -223,6 +223,69 @@ describe('lint-no-workflow-caching', () => { }) }) + // 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 @@ -470,6 +533,31 @@ describe('lint-no-workflow-caching', () => { }) }) + // 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 672e0c2a4..12e9ed346 100644 --- a/scripts/lint-no-workflow-caching.mjs +++ b/scripts/lint-no-workflow-caching.mjs @@ -172,10 +172,32 @@ const unresolved = [] // 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. -function checkStep(step, at) { +// +// `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. - if (step?.with && Object.hasOwn(step.with, 'cache') && step.with.cache) { + // + // 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`, ) @@ -260,25 +282,49 @@ function resolveActionFile(workspaceRoot, usesPath) { function walkSteps(steps, prefix, workspaceRoot, visited) { steps.forEach((step, idx) => { const at = `${prefix} step "${stepLabel(step, idx)}"` - checkStep(step, at) + // 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)) return + 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 } - if (visited.has(file)) return - visited.add(file) // 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. const doc = yaml.load(readFileSync(file, 'utf8')) + + // 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 @@ -374,7 +420,9 @@ function followReusableWorkflow(uses, prefix, workspaceRoot, visited) { // 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. +// 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 : [], @@ -411,12 +459,13 @@ if (unresolved.length > 0) { 'permanent exemption — fix the path, inline the job, or point it at a\n' + 'workflow in this checkout.', ) - // Exit 2, not 1: nothing was found caching — the linter could not look. Same - // contract as lint-no-hardcoded-runners.mjs uses for a missing scan target. - process.exit(2) } 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( @@ -435,8 +484,20 @@ if (offenders.length > 0) { '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) From 7f819fb140ce5454c8e43c4d261b8e5ee97eb2f9 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 12:42:28 +1000 Subject: [PATCH 3/3] fix(ci): report the files the no-caching gate cannot open instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all in the same class: a file this gate could not read took the run down with it rather than being reported. **Unguarded `yaml.load`.** All three read sites — a composite manifest, a called workflow, and the target workflow itself — threw straight out of the run on anything unparseable: a stack trace instead of a finding, exit 1 which is indistinguishable from "found a caching issue", and every remaining target never scanned. A malformed `release.yml` meant `tests-supply-chain.yml` was not looked at either. One shared `loadYaml` helper now reports and returns null, and a file that will not parse lands in the un-auditable list for the reason that list exists: it hands the traversal no step list, so nothing below it is audited. Only the first line of the error is kept — js-yaml puts the position there and follows it with a source snippet whose own indentation would wreck the report's bullets. Each caller decides what null means: `walkSteps` reads it as a NON-composite manifest, so `checkStep` keeps every rule including `with.cache`, and the target loop continues to the remaining targets. **`LOCAL_USES` accepted `../`, which GitHub does not.** The comment above it already said "GitHub requires the `./` prefix"; the regex said `{1,2}`. That was not a harmless widening, because "local" means two things here — exempt from AUDITED_ACTIONS, and handed to a resolver that `resolve()`s the value against the workspace root. Confirmed by dropping a workflow holding `actions/cache@v4` one directory above a fixture root: `uses: ../outside-workflow.yml` opened it, audited it, and printed the finding with a `../` trail. A file outside the checkout, read as though it were inside. `../` now gets its own verdict rather than falling through to the remote branches, which would be fail-closed but would tell the reader to audit a published action that does not exist. **`resolveActionFile` lacked the `isFile` guard its sibling has.** A DIRECTORY named `action.yml` passed a bare `existsSync` and reached `readFileSync` as the manifest — unhandled EISDIR. `resolveWorkflowFile` has guarded exactly this since it was written, with a comment explaining why; the asymmetry was the bug. Guarded, the directory is skipped and the existing "no action.yml or action.yaml there" report is what comes out. One further suggestion was checked and not taken: adding a `timeout` to the test helper's `execFileSync`, on the premise that a regression in cycle protection would hang the suite. Deleting the `visited` guard and running the cyclic fixture blows the stack and exits in well under a second — the script is synchronous end to end and has no way to hang. The comment claiming otherwise was the source of the suggestion and is corrected instead. Scripts suite 224 passing (+8); all three lint gates OK; biome 0 errors. --- .../dir-action-yml/action.yml/.gitkeep | 3 + .../.github/actions/malformed/action.yml | 19 +++ .../workflows/composite-dir-action-yml.yml | 22 +++ .../.github/workflows/composite-malformed.yml | 23 +++ .../lint-no-workflow-caching/malformed.yml | 23 +++ .../parent-uses-step.yml | 22 +++ .../.github/workflows/called-malformed.yml | 17 ++ .../.github/workflows/reusable-malformed.yml | 12 ++ .../workflows/reusable-parent-uses.yml | 17 ++ .../lint-no-workflow-caching.test.mjs | 151 +++++++++++++++++- scripts/lint-no-workflow-caching.mjs | 107 ++++++++++++- 11 files changed, 407 insertions(+), 9 deletions(-) create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/dir-action-yml/action.yml/.gitkeep create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/actions/malformed/action.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-dir-action-yml.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/composites/.github/workflows/composite-malformed.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/malformed.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/parent-uses-step.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/called-malformed.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-malformed.yml create mode 100644 scripts/__tests__/fixtures/lint-no-workflow-caching/reusable/.github/workflows/reusable-parent-uses.yml 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/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/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-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/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/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/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-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__/lint-no-workflow-caching.test.mjs b/scripts/__tests__/lint-no-workflow-caching.test.mjs index 6fe37f7a8..193bf3190 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -165,10 +165,14 @@ describe('lint-no-workflow-caching', () => { ) }) - // `execFileSync` has no timeout here, so an unguarded cycle hangs the suite - // rather than failing it. The offender count is the real assertion: a - // visited set that is per-branch rather than per-run terminates but reports - // `loop-b` once per path into it. + // `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) @@ -221,6 +225,20 @@ describe('lint-no-workflow-caching', () => { 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` @@ -423,6 +441,131 @@ describe('lint-no-workflow-caching', () => { }) }) + // 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 diff --git a/scripts/lint-no-workflow-caching.mjs b/scripts/lint-no-workflow-caching.mjs index 12e9ed346..d5a109b20 100644 --- a/scripts/lint-no-workflow-caching.mjs +++ b/scripts/lint-no-workflow-caching.mjs @@ -24,7 +24,26 @@ 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. -const LOCAL_USES = /^\.{1,2}\// +// +// `./` 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. @@ -166,6 +185,41 @@ 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 @@ -225,6 +279,16 @@ function checkStep(step, at, bodyAudited = false) { 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)`, @@ -253,11 +317,20 @@ function workspaceRootFor(workflowFile) { // 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)) return file + if (existsSync(file) && statSync(file).isFile()) return file } return null } @@ -314,7 +387,14 @@ function walkSteps(steps, prefix, workspaceRoot, visited) { // `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. - const doc = yaml.load(readFileSync(file, 'utf8')) + // + // 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, @@ -369,6 +449,16 @@ function resolveWorkflowFile(workspaceRoot, usesPath) { // 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 @@ -396,7 +486,10 @@ function followReusableWorkflow(uses, prefix, workspaceRoot, visited) { // One `visited` spans both node types, so `a.yml -> b.yml -> a.yml` // terminates the same way `a -> b -> a` does between composites. - const doc = yaml.load(readFileSync(file, 'utf8')) + // + // 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, @@ -441,7 +534,11 @@ for (const target of TARGETS) { const abs = resolve(REPO_ROOT, target) const rel = relative(REPO_ROOT, abs) const workspaceRoot = workspaceRootFor(abs) - const doc = yaml.load(readFileSync(abs, 'utf8')) + // 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)) { walkJob(job, `${rel}: job "${jobName}"`, workspaceRoot, new Set())