Skip to content

docs(plans): the protocol v4 wire break — forward secrecy, AAD over the frame's identity, and the cohort choreography - #361

Open
chrischall wants to merge 2 commits into
mainfrom
docs/protocol-v4-plan
Open

docs(plans): the protocol v4 wire break — forward secrecy, AAD over the frame's identity, and the cohort choreography#361
chrischall wants to merge 2 commits into
mainfrom
docs/protocol-v4-plan

Conversation

@chrischall

@chrischall chrischall commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Task H2 of the mcp-host single-tier plan asks for this document before anyone writes the code, because v4 is a wire break across ~31 npm packages and an installed Chrome extension, and the release choreography is the hard part.

The document covers what v4 changes: AAD over mcpId ‖ seq ‖ direction, an MCP-side per-session ephemeral X25519 in the hello covered by sessionSig so no identity holder can decrypt a past session, PROTOCOL_VERSION 3 → 4, and v3 refused at the hello with no negotiated downgrade.

The 2.0.0 break is recorded and was used as the template — its choreography is in the commit body of the ephemeral-key-binding change, with the standing paragraph in frames.ts and the one-liner in CLAUDE.md.

The ordering it lands on is cohort first, extension last. Both orderings have a broken window; only this one puts the window under the operator's hand. Extension-first breaks every bridged registration for the days it takes 31 npm releases and 20 re-pins to land; cohort-first breaks them for as long as one extension reload takes. A transitional dual-stack extension is explicitly forbidden, because it is the downgrade path the 2.0.0 advisory refused: a rewriting relay can set protocolVersion back to 3 and strip the ephemeral.

Worth stating plainly, since it bears on mcp-host's own work: sealing the stored bridge identity to the account's runner key narrows who holds it but does not give forward secrecy. That is this plan.

Documentation only. No code, no wire change, nothing released.

Closes #363

🤖 Generated with Claude Code

https://claude.ai/code/session_015Tar4Eh59YtFuxQy4BpRBQ

…he frame's identity, and the cohort choreography

PR H2 of the mcp-host single-tier plan, written before anyone starts because
it is a wire break and needs its own release choreography.

Sources: docs/beta-review-2026-09-10/fetchproxy-bridge.md H1 and M2 in
mcp-host. v4 gives the MCP a per-session ephemeral X25519 covered by its hello
signature (identities authenticate only, so no identity holder decrypts a past
session), puts `mcpId || seq || direction` in the AAD, salts the session key
with a transcript hash, and refuses v3 at the hello with no negotiated
downgrade — the #222 argument, unchanged.

The 2.0.0 break IS recorded (c13aeed, GHSA-j6jv-w774-77m6) and is the
template: advisory first, all packages together, old version refused at the
hello, fixtures moved onto one shared payload function rather than patched.
What is different is that the failure now lands on a hosted relay in front of
~20 bridged registrations, so v4 needs a refusal PATH and not only a refusal —
and both directions can be made clean by changing only the v4 side, which is
what makes the outage window one extension reload instead of an upgrade
ordering problem.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Tar4Eh59YtFuxQy4BpRBQ
@github-actions github-actions Bot added the auto-review Auto-review pipeline is handling this PR label Sep 11, 2026
@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 5m 45s —— View job


Review — docs(plans): protocol v4

  • Read CLAUDE.md conventions
  • Read the new plan document
  • Cross-check claims against the codebase (frames.ts, seal.ts, validate.ts, host.ts, peer.ts, hello.ts, socket.ts, SECURITY.md, release workflow)
  • Post review

This is a strong document and most of it verifies. I spot-checked every file:line it cites and they all land on what it says they land on: PROTOCOL_VERSION = 3 at frames.ts:40, HKDF_SESSION_INFO = 'fetchproxy/1.0.0/session' at :47, sealInnerFrame calling aesGcmSeal(sessionKey, iv, pt) with no AAD at seal.ts:140, the must be ${PROTOCOL_VERSION} throw at validate.ts:517-519, the console.warn drop at socket.ts:246-248, SESSION_READY_TIMEOUT_MS = 30_000 at session-ready.ts:9, the extension's hello going out in the socket open handler at socket.ts:197-208, claimInboundSeq/releaseInboundSeq at socket.ts:274-298, EXTENSION_ID_PLACEHOLDER at README.md:62. c13aeed is the #222 commit it claims (feat(protocol)!: bind the ephemeral key into the ready signature (#222), 2026-08-05) and ad6b673 is a real commit from 2026-09-11. The cohort list is internally consistent (31 names + mcp-utils = the 32 it says). The hello-rejected argument holds up — server-hello.ts:81 really does gate on accepts, and host.ts:569 really does fail the pending session on it.

