fix(web): compare deep-link auth tokens in constant time

[cliffhall]

Closes #2429

What changed

parseDeepLink() (clients/web/src/utils/deepLink.ts) compared the deep link's autoConnect and autoOpen params against the session API token with plain !== / ===, which returns at the first mismatched character.

• Adds constantTimeEqual(candidate, secret) to the same module: a synchronous XOR accumulator that visits every UTF-16 code unit of the secret whatever the candidate holds, and folds a length mismatch into the result rather than returning early. Only the secret's length drives the loop.
• Both token checks in parseDeepLink() now go through it. autoOpen keeps its exact semantics (absent or wrong means false).

Judgment calls

• Why not a platform primitive? crypto.timingSafeEqual is Node-only (the backend already uses it for x-mcp-remote-auth). crypto.subtle is async, and it is missing on non-secure origins, so it can't serve this synchronous parse path without changing the API of parseDeepLink.
• Placement: the helper lives in deepLink.ts instead of a new module. It is pure (no I/O), so it belongs under src/utils/ per the web source layout rule. Keeping it beside its only caller keeps the diff to two files. If a second browser-side caller turns up, moving it into its own utils module is a one-line import change.
• Length is not hidden: the secret's length is a fixed property of the launch-time token format, not of its value. The function's doc comment says so.

Tests

deepLink.test.ts adds:

• prefix, extension and first-character-mismatch rejections for both autoConnect and autoOpen;
• direct constantTimeEqual cases: equality, a length mismatch in either direction, a NUL code unit versus a missing one past the candidate's end, and code-unit (not normalized) comparison.

Per-file coverage of deepLink.ts: 98.0 statements / 95.3 branches / 100 functions / 97.7 lines.

Gate

npm run local:gate was run in a sandboxed container. Every stage passed except two, and both failed for container reasons unrelated to this diff, which touches only the two web files above:

• coverage:web: isOnMountPoint("bad\0path") in secret-store-selection.test.ts fails because of the container's mount layout. Integration tests that reach loopback through the forced NODE_USE_ENV_PROXY sandbox proxy fail with a proxy 502, and pass once that variable is unset.
• coverage:cli: the relogin-revocation / clear-stored-auth-for-relogin tests fail with the sandbox's HTTPS_PROXY set and pass with it unset (21/21).

validate:*, verify:skills:cli, coverage:tui, coverage:launcher, verify:build-gate, verify:bundle-externals, smoke, smoke:web:firefox and Storybook all passed. CI will run the suites in a clean environment.

No UI change, so no screenshots.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The security rationale incorrectly claims token lengths are always fixed despite supported user-configured tokens.

Review effort: Balanced
Findings: 1

Open (1)

• Document timing leak for custom-length API tokens · New

What changed in this PR

Hardens web deep-link authentication against prefix timing leakage.

Changes:

• Adds a synchronous XOR-based token comparator.
• Applies it to autoConnect and autoOpen.
• Adds edge-case coverage for mismatches and Unicode.

File	Description
clients/​web/​src/​utils/​deepLink.ts	Adds and uses constant-time comparison.
clients/​web/​src/​utils/​deepLink.test.ts	Tests comparison and deep-link rejection behavior.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot round 1, 1 finding:

• deepLink.ts:99, doc comment claimed a fixed token length despite user-supplied tokens: fixed in 679c5ce (comment-only). Reworded to say a custom token's length stays observable, which is the accepted limit; the prefix leak is what is removed.

No suppressed comments. Requesting round 2.

[Copilot]

Copilot review overview

🟡 Changes recommended

Browser JavaScript cannot guarantee the new loop executes in constant time, leaving the security requirement unresolved.

Review effort: Balanced
Findings: 1

Open (1)

• Browser loop does not guarantee constant-time comparison · New

Resolved since last review (1)

• Document timing leak for custom-length API tokens

[cliffhall]

Declining as scope expansion beyond #2429, which asks for a constant-time compare and explicitly accepts "a timing-safe-equal utility". Its triage scored it Low hardening, since both operands are same-origin browser values. You are right that ECMAScript makes no constant-time promise. But a branch-free XOR accumulator with a trip count fixed by the secret is the standard userland shape for this (it is what libraries like tsscmp do), and it removes the early exit that === has. That early exit is the leak the issue describes.

The alternatives both reshape more than this issue covers:

• Async Web Crypto digests: parseDeepLink is a synchronous parse feeding render-time state in App.tsx, and crypto.subtle is missing on non-secure origins (a LAN-host http: launch).
• Moving the gate to the backend: a new API round trip on page load.

Either one deserves its own issue if a maintainer wants that stronger guarantee. The helper's doc comment already explains why no platform primitive fits this path.

[cliffhall]

Copilot round 2, 1 finding:

• deepLink.ts:112, "a browser JS loop is not guaranteed constant-time; use async Web Crypto or move the gate to the backend": declined as scope expansion beyond Deep-link auth-token comparison in deepLink.ts isn't constant-time #2429. The issue accepts a timing-safe-equal utility, and both alternatives reshape the synchronous parse path or add a backend round trip. Reasoning is in the thread.

Round 1's finding shows as resolved. No suppressed comments.

Review loop closed: round 2 held only an out-of-scope finding, so nothing changed and another round would only re-argue scope (pr-flow 7c).