Skip to content

Fix custom scheme origin allowlist matching#94132

Closed
andreas-247 wants to merge 2 commits into
openclaw:mainfrom
andreas-247:fix-custom-scheme-origin-allowlist
Closed

Fix custom scheme origin allowlist matching#94132
andreas-247 wants to merge 2 commits into
openclaw:mainfrom
andreas-247:fix-custom-scheme-origin-allowlist

Conversation

@andreas-247

@andreas-247 andreas-247 commented Jun 17, 2026

Copy link
Copy Markdown

Summary

  • Fixes explicit origin allowlist matching for custom URL schemes such as app://localhost, used by ClawControl.
  • Node’s URL.origin returns "null" for non-special schemes, so OpenClaw was comparing "null" instead of the configured origin, causing gateway.controlUi.allowedOrigins = ["app://localhost"] to fail.
  • The intended outcome is that custom-scheme origins with a valid host can be allowlisted explicitly, without requiring wildcard "*".
  • Intentionally out of scope: changing broader origin policy, default trust behavior, wildcard behavior, or same-origin/private-network fallback logic.
  • Success looks like app://localhost matching only when explicitly listed in allowedOrigins, while existing HTTP/HTTPS origin behavior remains unchanged.
  • Reviewers should focus on the fallback construction for url.origin === "null" and whether the added test coverage captures the intended security boundary.

Linked context

Closes #46520

Related #28172

Was this requested by a maintainer or owner?

No direct maintainer request. This was investigated after reproducing the ClawControl connection failure on a real OpenClaw gateway setup.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed:

    • ClawControl could not connect to the gateway via WSS when gateway.controlUi.allowedOrigins was set to ["app://localhost"].
    • The gateway reported the origin as not allowed.
    • Using wildcard ["*"] worked, but that is broader than necessary.

  • Real environment tested:

    • OpenClaw built from source on a VPS.
    • Gateway running in Docker.
    • Windows ClawControl app connecting to the gateway via WSS.

  • Exact steps or command run after this patch:

    • Rebuilt OpenClaw base image from patched source.
    • Restarted the gateway container.
    • Configured the gateway with:
openclaw config set gateway.controlUi.allowedOrigins '["app://localhost"]'

  • Restarted the gateway.

  • Connected from the Windows ClawControl app via WSS.

  • Evidence after fix:

    • This PR author is using the Windows ClawControl app to communicate through the patched gateway after setting allowedOrigins to ["app://localhost"].
    • Unit test output from the patched checkout:
Test Files 3 passed (3)
Tests 57 passed (57)
Start at 11:37:09
Duration 6.18s (transform 5.84s, setup 3.74s, import 510ms, tests 1.08s, environment 0ms)

  • Observed result after fix:

    • ClawControl connects successfully via WSS with gateway.controlUi.allowedOrigins = ["app://localhost"].
    • Wildcard ["*"] is no longer required for this app-origin case.

  • What was not tested:

    • Other desktop-shell schemes such as tauri://localhost or electron://localhost in a live app.
    • Browser extension origins in a live extension environment.
    • Full project test suite.

  • Proof limitations or environment constraints:

    • The live behavior proof was validated on my own deployment, not a public reproduction environment.
    • Runtime logs/screenshots are not attached here to avoid exposing private gateway details.
    • The focused regression test covers the allowlist behavior directly.

  • Before evidence:

    • Before this patch, the same setup showed the ClawControl error:
      origin not allowed
    • The app only connected when gateway.controlUi.allowedOrigins was set to ["*"].

Tests and validation

Which commands did you run?

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

Result:

Test Files 3 passed (3)
Tests 57 passed (57)
Start at 11:37:09
Duration 6.18s (transform 5.84s, setup 3.74s, import 510ms, tests 1.08s, environment 0ms)

What regression coverage was added or updated?

  • Added coverage for explicit allowlist matching of app://localhost.
  • The test verifies that a configured custom-scheme origin matches through the normal allowlist path.

What failed before this fix, if known?

  • app://localhost did not match allowedOrigins: ["app://localhost"] because new URL("app://localhost").origin evaluates to "null".

If no test was added, why not?

  • A regression test was added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

Yes.

What is the highest-risk area?

  • Origin allowlist handling for non-special URL schemes.

