fix(cdp): harden Comet launch and reject non-http(s) navigation

[sergei-aronsen]

Summary

Two defense-in-depth changes around the CDP transport.

`--remote-allow-origins`

Chromium's CDP endpoint has no auth. Historically any local process or any browser tab that resolves a hostname pointing at `127.0.0.1` could attach to it (DNS-rebinding-against-CDP, CVE-2018-6056 and follow-ons). The standard mitigation, shipped since m90, is `--remote-allow-origins=`: connections from origins not in the list are rejected at the handshake.

We now always launch Comet with `--remote-allow-origins=http://127.0.0.1\` (centralised in `cometLaunchArgs()`), so a drive-by website served to any browser on the host can no longer pivot through `ws://127.0.0.1:9223` to read Comet tabs.

Applied at all three native spawn sites and the PowerShell WSL launch. The WSL path now builds `-ArgumentList` from the same array via array-quote-escape join so the values cannot shell-inject through the PowerShell layer.

Protocol allowlist on `Page.navigate`

`navigate()` and `navigateWithRetry()` reject any URL whose protocol is not `http:` or `https:` via a new `assertNavigableUrl()` helper. Specifically blocks `javascript:`, `data:`, `vbscript:`, `file://`, `chrome:`, `devtools:`, `view-source:`, `about:`, and empty / unparseable strings.

Defense-in-depth against indirect prompt injection that targets the navigation primitives.

Compatibility

`http(s)` URLs work unchanged. Comet launches transparently with the new flag. Existing CDP clients connecting from `127.0.0.1` (this MCP server, the Chrome DevTools UI) are unaffected.

Test plan

• `npm run test:unit` passes
• Manual: `comet_connect` launches Comet on macOS / Windows / WSL
• Manual: navigation to https://example.com still works
• Adversarial: `Page.navigate` to `javascript:alert(1)` rejected before reaching CDP
• Adversarial: another browser tab cannot connect to `ws://localhost:9223` (Chromium handshake refuses)

Related

Part of an audit pass. Other open PRs: security (#11), correctness (#12), drop unused dep (#13), upload allowlist (#14).

🤖 Generated with Claude Code

[RapierCraft]

Code Review: fix(cdp): harden Comet launch and reject non-http(s) navigation

Reviewed commit: da00094

Overall Assessment: APPROVE

This is a well-structured defense-in-depth PR addressing two real attack surfaces: DNS rebinding against the unauthenticated CDP endpoint, and indirect prompt injection targeting navigation primitives. The implementation is correct and the scope is appropriately limited.

---

URL Validation (assertNavigableUrl)

Verdict: Correct and sound.

• Uses the URL constructor for parsing, which correctly handles edge cases like javascript:alert(1) (parses to protocol javascript:), JAVASCRIPT: (case-normalized by URL constructor to javascript:), and malformed strings.
• Allowlist approach (http: / https: only) is the right pattern — blocklisting dangerous protocols is fragile against future additions.
• Both navigate() and navigateWithRetry() are guarded. The only Page.navigate CDP calls in the file are at lines 1101 (inside navigate()) and 1126 (inside navigateWithRetry()), both reachable only after validation passes.
• The call at line 661 (await this.navigate(url, true)) routes through the validated navigate() method. No bypass path exists.

Minor observation (non-blocking): navigateWithRetry returns attempts: 0 on validation failure, which is a sensible sentinel value. Consider documenting this in the JSDoc for callers who check attempts >= 1 to distinguish "tried and failed" from "refused to try."

---

Launch Hardening (cometLaunchArgs + --remote-allow-origins)

Verdict: Correct.

• --remote-allow-origins=http://127.0.0.1 is the correct Chromium flag (available since M90+). It restricts WebSocket upgrade handshakes on the CDP port to only connections whose Origin header matches. This mitigates DNS rebinding attacks where a malicious page resolves to 127.0.0.1 but presents a different Origin.
• The function is applied at all three spawn(COMET_PATH, ...) call sites (lines 848, 882, 931) and the PowerShell WSL path (line 794). No spawn site was missed.
• The centralized cometLaunchArgs() helper is a good pattern — future flags only need to be added in one place.

---

PowerShell ArgumentList Escaping (WSL path)

Verdict: Correct with one minor concern.

The escaping logic:

const launchArgs = cometLaunchArgs(port)
  .map(a => `'${a.replace(/'/g, "''")}'`)
  .join(',');

This correctly handles PowerShell single-quote escaping (doubling single-quote to double-single-quote). Since the args come from cometLaunchArgs() which produces only --remote-debugging-port=NNNN and --remote-allow-origins=http://127.0.0.1, neither contains single quotes, so the escape is defensive-correct but not exercised today.

Minor concern (non-blocking): The cometPath variable (line 780/782) is still interpolated directly into the PowerShell command string without the same escaping treatment. If a Windows username contained a single quote (e.g., O'Brien), the Start-Process -FilePath would break. This is a pre-existing issue not introduced by this PR, but worth noting for a follow-up.

---

Security Considerations

1. No information leakage in error messages: The error messages include the protocol or "Invalid URL" — no sensitive data echoed back. Good.
2. No TOCTOU risk: assertNavigableUrl is called synchronously before Page.navigate in the same method body. No window for the URL to change between validation and use.
3. port parameter trust: port flows from this.state.port (default 9223) or the connect() argument. It is a number type — no injection risk into the arg string.

---

Suggestions (non-blocking, for consideration)

1. Unit tests: The PR description mentions npm run test:unit — adding explicit test cases for assertNavigableUrl with javascript:, data:, file://, empty string, and valid https:// would lock in the behavior and prevent regressions.

2. Consider --remote-allow-origins port matching: The current flag is http://127.0.0.1 (no port). Chromium matches on origin (scheme+host+port). Without a port, this allows connections from any http://127.0.0.1:* origin. If you want to restrict to only the DevTools port, use http://127.0.0.1:${port}. However, the current value is defensible — it blocks external origins while allowing local DevTools UI on any port.

3. Logging on rejection: For debugging MCP tool calls that hit the validation, consider emitting a debug/warn log when assertNavigableUrl throws, so operators can see which upstream input triggered the rejection.

---

Summary

Area	Status
URL validation correctness	Pass
All navigation paths guarded	Pass
Launch args applied at all spawn sites	Pass
PowerShell escaping	Pass
No new security issues introduced	Pass
No regressions to existing functionality	Pass

Recommendation: Safe to merge. The implementation is correct, well-documented, and addresses real security concerns with appropriate defense-in-depth measures.

[RapierCraft]

This PR has merge conflicts with main after recent merges (PRs #11, #12, #16, #18 landed).

The conflicts should be straightforward — the launch hardening and URL validation logic doesn't overlap semantically with what merged, just positional conflicts in shared files. Please rebase against main.