The ordering argument (cohort first, extension last) and the refusal of a transitional dual-stack extension are both right, and the "both directions can be made clean by changing only the v4 side" table is the good insight in here.

Three things I'd want settled before this is executed.

1. 🔴 L6 can't be deferred past v4 — v4 is what makes it exploitable. (inline) Item 8 in "What this does NOT fix" says the missing record.identityEd25519Pub comparison costs no confidentiality. That's true of v3 only because the ECDH runs against identityX25519Pub (hello.ts:304) and is therefore an implicit proof-of-possession of the identity private key. §3 removes that, and helloSignaturePayload(mcpId, sessionNonce, sessionPub) doesn't cover identityX25519Pub either — so after v4 the trust-store primary key sha256(identityX25519Pub) (hello.ts:237-238) is a bare public label anyone who has read a plaintext hello can claim, with their own Ed25519 key and their own ephemeral, and the auto-trust branch has nothing left to catch it. The plan's own item 3 says the relay reads those hellos. Fold the one-line comparison into Group 3 (the record already carries the field, trust-store.ts:70) or sign identityX25519Pub.

2. 🔴 Task 2.2's "the same five changes" doesn't reach the peer path. (inline) A peer sends its hello once at dial and the host caches and replays it (host.ts:421-424, 373-376), so a peer minting at listen() gets a per-process ephemeral — precisely what §1 declines to call forward secrecy — on the path carrying most bridged registrations. The fix is available (the relayed extension hello at host.ts:367-370 is the mint trigger, and host.ts:409-421 already tolerates a re-hello on a live peer socket) but it needs host-side and extension-side changes the task doesn't name.

3. 🔴 @fetchproxy/cli is published, and drops out of the release verification. (inline) The table says "6 (4 published, 2 private)"; it's seven packages, five published — release-please.yml:251-259 publishes cli. Step 2 and "After every PR" both verify "all four", skipping the package Task 4.4 changes. release-please.yml:197-198 records that a hand-listed set has already silently skipped cli once.

One nit: Task 4.2 leaves the close code unspecified while Task 5.2 asserts 1008 (inline).

