[WRONG BRANCH] fix(responses): restrict encrypted task recovery to NEW_TASK envelopes - #483
[WRONG BRANCH] fix(responses): restrict encrypted task recovery to NEW_TASK envelopes#483luvs01 wants to merge 1 commit into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdbdaf330a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| const ROUTING_HEADER = /(?:^|\n)Message Type\s*:\s*(NEW_TASK|MESSAGE)\s*\nTask name\s*:\s*(\S+)\s*\nSender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/; | ||
| const ROUTING_HEADER = /(?:^|\n)Message Type\s*:\s*(NEW_TASK)\s*\nTask name\s*:\s*(\S+)\s*\nSender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/; |
There was a problem hiding this comment.
When an agent_message contains 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 👍 / 👎.
| expect(fetches).toBe(1); | ||
| }); | ||
|
|
||
| test("MESSAGE envelopes fail closed without recovery or cache restoration", async () => { |
There was a problem hiding this comment.
This new fail-closed expectation contradicts five unchanged cases in tests/server/server-agent-task-recovery-replay.test.ts, including “MESSAGE recovery reaches the provider,” the MESSAGE cache and mixed-history cases, the handler recovery case, and the MESSAGE-tail case at lines 184-217, 249-348, and 416-427. Those tests still expect recovery to return true or the handler to return HTTP 200, whereas this change now returns unsupported_envelope/HTTP 400, so the repository-wide test gate will fail until those scenarios are updated to the new contract.
AGENTS.md reference: AGENTS.md:L367-L370
Useful? React with 👍 / 👎.
1 participant
Motivation
MESSAGEenvelopes are detection-only and must fail closed, preventing the recovery path from turningMESSAGEciphertext into a plaintext oracle for routed providers.NEW_TASKrecovery so legitimate mid-thread NEW_TASK ciphertext can still be recovered and reused.Description
NEW_TASKonly by narrowing theAgentEnvelope.messageTypeandROUTING_HEADERinsrc/server/responses/agent-task-recovery.tssofindEnvelopewill not acceptMESSAGEfor recovery.NEW_TASKenvelopes intact so mid-thread NEW_TASK recovery behavior and cache reuse remain available.tests/server/agent-task-recovery.test.tsto remove MESSAGE-based recovery expectations, and to assert thatMESSAGEenvelopes are rejected withunsupported_envelopeand never trigger recovery I/O or cache restoration.Testing
node_modules/.bin/bun test tests/server/agent-task-recovery.test.tsand all tests passed (59 pass, 0 fail).bun run typecheckwhich succeeded.bun run privacy:scanwhich reportedPrivacy scan passed.bun run testfor broader context, which surfaced unrelated environment/parallel-runner failures in other suites; the focused recovery tests and typecheck above were green and validate the security regression fix.Codex Task