Skip to content

fix(gateway): support custom URL schemes in origin allowlist matching#98912

Closed
wendy-chsy wants to merge 1 commit into
openclaw:mainfrom
wendy-chsy:fix/custom-scheme-origin-allowlist
Closed

fix(gateway): support custom URL schemes in origin allowlist matching#98912
wendy-chsy wants to merge 1 commit into
openclaw:mainfrom
wendy-chsy:fix/custom-scheme-origin-allowlist

Conversation

@wendy-chsy

@wendy-chsy wendy-chsy commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Why This Change Was Made

The parseOrigin function in src/gateway/origin-check.ts uses new URL() which works for standard schemes (http, https) but returns null as the origin for custom schemes. This means even when a user explicitly allows tauri://localhost in allowedOrigins, the allowlist check fails because the parsed origin is "null" instead of "tauri://localhost".

User Impact

  • Tauri/Electron desktop app users can now use proper origin allowlisting instead of being forced to use "*"
  • No breaking changes to existing behavior for standard URL schemes
  • The custom scheme parser handles IPv6 addresses and port numbers correctly

Evidence

Behavior or issue addressed

  • Gateway WebSocket connections from Tauri/Electron clients with custom URL schemes are rejected by origin validation, even when the origin is explicitly listed in allowedOrigins

Real environment tested

  • Local development environment (Linux, Node 22)

Exact steps or command run after this patch

pnpm test src/gateway/origin-check.test.ts

Evidence after fix

