Skip to content

Commit 7d8436f

Browse files
committed
fix(gateway): refuse to promote unproven meta-tool pins to a window
Cursor's global bridge shares one mcp-remote process, and therefore one mcp-session-id, across every open window. A set_workspace_root call on that shared session names no window, so promoting it to window_pins handed every other window sharing the process whichever folder happened to be claimed first — confirmed live: a claim made in one window's chat landed on another window's session id and widened its FeatureSet. A header or single-candidate pin is self-attesting proof of single-window intent and still promotes unconditionally. A meta-tool pin now promotes only when the session's own candidate set independently narrows to one folder; otherwise it stays session-scoped and a warn names the skip. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent 5997a11 commit 7d8436f

2 files changed

Lines changed: 131 additions & 3 deletions

File tree

crates/mcpmux-gateway/src/services/session_roots.rs

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,17 @@ pub struct SessionRootsRegistry {
8282
/// resolver, the on-demand probe skip, and the prompt-root derivation all
8383
/// honor the header with no special-casing. Already normalized on insert.
8484
pinned: DashMap<String, String>,
85+
/// `session_id -> which claim produced the current [`Self::pinned`] entry.
86+
///
87+
/// Read by [`Self::promote_pin_to_window`] to decide whether the pin is
88+
/// safe to make durable: a header or single-candidate pin is itself proof
89+
/// the calling process serves exactly one window, but a `set_workspace_root`
90+
/// call carries no such proof — the global Cursor bridge shares one
91+
/// `mcp-remote` process (and one `mcp-session-id`) across every open
92+
/// window, so a meta-tool pin issued into that shared session with no
93+
/// corroborating single-folder candidate set could be *any* window's
94+
/// agent, not the one whose process key we'd be writing.
95+
pinned_source: DashMap<String, PinSource>,
8596
/// `client_id -> workspace path` held when `X-Mcpmux-Workspace` arrived
8697
/// before `mcp-session-id` (initialize). Applied on the first request
8798
/// that has a session id, or when initialize's response issues one.
@@ -186,6 +197,7 @@ impl SessionRootsRegistry {
186197
probe_lock: DashMap::new(),
187198
first_seen: DashMap::new(),
188199
pinned: DashMap::new(),
200+
pinned_source: DashMap::new(),
189201
pending_by_client: DashMap::new(),
190202
candidates: DashMap::new(),
191203
pending_candidates_by_client: DashMap::new(),
@@ -348,6 +360,7 @@ impl SessionRootsRegistry {
348360
self.last_resolution.remove(session_id);
349361
self.search_cache.remove(session_id);
350362
self.pinned.insert(session_id.to_string(), normalized);
363+
self.pinned_source.insert(session_id.to_string(), source);
351364
self.promote_pin_to_window(session_id);
352365
}
353366

@@ -411,12 +424,25 @@ impl SessionRootsRegistry {
411424
self.session_window.get(session_id).map(|key| *key)
412425
}
413426

414-
/// Copy this session's explicit pin onto its window, if both exist.
427+
/// Copy this session's explicit pin onto its window, if both exist and
428+
/// the pin is attributable to that one window.
415429
///
416430
/// No-op when the session has no pin or no window — initialize often
417431
/// pins before the peer socket has been mapped, and the next request
418432
/// retries.
419433
///
434+
/// A header or single-candidate pin is itself proof the calling process
435+
/// serves exactly one window, so those promote unconditionally. A
436+
/// `set_workspace_root` pin carries no such proof: Cursor's global bridge
437+
/// shares one `mcp-remote` process, and therefore one `mcp-session-id`,
438+
/// across every open window (confirmed in the field — see
439+
/// `docs/planning/window-scoped-workspace-pin.md` "Field evidence, second
440+
/// incident"). Writing that claim to `window_pins` would durably hand
441+
/// every future session on that shared process whichever window happened
442+
/// to call the tool first. It only promotes when the session's own
443+
/// candidate set independently narrows to that single folder — i.e. the
444+
/// window isn't sharing right now, whatever it might do later.
445+
///
420446
/// Called from the request hot path (every request carrying a session pin
421447
/// re-promotes), so an unchanged value skips both the write and the log.
422448
/// Without that guard the durability this feature exists to provide would
@@ -428,6 +454,27 @@ impl SessionRootsRegistry {
428454
let Some(root) = self.pinned.get(session_id).map(|value| value.clone()) else {
429455
return;
430456
};
457+
let source = self
458+
.pinned_source
459+
.get(session_id)
460+
.map(|value| *value)
461+
.unwrap_or(PinSource::MetaTool);
462+
if source == PinSource::MetaTool {
463+
let candidates = self.get_candidates(session_id);
464+
let single_folder_proven = matches!(candidates.as_deref(), Some([_]));
465+
if !single_folder_proven {
466+
warn!(
467+
%session_id,
468+
window_key = %key,
469+
workspace_root = %root,
470+
candidates = ?candidates,
471+
"[SessionRoots] window pin skipped — meta-tool claim without proof this \
472+
mcp-remote process serves a single window; this session keeps the pin, \
473+
but it will not survive a Reload MCP or session churn",
474+
);
475+
return;
476+
}
477+
}
431478
// Owned bool so the read guard drops before the insert below — see
432479
// `record_resolution` for the self-deadlock this avoids.
433480
let unchanged = self
@@ -1057,6 +1104,10 @@ mod tests {
10571104
let key = WindowKey::from_pid(std::process::id());
10581105
reg.attach_window("sess-1", key);
10591106
reg.set_pinned("sess-1", CANDIDATES[0], PinSource::WorkspaceHeader);
1107+
// A MetaTool pin only promotes with single-folder proof — see
1108+
// `meta_tool_pin_promotes_only_with_single_folder_proof` for the
1109+
// no-proof case this test deliberately avoids.
1110+
reg.set_candidates("sess-1", CANDIDATES[1]);
10601111
reg.set_pinned("sess-1", CANDIDATES[1], PinSource::MetaTool);
10611112

10621113
reg.attach_window("sess-2", key);
@@ -1067,6 +1118,61 @@ mod tests {
10671118
);
10681119
}
10691120

1121+
/// Regression test for the field-confirmed cross-window leak (see
1122+
/// `docs/planning/window-scoped-workspace-pin.md`, "second incident"):
1123+
/// Cursor's global bridge shares one `mcp-remote` process, and therefore
1124+
/// one `mcp-session-id`, across every open window. A `set_workspace_root`
1125+
/// call on that shared session names no window, so it must not become
1126+
/// the answer every other window inherits after this session ends.
1127+
#[test]
1128+
fn meta_tool_pin_promotes_only_with_single_folder_proof() {
1129+
let reg = SessionRootsRegistry::default();
1130+
let key = WindowKey::from_pid(std::process::id());
1131+
1132+
// No candidate set at all (the leak's actual trigger: an unexpanded
1133+
// X-Mcpmux-Workspace-Set never got stored) — must not promote.
1134+
reg.attach_window("sess-shared-a", key);
1135+
reg.set_pinned("sess-shared-a", CANDIDATES[0], PinSource::MetaTool);
1136+
assert_eq!(
1137+
reg.get("sess-shared-a"),
1138+
Some(vec![CANDIDATES[0].to_string()]),
1139+
"the calling session still gets its own answer"
1140+
);
1141+
1142+
reg.attach_window("sess-shared-b", key);
1143+
assert!(
1144+
reg.inherit_window_pin("sess-shared-b").is_none(),
1145+
"an unproven meta-tool claim must not leak to a sibling session \
1146+
on the same shared process"
1147+
);
1148+
1149+
// A multi-folder candidate set is equally insufficient — it proves
1150+
// the window is one of several, not that it is exactly one.
1151+
reg.attach_window("sess-shared-c", key);
1152+
reg.set_candidates("sess-shared-c", &CANDIDATES.join(","));
1153+
reg.set_pinned("sess-shared-c", CANDIDATES[0], PinSource::MetaTool);
1154+
reg.attach_window("sess-shared-d", key);
1155+
assert!(
1156+
reg.inherit_window_pin("sess-shared-d").is_none(),
1157+
"a multi-folder candidate set is not proof of single-window intent"
1158+
);
1159+
1160+
// A one-member candidate set IS proof — the window really has only
1161+
// one folder open right now. `set_candidates` self-pins in that case
1162+
// (decision 4b's SingleCandidate path), so the MetaTool call below is
1163+
// redundant in practice; it's here to confirm a MetaTool claim on an
1164+
// already-proven session is at worst a no-op, never a regression.
1165+
reg.attach_window("sess-solo-a", key);
1166+
reg.set_candidates("sess-solo-a", CANDIDATES[0]);
1167+
reg.set_pinned("sess-solo-a", CANDIDATES[0], PinSource::MetaTool);
1168+
reg.attach_window("sess-solo-b", key);
1169+
assert_eq!(
1170+
reg.inherit_window_pin("sess-solo-b"),
1171+
Some(CANDIDATES[0].to_string()),
1172+
"single-folder proof must still let the window pin work"
1173+
);
1174+
}
1175+
10701176
#[test]
10711177
fn explicit_session_pin_beats_window_pin() {
10721178
let reg = SessionRootsRegistry::default();

docs/planning/window-scoped-workspace-pin.md

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Window-Scoped Workspace Pin
22

33
**Last Updated:** Aug 20, 2026
4-
**Status:** Complete (Phases 1–4). Chosen from a six-option brainstorm as the highest value-per-line fix for the empty-`${workspaceFolder}` residual. Phase 1 load-bearing check passed in-process: unprivileged `netstat2` socket→PID lookup resolves a loopback peer to this process.
4+
**Status:** Phases 1–4 shipped, then patched same night after a field-confirmed cross-window leak on the shared global bridge process (decision 4b). Chosen from a six-option brainstorm as the highest value-per-line fix for the empty-`${workspaceFolder}` residual. Phase 1 load-bearing check passed in-process: unprivileged `netstat2` socket→PID lookup resolves a loopback peer to this process — see "second incident" below for why that check didn't catch the leak.
55
**Branch:** `root-resolution`
66
**Depends on:** [`cursor-workspace-routing-bridge.md`](./cursor-workspace-routing-bridge.md) Phases 1–3 (shipped) — this reuses the bridge's `X-Mcpmux-Workspace` / `X-Mcpmux-Workspace-Set` headers and the `set_workspace_root` escape hatch rather than replacing any of them
77
**Unblocks:** One workspace answer per Cursor window instead of one per MCP session — the residual cost that [`resilience-routing-leftovers.md`](./resilience-routing-leftovers.md) item 1 calls inherent
@@ -55,6 +55,26 @@ The daemon was never the problem during this window. `127.0.0.1:45818/health` re
5555

5656
Two things follow. First, the substitution flake is not scoped to `${workspaceFolder}` — it hits `${MCPMUX_API_KEY}` and `${WORKSPACE_FOLDER_PATHS}` through the same `mcp-remote` `${ENV}` pass, so any design that assumes "at least one header survives" is unsafe. Second, a mass respawn is precisely when session-keyed state is most expensive: every window loses its pin simultaneously, and every window has to be re-answered. A window-scoped pin survives the respawn if the process survives, and where the process doesn't survive, it at least collapses N sessions of re-asking into one.
5757

58+
### Field evidence, second incident (Aug 20, 11:09 PM — cross-window leak)
59+
60+
Phases 1–4 shipped and passed every unit/integration test, and still produced a real credential-scope bug the first night it ran against a live multi-window setup. Worth recording exactly, because the fix (decision 3, revised below) came directly from this trace and no test caught it first.
61+
62+
Setup: the global bridge entry (the one built from `buildCursorBridgeMcpJson`, `${workspaceFolder}` in argv) was open in more than one Cursor window at once. Cursor spawns **one `mcp-remote` process for that entry**, not one per window — confirmed by proving a claim made in window B's chat landed on window A's session id:
63+
64+
```
65+
window B calls mcpmux_set_workspace_root(/repo/generAIt)
66+
→ {"session_id": "8e8b54e8-...", ...} ← the id already serving window A's chat
67+
68+
window A, which never called the tool, immediately shows generAIt's
69+
FeatureSets as ready (Jira-GAIT, Netlify-GAIT, langfuse-GAIT, ...)
70+
```
71+
72+
`mcp-session-id` is therefore not a window identity on this transport — it's shared across every window that has the global entry open. The original design (decision 3, first cut) keyed `window_pins` on the owning PID, which is exactly as shared: writing a window pin for this process handed every window sharing it the same folder, and made that leak survive session churn instead of ending at session death. The window-scoping feature made an existing session-level leak *durable* — a regression on the dimension this doc set out to fix, introduced by the same change that fixed the dimension it was built for.
73+
74+
The false negative in Phase 1's own load-bearing check is worth naming: "one PID covering the same window's sessions across a Reload MCP" is also exactly what "one PID covering several windows' sessions" looks like from the log. The outcome that was supposed to falsify the design (decision 1's contingency) can't distinguish the good case from the bad one — only cross-window correlation (as above) can.
75+
76+
What the trace also shows: the two `PinSource` values are not equally trustworthy for this purpose. `WorkspaceHeader` and `SingleCandidate` are each a Cursor-side claim about *the calling request's own window* — a non-empty, non-template header value can only exist because some window's `${workspaceFolder}` resolved, and a one-member `X-Mcpmux-Workspace-Set` can only exist because exactly one folder is open right now. Both are self-attesting. `MetaTool` is not: an agent calling `mcpmux_set_workspace_root` asserts nothing about which window issued the call, and on a shared session there is no way to tell.
77+
5878
---
5979

6080
## Decisions
@@ -65,6 +85,7 @@ Two things follow. First, the substitution flake is not scoped to `${workspaceFo
6585
| 2 | How the PID lookup happens | [`netstat2`](https://crates.io/crates/netstat2) crate (one new dep, cross-platform socket→PID) | The alternatives are worse: shelling out to `lsof` spawns a child per new session (and the repo's child-process rules exist for good reason — `configure_child_process_platform()`), and hand-rolling libproc/`/proc`/`GetExtendedTcpTable` means three platform implementations with the CI blind spot documented in `AGENTS.md`. Phase 1 verifies it works unprivileged for own-user sockets before anything is built on it. |
6686
| 3 | Window key shape | `pid` plus a liveness re-check on read, **not** `pid` + process start time | Avoids a second dependency for start-time lookup. A stale entry requires the process to die *and* its PID to be reused *by another `mcp-remote` connected to this gateway*. `ponytail:` ceiling — narrow but not impossible; the upgrade path is adding start time from the same crate family if a misroute is ever observed. Mitigated by decision 5. |
6787
| 4 | What becomes durable | Only **explicit claims** — a substituted `X-Mcpmux-Workspace` header, or a `set_workspace_root` call | Probed `roots/list` values are already suspect (`listChanged: false`, stale across windows) and deductions are not proof. Promoting either to window scope would give a wrong answer a longer life, which is strictly worse than asking again. |
88+
| 4b | Which explicit claims may *promote* (revised after the second incident) | A header or single-candidate pin promotes unconditionally — each is self-attesting proof of single-window intent. A `set_workspace_root` pin promotes only if the session's own candidate set independently narrows to exactly one folder; otherwise it stays session-scoped and a warn names the skip. | Decision 4 assumed all explicit claims carry equal proof of *which* window made them. The second incident shows a meta-tool call on a shared session carries none — nothing about the call distinguishes window A's agent from window B's. Gating on the candidate set reuses machinery decision 5 already requires, so this is a narrower condition on an existing write path, not new state. |
6889
| 5 | Applying a window pin | Re-validate against the session's own candidate set when the set is present; skip validation when the set is absent or unexpanded | Keeps the invariant `set_workspace_root` already enforces — a session can only claim a folder its window actually has open. When the set header didn't survive (see Field evidence), there is nothing to validate against, and refusing would reintroduce the very friction this doc removes. |
6990
| 6 | Precedence | Explicit header for *this* session > window pin > probed roots > `PendingRoots` | A live explicit claim must always beat remembered state, so a genuine window switch is never overridden by a stale pin. This is a new tier inserted below `pinned`, not a change to any existing tier's behavior. |
7091
| 7 | Transport scope | Loopback peers only; remote/tunnel clients get no window pin | A tunnelled client has no local PID to resolve, so there's nothing to key on. Correct outcome, not a gap — those clients keep today's per-session behavior. |
@@ -191,7 +212,8 @@ Closes the ways a remembered pin could outlive its truth.
191212
- Evict `window_pins` when the owning process no longer holds a connection to the gateway; drop `session_window` in `remove()`
192213
- Assert precedence: a live explicit header always overrides an inherited pin (a genuine window switch must win)
193214
- Refuse inheritance when the session's candidate set is present and the remembered root isn't in it, with a warn naming both
194-
- Tests: precedence ordering, eviction on process exit, set-mismatch refusal, and inheritance skipped for non-loopback peers
215+
- **(added after the second incident)** Refuse to *write* a `set_workspace_root` pin to `window_pins` unless the session's candidate set independently narrows to one folder — decision 4b, implemented in `promote_pin_to_window()` via `pinned_source`
216+
- Tests: precedence ordering, eviction on process exit, set-mismatch refusal, inheritance skipped for non-loopback peers, and a meta-tool pin on a multi-candidate (or candidate-less) session never reaching `window_pins` while still applying to its own session
195217

196218
**Outcome:** Closing a Cursor window drops its pin (a later window reusing that PID inherits nothing), and a window that switches folders re-pins immediately instead of serving the previous answer. `pnpm test:rust` covers all four cases.
197219

0 commit comments

Comments
 (0)