Skip to content

feat(extension): dial configured remote bridges alongside loopback - #233

Merged
chrischall merged 1 commit into
mainfrom
feat/extension-remote-bridge-target
Aug 9, 2026
Merged

feat(extension): dial configured remote bridges alongside loopback#233
chrischall merged 1 commit into
mainfrom
feat/extension-remote-bridge-target

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Step 3 of chrischall/mcp-host#162's build order — the change that makes a hosted browser-bridge MCP reachable at all. Step 2 (the MCP-side identity pin, #208) shipped in 2.0.0 and is a precondition for pointing this at anything hosted; the cohort is already on @fetchproxy/server@^2.0.0.

What this is

The extension may now hold several bridges at once: ws://127.0.0.1:37149 always, plus zero-or-more configured wss:// targets (popup → Bridges). Remote targets are strictly additional — nothing in storage or the popup can remove or repoint the loopback link.

What actually changed, and why it isn't a refactor

Three things were global because there was only ever one socket. Each of them is a bug once there are two:

  • the handshake nonce. ready.sessionSig covers (mcpHelloNonce || extHelloNonce || extensionSessionPub) and the MCP verifies it against the hello it was handed. A global nonce signs one bridge's ready with another bridge's handshake. Now each link handshakes with its own, and multi-link.test.ts verifies a ready against both nonces — passing under its own, failing under the other.
  • routing. mcpId is <serverName>:<version>:<16-hex>, minted by the MCP, so it is not a name this extension may treat as unique across relays. An id binds to the link its hello arrived on; a second link claiming a bound id is refused, not re-pointed; a frame on the wrong link is dropped before decryption; a response goes out on the link the request came in on or not at all.
  • teardown. A dropped link takes its own sessions, scope grants and bindings — and nobody else's. clearSessionScopeFor joins clearAllSessionScopes on one derived list, so a thirteenth scope table fails a test rather than leaking a grant through the new path.

Refusals at save time, not in a retry loop

wss:// unless the host is loopback; no credentials in the URL (a URL is the part that gets pasted into a chat); and a credential that a WebSocket subprotocol can carry — a service worker cannot set a request header, so it travels as fetchproxy.token.<credential> beside fetchproxy.bridge.v1, and that grammar is narrower than base64. Each of these fails in front of the person making the decision.

A dead remote target is cheap: longer backoff than loopback, enforced by a deadline on the link rather than a timer, because the MV3 keepalive alarm calls connect() every 24 s regardless of any timer.

Docs

  • docs/SECURITY.md §T-remote-bridge — what widens (which relay to trust is now a user decision; the population that can reach this browser; pairing over a WAN) and what does not (a concentrator has never been able to read what it routes; every MCP still pairs, declares and encrypts identically).
  • docs/PROTOCOL.md §Remote relays — the contract a relay must satisfy: forward hello verbatim in both directions, route on its own bookkeeping and never on what a frame asserts, mint nothing, and expect a browser reconnect to invalidate every link built on the old hello.

Tests

44 new: remote-targets (validation/normalisation), links (binding rules), multi-link (two bridges end to end with real X25519/Ed25519/ECDH/AES-GCM — only the tab is faked), and the popup section. Each guard was mutation-checked: removing the routing refusal, the binding refusal or the per-link nonce each turns tests red.

The extension has had exactly one place to connect since it existed:
`ws://127.0.0.1:37149`, which only a process on the user's own machine can
reach. That is fetchproxy's whole stated trust model, and it is why hosting a
browser-bridge MCP has been impossible — the MCP has to be on the laptop.

This adds a second kind of link. A user may configure `wss://` targets in the
popup, each a URL plus a credential, and the extension holds them open
ALONGSIDE loopback. Nothing removes or repoints the local link; remote targets
are strictly additional.

The substance is in what became per-link, because a global was the shape of a
bug once there is more than one bridge:

- **the handshake nonce.** `ready.sessionSig` commits to the nonce of the
  connection its hello arrived on, so a global nonce would sign one bridge's
  ready with another's handshake. Now each link handshakes with its own.