Open question, not a finding (I can't reach the referenced trees): the plan depends on mcp-host's bridge-frames.ts:32-38, PR A1, PR H1, SANDBOX_CONCENTRATOR_MIN, the ~20 bridged registrations and the 31 cohort pins at ^2.10.0, plus the two ~/.claude/projects/.../*.md memory files. None of those are verifiable from this checkout — worth someone with the trees confirming the A1 prerequisite in particular, since step 1 gates the whole sequence on it being live rather than merged.

Nothing here is a reason not to write the document; #1 and #2 are reasons to amend it before an agent executes it verbatim, which is what its header says will happen.

Verdict: fail
· branch docs/protocol-v4-plan

Comment on lines +812 to +816
8. **`Ed25519` key confusion in the extension's trust match** (**L6**):
`hello.ts:261-272` matches on the X25519 hash, serverName, domains and the
extension's own identity, never `record.identityEd25519Pub ===
hello.identityEd25519Pub`. No confidentiality loss, and it is a one-line
comparison — but it is not in this plan, so it is not in this release.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 L6 cannot be deferred past v4 — v4 is what makes it exploitable.

The claim "No confidentiality loss, and it is a one-line comparison — but it is not in this plan" is true of v3 and stops being true the moment §3 switches the ECDH to ephemeral × ephemeral.

Under v3, handleServerHello derives against the identity key itself:

packages/extension-core/src/background/hello.ts:304
  const shared = await ecdhX25519(ephemeral.privateKey, identityX25519Pub);

That is an implicit proof-of-possession of identityX25519Priv, and it is the only thing holding the trust lookup together — because the lookup keys on a hash of a public value:

hello.ts:226-234   verify sessionSig with identityEd25519Pub TAKEN FROM THE HELLO   (self-signed)
hello.ts:237-238   const hash = toHex(await sha256(identityX25519Pub)); trust.get(hash)
hello.ts:261-271   match on serverName, domains, the extension's own identity — never record.identityEd25519Pub

So today an impersonator who knows a trusted MCP's identityX25519Pub (plaintext in the hello; this plan's own item 3 says the hello "is plaintext by construction", and the hosted relay reads it) does reach the auto-trust branch — and then derives a session key nobody else holds and the session is stillborn. Harmless, exactly as this item says.

Under v4 as specified, the ECDH no longer touches identityX25519Pub, and helloSignaturePayload(mcpId, sessionNonce, sessionPub) does not cover it either. Nothing in the v4 handshake binds the claimed X25519 identity to any secret. The attacker presents identityX25519Pub = the victim's, its own identityEd25519Pub, its own ephemeral, a valid self-signature, the same serverName and the same domains — the record matches (L6 means Ed25519 is never compared), auto-trust fires, the key agrees, and the session works with the victim's approved capabilities, cookie keys and domains. No pair prompt, because auto-trust doesn't raise one.

That is a new capability for the hosted relay, granted by the same release that is meant to take one away.

Two ways out, either is small, but one of them has to be in Group 1–3 rather than in this list:

  • add record.identityEd25519Pub === hello.identityEd25519Pub to the match at hello.ts:261 (the record already carries the field — packages/extension-core/src/trust-store.ts:70), and/or
  • cover identityX25519Pub in helloSignaturePayload, so the X25519 identity claim is at least signed by the key that is authenticated.

Related, and part of the same root: line 167-168 reassures that "the pair code commits to both identities" — but decision 2's default re-derives the pair code from transcriptHash (nonces + ephemerals), which drops that commitment. If decision 2 is taken, that sentence needs rewriting, because after v4 + decision 2 identityX25519Pub is a bare unauthenticated label that happens to be a trust-store primary key.

Fix this →

Comment on lines +411 to +420
**Task 2.2 — the peer.** Where: `packages/server/src/peer.ts:153`, `:275`,
`:385`, `frame-size.ts`.
Test (`packages/server/tests/peer-hello-auth.test.ts` and the integration
suites under `tests/integration/`, first): the peer path derives the same key
the host path does against the same mock extension; a peer whose hello omits
`sessionPub` is refused by the host at registration.
Do: the same five changes. The peer's `requireExtensionIdentity` /
`warnedUnverifiable` branch (`peer.ts:240-271`) is about a pre-1.12.0 *host*
and is orthogonal — leave it, and check its wording still reads right beside a
v4 refusal.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 "The same five changes" doesn't reach the peer path — a peer's hello is sent once, at dial, and cached by the host.

§1 is emphatic that the ephemeral must be minted per extension connection, and explicit that a per-process one is "worth having, not worth claiming as forward secrecy". For the host that follows from moving the build to host.ts:371. For a peer there is no equivalent site, and this task doesn't name one.

A peer builds its hello once in listen() and sends it once when it dials. The host then caches the frame and replays the cached copy at every extension connection:

packages/server/src/host.ts:421-424
  peers.set(frame.mcpId, { ws, helloFrame: frame });
  if (extensionWs) extensionWs.send(JSON.stringify(frame));
packages/server/src/host.ts:373-376
  // Then forward any peer hellos that arrived earlier.
  for (const slot of peers.values()) ws.send(JSON.stringify(slot.helloFrame));

So a peer that mints one ephemeral at listen() hands the same sessionPub to every extension connection for the life of the process — the property §1 declines to call forward secrecy, on the path that carries most of mcp-host's bridged registrations.

The mechanism exists to fix it, which is why this is worth spelling out rather than leaving to the executing agent: 1.12.0 already relays the extension hello to peers (host.ts:367-370), so that frame is the peer's mint trigger, and host.ts:409-421 already tolerates a re-hello on a live peer socket (existing.ws !== ws is false, so the slot is simply re-set and re-forwarded). But that makes it a host-side change too — accept the re-hello, replace slot.helloFrame, relay the new one — plus an extension-side question this task doesn't ask: what onServerHello does with a second hello for an mcpId that already has a live session. The file map's host.ts row lists "per-connection hello" meaning the host's own, and nothing about re-relaying a peer's.

Either name those two changes here, or say outright that the peer path's ephemeral is per-process for v4 and that §1's forward-secrecy claim is host-only — but it can't be left at "the same five changes".

Fix this →

Comment on lines +605 to +610
| Population | Count (measured 2026-09-11) | How it moves | Latency |
|---|---|---|---|
| `@fetchproxy/*` packages | 6 (4 published, 2 private) | release-please, one combined PR, one `v3.0.0` tag, one publish job | minutes after the release PR merges |
| Cohort npm consumers | **31 `*-mcp` repos** pinned at `^2.10.0` (moving to `^2.11.3` in PR H1), plus `@chrischall/mcp-utils`, whose declaration is a `*` **peer** and needs no range edit | 31 PRs, each `fix(deps):`, each its own release-please cycle, each its own npm publish | hours, and unattended it is days |
| Bridged registrations on mcp-host | **~20** on the shared tier | a source PUT (or `mcp-host update-all`) per registration → new `configHash` → new install slot → builder artifact → restart | minutes if driven; **a night** if left to the `follow` cron |
| The browser extension | effectively **one installed copy** today (see below) | rebuild `dist/`, reload at `chrome://extensions`, or install the GitHub-release `.zip` | seconds |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The package count is wrong and @fetchproxy/cli drops out of the release verification — the one miss this repo has already had.

"6 (4 published, 2 private)" is measured as of today:

packages/{bootstrap,cli,extension-chrome,extension-core,protocol,server,test-helpers}

Seven, and only extension-core / extension-chrome carry "private": true. @fetchproxy/cli is published — release-please-config.json propagates its version (and packages/cli/src/version.ts), and .github/workflows/release-please.yml:251-259 has a Publish @fetchproxy/cli to npm step. So it is 5 published, 2 private.

That is not just an arithmetic slip, because it propagates into the checks:

  • Step 2 (line 646-649): "npm view @fetchproxy/protocol version and the same for server, bootstrap, test-helpersall four must read 3.0.0."
  • "After every PR" (line 830): "npm view all four published packages before believing the tag."

Both skip cli — and Task 4.4 puts a change in packages/cli/src/bridge-errors.ts whose whole job is to give the operator an actionable remedy while debugging a mid-window straggler in step 5. A silent cli publish failure means fpx keeps emitting the generic bridge-unavailable hint exactly when the plan is relying on it.

The workflow itself records that this class of miss has already happened here:

.github/workflows/release-please.yml:197-198
  # Root build (all workspaces) so every publishable package's dist/ is
  # present — a hand-listed --workspace set silently skipped @fetchproxy/cli.

Fix the row to 7 (5 published, 2 private) and add cli to both npm view lists.

Fix this →

Comment on lines +482 to +491
Test (`packages/server/tests/host.test.ts`, first): a socket that sends an
extension hello with `protocolVersion: 3` is closed within one tick **with a
reason naming both versions** (assert the reason string, not only the code);
the pending `ownSessionReady` rejects immediately with an error whose message
names both versions and the extension version to install — **not** after
`SESSION_READY_TIMEOUT_MS` (assert against a fake clock); a `request()` issued
afterwards fails fast with the same message; a frame that is malformed for any
*other* reason still closes with today's generic `1002 'protocol error'` and
leaves the pending session alone, so the mismatch is the only case that gets
the new treatment.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: Task 4.2 never names the close code, and Task 5.2 asserts one.

Here the instruction is "closed within one tick with a reason naming both versions (assert the reason string, not only the code)", and the contrast case is "today's generic 1002 'protocol error'". Task 5.2 case 1 (line 523-525) then asserts "Host closes 1008".

An agent executing 4.2 in isolation has the existing host.ts:281-284 catch in front of it, which closes 1002 — so the natural implementation keeps 1002 and Task 5.2's test fails a group later. Worth pinning it in 4.2, since 1008 is the repo's established code for a policy refusal (host.ts:260, 291, 314, 327, 403, 418, 474, peer.ts:379) and is clearly what's intended.

Fix this →

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🔴 Auto-review verdict: fail — The revision addresses all four findings from the previous round correctly — L6 is now Task 3.3 with the proof-of-possession argument, the peer ephemeral has a real mint point, the package count is 7/5-published, and the close code is 1002 in both places — and every file:line and quoted comment I checked verifies against the tree. Two gaps remain: Task 2.2 fixes the cached peer-hello replay but not the dial hello that host.ts:425 forwards to an already-connected extension, and decision 2's default assigns the pair-code change to a "Task 8" that does not exist in the document.
📋 Tracking follow-ups: #363

…ng from

Four corrections from the review of #361.

Proof of possession. Under v3 the extension derives against
`identityX25519Pub` — the key its trust record is keyed on — so completing a
session IS proof the far end holds the pinned private key, and `hello.ts`
never comparing `record.identityEd25519Pub` (L6) is harmless because a party
that swapped the signing key still could not compute the session key. v4
inverts that: the key comes from `sessionPub`, bound to an identity only by a
signature under the half nothing checks, so an attacker holding nothing but
public values presents a trusted `identityX25519Pub` with an Ed25519 key and
ephemeral of their own, passes a self-consistent signature check, hits the
genuine record on `sha256(identityX25519Pub)`, auto-trusts with no pair
prompt, and owns the session. L6 therefore moves off "what this does not fix"
and becomes Task 3.3, with a test that fails when the comparison is deleted
and a fifth mutation in Task 5.3. The MCP side needs nothing and the plan now
says why: it never had the implicit proof, so `decideExtensionTrust` already
compares both pinned keys — its own comment is the sentence the extension's
match is missing.

The peer's ephemeral. "The same five changes" left it per-process, because
the peer's only socket goes to the host and outlives every extension session
(evictions arrive as fresh `ready` frames, which peer.ts:392 already calls a
renegotiation). Task 2.2 now mints on the relayed extension hello and names
where the lifetime ends, plus the two host consequences that make it work —
the cached peer hello replayed to each new extension (host.ts:374-376) is
stale by construction under v4 and the replay goes — and retires the
`warnedUnverifiable` branch, which cannot proceed once the transcript salt
needs an `extNonce` only that hello carries.

The package count. 7 packages, 5 published: `@fetchproxy/cli` publishes and
ships a v4 change of its own in Task 4.4. Corrected in the table, in step 2's
`npm view` check and in the closing line, with a note that CLAUDE.md's
workspace table is where the wrong count came from.

The close code. 1002 in both places — the code already there, and the one
this file spends on protocol errors rather than the 1008 it spends on
identity refusals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Tar4Eh59YtFuxQy4BpRBQ
@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 4m 36s —— View job


Re-review — docs(plans): protocol v4

  • Read CLAUDE.md conventions
  • Read 1d1f8e9 and the current document
  • Check whether the three prior 🔴 findings are addressed
  • Cross-check the new claims against the codebase
  • Post review

All four items from the last round are addressed, and the three I could check are checked correct.

  • L6 folded in. The new §2 argument — v3's ECDH against identityX25519Pub was an implicit proof of possession, v4's ephemeral×ephemeral removes it — is right, and L6 is now Task 3.3 with a mutation entry rather than item 8 of "what this does not fix". The citations hold: hello.ts:237-238 is the sha256(identityX25519Pub) lookup, :261-272 is the scopeIdentityChanged disjunction the new clause joins, trust-store.ts:115 has identityEd25519Pub: string (required) and :214 writes it unconditionally, and the decideExtensionTrust comment is quoted verbatim from packages/server/src/extension-trust.ts:192-194. The "do not normalise an absent stored value" warning is the right lesson from extensionIdentityX25519Pub ?? '' two lines below.
  • Peer ephemeral. Task 2.2 no longer says "the same five changes". peer.ts:181 / :202 / :350 / :357 / :385 / :392 and host.ts:370 / :374-376 / :411-420 / :424 all land where claimed, and the warnedUnverifiable retirement is sound: ReadyFrame (frames.ts:427-443) carries no extension nonce, so extNonce really does come only from the relayed hello and a peer without it cannot compute the transcript salt. See the one gap below.
  • Package count. ls packages/bootstrap cli extension-chrome extension-core protocol server test-helpers; only extension-core and extension-chrome carry "private": true. 7 / 5 published / 2 private, as the table now says, and cli is in both npm view lists.
  • Close code. 1002 in Task 4.2 and Task 5.2, with the RFC-6455 reasoning. Matches host.ts:284 (1002 'protocol error') against :291 :314 :327 :403 :418 :474 (all 1008, all identity/authorization). The nit is resolved and resolved the right way round.

Two things I'd still want settled, one of them the same path as last time.

1. 🔴 Task 2.2 fixes the cached replay but not the dial hello, and host.ts:425 forwards that one. (inline) The peer's registration hello must carry a sessionPub to survive the host's pre-routing validation (host.ts:380-404) — the task's own test says so — so startPeer still mints, which the "rather than on startPeer" instruction denies. And when a peer joins an already-connected extension, host.ts:425 hands that dial hello to the extension in the same block that hands extensionHello to the peer at :427, so the peer supersedes and zeroes the bootstrap key before the ready for it arrives: firstReady resolves on a dead key, then hello #2's ready makes isRenegotiation true (peer.ts:392) and fires rejectAllPending. That's a third host consequence, in a list the document calls exhaustive.

2. 🔴 Decision 2's default assigns the pair-code change to a "Task 8" that doesn't exist. (inline) Tasks run 1.1 → 6.5; there is no Task 7 or 8. pair-code.ts appears only as a file-map row, with no task, no test, and no entry in Task 5.3's (now five-item) mutation list. Take the stated default and L5 silently doesn't ship — the outcome decision 2 itself calls unacceptable.

One nit: three more stale flat task numbers — Task 7 at :383, Task 2 at :309, Task 6 at :357 (inline).

Open question, unchanged and still not a finding (the trees aren't reachable from this checkout): everything the plan asserts about mcp-host — bridge-frames.ts:32-38, PR A1's deploy state, SANDBOX_CONCENTRATOR_MIN, the ~20 bridged registrations, the 31 cohort pins — plus the ~/.claude/projects/.../*.md memory files. Step 1 gates the whole sequence on A1 being live, not merged, which is worth someone confirming with the gateway in front of them.

Verdict: fail
· branch docs/protocol-v4-plan

Comment on lines +489 to +502
**Two consequences in the host, both required for that to work.**

- The host caches each peer's hello (`peers.set(mcpId, {ws, helloFrame})`,
`:424`) and **replays it to every newly connected extension** (`:374-376`).
Under v4 the cached frame is stale by construction — the private half it
names is gone — so the extension would derive against a key nobody holds.
Drop the replay: the relay the host already performs at `:370` (extension
hello → every peer) is what prompts each peer to hello afresh, and the host
forwards those as they arrive. The cost is one round trip before a peer's
session opens, which the peer already pays waiting for `ready`.
- A same-socket, same-identity re-hello must REPLACE the slot rather than be
refused. It already does — the squat guard at `:411-420` fires only when
`existing.ws !== ws` — but under v4 that is load-bearing rather than
incidental, so assert it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 "Two consequences in the host, both required" misses the third, and it is the one on the common path. The dial hello still needs an ephemeral, and host.ts:425 forwards it to a connected extension.

The revision fixes the cached replay (:374-376). It leaves the immediate forward at registration:

packages/server/src/host.ts:424-427
  peers.set(frame.mcpId, { ws, helloFrame: frame });
  if (extensionWs) extensionWs.send(JSON.stringify(frame));
  // 1.12.0 (#208): a peer joining an already-connected extension needs
  // the same identity material a peer that was here first receives.
  if (extensionHello) ws.send(JSON.stringify(extensionHello));

Two things follow that this task does not say.

1. The dial hello must still carry a sessionPub, so startPeer still mints. The peer's registration hello is built at peer.ts:181 and sent at :201, and the host validates it before routing (host.ts:380-404, "authenticate the peer hello BEFORE touching the routing table"). Under Task 1.1 a server hello without sessionPub is refused — which this task's own test asserts ("a peer whose hello omits sessionPub is refused by the host at registration"). So a peer that mints only at peer.ts:350 cannot register, and never receives the relayed extension hello that was going to be its mint trigger. "Do: the five changes of Task 2.1, mounted on the mint point above rather than on startPeer" reads as an instruction to remove the startPeer mint; it has to say instead that startPeer mints a bootstrap pair for registration which the first relayed extension hello supersedes.

2. That bootstrap pair gets a ready for it. When a peer joins an already-connected extension — the normal case for a second MCP on a laptop and for every bridged spawn on mcp-host — the host forwards the dial hello to the extension (:425) and sends extensionHello to the peer (:427) in the same synchronous block. The peer therefore mints afresh and, per the lifetime rule ("the next extension hello superseding it"), zeroes the bootstrap sessionPrivbefore the extension's ready for the bootstrap sessionPub completes its round trip. That ready then derives a key against a private half that no longer exists, which is the failure frames.ts:44-46 already names ("the derived keys diverge and AES-GCM decryption fails silently"), and it arrives at peer.ts:369 as a first ready that resolves firstReady — so sessionLinked() goes true on a dead key. Hello #2 then produces ready #2, where isRenegotiation is now true (peer.ts:392) and renegotiateListeners fires rejectAllPending, failing any request issued in the window.

The fix is available and belongs in the same bullet list: under v4 the host must not forward a peer's dial hello to a connected extension either — the :370 relay already prompts a fresh one — or the peer must hold the bootstrap private half until superseded by a ready. Either way it is a third host consequence and a third test, not a detail the executing agent can be left to discover from a flaky suite.

Fix this →

Comment on lines +120 to +131
changelog from release-please's own arithmetic for one cosmetic gain.
2. **Does the pair code change in the same break?** Bridge report **L5** —
6 digits, `SHA256(mcpPub‖extPub)`, both inputs public and long-term, so a
MITM grinds a target code in ~10⁶ keygens. v4 introduces a transcript hash
that already contains both fresh nonces and both ephemerals; deriving the
pair code from it and lengthening to 8 digits is a dozen lines and removes
the grind entirely. It is a wire-visible change and this is the only break
scheduled. **Default: YES, fold it in** (Task 1 mints the transcript hash;
Task 8 re-derives the code from it), because a second break to fix L5 later
costs the whole of Group 4 again. If Chris says no, cut Task 8 and move L5
to "what this does not fix".
3. **Does mcp-host raise a protocol floor for bridged spawns?** Today

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Decision 2's default routes the pair-code change to a "Task 8" that does not exist, so with the default taken L5 ships in neither this break nor any task.

The default here is YES, fold it in, and the mechanism given is "Task 1 mints the transcript hash; Task 8 re-derives the code from it". The document's task list is:

$ grep -n '^\*\*Task ' docs/plans/2026-09-11-fetchproxy-protocol-v4.md
401:**Task 1.1  416:**Task 1.2  430:**Task 1.3
447:**Task 2.1  463:**Task 2.2
534:**Task 3.1  549:**Task 3.2  559:**Task 3.3
595:**Task 4.1  611:**Task 4.2  638:**Task 4.3  649:**Task 4.4
658 (Group 5): **Task 5.1  670:**Task 5.2  689:**Task 5.3
706:**Task 6.1 … 737:**Task 6.5

No Task 8, and no Task 7. The only other trace of the work is one file-map row — packages/protocol/src/pair-code.ts | "decision 2 only — derive from the transcript hash, 8 digits" — with no owning task, no test, and no entry in Task 5.3's mutation list (which the revision grew from four facts to five, for L6, and still does not include the pair code).

So an agent executing group by group produces a v4 whose pair code is still SHA256(mcpPub‖extPub) over two public long-term values, while the PR body and this document both record L5 as folded into the break. That is exactly the outcome decision 2 says is unacceptable — "a second break to fix L5 later costs the whole of Group 4 again" — arrived at silently.

Either add the task (it belongs in Group 1 beside Task 1.1, which mints transcriptHash, and needs the extension side too: hello.ts / approval.ts derive and display the code the user compares), or flip the default to NO and move L5 down to "what this does not fix", which the text already provides for.

Fix this →

| Modify | `packages/extension-core/src/popup/popup.ts` | a protocol-mismatch line on the link |
| Modify | `packages/cli/src/bridge-errors.ts` | map the mismatch reason to a remedy |
| Modify | `packages/test-helpers/src/index.ts` | mock signature follow-through |
| Create | `packages/server/tests/cross-version/` | frozen v3 fixtures + the refusal suite (Task 7) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: three more stale flat task numbers, from the draft where tasks weren't grouped.

$ grep -n "Task 7\|Task 6's test\|Task 2 asserts" docs/plans/2026-09-11-fetchproxy-protocol-v4.md
309:`MAX_FRAME_BYTES` derivation are untouched, and Task 2 asserts that so nobody
357:member pattern, and Task 6's test asserts the "grants nothing" half by
383:| Create | `packages/server/tests/cross-version/` | frozen v3 fixtures + the refusal suite (Task 7) |
  • :383 "(Task 7)" → Tasks 5.1 / 5.2.
  • :309 "Task 2 asserts" (the sealedFrameWireBytes-unchanged assertion) → Task 1.2, which is where that assertion actually lives.
  • :357 "Task 6's test asserts the 'grants nothing' half" (about peekHelloVersion) → Task 1.3.

Unlike the Task 8 above these all point at work that exists, so they cost a fresh agent a search rather than a task — but this document's header says each agent gets one task's text and only the files it names, so a number that resolves to nothing is worse here than in a document a human reads end to end.

Fix this →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Auto-review pipeline is handling this PR

Projects

None yet

1 participant