Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/reusable-validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ jobs:

- name: Validate skills
run: node scripts/ci/validate-skills.js

- name: Validate workflow security
run: node scripts/ci/validate-workflow-security.js
10 changes: 10 additions & 0 deletions hooks/hooks.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
"$schema": "https://json.schemastore.org/gemini-code-settings.json",
"hooks": {
"BeforeTool": [
{
"matcher": "tool == \"run_shell_command\"",
"hooks": [
{
"type": "command",
"command": "node \"$HOME/.gemini/extensions/everything-gemini-code/scripts/hooks/block-no-verify.js\""
}
],
"description": "Block git --no-verify and core.hooksPath= overrides on commit/push/merge/cherry-pick/rebase/am — protects pre-commit, commit-msg, and pre-push hooks from being skipped"
},
{
"matcher": "tool == \"run_shell_command\" && tool_input.command matches \"(npm run dev|pnpm( run)? dev|yarn dev|bun run dev)\"",
"hooks": [
Expand Down
138 changes: 138 additions & 0 deletions scripts/ci/validate-workflow-security.js
Original file line number Diff line number Diff line change
@@ -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,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regex for detecting unsafe expressions is not exhaustive. It misses bracket notation (e.g., github['event']...) and array wildcards (e.g., pull_requests[*]). While it covers the most common patterns, an attacker could potentially bypass this check using alternative expression syntax.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledging this as a known limitation, deferring to a follow-up.

The current regex covers the dot-notation patterns that show up in real-world unsafe workflows. Bracket notation (github['event']['workflow_run']['head_branch']) and array wildcards (pull_requests[*]) are theoretical bypasses but I haven't seen them used in the wild for this attack class — most write-ups (Salus, Snyk, GitHub's own guidance) target the dot-notation form.

Doing this properly needs a small expression-language parser rather than another regex layer; bolting more alternations onto the existing pattern would just shift the bypass surface around. I'd rather track this as a follow-up issue than ship a half-fix that gives a false sense of completeness.

Comment 6 (the quoted uses: filter bypass) is a different class — that one I fixed in ce2ca96 because it broke the coverage of the validator, not just one expression form.

},
{
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,
};
Loading
Loading