- **routing.** `mcpId` is minted by the MCP, so it is not a name this extension
  may assume is unique across relays. An id binds to the link its hello arrived
  on; a second link claiming a bound id is refused rather than re-pointed; a
  frame arriving on the wrong link is dropped before it is decrypted; and a
  response goes out on the link the request came in on or not at all.
- **teardown.** A link dropping takes its own sessions, scopes and bindings and
  leaves every other link's alone.

Refusals at save time rather than in a retry loop: `wss://` unless the host is
loopback, no credentials in the URL (a URL gets pasted into chats), and a
credential that is a valid WebSocket subprotocol — a service worker cannot set
a request header, so it travels as `fetchproxy.token.<credential>` next to
`fetchproxy.bridge.v1`, and the subprotocol grammar is narrower than base64.

A dead remote target is cheap: a longer backoff than loopback, and a deadline
on the link rather than a timer alone, because the MV3 keepalive alarm calls
`connect()` every 24 s no matter what a timer says.

docs/SECURITY.md §T-remote-bridge states what widens (which relay to trust is
now a decision; the population that can reach this browser; pairing across a
WAN) and what does not (a concentrator has never been able to read what it
routes). docs/PROTOCOL.md states what a relay MUST do — forward `hello`
verbatim in BOTH directions, route on its own bookkeeping, mint nothing.

The MCP-side identity pin (#208, 1.12.0) is a precondition for pointing this at
anything hosted, not an enhancement to it.

Refs chrischall/mcp-host#162

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vdja1urhRqVxutxqdALPNs
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Aug 9, 2026
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

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


Review — feat(extension): dial configured remote bridges alongside loopback

  • Read CLAUDE.md conventions
  • Read the diff (links.ts, socket.ts, remote-targets.ts, server-hello.ts, session-scope.ts, approval.ts, boot.ts, popup)
  • Check test coverage of the new paths
  • Post review

No 🔴 Important findings. The three globals-that-become-bugs the description names are each genuinely de-globalised, and the tests pin the parts that matter:

  • per-link nonceLink.sessionNonce (links.ts:56) is read at the point of signing in both senders (server-hello.ts:106-113, approval.ts:153-156), and multi-link.test.ts:248-278 verifies a ready under its own link's nonce and asserts it fails under the other's. That last negative assertion is the one that would catch a regression to a global.
  • routing — binding is created only from a hello that arrived on the link, bindMcpToLink refuses a cross-link claim without re-pointing (links.ts:109-114), onEncryptedFrame drops on identity before touching the session key (socket.ts:263), and sendInner refuses to answer an unbound id rather than picking whichever socket is open (send-inner.ts:32-33).
  • teardownteardownLink clears only that link's ids, and SCOPE_TABLES is now the single derived list behind both clearAllSessionScopes and clearSessionScopeFor, with session-scope-teardown.test.ts asserting set-equality against the module's own exports.

Two things I checked specifically because per-link teardown is narrower than the old clear-everything:

  • every path that writes the mcp* scope tables (server-hello.ts auto-trust, approval.ts pair and scope-update) runs only for an mcpId that is bound to a live link, so there's no id whose scope survives with nothing left to tear it down.
  • state.ws / state.currentExtSessionNonce / state.reconnectAttempt have no readers left outside links.ts and the header comments (rg over packages), so nothing is still reading the removed globals.

The backoff-as-deadline point is real and load-bearing given the 24 s keepalive, and multi-link.test.ts:375-401 covers both directions of it (remote held, loopback re-dialable at 600 ms).

🟡 Nits (non-blocking, posted inline)

  1. A rejected hello keeps the binding it createdserver-hello.ts:74-87 binds before deciding and returns on reject without unbinding, so a refused id is held until the link drops, and a hello flood grows mcpLink unboundedly. Fix needs to be conditional so a same-link re-bind over a live session isn't torn down.
  2. clearAllSessionScopes now has no production caller — only the definition and tests match; its doc comment still says "Called on WS teardown".
  3. The connection badge can't express "loopback is down"anyLinkOpen() keeps it green while a remote link is up, and the popup's bridge rows carry enabled but no connected bit.

Open questions (not findings)

  • I could not run npm test or npm run build in this environment — both were denied at the approval gate — so I have not observed the 44 new tests passing or the extension bundle typechecking. Everything above is read from source, not executed. Worth confirming CI is green before merge.
  • docs/PROTOCOL.md and docs/SECURITY.md describe the feature as 2.1.0+ while manifest.json/root package.json are on 2.0.0. That reads correct to me under the release-please convention (versions are never bumped by hand), but flagging it since the docs assert a version that doesn't exist yet at merge time.

Verdict: warn
· branch feat/extension-remote-bridge-target

`[fetchproxy] dropped hello for ${hello.mcpId} on ${link.label}: that id is already bound to another bridge`,
);
return;
}

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 — a rejected hello keeps the binding it just created.

