From 3792b69a38eaaff38682f8c50ecbb5bb8c25c6bb Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sun, 12 Apr 2026 23:23:01 -0700 Subject: [PATCH] fix: block unsafe privileged workflow checkouts --- .github/workflows/ci.yml | 4 + .github/workflows/reusable-validate.yml | 3 + scripts/ci/validate-workflow-security.js | 138 ++++++++++++++++++++ tests/ci/validate-workflow-security.test.js | 90 +++++++++++++ 4 files changed, 235 insertions(+) create mode 100644 scripts/ci/validate-workflow-security.js create mode 100644 tests/ci/validate-workflow-security.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04972a2c..1177a249 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,6 +190,10 @@ jobs: run: node scripts/ci/validate-install-manifests.js continue-on-error: false + - name: Validate workflow security + run: node scripts/ci/validate-workflow-security.js + continue-on-error: false + - name: Validate rules run: node scripts/ci/validate-rules.js continue-on-error: false diff --git a/.github/workflows/reusable-validate.yml b/.github/workflows/reusable-validate.yml index 7d3748ae..a606404b 100644 --- a/.github/workflows/reusable-validate.yml +++ b/.github/workflows/reusable-validate.yml @@ -42,6 +42,9 @@ jobs: - name: Validate install manifests run: node scripts/ci/validate-install-manifests.js + - name: Validate workflow security + run: node scripts/ci/validate-workflow-security.js + - name: Validate rules run: node scripts/ci/validate-rules.js diff --git a/scripts/ci/validate-workflow-security.js b/scripts/ci/validate-workflow-security.js new file mode 100644 index 00000000..7f6183ba --- /dev/null +++ b/scripts/ci/validate-workflow-security.js @@ -0,0 +1,138 @@ +#!/usr/bin/env node +/** + * Reject unsafe GitHub Actions patterns that execute or checkout untrusted PR code + * from privileged events such as workflow_run or pull_request_target. + */ + +const fs = require('fs'); +const path = require('path'); + +const DEFAULT_WORKFLOWS_DIR = path.join(__dirname, '../../.github/workflows'); + +const RULES = [ + { + event: 'workflow_run', + eventPattern: /\bworkflow_run\s*:/m, + description: 'workflow_run must not checkout an untrusted workflow_run head ref/repository', + expressionPattern: /\$\{\{\s*github\.event\.workflow_run\.(?:head_branch|head_sha|head_repository(?:\.[A-Za-z0-9_.]+)?)\s*\}\}|\$\{\{\s*github\.event\.workflow_run\.pull_requests\[\d+\]\.head\.(?:ref|sha|repo\.full_name)\s*\}\}/g, + }, + { + event: 'pull_request_target', + 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, + }, +]; + +function getWorkflowFiles(workflowsDir) { + if (!fs.existsSync(workflowsDir)) { + return []; + } + + return fs.readdirSync(workflowsDir) + .filter(file => /\.(?:yml|yaml)$/i.test(file)) + .map(file => path.join(workflowsDir, file)) + .sort(); +} + +function getLineNumber(source, index) { + return source.slice(0, index).split(/\r?\n/).length; +} + +function extractCheckoutSteps(source) { + const blocks = []; + const lines = source.split(/\r?\n/); + let current = null; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const stepStart = line.match(/^(\s*)-\s+/); + + if (stepStart) { + if (current) { + blocks.push(current); + } + + current = { + indent: stepStart[1].length, + startLine: i + 1, + lines: [line], + }; + continue; + } + + if (current) { + current.lines.push(line); + } + } + + if (current) { + blocks.push(current); + } + + return blocks + .map(block => ({ + startLine: block.startLine, + text: block.lines.join('\n'), + })) + .filter(block => /uses:\s*actions\/checkout@/m.test(block.text)); +} + +function findViolations(filePath, source) { + const violations = []; + const checkoutSteps = extractCheckoutSteps(source); + + for (const rule of RULES) { + if (!rule.eventPattern.test(source)) { + continue; + } + + for (const step of checkoutSteps) { + for (const match of step.text.matchAll(rule.expressionPattern)) { + violations.push({ + filePath, + event: rule.event, + description: rule.description, + expression: match[0], + line: step.startLine + getLineNumber(step.text, match.index) - 1, + }); + } + } + } + + return violations; +} + +function validateWorkflowSecurity(workflowsDir = DEFAULT_WORKFLOWS_DIR) { + const files = getWorkflowFiles(workflowsDir); + const violations = []; + + for (const filePath of files) { + const source = fs.readFileSync(filePath, 'utf8'); + violations.push(...findViolations(filePath, source)); + } + + if (violations.length > 0) { + for (const violation of violations) { + console.error( + `ERROR: ${path.basename(violation.filePath)}:${violation.line} - ${violation.description}`, + ); + console.error(` Unsafe expression: ${violation.expression}`); + } + return 1; + } + + console.log(`Validated workflow security for ${files.length} workflow files`); + return 0; +} + +if (require.main === module) { + process.exit(validateWorkflowSecurity(process.env.ECC_WORKFLOWS_DIR || DEFAULT_WORKFLOWS_DIR)); +} + +module.exports = { + DEFAULT_WORKFLOWS_DIR, + extractCheckoutSteps, + findViolations, + validateWorkflowSecurity, +}; diff --git a/tests/ci/validate-workflow-security.test.js b/tests/ci/validate-workflow-security.test.js new file mode 100644 index 00000000..b1e69a39 --- /dev/null +++ b/tests/ci/validate-workflow-security.test.js @@ -0,0 +1,90 @@ +#!/usr/bin/env node +/** + * Validate workflow security guardrails for privileged GitHub Actions events. + */ + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const SCRIPT_PATH = path.join(__dirname, '..', '..', 'scripts', 'ci', 'validate-workflow-security.js'); + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + return false; + } +} + +function runValidator(files) { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-workflow-security-')); + try { + for (const [name, contents] of Object.entries(files)) { + fs.writeFileSync(path.join(tempDir, name), contents); + } + + return spawnSync('node', [SCRIPT_PATH], { + encoding: 'utf8', + env: { + ...process.env, + ECC_WORKFLOWS_DIR: tempDir, + }, + }); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +} + +function run() { + console.log('\n=== Testing workflow security validation ===\n'); + + let passed = 0; + let failed = 0; + + if (test('allows safe workflow_run workflow that only checks out the base repository', () => { + const result = runValidator({ + 'safe.yml': `name: Safe\non:\n workflow_run:\n workflows: ["CI"]\n types: [completed]\njobs:\n repair:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n - run: echo safe\n`, + }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + })) passed++; else failed++; + + if (test('rejects workflow_run checkout using github.event.workflow_run.head_branch', () => { + const result = runValidator({ + 'unsafe-workflow-run.yml': `name: Unsafe\non:\n workflow_run:\n workflows: ["CI"]\n types: [completed]\njobs:\n repair:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n ref: \${{ github.event.workflow_run.head_branch }}\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail'); + assert.match(result.stderr, /workflow_run must not checkout an untrusted workflow_run head ref\/repository/); + assert.match(result.stderr, /head_branch/); + })) passed++; else failed++; + + if (test('rejects workflow_run checkout using github.event.workflow_run.head_repository.full_name', () => { + const result = runValidator({ + 'unsafe-repository.yml': `name: Unsafe\non:\n workflow_run:\n workflows: ["CI"]\n types: [completed]\njobs:\n repair:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n repository: \${{ github.event.workflow_run.head_repository.full_name }}\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail'); + assert.match(result.stderr, /head_repository\.full_name/); + })) passed++; else failed++; + + if (test('rejects pull_request_target checkout using github.event.pull_request.head.sha', () => { + const result = runValidator({ + 'unsafe-pr-target.yml': `name: Unsafe\non:\n pull_request_target:\n branches: [main]\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n with:\n ref: \${{ github.event.pull_request.head.sha }}\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail'); + assert.match(result.stderr, /pull_request_target must not checkout an untrusted pull_request head ref\/repository/); + assert.match(result.stderr, /pull_request\.head\.sha/); + })) passed++; else failed++; + + console.log(`\nPassed: ${passed}`); + console.log(`Failed: ${failed}`); + + process.exit(failed > 0 ? 1 : 0); +} + +run();