diff --git a/.changeset/harden-bootstrap-provenance.md b/.changeset/harden-bootstrap-provenance.md new file mode 100644 index 000000000..098a96dd6 --- /dev/null +++ b/.changeset/harden-bootstrap-provenance.md @@ -0,0 +1,8 @@ +--- +"@bradygaster/squad-cli": patch +"@bradygaster/squad-sdk": patch +--- + +Harden Squad Bootstrap provenance by requiring the exact default branch ref and +allowing trusted install commits to remain valid only while they are identical +to or ancestors of the live default branch tip. diff --git a/test/gh-aw-bootstrap-workflow.test.ts b/test/gh-aw-bootstrap-workflow.test.ts index a417041d4..1b6255420 100644 --- a/test/gh-aw-bootstrap-workflow.test.ts +++ b/test/gh-aw-bootstrap-workflow.test.ts @@ -46,6 +46,8 @@ import { const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); const WORKFLOW = readFileSync(resolve(ROOT, 'workflows/squad-bootstrap.md'), 'utf8').replace(/\r\n/g, '\n'); const VALIDATOR = resolve(ROOT, 'workflows/shared/squad-bootstrap-validator.mjs'); +const DEFAULT_BRANCH_REF_CONDITION = + "github.ref == format('refs/heads/{0}', github.event.repository.default_branch)"; const workspaces: string[] = []; const active = [ { id: 'lead', name: 'Lead', role: 'Technical Lead' }, @@ -446,6 +448,24 @@ function compileWorkflow(source = WORKFLOW): string { return readFileSync(join(workflowDir, 'squad-bootstrap.lock.yml'), 'utf8'); } +function evaluateDefaultBranchCondition( + condition: string, + event: { + event_name: 'push' | 'workflow_dispatch'; + ref: string; + ref_name: string; + repository: { default_branch: string }; + }, +): boolean { + if (condition === DEFAULT_BRANCH_REF_CONDITION) { + return event.ref === `refs/heads/${event.repository.default_branch}`; + } + if (condition === 'github.ref_name == github.event.repository.default_branch') { + return event.ref_name === event.repository.default_branch; + } + throw new Error(`Unsupported bootstrap condition: ${condition}`); +} + function materializeCandidatePayloadFromWorkflow( source: string, candidate: string, @@ -920,7 +940,7 @@ describe('automatic Squad bootstrap workflow', () => { expect(lock).toContain('branches:\n - "**"'); expect(lock).toContain('.github/workflows/squad-bootstrap.md'); expect(lock).toContain('.github/workflows/squad-bootstrap.lock.yml'); - expect(lock).toContain("github.ref_name == github.event.repository.default_branch"); + expect(lock).toContain(DEFAULT_BRANCH_REF_CONDITION); expect(lock).toContain('group: squad-bootstrap-${{ github.repository }}'); expect(lock).toContain('cancel-in-progress: false'); expect(lock).toMatch(/agent:[\s\S]*?permissions:\n\s+contents: read\n\s+copilot-requests: write\n\s+issues: read\n\s+pull-requests: read/); @@ -965,6 +985,24 @@ describe('automatic Squad bootstrap workflow', () => { expect(lock).not.toMatch(/\$\{\{[^}]*\\u00(?:26|3[cCeE])/); }, 180000); + it.each([ + ['default-branch push', 'push', 'refs/heads/main', 'main', true], + ['default-branch workflow dispatch', 'workflow_dispatch', 'refs/heads/main', 'main', true], + ['same-named tag workflow dispatch', 'workflow_dispatch', 'refs/tags/main', 'main', false], + ['non-default branch workflow dispatch', 'workflow_dispatch', 'refs/heads/release', 'release', false], + ])('applies the exact default branch ref gate for %s', (_label, eventName, ref, refName, expected) => { + const frontmatter = parse(WORKFLOW.split('---')[1]); + const condition = frontmatter.if; + expect(Object.hasOwn(frontmatter.on, eventName)).toBe(true); + expect(condition).toBe(DEFAULT_BRANCH_REF_CONDITION); + expect(evaluateDefaultBranchCondition(condition, { + event_name: eventName as 'push' | 'workflow_dispatch', + ref, + ref_name: refName, + repository: { default_branch: 'main' }, + })).toBe(expected); + }); + it('rejects a realistic source mutation that writes to the checkout before validation', () => { const assertValidationBeforeCheckoutWrite = (source: string) => { const validation = source.indexOf("validate(payload, 'placeholder')"); @@ -989,15 +1027,30 @@ describe('automatic Squad bootstrap workflow', () => { it('detects a realistic compiled-lock mutation that removes the default-branch gate', () => { const mutated = WORKFLOW.replace( - "if: github.ref_name == github.event.repository.default_branch\n", + `if: ${DEFAULT_BRANCH_REF_CONDITION}\n`, '', ); const lock = compileWorkflow(mutated); expect(lock).not.toContain( - "needs.pre_activation.outputs.activated == 'true' && (github.ref_name == github.event.repository.default_branch)", + `needs.pre_activation.outputs.activated == 'true' && (${DEFAULT_BRANCH_REF_CONDITION})`, ); }, 180000); + it('kills the ref-name mutation that would accept a same-named tag', () => { + const unsafeCondition = 'github.ref_name == github.event.repository.default_branch'; + const tagEvent = { + event_name: 'workflow_dispatch' as const, + ref: 'refs/tags/main', + ref_name: 'main', + repository: { default_branch: 'main' }, + }; + expect(evaluateDefaultBranchCondition(DEFAULT_BRANCH_REF_CONDITION, tagEvent)).toBe(false); + expect(evaluateDefaultBranchCondition(unsafeCondition, tagEvent)).toBe(true); + const mutated = WORKFLOW.replace(DEFAULT_BRANCH_REF_CONDITION, unsafeCondition); + expect(parse(mutated.split('---')[1]).if).toBe(unsafeCondition); + expect(compileWorkflow(mutated)).toContain(unsafeCondition); + }, 180000); + it('detects a compiled mutation that removes the file-backed safe-output CLI', () => { const assertMounted = (lock: string) => { const agent = parse(lock).jobs.agent; @@ -1038,7 +1091,7 @@ describe('automatic Squad bootstrap workflow', () => { // `actions/github-script` runs it, not a reimplementation of it. describe('gh-aw: squad-bootstrap pull-request-creation permission-denied fallback', () => { // context.sha inside this run's source is always the default branch's current commit (the - // workflow's own top-level `if: github.ref_name == github.event.repository.default_branch` + // workflow's own exact default-branch ref gate // gate guarantees this). Distinct from the Cast-branch head SHAs used elsewhere in this harness. const BASE_SHA = 'c'.repeat(40); diff --git a/test/gh-aw-review-guard.test.ts b/test/gh-aw-review-guard.test.ts index d7c090c95..b61aa2fed 100644 --- a/test/gh-aw-review-guard.test.ts +++ b/test/gh-aw-review-guard.test.ts @@ -109,6 +109,7 @@ function fixture(relay = false) { extraRuns: new Map>(), compareStatus: 'diverged' as 'identical' | 'ahead' | 'behind' | 'diverged', compareThrows: 0 as number, + compareOmitStatus: false, compareOmitBaseCommit: false, compareOmitMergeBaseCommit: false, compareBaseCommitShaOverride: undefined as string | undefined, @@ -138,7 +139,8 @@ function fixture(relay = false) { if (state.compareThrows) throw Object.assign(new Error('API error'), { status: state.compareThrows }); const status = baseSha === headSha ? 'identical' : state.compareStatus; const resolvedBaseSha = state.compareBaseCommitShaOverride ?? baseSha; - const result: Record = { status }; + const result: Record = {}; + if (!state.compareOmitStatus) result.status = status; if (!state.compareOmitBaseCommit) result.base_commit = { sha: resolvedBaseSha }; if (!state.compareOmitMergeBaseCommit) { result.merge_base_commit = { sha: state.compareMergeBaseShaOverride ?? resolvedBaseSha }; @@ -364,6 +366,75 @@ describe('independent Squad review guard', () => { .rejects.toThrow('base-controlled bootstrap run did not complete successfully'); }); + it('accepts bot-authored bootstrap provenance when the install commit is identical to the live base', async () => { + const f = fixture(); + makeBootstrap(f); + await expect(reviewTarget(f.env, f.get)).resolves.toMatchObject({ + author_agent: '@squad/base-controlled-bootstrap', + }); + expect(f.state.calls).toContain(`repos/${REPOSITORY}/compare/${BASE}...${BASE}`); + }); + + it('accepts bot-authored bootstrap provenance when the default branch fast-forwarded after installation', async () => { + const f = fixture(); + makeBootstrap(f); + const advancedBase = 'd'.repeat(40); + f.pr.base.sha = advancedBase; + f.env.SQUAD_REVIEW_WORKFLOW_SHA = advancedBase; + f.state.compareStatus = 'ahead'; + await expect(reviewTarget(f.env, f.get)).resolves.toMatchObject({ + author_agent: '@squad/base-controlled-bootstrap', + }); + expect(f.state.calls).toContain(`repos/${REPOSITORY}/compare/${BASE}...${advancedBase}`); + }); + + it.each([ + ['diverged', (f: ReturnType) => { f.state.compareStatus = 'diverged'; }], + ['behind', (f: ReturnType) => { f.state.compareStatus = 'behind'; }], + ['missing status', (f: ReturnType) => { + f.state.compareOmitStatus = true; + }], + ['missing base_commit', (f: ReturnType) => { + f.state.compareStatus = 'ahead'; + f.state.compareOmitBaseCommit = true; + }], + ['missing merge_base_commit', (f: ReturnType) => { + f.state.compareStatus = 'ahead'; + f.state.compareOmitMergeBaseCommit = true; + }], + ['mismatched base_commit', (f: ReturnType) => { + f.state.compareStatus = 'ahead'; + f.state.compareBaseCommitShaOverride = 'e'.repeat(40); + }], + ['mismatched merge_base_commit', (f: ReturnType) => { + f.state.compareStatus = 'ahead'; + f.state.compareMergeBaseShaOverride = 'f'.repeat(40); + }], + ])('rejects bot-authored bootstrap provenance when compare evidence is %s', async (_label, mutate) => { + const f = fixture(); + makeBootstrap(f); + f.pr.base.sha = 'd'.repeat(40); + f.env.SQUAD_REVIEW_WORKFLOW_SHA = f.pr.base.sha; + mutate(f); + await expect(reviewTarget(f.env, f.get)).rejects.toThrow( + 'bootstrap provenance install commit is not an ancestor of the pull request base', + ); + }); + + it.each([ + ['404', 404], + ['rate limit', 403], + ['general API failure', 500], + ])('fails closed when the bot-authored bootstrap compare API returns %s', async (_label, status) => { + const f = fixture(); + makeBootstrap(f); + f.pr.base.sha = 'd'.repeat(40); + f.env.SQUAD_REVIEW_WORKFLOW_SHA = f.pr.base.sha; + f.state.compareStatus = 'ahead'; + f.state.compareThrows = status; + await expect(reviewTarget(f.env, f.get)).rejects.toMatchObject({ status }); + }); + it('accepts the documented manual bootstrap pull request fallback when live data exactly matches the signed provenance record', async () => { const f = fixture(); makeBootstrapPrFallback(f); diff --git a/workflows/package/squad-bootstrap.md b/workflows/package/squad-bootstrap.md index 6aebc23a0..d50413902 100644 --- a/workflows/package/squad-bootstrap.md +++ b/workflows/package/squad-bootstrap.md @@ -16,7 +16,7 @@ on: - .github/aw/squad/runtime/** - .github/workflows/shared/squad-bootstrap-trigger-probe.json workflow_dispatch: null -if: github.ref_name == github.event.repository.default_branch +if: github.ref == format('refs/heads/{0}', github.event.repository.default_branch) permissions: contents: read copilot-requests: write @@ -507,7 +507,7 @@ safe-outputs: server: process.env.GITHUB_SERVER_URL, }); // context.sha is the default branch's exact commit this run executed on (the - // top-level `if: github.ref_name == github.event.repository.default_branch` + // top-level exact default-branch ref gate // guards both the push and workflow_dispatch trigger paths), so it is a // reliable, zero-extra-API-call stand-in for "the base commit any fresh // provenance record produced by this run would be bound to". diff --git a/workflows/shared/squad-review-guard.mjs b/workflows/shared/squad-review-guard.mjs index b0d983568..037b952d8 100644 --- a/workflows/shared/squad-review-guard.mjs +++ b/workflows/shared/squad-review-guard.mjs @@ -163,6 +163,17 @@ function bootstrapRecord(body, label = 'bootstrap provenance') { return value; } +async function requireRecordedBaseAncestor(get, repository, recordedSha, liveSha, message) { + requireThat(SHA.test(recordedSha) && SHA.test(liveSha), message); + const comparison = await get(`repos/${repository}/compare/${recordedSha}...${liveSha}`); + requireThat( + (comparison?.status === 'identical' || comparison?.status === 'ahead') && + comparison?.base_commit?.sha === recordedSha && + comparison?.merge_base_commit?.sha === recordedSha, + message, + ); +} + // The post-install bootstrap PR fallback (documented alongside `can_approve_pull_request_reviews: false` // in docs/src/content/docs/guide/gh-aw.md) asks a human to open the Cast PR manually from a compare-URL // link after `pulls.create` is permission-denied. That PR is human-authored, so it cannot satisfy the @@ -237,11 +248,11 @@ async function validateBootstrapPrFallbackAttribution(env, get, repository, pr, // SHAs back to `provenance.base_sha`: a `status` of 'identical'/'ahead' alone does not guarantee // the API resolved *this* base against *this* provenance commit (e.g. a short-SHA or mistyped // route could silently compare against the wrong ref while still reporting a plausible status). - const baseComparison = await get(`repos/${repository}/compare/${provenance.base_sha}...${pr.base.sha}`); - requireThat( - (baseComparison?.status === 'identical' || baseComparison?.status === 'ahead') && - baseComparison?.base_commit?.sha === provenance.base_sha && - baseComparison?.merge_base_commit?.sha === provenance.base_sha, + await requireRecordedBaseAncestor( + get, + repository, + provenance.base_sha, + pr.base.sha, 'bootstrap PR fallback provenance base commit is not an ancestor of the pull request base', ); const expectedCompareUrl = buildBootstrapPrFallbackCompareUrl({ @@ -255,11 +266,11 @@ async function validateBootstrapPrFallbackAttribution(env, get, repository, pr, 'bootstrap PR fallback provenance compare URL does not match the expected manual link'); requireThat(timestamp(fallbackIssue.created_at) <= timestamp(pr.created_at), 'bootstrap PR fallback issue was created after this pull request'); - // Mirrors validateBootstrapAttribution's run.head_sha === pr.base.sha check below, but binds to - // the immutable `provenance.base_sha` rather than the live, drifting `pr.base.sha` (see the - // ancestry check above for why): without this, a fallback issue's provenance.run_id only has to - // name *some* historical successful bootstrap run on the same base branch, not the run that - // actually produced the exact base commit recorded in this provenance. + // Both bootstrap attribution paths bind the trusted run to the immutable SHA recorded in their + // provenance rather than the live, drifting `pr.base.sha` (see the ancestry check above for why). + // Without this, a fallback issue's provenance.run_id only has to name *some* historical + // successful bootstrap run on the same base branch, not the run that actually produced the + // exact base commit recorded in this provenance. const run = await get(`repos/${repository}/actions/runs/${provenance.run_id}`); requireThat( run.event === 'push' && @@ -303,16 +314,23 @@ async function validateBootstrapAttribution(get, repository, pr, requireRunSucce pr.user?.type === 'Bot' && provenance.repository === repository && /^[1-9]\d*$/.test(provenance.run_id ?? '') && - provenance.install_sha === pr.base.sha && + SHA.test(provenance.install_sha ?? '') && provenance.cast_sha === pr.head.sha, 'invalid base-controlled bootstrap pull request', ); + await requireRecordedBaseAncestor( + get, + repository, + provenance.install_sha, + pr.base.sha, + 'bootstrap provenance install commit is not an ancestor of the pull request base', + ); const run = await get(`repos/${repository}/actions/runs/${provenance.run_id}`); requireThat( run.event === 'push' && run.path === BOOTSTRAP_WORKFLOW && run.repository?.full_name === repository && - run.head_sha === pr.base.sha && + run.head_sha === provenance.install_sha && run.head_branch === pr.base.ref && (!requireRunSuccess || (run.status === 'completed' && run.conclusion === 'success')), requireRunSuccess diff --git a/workflows/squad-bootstrap.md b/workflows/squad-bootstrap.md index f05ecf52e..540f137dc 100644 --- a/workflows/squad-bootstrap.md +++ b/workflows/squad-bootstrap.md @@ -16,7 +16,7 @@ on: - ".github/aw/squad/runtime/**" - ".github/workflows/shared/squad-bootstrap-trigger-probe.json" workflow_dispatch: -if: github.ref_name == github.event.repository.default_branch +if: github.ref == format('refs/heads/{0}', github.event.repository.default_branch) permissions: contents: read copilot-requests: write @@ -498,7 +498,7 @@ safe-outputs: server: process.env.GITHUB_SERVER_URL, }); // context.sha is the default branch's exact commit this run executed on (the - // top-level `if: github.ref_name == github.event.repository.default_branch` + // top-level exact default-branch ref gate // guards both the push and workflow_dispatch trigger paths), so it is a // reliable, zero-extra-API-call stand-in for "the base commit any fresh // provenance record produced by this run would be bound to". diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index d17319f8b..ce9c2c539 100644 --- a/workflows/squad-workflows.manifest.json +++ b/workflows/squad-workflows.manifest.json @@ -68,9 +68,9 @@ "source": "workflows/package/squad-bootstrap.md", "destination": ".github/workflows/squad-bootstrap.md", "lock": ".github/workflows/squad-bootstrap.lock.yml", - "source_sha256": "42b929f7befe6892b4f63d7531c5ee55c246a9ecbe4ec0db04041e424d07733e", - "lock_sha256": "dd3a391a655ed9a65ec06b2fb16a1d2cb327321631f618147b86a9bddc6c1db9", - "package_lock_sha256": "0037cae72c220cdd05c0d7452b11bccd15956cc1cac98a187666af583a3dd5ff" + "source_sha256": "a266ed20e76b751e5527b49c9d3517482504a0fd6a6704d04531b57f99dbbaa1", + "lock_sha256": "3642cfb42e6b6230354fb245f84bf369d4db29051a6db184c93147504acc221d", + "package_lock_sha256": "a2a635dd98427b713c843e7baf22f98a01a0688f450bb2fc993b1fcaa960fb4e" }, { "name": "squad-command-router", @@ -89,7 +89,7 @@ "package_destination": ".github/workflows/shared/squad-review-guard.mjs", "destination": ".github/workflows/shared/squad-review-guard.mjs", "ownership": "manifest", - "sha256": "2e5e16cfca47e5c0b50744e0c3cf56c5bedebd878f7422d4ed29d090c5abae11" + "sha256": "b499ad06b33a52f55f3618ac37cfe739b7b59c45b867ea2b5f47d81598b874c6" }, { "path": "shared/squad-install-verifier.mjs",