bindMcpToLink runs before handleServerHello, and the reject return at line 85-87 leaves the mcpId → link entry in place. Two consequences, both small but both avoidable:

  • an mcpId that was refused on the remote link is now unavailable to the loopback link until the remote link drops (unbindLink in teardownLink is the only remover — rg finds no other caller);
  • a relay that streams hellos with fresh mcpIds grows mcpLink without bound for the lifetime of the link, since none of them are ever bound to a session that could be torn down.

Unbinding on reject would close both, but it has to be conditional: an id already bound to this link by an earlier successful hello must not be unbound by a later bad one (bindMcpToLink returns true for a same-link re-bind, so the "did I just create this?" bit has to come from linkForMcp(hello.mcpId) read before the bind).

Comment on lines 226 to +232
/**
* Clear every per-mcpId scope table. Called on WS teardown so a reconnect
* cannot inherit a previous connection's granted scope.
*/
export function clearAllSessionScopes(): void {
mcpDomains.clear();
mcpCapabilities.clear();
mcpCookieKeys.clear();
mcpLocalStorageKeys.clear();
mcpSessionStorageKeys.clear();
mcpCaptureHeaders.clear();
mcpIndexedDbScopes.clear();
mcpDomSelectors.clear();
mcpGraphqlOps.clear();
mcpLocalStoragePointers.clear();
mcpSessionStoragePointers.clear();
// Part 3: clear identity hash map on teardown.
mcpIdentityHash.clear();
for (const table of SCOPE_TABLES) table.clear();
}

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 — clearAllSessionScopes has no production caller after this change, and its doc comment now describes something that no longer happens.

socket.ts imports only clearSessionScopeFor (line 54), and teardown is per link. Searching the tree:

