-
Notifications
You must be signed in to change notification settings - Fork 822
fix(replay): scope durable thought signatures per credential and bound persist visibility #2078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ea0b7a0
1d1d8c2
9697e89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # 020 — #2064 RCA: Remote raw-thinking leak (FIXED-ON-DEV) | ||
|
|
||
| Reported on 2.24.2: Codex Remote paints raw Grok thinking live | ||
| (response.reasoning_text.delta), then swaps to the progress line leaving | ||
| italic fragments; stored items are summary:[] + content reasoning_text. | ||
|
|
||
| RCA (sol lane + main verification, 2026-08-19): | ||
|
|
||
| - v2.24.2 bridge emitted the raw channel: git show v2.24.2:src/bridge.ts has | ||
| response.reasoning_text.delta; fix commit 56752d7c5 (#2007, landed via | ||
| PR #2016 merge 891c8284b) is NOT an ancestor of v2.24.2, IS on dev. | ||
| - Current dev has no escape path for openai-chat routed models: the bridge | ||
| emits summary-channel deltas and summary-shaped items only | ||
| (src/bridge.ts:987,1016,591-606); the native-Responses rewrite covers WS | ||
| upstream, eager relay, HTTP SSE tee, and JSON reframing legs | ||
| (src/server/responses/core.ts:2835-3066, | ||
| src/server/responses-reasoning-summary-rewrite.ts:51-110). | ||
| - Suites: tests/responses-reasoning-summary-rewrite.test.ts + | ||
| tests/bridge.test.ts = 73 pass / 0 fail (fresh). | ||
|
|
||
| Outcome: no new code. Issue #2064 closed as fixed-on-dev with a note asking | ||
| for on-device Remote verification on the next release build. Model-side | ||
| intermittent reasoning exposure (user caution) stays out of scope — our relay | ||
| provably converts the channel on every leg. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,18 +20,23 @@ | |
| * destination, adapter, model and credential — so a signature can only ever be replayed into | ||
| * the turn that produced it. | ||
| */ | ||
| import { readFileSync } from "node:fs"; | ||
| import { chmodSync, readFileSync, writeFileSync } from "node:fs"; | ||
| import { randomBytes } from "node:crypto"; | ||
| import { join } from "node:path"; | ||
| import { atomicWriteFileAsync, getConfigDir } from "../config"; | ||
| import type { OcxProviderOpaqueToolCallMetadata, OcxReasoningReplayScopeRef } from "../types"; | ||
| import { isCarryableSignature, responsesExtraContentFromProviderMetadata } from "./provider-opaque-metadata"; | ||
|
|
||
| const STORE_FILE_NAME = "thought-signature-replay.json"; | ||
| const SALT_FILE_NAME = "thought-signature-replay.salt"; | ||
| /** | ||
| * Bumped whenever `keyFor` changes shape. v3 added the durable destination identity, so a | ||
| * v2 file's keys can never match and are dropped on load instead of aging out invisibly. | ||
| * Bumped whenever `keyFor` changes shape. v3 added the durable destination identity; v4 | ||
| * added the salted durable credential identity (#1926), so a v3 file's keys can never | ||
| * match and are dropped on load instead of aging out invisibly. v3 rows carried no | ||
| * credential information, so they are not upgradable — the next Gemini turn re-accumulates | ||
| * its signatures (bounded, best-effort loss identical to the pre-store status quo). | ||
| */ | ||
| const STORE_VERSION = 3; | ||
| const STORE_VERSION = 4; | ||
|
|
||
| /** Bound on remembered entries; real signatures are a few hundred bytes, so this stays small. */ | ||
| const MAX_ENTRIES = 16_384; | ||
|
|
@@ -64,6 +69,45 @@ function storePath(): string { | |
| return join(getConfigDir(), STORE_FILE_NAME); | ||
| } | ||
|
|
||
| function saltPath(): string { | ||
| return join(getConfigDir(), SALT_FILE_NAME); | ||
| } | ||
|
|
||
| let cachedSalt: Buffer | undefined; | ||
| let saltLoaded = false; | ||
|
|
||
| /** | ||
| * Installation-local salt for the durable credential identity (#1926). Created once and | ||
| * persisted beside the store; losing it invalidates every stored key (the entries then | ||
| * never match and age out), which is safe — signatures re-accumulate per turn. | ||
| */ | ||
| export function thoughtSignatureReplaySalt(): Buffer | undefined { | ||
| if (saltLoaded) return cachedSalt; | ||
| saltLoaded = true; | ||
| try { | ||
| const raw = readFileSync(saltPath()); | ||
| if (raw.length >= 16) { | ||
| // Re-assert owner-only permissions on every load: a pre-existing file may have | ||
| // been created before this guard or loosened by external tooling. | ||
| try { chmodSync(saltPath(), 0o600); } catch { /* best effort on exotic filesystems */ } | ||
| cachedSalt = raw; | ||
| return cachedSalt; | ||
| } | ||
| } catch { | ||
| // fall through to mint | ||
| } | ||
| try { | ||
| const minted = randomBytes(32); | ||
| writeFileSync(saltPath(), minted, { mode: 0o600 }); | ||
| cachedSalt = minted; | ||
| } catch { | ||
| // Unwritable config dir: no durable credential identity this process; the durable | ||
| // store fails closed (keyFor returns undefined) rather than keying under a shared id. | ||
| cachedSalt = undefined; | ||
| } | ||
| return cachedSalt; | ||
|
Comment on lines
+84
to
+108
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Require the specified 256-bit salt at every boundary. The loader accepts a 16-byte salt at
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| function nonEmpty(value: unknown): value is string { | ||
| return typeof value === "string" && value.trim().length > 0; | ||
| } | ||
|
|
@@ -84,6 +128,10 @@ function keyFor(callId: string, scope: OcxReasoningReplayScopeRef | undefined): | |
| || !nonEmpty(identity?.providerName) | ||
| || !nonEmpty(identity?.adapterName) | ||
| || !nonEmpty(identity?.modelId) | ||
| // v4 (#1926): a missing durable credential identity means we cannot isolate this | ||
| // entry per credential across restarts. Refusing the key is the fail-closed choice — | ||
| // "credential:unknown" would let two different credentials share one durable slot. | ||
| || !nonEmpty(identity?.credentialDurableIdentity) | ||
| ) return undefined; | ||
| return JSON.stringify([ | ||
| scope.clientThreadId, | ||
|
|
@@ -93,6 +141,7 @@ function keyFor(callId: string, scope: OcxReasoningReplayScopeRef | undefined): | |
| // reasoning cache's randomBytes-keyed HMAC cannot. Without it, one provider NAME | ||
| // serving two endpoints shares signatures across both. | ||
| identity.providerDestinationDurableIdentity ?? "destination:unknown", | ||
| identity.credentialDurableIdentity, | ||
| identity.adapterName, | ||
| identity.modelId, | ||
| callId, | ||
|
|
@@ -265,6 +314,8 @@ export function resetThoughtSignatureReplayForTests(): void { | |
| totalBytes = 0; | ||
| loaded = false; | ||
| persistChain = Promise.resolve(); | ||
| cachedSalt = undefined; | ||
| saltLoaded = false; | ||
| } | ||
|
|
||
| export function thoughtSignatureReplayCountForTests(): number { | ||
|
|
@@ -275,3 +326,22 @@ export function thoughtSignatureReplayCountForTests(): number { | |
| export function flushThoughtSignatureReplayForTests(): Promise<void> { | ||
| return persistChain; | ||
| } | ||
|
|
||
| /** | ||
| * Bounded commit barrier (#1926 gap 2). All durable writes serialize onto | ||
| * `persistChain`, so awaiting it (with a cap) before a turn's terminal frame becomes | ||
| * externally visible bounds the restart window in which a handed-out signature could | ||
| * be lost. Bounded BEST EFFORT: on timeout the turn proceeds — availability wins, and | ||
| * the miss cost is one turn's re-accumulation (the pre-store status quo). The | ||
| * in-memory map is updated synchronously at remember() time, so within one process | ||
| * lifetime replay never races this barrier at all. | ||
| */ | ||
| export function awaitThoughtSignatureDurability(capMs = 250): Promise<void> { | ||
| let timer: ReturnType<typeof setTimeout> | undefined; | ||
| const cap = new Promise<void>(resolve => { | ||
| timer = setTimeout(resolve, capMs); | ||
| }); | ||
| return Promise.race([persistChain, cap]).then(() => { | ||
| if (timer !== undefined) clearTimeout(timer); | ||
| }); | ||
| } | ||
|
Comment on lines
+339
to
+347
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift Expose persistence failures to the durability barrier.
Keep the queue usable after a failure, but retain a 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,11 @@ import { | |
| reasoningReplayCodexCredentialIdentity, | ||
| reasoningReplayDestinationIdentity, | ||
| durableReplayDestinationIdentity, | ||
| durableReplayCredentialIdentity, | ||
| reasoningReplayKeyCredentialIdentity, | ||
| reasoningReplayOAuthCredentialIdentity, | ||
| } from "../../responses/reasoning-replay-cache"; | ||
| import { awaitThoughtSignatureDurability, thoughtSignatureReplaySalt } from "../../responses/thought-signature-replay"; | ||
| import { buildCompactV1Output, COMPACT_PROMPT, decodeCompactionSummary, extractCompactUserMessages } from "../../responses/compaction"; | ||
| import { FORWARD_HEADERS, sanitizeReasoningInputContent } from "../../adapters/openai-responses"; | ||
| import { | ||
|
|
@@ -326,11 +328,21 @@ function bindRouteReasoningReplayScope(args: { | |
| }): void { | ||
| const { parsed, providerName, provider, adapterName } = args; | ||
| let credentialIdentity: string | undefined; | ||
| let credentialDurableIdentity: string | undefined; | ||
| const durableSalt = thoughtSignatureReplaySalt(); | ||
| if (provider.authMode === "oauth") { | ||
| credentialIdentity = reasoningReplayOAuthCredentialIdentity( | ||
| args.oauthCredentialSnapshot, | ||
| provider.headers, | ||
| ); | ||
| // The persisted account-slot id survives token refresh and restarts; the rotating | ||
| // generation deliberately does NOT participate (#1926 design: rotation-safe). | ||
| credentialDurableIdentity = durableReplayCredentialIdentity( | ||
| "oauth", | ||
| args.oauthCredentialSnapshot?.accountId, | ||
| provider.headers, | ||
| durableSalt, | ||
| ); | ||
|
Comment on lines
333
to
+345
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Bind replay scope for Anthropic OAuth account-pool selections. When the Anthropic OAuth account pool is enabled, Carry the selected account-slot identity and valid transient credential state into 🤖 Prompt for AI Agents |
||
| } else if (provider.authMode === "forward") { | ||
| const poolContext = args.codexAuthContext?.kind === "pool" | ||
| || args.codexAuthContext?.kind === "main-pool" | ||
|
|
@@ -349,8 +361,28 @@ function bindRouteReasoningReplayScope(args: { | |
| writerGeneration: poolContext?.writerGeneration, | ||
| headers: provider.headers, | ||
| }); | ||
| // Durable identity requires a STABLE, TRUSTED account handle. Pool context comes from | ||
| // our own account store; a client-supplied chatgpt-account-id header is attacker | ||
| // -influenceable bucket selection and a bearer alone is rotating material — both are | ||
| // refused, so direct-forward turns get no durable scope (fail closed; the in-process | ||
| // cache still covers same-process replay). | ||
| const codexDurableHandle = poolContext?.accountId | ||
| ?? poolContext?.chatgptAccountId | ||
| ?? undefined; | ||
| credentialDurableIdentity = durableReplayCredentialIdentity( | ||
| "codex", | ||
| codexDurableHandle ?? undefined, | ||
| provider.headers, | ||
| durableSalt, | ||
| ); | ||
| } else if (provider.authMode !== "local") { | ||
| credentialIdentity = reasoningReplayKeyCredentialIdentity(provider); | ||
| credentialDurableIdentity = durableReplayCredentialIdentity( | ||
| "key", | ||
| nonEmptyProviderApiKey(provider), | ||
| provider.headers, | ||
| durableSalt, | ||
| ); | ||
| } | ||
| const providerDestinationIdentity = reasoningReplayDestinationIdentity(provider.baseUrl); | ||
| bindReasoningReplayScope( | ||
|
|
@@ -363,11 +395,18 @@ function bindRouteReasoningReplayScope(args: { | |
| adapterName, | ||
| modelId: parsed.modelId, | ||
| credentialIdentity, | ||
| ...(credentialDurableIdentity ? { credentialDurableIdentity } : {}), | ||
| } | ||
| : undefined, | ||
| ); | ||
| } | ||
|
|
||
| function nonEmptyProviderApiKey(provider: OcxProviderConfig): string | undefined { | ||
| return typeof provider.apiKey === "string" && provider.apiKey.trim().length > 0 | ||
| ? provider.apiKey | ||
| : undefined; | ||
| } | ||
|
|
||
| function isFixedCodexAccount(authCtx: CodexAuthContext): boolean { | ||
| return (authCtx.kind === "pool" || authCtx.kind === "main-pool") | ||
| && authCtx.fixedAccount === true; | ||
|
|
@@ -3616,6 +3655,10 @@ async function handleResponsesInner( | |
| responseStateOptions(adapterNeedsForcedContinuation(adapter.name)), | ||
| ); | ||
| } | ||
| // #1926 gap 2: the buffered path queued its signature persists inside | ||
| // buildResponseJSON; bound the durability window before the JSON becomes | ||
| // externally visible. | ||
| await awaitThoughtSignatureDurability(); | ||
| return new Response(JSON.stringify(json), { headers: { "Content-Type": "application/json" } }); | ||
| } | ||
|
|
||
|
|
@@ -4461,6 +4504,8 @@ async function handleResponsesInner( | |
| responseStateOptions(activeAdapter.name === "kiro"), | ||
| ); | ||
| } | ||
| // #1926 gap 2: same buffered-path durability bound as the primary branch. | ||
| await awaitThoughtSignatureDurability(); | ||
| return new Response(JSON.stringify(json), { headers: { "Content-Type": "application/json" } }); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Move the barrier before every signature-bearing terminal frame.
The waits execute after
closeCurrentToolCall()andfailCurrentToolCall()have already emittedresponse.output_item.done. A client can therefore receive a tool item before its replay write settles.Several failure paths also emit terminal frames without any wait:
Make signature-bearing close functions asynchronous, or stage their frames until
awaitThoughtSignatureDurability()completes. Route every asynchronous terminal path through one shared finalization helper. Keep only the documented synchronous stall-timeout path as best effort.Also applies to: 1223-1223, 1244-1244, 1273-1274, 1335-1335
🤖 Prompt for AI Agents