How is that risk mitigated?

  • The change does not broaden trust by default.
  • Custom-scheme origins are accepted only when they exactly match an explicit allowlist entry, unless existing wildcard behavior is already configured.
  • Existing HTTP/HTTPS behavior continues to use url.origin.
  • Missing, malformed, or literal "null" origins are still rejected.
  • Same-origin, private-network, loopback, and host-header fallback logic is intentionally unchanged.

Current review state

What is the next action?

  • Await maintainer review and CI.

What is still waiting on author, maintainer, CI, or external proof?

  • Waiting on CI and maintainer feedback.
  • Author can provide additional logs or screenshots if maintainers want more live proof, with private gateway details redacted.

Which bot or reviewer comments were addressed?

  • None yet.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 17, 2026
@andreas-247

andreas-247 commented Jun 17, 2026

Copy link
Copy Markdown
Author

I noticed #73511 addresses the same root cause. I’m happy for maintainers to merge whichever PR they prefer. I opened this one because I had live ClawControl proof for app://localhost and added focused validation.

@clawsweeper

clawsweeper Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 8:20 AM ET / 12:20 UTC.

Summary
The branch changes Gateway origin parsing so hosted custom schemes with URL.origin === "null" fall back to exact scheme://host matching, and adds unit cases for app, Electron, and Tauri allowlist entries.

PR surface: Source +13, Tests +27. Total +40 across 2 files.

Reproducibility: yes. Current source compares configured entries against URL.origin, and a Node probe shows the reported custom schemes serialize that origin as null before exact allowlist matching.

Review metrics: 1 noteworthy metric.

  • Control UI allowlist behavior: 1 existing config path behavior changed. The PR changes how gateway.controlUi.allowedOrigins authorizes custom-scheme browser origins, which matters because that setting gates Gateway browser-origin access.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #46520
Summary: The canonical issue reports exact custom-scheme gateway.controlUi.allowedOrigins failures caused by URL.origin === "null"; this PR and sibling exact-origin PRs are candidate fixes for the same parser root cause, while wildcard support is broader adjacent work.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted after-fix Gateway and ClawControl diagnostics such as terminal output, logs, a network trace, or a recording with diagnostics.
  • Rerun or triage the failed relevant Gateway and security-sensitive checks.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR describes private after-fix Gateway and ClawControl validation but does not attach redacted diagnostics; add logs, terminal output, a network trace, or a recording with private details redacted, then update the PR body for automatic re-review or ask a maintainer for @clawsweeper re-review.

Proof path suggestion
A maintainer-run diagnostic proof would materially help because the changed behavior is network/origin authorization rather than purely unit-visible behavior. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P2] The fallback changes the helper that gates Gateway WebSocket and trusted-proxy HTTP browser origins; maintainers should confirm the opaque-origin reconstruction stays limited to explicit allowlist matches.
  • [P1] The live ClawControl validation is described in prose, but no redacted logs, terminal output, network trace, or diagnostic recording are attached for this network/security path.
  • [P1] The current head is mergeable but still has failed security-sensitive-guard-detect and checks-node-agentic-gateway-methods checks that need rerun or triage.

Maintainer options:

  1. Require Redacted Gateway Diagnostics (recommended)
    Ask for redacted logs, terminal output, a network trace, or a diagnostic recording showing the custom origin passes only when explicitly allowlisted and an unlisted custom-scheme origin still fails.
  2. Accept Source-Level Risk
    Maintainers may intentionally land based on code review, focused unit coverage, and private validation while owning the remaining security-boundary proof gap.
  3. Use A Sibling Only If It Clears
    If maintainers prefer another branch, keep this PR open until the replacement is current-main clean, proof-positive, and preserves the same exact allowlist behavior.

Next step before merge

  • [P1] Manual review remains because the blockers are contributor-supplied redacted diagnostics, clean relevant checks, and maintainer acceptance of a security-boundary parser change, not an automated code repair.

Security
Cleared: No concrete code-level security or supply-chain regression was found, but the diff changes an authorization boundary and still needs maintainer proof review.

Review details

Best possible solution:

Land this or an equivalent current-main parser fix after redacted Gateway proof and clean relevant checks, preserving exact allowlist semantics while rejecting literal, malformed, and unlisted custom origins.

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

Yes. Current source compares configured entries against URL.origin, and a Node probe shows the reported custom schemes serialize that origin as null before exact allowlist matching.

