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.
This commit is contained in:
Jinwoo Hong
2026-09-17 20:33:57 -04:00
committed by GitHub
parent 399306c171
commit ff8f7085cc
3 changed files with 152 additions and 8 deletions
@@ -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'],
@@ -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)
})