From 251e256ff6e9a6e94befb3cbcaaafc2c91ca47c6 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Mon, 28 Sep 2026 12:33:16 -0700 Subject: [PATCH 1/2] ci: add repo-health PR-head shadow harness Add a read-only pull_request shadow that exercises proposed scanners and the real reporter contract without mutating GitHub. Document the authoritative post-merge hostile-filename canary and label production comments by evidence level. Closes #1800 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/REPO_HEALTH_RUNBOOK.md | 96 +++++++++++++ .../workflows/squad-repo-health-shadow.yml | 73 ++++++++++ scripts/repo-health-comment.mjs | 6 + .../repo-health-comment-transport.test.ts | 134 +++++++++++++++++- 4 files changed, 302 insertions(+), 7 deletions(-) create mode 100644 .github/REPO_HEALTH_RUNBOOK.md create mode 100644 .github/workflows/squad-repo-health-shadow.yml diff --git a/.github/REPO_HEALTH_RUNBOOK.md b/.github/REPO_HEALTH_RUNBOOK.md new file mode 100644 index 000000000..917c2b7a8 --- /dev/null +++ b/.github/REPO_HEALTH_RUNBOOK.md @@ -0,0 +1,96 @@ +# Repo Health workflow verification + +`Repo Health` is intentionally base-controlled. Its `pull_request_target` trigger supplies the +workflow definition from the target branch, and every checkout keeps the trusted base revision. +The PR head is fetched only as Git data. Never check out or execute PR-head code in that workflow. + +## Evidence levels + +- **Shadow / non-authoritative:** `Repo Health Shadow (Non-Authoritative)` runs proposed scanners, + reporter script bodies, and the reporter helper from the PR head under `pull_request`, a + read-only token, no persisted checkout credentials, and fake Octokit. It can prove that proposed + code compiles, preserves hostile text, and propagates API failures, but it cannot prove the + elevated production workflow has picked up the merged implementation. +- **Post-merge / authoritative:** only a disposable PR opened after the change reaches `dev` and + observed through `Repo Health` proves the base-controlled writer now creates and cleans up its + marker comment. + +Do not weaken a scanner, add an exclusion, or change a finding merely to make the base-controlled +check on the implementation PR green. + +## Authoritative post-merge canary + +Run this only after the workflow/helper change is on `dev`. It creates a disposable branch and PR +whose filename contains paired backticks, a bare `${`, an apostrophe, a double quote, and a +newline. The filename is data; do not execute it. + +```bash +git fetch origin dev +git switch --create canary/repo-health-$(date +%s) origin/dev + +node --input-type=module <<'NODE' +import { mkdirSync, writeFileSync } from 'node:fs'; +const path = '.squad/canary-`paired`-${-apostrophe-\'-"double quote"\nsecond-line.md'; +mkdirSync('.squad', { recursive: true }); +writeFileSync(path, 'repo-health post-merge canary\n'); +NODE + +CANARY_PATH=$(node --input-type=module -e \ + 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\"\nsecond-line.md")') +git add -- "$CANARY_PATH" +git commit -m 'test: repo-health post-merge canary' +git push -u origin HEAD +PR_URL=$(gh pr create --base dev --title 'test: repo-health post-merge canary' \ + --body 'Disposable authoritative canary; do not merge.') +PR_NUMBER=${PR_URL##*/} +``` + +Wait for `Repo Health / Squad File Leakage`, then verify exactly one authoritative marker and the +exact hostile filename: + +```bash +EXPECTED=$(node --input-type=module -e \ + 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\"\nsecond-line.md")') + +gh api --paginate --slurp "repos/bradygaster/squad/issues/$PR_NUMBER/comments" \ + > canary-comments.json +node --input-type=module - "$EXPECTED" <<'NODE' +import { readFileSync } from 'node:fs'; +const expected = process.argv[2]; +const comments = JSON.parse(readFileSync('canary-comments.json', 'utf8')).flat(); +const marked = comments.filter(({ body = '' }) => + body.includes('')); +if (marked.length !== 1) throw new Error(`expected one marker comment, found ${marked.length}`); +if (!marked[0].body.includes(expected)) throw new Error('hostile filename was not preserved exactly'); +if (!marked[0].body.includes('Authoritative evidence')) { + throw new Error('comment was not labeled authoritative'); +} +NODE +rm canary-comments.json +``` + +Delete the file in the same PR, push, wait for the rerun, and prove stale cleanup: + +```bash +git rm -- "$CANARY_PATH" +git commit -m 'test: clean up repo-health post-merge canary' +git push + +# After Repo Health / Squad File Leakage completes: +gh api --paginate --slurp "repos/bradygaster/squad/issues/$PR_NUMBER/comments" \ + > canary-comments.json +node --input-type=module <<'NODE' +import { readFileSync } from 'node:fs'; +const comments = JSON.parse(readFileSync('canary-comments.json', 'utf8')).flat(); +const marked = comments.filter(({ body = '' }) => + body.includes('')); +if (marked.length !== 0) throw new Error(`stale marker remains: ${marked.length}`); +NODE +rm canary-comments.json +gh pr close "$PR_NUMBER" --delete-branch +git switch dev +``` + +Record the PR URL and both successful `Repo Health` run URLs in the implementing PR or issue. +This post-merge canary is the only authoritative evidence for a repo-health workflow/reporter +change; the pre-merge shadow remains intentionally non-authoritative. diff --git a/.github/workflows/squad-repo-health-shadow.yml b/.github/workflows/squad-repo-health-shadow.yml new file mode 100644 index 000000000..d31dc17e1 --- /dev/null +++ b/.github/workflows/squad-repo-health-shadow.yml @@ -0,0 +1,73 @@ +name: Repo Health Shadow (Non-Authoritative) + +on: + pull_request: + branches: [dev] + types: [opened, synchronize, reopened] + paths: + - '.github/workflows/squad-repo-health.yml' + - '.github/workflows/squad-repo-health-shadow.yml' + - 'scripts/architectural-review.mjs' + - 'scripts/check-squad-leakage.mjs' + - 'scripts/repo-health-comment.mjs' + - 'scripts/security-review.mjs' + - 'test/scripts/repo-health-comment-transport.test.ts' + +# This workflow intentionally runs the PR-head implementation with a read-only token. +# Its scanner output and fake-Octokit reporter exercise are shadow evidence only. +# The pull_request_target workflow remains the authoritative, base-controlled writer. +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + shadow: + name: PR-head scanner/reporter shadow (non-authoritative) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #v7 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 #v6 + with: + node-version: '22' + cache: npm + + - name: Run PR-head scanners as shadow evidence + shell: bash + run: | + set -euo pipefail + git fetch --no-tags origin "${{ github.event.pull_request.base.sha }}" + { + echo '# Repo Health PR-head Shadow' + echo + echo '> **NON-AUTHORITATIVE:** These scanners came from the PR head and have no write token.' + echo '> Only a post-merge canary through `Repo Health` is authoritative for workflow/reporter changes.' + echo + echo '## Squad leakage' + echo '```text' + node scripts/check-squad-leakage.mjs "${{ github.event.pull_request.base.sha }}" HEAD + echo '```' + echo + echo '## Architectural review' + echo '```text' + node scripts/architectural-review.mjs "${{ github.event.pull_request.base.sha }}" HEAD + echo '```' + echo + echo '## Security review' + echo '```text' + node scripts/security-review.mjs "${{ github.event.pull_request.base.sha }}" HEAD + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Install test dependencies without lifecycle scripts + run: npm ci --ignore-scripts + + - name: Exercise real reporter scripts with fake Octokit + run: npx --no-install vitest run test/scripts/repo-health-comment-transport.test.ts diff --git a/scripts/repo-health-comment.mjs b/scripts/repo-health-comment.mjs index 4ebe1570a..423ab5e52 100644 --- a/scripts/repo-health-comment.mjs +++ b/scripts/repo-health-comment.mjs @@ -19,6 +19,8 @@ const JOBS = { return [ '## ⚠️ Squad File Leakage Detected', '', + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + '', 'The following `.squad/` files were modified in this PR:', '', fileList, @@ -51,6 +53,8 @@ const JOBS = { return [ '## 🏗️ Architectural Review', '', + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + '', parsed.summary, '', '| Severity | Category | Finding | Files |', @@ -83,6 +87,8 @@ const JOBS = { return [ '## 🔒 Security Review', '', + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + '', parsed.summary, '', '| Severity | Category | Finding | Location |', diff --git a/test/scripts/repo-health-comment-transport.test.ts b/test/scripts/repo-health-comment-transport.test.ts index a30958f87..c11718324 100644 --- a/test/scripts/repo-health-comment-transport.test.ts +++ b/test/scripts/repo-health-comment-transport.test.ts @@ -104,25 +104,43 @@ function substitute(text: string, values: Record): string { }); } -function makeOctokit() { +type ApiMethod = 'createComment' | 'updateComment' | 'deleteComment' | 'listComments'; + +function makeOctokit(options: { + comments?: Array<{ id: number; body: string }>; + fail?: ApiMethod; +} = {}) { const created: Array<{ body: string }> = []; const updated: Array<{ body: string }> = []; + const deleted: Array<{ comment_id: number }> = []; + const failure = new Error(`mocked ${options.fail ?? 'API'} failure`); + const rejectIf = (method: ApiMethod) => { + if (options.fail === method) throw failure; + }; const github = { - paginate: async () => [] as Array<{ id: number; body: string }>, + paginate: async () => { + rejectIf('listComments'); + return options.comments ?? []; + }, rest: { issues: { listComments: () => undefined, createComment: async (params: { body: string }) => { + rejectIf('createComment'); created.push(params); }, updateComment: async (params: { body: string }) => { + rejectIf('updateComment'); updated.push(params); }, - deleteComment: async () => undefined, + deleteComment: async (params: { comment_id: number }) => { + rejectIf('deleteComment'); + deleted.push(params); + }, }, }, }; - return { github, created, updated }; + return { github, created, updated, deleted, failure }; } const context = { repo: { owner: 'bradygaster', repo: 'squad' }, issue: { number: 1770 } }; @@ -131,9 +149,14 @@ const context = { repo: { owner: 'bradygaster', repo: 'squad' }, issue: { number * Execute a workflow script body exactly the way `actions/github-script` does: * compile the resolved text as an async function and invoke it. */ -async function runWorkflowScript(step: Step, values: Record) { +async function runWorkflowScript( + step: Step, + values: Record, + octokitOptions: Parameters[0] = {}, +) { const script = stepScript(step); expect(script, `step "${step.name}" has no script block`).toBeTruthy(); + expect(script).not.toContain('${{'); const env = stepEnv(step); const previous: Record = {}; @@ -145,7 +168,7 @@ async function runWorkflowScript(step: Step, values: Record) { process.env[key] = value; } - const octokit = makeOctokit(); + const octokit = makeOctokit(octokitOptions); try { // `actions/github-script` compiles the body as an async function body. We do the // same, via `vm.compileFunction` so that the `await import(...)` in the body can @@ -181,7 +204,8 @@ const HOSTILE_MESSAGE = 'Unsafe git operation: `git cherry-pick --no-commit` — quoting `code` here, ' + 'plus a bare ${ opener, an apostrophe \' and a "double quote".\nSecond line of the message.'; -const HOSTILE_PATH = 'docs/using-`git cherry-pick`-and-${-in-a-name.md'; +const HOSTILE_PATH = + '.squad/using-`git cherry-pick`-and-${-an-apostrophe-\'-and-"double quote"\nsecond-line.md'; function securityReport() { const payload = { @@ -239,6 +263,9 @@ describe('repo-health reporters survive hostile finding content (#1770)', () => expect(created).toHaveLength(1); expect(created[0].body).toContain(''); expect(created[0].body).toContain(HOSTILE_MESSAGE); + expect(created[0].body).toContain( + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + ); }); it('reports an architectural finding whose message contains backticks', async () => { @@ -254,6 +281,9 @@ describe('repo-health reporters survive hostile finding content (#1770)', () => expect(created).toHaveLength(1); expect(created[0].body).toContain(''); expect(created[0].body).toContain(HOSTILE_MESSAGE); + expect(created[0].body).toContain( + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + ); }); it('reports leaked squad files whose paths contain backticks', async () => { @@ -269,9 +299,64 @@ describe('repo-health reporters survive hostile finding content (#1770)', () => expect(created).toHaveLength(1); expect(created[0].body).toContain(''); expect(created[0].body).toContain(HOSTILE_PATH); + expect(created[0].body).toContain( + '> **Authoritative evidence:** base-controlled `pull_request_target` reporter.', + ); }); }); +describe('repo-health comment lifecycle uses the real workflow/helper contract', () => { + const step = findStep( + 'squad-repo-health.yml', + (candidate) => (stepScript(candidate) ?? '').includes("job: 'security'"), + ); + const marker = ''; + + it('updates the one existing marker comment and preserves hostile multiline text exactly', async () => { + const { created, updated, deleted } = await runWorkflowScript( + step, + { 'steps.security.outputs.result': securityReport() }, + { comments: [{ id: 42, body: `${marker}\nold` }] }, + ); + + expect(created).toEqual([]); + expect(deleted).toEqual([]); + expect(updated).toHaveLength(1); + expect(updated[0].body.match(//g)).toHaveLength(1); + expect(updated[0].body).toContain(HOSTILE_MESSAGE); + }); + + it('deletes a stale marker comment when the scanner is clean', async () => { + const { created, updated, deleted } = await runWorkflowScript( + step, + { 'steps.security.outputs.result': JSON.stringify({ findings: [], summary: 'clean' }) }, + { comments: [{ id: 42, body: `${marker}\nold` }] }, + ); + + expect(created).toEqual([]); + expect(updated).toEqual([]); + expect(deleted).toEqual([{ owner: 'bradygaster', repo: 'squad', comment_id: 42 }]); + }); + + for (const method of ['listComments', 'createComment', 'updateComment', 'deleteComment'] as const) { + it(`propagates mocked ${method} failures so the github-script step is red`, async () => { + const clean = method === 'deleteComment'; + const existing = method === 'updateComment' || method === 'deleteComment' + ? [{ id: 42, body: `${marker}\nold` }] + : []; + const output = clean + ? JSON.stringify({ findings: [], summary: 'clean' }) + : securityReport(); + + await expect(runWorkflowScript( + step, + { 'steps.security.outputs.result': output }, + { comments: existing, fail: method }, + )).rejects.toThrow(`mocked ${method} failure`); + }); + } +}); + describe('hand-written github-script bodies never embed workflow expressions', () => { // Structural guard: `${{ }}` inside a script body means untrusted-shaped data can // become part of the parsed script. Pass it through `env:` instead. @@ -285,3 +370,38 @@ describe('hand-written github-script bodies never embed workflow expressions', ( }); } }); + +describe('repo-health workflow trust boundary (#1800)', () => { + const production = readWorkflow('squad-repo-health.yml'); + const shadow = readWorkflow('squad-repo-health-shadow.yml'); + + it('keeps the authoritative writer base-controlled and least-privileged', () => { + expect(production).toMatch(/^on:\n pull_request_target:/m); + expect(production).toMatch( + /permissions:\n contents: read\n pull-requests: write\n issues: write/, + ); + expect(production).not.toMatch(/ref:\s*\$\{\{\s*github\.event\.pull_request\.head\.(?:ref|sha)/); + expect(production).not.toMatch(/git (?:checkout|switch|reset)[^\n]*pull_request\.head/); + expect(production).toContain('Fetch PR head (data only — not executed)'); + const headDataLines = production + .split(/\r?\n/) + .filter((line) => line.includes('github.event.pull_request.head.sha')); + expect(headDataLines.length).toBeGreaterThan(0); + expect(headDataLines.every((line) => + /git fetch origin|git diff|git rev-list|node scripts\//.test(line), + )).toBe(true); + expect(production).toContain( + 'import(`${process.env.GITHUB_WORKSPACE}/scripts/repo-health-comment.mjs`)', + ); + }); + + it('runs PR-head proposals only in an explicitly non-authoritative read-only workflow', () => { + expect(shadow).toMatch(/^name: Repo Health Shadow \(Non-Authoritative\)$/m); + expect(shadow).toMatch(/^on:\n pull_request:/m); + expect(shadow).toMatch(/permissions:\n contents: read\n/); + expect(shadow).not.toMatch(/^\s+pull-requests: write|^\s+issues: write|^\s+pull_request_target:/m); + expect(shadow).toContain('persist-credentials: false'); + expect(shadow).toContain('fake Octokit'); + expect(shadow).toContain('NON-AUTHORITATIVE'); + }); +}); From 25c5d7fa2de74e9ab686b50c63d66c30b5e9091d Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Mon, 28 Sep 2026 12:37:15 -0700 Subject: [PATCH 2/2] fix: align repo-health fixtures with git transport Closes #1800 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/REPO_HEALTH_RUNBOOK.md | 11 ++++++----- .github/workflows/squad-repo-health-shadow.yml | 2 +- test/scripts/repo-health-comment-transport.test.ts | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/REPO_HEALTH_RUNBOOK.md b/.github/REPO_HEALTH_RUNBOOK.md index 917c2b7a8..09532a049 100644 --- a/.github/REPO_HEALTH_RUNBOOK.md +++ b/.github/REPO_HEALTH_RUNBOOK.md @@ -21,8 +21,9 @@ check on the implementation PR green. ## Authoritative post-merge canary Run this only after the workflow/helper change is on `dev`. It creates a disposable branch and PR -whose filename contains paired backticks, a bare `${`, an apostrophe, a double quote, and a -newline. The filename is data; do not execute it. +whose filename contains paired backticks, a bare `${`, an apostrophe, and a double quote. The +filename is data; do not execute it. Multiline transport coverage belongs in scanner diagnostic +messages, because the leakage scanner consumes Git's line-delimited `--name-only` output. ```bash git fetch origin dev @@ -30,13 +31,13 @@ git switch --create canary/repo-health-$(date +%s) origin/dev node --input-type=module <<'NODE' import { mkdirSync, writeFileSync } from 'node:fs'; -const path = '.squad/canary-`paired`-${-apostrophe-\'-"double quote"\nsecond-line.md'; +const path = '.squad/canary-`paired`-${-apostrophe-\'-"double quote".md'; mkdirSync('.squad', { recursive: true }); writeFileSync(path, 'repo-health post-merge canary\n'); NODE CANARY_PATH=$(node --input-type=module -e \ - 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\"\nsecond-line.md")') + 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\".md")') git add -- "$CANARY_PATH" git commit -m 'test: repo-health post-merge canary' git push -u origin HEAD @@ -50,7 +51,7 @@ exact hostile filename: ```bash EXPECTED=$(node --input-type=module -e \ - 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\"\nsecond-line.md")') + 'process.stdout.write(".squad/canary-`paired`-${-apostrophe-'\''-\"double quote\".md")') gh api --paginate --slurp "repos/bradygaster/squad/issues/$PR_NUMBER/comments" \ > canary-comments.json diff --git a/.github/workflows/squad-repo-health-shadow.yml b/.github/workflows/squad-repo-health-shadow.yml index d31dc17e1..a9568d6ef 100644 --- a/.github/workflows/squad-repo-health-shadow.yml +++ b/.github/workflows/squad-repo-health-shadow.yml @@ -48,7 +48,7 @@ jobs: echo '# Repo Health PR-head Shadow' echo echo '> **NON-AUTHORITATIVE:** These scanners came from the PR head and have no write token.' - echo '> Only a post-merge canary through `Repo Health` is authoritative for workflow/reporter changes.' + printf '%s\n' "> Only a post-merge canary through \`Repo Health\` is authoritative for workflow/reporter changes." echo echo '## Squad leakage' echo '```text' diff --git a/test/scripts/repo-health-comment-transport.test.ts b/test/scripts/repo-health-comment-transport.test.ts index c11718324..5e3a8da72 100644 --- a/test/scripts/repo-health-comment-transport.test.ts +++ b/test/scripts/repo-health-comment-transport.test.ts @@ -205,7 +205,7 @@ const HOSTILE_MESSAGE = 'plus a bare ${ opener, an apostrophe \' and a "double quote".\nSecond line of the message.'; const HOSTILE_PATH = - '.squad/using-`git cherry-pick`-and-${-an-apostrophe-\'-and-"double quote"\nsecond-line.md'; + '.squad/using-`git cherry-pick`-and-${-an-apostrophe-\'-and-"double quote".md'; function securityReport() { const payload = {