Repository navigation
go-live 7/9: triage and bulk-close the v1 PR backlog #1819
Description
Activity
- changed the title
[-]go-live 5/9: publish 2.0.0 (rc under next first; rollback plan written first)[/-][+]go-live 6/9: triage and bulk-close the v1 backlog[/+]on Jul 27, 2026 - changed the title
[-]go-live 6/9: triage and bulk-close the v1 backlog[/-][+]go-live 7/9: triage and bulk-close the v1 backlog[/+]on Jul 28, 2026 v1 PR backlog triage — security review before bulk close (#1819)
Scope: all 125 open PRs targeting
v1/main, evaluated by 7 parallel agents reading each diff.
Question asked: does any PR contain an important security fix worth one more v1.x patch release (1.0.2) before bulk-closing the rest as deprecated?Answer: 3 candidates. Everything else closes.
Baseline:
1.0.1 — v1 deprecatedshipped 2026-07-28. Note the repo also has 13 unresolved advisories intriagestate (2 critical RCE, 4 high) — none of which these PRs fix. Cutting a1.0.2for the items below does not clear those; see §4.
1. Merge candidates — worth cherry-picking into v1.0.2
🔴 #1732 — pin resolved IPs to kill DNS-rebinding TOCTOU in
safeProxyFetchVerdict: strongest candidate. Medium-high.
manjunathbhaskar·MERGEABLE/BLOCKED (review only, no conflict) · +455/−85 in 5 files · has tests (265-line vitest suite, first in theserverpackage)- Class: SSRF via DNS rebinding / TOCTOU in the
/fetchproxy. - Why it's real:
assertSafeProxyTargetresolves the hostname and checks it against the link-local/RFC1918 block-list — thennode-fetchresolves independently and connects to whatever the second lookup returns. The block-list exists specifically as the SSRF control, and the double resolution makes it ineffective. The fix pins the validated addresses via a customlookupagent, per redirect hop. - Attack: developer points the Inspector at
evil.example.com(short TTL) → first resolution passes the block-list → second resolves to169.254.169.254→ cloud IMDS credentials come back through the proxy response. - Severity call: medium-high. Discounted from high because it needs the dev to connect to an attacker-chosen URL and
/fetchsits behind the session token; elevated above medium for cloud/CI-hosted Inspector deployments where IMDS creds are the payload. - Substantive change is only ~20 lines — ~85 of the deletions are a pure move of the existing helpers into a new
server/proxy-security.ts. - Review nits (neither is a security problem): pins
validatedAddresses[0]unconditionally, so a multi-homed host whose first record is unreachable now fails rather than falling through; carries oneagent as anywith an eslint-disable (node-fetch'sagentvs. browserRequestInit). Also addsvitest+ atestscript toserver/package.jsonbut does not wire it into CI — the suite runs only on demand. - This one directly relates to the open SSRF advisory cluster (GHSA-x79p, GHSA-55hw, GHSA-2vhc, GHSA-797v, GHSA-r326 — all
/fetchSSRF).
🟠 #1161 — reject requests with missing
Originheader (CWE-346)Verdict: sound one-line fix, but verify the blast radius first.
sebastiondev·MERGEABLE/BLOCKED · +1/−1 in 1 file · no tests (author's claim of tests is false — it points at aserver/build/artifact path)- Class: fail-open origin validation. An absent
Originheader skips validation entirely on/stdio,/mcp,/sse,/message,/config. - Attack: victim runs with
DANGEROUSLY_OMIT_AUTH=true, visitsevil.com, attacker rebinds DNS to127.0.0.1and issues a request browsers send without anOrigin→ middleware short-circuits →/stdio?command=…spawns an arbitrary local process. - Severity: high with auth disabled, medium by default (the session token still blocks the rebound page, which can't read it). Correct fail-closed posture.
⚠️ Before merging: this now 403s every non-browser client that omitsOrigin— curl, scripts, CI hitting/configor/mcp. Confirm the v1 CLI sends an allowedOrigin, or scope the fail-closed branch to the process-spawning routes. The agent's confidence in the PR description was low; the code change itself is sound.
🟡 #1296 — redact sensitive env vars and headers from connection logs
Verdict: clean, tested, low risk to take.
SarthakB11·MERGEABLE/BLOCKED · +127/−3 in 4 files · has tests (6 node:test cases, newserver/redact.ts)- Class: CWE-532, sensitive info in logs. The proxy prints MCP server
env(GITHUB_TOKEN,AWS_SECRET_ACCESS_KEY, …) andAuthorizationheaders verbatim to stdout → terminal scrollback, CI logs, screen recordings. - Severity: low (local disclosure, requires the log to be shared) — but small, self-contained, and tested.
- Maps to two open advisories: GHSA-8qgq-27v6-55w3 (SSE auth headers logged verbatim, medium) and partially GHSA-2m3v-mfc6-w38f (env exposure, high). Merging this lets you resolve at least one advisory in triage, which is the strongest argument for cutting 1.0.2 at all.
2.
⚠️ Do NOT merge — PRs that weaken security (close with an explicit note)These matter more than the average close: each is framed as an improvement, and a silent bulk-close risks one being resurrected into v2 later.
PR What it actually does #1050 Buried in a 10k-line "UI responsiveness and proxy stability" PR: changes proxy auth to off-by-default on localhost ( isLocal && REQUIRE_AUTH !== "true"), leaving origin validation as the only control. Exactly the layered defense the threat model depends on. Flag loudly.#898 Sets ENV HOST=0.0.0.0in the Dockerfile — binds the process-spawning proxy to all interfaces by default.#1278 Widens the MCP Apps sandbox referrer allowlist to .local, RFC1918,0.0.0.0, and https — attacker-reachable on a shared LAN. Also adds a UTF-8 BOM.#1121 Stops wiping client_id/client_secretfrom sessionStorage on clear/disconnect. Framed as a fix; it's a cleanup regression.#1154 Drops the RFC 8707 resourceparam (Azure Entra workaround) — removes audience restriction the MCP spec requires.#1580 Adds a credentials: 'include'toggle → browser cookies sent to arbitrary direct targets. Default-off, but adds risk.#1409 Client-supplied ?socks5=steers the backend's upstream connections.#585 Reads redirectUrlfrom a server-influenced/configresponse and spawns extra callback listeners. Lockfile adds the package as a devDependency of itself — red flag.3. Security-adjacent but close anyway (29)
Touch OAuth/CORS/proxy/sandbox/deps, but are compat or functionality fixes, not vulnerability fixes. No action needed; listed so the close note can't be argued with.
63264289291910741078110511101122118811891190123213081327134214291433143415071618163816951696Two are borderline if you want a bigger 1.0.2:
- server: restrict default CORS to allowed origins #1074 (restrict default CORS to the allowlist) — real defense-in-depth, but every sensitive route already chains origin+auth middleware, so no live bypass.
CONFLICTING, no tests. Only worth it with a rebase + test. - Security: Potential reverse tabnabbing via
window.openwith_blank#1190 (noopener,noreferreronwindow.open) — the cheapest possible hardening line (+1/−1). Modern browsers already implynoopenerfor_blank, so value is defense-in-depth only. - Security: OAuth callback does not validate
stateparameter #1189 (OAuthstatevalidation) — the gap is real (v1 never sends or validatesstate), but the patch is misplaced: it generates state in the MCP-Apps link handler, not the SDK redirect path, while hard-rejecting every callback without a stored state — i.e. it would break all normal OAuth logins. Reads AI-generated. If you want this fixed, it needs a fresh patch inredirectToAuthorization.
4. The thing this triage does not solve
13 advisories sit unresolved in
triage, including two critical RCEs (GHSA-4fc4 OAuth-callback stdio transport switch; GHSA-6x67 MCP App steals proxy token) and four highs. No open PR fixes any of them. If v1 is getting one final security release, the honest scope question is whether it should address those rather than the three PRs above — or whether v1 is deprecated firmly enough that the answer is "upgrade to v2." That's your call, not something the backlog decides.
Recommended sequence
- Decide: cut
1.0.2or not. The case for it is fix: prevent DNS-rebinding TOCTOU in safeProxyFetch by pinning resolved IPs #1732 + fix(server): redact sensitive env vars and headers from connection logs #1296 (closes ≥1 advisory); the case against is that it doesn't touch the 2 critical RCEs, and a patch release signals more support than intended. - If yes → cherry-pick fix: prevent DNS-rebinding TOCTOU in safeProxyFetch by pinning resolved IPs #1732, fix(server): redact sensitive env vars and headers from connection logs #1296, and fix: reject requests with missing Origin header in origin validation middleware (CWE-346) #1161 (after the non-browser-client check), wire fix: prevent DNS-rebinding TOCTOU in safeProxyFetch by pinning resolved IPs #1732's vitest suite into CI, release, then resolve GHSA-8qgq and the
/fetchSSRF advisory cluster. - Bulk-close the remaining 122 with the deprecation note +
closed-v1-deprecatedlabel, throttled. - For the §2 list, use a modified close comment noting the change was reviewed and deliberately declined on security grounds — so it's queryable and doesn't get re-proposed against v2.
- Do not lock threads (per go-live 7/9: triage and bulk-close the v1 PR backlog #1819).
- Class: SSRF via DNS rebinding / TOCTOU in the
- changed the title
[-]go-live 7/9: triage and bulk-close the v1 backlog[/-][+]go-live 7/9: triage and bulk-close the v1 PR backlog[/+]on Jul 31, 2026
Phase 7 of 9 in the v2 go-live runbook — see #1804 (§8). Depends on phase 6.
Close the open v1 backlog with an audit trail.
Tasks
v2" — some open v1 issues describe real bugs that still exist in v2, or security items we actually want. Split into port to v2 (relabelv2, add to board Add tab and approval flow for server -> client sampling #28) vs. close as deprecated.closed-v1-deprecated) as well as the comment, so the set stays queryable later. A comment alone is very hard to find again.