Codex lineage: only a namespaced transport signal may trust the flat client_metadata pair (#473) - #517
Codex lineage: only a namespaced transport signal may trust the flat client_metadata pair (#473)#517philcunliffe wants to merge 4 commits into
Conversation
Deferred review findings from PR #467, per the maintainer decisions on issue #473 (A: tighten, B: proceed). A. `hasCodexTransportSignal` answered both "may this exchange be called Codex" and "may its unnamespaced `client_metadata` identity pair be trusted". A forged `user-agent: codex_cli_rs/1.0` therefore let a flat `session_id` / `thread_id` pair dictate `conversation_id` and the LLP 0030 partition key. Split the predicate: `hasCodexNamespaceSignal` (chatgpt upstream, `/backend-api/codex/` namespace, `x-codex-*` header) is what corroborates the pair; the user-agent branch stays only in the loose predicate, which names the client. Real Codex is unaffected, since its map always carries `x-codex-installation-id` and `x-codex-window-id`. B. LLP 0151's header audit claimed nothing emits `thread-id` / `session-id`. Both are real names built by `build_session_headers` on Codex's `/responses/compact` and websocket-handshake paths. They remain correctly unread, since the turn-metadata blob states the same ids for the one request kind that carries them. Corrected the claim at every echo. LLP 0151 is Active and immutable, so both land in new LLP 0164 with `Extended-by` forward-refs on the two affected sections. Co-Authored-By: Claude <noreply@anthropic.com>
PR #502 (fix/issue-421) already mints llp/0164-status-names-recent-clients- from-gateway-entrypoints.decision.md and is held+approved awaiting a human merge, so it has the prior claim on 0164. This branch's decision doc and every @ref pointing at it move to 0165, which is free across master and all open PR branches. No content change: the file is renamed and the number updated at all 13 reference sites (5 source, 3 test, 3 in LLP 0151's Extended-by forward refs, the doc title, and the smoke flow). Both anchors still resolve.
🤖 neutral: renumbered LLP 0164 -> 0165 (cross-branch collision)This branch minted #502 is held, approved, and awaiting your merge, so it has the prior claim. This branch moves to 0165, which is free across Pushed as
|
Three open branches independently minted 0164 off the same master high-water mark, none able to see the others: fix/issue-421 (PR #502) status-names-recent-clients-from-gateway-entrypoints fix/issue-473 (PR #517) codex-flat-pair-needs-a-namespace-signal -> 0165 fix/issue-451 (PR #520) session-control-plane-states-its-guarantees -> 0166 PR #502 is held and approved awaiting a human merge, so it keeps 0164. This branch takes 0166; 0165 went to PR #517. No content change: the file is renamed and the number updated at every reference site (3 in session_command.js, 1 in the new responder-trust test, the Extended-by forward-ref in LLP 0067, and the doc title). The #stated-not-proved and #membership-not-grain anchors still resolve.
PR #517 corrected five echoes of "nothing emits `thread-id` / `session-id`". Two more survive. `readCodexClientMetadata`'s JSDoc still calls `thread-id` a "fictional" header, three lines above the block this PR rewrote. Per LLP 0165 #header-audit-correction only `parent-thread-id` is fictional; the defect class the sentence names is unchanged, so only the adjective moves. LLP 0083 repeats the claim twice, in a doc the correction sweep did not reach: `codex-rs` "defines ... and nothing else", and the bare names "appear nowhere in it". LLP 0083 is Accepted and immutable, so both take `Extended-by` forward-refs to LLP 0165, the same shape this PR gave 0151. npm test: 3080 pass, 8 fail, the pre-existing leave-command failures (issue #512) and no others. Co-Authored-By: Claude <noreply@anthropic.com>
Verdict: approve with fixes applied (2 low-severity findings, both fixed in
|
The rollout-cwd fallback's JSDoc still asserted the corrected claim: the bare `session-id` header name "is not one Codex emits or this file reads". Per LLP 0165 #header-audit-correction it is a real Codex name, emitted by `build_session_headers` on the `/responses/compact` and websocket paths. Only the "or this file reads" half survives, and it survives for a stated reason, so the sentence now says the file reads it on no path and refs LLP 0165 for why that stays right. Rounds 1 and 2 of review have now found three echoes past the five the PR corrected; this is the last occurrence in the tree. npm test: 3080 pass, 8 fail, the pre-existing leave-command failures (issue #512) and no others. npm run typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com>
Verdict: approve with fixes applied (round 2, 1 low-severity finding, fixed in
|
|
Rung: TRIAGE (LLP 0017), round-2 review-cap judgement call on PR #517 ( What neutral was doing: Both review rounds verified the core fix (every call Why it cannot proceed — residual 1 is a TRUE BLOCKER, verified by reproduction:
That route string is not something "the client had to learn from Codex" (the bar I reproduced this directly against the PR's own projector at head Root cause of why the failed upstream call doesn't help: This means the "weaker bar" flagged in round 1 is not a stylistic gap: it lets Also checked, NOT a blocker — residual 2 ( Also checked, NOT a blocker — residual 3 (JSDoc/LLP 0165:53 say "an What neutral needs from you: a decision on how to close the route-namespace
Reproduction script used above is available on request (not committed; it lives reply with a comment on this PR (or push to the branch); neutral monitors this |
…451) (#520) * hyp session states that the control responder is never authenticated (#451) `validateControlResponse` (PR #439) proves the responder echoed our token; it cannot prove the responder IS the gateway. A local process that binds the resolved port and echoes the token back still yields `exit 0, ignored: true`, so the privacy control tells a user "you are opted out" when nothing recorded the decision. Per the maintainer's direction on #451, this is accept-and-document: the attack presupposes local code execution as this user, a gateway-written secret is readable by whoever can bind the port, and peer-process identity has no portable form (three platform implementations plus a native dependency). So the guarantee is stated rather than proved. - human output gains a `trust:` note beside every confirmed answer, on `status` and on both mutation verbs, naming the endpoint that was trusted - `--json` gains `endpoint_authenticated: false`, constant by contract rather than by outcome, so a consumer never infers authenticity from silence - the disclosure is unconditional and rides the `daemon_status` path too, which said nothing at all before: a live daemon's status.json is evidence about a past bind, not about who answers now - `endpoint_source` and every existing fail-closed refusal are unchanged LLP 0164 records the decision and carries the companion #460 contract statement (membership is not a match) so the control-plane guarantees read as one story; LLP 0067 §cli-response-check gets the forward-ref. test/plugins/ai-gateway-session-responder-trust.test.js stands up an impostor that echoes the token, pins that its answer is still believed (the accepted residual), and asserts the disclosure on every surface. Fixes #451 Co-Authored-By: Claude <noreply@anthropic.com> * Renumber LLP 0164 -> 0166 to resolve a cross-branch number collision Three open branches independently minted 0164 off the same master high-water mark, none able to see the others: fix/issue-421 (PR #502) status-names-recent-clients-from-gateway-entrypoints fix/issue-473 (PR #517) codex-flat-pair-needs-a-namespace-signal -> 0165 fix/issue-451 (PR #520) session-control-plane-states-its-guarantees -> 0166 PR #502 is held and approved awaiting a human merge, so it keeps 0164. This branch takes 0166; 0165 went to PR #517. No content change: the file is renamed and the number updated at every reference site (3 in session_command.js, 1 in the new responder-trust test, the Extended-by forward-ref in LLP 0067, and the doc title). The #stated-not-proved and #membership-not-grain anchors still resolve. * review: document endpoint_authenticated where the report shape lives Three review fixes on top of #451's disclosure work, no behaviour change: - `types.d.ts`: `SessionStatusReport` bills itself as "what `hyp session status` reports, in `--json` field order", and a maintainer adding a report shape reads it. It said nothing about `endpoint_authenticated`, so the constant-by-contract rule lived only inside `writeStatus`. State the envelope, the by-contract choice, and the "a real check needs a new field" consequence there, with the LLP 0166 ref. - LLP 0166 named `RESPONDER_TRUST_NOTE`, a symbol that does not exist; the code has `responderTrustNote(endpoint)`, and the reason it is a function (it names the endpoint) is the point. - Drop the unused `@import { IncomingMessage, ServerResponse }` copied into the new test from its sibling suite. Co-Authored-By: Claude <noreply@anthropic.com> * review: point the LLP 0067 link at the section its label names The new `responderTrustNote` docblock links `[LLP 0067 §cli-response-check]` at the file with no fragment, so a reader who follows the label lands at the top of a 600-line design doc. Every other fragment-bearing LLP link in the tree carries its anchor (`sync.js`, `verb_codec.js`, `remote_commands.js`, `first_sync_hold.js`); this one was the outlier. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: test <test@test.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: test <test@example.com>
Addresses both deferred, non-blocking review findings from PR #467, following the maintainer decisions recorded on issue #473 (A: tighten, B: proceed). Numbering note: the doc reviewed as LLP 0144 on the #467 branch landed on
masteras LLP 0151 (renumbered per LLP 0156); all references below use the merged number.A. A Codex user-agent could steer the LLP 0030 partition key - BEHAVIOURAL
Root cause.
hasCodexTransportSignalwas one predicate answering two different questions. It decided both may this exchange be called Codex (a label: revisable, cheap if wrong) and may this exchange's unnamespacedclient_metadataidentity pair be trusted (an identity: it clusters rows on disk and scopes the fallbackmessage_id). Its fourth branch is acodex-prefixeduser-agentproduct string, which any local process can copy without knowing anything Codex-specific, unlike the other three, which require Codex's proprietary route or header vocabulary. So a forgeduser-agent: codex_cli_rs/1.0plus a bodyclient_metadata: { session_id, thread_id }carrying nox-codex-*key let that pair dictateconversation_idandsession_id, the LLP 0030 partition key.Fix. Split the predicate exactly as the PR author sketched (LLP 0151 lines ~95-104):
hasCodexNamespaceSignal- thechatgptupstream, the/backend-api/codex/namespace, anx-codex-*compatibility header. This is now the only thing passed toreadCodexClientMetadataas corroboration.hasCodexTransportSignal- that union plus the user-agent branch, unchanged in effect for client naming and for whether a codex context resolves at all.Real Codex loses nothing: its map carries
x-codex-installation-idandx-codex-window-idon every request, which is the self-naming branch that never needed corroboration. The accepted cost, per the maintainer decision, is that a hypothetical future Codex build reachable only by user-agent and writing nox-codex-*map key loses lineage until this file is updated. That build surfaces as an absentlineage_sourcerather than as silence.Regression test evidence. New test
a Codex user-agent alone does not let a flat client_metadata pair dictate row identityasserts both halves of the asymmetry in one place: the row is stillclient_name: 'codex'(loose predicate), and the flat pair contributes nothing, with the row byte-identical to the same request sent without the map (strict predicate).Before the fix, on the test change alone:
After the fix:
Two companion tests pin what must NOT change: a flat-only map still resolves its lineage when a Codex-namespaced header corroborates it, and a user-agent-only request keeps full lineage whenever its map carries an
x-codex-*key (the shape all real Codex traffic has). The existing pinned-literal identity test (part_id and message_id stay byte-identical...) is untouched and still passes, so no recorded shape re-keys.B. LLP 0151's header audit overstated "nothing emits them" - NON-BEHAVIOURAL
No regression test is possible for this finding, and none is included. It is a documentation-accuracy defect only: the code's behaviour was already correct and is unchanged by this PR. The claim corrected is prose about an upstream project (
openai/codex), which is not observable from HypAware's own test surface.Root cause. LLP 0151 audited the header names the projector reads against Codex's turn-streaming path (
build_responses_request/stream_responses_api) and generalised the result to every path.codex-rs/codex-api/src/requests/headers.rs::build_session_headersinserts headers literally namedsession-idandthread-id, and has two further call sites incodex-rs/core/src/client.rs:compact_conversation_history(the/responses/compactendpoint, which the projector'sisOpenAiResponsesPathmatches) andbuild_websocket_headers.Fix. The audit's verdict survives its reason being wrong, so the correction is to the justification only. Both names remain correctly unread: compaction requests are
CodexResponsesRequestKind::Compaction, which Codex markshas_turn_identity = true, so theirx-codex-turn-metadatablob independently states the samesession_idandthread_idthatreadCodexTurnMetadataalready reads.Memoryrequests state identity on no surface at all by Codex's own design. Reading the two bare names would add no id HypAware does not already have, while reopening exactly what LLP 0151 closed.parent-thread-idis fictional as originally stated.Corrected at every echo, not only the two sites the issue names:
llp/0151-...md(Context consequence 2, and the audit table row) - via forward-refs, see belowtest/plugins/codex-exchange-projector.test.js(the comment the issue flags)hypaware-core/plugins-workspace/codex/src/exchange-projector.js(resolveConversationId)hypaware-core/smoke/flows/gateway_codex_capture.jstest/plugins/codex-rollout-cwd.test.jsLLP handling
LLP 0151 is
Active, and the house rule is that such a doc is a record, not a worksheet. Finding A changes what its#body-is-a-codex-signalsection decided, and finding B corrects a factual claim rather than a typo, so neither is the permitted editorial fix. Both land in new LLP 0164 (llp/0164-codex-flat-pair-needs-a-namespace-signal.decision.md), following the precedent of LLP 0133 carrying corrections for the immutable LLP 0115. LLP 0151 receives onlyExtended-byforward-refs at the two affected sections, which the convention explicitly allows. New@refanchors#flat-pair-corroborationand#header-audit-correctionare annotated on the code and tests that realize them.Checks
npm test: 3080 pass, 8 fail. All 8 failures (leave after join..., tests 892-900) are pre-existing onorigin/master, verified by stashing this branch's changes and re-running: identical 8 failures,# pass 3078. The +2 is this PR's net new tests.npm run typecheck: clean.npm run smoke -- gateway_codex_capture: ok. Its fixture carries anx-codex-installation-idmap key on a namespaced route, so it exercises the unchanged self-naming branch.Fixes #473