Is this the best way to solve the issue?

Yes for the parser layer, with merge-readiness caveats. Reconstructing only hosted opaque origins is narrower than wildcard, config, or Host-header policy changes, but the PR still needs redacted Gateway proof and clean relevant checks.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded Gateway allowlist bug with a security-relevant wildcard workaround, but it is not a core outage or emergency.
  • merge-risk: 🚨 security-boundary: The changed helper decides which browser-origin Gateway WebSocket and trusted-proxy HTTP requests may pass into authenticated handling.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR describes private after-fix Gateway and ClawControl validation but does not attach redacted diagnostics; add logs, terminal output, a network trace, or a recording with private details redacted, then update the PR body for automatic re-review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +13, Tests +27. Total +40 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 18 5 +13
Tests 1 27 0 +27
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 45 5 +40

What I checked:

  • Current main parser behavior: Current main stores new URL(origin).origin directly, so hosted custom schemes compare against null instead of the configured scheme://host allowlist entry. (src/gateway/origin-check.ts:25, af2b0a6118bc)
  • Gateway WebSocket caller: The Control UI/WebChat handshake passes gateway.controlUi.allowedOrigins into checkBrowserOrigin and rejects failed checks as origin-mismatch, making this helper the relevant runtime authorization boundary. (src/gateway/server/ws-connection/message-handler.ts:864, af2b0a6118bc)
  • Trusted-proxy HTTP caller: Trusted-proxy HTTP browser-origin authorization also delegates to checkBrowserOrigin, so the change affects more than the WebSocket path. (src/gateway/auth.ts:376, af2b0a6118bc)
  • Node URL behavior: A local Node probe showed app://localhost, electron://localhost, tauri://localhost, and chrome-extension://... retain protocol and host while reporting origin=null, matching the reported root cause.
  • PR patch shape: The PR keeps normal url.origin when available and only reconstructs protocol//host when the parsed origin is opaque and both protocol and host are present. (src/gateway/origin-check.ts:25, 5880f5022237)
  • Regression coverage: The PR adds allowlist acceptance cases for app://localhost, electron://localhost, and tauri://localhost. (src/gateway/origin-check.test.ts:100, 5880f5022237)

Likely related people:

  • steipete: Peter Steinberger authored the initial Control UI WebSocket origin hardening, explicit non-loopback origin requirement, and loopback fallback hardening around the same helper. (role: introduced and hardened behavior; confidence: high; commits: 66d8117d4483, 223d7dc23d73, d5ae4b83378d; files: src/gateway/origin-check.ts, src/gateway/origin-check.test.ts, src/gateway/server/ws-connection/message-handler.ts)
  • frankekn: Frank Yang authored the merged wildcard support in the same gateway.controlUi.allowedOrigins comparison path. (role: adjacent allowlist contributor; confidence: high; commits: 1636f7ff5fd7; files: src/gateway/origin-check.ts, src/gateway/origin-check.test.ts)
  • vincentkoc: Vincent Koc authored trusted-proxy HTTP origin enforcement, which reuses checkBrowserOrigin for HTTP browser-origin authorization. (role: adjacent auth contributor; confidence: medium; commits: 6b3f99a11f4d; files: src/gateway/auth.ts, src/gateway/origin-check.ts)
  • joshavant: The current main copy of origin-check.ts and its test file is most recently attributed to a broad commit that reintroduced or touched the files, making this a useful recent-history routing signal even though the commit title is unrelated. (role: recent area contributor; confidence: low; commits: 8739f1e17e03; files: src/gateway/origin-check.ts, src/gateway/origin-check.test.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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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 Jun 17, 2026
This was referenced Jun 18, 2026
Closed
Closed
Merged
Closed
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
5 tasks
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Superseded by #38290, landed as fa0349a.

The landed fix preserves the same exact-match behavior for app://localhost and other hosted custom schemes, rebuilt on current main and hardened against parser-normalized dot segments and backslash path separators. It does not add prefix or scheme wildcard trust.

Verification covered 146 focused validator and real Gateway-auth tests, a live WebSocket connect handshake for an exact custom origin, fresh autoreview, and exact-head CI/CodeQL/OpenGrep. Thanks for reproducing this in a real ClawControl deployment and documenting the user impact.

@steipete steipete closed this Jul 9, 2026
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. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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://)

2 participants

@andreas-247 @steipete