From f15a2ea801c0d37a0594b84be2d45bc28bf90dc2 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Wed, 2 Sep 2026 22:15:04 -0700 Subject: [PATCH 1/2] gh-aw: enforce byte-identical built-in charters in Cast validator Strengthen the GH-AW Cast validator so it fails Cast validation unless every final .squad/agents/{scribe,ralph,rai,fact-checker}/charter.md is byte-for-byte identical to its canonical resource under .github/workflows/shared/builtins/{id}-charter.md. Previously the validator only checked that the four built-in directories existed and were referenced correctly, not that their content survived intact through a squad init run or agent rewrite after the deterministic materialization step. - Add validateBuiltinCharterFidelity to workflows/shared/squad-cast-validator.mjs, mirrored verbatim to scripts/validate-gh-aw-cast.mjs. - Update the validator SHA-256 pin in workflows/squad.md for the new validator bytes. - Update test/gh-aw-cast-validator.test.ts fixtures to materialize real canonical built-in charter content (read from workflows/shared/builtins/) instead of placeholders, and add focused tests covering byte-mismatch rejection, missing canonical resource, and byte-identical acceptance. - Update the hardcoded SHA-256 expectation in test/gh-aw-quality.test.ts. Validated: targeted vitest run (226 passed), full gh-aw-*.test.ts run (699 passed, 1 pre-existing unrelated failure from an unbuilt SDK workspace package), and gh aw compile --strict for all four Squad workflows using the pinned v0.87.10 compiler via the repo's CI-mirroring recipe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- scripts/validate-gh-aw-cast.mjs | 47 +++++++++++++++++ test/gh-aw-cast-validator.test.ts | 61 +++++++++++++++++++++-- test/gh-aw-quality.test.ts | 2 +- workflows/shared/squad-cast-validator.mjs | 47 +++++++++++++++++ workflows/squad.md | 2 +- 5 files changed, 154 insertions(+), 5 deletions(-) diff --git a/scripts/validate-gh-aw-cast.mjs b/scripts/validate-gh-aw-cast.mjs index aea594a72..ff02ea152 100644 --- a/scripts/validate-gh-aw-cast.mjs +++ b/scripts/validate-gh-aw-cast.mjs @@ -32,6 +32,12 @@ const BUILTIN_DISPLAY_NAMES = { const REQUIRED_BUILTIN_CHARTERS = REQUIRED_BUILTIN_IDS .map((id) => `.squad/agents/${id}/charter.md`) .sort(); +// The canonical resource a gh-aw "Materialize canonical built-in support +// agents" step copies from verbatim before the agent ever runs. The final +// emitted `.squad/agents/{id}/charter.md` must remain byte-identical to it — +// this is the only way to prove a `squad init` or agent rewrite never +// clobbered the materialized built-in after the deterministic copy step. +const BUILTIN_CANONICAL_DIR = '.github/workflows/shared/builtins'; const BUILTIN_SECTION_HEADING = '## Built-in Support Agents'; const CAST_SOURCES_HEADING = '## Cast sources'; const BUILTIN_NAME_ROW_PATTERN = /^\|\s*(Scribe|Ralph|Rai|Fact Checker)\s*\|/gmi; @@ -69,6 +75,45 @@ function readText(root, relativePath) { return readFileSync(join(root, ...relativePath.split('/')), 'utf8').replace(/\r\n/g, '\n'); } +/** Raw file bytes, with no text normalization, for true byte-for-byte comparison. */ +function readBytes(root, relativePath) { + return readFileSync(join(root, ...relativePath.split('/'))); +} + +/** + * Every materialized built-in charter must remain byte-for-byte identical to + * the canonical resource shipped with the workflow. A deterministic step + * copies the canonical resource verbatim before the agent runs; if `squad + * init` or the Cast agent later rewrites, paraphrases, or reinterprets one of + * these files, this is the only check that catches the divergence. + */ +function validateBuiltinCharterFidelity(root, errors) { + for (const id of REQUIRED_BUILTIN_IDS) { + const canonicalPath = `${BUILTIN_CANONICAL_DIR}/${id}-charter.md`; + const materializedPath = `.squad/agents/${id}/charter.md`; + let canonicalBytes; + try { + canonicalBytes = readBytes(root, canonicalPath); + } catch (error) { + errors.push(`builtin: canonical resource ${canonicalPath} for "${id}" is missing or unreadable (${error.message})`); + continue; + } + let materializedBytes; + try { + materializedBytes = readBytes(root, materializedPath); + } catch (error) { + errors.push(`builtin: materialized charter ${materializedPath} for "${id}" is missing or unreadable (${error.message})`); + continue; + } + if (!canonicalBytes.equals(materializedBytes)) { + errors.push( + `builtin: ${materializedPath} is not byte-identical to the canonical resource ` + + `${canonicalPath} — built-in charters must never be regenerated, edited, or reinterpreted`, + ); + } + } +} + function normalizePayloadPath(value) { if ( typeof value !== 'string' @@ -333,6 +378,8 @@ export function validateCastTree({ root, payloadPath }) { } } + validateBuiltinCharterFidelity(root, errors); + let team = ''; let routing = ''; let coordinator = ''; diff --git a/test/gh-aw-cast-validator.test.ts b/test/gh-aw-cast-validator.test.ts index ef6d3e316..64106e392 100644 --- a/test/gh-aw-cast-validator.test.ts +++ b/test/gh-aw-cast-validator.test.ts @@ -26,6 +26,13 @@ const builtins = [ { id: 'fact-checker', name: 'Fact Checker' }, ]; +/** Byte-for-byte canonical content of a built-in charter, as shipped with the workflow. */ +function builtinCanonicalContent(id: string): Buffer { + return readFileSync(join(process.cwd(), 'workflows', 'shared', 'builtins', `${id}-charter.md`)); +} + +const builtinCanonicalRelativePath = '.github/workflows/shared/builtins'; + const corePayload = [ '.squad/team.md', '.squad/routing.md', @@ -38,7 +45,7 @@ const corePayload = [ 'meet-the-squad.md', ]; -function write(root: string, path: string, content: string): void { +function write(root: string, path: string, content: string | Buffer): void { const fullPath = join(root, ...path.split('/')); mkdirSync(dirname(fullPath), { recursive: true }); writeFileSync(fullPath, content, 'utf8'); @@ -171,7 +178,12 @@ function createFixture(): { root: string; payload: string; runnerTemp: string } write(root, `.squad/agents/${member.id}/charter.md`, `# ${member.name} — ${member.role}\n`); } for (const builtin of builtins) { - write(root, `.squad/agents/${builtin.id}/charter.md`, `# ${builtin.name}\n`); + const canonical = builtinCanonicalContent(builtin.id); + // Mirrors the gh-aw resource materialization: the canonical charter is + // installed under .github/workflows/shared/builtins/, then copied + // verbatim to .squad/agents/{id}/charter.md by a deterministic step. + write(root, `${builtinCanonicalRelativePath}/${builtin.id}-charter.md`, canonical); + write(root, `.squad/agents/${builtin.id}/charter.md`, canonical); } write(root, '.github/agents/squad.agent.md', coordinatorMarkdown()); write(root, 'meet-the-squad.md', '# Meet the Squad\n'); @@ -417,7 +429,7 @@ describe('GH-AW Cast final-tree validator', () => { const result = runValidatorCommand(fixture); expect(result.status).not.toBe(0); expect(result.stderr).toMatch( - /Cast validator SHA-256 mismatch: expected 5b8e1432de63c78488ee7ff6da4e67ea0c91044582109f678153c231704a0097, got [a-f0-9]{64}\./, + /Cast validator SHA-256 mismatch: expected f0c79694d9832c53070f059d4bff181a8ccd857e1be49d24b8d5b72ed8887251, got [a-f0-9]{64}\./, ); expect(result.stdout).not.toContain('Cast validation passed.'); expect(authorizesPullRequest(result)).toBe(false); @@ -625,6 +637,49 @@ describe('GH-AW Cast final-tree validator', () => { expect(result.stderr).toContain('watcher'); }); + it('rejects a built-in charter that has been regenerated or edited (byte mismatch vs. canonical resource)', () => { + const fixture = createFixture(); + write( + fixture.root, + '.squad/agents/ralph/charter.md', + `${builtinCanonicalContent('ralph').toString('utf8')}\n\n`, + ); + const result = validate(fixture.root, fixture.payload); + expect(result.status).toBe(1); + expect(result.stderr).toMatch( + /builtin: \.squad\/agents\/ralph\/charter\.md is not byte-identical to the canonical resource \.github\/workflows\/shared\/builtins\/ralph-charter\.md/i, + ); + }); + + it('rejects a built-in charter with a trivial byte-level divergence (trailing newline) from the canonical resource', () => { + const fixture = createFixture(); + write(fixture.root, '.squad/agents/scribe/charter.md', `${builtinCanonicalContent('scribe').toString('utf8')}\n`); + const result = validate(fixture.root, fixture.payload); + expect(result.status).toBe(1); + expect(result.stderr).toMatch(/builtin: \.squad\/agents\/scribe\/charter\.md is not byte-identical/i); + }); + + it('rejects a Cast tree whose canonical built-in resource is missing from .github/workflows/shared/builtins', () => { + const fixture = createFixture(); + rmSync(join(fixture.root, builtinCanonicalRelativePath, 'fact-checker-charter.md'), { force: true }); + const result = validate(fixture.root, fixture.payload); + expect(result.status).toBe(1); + expect(result.stderr).toMatch( + /builtin: canonical resource \.github\/workflows\/shared\/builtins\/fact-checker-charter\.md for "fact-checker" is missing or unreadable/i, + ); + }); + + it('accepts a built-in charter that is byte-identical to the canonical resource for all four built-ins', () => { + const fixture = createFixture(); + const result = validate(fixture.root, fixture.payload); + expect(result.status, result.stderr).toBe(0); + for (const builtin of builtins) { + expect( + readFileSync(join(fixture.root, '.squad', 'agents', builtin.id, 'charter.md')), + ).toEqual(builtinCanonicalContent(builtin.id)); + } + }); + it('rejects a built-in registered as an active specialist in the casting registry', () => { const fixture = createFixture(); write(fixture.root, '.squad/casting/registry.json', JSON.stringify({ diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index ef0fb0d1b..eddccbaff 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -1506,7 +1506,7 @@ describe('gh-aw: compiled workflow shell input security contract', () => { '--payload "${GITHUB_WORKSPACE:?}/.github/workflows/squad-cast-payload.json"', ); expect(normalizedRunnerStep).not.toContain('RUNNER_TEMP'); - expect(normalizedRunnerStep).toContain('validator_expected_sha256="5b8e1432de63c78488ee7ff6da4e67ea0c91044582109f678153c231704a0097"'); + expect(normalizedRunnerStep).toContain('validator_expected_sha256="f0c79694d9832c53070f059d4bff181a8ccd857e1be49d24b8d5b72ed8887251"'); expect(normalizedRunnerStep).toContain("outcome: 'cast_failure'"); expect(normalizedRunnerStep).toContain('chmod 500 "$validator_runner"'); expect(compiled.indexOf('name: Prepare deterministic Cast validator runner')).toBeLessThan( diff --git a/workflows/shared/squad-cast-validator.mjs b/workflows/shared/squad-cast-validator.mjs index aea594a72..ff02ea152 100644 --- a/workflows/shared/squad-cast-validator.mjs +++ b/workflows/shared/squad-cast-validator.mjs @@ -32,6 +32,12 @@ const BUILTIN_DISPLAY_NAMES = { const REQUIRED_BUILTIN_CHARTERS = REQUIRED_BUILTIN_IDS .map((id) => `.squad/agents/${id}/charter.md`) .sort(); +// The canonical resource a gh-aw "Materialize canonical built-in support +// agents" step copies from verbatim before the agent ever runs. The final +// emitted `.squad/agents/{id}/charter.md` must remain byte-identical to it — +// this is the only way to prove a `squad init` or agent rewrite never +// clobbered the materialized built-in after the deterministic copy step. +const BUILTIN_CANONICAL_DIR = '.github/workflows/shared/builtins'; const BUILTIN_SECTION_HEADING = '## Built-in Support Agents'; const CAST_SOURCES_HEADING = '## Cast sources'; const BUILTIN_NAME_ROW_PATTERN = /^\|\s*(Scribe|Ralph|Rai|Fact Checker)\s*\|/gmi; @@ -69,6 +75,45 @@ function readText(root, relativePath) { return readFileSync(join(root, ...relativePath.split('/')), 'utf8').replace(/\r\n/g, '\n'); } +/** Raw file bytes, with no text normalization, for true byte-for-byte comparison. */ +function readBytes(root, relativePath) { + return readFileSync(join(root, ...relativePath.split('/'))); +} + +/** + * Every materialized built-in charter must remain byte-for-byte identical to + * the canonical resource shipped with the workflow. A deterministic step + * copies the canonical resource verbatim before the agent runs; if `squad + * init` or the Cast agent later rewrites, paraphrases, or reinterprets one of + * these files, this is the only check that catches the divergence. + */ +function validateBuiltinCharterFidelity(root, errors) { + for (const id of REQUIRED_BUILTIN_IDS) { + const canonicalPath = `${BUILTIN_CANONICAL_DIR}/${id}-charter.md`; + const materializedPath = `.squad/agents/${id}/charter.md`; + let canonicalBytes; + try { + canonicalBytes = readBytes(root, canonicalPath); + } catch (error) { + errors.push(`builtin: canonical resource ${canonicalPath} for "${id}" is missing or unreadable (${error.message})`); + continue; + } + let materializedBytes; + try { + materializedBytes = readBytes(root, materializedPath); + } catch (error) { + errors.push(`builtin: materialized charter ${materializedPath} for "${id}" is missing or unreadable (${error.message})`); + continue; + } + if (!canonicalBytes.equals(materializedBytes)) { + errors.push( + `builtin: ${materializedPath} is not byte-identical to the canonical resource ` + + `${canonicalPath} — built-in charters must never be regenerated, edited, or reinterpreted`, + ); + } + } +} + function normalizePayloadPath(value) { if ( typeof value !== 'string' @@ -333,6 +378,8 @@ export function validateCastTree({ root, payloadPath }) { } } + validateBuiltinCharterFidelity(root, errors); + let team = ''; let routing = ''; let coordinator = ''; diff --git a/workflows/squad.md b/workflows/squad.md index 7cae87d25..e0d1c19bd 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -121,7 +121,7 @@ steps: fi validator_script="$(cd "$(dirname "$validator_script")" && pwd -P)/$(basename "$validator_script")" - validator_expected_sha256="5b8e1432de63c78488ee7ff6da4e67ea0c91044582109f678153c231704a0097" + validator_expected_sha256="f0c79694d9832c53070f059d4bff181a8ccd857e1be49d24b8d5b72ed8887251" : > "$stderr_file" validator_actual_sha256="$( node -e 'const c=require("node:crypto"),f=require("node:fs");process.stdout.write(c.createHash("sha256").update(f.readFileSync(process.argv[1])).digest("hex"))' \ From 4ec1793d852783f539675c61a57183af21224d66 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Wed, 2 Sep 2026 22:25:47 -0700 Subject: [PATCH 2/2] gh-aw: run built-in charter materialization as a pre-agent-step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the residual clobbering path the byte-fidelity validator (previous commit) could only detect, not prevent: gh-aw's native "Restore agent config folders from base branch" and the (unconditional, no runtime if:-guard) repeated "Restore ambient folders from activation artifact" step run AFTER our old steps: placement, and can reintroduce a stale committed .squad/ snapshot over the freshly materialized built-in charters whenever this workflow's pull_request_comment trigger causes checkout-pr to succeed and a stale .squad/ already exists on the relevant ref (matching the octodemo/zava-social-squad-backend#13 scenario, and the residual gap identified in review of PR #1992). Root-cause detail confirmed empirically against the compiled lock file (pkg/workflow/compiler_yaml_ai_execution.go in github/gh-aw): custom steps: are always spliced in early (right after the activation-artifact download), while base-branch restore, both ambient-folder restores, and inline sub-agent/skill restore are compiler-injected later in the same job — before pre-agent-steps, MCP setup, and Copilot CLI execution. The existing steps: mechanism was structurally unable to run after those restores; pre-agent-steps is the documented gh-aw hook designed for exactly this ordering need. - workflows/squad.md: move "Materialize canonical built-in support agents" and "Prepare deterministic Cast validator runner" from steps: to pre-agent-steps:. Verified via `gh aw compile --strict` that both now execute after every restore step and before MCP setup / "Execute GitHub Copilot CLI". - test/gh-aw-quality.test.ts: replace the now-inverted ordering assertion (validator-runner used to run before "Restore inline skills"; it now correctly runs after every restore and before agent execution) and add a new focused test asserting the full ordering invariant end-to-end: activation job (squad init) precedes the agent job; materialization runs after init and after every restore step (Squad state, base-branch, both ambient-folder occurrences, inline sub-agents, inline skills); and materialization precedes "Execute GitHub Copilot CLI". Validated with the mandatory npm proxy set before every npm/npx invocation: - `npx vitest run test/gh-aw-cast-validator.test.ts test/gh-aw-quality.test.ts` -> 227/227 passed - `npx vitest run test/gh-aw-*.test.ts` -> 700/700 passed (1 pre-existing unrelated failure from an unbuilt SDK workspace package, same as before this change and on dev) - `gh aw compile --strict` for all four Squad workflows using the pinned v0.87.10 compiler via the repo's CI-mirroring recipe — all 4 succeeded - Confirmed step order in the compiled squad.lock.yml directly: Restore agent config folders from base branch -> Restore ambient folders from activation artifact (2nd occurrence) -> Restore inline sub-agents -> Restore inline skills -> Materialize canonical built-in support agents -> Prepare deterministic Cast validator runner -> Start MCP Gateway -> Execute GitHub Copilot CLI Only the already-authorized workflow/validator/test files were touched; no changeset added (workflow/test-only scope is exempt from the changelog gate's SDK/CLI path regex). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- test/gh-aw-quality.test.ts | 55 +++++++++++++++++++++++++++++++++++++- workflows/squad.md | 6 ++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index eddccbaff..ecf70cf15 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -1509,11 +1509,64 @@ describe('gh-aw: compiled workflow shell input security contract', () => { expect(normalizedRunnerStep).toContain('validator_expected_sha256="f0c79694d9832c53070f059d4bff181a8ccd857e1be49d24b8d5b72ed8887251"'); expect(normalizedRunnerStep).toContain("outcome: 'cast_failure'"); expect(normalizedRunnerStep).toContain('chmod 500 "$validator_runner"'); + // Prepared as a pre-agent-step (see the built-in fidelity ordering test below): + // it must run AFTER the base-branch/ambient restores that can reintroduce a + // stale committed .squad snapshot, and still before the agent turn begins. + expect(compiled.indexOf('name: Restore inline skills from activation artifact')).toBeLessThan( + compiled.indexOf('name: Prepare deterministic Cast validator runner'), + ); expect(compiled.indexOf('name: Prepare deterministic Cast validator runner')).toBeLessThan( - compiled.indexOf('name: Restore inline skills from activation artifact'), + compiled.indexOf('name: Execute GitHub Copilot CLI'), ); }, 20000); + it('materializes built-in charters after init and every late restore, and before the Cast agent turn', () => { + const compiled = lockText(); + + // Job-level guarantee: the agent job cannot start until the activation + // job (which runs `squad init --preset default` when no cast exists yet) + // has completed. + expect(compiled).toMatch(/\n {2}agent:\n {4}needs: activation\n/); + const activationJobIndex = compiled.indexOf('\n activation:\n'); + const agentJobIndex = compiled.indexOf('\n agent:\n'); + expect(activationJobIndex).toBeGreaterThan(-1); + expect(agentJobIndex).toBeGreaterThan(activationJobIndex); + + const initIndex = compiled.indexOf('name: Initialize Squad team'); + const materializeIndex = compiled.indexOf('name: Materialize canonical built-in support agents'); + expect(initIndex).toBeGreaterThan(-1); + expect(materializeIndex).toBeGreaterThan(initIndex); + + // Materialization is a pre-agent-step: it must run strictly after every + // restore step that can reintroduce a stale committed `.squad/` snapshot + // late in the job (base-branch restore, both ambient-folder restores, + // inline sub-agent/skill restores), and strictly before the Cast agent + // turn (Copilot CLI execution) starts. + for (const priorRestore of [ + 'name: Restore Squad state from activation artifact', + 'name: Restore agent config folders from base branch', + 'name: Restore inline sub-agents from activation artifact', + 'name: Restore inline skills from activation artifact', + ]) { + const restoreIndex = compiled.indexOf(priorRestore); + expect(restoreIndex, `${priorRestore} must be present`).toBeGreaterThan(-1); + expect(materializeIndex, `${priorRestore} must precede materialization`).toBeGreaterThan(restoreIndex); + } + // Both occurrences of the unconditional ambient-folder restore (gh-aw emits + // it once after the activation-artifact download and again after PR + // checkout) must precede materialization too. + const ambientRestoreIndices = [...compiled.matchAll(/name: Restore ambient folders from activation artifact/g)] + .map((match) => match.index ?? -1); + expect(ambientRestoreIndices.length).toBeGreaterThanOrEqual(2); + for (const restoreIndex of ambientRestoreIndices) { + expect(materializeIndex).toBeGreaterThan(restoreIndex); + } + + const agentExecutionIndex = compiled.indexOf('name: Execute GitHub Copilot CLI'); + expect(agentExecutionIndex).toBeGreaterThan(-1); + expect(agentExecutionIndex).toBeGreaterThan(materializeIndex); + }, 20000); + it('keeps the v0.87.10 completion hook neutral when a custom safe job fails', () => { const compiled = lockText(); const completionStep = compiled.match( diff --git a/workflows/squad.md b/workflows/squad.md index e0d1c19bd..44427433e 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -54,7 +54,11 @@ tools: github: mode: gh-proxy toolsets: [default] -steps: +# pre-agent-steps (not steps:): runs after gh-aw's native base-branch/ambient +# restores that can reintroduce a stale committed .squad/ snapshot late in the +# job, so this stays the last writer of the four built-in charters before the +# agent turn begins. +pre-agent-steps: - name: Materialize canonical built-in support agents shell: bash run: |