fix: bind gateway to 0.0.0.0 and document remote dashboard access (Fixes #20)#114
fix: bind gateway to 0.0.0.0 and document remote dashboard access (Fixes #20)#114deepujain wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
wscurran
added
enhancement
|
Thanks for enhancing the gateway and making it more accessible, this will definitely improve the user experience for remote users. |
wscurran
added
the
priority: medium
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
c4a3c25 to
f32c2b3
Compare
|
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:
📝 WalkthroughWalkthroughAdds a Python CLI to generate/update OpenClaw gateway config, updates the start script to export UI-related env vars and invoke the generator, documents SSH port‑forwarded remote dashboard access and GATEWAY_BIND, and adds tests validating the written gateway JSON. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Laptop as "Laptop (ssh -L)"
participant Remote as "Remote Gateway"
participant Backend as "App Backend"
Browser->>Laptop: Open http://127.0.0.1:18789/ (forwarded)
Laptop->>Remote: SSH tunnel forwards localhost:18789 -> remote:18789
Remote->>Remote: Validate Origin against allowedOrigins (PUBLIC_PORT, CHAT_UI_URL)
Remote-->>Browser: Serve dashboard HTML (may include `#token` fragment)
Browser->>Remote: Open websocket (origin matches, device auth disabled)
Remote->>Backend: Proxy websocket/requests
Backend-->>Browser: Responses via gateway
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Rebased onto latest main to resolve conflicts. Key change during rebase: updated write-openclaw-gateway-config.py to honor the new NEMOCLAW_MODEL env var conditionally (matching the Ollama integration merged upstream in #295) instead of hardcoding the default model. All gateway-config tests pass. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docs/deployment/deploy-to-remote-gpu.md (2)
64-64: Multiple sentences on one line.This line contains two independent clauses joined, making diffs harder to read. Split into separate lines.
Suggested fix
-When you access the dashboard from your laptop via SSH port-forward, set `CHAT_UI_URL` to the URL your browser will use so the gateway allows that origin and websocket connections succeed. +When you access the dashboard from your laptop via SSH port-forward, set `CHAT_UI_URL` to the URL your browser uses. +This ensures the gateway allows that origin and websocket connections succeed.As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/deploy-to-remote-gpu.md` at line 64, The sentence mentioning SSH port-forward and CHAT_UI_URL contains two independent clauses on one line; edit the sentence so each independent clause is on its own line (i.e., split into two lines), keeping the same wording and preserving the `CHAT_UI_URL` reference and meaning so diffs are readable and follow "one sentence per line" guidelines.
66-66: Passive voice detected."Device auth is disabled" uses passive voice. Per the style guide, active voice is required.
Suggested fix
-The gateway inside the sandbox binds to all interfaces (`0.0.0.0`) by default so that port-forwarding works. Device auth is disabled for the sandbox Control UI; use the token in the dashboard URL to authenticate. +The gateway inside the sandbox binds to all interfaces (`0.0.0.0`) by default so that port-forwarding works. +The sandbox Control UI disables device auth; use the token in the dashboard URL to authenticate.As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/deploy-to-remote-gpu.md` at line 66, The sentence "Device auth is disabled for the sandbox Control UI; use the token in the dashboard URL to authenticate." uses passive voice—reword it in active voice (e.g., "The sandbox Control UI disables device auth; use the token in the dashboard URL to authenticate.") so the documentation adheres to the active-voice style guideline; update the sentence referencing "sandbox Control UI" and the token usage accordingly.test/gateway-config.test.js (2)
4-8: Inconsistent import style.Lines 4-5 use the
node:protocol prefix while lines 6-8 use bare module names. Consider usingnode:consistently for built-in modules.Suggested fix
const { describe, it } = require("node:test"); const assert = require("node:assert/strict"); -const fs = require("fs"); -const path = require("path"); -const { execSync } = require("child_process"); +const fs = require("node:fs"); +const path = require("node:path"); +const { execSync } = require("node:child_process");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/gateway-config.test.js` around lines 4 - 8, The imports use mixed styles: `require("node:test")` and `require("node:assert/strict")` but the rest use bare names (`fs`, `path`, `child_process`); update the requires for the built-ins (`fs`, `path`, and `child_process`) to use the `node:` protocol so all built-in modules are imported consistently (i.e., change the `require("fs")`, `require("path")`, and `require("child_process")` usages that provide `fs`, `path`, and `execSync` to `require("node:fs")`, `require("node:path")`, and `require("node:child_process")` respectively).
13-20: Consider cleaning up temp directories after tests.The
runGatewayConfigScripthelper creates temp directories that are never removed. While the OS cleans/tmpperiodically, explicitly cleaning up prevents accumulation during repeated test runs.Suggested approach
+const os = require("node:os"); + function runGatewayConfigScript(env = {}) { - const configDir = fs.mkdtempSync(path.join(require("os").tmpdir(), "nemoclaw-gateway-config-")); + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-config-")); const configPath = path.join(configDir, "openclaw.json"); const fullEnv = { ...process.env, OPENCLAW_JSON_PATH: configPath, ...env }; execSync(`python3 "${PYTHON_SCRIPT}"`, { env: fullEnv, encoding: "utf-8" }); const content = fs.readFileSync(configPath, "utf-8"); - return { config: JSON.parse(content), configPath }; + return { config: JSON.parse(content), configPath, cleanup: () => fs.rmSync(configDir, { recursive: true, force: true }) }; }Then call
cleanup()after each test or useafter()hooks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/gateway-config.test.js` around lines 13 - 20, The helper runGatewayConfigScript currently leaves temp directories behind; update it to remove the temp directory after use (or return a cleanup function so callers can remove it). Specifically, adjust runGatewayConfigScript to either (A) read the file then delete configDir with fs.rmSync(configDir, { recursive: true, force: true }) before returning, or (B) return { config, configPath, cleanup: () => fs.rmSync(configDir, { recursive: true, force: true }) } and update tests to call cleanup() in an after/afterEach hook; reference the function name runGatewayConfigScript and the local variable configDir/configPath to locate where to add the removal logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/deployment/deploy-to-remote-gpu.md`:
- Around line 69-75: Update the two CLI code blocks that currently use the bash
fence to use the console fence and add a leading "$ " prompt to each command;
specifically change the block containing export
CHAT_UI_URL="http://YOUR_HOST_IP_OR_NAME:18789" and the block containing ssh -L
18789:localhost:18789 <instance-name> so their fences are ```console and each
command line begins with "$ ".
In `@scripts/write-openclaw-gateway-config.py`:
- Around line 19-22: The code currently calls json.load(f) into cfg when the
file at config_path exists but doesn't catch malformed JSON; wrap the
json.load(...) call in a try/except that catches json.JSONDecodeError (and
optionally ValueError), log a clear error mentioning config_path and the
exception (or print to stderr), and then either exit with a non‑zero status or
fall back to an empty cfg; update the block around config_path, cfg and the
json.load call to implement this handling so the script no longer crashes on
invalid JSON.
- Around line 39-43: The code currently assigns gateway['controlUi'] = { ... }
which overwrites any existing controlUi config; update the assignment to merge
with existing values (e.g., use gateway.setdefault('controlUi', {}) and
gateway['controlUi'].setdefault('allowInsecureAuth', True) /
.setdefault('dangerouslyDisableDeviceAuth', True) /
.setdefault('allowedOrigins', origins)) so existing keys are preserved, or if
intentional, add a concise comment next to the gateway['controlUi'] block
stating that sandbox defaults intentionally replace any preexisting controlUi
settings for clarity and consistency with the earlier use of setdefault().
---
Nitpick comments:
In `@docs/deployment/deploy-to-remote-gpu.md`:
- Line 64: The sentence mentioning SSH port-forward and CHAT_UI_URL contains two
independent clauses on one line; edit the sentence so each independent clause is
on its own line (i.e., split into two lines), keeping the same wording and
preserving the `CHAT_UI_URL` reference and meaning so diffs are readable and
follow "one sentence per line" guidelines.
- Line 66: The sentence "Device auth is disabled for the sandbox Control UI; use
the token in the dashboard URL to authenticate." uses passive voice—reword it in
active voice (e.g., "The sandbox Control UI disables device auth; use the token
in the dashboard URL to authenticate.") so the documentation adheres to the
active-voice style guideline; update the sentence referencing "sandbox Control
UI" and the token usage accordingly.
In `@test/gateway-config.test.js`:
- Around line 4-8: The imports use mixed styles: `require("node:test")` and
`require("node:assert/strict")` but the rest use bare names (`fs`, `path`,
`child_process`); update the requires for the built-ins (`fs`, `path`, and
`child_process`) to use the `node:` protocol so all built-in modules are
imported consistently (i.e., change the `require("fs")`, `require("path")`, and
`require("child_process")` usages that provide `fs`, `path`, and `execSync` to
`require("node:fs")`, `require("node:path")`, and
`require("node:child_process")` respectively).
- Around line 13-20: The helper runGatewayConfigScript currently leaves temp
directories behind; update it to remove the temp directory after use (or return
a cleanup function so callers can remove it). Specifically, adjust
runGatewayConfigScript to either (A) read the file then delete configDir with
fs.rmSync(configDir, { recursive: true, force: true }) before returning, or (B)
return { config, configPath, cleanup: () => fs.rmSync(configDir, { recursive:
true, force: true }) } and update tests to call cleanup() in an after/afterEach
hook; reference the function name runGatewayConfigScript and the local variable
configDir/configPath to locate where to add the removal logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e4ebaca-58e8-437a-a420-df6815f443a7
📒 Files selected for processing (4)
docs/deployment/deploy-to-remote-gpu.mdscripts/nemoclaw-start.shscripts/write-openclaw-gateway-config.pytest/gateway-config.test.js
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
f32c2b3 to
67a6002
Compare
|
Addressed all CodeRabbit review comments |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/deployment/deploy-to-remote-gpu.md (1)
70-70: Use present tense in the setup instruction.Line 70 uses future tense (“you will use”). Rewrite it in present tense (for example, “the origin you use in the browser”) to match the docs voice standard.
As per coding guidelines, “Present tense. Flag future tense (‘will’) in descriptions of current behavior.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/deploy-to-remote-gpu.md` at line 70, Change the sentence "On the remote host, ensure the sandbox is started with the origin you will use in the browser, for example:" to present tense by replacing "you will use" with "you use" (i.e., "On the remote host, ensure the sandbox is started with the origin you use in the browser, for example:") so the setup instruction in the deploy-to-remote-gpu documentation follows the present-tense docs voice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/deployment/deploy-to-remote-gpu.md`:
- Line 70: Change the sentence "On the remote host, ensure the sandbox is
started with the origin you will use in the browser, for example:" to present
tense by replacing "you will use" with "you use" (i.e., "On the remote host,
ensure the sandbox is started with the origin you use in the browser, for
example:") so the setup instruction in the deploy-to-remote-gpu documentation
follows the present-tense docs voice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2fe373f0-ef90-4e90-b8c3-ca6b70b6c663
📒 Files selected for processing (4)
docs/deployment/deploy-to-remote-gpu.mdscripts/nemoclaw-start.shscripts/write-openclaw-gateway-config.pytest/gateway-config.test.js
✅ Files skipped from review due to trivial changes (1)
- scripts/write-openclaw-gateway-config.py
🚧 Files skipped from review as they are similar to previous changes (2)
- test/gateway-config.test.js
- scripts/nemoclaw-start.sh
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
67a6002 to
21cc648
Compare
|
Addressed the CodeRabbit nitpick. Rebased on main, resolved conflicts . |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/gateway-config.test.js`:
- Around line 14-21: The temp directory created in runGatewayConfigScript can
leak if execSync or JSON.parse throws; wrap the body that creates/uses configDir
in a try/finally so fs.rmSync(configDir, { recursive: true, force: true })
always runs, and rethrow (or let) the original error after cleanup; keep use of
configPath, config, and fullEnv unchanged and ensure the function still returns
the parsed config when no error occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66e5fe22-b5c8-41d7-80d7-9a7a0761df74
📒 Files selected for processing (4)
docs/deployment/deploy-to-remote-gpu.mdscripts/nemoclaw-start.shscripts/write-openclaw-gateway-config.pytest/gateway-config.test.js
✅ Files skipped from review due to trivial changes (1)
- scripts/write-openclaw-gateway-config.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/nemoclaw-start.sh
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
21cc648 to
d175e5c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/deployment/deploy-to-remote-gpu.md (1)
70-70: Use periods instead of colons before the code blocks in these steps.Line 70 and Line 74 use colons as general punctuation before examples; this page style reserves colons for introducing lists.
As per coding guidelines, "Colons should only introduce a list. Flag colons used as general punctuation between clauses."
Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/deploy-to-remote-gpu.md` at line 70, Replace the use of colons as general punctuation before the inline/example code blocks in the sentences "On the remote host, ensure the sandbox is started with the origin you use in the browser, for example:" and the similar sentence at the other occurrence (line containing "for example:") — change the trailing colon to a period and ensure the following code block remains directly beneath the sentence so the colon is only used for lists per style guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/deployment/deploy-to-remote-gpu.md`:
- Line 70: Replace the use of colons as general punctuation before the
inline/example code blocks in the sentences "On the remote host, ensure the
sandbox is started with the origin you use in the browser, for example:" and the
similar sentence at the other occurrence (line containing "for example:") —
change the trailing colon to a period and ensure the following code block
remains directly beneath the sentence so the colon is only used for lists per
style guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ddca0bb2-911e-4dbc-8e78-1f662b1a6a1c
📒 Files selected for processing (4)
docs/deployment/deploy-to-remote-gpu.mdscripts/nemoclaw-start.shscripts/write-openclaw-gateway-config.pytest/gateway-config.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- test/gateway-config.test.js
- scripts/write-openclaw-gateway-config.py
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
d175e5c to
d0a0afa
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/nemoclaw-start.sh (1)
19-22: Consider declaringSCRIPTS_DIRas local.
SCRIPTS_DIRis only used within this function but isn't declaredlocal, so it leaks into the global shell namespace.♻️ Suggested improvement
fix_openclaw_config() { - SCRIPTS_DIR="$(cd "$(dirname "$0")" && pwd)" + local SCRIPTS_DIR + SCRIPTS_DIR="$(cd "$(dirname "$0")" && pwd)" python3 "${SCRIPTS_DIR}/write-openclaw-gateway-config.py" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 19 - 22, The function fix_openclaw_config declares SCRIPTS_DIR without scoping, leaking it into the global shell namespace; make SCRIPTS_DIR a local variable by declaring it local inside fix_openclaw_config (referencing SCRIPTS_DIR and the fix_openclaw_config function that computes the script path used to call write-openclaw-gateway-config.py) so the variable is confined to the function's scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 19-22: The function fix_openclaw_config declares SCRIPTS_DIR
without scoping, leaking it into the global shell namespace; make SCRIPTS_DIR a
local variable by declaring it local inside fix_openclaw_config (referencing
SCRIPTS_DIR and the fix_openclaw_config function that computes the script path
used to call write-openclaw-gateway-config.py) so the variable is confined to
the function's scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76bcca86-75af-4cd4-ab05-090251c961ac
📒 Files selected for processing (4)
docs/deployment/deploy-to-remote-gpu.mdscripts/nemoclaw-start.shscripts/write-openclaw-gateway-config.pytest/gateway-config.test.js
✅ Files skipped from review due to trivial changes (2)
- docs/deployment/deploy-to-remote-gpu.md
- scripts/write-openclaw-gateway-config.py
🚧 Files skipped from review as they are similar to previous changes (1)
- test/gateway-config.test.js
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Not ready to merge — needs reconciliation with several in-flight PRs and current main.
Stale against main
nemoclaw-start.sh has diverged. Main now locks openclaw.json to root-owned mode 444 and verifies its SHA-256 hash at startup. The write-openclaw-gateway-config.py script writes to openclaw.json at runtime, which will either fail against the locked file or require undoing the security hardening.
Conflicts with #721 (gateway isolation)
#721 restructures the entire entrypoint with gosu privilege separation, PATH hardening, and config integrity checks. If #721 merges first, this PR will conflict heavily. The runtime config write approach is incompatible with #721's security model — gateway config changes would need to happen at build time or through a different mechanism (e.g., environment variables that the gateway reads directly, rather than rewriting the JSON).
Overlaps with #740
PR #740 solves the same remote dashboard access problem by wildcarding allowedOrigins in the Dockerfile. These two PRs should be reconciled — either the dynamic approach here or the static approach in #740, not both.
dangerouslyDisableDeviceAuth: true is unconditional
There's no way to opt out. This weakens the auth model for all deployments, including ones that don't need remote access. Should be conditional on GATEWAY_BIND=lan or a separate opt-in env var.
Tests are good
The Python script is clean, testable, and the vitest tests use proper temp directory isolation.
NVIDIA#20) - Set gateway.bind to lan in sandbox so port-forward works; optional GATEWAY_BIND env. - Extract gateway config to write-openclaw-gateway-config.py for tests. - Add Remote dashboard access section to deploy-to-remote-gpu.md. - Add test/gateway-config.test.js for bind and allowedOrigins. - Address CodeRabbit review: console fences, active voice, JSON error handling, temp cleanup. Fixes NVIDIA#20 Signed-off-by: Deepak Jain <deepujain@gmail.com>
deepujain
force-pushed
the
fix/20-remote-dashboard-bind
branch
from
d0a0afa to
25ec520
Compare
3 participants
Summary
When NemoClaw runs on a remote host, the dashboard gateway bound to
127.0.0.1, so forwarded browser connections could fail with "origin not allowed" or device-auth issues even when the port was forwarded. This change makes remote dashboard access work by binding the gateway to all interfaces by default and documenting the supported pattern.Fixes #20.
Changes
gateway.bindtolan(OpenClaw listens on0.0.0.0) so port-forwarding from the host reaches the gateway. Optional envGATEWAY_BIND(defaultlan; useloopbackto restrict to 127.0.0.1).CHAT_UI_URLandGATEWAY_BINDin the script header.scripts/write-openclaw-gateway-config.pyfor reuse and testing.OPENCLAW_JSON_PATHwhen set (for tests).CHAT_UI_URLto the browser origin, use SSH port-forward, and open the dashboard URL with token; note that gateway binds to0.0.0.0by default and device auth is disabled for the sandbox UI.gateway.bind === 'lan',GATEWAY_BIND=loopbackhonored,allowedOriginsincludes local andCHAT_UI_URLorigin,dangerouslyDisableDeviceAuthtrue.Testing
npm testpasses (including the new gateway-config tests).nemoclaw/was changed; build step not run.Summary by CodeRabbit
New Features
Documentation
Tests