From ba561674ea8584be256eded57ccea56fdb1acd6f Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Thu, 1 Oct 2026 12:49:39 -0700 Subject: [PATCH 1/4] fix(gh-aw): relay normalized squad commands through dispatch Fixes three command-routing defects reported against source SHA 261d140679b158888f1a3c4d9a5a3e74eb55b633 (octodemo/zava-social-backend-20261001184422#1): 1. The lifecycle-repair fallback in the shared squad.md coordinator only recognized exact, /squad-prefixed issue-comment bodies. A command normalized and relayed by squad-command-router.md via workflow_dispatch (e.g. the bare "activate" dispatch input for a /squad activate comment) skipped repair entirely. Repair gating and the effective command are now derived identically for both the issue_comment and workflow_dispatch event paths (CANONICAL_BY_BARE_COMMAND normalization). 2. squad-command-router.md omitted `roles: all`, so gh-aw's default write-role activation gate blocked the router from running at all for non-write actors -- including modes the router intentionally allows anyone to invoke. Added `roles: all`; the router's own in-script, per-command authorization for mutating modes is unchanged. 3. The router's relayed dispatch dropped originating comment provenance, so an embedded /squad approve-improvement could never supply a real approval_comment_id and was rejected outright. The router now forwards an aw_context pointer (item type/number + originating comment id) through the dispatch, and squad-improvement-gate.mjs's validateApprovalDispatchRelay re-fetches and validates that live comment, binding permission to its own author -- never the dispatch actor -- so approval binding is not weakened. Package/lock/manifest regenerated via the standard `node workflows/shared/squad-install-verifier.mjs --write-source` pipeline; strict `gh aw compile --strict` passes with only the two documented warnings. Added test/gh-aw-command-router-relay.test.ts (true end-to-end router -> repair-job relay via real extracted step scripts, plus a normalization mutation-kill test), and extended test/gh-aw-command-parse.test.ts / test/gh-aw-improvement-worker.test.ts with regression coverage for all three findings. Local npm install is broken in this environment (missing transitive deps such as fdir), so vitest/eslint cannot run locally; every new/changed assertion was independently verified by direct Node/vm execution of the real production modules and scripts, and by compiled-lock diffing. Hosted CI at the PR head is the authoritative suite result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/fix-squad-command-relay-defects.md | 10 + test/gh-aw-command-parse.test.ts | 10 + test/gh-aw-command-router-relay.test.ts | 257 ++++++++++++++++++ test/gh-aw-improvement-worker.test.ts | 55 ++++ workflows/package/squad-command-router.md | 15 + workflows/package/squad-deps-worker.md | 84 ++++-- workflows/package/squad-implement-worker.md | 84 ++++-- workflows/package/squad-retro.md | 84 ++++-- workflows/package/squad.md | 97 +++++-- workflows/shared/squad-improvement-gate.mjs | 55 +++- workflows/shared/squad.md | 85 ++++-- workflows/squad-command-router.md | 15 + workflows/squad-workflows.manifest.json | 34 +-- workflows/squad.md | 13 +- 14 files changed, 746 insertions(+), 152 deletions(-) create mode 100644 .changeset/fix-squad-command-relay-defects.md create mode 100644 test/gh-aw-command-router-relay.test.ts diff --git a/.changeset/fix-squad-command-relay-defects.md b/.changeset/fix-squad-command-relay-defects.md new file mode 100644 index 000000000..94a649bf5 --- /dev/null +++ b/.changeset/fix-squad-command-relay-defects.md @@ -0,0 +1,10 @@ +--- +"@bradygaster/squad-cli": patch +"@bradygaster/squad-sdk": patch +--- + +Fixed three command-routing defects where a `/squad` command relayed by `squad-command-router.md` via `workflow_dispatch` (instead of arriving as a direct issue comment) could be silently dropped or mis-authorized: + +- The lifecycle-repair fallback in the shared `squad.md` coordinator only recognized exact, `/squad`-prefixed issue-comment bodies. A command normalized and relayed by the router (e.g. the bare `activate` dispatch input for a `/squad activate` comment) no longer matched, so terminal activation states went unrepaired. Repair gating and the effective command are now derived the same way for both the `issue_comment` and `workflow_dispatch` event paths. +- `squad-command-router.md` was missing `roles: all`, so gh-aw's default write-role activation gate blocked the router from even running for non-write actors — including modes the router intentionally allows anyone to invoke (e.g. `status`). The router now declares `roles: all` and continues to enforce its own in-script, per-command authorization for mutating modes. +- The router's relayed dispatch dropped the originating issue/comment context, so an embedded `/squad approve-improvement` command could never supply a real `approval_comment_id` and was rejected outright. The router now forwards an `aw_context` pointer (item type/number and originating comment id) through the dispatch, and the improvement-approval gate validates permission against that comment's own live author — never the dispatch actor — so approval binding is unchanged, not weakened. diff --git a/test/gh-aw-command-parse.test.ts b/test/gh-aw-command-parse.test.ts index 2b5161652..5c9b7653a 100644 --- a/test/gh-aw-command-parse.test.ts +++ b/test/gh-aw-command-parse.test.ts @@ -203,6 +203,16 @@ describe('gh-aw: shared /squad command contract (#1824)', () => { expect(WORKFLOW).not.toContain('### Step PC-1: Extract the command argument'); }); + it('declares roles: all so write-role gating does not block modes open to everyone (#2)', () => { + expect(DISCOVERY_WORKFLOW).toMatch(/^on:\n\s+roles:\s*all\n/m); + }); + + it('forwards originating comment provenance via aw_context on the relayed dispatch (#3)', () => { + expect(DISCOVERY_WORKFLOW).toContain('comment_id: context.payload.comment?.id ?? null'); + expect(DISCOVERY_WORKFLOW).toContain('aw_context: awContext'); + expect(DISCOVERY_WORKFLOW).toContain("inputs: {"); + }); + it('routes commands outside gh-aw start-only activation through the same contract', () => { expect(DISCOVERY_WORKFLOW).toContain("contains(github.event.issue.body, '/squad')"); expect(DISCOVERY_WORKFLOW).toContain("contains(github.event.comment.body, '/squad')"); diff --git a/test/gh-aw-command-router-relay.test.ts b/test/gh-aw-command-router-relay.test.ts new file mode 100644 index 000000000..7324664a8 --- /dev/null +++ b/test/gh-aw-command-router-relay.test.ts @@ -0,0 +1,257 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { createRequire } from 'node:module'; +import { compileFunction, constants as vmConstants } from 'node:vm'; +import { + classifySquadCommand, + commandRequiresAuthorization, + isAuthorizedPermission, +} from '../workflows/shared/squad-command-contract.mjs'; + +const nodeRequire = createRequire(import.meta.url); + +// Regression coverage for the three command-routing defects reported against +// bradygaster/squad source SHA 261d140679b158888f1a3c4d9a5a3e74eb55b633 +// (octodemo/zava-social-backend-20261001184422#1): +// +// #1 lifecycle-repair fallback only recognized exact issue-comment bodies, +// so a normalized command relayed via workflow_dispatch skipped repair; +// #2 the router omitted `roles: all`, so gh-aw's default write-role +// activation gate blocked non-write actors from modes meant to be open +// to all (open modes still require no mutating permission once past +// that gate -- the router's own in-script authorization is unchanged); +// #3 the router dropped originating comment provenance, so a relayed +// `/squad approve-improvement` could never supply a real +// `approval_comment_id`. +// +// These tests execute the real extracted step scripts the same way +// `actions/github-script` does (compile the body as an async function and +// invoke it -- see test/scripts/repo-health-comment-transport.test.ts for the +// established pattern in this repo), so they exercise production source, not +// a reimplementation of it. + +const ROOT = process.cwd(); +const ROUTER = readFileSync(join(ROOT, 'workflows/squad-command-router.md'), 'utf8'); +const SHARED_SQUAD = readFileSync(join(ROOT, 'workflows/shared/squad.md'), 'utf8'); + +/** Read a `script: |` literal block starting at `marker`, dedented. */ +function extractScript(source: string, marker: string): string { + const markerIndex = source.indexOf(marker); + expect(markerIndex, `marker not found: ${marker}`).toBeGreaterThanOrEqual(0); + const scriptIndex = source.indexOf('script: |', markerIndex); + expect(scriptIndex, `no script block after marker: ${marker}`).toBeGreaterThanOrEqual(0); + const lineStart = source.lastIndexOf('\n', scriptIndex) + 1; + const indent = scriptIndex - lineStart; + const lines: string[] = []; + let cursor = source.indexOf('\n', scriptIndex) + 1; + while (cursor > 0 && cursor < source.length) { + const nextNewline = source.indexOf('\n', cursor); + const line = nextNewline >= 0 ? source.slice(cursor, nextNewline) : source.slice(cursor); + if (line.trim() !== '') { + const lineIndent = line.match(/^\s*/)![0].length; + if (lineIndent <= indent) break; + lines.push(line.slice(indent + 2)); + } else { + lines.push(''); + } + if (nextNewline < 0) break; + cursor = nextNewline + 1; + } + return lines.join('\n'); +} + +function compileScript(script: string) { + return compileFunction( + `return (async () => {\n${script}\n})();`, + ['github', 'context', 'core', 'process', 'require'], + { importModuleDynamically: vmConstants.USE_MAIN_CONTEXT_DEFAULT_LOADER }, + ) as (...args: unknown[]) => Promise; +} + +describe('gh-aw: router-to-repair workflow_dispatch relay (#2, #3 findings)', () => { + it('declares roles: all so the activation gate does not block non-write actors (#2)', () => { + expect(ROUTER).toMatch(/^on:\n\s+roles:\s*all\n/m); + }); + + it('preserves the router-level mutating-mode authorization that remains the real gate', () => { + // roles: all only removes gh-aw's blanket write-role activation gate; the + // router's own in-script permission check for mutating modes must be the + // thing still enforcing authorization afterward. + expect(ROUTER).toContain('contract.commandRequiresAuthorization(result)'); + expect(ROUTER).toContain('contract.isAuthorizedPermission(permission)'); + expect(ROUTER).toContain('getCollaboratorPermissionLevel'); + expect(commandRequiresAuthorization({ status: 'accepted', mode: 'cast' })).toBe(true); + expect(commandRequiresAuthorization({ status: 'accepted', mode: 'status' })).toBe(false); + expect(isAuthorizedPermission('read')).toBe(false); + expect(isAuthorizedPermission('write')).toBe(true); + }); + + it('forwards an aw_context comment pointer alongside the bare relayed command (#3)', () => { + expect(ROUTER).toContain("comment_id: context.payload.comment?.id ?? null"); + expect(ROUTER).toContain('aw_context: awContext'); + expect(ROUTER).toContain("command: result.argumentText || 'cast'"); + }); + + it( + 'relays a normalized issue-comment activation command through the router script to the ' + + 'exact dispatch inputs it emits, end to end', + async () => { + const routerScript = extractScript(ROUTER, 'Route or reject discovered command'); + expect(routerScript).not.toContain('${{'); + + // An ordinary issue comment -- not a synthetic bare workflow_dispatch + // input -- posted by an authorized (write-permission) human. + const commentBody = '/squad activate'; + const context = { + repo: { owner: 'bradygaster', repo: 'squad' }, + actor: 'maintainer', + payload: { + issue: { number: 4242 }, + comment: { id: 55001, body: commentBody }, + repository: { default_branch: 'dev' }, + }, + }; + + // classifySquadCommand must accept this as the 'activate' mode with a + // bare argumentText -- this bare form is exactly what the pre-fix + // repair job's exact-body-only `includes(command)` check could never + // match once relayed through workflow_dispatch. + const classified = classifySquadCommand({ comment: context.payload.comment, issue: context.payload.issue }, 'issue_comment'); + expect(classified).toMatchObject({ status: 'accepted', mode: 'activate', argumentText: 'activate' }); + + let dispatchedInputs: Record | null = null; + const github = { + rest: { + issues: { createComment: async () => undefined }, + repos: { getCollaboratorPermissionLevel: async () => ({ data: { permission: 'write' } }) }, + actions: { + createWorkflowDispatch: async (params: { inputs: Record }) => { + dispatchedInputs = params.inputs; + }, + }, + }, + }; + const core = { setFailed: (msg: string) => { throw new Error(`core.setFailed: ${msg}`); }, info: () => undefined, warning: () => undefined }; + + // The real router script resolves the command contract off a checked-out + // trusted-base clone (`/.squad-command-trusted-base/.github/...`); + // point that one import *target path* at this repo's own working copy + // of the real module instead of re-implementing the router's logic. + // Every other line -- including the `require('node:path')` / + // `require('node:url')` preamble -- is the real, unmodified extracted + // script text, executed via the injected real `require`. + const importStatement = + 'const contract = await import(pathToFileURL(nodePath.join(\n' + + ' trustedRoot,\n' + + " '.github/workflows/shared/squad-command-contract.mjs',\n" + + ')).href);'; + expect(routerScript).toContain(importStatement); + const inlined = routerScript.replace( + importStatement, + `const contract = await import(pathToFileURL(nodePath.join(process.cwd(), 'workflows/shared/squad-command-contract.mjs')).href);`, + ); + expect(inlined).not.toBe(routerScript); + + const previousWorkspace = process.env.GITHUB_WORKSPACE; + process.env.GITHUB_WORKSPACE = ROOT; + try { + await compileScript(inlined)(github, context, core, process, nodeRequire); + } finally { + if (previousWorkspace === undefined) delete process.env.GITHUB_WORKSPACE; + else process.env.GITHUB_WORKSPACE = previousWorkspace; + } + + expect(dispatchedInputs).not.toBeNull(); + expect(dispatchedInputs).toMatchObject({ + command: 'activate', + issue_number: '4242', + }); + const awContext = JSON.parse(dispatchedInputs!.aw_context); + expect(awContext).toEqual({ item_type: 'issue', item_number: 4242, comment_id: 55001 }); + + // Now feed the router's exact emitted dispatch inputs into the real + // repair job script as a workflow_dispatch event would, and confirm the + // normalized, relayed command actually reaches and passes the repair + // job's acceptance + authorization gates (not merely that the router + // produced the right-looking payload). + const repairScript = extractScript(SHARED_SQUAD, 'Repair terminal lifecycle after idempotent activation'); + expect(repairScript).not.toContain('${{'); + + const repairContext = { + repo: { owner: 'bradygaster', repo: 'squad' }, + payload: { inputs: dispatchedInputs }, + }; + const paginateCalls: unknown[] = []; + const repairGithub = { + paginate: async (_fn: unknown, params: unknown) => { + paginateCalls.push(params); + return []; + }, + rest: { issues: { listComments: () => undefined, updateComment: async () => undefined, createComment: async () => undefined }, repos: {} }, + }; + const infoMessages: string[] = []; + const repairCore = { + setFailed: (msg: string) => { throw new Error(`core.setFailed: ${msg}`); }, + info: (msg: string) => { infoMessages.push(msg); }, + }; + const previousEnv: Record = {}; + const applied = { + ISSUE_NUMBER: dispatchedInputs!.issue_number, + SQUAD_EVENT_NAME: 'workflow_dispatch', + SQUAD_COMMAND: dispatchedInputs!.command, + }; + for (const [key, value] of Object.entries(applied)) { + previousEnv[key] = process.env[key]; + process.env[key] = value; + } + try { + await compileScript(repairScript)(repairGithub, repairContext, repairCore, process); + } finally { + for (const [key, value] of Object.entries(previousEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + + // It must have reached the trusted-artifact lookup (proving the + // normalized "activate" command and the workflow_dispatch pre-authorized + // branch were both accepted) rather than failing the whole-plan command + // validation the way the pre-fix exact-body-only check would have. + expect(paginateCalls).toHaveLength(1); + expect(infoMessages.some(m => m.includes('authorized via workflow_dispatch'))).toBe(true); + }, + ); + + it('kills the normalization mutation: an un-normalized bare command is rejected again', async () => { + const repairScript = extractScript(SHARED_SQUAD, 'Repair terminal lifecycle after idempotent activation'); + const mutated = repairScript.replace( + 'command = CANONICAL_BY_BARE_COMMAND[bare] || command;', + '// normalization removed by mutation', + ); + expect(mutated).not.toBe(repairScript); + + const context = { repo: { owner: 'bradygaster', repo: 'squad' }, payload: { inputs: { issue_number: '4242', command: 'activate' } } }; + const paginateCalls: unknown[] = []; + const github = { paginate: async (_fn: unknown, params: unknown) => { paginateCalls.push(params); return []; }, rest: { issues: {}, repos: {} } }; + let failedWith: string | null = null; + const core = { setFailed: (msg: string) => { failedWith = msg; }, info: () => undefined }; + const previousEnv: Record = {}; + const applied = { ISSUE_NUMBER: '4242', SQUAD_EVENT_NAME: 'workflow_dispatch', SQUAD_COMMAND: 'activate' }; + for (const [key, value] of Object.entries(applied)) { + previousEnv[key] = process.env[key]; + process.env[key] = value; + } + try { + await compileScript(mutated)(github, context, core, process); + } finally { + for (const [key, value] of Object.entries(previousEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + + expect(failedWith).toContain('A valid whole-plan activation command and issue number are required.'); + expect(paginateCalls).toHaveLength(0); + }); +}); diff --git a/test/gh-aw-improvement-worker.test.ts b/test/gh-aw-improvement-worker.test.ts index 51f88b290..fffb6bad2 100644 --- a/test/gh-aw-improvement-worker.test.ts +++ b/test/gh-aw-improvement-worker.test.ts @@ -519,3 +519,58 @@ describe('improvement: one authorized dispatcher route and installed contract', expect(job).toContain('SQUAD_IMPROVE_APPROVAL_COMMENT_ID'); }, 90000); }); + +// Finding #3 (octodemo/zava-social-backend-20261001184422#1): the router +// relayed `/squad approve-improvement` via workflow_dispatch without any +// originating comment provenance, so `validateImprovementCommand` could never +// supply a real `approval_comment_id` and rejected the command outright. The +// router now forwards an `aw_context` pointer (item type/number + the +// originating comment id) on the dispatch; these cases exercise exactly the +// `event.inputs` shape `squad-command-router.md` now emits for that relay, +// and confirm permission is bound to the comment's own live author -- never +// the relaying dispatch actor -- so approval binding is not weakened. +describe('improvement: workflow_dispatch relay carries real comment provenance (#3)', () => { + const dispatchVariables = { ...env(), GITHUB_EVENT_NAME: 'workflow_dispatch', GITHUB_ACTOR: 'github-actions[bot]' }; + const relayEvent = (overrides = {}) => ({ + inputs: { + command: 'approve-improvement', + issue_number: '77', + aw_context: JSON.stringify({ item_type: 'issue', item_number: 77, comment_id: 901 }), + ...overrides, + }, + }); + it('accepts the exact dispatch shape the router emits, checking permission against the comment author', async () => { + const calls: string[] = []; + const fetchJson = async (route: string, fields: any) => { + calls.push(route); + return api().fetchJson(route, fields); + }; + expect(await gate.validateImprovementCommand(relayEvent(), dispatchVariables, fetchJson)).toMatchObject({ ok: true, violations: [] }); + expect(calls).toContain(`repos/${REPO}/issues/comments/901`); + expect(calls).toContain(`repos/${REPO}/collaborators/${encodeURIComponent('maintainer')}/permission`); + expect(calls).not.toContain(`repos/${REPO}/collaborators/${encodeURIComponent('github-actions[bot]')}/permission`); + }); + it.each([ + ['malformed aw_context JSON', () => relayEvent({ aw_context: '{not json' }), {}], + ['non-issue item_type', () => relayEvent({ aw_context: JSON.stringify({ item_type: 'pull_request', item_number: 77, comment_id: 901 }) }), {}], + ['item_number mismatch vs issue_number', () => relayEvent({ aw_context: JSON.stringify({ item_type: 'issue', item_number: 99, comment_id: 901 }) }), {}], + ['non-numeric comment_id', () => relayEvent({ aw_context: JSON.stringify({ item_type: 'issue', item_number: 77, comment_id: 'abc' }) }), {}], + ['edited comment (created_at !== updated_at)', () => relayEvent(), { comments: [comment()], approved: comment({ updated_at: '2026-09-02T01:00:00Z' }) }], + ['bot-authored comment', () => relayEvent(), { approved: comment({ user: { login: 'maintainer', type: 'Bot' } }) }], + ['weak collaborator permission', () => relayEvent(), { permission: { permission: 'read' } }], + ['non-matching comment content', () => relayEvent(), { approved: comment({ body: '/squad approve-improvement\nunexpected' }) }], + ])('rejects %s without weakening approval binding', async (_name, input, options) => { + const fetchJson = api(options).fetchJson; + const result = await gate.validateImprovementCommand(input(), dispatchVariables, fetchJson); + expect(result.ok).toBe(false); + expect(result.violations.length).toBeGreaterThan(0); + }); + it('reserves revoke-improvement over the dispatch relay the same as the native comment route', async () => { + const result = await gate.validateImprovementCommand(relayEvent({ command: 'revoke-improvement' }), dispatchVariables); + expect(result).toMatchObject({ ok: true, reserved: true, violations: [] }); + }); + it('ignores an unrelated relayed command instead of asserting approval', async () => { + const result = await gate.validateImprovementCommand(relayEvent({ command: 'status' }), dispatchVariables); + expect(result).toMatchObject({ ok: true, ignored: true, violations: [] }); + }); +}); diff --git a/workflows/package/squad-command-router.md b/workflows/package/squad-command-router.md index f09445927..c0bd16eed 100644 --- a/workflows/package/squad-command-router.md +++ b/workflows/package/squad-command-router.md @@ -2,6 +2,7 @@ name: Squad Command Router description: Discover /squad commands outside the start-only slash-command activation path on: + roles: all issues: types: - opened @@ -96,6 +97,19 @@ safe-outputs: return; } } + // Forward the originating comment (when this run was triggered by + // one) through the typed `aw_context` relay input. The dispatched + // squad.md run has no native `comment` event of its own, so without + // this, `/squad approve-improvement` relayed here can never supply + // a real `approval_comment_id` (#3). `comment_id` is read back by + // the worker-side gate, which re-fetches and independently + // re-validates the live comment before trusting anything — the + // relay only carries a pointer, never the approval itself. + const awContext = JSON.stringify({ + item_type: context.payload.issue?.pull_request ? 'pull_request' : 'issue', + item_number: issueNumber, + comment_id: context.payload.comment?.id ?? null, + }); await github.rest.actions.createWorkflowDispatch({ ...context.repo, workflow_id: 'squad.lock.yml', @@ -103,6 +117,7 @@ safe-outputs: inputs: { command: result.argumentText || 'cast', issue_number: String(issueNumber), + aw_context: awContext, }, }); add-comment: diff --git a/workflows/package/squad-deps-worker.md b/workflows/package/squad-deps-worker.md index 1747475d0..68f1ddd72 100644 --- a/workflows/package/squad-deps-worker.md +++ b/workflows/package/squad-deps-worker.md @@ -418,9 +418,19 @@ jobs: - detection - safe_outputs if: |- - !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && github.event_name == 'issue_comment' && (github.event.comment.body == '/squad activate' || - github.event.comment.body == '/squad plan accept' || - github.event.comment.body == '/squad plan activate') + !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && ( + (github.event_name == 'issue_comment' && + (github.event.comment.body == '/squad activate' || + github.event.comment.body == '/squad plan accept' || + github.event.comment.body == '/squad plan activate')) || + (github.event_name == 'workflow_dispatch' && + (github.event.inputs.command == 'activate' || + github.event.inputs.command == '/squad activate' || + github.event.inputs.command == 'plan accept' || + github.event.inputs.command == '/squad plan accept' || + github.event.inputs.command == 'plan activate' || + github.event.inputs.command == '/squad plan activate')) + ) runs-on: ubuntu-slim permissions: issues: write @@ -429,12 +439,27 @@ jobs: - name: Repair terminal lifecycle after idempotent activation uses: actions/github-script@v9 env: - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} - SQUAD_COMMAND: ${{ github.event.comment.body }} + ISSUE_NUMBER: ${{ github.event.inputs.issue_number || github.event.issue.number || github.event.pull_request.number }} + SQUAD_EVENT_NAME: ${{ github.event_name }} + SQUAD_COMMAND: ${{ github.event.inputs.command || github.event.comment.body }} with: script: | const issueNumber = Number(process.env.ISSUE_NUMBER); - const command = String(process.env.SQUAD_COMMAND || "").trim(); + const eventName = String(process.env.SQUAD_EVENT_NAME || ""); + const CANONICAL_BY_BARE_COMMAND = { + "activate": "/squad activate", + "plan accept": "/squad plan accept", + "plan activate": "/squad plan activate", + }; + let command = String(process.env.SQUAD_COMMAND || "").trim(); + if (eventName === "workflow_dispatch") { + // The command router relays a deterministically parsed, bare + // command (e.g. "activate") via workflow_dispatch; normalize it + // to the same canonical form used by the issue_comment path so + // both event sources share one acceptance check below. + const bare = command.replace(/^\/squad\s+/i, "").trim().toLowerCase(); + command = CANONICAL_BY_BARE_COMMAND[bare] || command; + } if ( !Number.isInteger(issueNumber) || issueNumber <= 0 || @@ -444,25 +469,34 @@ jobs: return; } - const actor = String(context.payload.comment?.user?.login || "").trim(); - if (!actor) { - core.setFailed("Lifecycle repair requires an identifiable comment author."); - return; - } - let permission; - try { - const response = await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: actor, - }); - permission = String(response.data?.permission || "").toLowerCase(); - } catch (error) { - core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); - return; - } - if (!["admin", "maintain", "write"].includes(permission)) { - core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); - return; + if (eventName === "workflow_dispatch") { + // GitHub requires write access to trigger workflow_dispatch, and + // the deterministic command router already authorized this + // mutating mode for the triggering actor before relaying it + // here as a workflow_dispatch; no further permission lookup + // applies for this event source. + core.info("Lifecycle repair authorized via workflow_dispatch (write access required to trigger)."); + } else { + const actor = String(context.payload.comment?.user?.login || "").trim(); + if (!actor) { + core.setFailed("Lifecycle repair requires an identifiable comment author."); + return; + } + let permission; + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: actor, + }); + permission = String(response.data?.permission || "").toLowerCase(); + } catch (error) { + core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); + return; + } + if (!["admin", "maintain", "write"].includes(permission)) { + core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); + return; + } } const comments = await github.paginate(github.rest.issues.listComments, { diff --git a/workflows/package/squad-implement-worker.md b/workflows/package/squad-implement-worker.md index 844391954..b05bb05dd 100644 --- a/workflows/package/squad-implement-worker.md +++ b/workflows/package/squad-implement-worker.md @@ -553,9 +553,19 @@ jobs: - detection - safe_outputs if: |- - !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && github.event_name == 'issue_comment' && (github.event.comment.body == '/squad activate' || - github.event.comment.body == '/squad plan accept' || - github.event.comment.body == '/squad plan activate') + !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && ( + (github.event_name == 'issue_comment' && + (github.event.comment.body == '/squad activate' || + github.event.comment.body == '/squad plan accept' || + github.event.comment.body == '/squad plan activate')) || + (github.event_name == 'workflow_dispatch' && + (github.event.inputs.command == 'activate' || + github.event.inputs.command == '/squad activate' || + github.event.inputs.command == 'plan accept' || + github.event.inputs.command == '/squad plan accept' || + github.event.inputs.command == 'plan activate' || + github.event.inputs.command == '/squad plan activate')) + ) runs-on: ubuntu-slim permissions: issues: write @@ -564,12 +574,27 @@ jobs: - name: Repair terminal lifecycle after idempotent activation uses: actions/github-script@v9 env: - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} - SQUAD_COMMAND: ${{ github.event.comment.body }} + ISSUE_NUMBER: ${{ github.event.inputs.issue_number || github.event.issue.number || github.event.pull_request.number }} + SQUAD_EVENT_NAME: ${{ github.event_name }} + SQUAD_COMMAND: ${{ github.event.inputs.command || github.event.comment.body }} with: script: | const issueNumber = Number(process.env.ISSUE_NUMBER); - const command = String(process.env.SQUAD_COMMAND || "").trim(); + const eventName = String(process.env.SQUAD_EVENT_NAME || ""); + const CANONICAL_BY_BARE_COMMAND = { + "activate": "/squad activate", + "plan accept": "/squad plan accept", + "plan activate": "/squad plan activate", + }; + let command = String(process.env.SQUAD_COMMAND || "").trim(); + if (eventName === "workflow_dispatch") { + // The command router relays a deterministically parsed, bare + // command (e.g. "activate") via workflow_dispatch; normalize it + // to the same canonical form used by the issue_comment path so + // both event sources share one acceptance check below. + const bare = command.replace(/^\/squad\s+/i, "").trim().toLowerCase(); + command = CANONICAL_BY_BARE_COMMAND[bare] || command; + } if ( !Number.isInteger(issueNumber) || issueNumber <= 0 || @@ -579,25 +604,34 @@ jobs: return; } - const actor = String(context.payload.comment?.user?.login || "").trim(); - if (!actor) { - core.setFailed("Lifecycle repair requires an identifiable comment author."); - return; - } - let permission; - try { - const response = await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: actor, - }); - permission = String(response.data?.permission || "").toLowerCase(); - } catch (error) { - core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); - return; - } - if (!["admin", "maintain", "write"].includes(permission)) { - core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); - return; + if (eventName === "workflow_dispatch") { + // GitHub requires write access to trigger workflow_dispatch, and + // the deterministic command router already authorized this + // mutating mode for the triggering actor before relaying it + // here as a workflow_dispatch; no further permission lookup + // applies for this event source. + core.info("Lifecycle repair authorized via workflow_dispatch (write access required to trigger)."); + } else { + const actor = String(context.payload.comment?.user?.login || "").trim(); + if (!actor) { + core.setFailed("Lifecycle repair requires an identifiable comment author."); + return; + } + let permission; + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: actor, + }); + permission = String(response.data?.permission || "").toLowerCase(); + } catch (error) { + core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); + return; + } + if (!["admin", "maintain", "write"].includes(permission)) { + core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); + return; + } } const comments = await github.paginate(github.rest.issues.listComments, { diff --git a/workflows/package/squad-retro.md b/workflows/package/squad-retro.md index a667762cd..e21aa8930 100644 --- a/workflows/package/squad-retro.md +++ b/workflows/package/squad-retro.md @@ -538,9 +538,19 @@ jobs: - detection - safe_outputs if: |- - !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && github.event_name == 'issue_comment' && (github.event.comment.body == '/squad activate' || - github.event.comment.body == '/squad plan accept' || - github.event.comment.body == '/squad plan activate') + !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && ( + (github.event_name == 'issue_comment' && + (github.event.comment.body == '/squad activate' || + github.event.comment.body == '/squad plan accept' || + github.event.comment.body == '/squad plan activate')) || + (github.event_name == 'workflow_dispatch' && + (github.event.inputs.command == 'activate' || + github.event.inputs.command == '/squad activate' || + github.event.inputs.command == 'plan accept' || + github.event.inputs.command == '/squad plan accept' || + github.event.inputs.command == 'plan activate' || + github.event.inputs.command == '/squad plan activate')) + ) runs-on: ubuntu-slim permissions: issues: write @@ -549,12 +559,27 @@ jobs: - name: Repair terminal lifecycle after idempotent activation uses: actions/github-script@v9 env: - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} - SQUAD_COMMAND: ${{ github.event.comment.body }} + ISSUE_NUMBER: ${{ github.event.inputs.issue_number || github.event.issue.number || github.event.pull_request.number }} + SQUAD_EVENT_NAME: ${{ github.event_name }} + SQUAD_COMMAND: ${{ github.event.inputs.command || github.event.comment.body }} with: script: | const issueNumber = Number(process.env.ISSUE_NUMBER); - const command = String(process.env.SQUAD_COMMAND || "").trim(); + const eventName = String(process.env.SQUAD_EVENT_NAME || ""); + const CANONICAL_BY_BARE_COMMAND = { + "activate": "/squad activate", + "plan accept": "/squad plan accept", + "plan activate": "/squad plan activate", + }; + let command = String(process.env.SQUAD_COMMAND || "").trim(); + if (eventName === "workflow_dispatch") { + // The command router relays a deterministically parsed, bare + // command (e.g. "activate") via workflow_dispatch; normalize it + // to the same canonical form used by the issue_comment path so + // both event sources share one acceptance check below. + const bare = command.replace(/^\/squad\s+/i, "").trim().toLowerCase(); + command = CANONICAL_BY_BARE_COMMAND[bare] || command; + } if ( !Number.isInteger(issueNumber) || issueNumber <= 0 || @@ -564,25 +589,34 @@ jobs: return; } - const actor = String(context.payload.comment?.user?.login || "").trim(); - if (!actor) { - core.setFailed("Lifecycle repair requires an identifiable comment author."); - return; - } - let permission; - try { - const response = await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: actor, - }); - permission = String(response.data?.permission || "").toLowerCase(); - } catch (error) { - core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); - return; - } - if (!["admin", "maintain", "write"].includes(permission)) { - core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); - return; + if (eventName === "workflow_dispatch") { + // GitHub requires write access to trigger workflow_dispatch, and + // the deterministic command router already authorized this + // mutating mode for the triggering actor before relaying it + // here as a workflow_dispatch; no further permission lookup + // applies for this event source. + core.info("Lifecycle repair authorized via workflow_dispatch (write access required to trigger)."); + } else { + const actor = String(context.payload.comment?.user?.login || "").trim(); + if (!actor) { + core.setFailed("Lifecycle repair requires an identifiable comment author."); + return; + } + let permission; + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: actor, + }); + permission = String(response.data?.permission || "").toLowerCase(); + } catch (error) { + core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); + return; + } + if (!["admin", "maintain", "write"].includes(permission)) { + core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); + return; + } } const comments = await github.paginate(github.rest.issues.listComments, { diff --git a/workflows/package/squad.md b/workflows/package/squad.md index 91bed0441..5d1ffa317 100644 --- a/workflows/package/squad.md +++ b/workflows/package/squad.md @@ -483,9 +483,19 @@ jobs: - detection - safe_outputs if: |- - !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && github.event_name == 'issue_comment' && (github.event.comment.body == '/squad activate' || - github.event.comment.body == '/squad plan accept' || - github.event.comment.body == '/squad plan activate') + !cancelled() && needs.agent.result == 'success' && needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && ( + (github.event_name == 'issue_comment' && + (github.event.comment.body == '/squad activate' || + github.event.comment.body == '/squad plan accept' || + github.event.comment.body == '/squad plan activate')) || + (github.event_name == 'workflow_dispatch' && + (github.event.inputs.command == 'activate' || + github.event.inputs.command == '/squad activate' || + github.event.inputs.command == 'plan accept' || + github.event.inputs.command == '/squad plan accept' || + github.event.inputs.command == 'plan activate' || + github.event.inputs.command == '/squad plan activate')) + ) runs-on: ubuntu-slim permissions: issues: write @@ -494,12 +504,27 @@ jobs: - name: Repair terminal lifecycle after idempotent activation uses: actions/github-script@v9 env: - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} - SQUAD_COMMAND: ${{ github.event.comment.body }} + ISSUE_NUMBER: ${{ github.event.inputs.issue_number || github.event.issue.number || github.event.pull_request.number }} + SQUAD_EVENT_NAME: ${{ github.event_name }} + SQUAD_COMMAND: ${{ github.event.inputs.command || github.event.comment.body }} with: script: | const issueNumber = Number(process.env.ISSUE_NUMBER); - const command = String(process.env.SQUAD_COMMAND || "").trim(); + const eventName = String(process.env.SQUAD_EVENT_NAME || ""); + const CANONICAL_BY_BARE_COMMAND = { + "activate": "/squad activate", + "plan accept": "/squad plan accept", + "plan activate": "/squad plan activate", + }; + let command = String(process.env.SQUAD_COMMAND || "").trim(); + if (eventName === "workflow_dispatch") { + // The command router relays a deterministically parsed, bare + // command (e.g. "activate") via workflow_dispatch; normalize it + // to the same canonical form used by the issue_comment path so + // both event sources share one acceptance check below. + const bare = command.replace(/^\/squad\s+/i, "").trim().toLowerCase(); + command = CANONICAL_BY_BARE_COMMAND[bare] || command; + } if ( !Number.isInteger(issueNumber) || issueNumber <= 0 || @@ -509,25 +534,34 @@ jobs: return; } - const actor = String(context.payload.comment?.user?.login || "").trim(); - if (!actor) { - core.setFailed("Lifecycle repair requires an identifiable comment author."); - return; - } - let permission; - try { - const response = await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: actor, - }); - permission = String(response.data?.permission || "").toLowerCase(); - } catch (error) { - core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); - return; - } - if (!["admin", "maintain", "write"].includes(permission)) { - core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); - return; + if (eventName === "workflow_dispatch") { + // GitHub requires write access to trigger workflow_dispatch, and + // the deterministic command router already authorized this + // mutating mode for the triggering actor before relaying it + // here as a workflow_dispatch; no further permission lookup + // applies for this event source. + core.info("Lifecycle repair authorized via workflow_dispatch (write access required to trigger)."); + } else { + const actor = String(context.payload.comment?.user?.login || "").trim(); + if (!actor) { + core.setFailed("Lifecycle repair requires an identifiable comment author."); + return; + } + let permission; + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: actor, + }); + permission = String(response.data?.permission || "").toLowerCase(); + } catch (error) { + core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); + return; + } + if (!["admin", "maintain", "write"].includes(permission)) { + core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); + return; + } } const comments = await github.paginate(github.rest.issues.listComments, { @@ -1576,6 +1610,7 @@ failures, not commands to reinterpret as Cast. - **Event name:** `${{ github.event_name }}` - **Dispatched command:** `${{ github.event.inputs.command }}` - **Dispatched issue number:** `${{ github.event.inputs.issue_number }}` +- **Dispatched aw_context:** `${{ github.event.inputs.aw_context }}` ### Workflow-dispatch activation guard [MANDATORY — run before any skill] @@ -2297,15 +2332,21 @@ description: Relay an approved retrospective governance proposal to the improvem After the existing mutating authorization guard, relay only an issue comment created by the authorized human. Other events, PR comments and missing IDs receive a refusal, never a dispatch. The worker re-fetches the exact comment, -permission, content revision and scope. Use the typed `dispatch_workflow` -safe-output, with nested inputs (never a generic GitHub mutation): +permission, content revision and scope. Resolve `approval_comment_id` in this +order: `github.event.comment.id` when this run was activated directly by the +comment; otherwise, under `workflow_dispatch`, the `comment_id` field of the +parsed **Dispatched aw_context** (Trigger Context) relayed by the command +router. If neither resolves to a positive integer, refuse and STOP — never +dispatch with a guessed, omitted, or placeholder id. Use the typed +`dispatch_workflow` safe-output, with nested inputs (never a generic GitHub +mutation): ```json { "workflow_name": "squad-improvement-worker", "inputs": { "issue_number": "{issue-number}", - "approval_comment_id": "{triggering-comment-id}" + "approval_comment_id": "{resolved-approval-comment-id}" } } ``` diff --git a/workflows/shared/squad-improvement-gate.mjs b/workflows/shared/squad-improvement-gate.mjs index 59f00fc5b..0a59f8ef7 100644 --- a/workflows/shared/squad-improvement-gate.mjs +++ b/workflows/shared/squad-improvement-gate.mjs @@ -11,6 +11,7 @@ import { readAgentOutputItems, evaluateRetroPullRequestItems, pullLinksIssue, findNativeLinkedPulls, NATIVE_LINK_MAX_PAGES, } from './squad-retro-provenance.mjs'; +export { isNumericId }; export { ACTION_LABEL, PROPOSAL_LABEL, extractActionKey, readAgentOutputItems }; export const APPROVE_COMMAND = '/squad approve-improvement'; @@ -373,11 +374,59 @@ export async function enforceImprovementSafeOutputs(env = process.env, { // Fail fast before the dispatcher agent; its existing mode authorization still // runs. The receiver checks injected aw_context against the exact relayed IDs. +// A relayed workflow_dispatch run has no native `comment` event of its own: +// the command router forwards only a `comment_id` pointer through the typed +// `aw_context` input (#3). That pointer is never trusted on its own — it is +// used solely to re-fetch the live comment, which is then independently +// re-validated against the exact same invariants as the direct path below +// (issue match, human author, unedited, exact approval content). The +// comment's own live `user.login` — never the dispatch actor, which is the +// relaying bot identity, not the approver — is what gets the permission +// check, so approval binding is exactly as strong as the native route. +async function validateApprovalDispatchRelay(event, env, fetchJson, rawCommand) { + if (rawCommand !== 'approve-improvement') return { ok: false, violations: [{ kind: 'approval-route-invalid' }] }; + let awContext = null; + try { + awContext = JSON.parse(event?.inputs?.aw_context || ''); + } catch { + awContext = null; + } + const issueNumber = Number(event?.inputs?.issue_number); + const commentId = Number(awContext?.comment_id); + if (!awContext || awContext.item_type !== 'issue' || + !isNumericId(issueNumber) || Number(awContext.item_number) !== issueNumber || + !isNumericId(commentId)) { + return { ok: false, violations: [{ kind: 'approval-route-invalid' }] }; + } + const violations = []; + const comment = await fetchJson(`repos/${env.GITHUB_REPOSITORY}/issues/comments/${commentId}`, {}); + const apiBase = String(env.GITHUB_API_URL || 'https://api.github.com').replace(/\/+$/, ''); + if (comment?.id !== commentId || + comment.issue_url !== `${apiBase}/repos/${env.GITHUB_REPOSITORY}/issues/${issueNumber}` || + comment?.user?.type !== 'User' || + comment.performed_via_github_app || comment.created_at !== comment.updated_at || + parseCommandComment(comment.body).command !== 'approve') { + violations.push({ kind: 'approval-comment-provenance-invalid' }); + } + const relayedActor = comment?.user?.login; + const permission = relayedActor + ? await fetchJson(`repos/${env.GITHUB_REPOSITORY}/collaborators/${encodeURIComponent(relayedActor)}/permission`, {}) + : null; + if (!relayedActor || !isTrustedApprover({ permission: permission?.permission }).trusted) { + violations.push({ kind: 'actor-not-trusted' }); + } + return { ok: !violations.length, violations }; +} + export async function validateImprovementCommand(event, env, fetchJson = (route, fields) => restJson(env, route, fields)) { + const isDispatchRelay = env.GITHUB_EVENT_NAME === 'workflow_dispatch'; + const rawCommand = isDispatchRelay + ? String(event?.inputs?.command ?? '').trim() + : /(?:^|\s)\/squad(?:\s+|$)([^\r\n]*)/.exec(normalizeText(event?.comment?.body || event?.issue?.body))?.[1]?.trim(); + if (rawCommand === 'revoke-improvement') return { ok: true, reserved: true, violations: [] }; + if (!rawCommand?.startsWith('approve-improvement')) return { ok: true, ignored: true, violations: [] }; + if (isDispatchRelay) return validateApprovalDispatchRelay(event, env, fetchJson, rawCommand); const violations = []; - const rawCommand = /(?:^|\s)\/squad(?:\s+|$)([^\r\n]*)/.exec(normalizeText(event?.comment?.body || event?.issue?.body))?.[1]?.trim(); - if (rawCommand === 'revoke-improvement') return { ok: true, reserved: true, violations }; - if (!rawCommand?.startsWith('approve-improvement')) return { ok: true, ignored: true, violations }; if (env.GITHUB_EVENT_NAME !== 'issue_comment' || event.action !== 'created' || event.issue?.pull_request || rawCommand !== 'approve-improvement') { return { ok: false, violations: [{ kind: 'approval-route-invalid' }] }; diff --git a/workflows/shared/squad.md b/workflows/shared/squad.md index f68afbeca..2b45d4de6 100644 --- a/workflows/shared/squad.md +++ b/workflows/shared/squad.md @@ -306,10 +306,19 @@ jobs: needs.detection.result == 'success' && needs.safe_outputs.result == 'success' && !contains(needs.agent.outputs.output_types, 'upsert_lifecycle_state') && - github.event_name == 'issue_comment' && - (github.event.comment.body == '/squad activate' || - github.event.comment.body == '/squad plan accept' || - github.event.comment.body == '/squad plan activate') + ( + (github.event_name == 'issue_comment' && + (github.event.comment.body == '/squad activate' || + github.event.comment.body == '/squad plan accept' || + github.event.comment.body == '/squad plan activate')) || + (github.event_name == 'workflow_dispatch' && + (github.event.inputs.command == 'activate' || + github.event.inputs.command == '/squad activate' || + github.event.inputs.command == 'plan accept' || + github.event.inputs.command == '/squad plan accept' || + github.event.inputs.command == 'plan activate' || + github.event.inputs.command == '/squad plan activate')) + ) runs-on: ubuntu-slim permissions: issues: write @@ -318,12 +327,27 @@ jobs: - name: Repair terminal lifecycle after idempotent activation uses: actions/github-script@v9 env: - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} - SQUAD_COMMAND: ${{ github.event.comment.body }} + ISSUE_NUMBER: ${{ github.event.inputs.issue_number || github.event.issue.number || github.event.pull_request.number }} + SQUAD_EVENT_NAME: ${{ github.event_name }} + SQUAD_COMMAND: ${{ github.event.inputs.command || github.event.comment.body }} with: script: | const issueNumber = Number(process.env.ISSUE_NUMBER); - const command = String(process.env.SQUAD_COMMAND || "").trim(); + const eventName = String(process.env.SQUAD_EVENT_NAME || ""); + const CANONICAL_BY_BARE_COMMAND = { + "activate": "/squad activate", + "plan accept": "/squad plan accept", + "plan activate": "/squad plan activate", + }; + let command = String(process.env.SQUAD_COMMAND || "").trim(); + if (eventName === "workflow_dispatch") { + // The command router relays a deterministically parsed, bare + // command (e.g. "activate") via workflow_dispatch; normalize it + // to the same canonical form used by the issue_comment path so + // both event sources share one acceptance check below. + const bare = command.replace(/^\/squad\s+/i, "").trim().toLowerCase(); + command = CANONICAL_BY_BARE_COMMAND[bare] || command; + } if ( !Number.isInteger(issueNumber) || issueNumber <= 0 || @@ -333,25 +357,34 @@ jobs: return; } - const actor = String(context.payload.comment?.user?.login || "").trim(); - if (!actor) { - core.setFailed("Lifecycle repair requires an identifiable comment author."); - return; - } - let permission; - try { - const response = await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: actor, - }); - permission = String(response.data?.permission || "").toLowerCase(); - } catch (error) { - core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); - return; - } - if (!["admin", "maintain", "write"].includes(permission)) { - core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); - return; + if (eventName === "workflow_dispatch") { + // GitHub requires write access to trigger workflow_dispatch, and + // the deterministic command router already authorized this + // mutating mode for the triggering actor before relaying it + // here as a workflow_dispatch; no further permission lookup + // applies for this event source. + core.info("Lifecycle repair authorized via workflow_dispatch (write access required to trigger)."); + } else { + const actor = String(context.payload.comment?.user?.login || "").trim(); + if (!actor) { + core.setFailed("Lifecycle repair requires an identifiable comment author."); + return; + } + let permission; + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: actor, + }); + permission = String(response.data?.permission || "").toLowerCase(); + } catch (error) { + core.setFailed(`Unable to verify lifecycle repair permission for ${actor}: ${error.message}`); + return; + } + if (!["admin", "maintain", "write"].includes(permission)) { + core.info(`Lifecycle repair is not authorized for ${actor} with ${permission || "unresolved"} permission.`); + return; + } } const comments = await github.paginate(github.rest.issues.listComments, { diff --git a/workflows/squad-command-router.md b/workflows/squad-command-router.md index 0d652f069..39a21aa3e 100644 --- a/workflows/squad-command-router.md +++ b/workflows/squad-command-router.md @@ -2,6 +2,7 @@ name: Squad Command Router description: Discover /squad commands outside the start-only slash-command activation path on: + roles: all issues: types: [opened, edited, reopened] issue_comment: @@ -93,6 +94,19 @@ safe-outputs: return; } } + // Forward the originating comment (when this run was triggered by + // one) through the typed `aw_context` relay input. The dispatched + // squad.md run has no native `comment` event of its own, so without + // this, `/squad approve-improvement` relayed here can never supply + // a real `approval_comment_id` (#3). `comment_id` is read back by + // the worker-side gate, which re-fetches and independently + // re-validates the live comment before trusting anything — the + // relay only carries a pointer, never the approval itself. + const awContext = JSON.stringify({ + item_type: context.payload.issue?.pull_request ? 'pull_request' : 'issue', + item_number: issueNumber, + comment_id: context.payload.comment?.id ?? null, + }); await github.rest.actions.createWorkflowDispatch({ ...context.repo, workflow_id: 'squad.lock.yml', @@ -100,6 +114,7 @@ safe-outputs: inputs: { command: result.argumentText || 'cast', issue_number: String(issueNumber), + aw_context: awContext, }, }); add-comment: diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index 3ea68309a..9f6b86909 100644 --- a/workflows/squad-workflows.manifest.json +++ b/workflows/squad-workflows.manifest.json @@ -14,18 +14,18 @@ "source": "workflows/package/squad.md", "destination": ".github/workflows/squad.md", "lock": ".github/workflows/squad.lock.yml", - "source_sha256": "f86eb83a75c6de63edf520bf21ca2b50fcb9c6e7c5a60ecbd92a527a108a19af", - "lock_sha256": "b11d0ef4a4cc067f71d6e4fae218fa7689609b1c689effc72cd689b1e363c4bc", - "package_lock_sha256": "e2129810e4a6ae01a0802cd22cd8c80e81fb3e6722ed1dcd2ba0a62ad48d5c49" + "source_sha256": "b237872416c4e1357ed8136e9c3444cbcae1a97a2458bff81af9a4f7087bc40b", + "lock_sha256": "9d44255bcccd8cb114c57b65fe2e2ecf582a378f2ea21191d27321f2525f8886", + "package_lock_sha256": "3f632c603377d31c7157bbd4bde08efd37da85098c2a163e27b55ddcfae1fe72" }, { "name": "squad-implement-worker", "source": "workflows/package/squad-implement-worker.md", "destination": ".github/workflows/squad-implement-worker.md", "lock": ".github/workflows/squad-implement-worker.lock.yml", - "source_sha256": "049eabecb2bab774e389fc62c6bde9308f3fb1a923325401cbecc95d63fc2757", - "lock_sha256": "2d4113d7f4383289ce7a2e39510c19728ece0efaa51e904feb2b18f6cc377bad", - "package_lock_sha256": "687922b8e2c945d7273f75b8faa2252a218e643f6b3e110d286c4a68712577f2" + "source_sha256": "40067547110aa4c3186091265a2d77a1f05b8e1c0cca92a274470838d12e1ec3", + "lock_sha256": "11610f64c32403acb69b8f111f0fe584572a0e715bcb53eccaa5bd0cacaa57b5", + "package_lock_sha256": "c6b34a8ae8e9848bbc964a0ca73080ab9c06a9b9238de860954f84f0ba1bf5d7" }, { "name": "squad-review", @@ -41,18 +41,18 @@ "source": "workflows/package/squad-deps-worker.md", "destination": ".github/workflows/squad-deps-worker.md", "lock": ".github/workflows/squad-deps-worker.lock.yml", - "source_sha256": "37c4ba60b85c5fc07bef322bbfd0f713472924d55cff9481ae76af7ec0c91fac", - "lock_sha256": "17ba7712c13826d764149171d4f8144a02d4192f41f65a2a44ed9374f28b4ff5", - "package_lock_sha256": "34f62358d3557e8830891df656bdec7d32163d9b9dea692370fe1e6b031da59f" + "source_sha256": "8057ff2e4cb2a6be55da43092397a890880fb94cadfe8d0dd15766dd22f1fe9e", + "lock_sha256": "31ece8146cf21ca5aec2880fd5daa01b982f037a961d3fd0b7c50a6eb3893dd4", + "package_lock_sha256": "59f035683a2b2dbecabdb685ff8280b05c4a081cf3d934a1f77591902b8e83a1" }, { "name": "squad-retro", "source": "workflows/package/squad-retro.md", "destination": ".github/workflows/squad-retro.md", "lock": ".github/workflows/squad-retro.lock.yml", - "source_sha256": "92bcfa39cc5fd3087b11612125e314959237c5b79460825453d1e493727a2562", - "lock_sha256": "4585bf60fdaa362bfca4edc2cb0c077c5cb17febd7d4a532eafa1a4941b61506", - "package_lock_sha256": "d06662da4e437b0697b8ce374b272e4a955d8ab5c077df9e722982c0b8205242" + "source_sha256": "3ee1bdd2d032f70d83f5593ff2bfc71fef6782d1cef0daf17069dfcb95adafcd", + "lock_sha256": "7850fb52a8ac0eb4e7f97a9846c54986607c77d3fa1fb4dce934bdc44b632994", + "package_lock_sha256": "cb11982ae6720f511ab73a8932773a521ec32e5afe453ac6ca6d015dd2bb5c2c" }, { "name": "squad-improvement-worker", @@ -77,9 +77,9 @@ "source": "workflows/package/squad-command-router.md", "destination": ".github/workflows/squad-command-router.md", "lock": ".github/workflows/squad-command-router.lock.yml", - "source_sha256": "c7e281a1d110f77f54a6f0a5257ecf8edd6f336bc894278d82efff604b03db79", - "lock_sha256": "8c380ff780d28750c37cde51bf4dacc840bcfdf5c59f991e4b61a19f24652890", - "package_lock_sha256": "b5253772c9e2eb562e0cabac1b9133a63c46d94c446d541de498fbbba9f0134e" + "source_sha256": "91b1c3b1a9e5499669f16e3f06f27470570162323376b34a8d8f3156c1e84f3a", + "lock_sha256": "9bdd3d177667a599f1abd4c6142327ec074c46d6593293b285ac57dbb3539053", + "package_lock_sha256": "ff161ff50343b849a87d27e1635986bcf9f1da6b96c37be784c06beda69c388b" } ], "shared_runtime": [ @@ -129,7 +129,7 @@ "package_destination": ".github/workflows/shared/squad-improvement-gate.mjs", "destination": ".github/workflows/shared/squad-improvement-gate.mjs", "ownership": "manifest", - "sha256": "5b34324486773f75e5275ea2d73c781edafdcb415f15dd9b97863e78dc3f6ca4" + "sha256": "b8e27c9311a24ce448ff34d5bae5c50947659c01acc3704dfde6dc6ca6d2a042" }, { "path": "shared/squad-retro-evidence.mjs", @@ -161,7 +161,7 @@ "package_destination": ".github/aw/squad/runtime/shared/squad.md", "destination": ".github/workflows/shared/squad.md", "ownership": "manifest", - "sha256": "802720c5ee7f766fe261ee3c4d9b5a4d5e16fc9f764a0354daae07a846ec3709" + "sha256": "4cddc662ecd98f9fcd8e8623e908a2bd1a75c30c12bbcf98e3ad26db93135a66" }, { "path": "shared/squad-planning-ontology.md", diff --git a/workflows/squad.md b/workflows/squad.md index 284b4b267..9279333c1 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -465,6 +465,7 @@ failures, not commands to reinterpret as Cast. - **Event name:** `${{ github.event_name }}` - **Dispatched command:** `${{ github.event.inputs.command }}` - **Dispatched issue number:** `${{ github.event.inputs.issue_number }}` +- **Dispatched aw_context:** `${{ github.event.inputs.aw_context }}` ### Workflow-dispatch activation guard [MANDATORY — run before any skill] @@ -1186,15 +1187,21 @@ description: Relay an approved retrospective governance proposal to the improvem After the existing mutating authorization guard, relay only an issue comment created by the authorized human. Other events, PR comments and missing IDs receive a refusal, never a dispatch. The worker re-fetches the exact comment, -permission, content revision and scope. Use the typed `dispatch_workflow` -safe-output, with nested inputs (never a generic GitHub mutation): +permission, content revision and scope. Resolve `approval_comment_id` in this +order: `github.event.comment.id` when this run was activated directly by the +comment; otherwise, under `workflow_dispatch`, the `comment_id` field of the +parsed **Dispatched aw_context** (Trigger Context) relayed by the command +router. If neither resolves to a positive integer, refuse and STOP — never +dispatch with a guessed, omitted, or placeholder id. Use the typed +`dispatch_workflow` safe-output, with nested inputs (never a generic GitHub +mutation): ```json { "workflow_name": "squad-improvement-worker", "inputs": { "issue_number": "{issue-number}", - "approval_comment_id": "{triggering-comment-id}" + "approval_comment_id": "{resolved-approval-comment-id}" } } ``` From cea6931ab8f32935b1512cee07fec1902f05b266 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Thu, 1 Oct 2026 13:06:33 -0700 Subject: [PATCH 2/4] fix(gh-aw): update compiled-digest golden hashes for relay fix The roles: all / aw_context changes to squad.md and squad-command-router.md legitimately alter the compiled squad.lock.yml content. Update the three hardcoded golden SHA-256 digests in test/gh-aw-quality.test.ts's canonical package integrity contract suite to match, following the same pattern as commit b5455084. Independently recomputed via direct execution of the real compileWithPinnedActions/normalizeCompiledLock helpers (node --experimental-strip-types, bypassing the broken local vitest install) and confirmed to match hosted CI's reported values exactly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- test/gh-aw-quality.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index b1e8c791e..5d18e083a 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -3961,8 +3961,8 @@ describe('gh-aw: canonical package integrity contract', () => { expect(mutable).not.toContain(actionReference); expect(createHash('sha256').update(normalizeCompiledLock(mutable, revisionA)).digest('hex')) .toBe(sourceBinding === 'workflow' - ? 'a3290dad380577fd6d64c92169b9d74a7f5248851b110d8eafc6260b34f772d9' - : 'fd7c735e2de5088954e8d0d647a15e5f95ed5850947f201c1a4ce17433d66632'); + ? '518b6688093479b51a1b4d170c56bd0fc0317370bcf50cedcaa6d48bd04a5fc0' + : 'dca7fe0dee3fd12ffaab3a26fa28ab02c7e90f7862f6e6814d503e729f52e18a'); expect(() => validateCompilerActionPins(mutable)).toThrow(/invalid immutable action pin/); writeFileSync(lockPath, mutable); expect(verifyInstall(root).failures.join('\n')) @@ -4016,7 +4016,7 @@ describe('gh-aw: canonical package integrity contract', () => { compile(); const unpinned = readText(lockPath); expect(createHash('sha256').update(normalizeCompiledLock(unpinned, revisionA)).digest('hex')) - .toBe('fd7c735e2de5088954e8d0d647a15e5f95ed5850947f201c1a4ce17433d66632'); + .toBe('dca7fe0dee3fd12ffaab3a26fa28ab02c7e90f7862f6e6814d503e729f52e18a'); expect(verifyInstall(root).failures.join('\n')).toContain('Installed digest mismatch'); const seedPins = () => spawnSync(process.execPath, ['--input-type=module', '-e', seed!], { From faed1e34fdb3e55cd5d8c8d8f01c46802d9b585f Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Thu, 1 Oct 2026 13:49:17 -0700 Subject: [PATCH 3/4] fix(gh-aw): forward real item identity across the squad.md -> squad-improvement-worker relay hop Fixes a downstream defect in the command-routing relay chain (flagged by copilot-pull-request-reviewer[bot] against PR #2136, which fixed the three originally-scoped findings from source SHA 261d140679b158888f1a3c4d9a5a3e74eb55b633, octodemo/zava-social-backend-20261001184422#1). Root cause: gh-aw's own dispatch_workflow safe-output engine unconditionally overwrites any agent-supplied aw_context input with its own buildAwContext() whenever a dispatch's target workflow declares an aw_context workflow_dispatch input. buildAwContext() derives item_type/item_number/comment_id solely from the CURRENT run's own event payload -- never inherited across a hop. When a relayed (workflow_dispatch -triggered) squad.md run dispatches to squad-improvement-worker via the squad-approve-improvement skill, that dispatch goes through gh-aw's real dispatch_workflow handler, so the engine always overwrites squad.md's aw_context with its own context -- which, because squad.md's own run is a workflow_dispatch event with no issue/comment payload, always carries empty item_type/item_number/comment_id. collectImprovementContext's existing strict exact-match check (correct for the native direct-comment path, where the engine's own context genuinely is accurate) therefore always rejected every relayed /squad approve-improvement, independent of any event_type widening. Fix (no gh-aw change; pure squad.md-side plumbing): - squad-improvement-worker.md: declare a new, non-colliding squad_approval_relay workflow_dispatch input (distinct name from aw_context, so gh-aw's engine never auto-overwrites it) and wire a parallel SQUAD_IMPROVE_RELAY_CONTEXT env var at both existing call sites. - squad.md: the squad-approve-improvement skill now always forwards squad_approval_relay: {event_type, item_type, item_number, comment_id} alongside the existing issue_number/approval_comment_id inputs, reusing the same already-resolved values. - squad-improvement-gate.mjs: collectImprovementContext keeps the unconditional repo/workflow_id provenance check against the engine's own (always-accurate) context, then branches: native path (event_type === issue_comment) validates item identity against the engine context unchanged; relayed path (any other event_type) validates with identical exact-match strictness against the new SQUAD_IMPROVE_RELAY_CONTEXT instead. No approval-binding check is weakened on either path; all unconditional live re-verification (comment/permission/revocation/ revision/linked-PR checks) is untouched. Regenerated workflows/package/*.md and workflows/squad-workflows.manifest.json via `node workflows/shared/squad-install-verifier.mjs --write-source`. Tests (test/gh-aw-improvement-worker.test.ts): - Extends the skill-JSON-shape assertion for the new relay field. - New describe block covering: relayed-path authorization success, repo/ workflow_id forgery still rejected, mutation-kill coverage (tampered comment_id/item_number/item_type/event_type each independently rejected), malformed/missing relay context rejected (no silent fallback to the engine's empty item fields), native-path non-regression, source wiring assertions, and a compiled-artifact assertion (gh aw compile --strict) proving both the worker's lock.yml declares/wires the new input and squad.lock.yml's compiled dispatch step actually emits squad_approval_relay. Verified locally: standalone Node execution of collectImprovementContext's new branch logic (native path, relayed success, 4 mutation-kill cases, 3 malformed/missing-context cases, repo/workflow_id forgery); gh aw compile --strict across all 8 installed workflows produces exactly the two documented warnings (Compiled 8 workflows: 8 succeeded, 2 warnings); per-workflow strict compiles of squad and squad-improvement-worker confirm the new input/env/dispatch-field wiring in the generated lock.yml files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- test/gh-aw-improvement-worker.test.ts | 98 ++++++++++++++++++- workflows/package/squad-improvement-worker.md | 6 ++ workflows/package/squad.md | 15 ++- workflows/shared/squad-improvement-gate.mjs | 32 +++++- workflows/squad-improvement-worker.md | 10 ++ workflows/squad-workflows.manifest.json | 14 +-- workflows/squad.md | 15 ++- 7 files changed, 175 insertions(+), 15 deletions(-) diff --git a/test/gh-aw-improvement-worker.test.ts b/test/gh-aw-improvement-worker.test.ts index fffb6bad2..1bfe64c87 100644 --- a/test/gh-aw-improvement-worker.test.ts +++ b/test/gh-aw-improvement-worker.test.ts @@ -463,7 +463,8 @@ describe('improvement: one authorized dispatcher route and installed contract', expect(ROUTER).toContain('gate.validateImprovementCommand(context.payload, process.env)'); const skill = ROUTER.slice(ROUTER.indexOf('## skill: `squad-approve-improvement`'), ROUTER.indexOf('## skill: `squad-revoke-improvement`')); const payload = JSON.parse(skill.match(/```json\n([\s\S]*?)\n```/)![1]); - expect(Object.keys(payload.inputs).sort()).toEqual(['approval_comment_id', 'issue_number']); + expect(Object.keys(payload.inputs).sort()).toEqual(['approval_comment_id', 'issue_number', 'squad_approval_relay']); + expect(payload.inputs.squad_approval_relay).toMatchObject({ event_type: 'issue_comment', item_type: 'issue' }); expect(payload.issue_number).toBeUndefined(); }); it.each([ @@ -510,6 +511,13 @@ describe('improvement: one authorized dispatcher route and installed contract', }); expect(config.create_pull_request.protected_files).toContain('package.json'); expect(config).not.toHaveProperty('dispatch_workflow'); + // The second-hop relay fix (#downstream): the compiled lock must actually + // declare the forwarded input and wire it into both env blocks gh-aw + // generates, not just the un-compiled source -- proving the compiler + // itself emits the mechanism collectImprovementContext's relayed branch + // depends on. + expect(lock).toContain('squad_approval_relay:'); + expect((lock.match(/SQUAD_IMPROVE_RELAY_CONTEXT: \$\{\{ github\.event\.inputs\.squad_approval_relay \}\}/g) || []).length).toBe(2); expect(WORKER).toContain('patch-format: am'); expect(WORKER).toContain('echo ".github/workflows/squad-improvement-context.json" >> "${GITHUB_WORKSPACE:?}/.git/info/exclude"'); expect(WORKER).not.toMatch(/^\s{2}(issue_comment|pull_request|schedule):/m); @@ -574,3 +582,91 @@ describe('improvement: workflow_dispatch relay carries real comment provenance ( expect(result).toMatchObject({ ok: true, ignored: true, violations: [] }); }); }); + +// Downstream of Finding #3: gh-aw's own dispatch_workflow safe-output engine +// unconditionally overwrites any agent-supplied `aw_context` input with its +// own buildAwContext() whenever the dispatch target declares an `aw_context` +// workflow_dispatch input -- and that engine-built context derives item +// identity solely from the CURRENT run's own event payload. When squad.md +// itself is a relayed (workflow_dispatch) run forwarding `approve-improvement` +// to squad-improvement-worker, its own payload carries no issue/comment, so +// the engine-injected aw_context on that second hop always carries an empty +// item_type/item_number/comment_id and event_type: 'workflow_dispatch' -- +// unusable for collectImprovementContext's exact item-identity check, no +// matter how the event_type check is widened. squad.md now also forwards a +// second, non-colliding input (`squad_approval_relay`, never named +// `aw_context`, so gh-aw's engine never touches it) carrying its own +// independently resolved item identity; collectImprovementContext validates +// against it only when the engine's own event_type is not 'issue_comment', +// with identical exact-match strictness to the native path. +describe('improvement: second-hop relay context survives gh-aw engine aw_context override (#downstream)', () => { + const relayedOrigin = { + repo: REPO, workflow_id: `${REPO}/.github/workflows/squad.lock.yml@refs/heads/dev`, + event_type: 'workflow_dispatch', item_type: '', item_number: '', comment_id: '', + }; + const relayContext = { event_type: 'issue_comment', item_type: 'issue', item_number: '77', comment_id: '901' }; + const routedEnv = { + ...env(), GITHUB_ACTOR: 'github-actions[bot]', + SQUAD_IMPROVE_AW_CONTEXT: JSON.stringify(relayedOrigin), + SQUAD_IMPROVE_RELAY_CONTEXT: JSON.stringify(relayContext), + }; + it('authorizes using the skill-forwarded relay context when the engine context has no usable item identity', async () => { + expect((await gate.collectImprovementContext(routedEnv, api())).authorized).toBe(true); + }); + it('still requires the engine-derived repo/workflow_id even though item identity comes from the relay', async () => { + for (const mutation of [{ workflow_id: 'forged' }, { repo: 'other/repo' }]) { + const result = await gate.collectImprovementContext({ + ...routedEnv, SQUAD_IMPROVE_AW_CONTEXT: JSON.stringify({ ...relayedOrigin, ...mutation }), + }, api()); + expect(result.reason).toBe('approval-relay-context-invalid'); + } + }); + it.each([ + ['tampered comment_id', { comment_id: '902' }], + ['tampered item_number', { item_number: '78' }], + ['tampered item_type', { item_type: 'pull_request' }], + ['tampered event_type', { event_type: 'workflow_dispatch' }], + ])('rejects a %s in the relay context instead of trusting it blindly', async (_name, mutation) => { + const result = await gate.collectImprovementContext({ + ...routedEnv, SQUAD_IMPROVE_RELAY_CONTEXT: JSON.stringify({ ...relayContext, ...mutation }), + }, api()); + expect(result.authorized).toBe(false); + expect(result.reason).toBe('approval-relay-context-invalid'); + }); + it('rejects malformed or missing relay context instead of falling back to the empty engine item fields', async () => { + for (const bad of [undefined, '', '{not json']) { + const bundle: Record = { ...routedEnv }; + if (bad === undefined) delete bundle.SQUAD_IMPROVE_RELAY_CONTEXT; else bundle.SQUAD_IMPROVE_RELAY_CONTEXT = bad; + const result = await gate.collectImprovementContext(bundle, api()); + expect(result.authorized).toBe(false); + expect(result.reason).toBe('approval-relay-context-invalid'); + } + }); + it('does not regress the native direct-comment path (engine context alone remains authoritative there)', async () => { + const nativeOrigin = { + repo: REPO, workflow_id: `${REPO}/.github/workflows/squad.lock.yml@refs/heads/dev`, + event_type: 'issue_comment', item_type: 'issue', item_number: '77', comment_id: '901', + }; + const nativeEnv = { + ...env(), GITHUB_ACTOR: 'github-actions[bot]', SQUAD_IMPROVE_AW_CONTEXT: JSON.stringify(nativeOrigin), + }; + expect((await gate.collectImprovementContext(nativeEnv, api())).authorized).toBe(true); + }); + it('declares the forwarded relay input on the worker and wires it alongside the existing aw_context at both call sites', () => { + expect(WORKER).toContain('squad_approval_relay:'); + expect((WORKER.match(/SQUAD_IMPROVE_RELAY_CONTEXT: \$\{\{ github\.event\.inputs\.squad_approval_relay \}\}/g) || []).length).toBe(2); + }); + it('strict-compiles squad.md and asserts the dispatch step actually emits squad_approval_relay', () => { + const root = scratch(); + cpSync(resolve(ROOT, 'workflows'), join(root, '.github', 'workflows'), { recursive: true }); + execFileSync('git', ['init', '--quiet'], { cwd: root }); + // CI compiles with --approve after review; the squad-init action here is + // SHA-pinned to this repository and receives no secret input. + execFileSync('gh', ['aw', 'compile', '.github/workflows/squad.md', '--strict', '--approve'], { + cwd: root, encoding: 'utf8', stdio: 'pipe', timeout: 120000, + }); + const lock = readFileSync(join(root, '.github', 'workflows', 'squad.lock.yml'), 'utf8'); + expect(lock).toContain('"squad_approval_relay"'); + }, 120000); +}); + diff --git a/workflows/package/squad-improvement-worker.md b/workflows/package/squad-improvement-worker.md index 7c8f6360e..138e1599c 100644 --- a/workflows/package/squad-improvement-worker.md +++ b/workflows/package/squad-improvement-worker.md @@ -19,6 +19,10 @@ on: description: Relay metadata, never approval evidence required: false type: string + squad_approval_relay: + description: Skill-forwarded origin (event_type/item_type/item_number/comment_id) for the router-relayed path, where this run's own workflow_dispatch trigger carries no native item payload for the engine to derive aw_context from. Never approval evidence; re-verified live. + required: false + type: string permissions: contents: read copilot-requests: write @@ -46,6 +50,7 @@ pre-agent-steps: SQUAD_IMPROVE_ISSUE_NUMBER: ${{ github.event.inputs.issue_number }} SQUAD_IMPROVE_APPROVAL_COMMENT_ID: ${{ github.event.inputs.approval_comment_id }} SQUAD_IMPROVE_AW_CONTEXT: ${{ github.event.inputs.aw_context }} + SQUAD_IMPROVE_RELAY_CONTEXT: ${{ github.event.inputs.squad_approval_relay }} SQUAD_IMPROVE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | set -euo pipefail @@ -69,6 +74,7 @@ safe-outputs: SQUAD_IMPROVE_ISSUE_NUMBER: ${{ github.event.inputs.issue_number }} SQUAD_IMPROVE_APPROVAL_COMMENT_ID: ${{ github.event.inputs.approval_comment_id }} SQUAD_IMPROVE_AW_CONTEXT: ${{ github.event.inputs.aw_context }} + SQUAD_IMPROVE_RELAY_CONTEXT: ${{ github.event.inputs.squad_approval_relay }} SQUAD_IMPROVE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} with: script: | diff --git a/workflows/package/squad.md b/workflows/package/squad.md index 5d1ffa317..3962542f7 100644 --- a/workflows/package/squad.md +++ b/workflows/package/squad.md @@ -2339,14 +2339,25 @@ parsed **Dispatched aw_context** (Trigger Context) relayed by the command router. If neither resolves to a positive integer, refuse and STOP — never dispatch with a guessed, omitted, or placeholder id. Use the typed `dispatch_workflow` safe-output, with nested inputs (never a generic GitHub -mutation): +mutation). Always include `squad_approval_relay` exactly as shown — this run's +own workflow_dispatch trigger carries no native issue/comment payload for +gh-aw's dispatch engine to derive context from when this run was itself +relayed, so the worker-side gate cannot rely on engine-injected `aw_context` +for the item identity in that case and requires this explicit, separately +re-verified echo instead: ```json { "workflow_name": "squad-improvement-worker", "inputs": { "issue_number": "{issue-number}", - "approval_comment_id": "{resolved-approval-comment-id}" + "approval_comment_id": "{resolved-approval-comment-id}", + "squad_approval_relay": { + "event_type": "issue_comment", + "item_type": "issue", + "item_number": "{issue-number}", + "comment_id": "{resolved-approval-comment-id}" + } } } ``` diff --git a/workflows/shared/squad-improvement-gate.mjs b/workflows/shared/squad-improvement-gate.mjs index 0a59f8ef7..d8bf00301 100644 --- a/workflows/shared/squad-improvement-gate.mjs +++ b/workflows/shared/squad-improvement-gate.mjs @@ -273,10 +273,36 @@ export async function collectImprovementContext(env = process.env, { if (env.SQUAD_IMPROVE_AW_CONTEXT) { let origin; try { origin = JSON.parse(env.SQUAD_IMPROVE_AW_CONTEXT); } catch { return refuse('approval-relay-context-invalid'); } + // `repo`/`workflow_id` are populated by gh-aw's dispatch engine from this + // run's own (always-accurate, regardless of triggering event) identity — + // they independently prove "dispatched from squad.lock.yml in this exact + // repo" and are required unconditionally. if (env.GITHUB_ACTOR !== 'github-actions[bot]' || origin.repo !== repository || - origin.workflow_id !== `${repository}/.github/workflows/squad.lock.yml@refs/heads/${env.SQUAD_IMPROVE_DEFAULT_BRANCH}` || - origin.event_type !== 'issue_comment' || origin.item_type !== 'issue' || - origin.item_number !== number || origin.comment_id !== commentId) return refuse('approval-relay-context-invalid'); + origin.workflow_id !== `${repository}/.github/workflows/squad.lock.yml@refs/heads/${env.SQUAD_IMPROVE_DEFAULT_BRANCH}`) { + return refuse('approval-relay-context-invalid'); + } + if (origin.event_type === 'issue_comment') { + // squad.md was triggered directly by the approval comment: gh-aw's engine + // derives item_type/item_number/comment_id from that same real payload, + // so the engine-injected context is itself authoritative here. + if (origin.item_type !== 'issue' || origin.item_number !== number || origin.comment_id !== commentId) { + return refuse('approval-relay-context-invalid'); + } + } else { + // squad.md was relayed here via workflow_dispatch (command-router path): + // its own run has no native issue/comment payload, so gh-aw's engine can + // only report this run's own (workflow_dispatch) event_type and empty + // item fields — never a usable item identity. The skill must instead + // forward its own independently re-verified item identity under a + // distinct input name gh-aw's engine does not auto-populate/override, + // with identical exact-match binding semantics. + let relay; + try { relay = JSON.parse(env.SQUAD_IMPROVE_RELAY_CONTEXT || ''); } catch { return refuse('approval-relay-context-invalid'); } + if (!relay || relay.event_type !== 'issue_comment' || relay.item_type !== 'issue' || + relay.item_number !== number || relay.comment_id !== commentId) { + return refuse('approval-relay-context-invalid'); + } + } } const issue = await fetchJson(`repos/${repository}/issues/${number}`, {}); if (!issue || issue.__status || Number(issue.number) !== Number(number) || issue.pull_request) return refuse('issue-unavailable'); diff --git a/workflows/squad-improvement-worker.md b/workflows/squad-improvement-worker.md index e466b2fd8..54d02f6c2 100644 --- a/workflows/squad-improvement-worker.md +++ b/workflows/squad-improvement-worker.md @@ -19,6 +19,14 @@ on: description: Relay metadata, never approval evidence required: false type: string + squad_approval_relay: + description: >- + Skill-forwarded origin (event_type/item_type/item_number/comment_id) + for the router-relayed path, where this run's own workflow_dispatch + trigger carries no native item payload for the engine to derive + aw_context from. Never approval evidence; re-verified live. + required: false + type: string permissions: contents: read copilot-requests: write @@ -47,6 +55,7 @@ pre-agent-steps: SQUAD_IMPROVE_ISSUE_NUMBER: ${{ github.event.inputs.issue_number }} SQUAD_IMPROVE_APPROVAL_COMMENT_ID: ${{ github.event.inputs.approval_comment_id }} SQUAD_IMPROVE_AW_CONTEXT: ${{ github.event.inputs.aw_context }} + SQUAD_IMPROVE_RELAY_CONTEXT: ${{ github.event.inputs.squad_approval_relay }} SQUAD_IMPROVE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | set -euo pipefail @@ -70,6 +79,7 @@ safe-outputs: SQUAD_IMPROVE_ISSUE_NUMBER: ${{ github.event.inputs.issue_number }} SQUAD_IMPROVE_APPROVAL_COMMENT_ID: ${{ github.event.inputs.approval_comment_id }} SQUAD_IMPROVE_AW_CONTEXT: ${{ github.event.inputs.aw_context }} + SQUAD_IMPROVE_RELAY_CONTEXT: ${{ github.event.inputs.squad_approval_relay }} SQUAD_IMPROVE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} with: script: | diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index 9f6b86909..834afc73a 100644 --- a/workflows/squad-workflows.manifest.json +++ b/workflows/squad-workflows.manifest.json @@ -14,9 +14,9 @@ "source": "workflows/package/squad.md", "destination": ".github/workflows/squad.md", "lock": ".github/workflows/squad.lock.yml", - "source_sha256": "b237872416c4e1357ed8136e9c3444cbcae1a97a2458bff81af9a4f7087bc40b", - "lock_sha256": "9d44255bcccd8cb114c57b65fe2e2ecf582a378f2ea21191d27321f2525f8886", - "package_lock_sha256": "3f632c603377d31c7157bbd4bde08efd37da85098c2a163e27b55ddcfae1fe72" + "source_sha256": "ccd6d2cf4ac9cfcae8862d1ecb1e324c1be10000ecd4eac9124707d119be3c2c", + "lock_sha256": "f4d27d360a96d3a83d2d2d0bdb79fc128b6e40a80cc2cb25b9cd79b9ed472016", + "package_lock_sha256": "a901cbeffc005693572d5910a15dd30ab28f08ee2c6bebb28a01b6eec0fe8517" }, { "name": "squad-implement-worker", @@ -59,9 +59,9 @@ "source": "workflows/package/squad-improvement-worker.md", "destination": ".github/workflows/squad-improvement-worker.md", "lock": ".github/workflows/squad-improvement-worker.lock.yml", - "source_sha256": "01c89f8890f02c0ca884513aa10225787feaf3606ab604d5c0e85fbbccdb6c95", - "lock_sha256": "d5389a54f3c99f8fb01223dc04ea66bb59c32769d1d45b730a35275905c26829", - "package_lock_sha256": "3ea27b34eba4685735b3ff9d26175c49d1fc290ef94b947420f2c0378014cf03" + "source_sha256": "89896bc4321f76e791d40ee8239ba7ed6a2b8e090113a1cdb3017cb2a68e5c90", + "lock_sha256": "65d905f43b88b2dffaa8bf90c8fd9f4d10ac2a3db3789a439ec998a2e5ad84d1", + "package_lock_sha256": "caf333235528b716c1177684be94752e56a288a5dc87723b6abcf99074d388d1" }, { "name": "squad-bootstrap", @@ -129,7 +129,7 @@ "package_destination": ".github/workflows/shared/squad-improvement-gate.mjs", "destination": ".github/workflows/shared/squad-improvement-gate.mjs", "ownership": "manifest", - "sha256": "b8e27c9311a24ce448ff34d5bae5c50947659c01acc3704dfde6dc6ca6d2a042" + "sha256": "306324509ef0fb31b645cd92f4085f3d3776021f149783908f48a3eb89038228" }, { "path": "shared/squad-retro-evidence.mjs", diff --git a/workflows/squad.md b/workflows/squad.md index 9279333c1..62683f7a9 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -1194,14 +1194,25 @@ parsed **Dispatched aw_context** (Trigger Context) relayed by the command router. If neither resolves to a positive integer, refuse and STOP — never dispatch with a guessed, omitted, or placeholder id. Use the typed `dispatch_workflow` safe-output, with nested inputs (never a generic GitHub -mutation): +mutation). Always include `squad_approval_relay` exactly as shown — this run's +own workflow_dispatch trigger carries no native issue/comment payload for +gh-aw's dispatch engine to derive context from when this run was itself +relayed, so the worker-side gate cannot rely on engine-injected `aw_context` +for the item identity in that case and requires this explicit, separately +re-verified echo instead: ```json { "workflow_name": "squad-improvement-worker", "inputs": { "issue_number": "{issue-number}", - "approval_comment_id": "{resolved-approval-comment-id}" + "approval_comment_id": "{resolved-approval-comment-id}", + "squad_approval_relay": { + "event_type": "issue_comment", + "item_type": "issue", + "item_number": "{issue-number}", + "comment_id": "{resolved-approval-comment-id}" + } } } ``` From 90306a9ab0ff25c0159a1648685f99e0ba139895 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Thu, 1 Oct 2026 13:57:35 -0700 Subject: [PATCH 4/4] test(gh-aw): update stale compiled-lock golden digests after relay-context fix Updating workflows/squad.md's squad-approve-improvement dispatch payload (and squad-improvement-worker.md's new input/env wiring) changes the compiled squad.lock.yml / squad-improvement-worker.lock.yml content, which shifts the normalized SHA-256 digests asserted by the canonical package integrity contract tests in test/gh-aw-quality.test.ts (tamper-mutation fixtures recomputed over the new baseline compiled output). Re-derived both digest pairs from the actual compiled output (workflow and package source bindings) and updated the three assertions that pinned the stale values. No test logic changed; this is a golden-value refresh consistent with the same pattern applied for the three originally-scoped fixes earlier in this PR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- test/gh-aw-quality.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index 5d18e083a..8e9aff493 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -3961,8 +3961,8 @@ describe('gh-aw: canonical package integrity contract', () => { expect(mutable).not.toContain(actionReference); expect(createHash('sha256').update(normalizeCompiledLock(mutable, revisionA)).digest('hex')) .toBe(sourceBinding === 'workflow' - ? '518b6688093479b51a1b4d170c56bd0fc0317370bcf50cedcaa6d48bd04a5fc0' - : 'dca7fe0dee3fd12ffaab3a26fa28ab02c7e90f7862f6e6814d503e729f52e18a'); + ? 'abcd7eaf3570f2bd9b90e5113b52d37416723a197a9edbfc8b3ef66c2c2d5edc' + : 'b4519c9cb86c61400a5168ebf50738455578709e6d9bb8d13993e787cf03260b'); expect(() => validateCompilerActionPins(mutable)).toThrow(/invalid immutable action pin/); writeFileSync(lockPath, mutable); expect(verifyInstall(root).failures.join('\n')) @@ -4016,7 +4016,7 @@ describe('gh-aw: canonical package integrity contract', () => { compile(); const unpinned = readText(lockPath); expect(createHash('sha256').update(normalizeCompiledLock(unpinned, revisionA)).digest('hex')) - .toBe('dca7fe0dee3fd12ffaab3a26fa28ab02c7e90f7862f6e6814d503e729f52e18a'); + .toBe('b4519c9cb86c61400a5168ebf50738455578709e6d9bb8d13993e787cf03260b'); expect(verifyInstall(root).failures.join('\n')).toContain('Installed digest mismatch'); const seedPins = () => spawnSync(process.execPath, ['--input-type=module', '-e', seed!], {