ci: Add workflow pull_request_target safety rule to code-health (no-changelog) (#30327)

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

.github/workflows/ci-pr-quality.yml

@@ -134,6 +134,7 @@ jobs:
               pnpm-workspace.yaml
               .code-health-baseline.json
               packages/testing/code-health/**
+              .github/workflows/**
 
   check-static-analysis:
     name: Static Analysis

packages/testing/code-health/src/index.ts

@@ -3,9 +3,11 @@ import type { RuleSettingsMap } from '@n8n/rules-engine';
 
 import type { CodeHealthContext } from './context.js';
 import { CatalogViolationsRule } from './rules/catalog-violations.rule.js';
+import { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js';
 
 export type { CodeHealthContext } from './context.js';
 export { CatalogViolationsRule } from './rules/catalog-violations.rule.js';
+export { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js';
 
 const defaultRuleSettings: RuleSettingsMap = {
 	'catalog-violations': {
@@ -13,6 +15,11 @@ const defaultRuleSettings: RuleSettingsMap = {
 		severity: 'error',
 		options: { workspaceFile: 'pnpm-workspace.yaml' },
 	},
+	'workflow-pr-target-safety': {
+		enabled: true,
+		severity: 'error',
+		options: { allowedWorkflows: ['ci-cla-check.yml'] },
+	},
 };
 
 function mergeSettings(defaults: RuleSettingsMap, overrides?: RuleSettingsMap): RuleSettingsMap {
@@ -31,6 +38,7 @@ function mergeSettings(defaults: RuleSettingsMap, overrides?: RuleSettingsMap):
 export function createDefaultRunner(settings?: RuleSettingsMap): RuleRunner<CodeHealthContext> {
 	const runner = new RuleRunner<CodeHealthContext>();
 	runner.registerRule(new CatalogViolationsRule());
+	runner.registerRule(new WorkflowPrTargetSafetyRule());
 	runner.applySettings(mergeSettings(defaultRuleSettings, settings));
 	return runner;
 }

packages/testing/code-health/src/rules/workflow-pr-target-safety.rule.test.ts

@@ -0,0 +1,267 @@
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+import type { CodeHealthContext } from '../context.js';
+import { WorkflowPrTargetSafetyRule } from './workflow-pr-target-safety.rule.js';
+
+function createTempDir(): string {
+	return fs.mkdtempSync(path.join(os.tmpdir(), 'code-health-workflow-test-'));
+}
+
+function writeWorkflow(dir: string, name: string, content: string): void {
+	const fullPath = path.join(dir, '.github', 'workflows', name);
+	fs.mkdirSync(path.dirname(fullPath), { recursive: true });
+	fs.writeFileSync(fullPath, content);
+}
+
+describe('WorkflowPrTargetSafetyRule', () => {
+	let tmpDir: string;
+	let rule: WorkflowPrTargetSafetyRule;
+
+	beforeEach(() => {
+		tmpDir = createTempDir();
+		rule = new WorkflowPrTargetSafetyRule();
+		rule.configure({ options: { allowedWorkflows: ['ci-cla-check.yml'] } });
+	});
+
+	afterEach(() => {
+		fs.rmSync(tmpDir, { recursive: true, force: true });
+	});
+
+	function context(): CodeHealthContext {
+		return { rootDir: tmpDir };
+	}
+
+	it('ignores workflows that only use pull_request', async () => {
+		writeWorkflow(
+			tmpDir,
+			'safe.yml',
+			`
+name: Safe
+on:
+  pull_request:
+    types: [opened]
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: \${{ github.event.pull_request.head.sha }}
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(0);
+	});
+
+	it('flags any non-allowlisted workflow that uses pull_request_target', async () => {
+		writeWorkflow(
+			tmpDir,
+			'risky.yml',
+			`
+name: Risky
+on:
+  pull_request_target:
+    types: [opened]
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0].rule).toBe('workflow-pr-target-safety');
+		expect(violations[0].message).toContain('pull_request_target');
+		expect(violations[0].message).toContain('Prefer pull_request');
+	});
+
+	it('flags pull_request_target even when on: is a list', async () => {
+		writeWorkflow(
+			tmpDir,
+			'list-trigger.yml',
+			`
+name: List
+on: [pull_request_target]
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+	});
+
+	it('allows pull_request_target in an allowlisted workflow with no checkout override', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+    types: [opened]
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          sparse-checkout: .github/scripts/cla
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(0);
+	});
+
+	it('flags allowlisted workflow that checks out PR head sha', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+    types: [opened]
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: \${{ github.event.pull_request.head.sha }}
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0].message).toContain('PR-author-controlled ref');
+		expect(violations[0].message).toContain('their code, our keys');
+	});
+
+	it('flags allowlisted workflow that checks out github.head_ref', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: \${{ github.head_ref }}
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0].message).toContain('PR-author-controlled ref');
+	});
+
+	it('flags allowlisted workflow that checks out PR fork repository', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          repository: \${{ github.event.pull_request.head.repo.full_name }}
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0].message).toContain('PR-author-controlled repository');
+	});
+
+	it('flags shell git checkout of PR head ref', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Fetch PR
+        run: |
+          git fetch origin \${{ github.event.pull_request.head.sha }}
+          git checkout FETCH_HEAD
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0].message).toContain('shell git command');
+	});
+
+	it('does not flag a checkout with no ref override', async () => {
+		writeWorkflow(
+			tmpDir,
+			'ci-cla-check.yml',
+			`
+name: CLA Check
+on:
+  pull_request_target:
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          path: vendor
+`,
+		);
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(0);
+	});
+
+	it('handles workflows with no triggers without throwing', async () => {
+		writeWorkflow(tmpDir, 'broken.yml', 'name: Broken\njobs: {}\n');
+
+		const violations = await rule.analyze(context());
+
+		expect(violations).toHaveLength(0);
+	});
+
+	it('skips files that are not valid YAML', async () => {
+		writeWorkflow(tmpDir, 'invalid.yml', '::: not yaml :::\n\t- broken');
+
+		await expect(rule.analyze(context())).resolves.toBeDefined();
+	});
+});

packages/testing/code-health/src/rules/workflow-pr-target-safety.rule.ts

@@ -0,0 +1,150 @@
+import { BaseRule } from '@n8n/rules-engine';
+import type { Violation } from '@n8n/rules-engine';
+import * as path from 'node:path';
+
+import type { CodeHealthContext } from '../context.js';
+import {
+	findLineContaining,
+	findWorkflowFiles,
+	parseWorkflow,
+	type WorkflowFile,
+	type WorkflowJobStep,
+} from '../utils/workflow-scanner.js';
+
+const RISKY_TRIGGER = 'pull_request_target';
+
+const UNTRUSTED_REF_EXPRESSIONS = [
+	'github.event.pull_request.head.sha',
+	'github.event.pull_request.head.ref',
+	'github.event.pull_request.merge_commit_sha',
+	'github.head_ref',
+];
+
+const UNTRUSTED_REPOSITORY_EXPRESSIONS = [
+	'github.event.pull_request.head.repo.full_name',
+	'github.event.pull_request.head.repo.clone_url',
+];
+
+export class WorkflowPrTargetSafetyRule extends BaseRule<CodeHealthContext> {
+	readonly id = 'workflow-pr-target-safety';
+	readonly name = 'Workflow pull_request_target Safety';
+	readonly description =
+		'Disallow pull_request_target triggers. Allowlisted workflows may use it only if they do not check out PR-author-controlled code.';
+	readonly severity = 'error' as const;
+
+	async analyze(context: CodeHealthContext): Promise<Violation[]> {
+		const { rootDir } = context;
+		const options = this.getOptions();
+		const allowedWorkflows = Array.isArray(options.allowedWorkflows)
+			? (options.allowedWorkflows as string[])
+			: [];
+
+		const files = await findWorkflowFiles(rootDir);
+		const violations: Violation[] = [];
+
+		for (const filePath of files) {
+			const workflow = parseWorkflow(filePath, rootDir);
+			if (!workflow) continue;
+			if (!workflow.triggers.includes(RISKY_TRIGGER)) continue;
+
+			const fileName = path.basename(filePath);
+			if (!allowedWorkflows.includes(fileName)) {
+				violations.push(this.flagTrigger(workflow));
+				continue;
+			}
+
+			violations.push(...this.flagUnsafeCheckouts(workflow));
+		}
+
+		return violations;
+	}
+
+	private flagTrigger(workflow: WorkflowFile): Violation {
+		return this.createViolation(
+			workflow.filePath,
+			findLineContaining(workflow.lines, RISKY_TRIGGER),
+			1,
+			`${workflow.relativePath} uses pull_request_target. Prefer pull_request — pull_request_target runs in the base-repo context with secrets exposed, which is unsafe when combined with PR-author-controlled code.`,
+			'Switch to pull_request, or if pull_request_target is required, request a security review and add this workflow to the rule allowlist.',
+		);
+	}
+
+	private flagUnsafeCheckouts(workflow: WorkflowFile): Violation[] {
+		const violations: Violation[] = [];
+
+		for (const job of workflow.jobs) {
+			for (const step of job.steps) {
+				violations.push(...this.flagCheckoutAction(workflow, job.id, step));
+				violations.push(...this.flagShellCheckout(workflow, job.id, step));
+			}
+		}
+
+		return violations;
+	}
+
+	private flagCheckoutAction(
+		workflow: WorkflowFile,
+		jobId: string,
+		step: WorkflowJobStep,
+	): Violation[] {
+		if (!step.uses?.startsWith('actions/checkout@')) return [];
+		const stepWith = step.with;
+		if (!stepWith) return [];
+
+		const violations: Violation[] = [];
+		const stepLine = findLineContaining(workflow.lines, step.uses);
+
+		const ref = typeof stepWith.ref === 'string' ? stepWith.ref : undefined;
+		if (ref && containsExpression(ref, UNTRUSTED_REF_EXPRESSIONS)) {
+			violations.push(
+				this.createViolation(
+					workflow.filePath,
+					findLineContaining(workflow.lines, 'ref:', stepLine),
+					1,
+					`${workflow.relativePath} job "${jobId}" checks out PR-author-controlled ref under pull_request_target. This pattern ("their code, our keys") is the canonical pull_request_target exploit.`,
+					'Remove the ref override (defaults to the base branch), or move this step to a workflow triggered by pull_request.',
+				),
+			);
+		}
+
+		const repository = typeof stepWith.repository === 'string' ? stepWith.repository : undefined;
+		if (repository && containsExpression(repository, UNTRUSTED_REPOSITORY_EXPRESSIONS)) {
+			violations.push(
+				this.createViolation(
+					workflow.filePath,
+					findLineContaining(workflow.lines, 'repository:', stepLine),
+					1,
+					`${workflow.relativePath} job "${jobId}" checks out a PR-author-controlled repository under pull_request_target.`,
+					'Remove the repository override so the checkout stays on github.repository.',
+				),
+			);
+		}
+
+		return violations;
+	}
+
+	private flagShellCheckout(
+		workflow: WorkflowFile,
+		jobId: string,
+		step: WorkflowJobStep,
+	): Violation[] {
+		if (!step.run) return [];
+		const usesGitCheckout = /\bgit\s+(checkout|fetch|pull)\b/.test(step.run);
+		if (!usesGitCheckout) return [];
+		if (!containsExpression(step.run, UNTRUSTED_REF_EXPRESSIONS)) return [];
+
+		return [
+			this.createViolation(
+				workflow.filePath,
+				findLineContaining(workflow.lines, 'run:'),
+				1,
+				`${workflow.relativePath} job "${jobId}" runs a shell git command that fetches PR-author-controlled refs under pull_request_target.`,
+				'Avoid checking out PR head from a pull_request_target workflow; do the work in a pull_request-triggered workflow instead.',
+			),
+		];
+	}
+}
+
+function containsExpression(value: string, needles: string[]): boolean {
+	return needles.some((needle) => value.includes(needle));
+}

packages/testing/code-health/src/utils/workflow-scanner.ts

@@ -0,0 +1,105 @@
+import fg from 'fast-glob';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import { parse as parseYaml } from 'yaml';
+
+export interface WorkflowJobStep {
+	name?: string;
+	uses?: string;
+	run?: string;
+	with?: Record<string, unknown>;
+}
+
+export interface WorkflowJob {
+	id: string;
+	steps: WorkflowJobStep[];
+}
+
+export interface WorkflowFile {
+	filePath: string;
+	relativePath: string;
+	lines: string[];
+	triggers: string[];
+	jobs: WorkflowJob[];
+}
+
+export async function findWorkflowFiles(rootDir: string): Promise<string[]> {
+	return await fg('.github/workflows/*.{yml,yaml}', {
+		cwd: rootDir,
+		absolute: true,
+		dot: true,
+	});
+}
+
+export function parseWorkflow(filePath: string, rootDir: string): WorkflowFile | null {
+	const content = fs.readFileSync(filePath, 'utf-8');
+	const lines = content.split('\n');
+
+	let doc: unknown;
+	try {
+		doc = parseYaml(content);
+	} catch {
+		return null;
+	}
+
+	if (!doc || typeof doc !== 'object') return null;
+
+	const workflow = doc as Record<string, unknown>;
+	// YAML's `on:` keyword gets parsed as boolean `true` by the yaml lib
+	// unless quoted, so check both keys.
+	const onValue = workflow.on ?? workflow.true;
+
+	return {
+		filePath,
+		relativePath: path.relative(rootDir, filePath),
+		lines,
+		triggers: extractTriggers(onValue),
+		jobs: extractJobs(workflow.jobs),
+	};
+}
+
+function extractTriggers(onValue: unknown): string[] {
+	if (typeof onValue === 'string') return [onValue];
+	if (Array.isArray(onValue)) return onValue.filter((t): t is string => typeof t === 'string');
+	if (onValue && typeof onValue === 'object') return Object.keys(onValue);
+	return [];
+}
+
+function extractJobs(jobsValue: unknown): WorkflowJob[] {
+	if (!jobsValue || typeof jobsValue !== 'object') return [];
+
+	const jobs: WorkflowJob[] = [];
+	for (const [id, raw] of Object.entries(jobsValue as Record<string, unknown>)) {
+		if (!raw || typeof raw !== 'object') continue;
+		const job = raw as Record<string, unknown>;
+		const stepsValue = job.steps;
+		if (!Array.isArray(stepsValue)) {
+			jobs.push({ id, steps: [] });
+			continue;
+		}
+
+		const steps: WorkflowJobStep[] = [];
+		for (const stepRaw of stepsValue) {
+			if (!stepRaw || typeof stepRaw !== 'object') continue;
+			const step = stepRaw as Record<string, unknown>;
+			steps.push({
+				name: typeof step.name === 'string' ? step.name : undefined,
+				uses: typeof step.uses === 'string' ? step.uses : undefined,
+				run: typeof step.run === 'string' ? step.run : undefined,
+				with:
+					step.with && typeof step.with === 'object'
+						? (step.with as Record<string, unknown>)
+						: undefined,
+			});
+		}
+		jobs.push({ id, steps });
+	}
+	return jobs;
+}
+
+export function findLineContaining(lines: string[], needle: string, startLine = 0): number {
+	for (let i = startLine; i < lines.length; i++) {
+		if (lines[i].includes(needle)) return i + 1;
+	}
+	return 1;
+}