-
Notifications
You must be signed in to change notification settings - Fork 0
[WRONG BRANCH] fix(responses): restrict encrypted task recovery to NEW_TASK envelopes #483
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
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 |
|---|---|---|
|
|
@@ -36,48 +36,62 @@ describe("agent task recovery (opt-in, default off)", () => { | |
| resetAgentTaskRecoveryState(); | ||
| }); | ||
|
|
||
| for (const messageType of ["NEW_TASK", "MESSAGE"] as const) { | ||
| test(`typed ${messageType} recovery preserves boolean, replay and discard contracts`, async () => { | ||
| const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); | ||
| const config = routedConfig(); | ||
| const context = { parentThreadId: "parent-diagnostics" }; | ||
| const input = () => agentMessage([ | ||
| { type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType) }, | ||
| { type: "encrypted_content", encrypted_content: FERNET_TASK }, | ||
| ]); | ||
| let fetches = 0; | ||
| globalThis.fetch = (async () => { | ||
| fetches += 1; | ||
| return new Response(recoverySse("Recovered diagnostic fixture.")); | ||
| }) as typeof fetch; | ||
| test("typed NEW_TASK recovery preserves boolean, replay and discard contracts", async () => { | ||
| const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); | ||
| const config = routedConfig(); | ||
| const context = { parentThreadId: "parent-diagnostics" }; | ||
| const input = () => agentMessage([ | ||
| { type: "input_text", text: ROUTING_ENVELOPE }, | ||
| { type: "encrypted_content", encrypted_content: FERNET_TASK }, | ||
| ]); | ||
| let fetches = 0; | ||
| globalThis.fetch = (async () => { | ||
| fetches += 1; | ||
| return new Response(recoverySse("Recovered diagnostic fixture.")); | ||
| }) as typeof fetch; | ||
|
|
||
| const typedInput = input(); | ||
| expect(await recoverEncryptedAgentTaskWithResult(req, typedInput, {}, config, context)) | ||
| .toEqual({ recovered: true }); | ||
| const booleanInput = input(); | ||
| expect(await recoverEncryptedAgentTask(req, booleanInput, {}, config, context)).toBe(true); | ||
| expect(booleanInput).toEqual(typedInput); | ||
| expect(typedInput).toEqual([{ | ||
| type: "message", role: "user", content: [ | ||
| { type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType) }, | ||
| { type: "input_text", text: "Recovered diagnostic fixture." }, | ||
| ], | ||
| }]); | ||
| const replay = input(); | ||
| expect(restoreCachedEncryptedAgentTasks(req, replay, config, context)).toBe(1); | ||
| expect(replay).toEqual(typedInput); | ||
| expect(fetches).toBe(1); | ||
| const typedInput = input(); | ||
| expect(await recoverEncryptedAgentTaskWithResult(req, typedInput, {}, config, context)) | ||
| .toEqual({ recovered: true }); | ||
| const booleanInput = input(); | ||
| expect(await recoverEncryptedAgentTask(req, booleanInput, {}, config, context)).toBe(true); | ||
| expect(booleanInput).toEqual(typedInput); | ||
| expect(typedInput).toEqual([{ | ||
| type: "message", role: "user", content: [ | ||
| { type: "input_text", text: ROUTING_ENVELOPE }, | ||
| { type: "input_text", text: "Recovered diagnostic fixture." }, | ||
| ], | ||
| }]); | ||
| const replay = input(); | ||
| expect(restoreCachedEncryptedAgentTasks(req, replay, config, context)).toBe(1); | ||
| expect(replay).toEqual(typedInput); | ||
| expect(fetches).toBe(1); | ||
|
|
||
| const otherType = agentMessage([ | ||
| { type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType === "MESSAGE" ? "NEW_TASK" : "MESSAGE") }, | ||
| { type: "encrypted_content", encrypted_content: FERNET_TASK }, | ||
| ]); | ||
| expect(restoreCachedEncryptedAgentTasks(req, otherType, config, context)).toBe(0); | ||
| discardEncryptedAgentTaskRecovery(req, input(), config, context); | ||
| expect(restoreCachedEncryptedAgentTasks(req, input(), config, context)).toBe(0); | ||
| expect(fetches).toBe(1); | ||
| }); | ||
| } | ||
| discardEncryptedAgentTaskRecovery(req, input(), config, context); | ||
| expect(restoreCachedEncryptedAgentTasks(req, input(), config, context)).toBe(0); | ||
| expect(fetches).toBe(1); | ||
| }); | ||
|
|
||
| test("MESSAGE envelopes fail closed without recovery or cache restoration", async () => { | ||
|
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.
This new fail-closed expectation contradicts five unchanged cases in AGENTS.md reference: AGENTS.md:L367-L370 Useful? React with 👍 / 👎. |
||
| const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); | ||
| const config = routedConfig(); | ||
| const input = agentMessage([ | ||
| { type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE") }, | ||
| { type: "encrypted_content", encrypted_content: FERNET_TASK }, | ||
| ]); | ||
| const original = structuredClone(input); | ||
| let fetches = 0; | ||
| globalThis.fetch = (async () => { | ||
| fetches += 1; | ||
| return new Response(recoverySse("must not be recovered")); | ||
| }) as typeof fetch; | ||
|
|
||
| expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, config)) | ||
| .toEqual({ recovered: false, reason: "unsupported_envelope" }); | ||
| expect(input).toEqual(original); | ||
| expect(restoreCachedEncryptedAgentTasks(req, input, config)).toBe(0); | ||
| expect(fetches).toBe(0); | ||
| }); | ||
|
|
||
| const failedRecoveries: Array<[string, () => Response, AgentTaskRecoveryFailureReason]> = [ | ||
| ["HTTP 401", () => new Response("private-error", { status: 401 }), "recovery_http_rejected"], | ||
|
|
@@ -933,6 +947,26 @@ describe("mid-thread encrypted agent task recovery (#4089)", () => { | |
| expect(providerBody).not.toContain(FERNET_TASK); | ||
| }); | ||
|
|
||
| test("a mid-thread MESSAGE fails closed without reaching recovery or the routed provider", async () => { | ||
| let fetches = 0; | ||
| globalThis.fetch = (async () => { | ||
| fetches += 1; | ||
| throw new Error("MESSAGE ciphertext must not leave the proxy"); | ||
| }) as typeof fetch; | ||
| const input = agentMessage([ | ||
| { type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE") }, | ||
| { type: "encrypted_content", encrypted_content: FERNET_TASK }, | ||
| ]); | ||
|
|
||
| const response = await post(routedConfig(), "xai/grok-4.5", input, midThreadHeaders()); | ||
|
|
||
| expect(response.status).toBe(400); | ||
| expect(await response.json()).toMatchObject({ | ||
| error: { code: "unreadable_encrypted_agent_task", recovery_reason: "unsupported_envelope" }, | ||
| }); | ||
| expect(fetches).toBe(0); | ||
| }); | ||
|
|
||
| test("a mid-thread replay reuses the cached plaintext instead of recovering again", async () => { | ||
| // The report's third observation: the cache restore sits inside the same gate, so a | ||
| // mid-thread turn could never reuse a plaintext this proxy had already paid for. | ||
|
|
||
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.
When an
agent_messagecontains a MESSAGE routing-header part followed by a valid NEW_TASK routing-header part and ciphertext, this narrowed regex ignores the MESSAGE part, accepts the NEW_TASK part, and sends the ciphertext through recovery; previously both headers matched and the duplicate-header check rejected the input. A MESSAGE ciphertext can therefore be relabeled by adding a NEW_TASK sibling, bypassing the intended fail-closed boundary. Keep header recognition broad enough to detect both types, then explicitly admit only a sole NEW_TASK envelope.AGENTS.md reference: AGENTS.md:L357-L363
Useful? React with 👍 / 👎.