Track the 5 open v1 security PRs (board #11 issues-only cleanup)

[cliffhall]

Summary

Five open security PRs target v1/main and were the last items still sitting on
project board #11 as PR cards. Board #11 is issues-only (see AGENTS.md →
Issue-driven Work Style), and the umbrella they each reference — #1819,
"go-live 7/9: triage and bulk-close the v1 PR backlog" — is closed, so
removing those cards would have left this work with no board tracking at all.

This issue is that tracking. The PR cards were removed from #11; each PR
below is reachable from here.

Decisions — 4 of 5 resolved

Each PR was put through repeated Copilot review rounds until clean, then given
an explicit call. The governing rule: v1/main is deprecated and ships fixes
for high-severity vulnerabilities only — not general hardening,
defense-in-depth, or hygiene work, however sound.

• Security: OAuth callback does not validate state parameter #1189 — closed (closed-v1-deprecated). Not a vulnerability. v1 already
  implements PKCE (saveCodeVerifier/codeVerifier + SDK 1.25.2 sending
  code_challenge), which defeats code injection; the unvalidated state is a
  half-built mechanism (state() generates and sends a token that is then
  discarded), so completing it is hardening. v2 reached the same conclusion
  independently and does a shape check only.
• Security: Potential reverse tabnabbing via window.open with _blank #1190 — closed (closed-v1-deprecated). Finding is real — the implicit
  noopener browsers apply to <a target="_blank"> does not extend to
  window.open() — but it is bounded by a hostile-server-plus-user-click
  precondition and yields tab navigation, not code execution or data access.
  v2 is unaffected; nothing to forward-port.
• fix(server): redact sensitive env vars and headers from connection logs #1296 — closed (closed-v1-deprecated). Legitimate finding, and the review
  rounds sharpened it from a name-pattern denylist into unconditional
  redaction. But the exposure is local — the user's own console — so it is
  hygiene, not a remotely exploitable vulnerability.
• fix: reject requests with missing Origin header in origin validation middleware (CWE-346) #1161 — closed (closed-v1-security-declined). Reviewed and declined on the
  merits: every origin-validated route also requires the session token, so
  missing-Origin is not an unauthenticated bypass, and rejecting it breaks
  non-browser callers (curl/CI) while putting v1 at odds with v2's documented
  posture.
• fix: prevent DNS-rebinding TOCTOU in safeProxyFetch by pinning resolved IPs #1732 — kept open. The one PR that plausibly clears the bar. A DNS-rebinding
  TOCTOU in safeProxyFetch is an SSRF-guard bypass reachable at the cloud
  metadata service (169.254.169.254) — the threat model being a hostile MCP
  app inducing /fetch, not a token-holding attacker. Closing it under a
  "high-severity only" rationale would contradict itself. Needs a merge call.

Notes carried out of the review

Found during review and not addressed, since none is v1-shippable:

• Same log-leak class as fix(server): redact sensitive env vars and headers from connection logs #1296 remains in the STDIO/SSE paths — command=/args=
  logged verbatim and the SSE url=, which can carry embedded credentials. Worth
  an issue against v2.
• client/src/components/OAuthFlowProgress.tsx:381 has the same bare window.open
  as Security: Potential reverse tabnabbing via window.open with _blank #1190 (its sibling at :251 is already hardened). v1 only.
• Fork PRs on this repo run no workflows at all (gh pr checks → "no checks
  reported"), so nothing on v1/main is verified by the repo's own gates. That
  materially raises the risk of any change here and is part of why the bar is high.
• Security: OAuth callback does not validate state parameter #1189's contributor commits carry a DCO name/email mismatch that would need a
  force-push rewrite to repair. Moot now that it's closed.

Contributors' branches keep the commits pushed during review; nothing was merged.

[cliffhall]

Dispositions recorded — 4 of 5 closed, #1732 kept open.

Each of the five went through repeated Copilot review rounds until a review came back clean, then got an explicit call against the governing rule (v1/main ships fixes for high-severity vulnerabilities only). Issue body updated with the reasoning per PR.

PR	Call	Label
#1161	closed — reviewed, declined on the merits	closed-v1-security-declined
#1189	closed — not a vulnerability (PKCE covers it); half-built mechanism	closed-v1-deprecated
#1190	closed — real but bounded; tab navigation only	closed-v1-deprecated
#1296	closed — local console exposure; hygiene, not remote	closed-v1-deprecated
#1732	open — plausibly clears the bar; needs a merge call	—

#1732 is the outlier: a DNS-rebinding TOCTOU in safeProxyFetch is an SSRF-guard bypass that can reach 169.254.169.254, with the threat model being a hostile MCP app inducing /fetch rather than a token-holding attacker. Closing it under a "high-severity only" rationale would have contradicted itself, so it stays open pending that decision. Note its review rounds also fixed a functionality-breaking bug in the PR (the pinned lookup hook ignored Node's {all: true} shape, so /fetch could only reach literal-IP targets) and stopped the test suite shipping inside the npm tarball.

Leaving this issue open until #1732 is decided.