From ff8f7085ccffcc35696e19784c28c1fb6db813d4 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:33:57 -0400 Subject: [PATCH] fix(relay-ops): bind the canary cell's admission class into same-cap batch authority (#21313) A batch-apply wave verified only that the sealed canary named some approved same-cap cell, so a canary rolled on the migration-only, zero-host, 600-cap c17 or c18 was accepted as authority for a general 1000/3000-cap batch. The verify step now hands the batch's own cells to the check, which requires the sealed cell's entry admission to equal the batch's class. --- ...cloud-deploy-relay-production-same-cap.yml | 14 +- .../relay-production-same-cap-wave.mjs | 11 ++ .../relay-production-same-cap-wave.test.mjs | 135 +++++++++++++++++- 3 files changed, 152 insertions(+), 8 deletions(-) diff --git a/.github/workflows/cloud-deploy-relay-production-same-cap.yml b/.github/workflows/cloud-deploy-relay-production-same-cap.yml index fa2cae818b2..421fa080072 100644 --- a/.github/workflows/cloud-deploy-relay-production-same-cap.yml +++ b/.github/workflows/cloud-deploy-relay-production-same-cap.yml @@ -181,14 +181,20 @@ jobs: if: ${{ inputs.mode == 'batch-apply' }} env: CANARY_RUN_ID: ${{ inputs.canary-run-id }} + CELL_IDS: ${{ inputs.cell-ids }} + TARGET_DIGEST: ${{ inputs.target-image-digest }} + ROLLBACK_DIGEST: ${{ inputs.rollback-image-digest }} + SELECTOR_GENERATION: ${{ inputs.expected-selector-generation }} + REHOME_GENERATION: ${{ inputs.expected-rehome-generation }} run: | node dev/scripts/relay-production-same-cap-wave.mjs verify-canary \ --file "${RUNNER_TEMP}/relay-same-cap-canary/authority.json" \ --commit-sha "${GITHUB_SHA}" --run-id "${CANARY_RUN_ID}" \ - --target-digest "${{ inputs.target-image-digest }}" \ - --rollback-digest "${{ inputs.rollback-image-digest }}" \ - --selector-generation "${{ inputs.expected-selector-generation }}" \ - --rehome-generation "${{ inputs.expected-rehome-generation }}" + --cell-ids "${CELL_IDS}" \ + --target-digest "${TARGET_DIGEST}" \ + --rollback-digest "${ROLLBACK_DIGEST}" \ + --selector-generation "${SELECTOR_GENERATION}" \ + --rehome-generation "${REHOME_GENERATION}" - name: Reject previously consumed aggregate safety evidence if: ${{ inputs.mode != 'verify' && inputs.gate-override-confirmation == '' }} diff --git a/cloud/dev/scripts/relay-production-same-cap-wave.mjs b/cloud/dev/scripts/relay-production-same-cap-wave.mjs index a3b6fee31b6..654d9fc55bd 100644 --- a/cloud/dev/scripts/relay-production-same-cap-wave.mjs +++ b/cloud/dev/scripts/relay-production-same-cap-wave.mjs @@ -134,6 +134,8 @@ export function canaryAuthority(input) { export function verifyCanaryAuthority(authority, expected, repositoryRoot) { const selectorGeneration = Number(expected.selectorGeneration) + // A mixed wave is already rejected, so the batch's first cell names the whole batch's class. + const batchAdmission = entryAdmission(cells(expected.cellIds ?? '')[0]) if ( authority?.v !== 1 || !/^[0-9a-f]{40}$/.test(authority.commitSha ?? '') || @@ -147,6 +149,14 @@ export function verifyCanaryAuthority(authority, expected, repositoryRoot) { authority.rehomeGeneration !== Number(expected.rehomeGeneration) || !SAME_CAP_CELLS.includes(authority.cellId) ) throw new Error('canary authority does not match this batch') + // A migration-only cell carries no hosts and a different cap, so rolling it proves nothing + // about a general batch, and its wave advances a different selector delta. + if (entryAdmission(authority.cellId) !== batchAdmission) { + throw new Error( + `canary authority cell ${authority.cellId} is ${entryAdmission(authority.cellId)}, ` + + `but this batch is ${batchAdmission}` + ) + } // Each cell checks exact live selector state; later batches may reuse this control epoch's canary. requireSameEvidenceCode({ sealedSha: authority.commitSha, @@ -215,6 +225,7 @@ export function main(argv = process.argv.slice(2)) { verifyCanaryAuthority(JSON.parse(readFileSync(input.file, 'utf8')), { commitSha: input['commit-sha'], runId: input['run-id'], + cellIds: input['cell-ids'], targetDigest: input['target-digest'], rollbackDigest: input['rollback-digest'], selectorGeneration: input['selector-generation'], diff --git a/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs b/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs index 997b5975968..0d2ac6f1a96 100644 --- a/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs +++ b/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { execFileSync } from 'node:child_process' +import { execFileSync, spawnSync } from 'node:child_process' import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' @@ -13,6 +13,7 @@ import { validateSameCapWave, verifyCanaryAuthority } from './relay-production-same-cap-wave.mjs' +import { readRelayWorkflow } from './relay-repository.mjs' const targetDigest = `sha256:${'a'.repeat(64)}` const rollbackDigest = `sha256:${'b'.repeat(64)}` @@ -103,10 +104,11 @@ test('seals a migration-only canary at the generation its wave leaves behind', ( // Isolate and restore are both no-ops on a migration-only cell, so nothing advances. assert.equal(seal('production-gce-c17').selectorGeneration, 11) assert.equal(seal('production-gce-c7').selectorGeneration, 13) - // That canary still authorizes a later general batch; it is evidence about the image. + // That canary still authorizes a later batch of its own class; it is evidence about the image. assert.equal(verifyCanaryAuthority(seal('production-gce-c17'), { commitSha: 'c'.repeat(40), runId: '42', + cellIds: 'production-gce-c17,production-gce-c18', targetDigest, rollbackDigest, selectorGeneration: '11', @@ -173,6 +175,7 @@ test('seals and verifies canary authority for later batches', () => { assert.equal(verifyCanaryAuthority(authority, { commitSha: 'c'.repeat(40), runId: '42', + cellIds: 'production-gce-c8,production-gce-c9', targetDigest, rollbackDigest, selectorGeneration: '13', @@ -181,6 +184,7 @@ test('seals and verifies canary authority for later batches', () => { assert.throws(() => verifyCanaryAuthority(authority, { commitSha: 'd'.repeat(40), runId: '42', + cellIds: 'production-gce-c8,production-gce-c9', targetDigest, rollbackDigest, selectorGeneration: '11', @@ -195,8 +199,8 @@ test('reuses a canary across selector advances only within the same control epoc commitSha: 'c'.repeat(40), runId: '42', selectorGeneration: '11', rehomeGeneration: '4' }) const expected = { - commitSha: 'c'.repeat(40), runId: '42', targetDigest, rollbackDigest, - selectorGeneration: '21', rehomeGeneration: '4' + commitSha: 'c'.repeat(40), runId: '42', cellIds: 'production-gce-c8,production-gce-c9', + targetDigest, rollbackDigest, selectorGeneration: '21', rehomeGeneration: '4' } for (const generation of ['13', '14', '21', '29']) { assert.equal(verifyCanaryAuthority(authority, { @@ -270,6 +274,7 @@ test('a batch trusts a canary sealed by identical code at an ancestor commit', a const verifyAt = (commitSha, repositoryRoot) => verifyCanaryAuthority(authority, { commitSha, runId: '42', + cellIds: 'production-gce-c8,production-gce-c9', targetDigest, rollbackDigest, selectorGeneration: '21', @@ -392,6 +397,7 @@ test('seals the override into the canary authority as audit trail only', () => { const expected = { commitSha: 'f'.repeat(40), runId: '42', + cellIds: 'production-gce-c8,production-gce-c9', targetDigest, rollbackDigest, selectorGeneration: '21', @@ -416,3 +422,124 @@ test('seals the override into the canary authority as audit trail only', () => { 'production-gce-c7' ) }) + +function sealedCanary(cellId) { + return canaryAuthority({ + cellIds: cellId, + targetDigest, + rollbackDigest, + confirmation: `ROLL_RELAY_SAME_CAP ${targetDigest} ${cellId}`, + commitSha: 'c'.repeat(40), + runId: '42', + selectorGeneration: '11', + rehomeGeneration: '4' + }) +} + +// Why: a migration-only cell holds zero hosts at a different cap and its wave advances no +// selector, so rolling one is no evidence for a general batch, and the reverse is no evidence +// either. Nothing but the sealed cell id says which class a canary actually proved. +test('refuses a canary sealed on a cell of the other admission class', () => { + const expected = { + commitSha: 'c'.repeat(40), + runId: '42', + targetDigest, + rollbackDigest, + selectorGeneration: '99', + rehomeGeneration: '4' + } + const general = 'production-gce-c8,production-gce-c9' + const migrationOnly = SAME_CAP_MIGRATION_ONLY_CELLS.join(',') + assert.throws( + () => verifyCanaryAuthority(sealedCanary('production-gce-c17'), { + ...expected, cellIds: general + }), + /canary authority cell production-gce-c17 is migration-only, but this batch is general/ + ) + assert.throws( + () => verifyCanaryAuthority(sealedCanary('production-gce-c7'), { + ...expected, cellIds: migrationOnly + }), + /canary authority cell production-gce-c7 is general, but this batch is migration-only/ + ) + assert.equal( + verifyCanaryAuthority(sealedCanary('production-gce-c7'), { + ...expected, cellIds: general + }).cellId, + 'production-gce-c7' + ) + assert.equal( + verifyCanaryAuthority(sealedCanary('production-gce-c17'), { + ...expected, cellIds: migrationOnly + }).cellId, + 'production-gce-c17' + ) + // A caller that names no batch at all gets no verdict, rather than an unchecked class. + assert.throws( + () => verifyCanaryAuthority(sealedCanary('production-gce-c7'), expected), + /same-cap wave cells are invalid/ + ) +}) + +// The dispatch workflow is the only caller, so the class check only binds anything if that +// step actually hands the batch over; run the step's own shell exactly as written. +function verifyCanaryStepScript() { + const dispatch = readRelayWorkflow('deploy-relay-production-same-cap.yml') + const first = ' node dev/scripts/relay-production-same-cap-wave.mjs verify-canary \\\n' + const start = dispatch.indexOf(first) + assert.notEqual(start, -1, 'the dispatch workflow has no verify-canary step') + const last = ' --rehome-generation "${REHOME_GENERATION}"\n' + const end = dispatch.indexOf(last, start) + assert.notEqual(end, -1, 'the verify-canary step does not end at the rehome generation') + return dispatch.slice(start, end + last.length).replace(/^ {10}/gm, '') +} + +async function runVerifyCanaryStep(authority, cellIds) { + const temporary = await mkdtemp(join(tmpdir(), 'relay-same-cap-verify-')) + try { + await mkdir(join(temporary, 'relay-same-cap-canary'), { recursive: true }) + await writeFile( + join(temporary, 'relay-same-cap-canary', 'authority.json'), + JSON.stringify(authority) + ) + return spawnSync('bash', ['-euo', 'pipefail', '-c', verifyCanaryStepScript()], { + cwd: new URL('../..', import.meta.url), + env: { + ...process.env, + RUNNER_TEMP: temporary, + GITHUB_SHA: authority.commitSha, + CANARY_RUN_ID: authority.runId, + CELL_IDS: cellIds, + TARGET_DIGEST: targetDigest, + ROLLBACK_DIGEST: rollbackDigest, + SELECTOR_GENERATION: '99', + REHOME_GENERATION: '4' + }, + encoding: 'utf8' + }) + } finally { + await rm(temporary, { recursive: true, force: true }) + } +} + +test('the batch gate hands its own cells to the canary check', async () => { + const accepted = await runVerifyCanaryStep( + sealedCanary('production-gce-c7'), + 'production-gce-c8,production-gce-c9' + ) + assert.equal(accepted.status, 0, accepted.stderr) + const crossed = await runVerifyCanaryStep( + sealedCanary('production-gce-c17'), + 'production-gce-c8,production-gce-c9' + ) + assert.equal(crossed.status, 1, crossed.stdout) + assert.match( + crossed.stderr, + /canary authority cell production-gce-c17 is migration-only, but this batch is general/ + ) + const migrationOnly = await runVerifyCanaryStep( + sealedCanary('production-gce-c17'), + SAME_CAP_MIGRATION_ONLY_CELLS.join(',') + ) + assert.equal(migrationOnly.status, 0, migrationOnly.stderr) +})