Two round-1 review findings, fixed together because they touch the
same regex/loop region of `findViolations`:
1. **cubic P0 — quoted write-all bypass**.
`WRITE_ALL_PATTERN` was `/^\s*permissions:\s*write-all\b/m`, which
does not match the perfectly valid YAML forms
`permissions: "write-all"` and `permissions: 'write-all'`. A
workflow that quoted the shorthand slipped right through the
persist-credentials gate the previous commit was supposed to close.
Reproduced before this commit:
$ cat /tmp/q.yml
name: bad
on: [push]
permissions: "write-all"
jobs:
do:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
$ ECC_WORKFLOWS_DIR=/tmp node scripts/ci/validate-workflow-security.js
Validated workflow security for 1 workflow files
exit=0
Fix: tighten the regex to
/^\s*permissions:\s*["']?write-all["']?\s*$/m
which accepts the bare, double-quoted, and single-quoted YAML forms
while still anchoring on the `permissions:` key. The trailing `\s*$`
prevents accidentally matching keys whose value happens to start
with `write-all` (e.g. some future literal `write-all-something`).
2. **greptile P2 — duplicate violation when both patterns match**.
A `ref: refs/pull/${{ github.event.pull_request.head.sha }}/merge`
value matches both the `pull_request_target` rule's
`expressionPattern` (the `head.sha` interpolation) and its
`refPattern` (the `refs/pull/` literal). Each push generates an
ERROR line with the same description and just a different
`expression:` echo, so the reviewer sees the same violation twice.
Fix: track `stepFlagged` inside the per-step loop and skip the
`refPattern` fallback once any `expressionPattern` match has already
produced a violation for this step. The `refPattern` is a fallback
for ref-only forms (`refs/pull/123/head`, `${{ env.X }}` whose
resolved value is a PR ref); when the more specific expression
already fires, the fallback is redundant by definition.
After both fixes, the round-1 reproductions resolve cleanly:
$ # quoted form now blocks
$ ECC_WORKFLOWS_DIR=/tmp/q1/.github/workflows node scripts/ci/validate-workflow-security.js
ERROR: quoted.yml:8 - workflows with write permissions must disable checkout credential persistence
exit=1
$ # combined head.sha + refs/pull now prints one ERROR, not two
$ ECC_WORKFLOWS_DIR=/tmp/q2/.github/workflows node scripts/ci/validate-workflow-security.js
ERROR: dup.yml:10 - pull_request_target must not checkout an untrusted pull_request head ref/repository
Unsafe expression: ${{ github.event.pull_request.head.sha }}
exit=1
Test additions land in the next commit.
The `pull_request_target` rule's `expressionPattern` matches only
the canonical `github.event.pull_request.head.{ref,sha,repo.full_name}`
interpolations. It does not match the second canonical form of
the same exploit — fetching `refs/pull/<N>/{head,merge}` directly:
- uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/merge
The merge-ref variant is what GitHub's own security guidance calls
out as the highest-severity privilege-escalation pattern under
`pull_request_target`: it materialises the PR's merge commit
(attacker code spliced with base), executes inside a workflow that
has full repo-scoped tokens, and gives the attacker the chance to
exfiltrate secrets or push to default branches. `refs/pull/N/head`
is functionally equivalent — same source, same trust boundary.
Reproduced on `main` before this commit:
$ cat /tmp/bad.yml
name: bad
on: { pull_request_target: { types: [opened] } }
permissions: { contents: read }
jobs:
do:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/merge
persist-credentials: false
- run: npm ci --ignore-scripts
$ ECC_WORKFLOWS_DIR=/tmp node scripts/ci/validate-workflow-security.js
Validated workflow security for 1 workflow files
$ echo $?
0
Expected: violation flagging the refs/pull checkout under pull_request_target.
Actual: passes silently.
Fix: add a `refPattern` to the `pull_request_target` rule:
/^\s*ref:\s*['"]?[^'"\n]*refs\/(?:remotes\/)?pull\/[^'"\n\s]+/m
and apply it per checkout step inside the existing
event-gated loop. The pattern matches the ref VALUE so it catches
all interpolation shapes — `refs/pull/123/head`,
`refs/pull/${{ github.event.pull_request.number }}/merge`,
`${{ env.FOO }}/refs/pull/N/head` — without enumerating the
possible interpolations themselves.
Scoping: the rule is already gated on the workflow containing
`pull_request_target:`, so non-privileged `pull_request` workflows
that legitimately check out a PR ref are not affected.
After this commit the reproduction above exits 1 with:
ERROR: bad.yml:10 - pull_request_target must not checkout an untrusted pull_request head ref/repository
Three new regression tests in `tests/ci/validate-workflow-security.test.js`:
- rejects pull_request_target + refs/pull/<N>/merge
- rejects pull_request_target + hardcoded refs/pull/<N>/head
- allows pull_request_target with no `with.ref:` (base-ref checkout —
the safe pattern from GitHub's own guidance)
Test count: 17 → 20 in this file; full `yarn test` still green.
Together with the previous commit, this closes the two
independent `validate-workflow-security.js` bypasses I found.
`WRITE_PERMISSION_PATTERN` in `validate-workflow-security.js`
enumerates named GitHub Actions scopes (`contents: write`,
`issues: write`, etc.) to decide whether a workflow needs to:
- disable `persist-credentials` on `actions/checkout`
- pass `--ignore-scripts` to `npm ci`
The pattern misses the top-level shorthand `permissions:
write-all`, which is the strictly broader form — it grants every
named scope write access in a single line. As a result, a
workflow that opts into write-all currently slips both gates.
Reproduced on `main` before this commit:
$ cat /tmp/bad.yml
name: bad
on: [push]
permissions: write-all
jobs:
do:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: npm ci
$ ECC_WORKFLOWS_DIR=/tmp node scripts/ci/validate-workflow-security.js
Validated workflow security for 1 workflow files
$ echo $?
0
Expected: at least two violations (missing `persist-credentials:
false`, missing `--ignore-scripts`).
Actual: passes silently.
Fix: add a sibling pattern `WRITE_ALL_PATTERN` that matches
`^\s*permissions:\s*write-all\b` and OR it with
`WRITE_PERMISSION_PATTERN` at the single gate. Both top-level
and job-level `permissions:` blocks satisfy the `^\s*` prefix.
After this commit the reproduction above exits 1 with:
ERROR: bad.yml:8 - workflows with write permissions must disable checkout credential persistence
ERROR: bad.yml:9 - workflows with write permissions must install npm dependencies with --ignore-scripts
Three new regression tests in `tests/ci/validate-workflow-security.test.js`:
- rejects write-all + credential-persisting checkout
- rejects write-all + `npm ci` without `--ignore-scripts`
- allows write-all when both gates are satisfied (no over-block)
Test count: 14 → 17 in this file; full `yarn test` still green.
A separate `refs/pull/N/merge` bypass under `pull_request_target`
exists in the same validator and is fixed in the next commit.
Add a repo-level supply-chain incident response playbook for npm/GitHub Actions package-registry incidents, anchored on the May 2026 TanStack compromise and prior Shai-Hulud-style npm incidents.
- add `docs/security/supply-chain-incident-response.md` with exposure checks, immediate response steps, workflow rules, publication rules, and escalation triggers
- link the playbook from `SECURITY.md`
- reject `pull_request_target` workflows that restore or save shared dependency caches
- add a regression test for the new `pull_request_target + actions/cache` guardrail
Validation:
- node tests/ci/validate-workflow-security.test.js (12 passed, 0 failed)
- node scripts/ci/validate-workflow-security.js (validated 7 workflow files)
- npx markdownlint-cli 'SECURITY.md' 'docs/security/supply-chain-incident-response.md'
- npx markdownlint-cli '**/*.md' --ignore node_modules
- git diff --check
- node tests/run-all.js (2377 passed, 0 failed)
- GitHub CI for #1848 green across Ubuntu, Windows, and macOS
No release, tag, npm publish, plugin tag, marketplace submission, or announcement was performed.
Require npm registry signature verification wherever workflow npm audit checks run.
- add npm audit signatures to CI Security Scan and maintenance security audit jobs
- teach the workflow security validator to reject npm audit without signature verification
- keep the repair and Copilot prompt tests portable across Windows path/case and CRLF frontmatter behavior
Validation:
- node tests/run-all.js (2376 passed, 0 failed)
- CI current-head matrix green on #1846
- run non-test workflow installs with npm ci --ignore-scripts where lifecycle scripts are not needed\n- reject plain npm ci in workflows with write permissions\n- reject actions/cache in id-token: write workflows to reduce OIDC publish cache-poisoning risk
