fix(onboard): include portless origin in allowedOrigins for reverse-proxy access - #3002
Conversation
senthilr-nv
added
bug
platform: brev
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe config generator now derives a portless origin (scheme://hostname, preserving IPv6 brackets) from CHAT_UI_URL when it includes an explicit port and the host is not loopback; the allowedOrigins list (loopback, full origin, optional portless origin) is de-duplicated and assigned to gateway.controlUi.allowedOrigins. Tests cover IPv4, IPv6, and loopback cases. ChangesOpenClaw Portless Origin Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
01b316f to
6583d8e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-openclaw-config.py`:
- Around line 202-206: portless_origin derivation should avoid raising
ValueError on malformed ports and must preserve IPv6 bracket notation: when
checking parsed.port, guard the access with a try/except (catch ValueError) and
treat invalid/non-numeric/out-of-range ports as "no port" (i.e., behave as if
port is absent); when building the origin from parsed.hostname, wrap IPv6
addresses in brackets (e.g., if hostname contains ':' and is not already
bracketed) so non-loopback IPv6 produces "https://[2001:db8::1]"; update the
logic around the port/hostname checks that set portless_origin to use the safe
parsed.port handling and the bracketed hostname, and still call
is_loopback(parsed.hostname) as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f6dc0e96-61dd-40e4-b20e-40c4d185ece9
📒 Files selected for processing (2)
scripts/generate-openclaw-config.pytest/generate-openclaw-config.test.ts
6583d8e to
10fe201
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/generate-openclaw-config.py (1)
202-206:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
parsed.portaccess to avoid crashing on malformedCHAT_UI_URL.
parsed.portcan throwValueError(e.g.,:abc,:99999), so the current condition can fail hard during config generation.Suggested minimal fix
- if parsed.scheme and parsed.hostname and parsed.port and not is_loopback(parsed.hostname): + try: + has_explicit_port = parsed.port is not None + except ValueError: + has_explicit_port = False + + if parsed.scheme and parsed.hostname and has_explicit_port and not is_loopback(parsed.hostname): host_part = f"[{parsed.hostname}]" if ":" in parsed.hostname else parsed.hostname portless_origin = f"{parsed.scheme}://{host_part}" else: portless_origin = None#!/bin/bash set -euo pipefail echo "1) Confirm direct parsed.port usage in current code:" rg -n --type=py 'parsed\.port' scripts/generate-openclaw-config.py -C2 echo echo "2) Reproduce stdlib behavior for malformed ports:" python3 - <<'PY' from urllib.parse import urlparse samples = [ "https://example.com:abc", "https://example.com:99999", "https://example.com:18789", ] for u in samples: p = urlparse(u) try: port = p.port except ValueError as e: port = f"ValueError: {e}" print(f"{u} -> {port}") PY🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate-openclaw-config.py` around lines 202 - 206, The condition directly accesses parsed.port which can raise ValueError for malformed CHAT_UI_URL; wrap the port access in a safe try/except (or pre-validate) and use the resulting port variable in the existing logic: e.g., try to get port = parsed.port except ValueError: port = None, then change the if to check parsed.scheme and parsed.hostname and port and not is_loopback(parsed.hostname), keeping the host_part and portless_origin assignment using parsed.hostname as before (references: parsed.port, parsed.hostname, host_part, portless_origin, is_loopback).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/generate-openclaw-config.py`:
- Around line 202-206: The condition directly accesses parsed.port which can
raise ValueError for malformed CHAT_UI_URL; wrap the port access in a safe
try/except (or pre-validate) and use the resulting port variable in the existing
logic: e.g., try to get port = parsed.port except ValueError: port = None, then
change the if to check parsed.scheme and parsed.hostname and port and not
is_loopback(parsed.hostname), keeping the host_part and portless_origin
assignment using parsed.hostname as before (references: parsed.port,
parsed.hostname, host_part, portless_origin, is_loopback).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9ef051b9-d0fe-4404-80eb-67ccb086fd65
📒 Files selected for processing (2)
scripts/generate-openclaw-config.pytest/generate-openclaw-config.test.ts
6ce3c7d to
70954ac
Compare
…roxy access When CHAT_UI_URL is set to an HTTPS URL without an explicit port (as documented for Brev deployments), onboard injects the internal dashboard port (e.g. :18789) into the origin. Reverse proxies (Brev Cloudflare Tunnel, nginx, Caddy) serve on standard :443, so the browser origin doesn't carry the port — causing "origin not allowed" on the dashboard. Add the portless origin alongside the port-annotated one in allowedOrigins for non-loopback URLs. This is safe (same host) and covers both direct and proxied access paths. Prior art: PR NVIDIA#812 and PR NVIDIA#2440 fixed the CHAT_UI_URL plumbing but the port-override gap remained. Fixes NVIDIA#3000 Ref: NVIDIA#795, NVIDIA#20 Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
70954ac to
76832c6
Compare
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed the current head. This is a narrow and correct fix for the reverse-proxy origin mismatch: it adds only the same non-loopback host without the explicit internal port, preserves loopback behavior, handles malformed ports and IPv6 safely, and has focused config-generation coverage. The current WSL failure is in Ubuntu apt metadata download before repo tests run, so it is unrelated to this PR.
wscurran
added
area: cli
3 participants
Summary
When
CHAT_UI_URLis set to an HTTPS URL without an explicit port — as documented for Brev deployments — onboard injects the internal dashboard port (:18789) into the origin viaonboard.ts:4002. Reverse proxies (Brev Cloudflare Tunnel, nginx, Caddy, Tailscale Funnel) serve on standard:443, so the browser sends originhttps://hostwhich doesn't matchhttps://host:18789inallowedOrigins— causing "origin not allowed" on the dashboard.Root cause:
generate-openclaw-config.pybuildsallowedOriginsfrom the port-overridden URL only.Fix: Also include the portless origin for non-loopback URLs. This is safe (same host) and covers both direct and reverse-proxy access.
Prior art
PR #812 and PR #2440 fixed the
CHAT_UI_URLplumbing but the port-override gap remained. This PR closes the remaining gap from the #795 / #20 lineage.Changes
scripts/generate-openclaw-config.pyallowedOriginsfor non-loopback URLs when a port is presenttest/generate-openclaw-config.test.tsTesting
vitest run test/generate-openclaw-config.test.ts)nemoclaw-gcp, n2-standard-4), confirmed that adding the portless origin toallowedOriginsresolves the CORS error when accessinghttps://brev-nc-xxx.brevlab.com/chat?session=mainWhat this does NOT fix
onboard.ts:4002still unconditionally overrides the port. A future improvement could skip the port override when the URL is HTTPS with no explicit port, but this config-level fix is sufficient and lower-risk.Related
Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests