fix(core): quarantine WireMeshTransport sessions until connection approval - #60
Merged
Mearman merged 2 commits intoSep 11, 2026
Merged
Conversation
…roval Every listener (the coordinator port, and any addListener-created listener, including a non-loopback one exposed via mesh_listen) promoted an accepted connection to a fully trusted, routed session the instant its TLS handshake completed. TLS only proves which key the far side holds, never that a human approved it as a mesh member -- the only gate that ever existed was intercepting connect_request specifically, but any other message type (state_update, a room send, a DM) reached mesh-store's own onMessage unfiltered, letting an unapproved peer forge arbitrary mesh state before ever requesting a connection. The previous TlsTransport substrate never had this gap: its own coordinator socket only ever parsed introduce/connect_request, dropping anything else, while a separate data connection (reachable only via a port learned through an already-approved introduction) carried full routing. Restore that boundary under the new substrate's unified accept path: a listener-accepted session is quarantined -- restricted to introduce (the pre-existing, ungated coordinator-handoff path, unchanged) or connect_request (now blocking in place on the human decision Promise acceptConnection/ rejectConnection resolve) -- and only promoted to full trust once approved. A session's own data-port dial (the second half of an already-approved mesh pairing) is unaffected, matching its pre-existing trust model exactly. Adds a test that forges a state_update from a connection which never sent connect_request at all, confirming it's refused rather than reaching mesh-store's state.
Every spawned child in this file require()s compiled dist/ output directly (exercising the actual built artifact across a real process boundary, not TS source), unlike every other *.test.ts file since the switch to running the suite straight from source via tsx. Left matching that general glob, it fails with a confusing MODULE_NOT_FOUND the moment dist/ isn't already present from an earlier, unrelated build. Renamed to mesh-smoke.runner.ts (matching delivery-receipt.runner.ts's own naming for the same reason) so plain pnpm test never picks it up, with its own pnpm test:smoke script that builds first. CI's test job runs it as an explicit step so this coverage isn't silently dropped from CI the same way federation/visibility coverage previously was.
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. |
Mearman
deleted the
fix/wire-mesh-transport-quarantine-unapproved-sessions
branch
September 11, 2026 19:14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while validating #59 (the remaining-bridges rollout) via Codex's own security review of that PR: any listener-accepted connection (the coordinator port, or any addListener-created listener, including a non-loopback one via mesh_listen) is promoted to a fully trusted, routed session the instant its TLS handshake completes, before any human ever approves it via connect_request. This is already live on main via #58 (the mcp bridge cutover), since it's a gap in WireMeshTransport itself (#57), not anything specific to #58 or #59 -- those PRs just widen how many bridges are exposed to it.
TLS only proves which key the far side holds, never that a human approved it as a mesh member. The old TlsTransport substrate's own coordinator socket only ever parsed
introduce/connect_request, dropping anything else; the new unified accept path lost that boundary and routes every message type straight to mesh-store's ownonMessage. An unapproved peer could forge astate_update(or a room send, a DM) into the mesh without ever sendingconnect_requestat all.Restores the boundary: a listener-accepted session is quarantined to
introduce(unchanged, still ungated -- the existing coordinator-handoff trust boundary) orconnect_request(now blocks in place on the human decision, resumed byacceptConnection/rejectConnection) until approved. A peer's own data-port dial, the second half of an already-approved pairing, is untouched. Added a test that forges astate_updatefrom a connection that never sentconnect_request, confirming it's refused rather than reaching mesh-store's state -- confirmed this test actually fails without the fix (reverted the gate locally, watched it hang/fail, restored it).Second, unrelated thing found while verifying this in a clean worktree (not the same stale one I'd been building in all session, which had a leftover
dist/masking it):mesh-smoke.integration.test.tsrequire()s compileddist/output directly in its spawned subprocesses, so it fails withMODULE_NOT_FOUNDunder the plain-sourcepnpm testfrom #58 unless a build already happened. Renamed tomesh-smoke.runner.tswith its ownpnpm test:smokescript (builds first) and a CI step for it, matchingdelivery-receipt.runner.ts's own precedent, so it isn't silently dropped from CI the same way federation/visibility briefly were.