$ rg -n "clearAllSessionScopes" --glob '!**/dist/**' packages docs
packages/extension-core/src/background/session-scope.ts:11: * `clearAllSessionScopes` is the one piece of new code in this move: it
packages/extension-core/src/background/session-scope.ts:230:export function clearAllSessionScopes(): void {
packages/extension-core/tests/session-scope-teardown.test.ts:5:  clearAllSessionScopes,
packages/extension-core/tests/session-scope-teardown.test.ts:37:    clearAllSessionScopes();
packages/extension-core/tests/session-scope-teardown.test.ts:74:    clearAllSessionScopes();

— only the definition and tests. "Called on WS teardown so a reconnect cannot inherit a previous connection's granted scope" is no longer true of any code path, and the module header at line 11-15 still frames it as the teardown function. Either drop it (the SCOPE_TABLES derivation test is what actually guards the thirteenth map, and it doesn't need this function) or say in the comment that it survives as a test/reset helper.

scheduleReconnect();
ws.addEventListener('close', (ev: CloseEvent) => {
teardownLink(link);
if (!anyLinkOpen()) setConnectionStatus('disconnected');

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 — with more than one link, the badge stops being able to say "the local bridge is down."

setConnectionStatus is a single global (badge.ts:37, currentConnectionStatus), so anyLinkOpen() here means: loopback drops, a remote link is still up → the badge stays green (BADGE_CONNECTED_COLOR) while every local MCP is unreachable. And the popup's new RemoteTargetView (popup.ts) carries enabled but no connected bit, so the Bridges list can't disambiguate it either — "configured and enabled" renders identically whether the socket is open or has been failing on 60 s backoff since boot.

Not wrong, and per-link badge colour isn't obviously the answer, but a connected flag on the bridge rows (loopback included) would give the user somewhere to look; the CLAUDE.md note that "the badge is the reliable surface" is doing less work now than it was.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

🟡 Auto-review verdict: warn — The per-link nonce, per-link routing and per-link teardown are correctly de-globalised and well covered by the new tests; no correctness or disclosure defect found. Three minor issues surfaced as nits (rejected-hello binding leak, dead clear-all teardown function, badge can no longer express a dead loopback link).
📋 Tracking follow-ups: #234

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Aug 9, 2026
@chrischall
chrischall enabled auto-merge (squash) August 9, 2026 21:31
@chrischall
chrischall merged commit cc81e8a into main Aug 9, 2026
17 checks passed
@chrischall
chrischall deleted the feat/extension-remote-bridge-target branch August 9, 2026 21:32
chrischall added a commit that referenced this pull request Aug 9, 2026
Follow-up to #233, and the last piece chrischall/mcp-host#162 lists for
the extension side.

`download` answers with a **filesystem path on the machine running the
browser**, on the explicit assumption that the MCP asking reads the same
disk. True for every MCP on `127.0.0.1`; false for one reached through a
relay — and two things go wrong there, not one:

- the path names a file the MCP cannot open, so a hosted MCP "succeeds"
and then fails somewhere unrelated;
- a remote MCP gets to write bytes into the user's Downloads folder. It
is the only verb in this protocol that leaves something behind on the
machine, so it is also the only one whose reach across a bridge is a
question about the machine rather than about the session.

A `download` arriving on a remote link is now refused with a reason the
calling MCP can print, **before the domain check** — "this cannot work
from there" is a fact about the link, not about the URL, and answering
it first stops an MCP concluding the verb works and its URL was wrong.
On the loopback link every gate behaves exactly as before.

Documented in `docs/SECURITY.md` §T-remote-bridge and `docs/PROTOCOL.md`
§Remote relays.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Aug 9, 2026
…bridge's own state (#236)

The three nits from #233's auto-review. Each is a small lie that gets
bigger once a second bridge is attached.

**A rejected hello kept the binding it took.** The bind happens before
the decision deliberately — so a second link cannot claim the id
mid-decision — but a *refused* id is one this extension is not speaking
for. Held until the link dropped, a hello flood grew the table without
bound, and a legitimate re-hello of the same id sat behind a rejection.
`unbindMcp` is scoped to the holder, so giving a binding back cannot
become a way to take somebody else's.

**`clearAllSessionScopes` had no production caller.** Teardown became
per-link with remote bridges, so `clearSessionScopeFor` is the path; the
whole-table version survived only in its own test, under a doc comment
still claiming it ran on WS teardown. Removed. `SCOPE_TABLES`, derived
against the module's own exports, is what still makes a forgotten
thirteenth map fail a test.

**One connection state hid the failure worth seeing.** The badge goes
green when *any* link is open, so with a remote bridge up a dead
**loopback** link — the one every MCP on this machine needs — was
invisible. Bridge rows now carry a dot each, loopback included, fed by a
per-link status the background answers on the query the popup already
makes. No dot at all when the background did not answer, rather than a
state nobody vouched for.

Mutation-checked: dropping the `unbindMcp` call turns the new multi-link
test red.

Closes #234

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Aug 9, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.1.0](v2.0.0...v2.1.0)
(2026-08-09)


### Features

* **extension:** dial configured remote bridges alongside loopback
([#233](#233))
([cc81e8a](cc81e8a))
* **server:** fall back to FETCHPROXY_WS_PORT for the concentrator port
([#231](#231))
([c221262](c221262))


### Bug Fixes

* **extension:** give a refused hello its binding back, and show each
bridge's own state
([#236](#236))
([1f8da11](1f8da11)),
closes [#234](#234)
* **extension:** refuse download over a remote bridge
([#235](#235))
([306d1f5](306d1f5))


### Refactor

* **extension-core:** split background.ts into purpose-shaped modules
([#226](#226))
([db30f4d](db30f4d)),
closes [#10](#10)
* **extension-core:** stop exporting background helpers nothing imports
([#229](#229))
([42c39a1](42c39a1)),
closes [#227](#227)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant