fix: allow custom control UI origins#73511
Conversation
Greptile SummaryThis PR fixes allowlist matching for custom-scheme browser origins (e.g. Confidence Score: 5/5Safe to merge — targeted fix with correct logic and no security regressions. The change is minimal and well-reasoned: it correctly handles the WHATWG URL No files require special attention. Reviews (1): Last reviewed commit: "fix: allow custom control UI origins" | Re-trigger Greptile |
|
Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 7:27 AM ET / 11:27 UTC. Summary PR surface: Source +10, Tests +77. Total +87 across 3 files. Reproducibility: yes. source-reproducible: current main compares the allowlist against Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the shared parser fix after redacted real behavior proof shows an exact custom-scheme allowlist succeeds and a mismatched custom-scheme origin is rejected; keep wildcard scheme policy in the separate product/security discussion. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main compares the allowlist against Is this the best way to solve the issue? Yes for the code shape: fixing the shared parser with an exact AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2806195e1127. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +10, Tests +77. Total +87 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
36d0bdb to
5d19030
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
5d19030 to
084906a
Compare
|
Updated this PR on top of current What changed:
Gateway behaviour proof added in this branch:
Additional parser regression coverage:
Local verification:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
084906a to
b8da2e3
Compare
|
Current head is now Updates since the previous review request:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Current state on
The branch already includes a real Gateway WebSocket handshake test for the changed runtime boundary:
I also tried to produce a standalone local Gateway proof from this exact worktree, but this machine cannot currently hydrate the missing Gateway startup dependency: npm registry tarball requests time out, and direct If source-level proof plus the green Gateway handshake CI is sufficient for this narrow parser-layer auth-boundary fix, I believe this is ready for maintainer review. If not, I will need to provide a desktop-wrapper or live Gateway transcript from an environment with working dependency access. |
1563ed7 to
84c30a6
Compare
84c30a6 to
17f2234
Compare
|
Superseded by #38290, landed as fa0349a. The landed fix uses the same exact hosted-origin approach, rebuilt on current Verification covered 146 focused validator and real Gateway-auth tests, a live WebSocket connect handshake for exact |
What Problem This Solves
gateway.controlUi.allowedOriginssupports explicit browser origin allowlists, but hosted custom desktop-app origins such astauri://localhost,electron://localhost, andapp://localhostcurrently fail exact matching. The root cause is WHATWG URL behaviour:new URL("tauri://localhost").originserialises asnull, so the Gateway compares the configuredtauri://localhostallowlist entry withnulland rejects the browser WebSocket connection.This matters for Tauri/Electron-style Control UI wrappers because operators currently have to use wildcard origins even when they want a specific origin allowlist.
Summary
URL.originbehaviour for standard web origins.protocol//hostvalue before matching againstgateway.controlUi.allowedOrigins.*behaviour.Origin: null, malformed origins, hostlessfile:/data:origins, and unlisted custom origins rejected.Linked Issue
Closes #46520.
Evidence
Validation added in this branch:
src/gateway/origin-check.test.tscovers exact allowlist success forapp://localhost,electron://localhost, andtauri://localhost.src/gateway/origin-check.test.tscovers rejection for an unlisted hosted custom-scheme origin.src/gateway/origin-check.test.tscovers fail-closed behaviour for literalOrigin: null, malformed origins,file:origins, anddata:origins.src/gateway/server.auth.browser-hardening.test.tsstarts a real Gateway, configuresgateway.controlUi.allowedOrigins, opens a WebSocket with a browserOriginheader, and verifies both sides of the runtime boundary:Origin: tauri://localhostwithallowedOrigins: ["tauri://localhost"]reacheshello-ok.Origin: electron://attacker.examplewithallowedOrigins: ["tauri://localhost"]is rejected through the existingCONTROL_UI_ORIGIN_NOT_ALLOWEDpath.Local verification:
git diff --check upstream/main...HEADpassed.ETIMEDOUTbefore Vitest started. GitHub CI is running the branch with dependencies available.Security Impact
The security boundary remains explicit allowlist matching. The parser fallback only changes the parsed origin string for successfully parsed URLs that have both a protocol and a host.
Compatibility
Backward compatible. No config, migration, or docs changes are required.