fix(ci): treat 'permissions: write-all' as a write-permission gate

`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.
This commit is contained in:
Jamkris
2026-05-15 14:58:47 +09:00
committed by Affaan Mustafa
parent f318e91b23
commit 7f971b7e6f
2 changed files with 35 additions and 1 deletions

View File

@@ -25,6 +25,12 @@ const RULES = [
];
const WRITE_PERMISSION_PATTERN = /^\s*(?:contents|issues|pull-requests|actions|checks|deployments|discussions|id-token|packages|pages|repository-projects|security-events|statuses):\s*write\b/m;
// `permissions: write-all` is GitHub Actions' shorthand for granting every
// scope write access. The named-scope pattern above misses it because there
// is no scope name on the left of the colon — just the literal `write-all`
// value at the permissions key. Treat both as equivalent for the purposes
// of the persist-credentials and lifecycle-script gates below.
const WRITE_ALL_PATTERN = /^\s*permissions:\s*write-all\b/m;
const NPM_AUDIT_PATTERN = /\bnpm\s+audit\b(?!\s+signatures\b)/;
const NPM_AUDIT_SIGNATURES_PATTERN = /\bnpm\s+audit\s+signatures\b/;
const ACTIONS_CACHE_PATTERN = /uses:\s*['"]?actions\/cache@/m;
@@ -124,7 +130,7 @@ function findViolations(filePath, source) {
}
}
if (WRITE_PERMISSION_PATTERN.test(source)) {
if (WRITE_PERMISSION_PATTERN.test(source) || WRITE_ALL_PATTERN.test(source)) {
for (const step of checkoutSteps) {
if (!/persist-credentials:\s*['"]?false['"]?\b/m.test(step.text)) {
violations.push({