mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-05-18 23:03:06 +08:00
e06d038257
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.