All 84 tests pass (78 existing + 6 new) covering:

  • Custom scheme origin accepted via allowlist (tauri://localhost)
  • Custom scheme origin with port accepted via allowlist (electron://localhost:5173)
  • Custom scheme origin rejected when not in allowlist
  • Custom scheme origin accepted via wildcard
  • Custom scheme origin with local-loopback dev fallback

Observed result after fix

 Test Files  4 passed (4)
      Tests  84 passed (84)

Tests and validation

pnpm test src/gateway/origin-check.test.ts  # all 84 tests pass

Risk checklist

  • No: Does this change affect user-visible behavior? (Restores expected behavior, not a new surface)
  • No: Does this change affect configuration surface?
  • No: Does this change affect security? (Improves security by enabling proper allowlisting)
  • No: Does this change affect plugin APIs?
  • No: Does this change require documentation updates?

Additional notes

AI-assisted fix. The change adds a parseCustomSchemeOrigin fallback for custom URL schemes while preserving the existing standard URL parsing path unchanged.

@wendy-chsy
fix(gateway): support custom URL schemes in origin allowlist matching
c273389 
The parseOrigin function used the standard URL constructor which assigns
an opaque origin ("null") to custom schemes like tauri:// and electron://.
This prevented those origins from being matched against the configured
allowedOrigins allowlist, forcing Tauri/Electron Control UI clients to use
the wildcard "*" workaround.

Add a parseCustomSchemeOrigin fallback that manually extracts scheme, host,
and hostname from custom scheme origins. The fallback is triggered both
when URL ctor throws and when it returns an opaque "null" origin.

Closes openclaw#46520
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 12:56 AM ET / 04:56 UTC.

Summary
The PR adds a manual origin parser fallback for opaque or rejected URL parses and adds gateway origin-check tests for Tauri/Electron custom schemes.

PR surface: Source +62, Tests +45. Total +107 across 2 files.

Reproducibility: yes. Current source passes the browser Origin through new URL() and compares parsedOrigin.origin, while Node reports custom schemes such as tauri://localhost with origin equal to null.

Review metrics: 1 noteworthy metric.

  • Control UI allowlist behavior: 1 existing config behavior changed. The PR changes how gateway.controlUi.allowedOrigins authorizes custom-scheme browser origins in the Gateway access-control path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #46520
Summary: This PR is a candidate fix for the canonical exact custom-scheme gateway.controlUi.allowedOrigins failure caused by URL.origin serializing hosted custom schemes as null.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Keep new URL() parse failures rejected except for the intended opaque custom-origin case.
  • [P1] Add redacted real Gateway proof showing an explicitly allowlisted custom origin succeeds and an unlisted custom origin fails.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Needs real behavior proof before merge: the PR body only shows unit-test output, so the contributor should add redacted Gateway logs, terminal output, a network trace, recording, or linked artifact and then update the PR body for re-review.

Mantis proof suggestion
A maintainer-run real Gateway/custom-origin proof would materially help because the changed behavior is a network authorization boundary. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify Gateway accepts Origin: tauri://localhost only when allowedOrigins includes tauri://localhost and rejects an unlisted custom-scheme origin.

Risk before merge

  • [P1] Merging as-is would make malformed standard Origin headers parseable in a Gateway authorization helper instead of fail-closed.
  • [P1] The external PR has only unit-test output; it still needs redacted real Gateway proof for this authorization-boundary change.

Maintainer options:

  1. Keep Invalid Origins Fail-Closed (recommended)
    Limit fallback parsing to successful opaque hosted URLs and add a regression case proving malformed standard origins still return origin missing or invalid.
  2. Accept The Broader Parser Risk
    Maintainers could intentionally accept malformed standard origins being parsed, but that would be a deliberate Gateway authorization-boundary change.
  3. Use A Cleaner Sibling Fix
    Maintainers can pause or close this branch if an equivalent sibling becomes current-main clean, proof-positive, and avoids the malformed-origin expansion.

Next step before merge

  • [P1] This needs contributor changes plus redacted real behavior proof for a Gateway authorization boundary; automation should not treat the PR as merge-ready from unit tests alone.

Security
Needs attention: The diff touches a Gateway browser-origin authorization boundary and introduces a concrete over-acceptance risk for malformed standard origins.

Review findings

  • [P1] Keep invalid standard origins rejected — src/gateway/origin-check.ts:38
Review details

Best possible solution:

Keep the custom-scheme fix local to successful hosted opaque origins, preserve rejection for standard URL parse failures, add invalid-origin coverage, and require redacted Gateway allow/reject proof before merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Current source passes the browser Origin through new URL() and compares parsedOrigin.origin, while Node reports custom schemes such as tauri://localhost with origin equal to null.

Is this the best way to solve the issue?

No as currently patched. The parser layer is the right place, but the best fix should reconstruct hosted opaque origins without parsing standard-scheme strings that new URL() rejected.

Full review comments:

  • [P1] Keep invalid standard origins rejected — src/gateway/origin-check.ts:38
    The new catch path sends every new URL() failure into the manual parser, so malformed standard origins such as http://localhost:abc or https://exa mple.com can become parsed hosts instead of staying invalid. This helper gates Gateway browser-origin authorization, so keep thrown URL parses fail-closed and only reconstruct successful opaque hosted origins.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b2c507c5f04c.

Label changes

Label changes:

  • add P2: This is a bounded Gateway origin-allowlist bugfix with security-boundary implications but no core outage.
  • add merge-risk: 🚨 security-boundary: The changed helper decides which browser-origin Gateway WebSocket and trusted-proxy HTTP requests may pass origin authorization.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Needs real behavior proof before merge: the PR body only shows unit-test output, so the contributor should add redacted Gateway logs, terminal output, a network trace, recording, or linked artifact and then update the PR body for re-review.

Label justifications:

  • P2: This is a bounded Gateway origin-allowlist bugfix with security-boundary implications but no core outage.
  • merge-risk: 🚨 security-boundary: The changed helper decides which browser-origin Gateway WebSocket and trusted-proxy HTTP requests may pass origin authorization.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Needs real behavior proof before merge: the PR body only shows unit-test output, so the contributor should add redacted Gateway logs, terminal output, a network trace, recording, or linked artifact and then update the PR body for re-review.
Evidence reviewed

PR surface:

Source +62, Tests +45. Total +107 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 62 0 +62
Tests 1 45 0 +45
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 107 0 +107

Security concerns:

  • [medium] Malformed standard origins can become authorized origins — src/gateway/origin-check.ts:38
    Current main rejects standard-scheme Origin values that new URL() cannot parse, but the PR catch fallback can turn those values into origin, host, and hostname data used by allowlist, host-header, and local-loopback decisions.
    Confidence: 0.9

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/gateway/origin-check.test.ts.
  • [P1] git diff --check.

What I checked:

  • Current main parser behavior: Current main parses browser origins with new URL(trimmed) and stores url.origin, so custom schemes whose origin serializes as null cannot match exact configured origins. (src/gateway/origin-check.ts:25, b2c507c5f04c)
  • Current allowlist comparison: The explicit allowlist only accepts * or exact equality with parsedOrigin.origin, which remains null for the reported schemes on current main. (src/gateway/origin-check.ts:54, b2c507c5f04c)
  • Node URL behavior proof: A local Node probe showed tauri://localhost and electron://localhost:5173 have origin=null, while malformed standard origins such as http://localhost:abc throw.
  • PR catch fallback broadens parsing: The PR head returns parseCustomSchemeOrigin(trimmed) from the new URL() catch path, so invalid standard URL parses no longer necessarily stay invalid. (src/gateway/origin-check.ts:38, c2733897ed03)
  • Fallback over-acceptance proof: A local reproduction of the PR parser showed http://localhost:abc and https://exa mple.com are invalid for new URL() but become parsed origin/host/hostname objects under the new fallback shape.
  • WebSocket caller: The Gateway WebSocket connect path passes requestOrigin and gateway.controlUi.allowedOrigins into checkBrowserOrigin and rejects failures as origin-mismatch. (src/gateway/server/ws-connection/message-handler.ts:865, b2c507c5f04c)

Likely related people:

  • steipete: Commit 66d8117d4483df542d450b9f4bb6dd2a2b769046 added the origin-check module, tests, and WebSocket origin hardening path. (role: introduced behavior and area hardening contributor; confidence: high; commits: 66d8117d4483; files: src/gateway/origin-check.ts, src/gateway/origin-check.test.ts, src/gateway/server/ws-connection/message-handler.ts)
  • Frank Yang: Commit 1636f7ff5fd78afc1bd7bc80a7970478d0821138 added wildcard support in the same gateway.controlUi.allowedOrigins comparison path. (role: adjacent allowlist contributor; confidence: medium; commits: 1636f7ff5fd7; files: src/gateway/origin-check.ts, src/gateway/origin-check.test.ts)
  • Vincent Koc: Commit 6b3f99a11f4d070fa5ed2533abbb3d7329ea4f0d added trusted-proxy HTTP origin checks that reuse this browser-origin helper. (role: adjacent auth contributor; confidence: medium; commits: 6b3f99a11f4d; files: src/gateway/auth.ts, src/gateway/origin-check.ts)
  • joshavant: Local blame attributes the current parser lines to commit 77f632e7dbd467c9898c53148fc52c8707211f65, though that appears to be a broad recent touch rather than the original design. (role: recent area contributor; confidence: low; commits: 77f632e7dbd4; files: src/gateway/origin-check.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 2, 2026
@wendy-chsy wendy-chsy closed this Jul 2, 2026
@wendy-chsy
wendy-chsy deleted the fix/custom-scheme-origin-allowlist branch July 2, 2026 06:30
@clawsweeper clawsweeper Bot mentioned this pull request Jul 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Origin validation fails for custom URL schemes (tauri://, electron://)

1 participant

@wendy-chsy