Skip to content

fix(gh-aw): make Squad Review safe outputs first-install safe #2104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bradygaster merged 2 commits into from
Sep 29, 2026
Merged

fix(gh-aw): make Squad Review safe outputs first-install safe #2104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c42035
fix(gh-aw): make review guard first-install safe
bradygaster Sep 29, 2026
fa940f4
fix(gh-aw): fail closed on missing installed review guard
bradygaster Sep 29, 2026
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
135 changes: 133 additions & 2 deletions test/gh-aw-review-workflow.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { afterAll, describe, expect, it } from 'vitest';
import { cpSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
import { execFileSync, spawnSync } from 'node:child_process';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { extractSafeOutputsConfigJson } from './helpers/gh-aw-lock.js';
import { parse } from 'yaml';

Expand Down Expand Up @@ -163,11 +163,95 @@ function assertCompiledGate(lock: string): void {
expect(steps[guard].with.script).toContain('await guard.enforceReviewOutputs');
expect(steps[guard].if).toBeUndefined();
expect(steps[guard]['continue-on-error']).toBeUndefined();
expect(steps[guard - 1].with.ref).toBe('${{ github.event.pull_request.base.sha || github.workflow_sha }}');
const baseCheckout = steps.find((step: { name: string }) =>
step.name === 'Checkout base commit for review guard');
const workflowCheckout = steps.find((step: { name: string }) =>
step.name === 'Checkout workflow commit for first-install review guard');
expect(baseCheckout.with.ref).toBe('${{ github.event.pull_request.base.sha || github.workflow_sha }}');
expect(workflowCheckout.with.ref).toBe('${{ github.workflow_sha }}');
expect(steps[guard].with.script).toContain('if (existsSync(baseGuard))');
expect(steps[guard].with.script).toContain('else if (!existsSync(baseManifest))');
expect(steps[guard].with.script).toContain('missing from an established base installation');
const finalGate = review.steps.find((step: { name: string }) =>
step.name === 'Enforce independent current-head verdict');
expect(finalGate.with.script).toContain('if (existsSync(baseGuard))');
expect(finalGate.with.script).toContain('else if (!existsSync(baseManifest))');
expect(finalGate.with.script).toContain('missing from an established base installation');
for (const step of review.steps) expect(step['continue-on-error']).toBeUndefined();
expect(review['continue-on-error']).toBeUndefined();
}

function compiledGuardScripts(lock: string): Array<{ name: string; script: string }> {
const workflow = parse(lock);
const safeOutputsStep = workflow.jobs.safe_outputs.steps.find((candidate: { name: string }) =>
candidate.name === 'Bind review output to committed agent identities and current head');
const finalGateStep = workflow.jobs.review.steps.find((candidate: { name: string }) =>
candidate.name === 'Enforce independent current-head verdict');
expect(safeOutputsStep, 'compiled safe_outputs guard step is missing').toBeDefined();
expect(finalGateStep, 'compiled final verdict guard step is missing').toBeDefined();
return [
{ name: 'safe outputs', script: safeOutputsStep.with.script },
{ name: 'final verdict', script: finalGateStep.with.script },
];
}

async function executeCompiledGuardLoader(
script: string,
{
baseGuard,
baseManifest = false,
workflowGuard,
head = 'a'.repeat(40),
}: { baseGuard?: string; baseManifest?: boolean; workflowGuard: string; head?: string },
): Promise<string> {
const workspace = mkdtempSync(resolve(ROOT, '.squad-review-loader-'));
compileWorkspaces.push(workspace);
const output = resolve(workspace, 'loaded.txt');
const guardPath = '.github/workflows/shared/squad-review-guard.mjs';
if (baseGuard !== undefined) {
const path = resolve(workspace, '.squad-review-base', guardPath);
mkdirSync(dirname(path), { recursive: true });
writeFileSync(path, baseGuard);
}
if (baseManifest) {
const path = resolve(
workspace,
'.squad-review-base',
'.github/aw/squad-workflows.manifest.json',
);
mkdirSync(dirname(path), { recursive: true });
writeFileSync(path, '{}\n');
}
const workflowPath = resolve(workspace, '.squad-review-workflow', guardPath);
mkdirSync(dirname(workflowPath), { recursive: true });
writeFileSync(workflowPath, workflowGuard);

const previousWorkspace = process.env.GITHUB_WORKSPACE;
const previousOutput = process.env.SQUAD_TEST_GUARD_OUTPUT;
const previousHead = process.env.SQUAD_REVIEW_HEAD;
process.env.GITHUB_WORKSPACE = workspace;
process.env.SQUAD_TEST_GUARD_OUTPUT = output;
process.env.SQUAD_REVIEW_HEAD = head;
try {
const runner = resolve(workspace, 'runner.mjs');
writeFileSync(runner, `
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
const github = { request: async () => ({ data: {} }) };
${script}
`);
await import(`${pathToFileURL(runner).href}?run=${Date.now()}`);
return readFileSync(output, 'utf8');
} finally {
if (previousWorkspace === undefined) delete process.env.GITHUB_WORKSPACE;
else process.env.GITHUB_WORKSPACE = previousWorkspace;
if (previousOutput === undefined) delete process.env.SQUAD_TEST_GUARD_OUTPUT;
else process.env.SQUAD_TEST_GUARD_OUTPUT = previousOutput;
if (previousHead === undefined) delete process.env.SQUAD_REVIEW_HEAD;
else process.env.SQUAD_REVIEW_HEAD = previousHead;
}
}

describe('gh-aw enforcing Squad reviewer', () => {
it('routes /squad review to the isolated workflow and supports automatic PR events', () => {
const routerDispatch = yamlBlock(ROUTER_FRONTMATTER, 'dispatch-workflow');
Expand Down Expand Up @@ -353,6 +437,53 @@ describe('gh-aw enforcing Squad reviewer', () => {
expect(lock).toContain('GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }}');
}, 30000);

it('executes the compiled first-install fallback without weakening established base control', async () => {
const { lock } = compileReviewer();
const stub = (source: string) => `
import { writeFileSync } from 'node:fs';
export async function enforceReviewOutputs(env) {
writeFileSync(process.env.SQUAD_TEST_GUARD_OUTPUT,
JSON.stringify({ source: ${JSON.stringify(source)}, operation: 'safe outputs', head: env.SQUAD_REVIEW_HEAD }));
}
export async function assertClearingReview(env) {
writeFileSync(process.env.SQUAD_TEST_GUARD_OUTPUT,
JSON.stringify({ source: ${JSON.stringify(source)}, operation: 'final verdict', head: env.SQUAD_REVIEW_HEAD }));
}
`;

for (const { name, script } of compiledGuardScripts(lock)) {
const head = name === 'safe outputs'
? 'edd6fbf0f84334ff086ed2bc8bbfed5503fdd56f'
: '9a898b5182ca432b62bef26dbf1f9022dd1d256b';
await expect(executeCompiledGuardLoader(script, {
workflowGuard: stub('workflow'),
head,
})).resolves.toBe(JSON.stringify({ source: 'workflow', operation: name, head }));

await expect(executeCompiledGuardLoader(script, {
baseGuard: stub('base'),
baseManifest: true,
workflowGuard: stub('workflow'),
head,
})).resolves.toBe(JSON.stringify({ source: 'base', operation: name, head }));

await expect(executeCompiledGuardLoader(script, {
baseManifest: true,
workflowGuard: stub('workflow'),
head,
})).rejects.toThrow(
'Squad Review guard is missing from an established base installation. Refusing workflow-source fallback.',
);

await expect(executeCompiledGuardLoader(script, {
baseGuard: 'this is not valid JavaScript',
baseManifest: true,
workflowGuard: stub('workflow'),
head,
})).rejects.toThrow();
}
}, 30000);

it('kills mutations of every important authority and provenance gate', () => {
const mutations = [
REVIEWER.replace('tools:\n bash:', 'tools:\n edit:\n bash:'),
Expand Down
55 changes: 49 additions & 6 deletions workflows/package/squad-review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ safe-outputs:
ref: ${{ github.event.pull_request.base.sha || github.workflow_sha }}
persist-credentials: false
path: .squad-review-base
- name: Checkout workflow commit for first-install review guard
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
with:
ref: ${{ github.workflow_sha }}
persist-credentials: false
path: .squad-review-workflow
- name: Bind review output to committed agent identities and current head
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
env:
Expand All @@ -64,10 +70,25 @@ safe-outputs:
SQUAD_REVIEW_HEAD: ${{ github.event.pull_request.head.sha || github.event.inputs.expected_head_sha }}
with:
script: |
const { existsSync } = require('node:fs');
const { pathToFileURL } = require('node:url');
const guard = await import(pathToFileURL(
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`
).href);
const baseGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`;
const baseManifest =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/aw/squad-workflows.manifest.json`;
const workflowGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-workflow/.github/workflows/shared/squad-review-guard.mjs`;
let guardPath;
if (existsSync(baseGuard)) {
guardPath = baseGuard;
} else if (!existsSync(baseManifest)) {
guardPath = workflowGuard;
} else {
throw new Error(
'Squad Review guard is missing from an established base installation. Refusing workflow-source fallback.'
);
}
const guard = await import(pathToFileURL(guardPath).href);
await guard.enforceReviewOutputs(process.env,
async (route, fields) => (await github.request(`GET /${route}`, fields)).data);
add-comment:
Expand Down Expand Up @@ -114,6 +135,13 @@ jobs:
ref: ${{ github.event.pull_request.base.sha }}
persist-credentials: false
path: .squad-review-base
- name: Checkout workflow commit for first-install final verdict gate
if: github.event_name == 'pull_request'
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
with:
ref: ${{ github.workflow_sha }}
persist-credentials: false
path: .squad-review-workflow
- name: Enforce independent current-head verdict
if: github.event_name == 'pull_request'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
Expand All @@ -122,10 +150,25 @@ jobs:
SQUAD_REVIEW_HEAD: ${{ github.event.pull_request.head.sha }}
with:
script: |
const { existsSync } = require('node:fs');
const { pathToFileURL } = require('node:url');
const guard = await import(pathToFileURL(
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`
).href);
const baseGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`;
const baseManifest =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/aw/squad-workflows.manifest.json`;
const workflowGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-workflow/.github/workflows/shared/squad-review-guard.mjs`;
let guardPath;
if (existsSync(baseGuard)) {
guardPath = baseGuard;
} else if (!existsSync(baseManifest)) {
guardPath = workflowGuard;
} else {
throw new Error(
'Squad Review guard is missing from an established base installation. Refusing workflow-source fallback.'
);
}
const guard = await import(pathToFileURL(guardPath).href);
await guard.assertClearingReview(process.env,
async (route, fields) => (await github.request(`GET /${route}`, fields)).data);
---
Expand Down
55 changes: 49 additions & 6 deletions workflows/squad-review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ safe-outputs:
ref: ${{ github.event.pull_request.base.sha || github.workflow_sha }}
persist-credentials: false
path: .squad-review-base
- name: Checkout workflow commit for first-install review guard
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.workflow_sha }}
persist-credentials: false
path: .squad-review-workflow
- name: Bind review output to committed agent identities and current head
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
Expand All @@ -62,10 +68,25 @@ safe-outputs:
SQUAD_REVIEW_HEAD: ${{ github.event.pull_request.head.sha || github.event.inputs.expected_head_sha }}
with:
script: |
const { existsSync } = require('node:fs');
const { pathToFileURL } = require('node:url');
const guard = await import(pathToFileURL(
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`
).href);
const baseGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`;
const baseManifest =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/aw/squad-workflows.manifest.json`;
const workflowGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-workflow/.github/workflows/shared/squad-review-guard.mjs`;
let guardPath;
if (existsSync(baseGuard)) {
guardPath = baseGuard;
} else if (!existsSync(baseManifest)) {
guardPath = workflowGuard;
} else {
throw new Error(
'Squad Review guard is missing from an established base installation. Refusing workflow-source fallback.'
);
}
const guard = await import(pathToFileURL(guardPath).href);
await guard.enforceReviewOutputs(process.env,
async (route, fields) => (await github.request(`GET /${route}`, fields)).data);
add-comment:
Expand Down Expand Up @@ -109,6 +130,13 @@ jobs:
ref: ${{ github.event.pull_request.base.sha }}
persist-credentials: false
path: .squad-review-base
- name: Checkout workflow commit for first-install final verdict gate
if: github.event_name == 'pull_request'
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.workflow_sha }}
persist-credentials: false
path: .squad-review-workflow
- name: Enforce independent current-head verdict
if: github.event_name == 'pull_request'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
Expand All @@ -117,10 +145,25 @@ jobs:
SQUAD_REVIEW_HEAD: ${{ github.event.pull_request.head.sha }}
with:
script: |
const { existsSync } = require('node:fs');
const { pathToFileURL } = require('node:url');
const guard = await import(pathToFileURL(
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`
).href);
const baseGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/workflows/shared/squad-review-guard.mjs`;
const baseManifest =
`${process.env.GITHUB_WORKSPACE}/.squad-review-base/.github/aw/squad-workflows.manifest.json`;
const workflowGuard =
`${process.env.GITHUB_WORKSPACE}/.squad-review-workflow/.github/workflows/shared/squad-review-guard.mjs`;
let guardPath;
if (existsSync(baseGuard)) {
guardPath = baseGuard;
} else if (!existsSync(baseManifest)) {
guardPath = workflowGuard;
} else {
throw new Error(
'Squad Review guard is missing from an established base installation. Refusing workflow-source fallback.'
);
}
const guard = await import(pathToFileURL(guardPath).href);
await guard.assertClearingReview(process.env,
async (route, fields) => (await github.request(`GET /${route}`, fields)).data);
---
Expand Down
2 changes: 1 addition & 1 deletion workflows/squad-workflows.manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"source": "workflows/package/squad-review.md",
"destination": ".github/workflows/squad-review.md",
"lock": ".github/workflows/squad-review.lock.yml",
"source_sha256": "d2adc1235555f96bce6abedf9e7ef665de1f33704da685f51b6abe45d62abbd2"
"source_sha256": "cae3f9ca8a32ca76cfd873941fafe38319fa53b35fb3478c2338732b8b9f63fd"
},
{
"name": "squad-deps-worker",
Expand Down
Loading