From 50c58e990187806fbc542bc7b3cd8f002780886c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 13:43:45 -0400 Subject: [PATCH 01/18] fix: invalidate pending worker pairings on revoke --- service/src/bridge/pairing.test.ts | 82 +++++----- service/src/bridge/pairing.ts | 232 +++++++++++++---------------- 2 files changed, 142 insertions(+), 172 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 310ad7d..71980ff 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -63,11 +63,8 @@ describe('RedisBridgePairingStore', () => { originalAuthorization.identityId, ); await expect( - pairings.authorize(requestFor(issued.credential, 'overlap-bound-proof')), - ).resolves.toMatchObject({ - workerId: 'vm-bound', - identityId: originalAuthorization.identityId, - }); + pairings.authorize(requestFor(issued.credential, 'superseded-bound-proof')), + ).rejects.toMatchObject({ code: 'CREDENTIAL_INVALID' }); }); test('preserves a legacy unmarked identity across its first rotation', async () => { @@ -338,45 +335,43 @@ describe('RedisBridgePairingStore', () => { ).rejects.toMatchObject({ code: 'CREDENTIAL_INVALID' }); }); - test('rotation keeps the prior same-identity credential usable for recovery', async () => { + test('revocation invalidates an unredeemed pairing code', async () => { const identity = createBridgeIdentity(); const pairing = await pairings.issue('vm-1'); - const original = await pairings.redeem({ - workerId: 'vm-1', - code: pairing.code, - publicKey: identity.publicKey, - }); - const rotated = await pairings.rotate('vm-1'); + await pairings.revoke('vm-1'); - const proofFor = ( - credential: string, - nonce: string, - ): Parameters[0] => { - const proof = { - credential, - method: 'POST', - path: '/v1/bridge/workers/vm-1/lease', - timestamp: new Date().toISOString(), - nonce, - body: JSON.stringify({ protocolVersion: 1, waitMs: 25_000 }), - }; - return { - ...proof, + await expect( + pairings.redeem({ workerId: 'vm-1', - signature: signBridgeRequest(identity.privateKey, proof), - }; - }; + code: pairing.code, + publicKey: identity.publicKey, + }), + ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); + }); + + test('issuing a replacement invalidates the prior unredeemed pairing code', async () => { + const identity = createBridgeIdentity(); + const first = await pairings.issue('vm-1'); + const replacement = await pairings.issue('vm-1'); await expect( - pairings.authorize(proofFor(original.credential, 'old-credential')), - ).resolves.toMatchObject({ workerId: 'vm-1' }); + pairings.redeem({ + workerId: 'vm-1', + code: first.code, + publicKey: identity.publicKey, + }), + ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); await expect( - pairings.authorize(proofFor(rotated.credential, 'new-credential')), + pairings.redeem({ + workerId: 'vm-1', + code: replacement.code, + publicKey: identity.publicKey, + }), ).resolves.toMatchObject({ workerId: 'vm-1' }); }); - test('recovers when a refresh response is lost after the server commits it', async () => { + test('rotation replaces rather than duplicates the active credential', async () => { const identity = createBridgeIdentity(); const pairing = await pairings.issue('vm-1'); const original = await pairings.redeem({ @@ -384,6 +379,9 @@ describe('RedisBridgePairingStore', () => { code: pairing.code, publicKey: identity.publicKey, }); + + const rotated = await pairings.rotate('vm-1'); + const proofFor = ( credential: string, nonce: string, @@ -391,10 +389,10 @@ describe('RedisBridgePairingStore', () => { const proof = { credential, method: 'POST', - path: '/v1/bridge/workers/vm-1/credentials/refresh', + path: '/v1/bridge/workers/vm-1/lease', timestamp: new Date().toISOString(), nonce, - body: JSON.stringify({ protocolVersion: 1 }), + body: JSON.stringify({ protocolVersion: 1, waitMs: 25_000 }), }; return { ...proof, @@ -403,17 +401,11 @@ describe('RedisBridgePairingStore', () => { }; }; - await pairings.rotate('vm-1'); - const retryAuthorization = await pairings.authorize( - proofFor(original.credential, 'refresh-response-lost'), - ); - const recovered = await pairings.rotate( - 'vm-1', - retryAuthorization.credentialId, - ); - await expect( - pairings.authorize(proofFor(recovered.credential, 'refresh-recovered')), + pairings.authorize(proofFor(original.credential, 'old-credential')), + ).rejects.toMatchObject({ code: 'CREDENTIAL_INVALID' }); + await expect( + pairings.authorize(proofFor(rotated.credential, 'new-credential')), ).resolves.toMatchObject({ workerId: 'vm-1' }); }); diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index fd1e8cf..7d58af8 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -10,64 +10,46 @@ import { verifyBridgeRequest } from '../../../packages/code/src/identity'; const PREFIX = 'codeapi:bridge:v1'; const DEFAULT_PAIRING_TTL_SECONDS = 10 * 60; -const DEFAULT_CREDENTIAL_TTL_SECONDS = 15 * 60; +const DEFAULT_CREDENTIAL_TTL_SECONDS = 5 * 60; const PROOF_NONCE_TTL_SECONDS = 2 * 60; const PROOF_CLOCK_SKEW_MS = 60_000; -const ROTATE_CREDENTIAL_SCRIPT = ` -local activeDigest = redis.call('GET', KEYS[1]) -local previous = redis.call('GET', KEYS[2]) -if not activeDigest or not previous then - return 0 -end -if activeDigest ~= ARGV[1] then - if ARGV[5] == '' or redis.call('GET', KEYS[4]) ~= ARGV[5] then - return 0 - end -end -redis.call('SET', KEYS[3], ARGV[3], 'EX', ARGV[4]) -redis.call('SET', KEYS[1], ARGV[2], 'EX', ARGV[4]) -if ARGV[5] ~= '' then - redis.call('SET', KEYS[4], ARGV[5], 'EX', ARGV[4]) -else - redis.call('DEL', KEYS[4]) -end -return 1 -`; const ISSUE_PAIRING_SCRIPT = ` local previous = redis.call('GET', KEYS[1]) if previous then redis.call('DEL', previous) end -redis.call('DEL', KEYS[3]) -redis.call('SET', KEYS[1], KEYS[2], 'EX', ARGV[2]) -redis.call('SET', KEYS[2], ARGV[1], 'EX', ARGV[2]) +redis.call('SET', KEYS[1], KEYS[3], 'EX', ARGV[3]) +redis.call('SET', KEYS[2], ARGV[1], 'EX', ARGV[3]) +redis.call('SET', KEYS[3], ARGV[2], 'EX', ARGV[3]) return 1 `; const REDEEM_PAIRING_SCRIPT = ` local pairing = redis.call('GET', KEYS[1]) -if not pairing then - return nil +if pairing ~= ARGV[1] then + return 0 end -if redis.call('GET', KEYS[2]) ~= KEYS[1] then +if redis.call('GET', KEYS[2]) ~= ARGV[2] then redis.call('DEL', KEYS[1]) - return nil + return 0 +end +redis.call('DEL', KEYS[1]) +if redis.call('GET', KEYS[5]) == KEYS[1] then + redis.call('DEL', KEYS[5]) end -redis.call('DEL', KEYS[1], KEYS[2]) -redis.call('SET', KEYS[3], ARGV[1], 'EX', ARGV[2]) -return pairing +redis.call('SET', KEYS[3], ARGV[3], 'EX', ARGV[4]) +redis.call('SET', KEYS[4], ARGV[5], 'EX', ARGV[4]) +return 1 `; -const INSTALL_REDEEMED_CREDENTIAL_SCRIPT = ` +const ROTATE_CREDENTIAL_SCRIPT = ` if redis.call('GET', KEYS[1]) ~= ARGV[1] then return 0 end -redis.call('SET', KEYS[2], ARGV[3], 'EX', ARGV[4]) -redis.call('SET', KEYS[3], ARGV[2], 'EX', ARGV[4]) -if ARGV[5] ~= '' then - redis.call('SET', KEYS[4], ARGV[5], 'EX', ARGV[4]) -else - redis.call('DEL', KEYS[4]) +if redis.call('EXISTS', KEYS[2]) ~= 1 then + return 0 end -redis.call('DEL', KEYS[1]) +redis.call('SET', KEYS[3], ARGV[3], 'EX', ARGV[4]) +redis.call('SET', KEYS[1], ARGV[2], 'EX', ARGV[4]) +redis.call('DEL', KEYS[2]) return 1 `; @@ -84,11 +66,13 @@ export interface BridgeWorkerBinding { interface StoredPairing { workerId: string; expiresAt: string; + generation: string; binding?: BridgeWorkerBinding; } interface StoredCredential { workerId: string; + /** Stable across refreshes; replaced only when the worker is paired again. */ identityId?: string; publicKey: string; expiresAt: string; @@ -138,16 +122,12 @@ function workerIdentityKey(workerId: string): string { return `${PREFIX}:identity:${workerId}`; } -function workerStableIdentityKey(workerId: string): string { - return `${PREFIX}:stable-identity:${workerId}`; -} - function workerPairingIndexKey(workerId: string): string { return `${PREFIX}:pairing-index:${workerId}`; } -function workerRedemptionKey(workerId: string): string { - return `${PREFIX}:redemption:${workerId}`; +function workerPairingGenerationKey(workerId: string): string { + return `${PREFIX}:pairing-generation:${workerId}`; } function proofNonceKey(credential: string, nonce: string): string { @@ -174,17 +154,24 @@ export class RedisBridgePairingStore { binding?: BridgeWorkerBinding, ): Promise { const code = randomBytes(24).toString('base64url'); + const generation = randomBytes(24).toString('base64url'); const expiresAt = new Date( Date.now() + this.pairingTtlSeconds * 1000, ).toISOString(); - const pairing: StoredPairing = { workerId, expiresAt, binding }; + const pairing: StoredPairing = { + workerId, + expiresAt, + generation, + binding, + }; const codeKey = pairingKey(code); await this.redis.eval( ISSUE_PAIRING_SCRIPT, 3, workerPairingIndexKey(workerId), + workerPairingGenerationKey(workerId), codeKey, - workerRedemptionKey(workerId), + generation, JSON.stringify(pairing), String(this.pairingTtlSeconds), ); @@ -196,44 +183,62 @@ export class RedisBridgePairingStore { code: string; publicKey: string; }): Promise { + const codeKey = pairingKey(args.code); + const raw = await this.redis.get(codeKey); + if (raw == null) { + throw new BridgePairingError( + 'PAIRING_INVALID', + 'Pairing code is invalid or expired', + ); + } + const pairing = JSON.parse(raw) as StoredPairing; + if (pairing.workerId !== args.workerId) { + await this.redis.del(codeKey); + throw new BridgePairingError( + 'PAIRING_INVALID', + 'Pairing code does not authorize this worker', + ); + } if (!validEd25519PublicKey(args.publicKey)) { + await this.redis.del(codeKey); throw new BridgePairingError( 'PUBLIC_KEY_INVALID', 'Worker public key must be an Ed25519 key', ); } - const codeKey = pairingKey(args.code); - const redemptionId = randomBytes(18).toString('base64url'); - const raw = await this.redis.eval( + + const credential = randomBytes(32).toString('base64url'); + const credentialDigest = digest(credential); + const expiresAt = new Date( + Date.now() + this.credentialTtlSeconds * 1000, + ).toISOString(); + const stored: StoredCredential = { + workerId: args.workerId, + publicKey: args.publicKey, + expiresAt, + binding: pairing.binding, + }; + const accepted = await this.redis.eval( REDEEM_PAIRING_SCRIPT, - 3, + 5, codeKey, + workerPairingGenerationKey(pairing.workerId), + credentialDigestKey(credentialDigest), + workerIdentityKey(args.workerId), workerPairingIndexKey(args.workerId), - workerRedemptionKey(args.workerId), - redemptionId, - String(this.pairingTtlSeconds), + raw, + pairing.generation, + JSON.stringify(stored), + String(this.credentialTtlSeconds), + credentialDigest, ); - if (typeof raw !== 'string') { + if (accepted !== 1) { throw new BridgePairingError( 'PAIRING_INVALID', 'Pairing code is invalid or expired', ); } - const pairing = JSON.parse(raw) as StoredPairing; - if (pairing.workerId !== args.workerId) { - throw new BridgePairingError( - 'PAIRING_INVALID', - 'Pairing code does not authorize this worker', - ); - } - return await this.issueCredential( - args.workerId, - args.publicKey, - undefined, - undefined, - pairing.binding, - redemptionId, - ); + return { workerId: args.workerId, credential, expiresAt }; } async authorize(args: { @@ -248,7 +253,6 @@ export class RedisBridgePairingStore { }): Promise<{ workerId: string; credentialId: string; - activeCredentialId: string; identityId?: string; binding?: BridgeWorkerBinding; }> { @@ -267,31 +271,13 @@ export class RedisBridgePairingStore { credentialDigestKey(credentialDigest), workerIdentityKey(args.workerId), ); - if (raw == null || activeDigest == null) { + if (raw == null || activeDigest !== credentialDigest) { throw new BridgePairingError( 'CREDENTIAL_INVALID', 'Worker credential is invalid or expired', ); } const stored = JSON.parse(raw) as StoredCredential; - if (activeDigest !== credentialDigest) { - const activeRaw = await this.redis.get( - credentialDigestKey(activeDigest), - ); - const active = activeRaw == null - ? undefined - : JSON.parse(activeRaw) as StoredCredential; - if ( - stored.identityId == null || - active?.identityId == null || - stored.identityId !== active.identityId - ) { - throw new BridgePairingError( - 'CREDENTIAL_INVALID', - 'Worker credential is invalid or expired', - ); - } - } if (stored.workerId !== args.workerId) { throw new BridgePairingError( 'CREDENTIAL_INVALID', @@ -320,21 +306,24 @@ export class RedisBridgePairingStore { return { workerId: stored.workerId, credentialId: credentialDigest, - activeCredentialId: activeDigest, ...(stored.identityId != null ? { identityId: stored.identityId } : {}), ...(stored.binding ? { binding: stored.binding } : {}), }; } async revoke(workerId: string): Promise { + // Rotate the pending generation before touching active credentials so an + // unredeemed code cannot race lifecycle deletion and create a new worker. + await this.redis.set( + workerPairingGenerationKey(workerId), + randomBytes(24).toString('base64url'), + 'EX', + this.pairingTtlSeconds, + ); const identityKey = workerIdentityKey(workerId); const credentialDigest = await this.redis.get(identityKey); if (credentialDigest == null) return; - await this.redis.del( - identityKey, - workerStableIdentityKey(workerId), - credentialDigestKey(credentialDigest), - ); + await this.redis.del(identityKey, credentialDigestKey(credentialDigest)); } async rotate( @@ -359,8 +348,8 @@ export class RedisBridgePairingStore { workerId, previous.publicKey, previousDigest, - previous.identityId ?? null, previous.binding, + previous.identityId ?? null, ); } @@ -368,16 +357,18 @@ export class RedisBridgePairingStore { workerId: string, publicKey: string, previousDigest?: string, - identityId: string | null | undefined = randomBytes(18).toString('base64url'), binding?: BridgeWorkerBinding, - redemptionId?: string, + identityId?: string | null, ): Promise { const credential = randomBytes(32).toString('base64url'); const credentialDigest = digest(credential); const expiresAt = new Date( Date.now() + this.credentialTtlSeconds * 1000, ).toISOString(); - const stableIdentityId = identityId ?? undefined; + const stableIdentityId = + identityId === undefined + ? randomBytes(18).toString('base64url') + : identityId ?? undefined; const stored: StoredCredential = { workerId, ...(stableIdentityId != null ? { identityId: stableIdentityId } : {}), @@ -388,16 +379,14 @@ export class RedisBridgePairingStore { if (previousDigest !== undefined) { const rotated = await this.redis.eval( ROTATE_CREDENTIAL_SCRIPT, - 4, + 3, workerIdentityKey(workerId), credentialDigestKey(previousDigest), credentialDigestKey(credentialDigest), - workerStableIdentityKey(workerId), previousDigest, credentialDigest, JSON.stringify(stored), String(this.credentialTtlSeconds), - stableIdentityId ?? '', ); if (rotated !== 1) { throw new BridgePairingError( @@ -405,32 +394,21 @@ export class RedisBridgePairingStore { 'Worker credential is invalid or expired', ); } - return { workerId, credential, expiresAt }; - } - if (redemptionId == null) { - throw new BridgePairingError( - 'PAIRING_INVALID', - 'Pairing redemption was not fenced', + } else { + const transaction = this.redis.multi(); + transaction.set( + credentialDigestKey(credentialDigest), + JSON.stringify(stored), + 'EX', + this.credentialTtlSeconds, ); - } - const installed = await this.redis.eval( - INSTALL_REDEEMED_CREDENTIAL_SCRIPT, - 4, - workerRedemptionKey(workerId), - credentialDigestKey(credentialDigest), - workerIdentityKey(workerId), - workerStableIdentityKey(workerId), - redemptionId, - credentialDigest, - JSON.stringify(stored), - String(this.credentialTtlSeconds), - stableIdentityId ?? '', - ); - if (installed !== 1) { - throw new BridgePairingError( - 'PAIRING_INVALID', - 'Pairing code was superseded before credential installation', + transaction.set( + workerIdentityKey(workerId), + credentialDigest, + 'EX', + this.credentialTtlSeconds, ); + await transaction.exec(); } return { workerId, credential, expiresAt }; } From 886bdd54d4ab69c42a51534735fc2dfd401838c6 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 14:11:06 -0400 Subject: [PATCH 02/18] fix: package bridge protocol in API image --- service/Dockerfile.api | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/Dockerfile.api b/service/Dockerfile.api index f1fdf9c..2921401 100644 --- a/service/Dockerfile.api +++ b/service/Dockerfile.api @@ -21,6 +21,7 @@ COPY service/src ./src COPY packages/code/src /packages/code/src COPY service/scripts ./scripts COPY shared /shared +COPY packages/code /packages/code COPY service/tsconfig.json ./ RUN bun build ./src/api-server.ts --minify --outdir .build --target bun --external '@opentelemetry/*' RUN bun build ./scripts/rehydrate-session-cache.ts --minify --outdir .build-migrations --target bun --external '@opentelemetry/*' @@ -49,6 +50,7 @@ COPY --from=install /temp/dev/node_modules node_modules COPY service/src ./src COPY packages/code/src /packages/code/src COPY shared /shared +COPY packages/code /packages/code COPY service/tsconfig.json ./ EXPOSE 3112 9230 CMD ["bun", "run", "--watch", "src/api-server.ts"] From fb08f150cc89ceaefa8784b69b7e4f2f44b4e63b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 14:29:57 -0400 Subject: [PATCH 03/18] fix: fence mixed-version pairing revocation --- .github/workflows/ci.yml | 3 +++ helm/codeapi/templates/api-deployment.yaml | 2 ++ helm/codeapi/values.yaml | 6 +++++ service/src/bridge/pairing.test.ts | 17 ++++++++++++++ service/src/bridge/pairing.ts | 27 ++++++++++++++++++++++ tests/bridge_pairing_rollout.sh | 18 +++++++++++++++ 6 files changed, 73 insertions(+) create mode 100755 tests/bridge_pairing_rollout.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94e2286..7ba2ef1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,9 @@ jobs: - name: Sandbox-runner liveness checks run: tests/sandbox_runner_healthcheck.sh + - name: Bridge pairing rollout safety + run: tests/bridge_pairing_rollout.sh + - name: Validate sandbox Dockerfiles run: | docker buildx build --check -f api/Dockerfile . diff --git a/helm/codeapi/templates/api-deployment.yaml b/helm/codeapi/templates/api-deployment.yaml index bf5f91e..ceefcb3 100644 --- a/helm/codeapi/templates/api-deployment.yaml +++ b/helm/codeapi/templates/api-deployment.yaml @@ -17,6 +17,8 @@ spec: {{- if not .Values.api.autoscaling.enabled }} replicas: {{ .Values.api.replicaCount }} {{- end }} + strategy: + {{- toYaml .Values.api.strategy | nindent 4 }} selector: matchLabels: {{- include "codeapi.api.selectorLabels" . | nindent 6 }} diff --git a/helm/codeapi/values.yaml b/helm/codeapi/values.yaml index 75abaaf..5cb7601 100644 --- a/helm/codeapi/values.yaml +++ b/helm/codeapi/values.yaml @@ -68,6 +68,12 @@ api: enabled: true replicaCount: 2 # Start with 2 API pods + # Pairing revocation relies on every serving replica honoring the Redis + # generation fence. Recreate prevents a pre-fence binary from redeeming an + # already-revoked code during the first rollout of paired bridge workers. + strategy: + type: Recreate + image: repository: codeapi-api tag: latest diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 71980ff..e2f9ac9 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -350,6 +350,23 @@ describe('RedisBridgePairingStore', () => { ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); }); + test('revocation removes pairing codes issued by a pre-fence replica', async () => { + const legacyCode = 'legacy-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ workerId: 'vm-1', expiresAt: new Date(Date.now() + 60_000).toISOString() }), + 'EX', + 60, + ); + + await pairings.revoke('vm-1'); + + await expect(redis.get(legacyKey)).resolves.toBeNull(); + }); + test('issuing a replacement invalidates the prior unredeemed pairing code', async () => { const identity = createBridgeIdentity(); const first = await pairings.issue('vm-1'); diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 7d58af8..e408467 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -320,12 +320,39 @@ export class RedisBridgePairingStore { 'EX', this.pairingTtlSeconds, ); + await this.removeLegacyPairings(workerId); const identityKey = workerIdentityKey(workerId); const credentialDigest = await this.redis.get(identityKey); if (credentialDigest == null) return; await this.redis.del(identityKey, credentialDigestKey(credentialDigest)); } + private async removeLegacyPairings(workerId: string): Promise { + let cursor = '0'; + do { + const [nextCursor, keys] = await this.redis.scan( + cursor, + 'MATCH', + `${PREFIX}:pairing:*`, + 'COUNT', + 100, + ); + cursor = nextCursor; + if (keys.length === 0) continue; + const values = await this.redis.mget(...keys); + const matching = keys.filter((_key, index) => { + const raw = values[index]; + if (raw == null) return false; + try { + return (JSON.parse(raw) as Partial).workerId === workerId; + } catch { + return false; + } + }); + if (matching.length > 0) await this.redis.del(...matching); + } while (cursor !== '0'); + } + async rotate( workerId: string, expectedCredentialId?: string, diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh new file mode 100755 index 0000000..9fe0987 --- /dev/null +++ b/tests/bridge_pairing_rollout.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +set -euo pipefail + +values=helm/codeapi/values.yaml +deployment=helm/codeapi/templates/api-deployment.yaml + +if ! grep -A 7 '^api:$' "$values" | grep -q '^ strategy:$'; then + echo 'api.strategy must be configured for pairing-safe rollouts' >&2 + exit 1 +fi +if ! grep -A 2 '^ strategy:$' "$values" | grep -q '^ type: Recreate$'; then + echo 'api.strategy.type must default to Recreate while pre-fence replicas may exist' >&2 + exit 1 +fi +if ! grep -q 'toYaml .Values.api.strategy' "$deployment"; then + echo 'the API Deployment must render api.strategy' >&2 + exit 1 +fi From c32a0ceb631871b8eb806c6f9581c55676d151eb Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 14:38:34 -0400 Subject: [PATCH 04/18] fix: redeem valid legacy pairing codes --- service/src/bridge/pairing.test.ts | 46 ++++++++++++++++++++++++++++++ service/src/bridge/pairing.ts | 12 ++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index e2f9ac9..8a5eec0 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -367,6 +367,52 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { + const identity = createBridgeIdentity(); + const legacyCode = 'unrevoked-legacy-pairing'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ workerId: 'vm-1', expiresAt: new Date(Date.now() + 60_000).toISOString() }), + 'EX', + 60, + ); + + await expect( + pairings.redeem({ + workerId: 'vm-1', + code: legacyCode, + publicKey: identity.publicKey, + }), + ).resolves.toMatchObject({ workerId: 'vm-1' }); + }); + + test('replacement invalidates a pairing code issued by a pre-fence replica', async () => { + const identity = createBridgeIdentity(); + const legacyCode = 'replaced-legacy-pairing'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ workerId: 'vm-1', expiresAt: new Date(Date.now() + 60_000).toISOString() }), + 'EX', + 60, + ); + + await pairings.issue('vm-1'); + + await expect( + pairings.redeem({ + workerId: 'vm-1', + code: legacyCode, + publicKey: identity.publicKey, + }), + ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); + }); + test('issuing a replacement invalidates the prior unredeemed pairing code', async () => { const identity = createBridgeIdentity(); const first = await pairings.issue('vm-1'); diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index e408467..9c08ec3 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -28,7 +28,13 @@ local pairing = redis.call('GET', KEYS[1]) if pairing ~= ARGV[1] then return 0 end -if redis.call('GET', KEYS[2]) ~= ARGV[2] then +local generation = redis.call('GET', KEYS[2]) +if ARGV[2] == '' then + if generation then + redis.call('DEL', KEYS[1]) + return 0 + end +elseif generation ~= ARGV[2] then redis.call('DEL', KEYS[1]) return 0 end @@ -66,7 +72,7 @@ export interface BridgeWorkerBinding { interface StoredPairing { workerId: string; expiresAt: string; - generation: string; + generation?: string; binding?: BridgeWorkerBinding; } @@ -227,7 +233,7 @@ export class RedisBridgePairingStore { workerIdentityKey(args.workerId), workerPairingIndexKey(args.workerId), raw, - pairing.generation, + pairing.generation ?? '', JSON.stringify(stored), String(this.credentialTtlSeconds), credentialDigest, From bd4a0d087878bc92d8131ecb548969828bde1d5f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 14:58:45 -0400 Subject: [PATCH 05/18] fix: harden pairing rollout compatibility --- helm/codeapi/templates/api-deployment.yaml | 2 ++ helm/codeapi/values.yaml | 1 + service/src/bridge/pairing.ts | 24 ++++++++++++++++++++++ tests/bridge_pairing_rollout.sh | 8 ++++++++ 4 files changed, 35 insertions(+) diff --git a/helm/codeapi/templates/api-deployment.yaml b/helm/codeapi/templates/api-deployment.yaml index ceefcb3..17147b2 100644 --- a/helm/codeapi/templates/api-deployment.yaml +++ b/helm/codeapi/templates/api-deployment.yaml @@ -24,6 +24,8 @@ spec: {{- include "codeapi.api.selectorLabels" . | nindent 6 }} template: metadata: + annotations: + codeapi.librechat.ai/pairing-fence-version: "1" labels: {{- include "codeapi.api.selectorLabels" . | nindent 8 }} spec: diff --git a/helm/codeapi/values.yaml b/helm/codeapi/values.yaml index 5cb7601..b0f7e18 100644 --- a/helm/codeapi/values.yaml +++ b/helm/codeapi/values.yaml @@ -73,6 +73,7 @@ api: # already-revoked code during the first rollout of paired bridge workers. strategy: type: Recreate + rollingUpdate: null image: repository: codeapi-api diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 9c08ec3..c3c775c 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -136,6 +136,10 @@ function workerPairingGenerationKey(workerId: string): string { return `${PREFIX}:pairing-generation:${workerId}`; } +function legacyPairingScanDeadlineKey(): string { + return `${PREFIX}:migration:legacy-pairing-scan-until`; +} + function proofNonceKey(credential: string, nonce: string): string { return `${PREFIX}:proof:${digest(credential)}:${digest(nonce)}`; } @@ -170,6 +174,10 @@ export class RedisBridgePairingStore { generation, binding, }; + // Pre-index binaries cannot remove a superseded code themselves. During + // the one pairing-TTL migration window, find and delete those records so + // rolling back cannot make a replaced code valid again. + await this.removeLegacyPairings(workerId); const codeKey = pairingKey(code); await this.redis.eval( ISSUE_PAIRING_SCRIPT, @@ -327,6 +335,10 @@ export class RedisBridgePairingStore { this.pairingTtlSeconds, ); await this.removeLegacyPairings(workerId); + const indexedPairing = await this.redis.get(workerPairingIndexKey(workerId)); + if (indexedPairing != null) { + await this.redis.del(indexedPairing, workerPairingIndexKey(workerId)); + } const identityKey = workerIdentityKey(workerId); const credentialDigest = await this.redis.get(identityKey); if (credentialDigest == null) return; @@ -334,6 +346,18 @@ export class RedisBridgePairingStore { } private async removeLegacyPairings(workerId: string): Promise { + const deadlineKey = legacyPairingScanDeadlineKey(); + const proposedDeadline = Date.now() + this.pairingTtlSeconds * 1000; + const initialized = await this.redis.set( + deadlineKey, + String(proposedDeadline), + 'NX', + ); + const deadline = + initialized === 'OK' + ? proposedDeadline + : Number(await this.redis.get(deadlineKey)); + if (!Number.isFinite(deadline) || Date.now() > deadline) return; let cursor = '0'; do { const [nextCursor, keys] = await this.redis.scan( diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index 9fe0987..8dd5aac 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -12,7 +12,15 @@ if ! grep -A 2 '^ strategy:$' "$values" | grep -q '^ type: Recreate$'; then echo 'api.strategy.type must default to Recreate while pre-fence replicas may exist' >&2 exit 1 fi +if ! grep -A 3 '^ strategy:$' "$values" | grep -q '^ rollingUpdate: null$'; then + echo 'api.strategy must clear rollingUpdate when switching existing deployments to Recreate' >&2 + exit 1 +fi if ! grep -q 'toYaml .Values.api.strategy' "$deployment"; then echo 'the API Deployment must render api.strategy' >&2 exit 1 fi +if ! grep -q 'codeapi.librechat.ai/pairing-fence-version: "1"' "$deployment"; then + echo 'the first pairing-fence chart upgrade must revise the API pod template' >&2 + exit 1 +fi From 36b3311fed1bf2fd7e883d3f6c578140cd3996b0 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 15:31:13 -0400 Subject: [PATCH 06/18] fix: make pairing revocation atomic --- service/src/bridge/pairing.ts | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index c3c775c..04b97df 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -58,6 +58,15 @@ redis.call('SET', KEYS[1], ARGV[2], 'EX', ARGV[4]) redis.call('DEL', KEYS[2]) return 1 `; +const REVOKE_PAIRING_SCRIPT = ` +local indexed = redis.call('GET', KEYS[1]) +redis.call('SET', KEYS[2], ARGV[1], 'EX', ARGV[2]) +if indexed then + redis.call('DEL', indexed) +end +redis.call('DEL', KEYS[1]) +return 1 +`; export type BridgePrincipalType = 'deployment' | 'tenant' | 'user' | 'role' | 'group'; @@ -326,19 +335,18 @@ export class RedisBridgePairingStore { } async revoke(workerId: string): Promise { - // Rotate the pending generation before touching active credentials so an - // unredeemed code cannot race lifecycle deletion and create a new worker. - await this.redis.set( + await this.removeLegacyPairings(workerId); + // Fence redemption and consume the currently indexed code atomically. An + // issue that linearized before this script is always removed; an issue + // that linearizes afterward installs a distinct generation and code. + await this.redis.eval( + REVOKE_PAIRING_SCRIPT, + 2, + workerPairingIndexKey(workerId), workerPairingGenerationKey(workerId), randomBytes(24).toString('base64url'), - 'EX', - this.pairingTtlSeconds, + String(this.pairingTtlSeconds), ); - await this.removeLegacyPairings(workerId); - const indexedPairing = await this.redis.get(workerPairingIndexKey(workerId)); - if (indexedPairing != null) { - await this.redis.del(indexedPairing, workerPairingIndexKey(workerId)); - } const identityKey = workerIdentityKey(workerId); const credentialDigest = await this.redis.get(identityKey); if (credentialDigest == null) return; From 758c820aa1b9375754e637aa6f6257a35e986f86 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 16:13:51 -0400 Subject: [PATCH 07/18] fix: preserve pairing identity across rollouts --- service/src/bridge/pairing.test.ts | 32 ++++++++++++++++++++++++++++++ service/src/bridge/pairing.ts | 16 +++++++++------ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 8a5eec0..15bde29 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -389,6 +389,38 @@ describe('RedisBridgePairingStore', () => { ).resolves.toMatchObject({ workerId: 'vm-1' }); }); + test('redeems an indexed legacy code issued after rollback', async () => { + const identity = createBridgeIdentity(); + await pairings.issue('vm-rollback'); + const legacyCode = 'rollback-issued-legacy-pairing'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-rollback', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + await redis.set( + 'codeapi:bridge:v1:pairing-index:vm-rollback', + legacyKey, + 'EX', + 60, + ); + + await expect( + pairings.redeem({ + workerId: 'vm-rollback', + code: legacyCode, + publicKey: identity.publicKey, + }), + ).resolves.toMatchObject({ workerId: 'vm-rollback' }); + }); + test('replacement invalidates a pairing code issued by a pre-fence replica', async () => { const identity = createBridgeIdentity(); const legacyCode = 'replaced-legacy-pairing'; diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 04b97df..3f31d50 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -30,7 +30,7 @@ if pairing ~= ARGV[1] then end local generation = redis.call('GET', KEYS[2]) if ARGV[2] == '' then - if generation then + if generation and redis.call('GET', KEYS[5]) ~= KEYS[1] then redis.call('DEL', KEYS[1]) return 0 end @@ -60,11 +60,16 @@ return 1 `; const REVOKE_PAIRING_SCRIPT = ` local indexed = redis.call('GET', KEYS[1]) +local credential = redis.call('GET', KEYS[3]) redis.call('SET', KEYS[2], ARGV[1], 'EX', ARGV[2]) if indexed then redis.call('DEL', indexed) end redis.call('DEL', KEYS[1]) +if credential then + redis.call('DEL', KEYS[3]) + redis.call('DEL', ARGV[3] .. credential) +end return 1 `; @@ -237,6 +242,7 @@ export class RedisBridgePairingStore { ).toISOString(); const stored: StoredCredential = { workerId: args.workerId, + identityId: randomBytes(18).toString('base64url'), publicKey: args.publicKey, expiresAt, binding: pairing.binding, @@ -341,16 +347,14 @@ export class RedisBridgePairingStore { // that linearizes afterward installs a distinct generation and code. await this.redis.eval( REVOKE_PAIRING_SCRIPT, - 2, + 3, workerPairingIndexKey(workerId), workerPairingGenerationKey(workerId), + workerIdentityKey(workerId), randomBytes(24).toString('base64url'), String(this.pairingTtlSeconds), + `${PREFIX}:credential:`, ); - const identityKey = workerIdentityKey(workerId); - const credentialDigest = await this.redis.get(identityKey); - if (credentialDigest == null) return; - await this.redis.del(identityKey, credentialDigestKey(credentialDigest)); } private async removeLegacyPairings(workerId: string): Promise { From 6ea76ccec9047e777b34aa2d514120d3a46ae94e Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 16:32:53 -0400 Subject: [PATCH 08/18] fix: reopen pairing cleanup after rollbacks --- service/src/bridge/pairing.test.ts | 34 ++++++++++++++++++++++++------ service/src/bridge/pairing.ts | 26 ++++++++++++++++++----- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 15bde29..e4d1f2c 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -169,16 +169,15 @@ describe('RedisBridgePairingStore', () => { }); let paused = false; redis.eval = (async (script: string, ...args: unknown[]) => { - const result = await (originalEval as (...evalArgs: unknown[]) => Promise)( - script, - ...args, - ); - if (!paused && script.includes('return pairing')) { + if (!paused && script.includes('if pairing ~= ARGV[1]')) { paused = true; firstRedeemed(); await releaseFirstPromise; } - return result; + return await (originalEval as (...evalArgs: unknown[]) => Promise)( + script, + ...args, + ); }) as typeof redis.eval; try { @@ -367,6 +366,29 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('reopens legacy cleanup after a rollback outlives the prior scan window', async () => { + const legacyCode = 'later-rollback-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + const deadlineKey = 'codeapi:bridge:v1:migration:legacy-pairing-scan-until'; + await redis.set(deadlineKey, '0'); + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-later-rollback', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + + await pairings.revoke('vm-later-rollback'); + + await expect(redis.get(legacyKey)).resolves.toBeNull(); + expect(await redis.pttl(deadlineKey)).toBeGreaterThan(0); + }); + test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { const identity = createBridgeIdentity(); const legacyCode = 'unrevoked-legacy-pairing'; diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 3f31d50..09fb47a 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -359,16 +359,32 @@ export class RedisBridgePairingStore { private async removeLegacyPairings(workerId: string): Promise { const deadlineKey = legacyPairingScanDeadlineKey(); - const proposedDeadline = Date.now() + this.pairingTtlSeconds * 1000; + const now = Date.now(); + const migrationWindowMs = this.pairingTtlSeconds * 1000; + const proposedDeadline = now + migrationWindowMs; const initialized = await this.redis.set( deadlineKey, String(proposedDeadline), + 'PX', + migrationWindowMs, 'NX', ); - const deadline = - initialized === 'OK' - ? proposedDeadline - : Number(await this.redis.get(deadlineKey)); + let deadline = proposedDeadline; + if (initialized !== 'OK') { + deadline = Number(await this.redis.get(deadlineKey)); + if (!Number.isFinite(deadline) || deadline <= now) { + deadline = proposedDeadline; + await this.redis.set( + deadlineKey, + String(deadline), + 'PX', + migrationWindowMs, + ); + } else { + // Retrofit an expiry onto markers written by the preceding release. + await this.redis.pexpire(deadlineKey, Math.max(1, deadline - now)); + } + } if (!Number.isFinite(deadline) || Date.now() > deadline) return; let cursor = '0'; do { From 310e129c9e076fe35ae0f4927ba8027c2beeaf2f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 16:49:15 -0400 Subject: [PATCH 09/18] fix: bound legacy pairing migration scans --- service/src/bridge/pairing.test.ts | 31 +++++++++++++- service/src/bridge/pairing.ts | 65 ++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index e4d1f2c..4df14c6 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -382,11 +382,40 @@ describe('RedisBridgePairingStore', () => { 'EX', 60, ); + await redis.set( + 'codeapi:bridge:v1:pairing-index:vm-later-rollback', + legacyKey, + 'EX', + 60, + ); await pairings.revoke('vm-later-rollback'); await expect(redis.get(legacyKey)).resolves.toBeNull(); - expect(await redis.pttl(deadlineKey)).toBeGreaterThan(0); + await expect(redis.get(deadlineKey)).resolves.toBe('0'); + }); + + test('does not restart an expired migration window without rollback evidence', async () => { + const legacyCode = 'unindexed-post-migration-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + const deadlineKey = 'codeapi:bridge:v1:migration:legacy-pairing-scan-until'; + await redis.set(deadlineKey, '0'); + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-no-rollback-signal', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + + await pairings.revoke('vm-no-rollback-signal'); + + await expect(redis.get(legacyKey)).resolves.not.toBeNull(); + await expect(redis.get(deadlineKey)).resolves.toBe('0'); }); test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 09fb47a..bebca08 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -154,6 +154,10 @@ function legacyPairingScanDeadlineKey(): string { return `${PREFIX}:migration:legacy-pairing-scan-until`; } +function legacyPairingWorkerScanKey(workerId: string): string { + return `${PREFIX}:migration:legacy-pairing-scanned:${workerId}`; +} + function proofNonceKey(credential: string, nonce: string): string { return `${PREFIX}:proof:${digest(credential)}:${digest(nonce)}`; } @@ -362,30 +366,47 @@ export class RedisBridgePairingStore { const now = Date.now(); const migrationWindowMs = this.pairingTtlSeconds * 1000; const proposedDeadline = now + migrationWindowMs; - const initialized = await this.redis.set( - deadlineKey, - String(proposedDeadline), - 'PX', - migrationWindowMs, - 'NX', - ); - let deadline = proposedDeadline; - if (initialized !== 'OK') { - deadline = Number(await this.redis.get(deadlineKey)); - if (!Number.isFinite(deadline) || deadline <= now) { - deadline = proposedDeadline; - await this.redis.set( - deadlineKey, - String(deadline), - 'PX', - migrationWindowMs, - ); - } else { - // Retrofit an expiry onto markers written by the preceding release. - await this.redis.pexpire(deadlineKey, Math.max(1, deadline - now)); + let rawDeadline = await this.redis.get(deadlineKey); + if (rawDeadline == null) { + const initialized = await this.redis.set( + deadlineKey, + String(proposedDeadline), + 'NX', + ); + rawDeadline = initialized === 'OK' + ? String(proposedDeadline) + : await this.redis.get(deadlineKey); + } else if ((await this.redis.pttl(deadlineKey)) > 0) { + // Markers from the preceding build expired and reopened forever. Keep + // their original deadline, but make it durable so normal idle periods + // cannot start another migration window. + await this.redis.persist(deadlineKey); + } + + const indexedKey = await this.redis.get(workerPairingIndexKey(workerId)); + const indexedRaw = indexedKey == null ? null : await this.redis.get(indexedKey); + let rollbackDetected = false; + if (indexedRaw != null) { + try { + rollbackDetected = (JSON.parse(indexedRaw) as StoredPairing).generation == null; + } catch { + rollbackDetected = false; } } - if (!Number.isFinite(deadline) || Date.now() > deadline) return; + + const deadline = Number(rawDeadline); + if (!rollbackDetected) { + if (!Number.isFinite(deadline) || now > deadline) return; + const claimed = await this.redis.set( + legacyPairingWorkerScanKey(workerId), + '1', + 'PX', + Math.max(1, deadline - now), + 'NX', + ); + if (claimed !== 'OK') return; + } + let cursor = '0'; do { const [nextCursor, keys] = await this.redis.scan( From dd7fe02459003429f88e9cc88053b7d2f658d9b4 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 17:19:30 -0400 Subject: [PATCH 10/18] fix: retry interrupted pairing migrations --- service/src/bridge/pairing.test.ts | 33 ++++++++++++++ service/src/bridge/pairing.ts | 73 ++++++++++++++++++++---------- 2 files changed, 81 insertions(+), 25 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 4df14c6..cc884cf 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -366,6 +366,39 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('retries legacy cleanup after a transient scan failure', async () => { + const legacyCode = 'retryable-legacy-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-scan-retry', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + const scan = redis.scan.bind(redis); + let failScan = true; + redis.scan = (async (...args: Parameters) => { + if (failScan) { + failScan = false; + throw new Error('transient scan failure'); + } + return scan(...args); + }) as Redis['scan']; + + await expect(pairings.revoke('vm-scan-retry')).rejects.toThrow( + 'transient scan failure', + ); + redis.scan = scan; + await pairings.revoke('vm-scan-retry'); + + await expect(redis.get(legacyKey)).resolves.toBeNull(); + }); + test('reopens legacy cleanup after a rollback outlives the prior scan window', async () => { const legacyCode = 'later-rollback-pairing-code'; const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index bebca08..bd4572d 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -395,41 +395,64 @@ export class RedisBridgePairingStore { } const deadline = Number(rawDeadline); + let scanClaim: + | { key: string; token: string } + | undefined; if (!rollbackDetected) { if (!Number.isFinite(deadline) || now > deadline) return; + const key = legacyPairingWorkerScanKey(workerId); + const token = randomBytes(24).toString('base64url'); const claimed = await this.redis.set( - legacyPairingWorkerScanKey(workerId), - '1', + key, + token, 'PX', Math.max(1, deadline - now), 'NX', ); if (claimed !== 'OK') return; + scanClaim = { key, token }; } - let cursor = '0'; - do { - const [nextCursor, keys] = await this.redis.scan( - cursor, - 'MATCH', - `${PREFIX}:pairing:*`, - 'COUNT', - 100, - ); - cursor = nextCursor; - if (keys.length === 0) continue; - const values = await this.redis.mget(...keys); - const matching = keys.filter((_key, index) => { - const raw = values[index]; - if (raw == null) return false; - try { - return (JSON.parse(raw) as Partial).workerId === workerId; - } catch { - return false; - } - }); - if (matching.length > 0) await this.redis.del(...matching); - } while (cursor !== '0'); + try { + let cursor = '0'; + do { + const [nextCursor, keys] = await this.redis.scan( + cursor, + 'MATCH', + `${PREFIX}:pairing:*`, + 'COUNT', + 100, + ); + cursor = nextCursor; + if (keys.length === 0) continue; + const values = await this.redis.mget(...keys); + const matching = keys.filter((_key, index) => { + const raw = values[index]; + if (raw == null) return false; + try { + return (JSON.parse(raw) as Partial).workerId === workerId; + } catch { + return false; + } + }); + if (matching.length > 0) await this.redis.del(...matching); + } while (cursor !== '0'); + } catch (error) { + if (scanClaim != null) { + await this.redis.eval( + [ + "if redis.call('GET', KEYS[1]) == ARGV[1] then", + " return redis.call('DEL', KEYS[1])", + 'end', + 'return 0', + ].join('\n'), + 1, + scanClaim.key, + scanClaim.token, + ); + } + throw error; + } } async rotate( From 83d9315ae28156311677cea5f755650572cca193 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 18:00:48 -0400 Subject: [PATCH 11/18] fix: harden pairing migration cleanup --- service/src/bridge/pairing.test.ts | 98 ++++++++++++++++++++++++++++++ service/src/bridge/pairing.ts | 79 ++++++++++++++++++------ 2 files changed, 159 insertions(+), 18 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index cc884cf..e6b05ec 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -124,6 +124,27 @@ describe('RedisBridgePairingStore', () => { ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); }); + test('preserves a pairing code after public-key validation fails', async () => { + const identity = createBridgeIdentity(); + const pairing = await pairings.issue('vm-public-key-retry'); + + await expect( + pairings.redeem({ + workerId: 'vm-public-key-retry', + code: pairing.code, + publicKey: 'not-a-public-key', + }), + ).rejects.toMatchObject({ code: 'PUBLIC_KEY_INVALID' }); + + await expect( + pairings.redeem({ + workerId: 'vm-public-key-retry', + code: pairing.code, + publicKey: identity.publicKey, + }), + ).resolves.toMatchObject({ workerId: 'vm-public-key-retry' }); + }); + test('only the newest pairing code can rebind a worker identity', async () => { const identity = createBridgeIdentity(); const older = await pairings.issue('vm-1', { @@ -399,6 +420,83 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('waits for a failed in-progress cleanup and confirms legacy removal itself', async () => { + const legacyCode = 'overlapping-legacy-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-overlapping-cleanup', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + const scan = redis.scan.bind(redis); + let releaseFirstScan = () => {}; + const firstScanGate = new Promise((resolve) => { + releaseFirstScan = resolve; + }); + let markFirstScanStarted = () => {}; + const firstScanStarted = new Promise((resolve) => { + markFirstScanStarted = resolve; + }); + let scanCalls = 0; + redis.scan = (async (...args: Parameters) => { + scanCalls += 1; + if (scanCalls === 1) { + markFirstScanStarted(); + await firstScanGate; + throw new Error('interrupted claimed scan'); + } + return scan(...args); + }) as Redis['scan']; + + const interrupted = pairings.revoke('vm-overlapping-cleanup'); + await firstScanStarted; + const overlapping = pairings.revoke('vm-overlapping-cleanup'); + let overlappingSettled = false; + void overlapping.then( + () => { + overlappingSettled = true; + }, + () => { + overlappingSettled = true; + }, + ); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(overlappingSettled).toBe(false); + releaseFirstScan(); + + await expect(interrupted).rejects.toThrow('interrupted claimed scan'); + await expect(overlapping).resolves.toBeUndefined(); + redis.scan = scan; + await expect(redis.get(legacyKey)).resolves.toBeNull(); + }); + + test('legacy cleanup does not delete generation-fenced pairings', async () => { + const fencedCode = 'concurrent-generation-pairing'; + const fencedKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(fencedCode) + .digest('hex')}`; + await redis.set( + fencedKey, + JSON.stringify({ + workerId: 'vm-generation-fenced', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + generation: 'new-generation', + }), + 'EX', + 60, + ); + + await pairings.revoke('vm-generation-fenced'); + + await expect(redis.get(fencedKey)).resolves.not.toBeNull(); + }); + test('reopens legacy cleanup after a rollback outlives the prior scan window', async () => { const legacyCode = 'later-rollback-pairing-code'; const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index bd4572d..ab8446e 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -13,6 +13,9 @@ const DEFAULT_PAIRING_TTL_SECONDS = 10 * 60; const DEFAULT_CREDENTIAL_TTL_SECONDS = 5 * 60; const PROOF_NONCE_TTL_SECONDS = 2 * 60; const PROOF_CLOCK_SKEW_MS = 60_000; +const LEGACY_SCAN_CLAIM_TTL_MS = 5_000; +const LEGACY_SCAN_POLL_INTERVAL_MS = 25; +const LEGACY_SCAN_COMPLETE = 'done'; const ISSUE_PAIRING_SCRIPT = ` local previous = redis.call('GET', KEYS[1]) if previous then @@ -72,6 +75,19 @@ if credential then end return 1 `; +const RELEASE_LEGACY_SCAN_CLAIM_SCRIPT = ` +if redis.call('GET', KEYS[1]) == ARGV[1] then + return redis.call('DEL', KEYS[1]) +end +return 0 +`; +const COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT = ` +if redis.call('GET', KEYS[1]) == ARGV[1] then + redis.call('SET', KEYS[1], ARGV[2], 'PX', ARGV[3]) + return 1 +end +return 0 +`; export type BridgePrincipalType = 'deployment' | 'tenant' | 'user' | 'role' | 'group'; @@ -232,7 +248,6 @@ export class RedisBridgePairingStore { ); } if (!validEd25519PublicKey(args.publicKey)) { - await this.redis.del(codeKey); throw new BridgePairingError( 'PUBLIC_KEY_INVALID', 'Worker public key must be an Ed25519 key', @@ -401,16 +416,35 @@ export class RedisBridgePairingStore { if (!rollbackDetected) { if (!Number.isFinite(deadline) || now > deadline) return; const key = legacyPairingWorkerScanKey(workerId); - const token = randomBytes(24).toString('base64url'); - const claimed = await this.redis.set( - key, - token, - 'PX', - Math.max(1, deadline - now), - 'NX', - ); - if (claimed !== 'OK') return; - scanClaim = { key, token }; + while (scanClaim == null) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return; + const existing = await this.redis.get(key); + if ( + existing === LEGACY_SCAN_COMPLETE || + (existing != null && !existing.startsWith('claim:')) + ) { + return; + } + const token = `claim:${randomBytes(24).toString('base64url')}`; + const claimed = await this.redis.set( + key, + token, + 'PX', + Math.max(1, Math.min(LEGACY_SCAN_CLAIM_TTL_MS, remainingMs)), + 'NX', + ); + if (claimed === 'OK') { + scanClaim = { key, token }; + break; + } + await new Promise((resolve) => + setTimeout( + resolve, + Math.max(1, Math.min(LEGACY_SCAN_POLL_INTERVAL_MS, remainingMs)), + ), + ); + } } try { @@ -430,22 +464,31 @@ export class RedisBridgePairingStore { const raw = values[index]; if (raw == null) return false; try { - return (JSON.parse(raw) as Partial).workerId === workerId; + const pairing = JSON.parse(raw) as Partial; + return pairing.workerId === workerId && pairing.generation == null; } catch { return false; } }); if (matching.length > 0) await this.redis.del(...matching); } while (cursor !== '0'); + if (scanClaim != null) { + const completed = await this.redis.eval( + COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT, + 1, + scanClaim.key, + scanClaim.token, + LEGACY_SCAN_COMPLETE, + String(Math.max(1, deadline - Date.now())), + ); + if (completed !== 1) { + await this.removeLegacyPairings(workerId); + } + } } catch (error) { if (scanClaim != null) { await this.redis.eval( - [ - "if redis.call('GET', KEYS[1]) == ARGV[1] then", - " return redis.call('DEL', KEYS[1])", - 'end', - 'return 0', - ].join('\n'), + RELEASE_LEGACY_SCAN_CLAIM_SCRIPT, 1, scanClaim.key, scanClaim.token, From 6178e1550ade4f8c3fd6254acde15e8bf0b6385d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 18:19:01 -0400 Subject: [PATCH 12/18] fix: make pairing cleanup recoverable --- helm/codeapi/values.yaml | 4 +- service/src/bridge/pairing.test.ts | 144 +++++++++++++++++++++++ service/src/bridge/pairing.ts | 179 ++++++++++++++++++++--------- tests/bridge_pairing_rollout.sh | 4 + 4 files changed, 276 insertions(+), 55 deletions(-) diff --git a/helm/codeapi/values.yaml b/helm/codeapi/values.yaml index b0f7e18..a052152 100644 --- a/helm/codeapi/values.yaml +++ b/helm/codeapi/values.yaml @@ -78,7 +78,9 @@ api: image: repository: codeapi-api tag: latest - pullPolicy: IfNotPresent # Use Always in production + # Recreate is a security fence only if replacement pods cannot reuse a + # cached pre-fence image behind the mutable default tag. + pullPolicy: Always # Resource limits resources: diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index e6b05ec..0ffcc9e 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -420,6 +420,146 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('retries a claimed cleanup after the migration deadline', async () => { + const legacyCode = 'post-deadline-retry-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-post-deadline-retry', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + await redis.set( + 'codeapi:bridge:v1:migration:legacy-pairing-scan-until', + String(Date.now() + 15), + ); + const scan = redis.scan.bind(redis); + let failScan = true; + redis.scan = (async (...args: Parameters) => { + if (failScan) { + failScan = false; + await new Promise((resolve) => setTimeout(resolve, 30)); + throw new Error('scan failed after deadline'); + } + return scan(...args); + }) as Redis['scan']; + + await expect(pairings.revoke('vm-post-deadline-retry')).rejects.toThrow( + 'scan failed after deadline', + ); + await pairings.revoke('vm-post-deadline-retry'); + + redis.scan = scan; + await expect(redis.get(legacyKey)).resolves.toBeNull(); + }); + + test('renews the cleanup claim while a shared-keyspace scan is in flight', async () => { + const store = new RedisBridgePairingStore(redis, 600, 300, 30); + const legacyCode = 'renewed-claim-pairing-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-renewed-claim', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + const scan = redis.scan.bind(redis); + let scanCalls = 0; + let renewCalls = 0; + let markScanStarted = () => {}; + const scanStarted = new Promise((resolve) => { + markScanStarted = resolve; + }); + redis.scan = (async (...args: Parameters) => { + scanCalls += 1; + if (scanCalls === 1) { + markScanStarted(); + await new Promise((resolve) => setTimeout(resolve, 80)); + } + return scan(...args); + }) as Redis['scan']; + const originalEval = redis.eval.bind(redis); + redis.eval = (async (script: string, ...args: unknown[]) => { + if (script.includes("redis.call('PEXPIRE'")) renewCalls += 1; + return await (originalEval as (...evalArgs: unknown[]) => Promise)( + script, + ...args, + ); + }) as typeof redis.eval; + + try { + const first = store.revoke('vm-renewed-claim'); + await scanStarted; + await new Promise((resolve) => setTimeout(resolve, 45)); + expect(renewCalls).toBeGreaterThan(0); + await first; + + expect(scanCalls).toBe(1); + await expect(redis.get(legacyKey)).resolves.toBeNull(); + } finally { + redis.scan = scan; + redis.eval = originalEval as typeof redis.eval; + } + }); + + test('rescans an ambiguous marker written by the preceding build', async () => { + const legacyCode = 'ambiguous-predecessor-marker-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + const workerId = 'vm-ambiguous-predecessor-marker'; + await redis.set( + legacyKey, + JSON.stringify({ + workerId, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + await redis.set( + `codeapi:bridge:v1:migration:legacy-pairing-scanned:${workerId}`, + 'predecessor-random-token', + 'PX', + 60_000, + ); + + await pairings.revoke(workerId); + + await expect(redis.get(legacyKey)).resolves.toBeNull(); + }); + + test('starts the advertised pairing lifetime after legacy cleanup', async () => { + const store = new RedisBridgePairingStore(redis, 60); + const scan = redis.scan.bind(redis); + const originalNow = Date.now; + let now = originalNow(); + Date.now = () => now; + redis.scan = (async (...args: Parameters) => { + const result = await scan(...args); + now += 10; + return result; + }) as Redis['scan']; + + try { + const pairing = await store.issue('vm-post-cleanup-expiry'); + expect(Date.parse(pairing.expiresAt) - Date.now()).toBe(60_000); + } finally { + Date.now = originalNow; + redis.scan = scan; + } + }); + test('waits for a failed in-progress cleanup and confirms legacy removal itself', async () => { const legacyCode = 'overlapping-legacy-pairing-code'; const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') @@ -519,6 +659,10 @@ describe('RedisBridgePairingStore', () => { 'EX', 60, ); + await redis.set( + 'codeapi:bridge:v1:migration:legacy-pairing-scanned:vm-later-rollback', + 'done', + ); await pairings.revoke('vm-later-rollback'); diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index ab8446e..2b5d465 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -15,6 +15,7 @@ const PROOF_NONCE_TTL_SECONDS = 2 * 60; const PROOF_CLOCK_SKEW_MS = 60_000; const LEGACY_SCAN_CLAIM_TTL_MS = 5_000; const LEGACY_SCAN_POLL_INTERVAL_MS = 25; +const LEGACY_SCAN_PENDING = 'pending'; const LEGACY_SCAN_COMPLETE = 'done'; const ISSUE_PAIRING_SCRIPT = ` local previous = redis.call('GET', KEYS[1]) @@ -81,9 +82,28 @@ if redis.call('GET', KEYS[1]) == ARGV[1] then end return 0 `; -const COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT = ` +const NORMALIZE_LEGACY_SCAN_STATE_SCRIPT = ` +if redis.call('GET', KEYS[1]) == ARGV[1] then + redis.call('SET', KEYS[1], ARGV[2]) + return 1 +end +return 0 +`; +const RENEW_LEGACY_SCAN_CLAIM_SCRIPT = ` if redis.call('GET', KEYS[1]) == ARGV[1] then - redis.call('SET', KEYS[1], ARGV[2], 'PX', ARGV[3]) + return redis.call('PEXPIRE', KEYS[1], ARGV[2]) +end +return 0 +`; +const COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT = ` +if redis.call('GET', KEYS[2]) == ARGV[1] then + local remaining = tonumber(ARGV[3]) + if remaining > 0 then + redis.call('SET', KEYS[1], ARGV[2], 'PX', remaining) + else + redis.call('DEL', KEYS[1]) + end + redis.call('DEL', KEYS[2]) return 1 end return 0 @@ -191,12 +211,17 @@ export class RedisBridgePairingStore { private readonly redis: Redis, private readonly pairingTtlSeconds = DEFAULT_PAIRING_TTL_SECONDS, private readonly credentialTtlSeconds = DEFAULT_CREDENTIAL_TTL_SECONDS, + private readonly legacyScanClaimTtlMs = LEGACY_SCAN_CLAIM_TTL_MS, ) {} async issue( workerId: string, binding?: BridgeWorkerBinding, ): Promise { + // Pre-index binaries cannot remove a superseded code themselves. During + // the one pairing-TTL migration window, find and delete those records so + // rolling back cannot make a replaced code valid again. + await this.removeLegacyPairings(workerId); const code = randomBytes(24).toString('base64url'); const generation = randomBytes(24).toString('base64url'); const expiresAt = new Date( @@ -208,10 +233,6 @@ export class RedisBridgePairingStore { generation, binding, }; - // Pre-index binaries cannot remove a superseded code themselves. During - // the one pairing-TTL migration window, find and delete those records so - // rolling back cannot make a replaced code valid again. - await this.removeLegacyPairings(workerId); const codeKey = pairingKey(code); await this.redis.eval( ISSUE_PAIRING_SCRIPT, @@ -410,43 +431,89 @@ export class RedisBridgePairingStore { } const deadline = Number(rawDeadline); - let scanClaim: - | { key: string; token: string } - | undefined; - if (!rollbackDetected) { - if (!Number.isFinite(deadline) || now > deadline) return; - const key = legacyPairingWorkerScanKey(workerId); - while (scanClaim == null) { - const remainingMs = deadline - Date.now(); - if (remainingMs <= 0) return; - const existing = await this.redis.get(key); + const stateKey = legacyPairingWorkerScanKey(workerId); + while (true) { + const state = await this.redis.get(stateKey); + if (state === LEGACY_SCAN_COMPLETE && !rollbackDetected) return; + if (state === LEGACY_SCAN_PENDING) break; + if (state == null) { if ( - existing === LEGACY_SCAN_COMPLETE || - (existing != null && !existing.startsWith('claim:')) + !rollbackDetected && + (!Number.isFinite(deadline) || Date.now() > deadline) ) { return; } - const token = `claim:${randomBytes(24).toString('base64url')}`; - const claimed = await this.redis.set( - key, - token, - 'PX', - Math.max(1, Math.min(LEGACY_SCAN_CLAIM_TTL_MS, remainingMs)), + const initialized = await this.redis.set( + stateKey, + LEGACY_SCAN_PENDING, 'NX', ); - if (claimed === 'OK') { - scanClaim = { key, token }; - break; - } - await new Promise((resolve) => - setTimeout( - resolve, - Math.max(1, Math.min(LEGACY_SCAN_POLL_INTERVAL_MS, remainingMs)), - ), - ); + if (initialized === 'OK') break; + continue; + } + // Predecessor builds stored an unqualified random token before scanning. + // It cannot prove whether that scan completed, so normalize it to a + // durable retry requirement instead of treating it as success. + const normalized = await this.redis.eval( + NORMALIZE_LEGACY_SCAN_STATE_SCRIPT, + 1, + stateKey, + state, + LEGACY_SCAN_PENDING, + ); + if (normalized === 1) break; + } + + const claimKey = `${stateKey}:claim`; + let scanClaim: { key: string; token: string } | undefined; + while (scanClaim == null) { + const state = await this.redis.get(stateKey); + if (state === LEGACY_SCAN_COMPLETE || state == null) return; + const token = `claim:${randomBytes(24).toString('base64url')}`; + const claimed = await this.redis.set( + claimKey, + token, + 'PX', + Math.max(1, this.legacyScanClaimTtlMs), + 'NX', + ); + if (claimed === 'OK') { + scanClaim = { key: claimKey, token }; + break; } + await new Promise((resolve) => + setTimeout(resolve, LEGACY_SCAN_POLL_INTERVAL_MS), + ); } + let renewalError: unknown; + let renewal = Promise.resolve(); + let renewalInFlight = false; + const renewClaim = async (): Promise => { + const renewed = await this.redis.eval( + RENEW_LEGACY_SCAN_CLAIM_SCRIPT, + 1, + scanClaim.key, + scanClaim.token, + String(Math.max(1, this.legacyScanClaimTtlMs)), + ); + if (renewed !== 1) { + throw new Error('Legacy pairing cleanup claim was lost'); + } + }; + const renewalTimer = setInterval(() => { + if (renewalInFlight || renewalError != null) return; + renewalInFlight = true; + renewal = renewClaim() + .catch((error: unknown) => { + renewalError = error; + }) + .finally(() => { + renewalInFlight = false; + }); + }, Math.max(1, Math.floor(this.legacyScanClaimTtlMs / 3))); + renewalTimer.unref?.(); + try { let cursor = '0'; do { @@ -457,6 +524,7 @@ export class RedisBridgePairingStore { 'COUNT', 100, ); + if (renewalError != null) throw renewalError; cursor = nextCursor; if (keys.length === 0) continue; const values = await this.redis.mget(...keys); @@ -472,28 +540,31 @@ export class RedisBridgePairingStore { }); if (matching.length > 0) await this.redis.del(...matching); } while (cursor !== '0'); - if (scanClaim != null) { - const completed = await this.redis.eval( - COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT, - 1, - scanClaim.key, - scanClaim.token, - LEGACY_SCAN_COMPLETE, - String(Math.max(1, deadline - Date.now())), - ); - if (completed !== 1) { - await this.removeLegacyPairings(workerId); - } + clearInterval(renewalTimer); + await renewal; + if (renewalError != null) throw renewalError; + await renewClaim(); + const completed = await this.redis.eval( + COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT, + 2, + stateKey, + scanClaim.key, + scanClaim.token, + LEGACY_SCAN_COMPLETE, + String(deadline - Date.now()), + ); + if (completed !== 1) { + await this.removeLegacyPairings(workerId); } } catch (error) { - if (scanClaim != null) { - await this.redis.eval( - RELEASE_LEGACY_SCAN_CLAIM_SCRIPT, - 1, - scanClaim.key, - scanClaim.token, - ); - } + clearInterval(renewalTimer); + await renewal; + await this.redis.eval( + RELEASE_LEGACY_SCAN_CLAIM_SCRIPT, + 1, + scanClaim.key, + scanClaim.token, + ); throw error; } } diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index 8dd5aac..fa5703e 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -24,3 +24,7 @@ if ! grep -q 'codeapi.librechat.ai/pairing-fence-version: "1"' "$deployment"; th echo 'the first pairing-fence chart upgrade must revise the API pod template' >&2 exit 1 fi +if ! grep -A 8 '^ image:$' "$values" | grep -q '^ pullPolicy: Always$'; then + echo 'the fenced API rollout must pull the current image even when the default tag is mutable' >&2 + exit 1 +fi From d3c61292c49f795f42f7f2910692845ee14b110a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 21:30:14 -0400 Subject: [PATCH 13/18] fix: preserve pairing recovery across lifecycle rollout --- service/src/bridge/pairing.test.ts | 50 +++++++++++++++++++-- service/src/bridge/pairing.ts | 70 +++++++++++++++++++++++++----- 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 0ffcc9e..ce1dc1a 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -64,7 +64,10 @@ describe('RedisBridgePairingStore', () => { ); await expect( pairings.authorize(requestFor(issued.credential, 'superseded-bound-proof')), - ).rejects.toMatchObject({ code: 'CREDENTIAL_INVALID' }); + ).resolves.toMatchObject({ + workerId: 'vm-bound', + identityId: originalAuthorization.identityId, + }); }); test('preserves a legacy unmarked identity across its first rotation', async () => { @@ -792,7 +795,7 @@ describe('RedisBridgePairingStore', () => { ).resolves.toMatchObject({ workerId: 'vm-1' }); }); - test('rotation replaces rather than duplicates the active credential', async () => { + test('rotation retains the prior same-identity credential for recovery', async () => { const identity = createBridgeIdentity(); const pairing = await pairings.issue('vm-1'); const original = await pairings.redeem({ @@ -824,12 +827,53 @@ describe('RedisBridgePairingStore', () => { await expect( pairings.authorize(proofFor(original.credential, 'old-credential')), - ).rejects.toMatchObject({ code: 'CREDENTIAL_INVALID' }); + ).resolves.toMatchObject({ workerId: 'vm-1' }); await expect( pairings.authorize(proofFor(rotated.credential, 'new-credential')), ).resolves.toMatchObject({ workerId: 'vm-1' }); }); + test('recovers when a refresh response is lost after the server commits it', async () => { + const identity = createBridgeIdentity(); + const pairing = await pairings.issue('vm-1'); + const original = await pairings.redeem({ + workerId: 'vm-1', + code: pairing.code, + publicKey: identity.publicKey, + }); + const proofFor = ( + credential: string, + nonce: string, + ): Parameters[0] => { + const proof = { + credential, + method: 'POST', + path: '/v1/bridge/workers/vm-1/credentials/refresh', + timestamp: new Date().toISOString(), + nonce, + body: JSON.stringify({ protocolVersion: 1 }), + }; + return { + ...proof, + workerId: 'vm-1', + signature: signBridgeRequest(identity.privateKey, proof), + }; + }; + + await pairings.rotate('vm-1'); + const retryAuthorization = await pairings.authorize( + proofFor(original.credential, 'refresh-response-lost'), + ); + const recovered = await pairings.rotate( + 'vm-1', + retryAuthorization.credentialId, + ); + + await expect( + pairings.authorize(proofFor(recovered.credential, 'refresh-recovered')), + ).resolves.toMatchObject({ workerId: 'vm-1' }); + }); + test('rejects a stale credential refresh after the worker is paired again', async () => { const originalIdentity = createBridgeIdentity(); const originalPairing = await pairings.issue('vm-1'); diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 2b5d465..3e54502 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -10,7 +10,7 @@ import { verifyBridgeRequest } from '../../../packages/code/src/identity'; const PREFIX = 'codeapi:bridge:v1'; const DEFAULT_PAIRING_TTL_SECONDS = 10 * 60; -const DEFAULT_CREDENTIAL_TTL_SECONDS = 5 * 60; +const DEFAULT_CREDENTIAL_TTL_SECONDS = 15 * 60; const PROOF_NONCE_TTL_SECONDS = 2 * 60; const PROOF_CLOCK_SKEW_MS = 60_000; const LEGACY_SCAN_CLAIM_TTL_MS = 5_000; @@ -48,18 +48,27 @@ if redis.call('GET', KEYS[5]) == KEYS[1] then end redis.call('SET', KEYS[3], ARGV[3], 'EX', ARGV[4]) redis.call('SET', KEYS[4], ARGV[5], 'EX', ARGV[4]) +redis.call('SET', KEYS[6], ARGV[6], 'EX', ARGV[4]) return 1 `; const ROTATE_CREDENTIAL_SCRIPT = ` -if redis.call('GET', KEYS[1]) ~= ARGV[1] then +local activeDigest = redis.call('GET', KEYS[1]) +local previous = redis.call('GET', KEYS[2]) +if not activeDigest or not previous then return 0 end -if redis.call('EXISTS', KEYS[2]) ~= 1 then - return 0 +if activeDigest ~= ARGV[1] then + if ARGV[5] == '' or redis.call('GET', KEYS[4]) ~= ARGV[5] then + return 0 + end end redis.call('SET', KEYS[3], ARGV[3], 'EX', ARGV[4]) redis.call('SET', KEYS[1], ARGV[2], 'EX', ARGV[4]) -redis.call('DEL', KEYS[2]) +if ARGV[5] ~= '' then + redis.call('SET', KEYS[4], ARGV[5], 'EX', ARGV[4]) +else + redis.call('DEL', KEYS[4]) +end return 1 `; const REVOKE_PAIRING_SCRIPT = ` @@ -74,6 +83,7 @@ if credential then redis.call('DEL', KEYS[3]) redis.call('DEL', ARGV[3] .. credential) end +redis.call('DEL', KEYS[4]) return 1 `; const RELEASE_LEGACY_SCAN_CLAIM_SCRIPT = ` @@ -178,6 +188,10 @@ function workerIdentityKey(workerId: string): string { return `${PREFIX}:identity:${workerId}`; } +function workerStableIdentityKey(workerId: string): string { + return `${PREFIX}:stable-identity:${workerId}`; +} + function workerPairingIndexKey(workerId: string): string { return `${PREFIX}:pairing-index:${workerId}`; } @@ -280,26 +294,29 @@ export class RedisBridgePairingStore { const expiresAt = new Date( Date.now() + this.credentialTtlSeconds * 1000, ).toISOString(); + const identityId = randomBytes(18).toString('base64url'); const stored: StoredCredential = { workerId: args.workerId, - identityId: randomBytes(18).toString('base64url'), + identityId, publicKey: args.publicKey, expiresAt, binding: pairing.binding, }; const accepted = await this.redis.eval( REDEEM_PAIRING_SCRIPT, - 5, + 6, codeKey, workerPairingGenerationKey(pairing.workerId), credentialDigestKey(credentialDigest), workerIdentityKey(args.workerId), workerPairingIndexKey(args.workerId), + workerStableIdentityKey(args.workerId), raw, pairing.generation ?? '', JSON.stringify(stored), String(this.credentialTtlSeconds), credentialDigest, + identityId, ); if (accepted !== 1) { throw new BridgePairingError( @@ -322,6 +339,7 @@ export class RedisBridgePairingStore { }): Promise<{ workerId: string; credentialId: string; + activeCredentialId: string; identityId?: string; binding?: BridgeWorkerBinding; }> { @@ -340,13 +358,31 @@ export class RedisBridgePairingStore { credentialDigestKey(credentialDigest), workerIdentityKey(args.workerId), ); - if (raw == null || activeDigest !== credentialDigest) { + if (raw == null || activeDigest == null) { throw new BridgePairingError( 'CREDENTIAL_INVALID', 'Worker credential is invalid or expired', ); } const stored = JSON.parse(raw) as StoredCredential; + if (activeDigest !== credentialDigest) { + const activeRaw = await this.redis.get( + credentialDigestKey(activeDigest), + ); + const active = activeRaw == null + ? undefined + : JSON.parse(activeRaw) as StoredCredential; + if ( + stored.identityId == null || + active?.identityId == null || + stored.identityId !== active.identityId + ) { + throw new BridgePairingError( + 'CREDENTIAL_INVALID', + 'Worker credential is invalid or expired', + ); + } + } if (stored.workerId !== args.workerId) { throw new BridgePairingError( 'CREDENTIAL_INVALID', @@ -375,6 +411,7 @@ export class RedisBridgePairingStore { return { workerId: stored.workerId, credentialId: credentialDigest, + activeCredentialId: activeDigest, ...(stored.identityId != null ? { identityId: stored.identityId } : {}), ...(stored.binding ? { binding: stored.binding } : {}), }; @@ -387,10 +424,11 @@ export class RedisBridgePairingStore { // that linearizes afterward installs a distinct generation and code. await this.redis.eval( REVOKE_PAIRING_SCRIPT, - 3, + 4, workerPairingIndexKey(workerId), workerPairingGenerationKey(workerId), workerIdentityKey(workerId), + workerStableIdentityKey(workerId), randomBytes(24).toString('base64url'), String(this.pairingTtlSeconds), `${PREFIX}:credential:`, @@ -622,14 +660,16 @@ export class RedisBridgePairingStore { if (previousDigest !== undefined) { const rotated = await this.redis.eval( ROTATE_CREDENTIAL_SCRIPT, - 3, + 4, workerIdentityKey(workerId), credentialDigestKey(previousDigest), credentialDigestKey(credentialDigest), + workerStableIdentityKey(workerId), previousDigest, credentialDigest, JSON.stringify(stored), String(this.credentialTtlSeconds), + stableIdentityId ?? '', ); if (rotated !== 1) { throw new BridgePairingError( @@ -651,6 +691,16 @@ export class RedisBridgePairingStore { 'EX', this.credentialTtlSeconds, ); + if (stableIdentityId != null) { + transaction.set( + workerStableIdentityKey(workerId), + stableIdentityId, + 'EX', + this.credentialTtlSeconds, + ); + } else { + transaction.del(workerStableIdentityKey(workerId)); + } await transaction.exec(); } return { workerId, credential, expiresAt }; From 7555cc6efb7a7a6d946bbbfce3d4ea2ea3d9f144 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 21:40:08 -0400 Subject: [PATCH 14/18] fix: drain API pods before pairing rollback --- helm/codeapi/README.md | 16 ++++++ helm/codeapi/scripts/safe-pairing-rollback.sh | 57 +++++++++++++++++++ helm/codeapi/values.yaml | 1 + tests/bridge_pairing_rollout.sh | 15 ++++- 4 files changed, 88 insertions(+), 1 deletion(-) create mode 100755 helm/codeapi/scripts/safe-pairing-rollback.sh diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index 9b3efbb..21c15cd 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -53,6 +53,22 @@ platform rather than templated here: external ingress/service mesh, KEDA-style queue-depth autoscaling, and cloud-IAM secret delivery (the env hooks below cover all of them). +**Pairing-fence rollbacks.** Do not use a direct `helm rollback` from a chart +revision containing the bridge pairing fence to an older revision. Helm runs +rollback hooks from the target revision, so a pre-fence target cannot stop its +own old and new API replicas from overlapping. Use the chart's fail-closed +helper instead: + +```bash +helm/codeapi/scripts/safe-pairing-rollback.sh RELEASE REVISION NAMESPACE +``` + +The helper deletes the API HPA, scales the live fenced API deployment to zero, +waits until every API pod is gone, and only then invokes `helm rollback`. This +causes an API outage by design. If rollback fails, it leaves the API scaled to +zero rather than restarting a potentially mixed-version deployment. The +operator running it needs permission to read/scale Deployments and delete HPAs. + **Execution profile.** By default this chart leaves `CODEAPI_EXECUTION_PROFILE` unset. Its bundled HTTP/stateless configuration is inferred as the AWS-free `default` profile and retains the existing diff --git a/helm/codeapi/scripts/safe-pairing-rollback.sh b/helm/codeapi/scripts/safe-pairing-rollback.sh new file mode 100755 index 0000000..2d573e6 --- /dev/null +++ b/helm/codeapi/scripts/safe-pairing-rollback.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +set -euo pipefail + +usage() { + echo "usage: $0 RELEASE REVISION [NAMESPACE] [helm rollback flags...]" >&2 + exit 64 +} + +release=${1:-} +revision=${2:-} +namespace=${3:-default} +if [[ -z "$release" || ! "$revision" =~ ^[1-9][0-9]*$ ]]; then + usage +fi +shift $(( $# >= 3 ? 3 : $# )) + +if [[ ! "$release" =~ ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ ]]; then + echo "invalid Helm release name: $release" >&2 + exit 64 +fi +if [[ ! "$namespace" =~ ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ ]]; then + echo "invalid Kubernetes namespace: $namespace" >&2 + exit 64 +fi + +timeout=${CODEAPI_ROLLBACK_TIMEOUT:-10m} +selector="app.kubernetes.io/instance=${release},app.kubernetes.io/component=api" +deployment=$(kubectl --namespace "$namespace" get deployment \ + --selector "$selector" --output name) +if [[ -z "$deployment" || "$deployment" == *$'\n'* ]]; then + echo "expected exactly one Code API deployment for $selector" >&2 + exit 1 +fi + +fence=$(kubectl --namespace "$namespace" get "$deployment" \ + --output 'jsonpath={.spec.template.metadata.annotations.codeapi\.librechat\.ai/pairing-fence-version}') +if [[ -z "$fence" ]]; then + echo "refusing rollback: the live API deployment has no pairing fence" >&2 + exit 1 +fi + +echo "Deleting API autoscalers before the rollback fence is lowered..." >&2 +kubectl --namespace "$namespace" delete horizontalpodautoscaler \ + --selector "$selector" --ignore-not-found --wait=true + +echo "Scaling the fenced API deployment to zero..." >&2 +kubectl --namespace "$namespace" scale "$deployment" --replicas=0 +mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ + --selector "$selector" --output name) +if (( ${#pods[@]} > 0 )); then + kubectl --namespace "$namespace" wait "${pods[@]}" \ + --for=delete --timeout "$timeout" +fi + +echo "All fenced API pods are gone; starting Helm rollback..." >&2 +helm rollback "$release" "$revision" \ + --namespace "$namespace" --wait --wait-for-jobs --timeout "$timeout" "$@" diff --git a/helm/codeapi/values.yaml b/helm/codeapi/values.yaml index a052152..ee997ce 100644 --- a/helm/codeapi/values.yaml +++ b/helm/codeapi/values.yaml @@ -71,6 +71,7 @@ api: # Pairing revocation relies on every serving replica honoring the Redis # generation fence. Recreate prevents a pre-fence binary from redeeming an # already-revoked code during the first rollout of paired bridge workers. + # Roll back to pre-fence revisions only with scripts/safe-pairing-rollback.sh. strategy: type: Recreate rollingUpdate: null diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index fa5703e..6ebffd4 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -3,8 +3,9 @@ set -euo pipefail values=helm/codeapi/values.yaml deployment=helm/codeapi/templates/api-deployment.yaml +rollback=helm/codeapi/scripts/safe-pairing-rollback.sh -if ! grep -A 7 '^api:$' "$values" | grep -q '^ strategy:$'; then +if ! grep -A 12 '^api:$' "$values" | grep -q '^ strategy:$'; then echo 'api.strategy must be configured for pairing-safe rollouts' >&2 exit 1 fi @@ -28,3 +29,15 @@ if ! grep -A 8 '^ image:$' "$values" | grep -q '^ pullPolicy: Always$'; then echo 'the fenced API rollout must pull the current image even when the default tag is mutable' >&2 exit 1 fi +if [[ ! -x "$rollback" ]]; then + echo 'the pairing-safe rollback helper must be executable' >&2 + exit 1 +fi +bash -n "$rollback" +if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || + ! grep -q 'scale "$deployment" --replicas=0' "$rollback" || + ! grep -q -- '--for=delete' "$rollback" || + ! grep -q 'helm rollback' "$rollback"; then + echo 'rollback must remove autoscaling, drain API pods, then invoke Helm' >&2 + exit 1 +fi From e277ea7d95330d7809a806e744834399410dce0c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 21:59:27 -0400 Subject: [PATCH 15/18] fix: fence rollback reentry and pairing epochs --- helm/codeapi/README.md | 14 +++-- helm/codeapi/scripts/safe-pairing-rollback.sh | 63 +++++++++++++++---- helm/codeapi/templates/api-deployment.yaml | 6 ++ service/src/bridge/pairing.test.ts | 34 ++++++++++ service/src/bridge/pairing.ts | 15 ++++- tests/bridge_pairing_rollout.sh | 10 ++- 6 files changed, 120 insertions(+), 22 deletions(-) diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index 21c15cd..4485d1c 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -63,11 +63,15 @@ helper instead: helm/codeapi/scripts/safe-pairing-rollback.sh RELEASE REVISION NAMESPACE ``` -The helper deletes the API HPA, scales the live fenced API deployment to zero, -waits until every API pod is gone, and only then invokes `helm rollback`. This -causes an API outage by design. If rollback fails, it leaves the API scaled to -zero rather than restarting a potentially mixed-version deployment. The -operator running it needs permission to read/scale Deployments and delete HPAs. +The helper records an out-of-band rollback epoch, deletes the API HPA, scales +the live fenced API deployment to zero, verifies that the Deployment and every +matching pod have converged to zero, and only then invokes `helm rollback`. +When a fenced revision is deployed again, the epoch forces one fresh cleanup of +legacy pairing codes even if the original migration window has expired. This +causes an API outage by design. If rollback fails, the helper repeats the drain +so a partially applied rollback cannot leave a mixed-version API running. The +operator running it needs permission to read/scale Deployments, delete HPAs, +and create or update the rollback ConfigMap. **Execution profile.** By default this chart leaves `CODEAPI_EXECUTION_PROFILE` unset. Its bundled HTTP/stateless configuration is diff --git a/helm/codeapi/scripts/safe-pairing-rollback.sh b/helm/codeapi/scripts/safe-pairing-rollback.sh index 2d573e6..7037c72 100755 --- a/helm/codeapi/scripts/safe-pairing-rollback.sh +++ b/helm/codeapi/scripts/safe-pairing-rollback.sh @@ -39,19 +39,56 @@ if [[ -z "$fence" ]]; then exit 1 fi -echo "Deleting API autoscalers before the rollback fence is lowered..." >&2 -kubectl --namespace "$namespace" delete horizontalpodautoscaler \ - --selector "$selector" --ignore-not-found --wait=true +deployment_name=${deployment#*/} +rollback_config_map=${deployment_name%-api}-pairing-rollback +rollback_epoch="$(date +%s)-${RANDOM}-${RANDOM}" -echo "Scaling the fenced API deployment to zero..." >&2 -kubectl --namespace "$namespace" scale "$deployment" --replicas=0 -mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ - --selector "$selector" --output name) -if (( ${#pods[@]} > 0 )); then - kubectl --namespace "$namespace" wait "${pods[@]}" \ - --for=delete --timeout "$timeout" -fi +echo "Recording pairing rollback epoch $rollback_epoch..." >&2 +kubectl --namespace "$namespace" create configmap "$rollback_config_map" \ + --from-literal="epoch=$rollback_epoch" --dry-run=client --output yaml | \ + kubectl --namespace "$namespace" apply --filename - + +drain_api() { + echo "Deleting API autoscalers before the rollback fence is lowered..." >&2 + kubectl --namespace "$namespace" delete horizontalpodautoscaler \ + --selector "$selector" --ignore-not-found --wait=true + + echo "Scaling the fenced API deployment to zero..." >&2 + kubectl --namespace "$namespace" scale "$deployment" --replicas=0 + kubectl --namespace "$namespace" rollout status "$deployment" \ + --timeout "$timeout" + + mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ + --selector "$selector" --output name) + if (( ${#pods[@]} > 0 )); then + kubectl --namespace "$namespace" wait "${pods[@]}" \ + --for=delete --timeout "$timeout" + fi + + # Relist immediately before Helm can lower the fence. This catches a new + # matching pod that appeared after the first snapshot. + replica_state=$(kubectl --namespace "$namespace" get "$deployment" \ + --output 'jsonpath={.spec.replicas},{.status.replicas},{.status.readyReplicas},{.status.availableReplicas},{.status.updatedReplicas}') + IFS=, read -r desired current ready available updated <<< "$replica_state" + mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ + --selector "$selector" --output name) + if (( ${#pods[@]} > 0 )) || + [[ ${desired:-0} != 0 || ${current:-0} != 0 || ${ready:-0} != 0 || + ${available:-0} != 0 || ${updated:-0} != 0 ]]; then + echo "refusing rollback: API deployment did not converge to zero replicas" >&2 + return 1 + fi +} + +drain_api echo "All fenced API pods are gone; starting Helm rollback..." >&2 -helm rollback "$release" "$revision" \ - --namespace "$namespace" --wait --wait-for-jobs --timeout "$timeout" "$@" +if helm rollback "$release" "$revision" \ + --namespace "$namespace" --wait --wait-for-jobs --timeout "$timeout" "$@"; then + exit 0 +else + rollback_status=$? + echo "Helm rollback failed; restoring the fail-closed API drain..." >&2 + drain_api + exit "$rollback_status" +fi diff --git a/helm/codeapi/templates/api-deployment.yaml b/helm/codeapi/templates/api-deployment.yaml index 17147b2..69985db 100644 --- a/helm/codeapi/templates/api-deployment.yaml +++ b/helm/codeapi/templates/api-deployment.yaml @@ -59,6 +59,12 @@ spec: secretKeyRef: name: {{ include "codeapi.fullname" . }}-secrets key: redis-password + - name: CODEAPI_BRIDGE_PAIRING_ROLLBACK_EPOCH + valueFrom: + configMapKeyRef: + name: {{ include "codeapi.fullname" . }}-pairing-rollback + key: epoch + optional: true # Service URLs - name: FILE_SERVER_URL value: "http://{{ include "codeapi.fullname" . }}-file-server:{{ .Values.fileServer.service.port }}" diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index ce1dc1a..bcedd1c 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -696,6 +696,40 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(deadlineKey)).resolves.toBe('0'); }); + test('restarts legacy cleanup once for an explicit rollback epoch', async () => { + const legacyCode = 'unindexed-rollback-epoch-code'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + const deadlineKey = 'codeapi:bridge:v1:migration:legacy-pairing-scan-until'; + await redis.set(deadlineKey, '0'); + await redis.set( + legacyKey, + JSON.stringify({ + workerId: 'vm-rollback-epoch', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + await redis.set( + 'codeapi:bridge:v1:migration:legacy-pairing-scanned:vm-rollback-epoch', + 'done', + ); + const rollbackAwarePairings = new RedisBridgePairingStore( + redis, + 600, + 300, + 5_000, + 'rollback-epoch-1', + ); + + await rollbackAwarePairings.revoke('vm-rollback-epoch'); + + await expect(redis.get(legacyKey)).resolves.toBeNull(); + await expect(redis.get(deadlineKey)).resolves.toBe('0'); + }); + test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { const identity = createBridgeIdentity(); const legacyCode = 'unrevoked-legacy-pairing'; diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 3e54502..7017b56 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -204,8 +204,13 @@ function legacyPairingScanDeadlineKey(): string { return `${PREFIX}:migration:legacy-pairing-scan-until`; } -function legacyPairingWorkerScanKey(workerId: string): string { - return `${PREFIX}:migration:legacy-pairing-scanned:${workerId}`; +function legacyPairingWorkerScanKey( + workerId: string, + rollbackEpoch?: string, +): string { + const epoch = rollbackEpoch?.trim(); + const epochSuffix = epoch ? `:${digest(epoch)}` : ''; + return `${PREFIX}:migration:legacy-pairing-scanned:${workerId}${epochSuffix}`; } function proofNonceKey(credential: string, nonce: string): string { @@ -226,6 +231,8 @@ export class RedisBridgePairingStore { private readonly pairingTtlSeconds = DEFAULT_PAIRING_TTL_SECONDS, private readonly credentialTtlSeconds = DEFAULT_CREDENTIAL_TTL_SECONDS, private readonly legacyScanClaimTtlMs = LEGACY_SCAN_CLAIM_TTL_MS, + private readonly rollbackEpoch = + process.env.CODEAPI_BRIDGE_PAIRING_ROLLBACK_EPOCH?.trim() ?? '', ) {} async issue( @@ -469,7 +476,8 @@ export class RedisBridgePairingStore { } const deadline = Number(rawDeadline); - const stateKey = legacyPairingWorkerScanKey(workerId); + const stateKey = legacyPairingWorkerScanKey(workerId, this.rollbackEpoch); + const rollbackEpochDetected = this.rollbackEpoch.trim().length > 0; while (true) { const state = await this.redis.get(stateKey); if (state === LEGACY_SCAN_COMPLETE && !rollbackDetected) return; @@ -477,6 +485,7 @@ export class RedisBridgePairingStore { if (state == null) { if ( !rollbackDetected && + !rollbackEpochDetected && (!Number.isFinite(deadline) || Date.now() > deadline) ) { return; diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index 6ebffd4..b236ead 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -37,7 +37,15 @@ bash -n "$rollback" if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || ! grep -q 'scale "$deployment" --replicas=0' "$rollback" || ! grep -q -- '--for=delete' "$rollback" || + ! grep -q 'create configmap "$rollback_config_map"' "$rollback" || + ! grep -q 'replica_state=' "$rollback" || + [[ $(grep -c '^ drain_api$' "$rollback") -lt 1 ]] || ! grep -q 'helm rollback' "$rollback"; then - echo 'rollback must remove autoscaling, drain API pods, then invoke Helm' >&2 + echo 'rollback must record an epoch, remove autoscaling, verify the drain, and fail closed' >&2 + exit 1 +fi +if ! grep -q 'CODEAPI_BRIDGE_PAIRING_ROLLBACK_EPOCH' "$deployment" || + ! grep -q 'optional: true' "$deployment"; then + echo 'the API Deployment must consume the optional rollback epoch' >&2 exit 1 fi From ca53b09634396f9104f1325175f467cb88e6f5b9 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 22:11:55 -0400 Subject: [PATCH 16/18] fix: close rollback verification gaps --- helm/codeapi/README.md | 7 +- helm/codeapi/scripts/safe-pairing-rollback.sh | 83 ++++++++++++++----- service/src/bridge/pairing.test.ts | 43 ++++++++++ service/src/bridge/pairing.ts | 11 ++- tests/bridge_pairing_rollout.sh | 4 +- 5 files changed, 123 insertions(+), 25 deletions(-) diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index 4485d1c..22e1e4b 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -69,9 +69,10 @@ matching pod have converged to zero, and only then invokes `helm rollback`. When a fenced revision is deployed again, the epoch forces one fresh cleanup of legacy pairing codes even if the original migration window has expired. This causes an API outage by design. If rollback fails, the helper repeats the drain -so a partially applied rollback cannot leave a mixed-version API running. The -operator running it needs permission to read/scale Deployments, delete HPAs, -and create or update the rollback ConfigMap. +after re-discovering every API Deployment and explicitly deletes any remaining +API pods, so a partially applied rollback cannot leave a mixed-version API +running. The operator running it needs permission to read/scale Deployments, +delete HPAs and pods, and create or update the rollback ConfigMap. **Execution profile.** By default this chart leaves `CODEAPI_EXECUTION_PROFILE` unset. Its bundled HTTP/stateless configuration is diff --git a/helm/codeapi/scripts/safe-pairing-rollback.sh b/helm/codeapi/scripts/safe-pairing-rollback.sh index 7037c72..ac1284c 100755 --- a/helm/codeapi/scripts/safe-pairing-rollback.sh +++ b/helm/codeapi/scripts/safe-pairing-rollback.sh @@ -25,12 +25,33 @@ fi timeout=${CODEAPI_ROLLBACK_TIMEOUT:-10m} selector="app.kubernetes.io/instance=${release},app.kubernetes.io/component=api" -deployment=$(kubectl --namespace "$namespace" get deployment \ - --selector "$selector" --output name) -if [[ -z "$deployment" || "$deployment" == *$'\n'* ]]; then + +discover_api_deployments() { + local output + output=$(kubectl --namespace "$namespace" get deployment \ + --selector "$selector" --output name) || return + deployments=() + if [[ -n "$output" ]]; then + mapfile -t deployments <<< "$output" + fi +} + +list_api_pods() { + local output + output=$(kubectl --namespace "$namespace" get pod \ + --selector "$selector" --output name) || return + pods=() + if [[ -n "$output" ]]; then + mapfile -t pods <<< "$output" + fi +} + +discover_api_deployments +if (( ${#deployments[@]} != 1 )); then echo "expected exactly one Code API deployment for $selector" >&2 exit 1 fi +deployment=${deployments[0]} fence=$(kubectl --namespace "$namespace" get "$deployment" \ --output 'jsonpath={.spec.template.metadata.annotations.codeapi\.librechat\.ai/pairing-fence-version}') @@ -49,38 +70,60 @@ kubectl --namespace "$namespace" create configmap "$rollback_config_map" \ kubectl --namespace "$namespace" apply --filename - drain_api() { + local pod_action=${1:-wait} + local replica_state desired current ready available updated + + # Helm may have partially installed a target with a different fullname. + # Resolve every matching API Deployment on each drain attempt. + discover_api_deployments + if (( ${#deployments[@]} == 0 )) && [[ "$pod_action" != delete ]]; then + echo "refusing rollback: no API deployment matched $selector" >&2 + return 1 + fi + echo "Deleting API autoscalers before the rollback fence is lowered..." >&2 kubectl --namespace "$namespace" delete horizontalpodautoscaler \ --selector "$selector" --ignore-not-found --wait=true echo "Scaling the fenced API deployment to zero..." >&2 - kubectl --namespace "$namespace" scale "$deployment" --replicas=0 - kubectl --namespace "$namespace" rollout status "$deployment" \ - --timeout "$timeout" + for deployment in "${deployments[@]}"; do + kubectl --namespace "$namespace" scale "$deployment" --replicas=0 + kubectl --namespace "$namespace" rollout status "$deployment" \ + --timeout "$timeout" + done - mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ - --selector "$selector" --output name) + list_api_pods if (( ${#pods[@]} > 0 )); then - kubectl --namespace "$namespace" wait "${pods[@]}" \ - --for=delete --timeout "$timeout" + if [[ "$pod_action" == delete ]]; then + kubectl --namespace "$namespace" delete pod \ + --selector "$selector" --wait=true --timeout "$timeout" + else + kubectl --namespace "$namespace" wait "${pods[@]}" \ + --for=delete --timeout "$timeout" + fi fi # Relist immediately before Helm can lower the fence. This catches a new # matching pod that appeared after the first snapshot. - replica_state=$(kubectl --namespace "$namespace" get "$deployment" \ - --output 'jsonpath={.spec.replicas},{.status.replicas},{.status.readyReplicas},{.status.availableReplicas},{.status.updatedReplicas}') - IFS=, read -r desired current ready available updated <<< "$replica_state" - mapfile -t pods < <(kubectl --namespace "$namespace" get pod \ - --selector "$selector" --output name) - if (( ${#pods[@]} > 0 )) || - [[ ${desired:-0} != 0 || ${current:-0} != 0 || ${ready:-0} != 0 || + discover_api_deployments + for deployment in "${deployments[@]}"; do + replica_state=$(kubectl --namespace "$namespace" get "$deployment" \ + --output 'jsonpath={.spec.replicas},{.status.replicas},{.status.readyReplicas},{.status.availableReplicas},{.status.updatedReplicas}') || return + IFS=, read -r desired current ready available updated <<< "$replica_state" + if [[ ${desired:-0} != 0 || ${current:-0} != 0 || ${ready:-0} != 0 || ${available:-0} != 0 || ${updated:-0} != 0 ]]; then - echo "refusing rollback: API deployment did not converge to zero replicas" >&2 + echo "refusing rollback: API deployment did not converge to zero replicas" >&2 + return 1 + fi + done + list_api_pods + if (( ${#pods[@]} > 0 )); then + echo "refusing rollback: API pods appeared after the drain" >&2 return 1 fi } -drain_api +drain_api wait echo "All fenced API pods are gone; starting Helm rollback..." >&2 if helm rollback "$release" "$revision" \ @@ -89,6 +132,6 @@ if helm rollback "$release" "$revision" \ else rollback_status=$? echo "Helm rollback failed; restoring the fail-closed API drain..." >&2 - drain_api + drain_api delete exit "$rollback_status" fi diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index bcedd1c..7f8a19e 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -728,6 +728,49 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); await expect(redis.get(deadlineKey)).resolves.toBe('0'); + const epochHash = createHash('sha256') + .update('rollback-epoch-1') + .digest('hex'); + const epochStateKey = + `codeapi:bridge:v1:migration:legacy-pairing-scanned:` + + `vm-rollback-epoch:${epochHash}`; + await expect(redis.get(epochStateKey)).resolves.toBe('done'); + await expect(redis.pttl(epochStateKey)).resolves.toBe(-1); + }); + + test('cleans a revoked legacy code before rollback-epoch redemption', async () => { + const identity = createBridgeIdentity(); + const workerId = 'vm-rollback-redeem'; + const legacyCode = 'revoked-rollback-pairing'; + const legacyKey = `codeapi:bridge:v1:pairing:${createHash('sha256') + .update(legacyCode) + .digest('hex')}`; + await redis.set('codeapi:bridge:v1:migration:legacy-pairing-scan-until', '0'); + await redis.set( + legacyKey, + JSON.stringify({ + workerId, + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }), + 'EX', + 60, + ); + const rollbackAwarePairings = new RedisBridgePairingStore( + redis, + 600, + 300, + 5_000, + 'rollback-epoch-redeem', + ); + + await expect( + rollbackAwarePairings.redeem({ + workerId, + code: legacyCode, + publicKey: identity.publicKey, + }), + ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); + await expect(redis.get(legacyKey)).resolves.toBeNull(); }); test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 7017b56..79e6f3d 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -108,7 +108,9 @@ return 0 const COMPLETE_LEGACY_SCAN_CLAIM_SCRIPT = ` if redis.call('GET', KEYS[2]) == ARGV[1] then local remaining = tonumber(ARGV[3]) - if remaining > 0 then + if ARGV[4] == '1' then + redis.call('SET', KEYS[1], ARGV[2]) + elseif remaining > 0 then redis.call('SET', KEYS[1], ARGV[2], 'PX', remaining) else redis.call('DEL', KEYS[1]) @@ -273,6 +275,12 @@ export class RedisBridgePairingStore { code: string; publicKey: string; }): Promise { + // A rollback epoch means an older binary may have issued or failed to + // revoke an unindexed code. Complete that worker's epoch scan before any + // legacy redemption can mint a credential. + if (this.rollbackEpoch.trim().length > 0) { + await this.removeLegacyPairings(args.workerId); + } const codeKey = pairingKey(args.code); const raw = await this.redis.get(codeKey); if (raw == null) { @@ -599,6 +607,7 @@ export class RedisBridgePairingStore { scanClaim.token, LEGACY_SCAN_COMPLETE, String(deadline - Date.now()), + rollbackEpochDetected ? '1' : '0', ); if (completed !== 1) { await this.removeLegacyPairings(workerId); diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index b236ead..b1f6f34 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -39,7 +39,9 @@ if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || ! grep -q -- '--for=delete' "$rollback" || ! grep -q 'create configmap "$rollback_config_map"' "$rollback" || ! grep -q 'replica_state=' "$rollback" || - [[ $(grep -c '^ drain_api$' "$rollback") -lt 1 ]] || + ! grep -q 'discover_api_deployments' "$rollback" || + ! grep -q 'list_api_pods' "$rollback" || + ! grep -q '^ drain_api delete$' "$rollback" || ! grep -q 'helm rollback' "$rollback"; then echo 'rollback must record an epoch, remove autoscaling, verify the drain, and fail closed' >&2 exit 1 From 175398a11aaee5e8cf984cc5a5a0ffa8033e7271 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 22:18:20 -0400 Subject: [PATCH 17/18] fix: keep rollback drain on one cluster --- helm/codeapi/README.md | 3 +++ helm/codeapi/scripts/safe-pairing-rollback.sh | 12 ++++++++++++ tests/bridge_pairing_rollout.sh | 8 ++++++++ 3 files changed, 23 insertions(+) diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index 22e1e4b..6c8f39f 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -73,6 +73,9 @@ after re-discovering every API Deployment and explicitly deletes any remaining API pods, so a partially applied rollback cannot leave a mixed-version API running. The operator running it needs permission to read/scale Deployments, delete HPAs and pods, and create or update the rollback ConfigMap. +Pass the intended cluster context to both `kubectl` and `helm` before invoking +the helper; it rejects forwarded kubeconfig, context, identity, API-server, and +namespace flags so the drain and rollback cannot target different clusters. **Execution profile.** By default this chart leaves `CODEAPI_EXECUTION_PROFILE` unset. Its bundled HTTP/stateless configuration is diff --git a/helm/codeapi/scripts/safe-pairing-rollback.sh b/helm/codeapi/scripts/safe-pairing-rollback.sh index ac1284c..e192bf1 100755 --- a/helm/codeapi/scripts/safe-pairing-rollback.sh +++ b/helm/codeapi/scripts/safe-pairing-rollback.sh @@ -22,6 +22,18 @@ if [[ ! "$namespace" =~ ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ ]]; then echo "invalid Kubernetes namespace: $namespace" >&2 exit 64 fi +for flag in "$@"; do + case "$flag" in + -n|-n?*|--namespace|--namespace=*|--kube-context|--kube-context=*|\ + --kubeconfig|--kubeconfig=*|--kube-apiserver|--kube-apiserver=*|\ + --kube-ca-file|--kube-ca-file=*|--kube-token|--kube-token=*|\ + --kube-as-user|--kube-as-user=*|--kube-as-group|--kube-as-group=*|\ + --kube-insecure-skip-tls-verify|--kube-insecure-skip-tls-verify=*) + echo "refusing target-changing Helm rollback flag: $flag" >&2 + exit 64 + ;; + esac +done timeout=${CODEAPI_ROLLBACK_TIMEOUT:-10m} selector="app.kubernetes.io/instance=${release},app.kubernetes.io/component=api" diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index b1f6f34..badd140 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -34,6 +34,14 @@ if [[ ! -x "$rollback" ]]; then exit 1 fi bash -n "$rollback" +if "$rollback" codeapi 1 default --kube-context other >/dev/null 2>&1; then + echo 'rollback must reject a Helm context that differs from the kubectl drain' >&2 + exit 1 +fi +if "$rollback" codeapi 1 default --kubeconfig=/tmp/other >/dev/null 2>&1; then + echo 'rollback must reject a Helm kubeconfig that differs from the kubectl drain' >&2 + exit 1 +fi if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || ! grep -q 'scale "$deployment" --replicas=0' "$rollback" || ! grep -q -- '--for=delete' "$rollback" || From 716fea55996a894956ec275ef9e2b200f1a950fd Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 30 Aug 2026 22:28:59 -0400 Subject: [PATCH 18/18] fix: bound rollback recovery triggers --- helm/codeapi/README.md | 4 +- helm/codeapi/scripts/safe-pairing-rollback.sh | 47 +++++++++++++++++-- service/src/bridge/pairing.test.ts | 35 ++++++++++++++ service/src/bridge/pairing.ts | 19 +++++--- tests/bridge_pairing_rollout.sh | 5 ++ 5 files changed, 100 insertions(+), 10 deletions(-) diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index 6c8f39f..fcdef01 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -75,7 +75,9 @@ running. The operator running it needs permission to read/scale Deployments, delete HPAs and pods, and create or update the rollback ConfigMap. Pass the intended cluster context to both `kubectl` and `helm` before invoking the helper; it rejects forwarded kubeconfig, context, identity, API-server, and -namespace flags so the drain and rollback cannot target different clusters. +namespace flags and Helm-specific target environment overrides so the drain and +rollback cannot target different clusters. Termination signals during Helm +also trigger a final recovery drain before the helper exits. **Execution profile.** By default this chart leaves `CODEAPI_EXECUTION_PROFILE` unset. Its bundled HTTP/stateless configuration is diff --git a/helm/codeapi/scripts/safe-pairing-rollback.sh b/helm/codeapi/scripts/safe-pairing-rollback.sh index e192bf1..c70bddc 100755 --- a/helm/codeapi/scripts/safe-pairing-rollback.sh +++ b/helm/codeapi/scripts/safe-pairing-rollback.sh @@ -27,6 +27,7 @@ for flag in "$@"; do -n|-n?*|--namespace|--namespace=*|--kube-context|--kube-context=*|\ --kubeconfig|--kubeconfig=*|--kube-apiserver|--kube-apiserver=*|\ --kube-ca-file|--kube-ca-file=*|--kube-token|--kube-token=*|\ + --kube-tls-server-name|--kube-tls-server-name=*|\ --kube-as-user|--kube-as-user=*|--kube-as-group|--kube-as-group=*|\ --kube-insecure-skip-tls-verify|--kube-insecure-skip-tls-verify=*) echo "refusing target-changing Helm rollback flag: $flag" >&2 @@ -34,6 +35,21 @@ for flag in "$@"; do ;; esac done +for variable in \ + HELM_KUBEAPISERVER \ + HELM_KUBEASGROUPS \ + HELM_KUBEASUSER \ + HELM_KUBECAFILE \ + HELM_KUBECONTEXT \ + HELM_KUBEINSECURE_SKIP_TLS_VERIFY \ + HELM_KUBETLS_SERVER_NAME \ + HELM_KUBETOKEN \ + HELM_NAMESPACE; do + if [[ -n ${!variable:-} ]]; then + echo "refusing Helm target override from environment: $variable" >&2 + exit 64 + fi +done timeout=${CODEAPI_ROLLBACK_TIMEOUT:-10m} selector="app.kubernetes.io/instance=${release},app.kubernetes.io/component=api" @@ -138,11 +154,36 @@ drain_api() { drain_api wait echo "All fenced API pods are gone; starting Helm rollback..." >&2 -if helm rollback "$release" "$revision" \ - --namespace "$namespace" --wait --wait-for-jobs --timeout "$timeout" "$@"; then +rollback_pid= +recover_interrupted_rollback() { + local exit_status=$1 + trap - HUP INT TERM + if [[ -n "$rollback_pid" ]]; then + kill -TERM "$rollback_pid" 2>/dev/null || true + wait "$rollback_pid" 2>/dev/null || true + fi + echo "Helm rollback interrupted; restoring the fail-closed API drain..." >&2 + set -e + drain_api delete + exit "$exit_status" +} +trap 'recover_interrupted_rollback 129' HUP +trap 'recover_interrupted_rollback 130' INT +trap 'recover_interrupted_rollback 143' TERM + +helm rollback "$release" "$revision" \ + --namespace "$namespace" --wait --wait-for-jobs --timeout "$timeout" "$@" & +rollback_pid=$! +set +e +wait "$rollback_pid" +rollback_status=$? +set -e +rollback_pid= +trap - HUP INT TERM + +if (( rollback_status == 0 )); then exit 0 else - rollback_status=$? echo "Helm rollback failed; restoring the fail-closed API drain..." >&2 drain_api delete exit "$rollback_status" diff --git a/service/src/bridge/pairing.test.ts b/service/src/bridge/pairing.test.ts index 7f8a19e..e709a4a 100644 --- a/service/src/bridge/pairing.test.ts +++ b/service/src/bridge/pairing.test.ts @@ -773,6 +773,41 @@ describe('RedisBridgePairingStore', () => { await expect(redis.get(legacyKey)).resolves.toBeNull(); }); + test('does not scan for a nonexistent rollback-epoch pairing code', async () => { + const identity = createBridgeIdentity(); + const scan = redis.scan.bind(redis); + let scanCalls = 0; + redis.scan = (async (...args: Parameters) => { + scanCalls += 1; + return scan(...args); + }) as Redis['scan']; + const rollbackAwarePairings = new RedisBridgePairingStore( + redis, + 600, + 300, + 5_000, + 'rollback-epoch-missing-code', + ); + + try { + await expect( + rollbackAwarePairings.redeem({ + workerId: 'attacker-chosen-worker', + code: 'nonexistent-code', + publicKey: identity.publicKey, + }), + ).rejects.toMatchObject({ code: 'PAIRING_INVALID' }); + expect(scanCalls).toBe(0); + await expect( + redis.keys( + 'codeapi:bridge:v1:migration:legacy-pairing-scanned:attacker-chosen-worker*', + ), + ).resolves.toEqual([]); + } finally { + redis.scan = scan; + } + }); + test('redeems an unrevoked pairing code issued by a pre-fence replica', async () => { const identity = createBridgeIdentity(); const legacyCode = 'unrevoked-legacy-pairing'; diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 79e6f3d..59dfbe6 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -275,12 +275,6 @@ export class RedisBridgePairingStore { code: string; publicKey: string; }): Promise { - // A rollback epoch means an older binary may have issued or failed to - // revoke an unindexed code. Complete that worker's epoch scan before any - // legacy redemption can mint a credential. - if (this.rollbackEpoch.trim().length > 0) { - await this.removeLegacyPairings(args.workerId); - } const codeKey = pairingKey(args.code); const raw = await this.redis.get(codeKey); if (raw == null) { @@ -303,6 +297,19 @@ export class RedisBridgePairingStore { 'Worker public key must be an Ed25519 key', ); } + // Validate the supplied code before it can trigger a shared-keyspace scan. + // A rollback epoch means any generation-less code may have survived a + // legacy revoke, so clean the authenticated worker and reject that code. + if ( + pairing.generation == null && + this.rollbackEpoch.trim().length > 0 + ) { + await this.removeLegacyPairings(args.workerId); + throw new BridgePairingError( + 'PAIRING_INVALID', + 'Pairing code is invalid or expired', + ); + } const credential = randomBytes(32).toString('base64url'); const credentialDigest = digest(credential); diff --git a/tests/bridge_pairing_rollout.sh b/tests/bridge_pairing_rollout.sh index badd140..c4246db 100755 --- a/tests/bridge_pairing_rollout.sh +++ b/tests/bridge_pairing_rollout.sh @@ -42,6 +42,10 @@ if "$rollback" codeapi 1 default --kubeconfig=/tmp/other >/dev/null 2>&1; then echo 'rollback must reject a Helm kubeconfig that differs from the kubectl drain' >&2 exit 1 fi +if HELM_KUBECONTEXT=other "$rollback" codeapi 1 default >/dev/null 2>&1; then + echo 'rollback must reject a Helm context inherited from the environment' >&2 + exit 1 +fi if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || ! grep -q 'scale "$deployment" --replicas=0' "$rollback" || ! grep -q -- '--for=delete' "$rollback" || @@ -50,6 +54,7 @@ if ! grep -q 'delete horizontalpodautoscaler' "$rollback" || ! grep -q 'discover_api_deployments' "$rollback" || ! grep -q 'list_api_pods' "$rollback" || ! grep -q '^ drain_api delete$' "$rollback" || + ! grep -q 'recover_interrupted_rollback' "$rollback" || ! grep -q 'helm rollback' "$rollback"; then echo 'rollback must record an epoch, remove autoscaling, verify the drain, and fail closed' >&2 exit 1