diff --git a/scripts/ci/validate-workflow-security.js b/scripts/ci/validate-workflow-security.js index bdefcdb7..4e65cdef 100644 --- a/scripts/ci/validate-workflow-security.js +++ b/scripts/ci/validate-workflow-security.js @@ -21,6 +21,15 @@ const RULES = [ eventPattern: /\bpull_request_target\s*:/m, description: 'pull_request_target must not checkout an untrusted pull_request head ref/repository', expressionPattern: /\$\{\{\s*github\.event\.pull_request\.head\.(?:ref|sha|repo\.full_name)\s*\}\}/g, + // Even without the standard `github.event.pull_request.head.*` expression, + // a checkout under `pull_request_target` that fetches a `refs/pull//{head,merge}` + // ref pulls attacker-controlled code into a workflow with write-scoped + // tokens. GitHub's security guidance treats both forms equivalently; + // we match the ref value directly so any interpolation that resolves + // to such a ref (`refs/pull/${{ github.event.pull_request.number }}/merge`, + // a hardcoded `refs/pull/123/head`, a `${{ env.X }}` that the maintainer + // assumes is safe, etc.) trips the same rule. + refPattern: /^\s*ref:\s*['"]?[^'"\n]*refs\/(?:remotes\/)?pull\/[^'"\n\s]+/m, }, ]; @@ -127,6 +136,18 @@ function findViolations(filePath, source) { line: step.startLine + getLineNumber(step.text, match.index) - 1, }); } + if (rule.refPattern) { + const refMatch = step.text.match(rule.refPattern); + if (refMatch) { + violations.push({ + filePath, + event: rule.event, + description: rule.description, + expression: refMatch[0].trim(), + line: step.startLine + getLineNumber(step.text, refMatch.index) - 1, + }); + } + } } } diff --git a/tests/ci/validate-workflow-security.test.js b/tests/ci/validate-workflow-security.test.js index cacbcc39..9c51d6d2 100644 --- a/tests/ci/validate-workflow-security.test.js +++ b/tests/ci/validate-workflow-security.test.js @@ -99,6 +99,34 @@ function run() { assert.match(result.stderr, /pull_request\.head\.sha/); })) passed++; else failed++; + // `refs/pull//{head,merge}` under `pull_request_target` is the canonical + // privilege-escalation pattern that the standard `github.event.pull_request.head.*` + // expression check did not cover. Either form pulls attacker-controlled code + // into a privileged workflow. + + if (test('rejects pull_request_target checkout fetching refs/pull/N/merge', () => { + const result = runValidator({ + 'unsafe-pr-target-merge-ref.yml': `name: Unsafe\non:\n pull_request_target:\n types: [opened]\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n ref: refs/pull/\${{ github.event.pull_request.number }}/merge\n persist-credentials: false\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail on refs/pull/N/merge under pull_request_target'); + assert.match(result.stderr, /pull_request_target must not checkout an untrusted pull_request head ref/); + })) passed++; else failed++; + + if (test('rejects pull_request_target checkout fetching hardcoded refs/pull/N/head', () => { + const result = runValidator({ + 'unsafe-pr-target-head-ref.yml': `name: Unsafe\non:\n pull_request_target:\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n ref: refs/pull/123/head\n persist-credentials: false\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail on hardcoded refs/pull/N/head'); + assert.match(result.stderr, /pull_request_target must not checkout an untrusted pull_request head ref/); + })) passed++; else failed++; + + if (test('allows pull_request_target checkout of the base ref (no with.ref)', () => { + const result = runValidator({ + 'safe-pr-target-base.yml': `name: Safe\non:\n pull_request_target:\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n persist-credentials: false\n - run: echo inspecting base\n`, + }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + })) passed++; else failed++; + if (test('rejects shared cache use in pull_request_target workflows', () => { const result = runValidator({ 'unsafe-pr-target-cache.yml': `name: Unsafe\non:\n pull_request_target:\n branches: [main]\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/cache@v5\n with:\n path: ~/.npm\n key: cache\n - run: echo inspect\n`,