feat: auto-allow remote gateway hostname in WS proxy#20
Open
alexmercier25 wants to merge 1 commit intodaggerhashimoto:masterfrom
Open
feat: auto-allow remote gateway hostname in WS proxy#20alexmercier25 wants to merge 1 commit intodaggerhashimoto:masterfrom
alexmercier25 wants to merge 1 commit intodaggerhashimoto:masterfrom
Conversation
📝 WalkthroughWalkthroughThe changes implement automatic augmentation of WS_ALLOWED_HOSTS with the gateway hostname. Setup script additions parse the gateway URL across three configuration codepaths, verify the host is non-loopback, and append the hostname to the allowlist. Config validation extracts the gateway hostname and adds a warning for remote gateways. URL parsing errors are handled gracefully with try-catch blocks. Changes
Possibly Related Issues
Poem
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
server/lib/config.ts (1)
89-93: Loopback hostnames are added redundantly to the Set.When
GATEWAY_URLis a loopback address (e.g.http://localhost:3000),gatewayHostnamewill be'localhost', which is already present in the Set. This is harmless due to Set deduplication, but it diverges from the explicit!isLoopback(gwHostname)guard used inscripts/setup.ts. Consider mirroring the same guard here for consistency.♻️ Proposed fix
+const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '::1']); + /** WebSocket proxy allowed hostnames (extend via WS_ALLOWED_HOSTS env var, comma-separated) */ export const WS_ALLOWED_HOSTS = new Set([ 'localhost', '127.0.0.1', '::1', - ...(gatewayHostname ? [gatewayHostname] : []), + ...(gatewayHostname && !LOOPBACK_HOSTS.has(gatewayHostname) ? [gatewayHostname] : []), ...(process.env.WS_ALLOWED_HOSTS?.split(',').map(h => h.trim()).filter(Boolean) ?? []), ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/config.ts` around lines 89 - 93, The WS_ALLOWED_HOSTS set currently spreads gatewayHostname unconditionally which can duplicate loopback entries; change the spread to only include gatewayHostname when it exists and is not a loopback by checking isLoopback(gatewayHostname) (mirroring scripts/setup.ts). In other words, guard the ...(gatewayHostname ? [gatewayHostname] : []) spread with a !isLoopback(gatewayHostname) check so only non-loopback gateway hostnames are added to WS_ALLOWED_HOSTS; keep the other env-derived hosts unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup.ts`:
- Around line 365-378: The augmentation that adds the gateway hostname
(gwHostname) to config.WS_ALLOWED_HOSTS is being clobbered by the tailscale and
network branches which set config.WS_ALLOWED_HOSTS = tsIp / ip; fix by either
(A) moving the "Auto-add WS_ALLOWED_HOSTS for remote gateways" block so it runs
after the entire access-mode if/else-if/else block, or (B) changing the
tailscale/network branches to compose instead of overwrite (e.g. append tsIp/ip
to existing config.WS_ALLOWED_HOSTS if present), ensuring the logic in the try
block that computes gwHostname and updates config.WS_ALLOWED_HOSTS runs last and
preserves both entries.
---
Nitpick comments:
In `@server/lib/config.ts`:
- Around line 89-93: The WS_ALLOWED_HOSTS set currently spreads gatewayHostname
unconditionally which can duplicate loopback entries; change the spread to only
include gatewayHostname when it exists and is not a loopback by checking
isLoopback(gatewayHostname) (mirroring scripts/setup.ts). In other words, guard
the ...(gatewayHostname ? [gatewayHostname] : []) spread with a
!isLoopback(gatewayHostname) check so only non-loopback gateway hostnames are
added to WS_ALLOWED_HOSTS; keep the other env-derived hosts unchanged.
Comment on lines
+365
to
+378
| // Auto-add WS_ALLOWED_HOSTS for remote gateways | ||
| try { | ||
| const gwHostname = new URL(config.GATEWAY_URL!).hostname; | ||
| if (!isLoopback(gwHostname)) { | ||
| config.WS_ALLOWED_HOSTS = config.WS_ALLOWED_HOSTS | ||
| ? (config.WS_ALLOWED_HOSTS.includes(gwHostname) ? config.WS_ALLOWED_HOSTS : `${config.WS_ALLOWED_HOSTS},${gwHostname}`) | ||
| : gwHostname; | ||
| success(`Added ${gwHostname} to WS_ALLOWED_HOSTS`); | ||
| console.log(''); | ||
| warn('Remote gateway detected. On the gateway host, ensure Nerve\'s origin is in'); | ||
| dim(' gateway.controlUi.allowedOrigins in your OpenClaw config (~/.openclaw/openclaw.json).'); | ||
| console.log(''); | ||
| } | ||
| } catch { /* invalid URL — already validated above */ } |
There was a problem hiding this comment.
WS_ALLOWED_HOSTS gateway entry is silently clobbered by the access mode section.
This is the core fix path for the PR, but it doesn't work for two common configurations.
The tailscale and network access mode branches (lines 479 and 502) unconditionally overwrite config.WS_ALLOWED_HOSTS with just the LAN/Tailscale IP:
// tailscale branch (line 479)
config.WS_ALLOWED_HOSTS = tsIp; // clobbers gwHostname
// network branch (line 502)
config.WS_ALLOWED_HOSTS = ip; // clobbers gwHostnameSo any user running Nerve on a LAN machine pointing at a remote gateway — which is exactly the scenario this PR targets (Nerve on macOS → Pi gateway) — loses the gateway hostname from WS_ALLOWED_HOSTS in the written .env, and the WS proxy will still reject the connection.
The local and custom branches don't touch WS_ALLOWED_HOSTS, so those paths are unaffected.
Fix: Move the augmentation to after the access mode block, or compose rather than overwrite in the tailscale/network branches.
🐛 Proposed fix — move augmentation after access mode
- // Auto-add WS_ALLOWED_HOSTS for remote gateways
- try {
- const gwHostname = new URL(config.GATEWAY_URL!).hostname;
- if (!isLoopback(gwHostname)) {
- config.WS_ALLOWED_HOSTS = config.WS_ALLOWED_HOSTS
- ? (config.WS_ALLOWED_HOSTS.includes(gwHostname) ? config.WS_ALLOWED_HOSTS : `${config.WS_ALLOWED_HOSTS},${gwHostname}`)
- : gwHostname;
- success(`Added ${gwHostname} to WS_ALLOWED_HOSTS`);
- console.log('');
- warn('Remote gateway detected. On the gateway host, ensure Nerve\'s origin is in');
- dim(' gateway.controlUi.allowedOrigins in your OpenClaw config (~/.openclaw/openclaw.json).');
- console.log('');
- }
- } catch { /* invalid URL — already validated above */ }
-
// ── 2/5: Agent Identity ──────────────────────────────────────────Add the augmentation block after the access mode } closing brace at the end of the if/else if/else block (after line 580):
// Auto-add WS_ALLOWED_HOSTS for remote gateways (runs after access mode sets the base value)
try {
const gwHostname = new URL(config.GATEWAY_URL!).hostname;
if (!isLoopback(gwHostname)) {
config.WS_ALLOWED_HOSTS = config.WS_ALLOWED_HOSTS
? (config.WS_ALLOWED_HOSTS.includes(gwHostname) ? config.WS_ALLOWED_HOSTS : `${config.WS_ALLOWED_HOSTS},${gwHostname}`)
: gwHostname;
success(`Added ${gwHostname} to WS_ALLOWED_HOSTS`);
console.log('');
warn('Remote gateway detected. On the gateway host, ensure Nerve\'s origin is in');
dim(' gateway.controlUi.allowedOrigins in your OpenClaw config (~/.openclaw/openclaw.json).');
console.log('');
}
} catch { /* invalid URL — already validated above */ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/setup.ts` around lines 365 - 378, The augmentation that adds the
gateway hostname (gwHostname) to config.WS_ALLOWED_HOSTS is being clobbered by
the tailscale and network branches which set config.WS_ALLOWED_HOSTS = tsIp /
ip; fix by either (A) moving the "Auto-add WS_ALLOWED_HOSTS for remote gateways"
block so it runs after the entire access-mode if/else-if/else block, or (B)
changing the tailscale/network branches to compose instead of overwrite (e.g.
append tsIp/ip to existing config.WS_ALLOWED_HOSTS if present), ensuring the
logic in the try block that computes gwHostname and updates
config.WS_ALLOWED_HOSTS runs last and preserves both entries.
Owner
|
Thanks for tackling this, remote gateway support is a real need. Blocking issues
Once these are in, this should be mergeable. 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes issue #19
Problem
When Nerve runs on a different machine than the OpenClaw gateway (e.g. Nerve on Mac, gateway on Raspberry Pi), the WS proxy rejects the connection with "target not allowed" because WS_ALLOWED_HOSTS only includes localhost, 127.0.0.1, and ::1.
Changes
server/lib/config.ts
• Extract hostname from GATEWAY_URL and auto-add it to WS_ALLOWED_HOSTS
• No change for localhost setups (no regression)
scripts/setup.ts
• When user enters a non-localhost gateway URL, auto-write WS_ALLOWED_HOSTS to .env
• Print warning about gateway.controlUi.allowedOrigins config needed on the gateway host
Testing
• Tested with Nerve on macOS connecting to OpenClaw gateway on Raspberry Pi (LAN)
• WS proxy connects without manual WS_ALLOWED_HOSTS config
• Localhost gateway setups unaffected
Summary by CodeRabbit