fix(openclaw): propagate custom gateway port - #3332
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughResolve dashboard port from NEMOCLAW_DASHBOARD_PORT or CHAT_UI_URL, embed the resolved gateway.port and allowedOrigins in generated config, export OPENCLAW_GATEWAY_PORT/OPENCLAW_GATEWAY_URL to the runtime env, and ensure gateway auth token exists before export (guarded). ChangesGateway Port Configuration & Export
Sequence Diagram(s)sequenceDiagram
participant ConfigGen as generate-openclaw-config.py
participant StartScript as nemoclaw-start.sh
participant OpenClawJSON as /sandbox/.openclaw/openclaw.json
participant RuntimeEnv as /tmp/nemoclaw-proxy-env.sh
ConfigGen->>ConfigGen: _resolve_gateway_port(NEMOCLAW_DASHBOARD_PORT, CHAT_UI_URL)
ConfigGen->>ConfigGen: set gateway.port and controlUi.allowedOrigins
StartScript->>OpenClawJSON: ensure_gateway_token() (read, maybe generate, atomic write)
OpenClawJSON->>StartScript: gateway.auth.token (present or created)
StartScript->>RuntimeEnv: write_runtime_shell_env() including OPENCLAW_GATEWAY_PORT/URL/TOKEN
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
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 |
e165da0 to
42af970
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/nemoclaw-start.test.ts (1)
434-437: 💤 Low valueTest harness doesn't replace hash file path.
The
ensureGatewayTokenextraction replaces theopenclaw.jsonpath but not/sandbox/.openclaw/.config-hash. This causes hash recomputation to be skipped in tests (the path doesn't exist, so the-f "$hash_file"check fails).This is acceptable for the current tests since they verify token generation, not hash management. However, for more complete coverage, consider also replacing the hash file path and verifying hash recomputation.
💡 Optional: Add hash file path replacement for completeness
const ensureGatewayToken = extractShellFunctionFromSource(src, "ensure_gateway_token").replaceAll( "/sandbox/.openclaw/openclaw.json", path.join(openclawDir, "openclaw.json"), +).replaceAll( + "/sandbox/.openclaw/.config-hash", + path.join(openclawDir, ".config-hash"), );Then create an empty hash file in the harness and verify it's updated after token generation.
🤖 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 `@test/nemoclaw-start.test.ts` around lines 434 - 437, The ensureGatewayToken extraction currently replaces the openclaw.json path but not the hash file path, causing the test harness to skip hash-file checks; update the extraction step around extractShellFunctionFromSource("ensure_gateway_token") to also replace "/sandbox/.openclaw/.config-hash" with path.join(openclawDir, ".config-hash"), then in the test harness create an empty hash file at that replaced location before invoking the extracted ensure_gateway_token and add an assertion that the file's contents change (or mtime updates) after token generation to verify hash recomputation.
🤖 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.
Nitpick comments:
In `@test/nemoclaw-start.test.ts`:
- Around line 434-437: The ensureGatewayToken extraction currently replaces the
openclaw.json path but not the hash file path, causing the test harness to skip
hash-file checks; update the extraction step around
extractShellFunctionFromSource("ensure_gateway_token") to also replace
"/sandbox/.openclaw/.config-hash" with path.join(openclawDir, ".config-hash"),
then in the test harness create an empty hash file at that replaced location
before invoking the extracted ensure_gateway_token and add an assertion that the
file's contents change (or mtime updates) after token generation to verify hash
recomputation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e04249c6-a167-45f4-8795-308562aff795
📒 Files selected for processing (4)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/generate-openclaw-config.test.ts
- scripts/generate-openclaw-config.py
d40d513 to
a09dfdc
Compare
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 `@test/nemoclaw-start.test.ts`:
- Around line 527-543: The test "generates a gateway token before writing the
runtime shell env" (using runGatewayTokenHarness) seeds a stale token but only
asserts that an OPENCLAW_GATEWAY_TOKEN export exists; update the assertions so
the exported token in envFile is compared to the regenerated runtime token
(configAfter.gateway.auth.token) and/or asserted to not equal the seeded
"stale-token". Concretely, after obtaining envFile and configAfter from
runGatewayTokenHarness, assert that envFile contains the exact value of
configAfter.gateway.auth.token (or assert envFile does not include
"stale-token") to ensure export_gateway_token actually replaced the stale value.
🪄 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: 24b30488-ff3a-46c9-a652-8100f7a46ee6
📒 Files selected for processing (4)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/generate-openclaw-config.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/nemoclaw-start.sh
- scripts/generate-openclaw-config.py
a09dfdc to
a0d9606
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
22ed52c to
95c0afb
Compare
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/nemoclaw-start.sh`:
- Around line 876-921: ensure_gateway_token currently writes to openclaw.json
without verifying it is not a symlink when running as non-root; add an explicit
symlink guard early in ensure_gateway_token similar to
prepare_openclaw_config_for_write so the function refuses to operate if
/sandbox/.openclaw/openclaw.json is a symlink (or otherwise unsafe). Locate
ensure_gateway_token and add the same check logic used by
prepare_openclaw_config_for_write (reference apply_model_override and
apply_cors_override which use that pattern) before attempting any read/write or
invoking the Python block, and return a non-zero error if the check fails.
🪄 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: a45ab770-5c61-411c-9e21-aac593838938
📒 Files selected for processing (4)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/generate-openclaw-config.test.ts
- scripts/generate-openclaw-config.py
- test/nemoclaw-start.test.ts
95c0afb to
6cf440e
Compare
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/nemoclaw-start.sh`:
- Around line 199-200: When NEMOCLAW_DASHBOARD_PORT is unset the script should
derive the fallback port from CHAT_UI_URL before assigning _DASHBOARD_PORT and
exporting related vars; update the logic that currently hard-defaults
_DASHBOARD_PORT to 18789 to first parse CHAT_UI_URL for a valid port (extract
numeric port from the URL, validate it) and use that value if present, otherwise
fall back to 18789; then set PUBLIC_PORT, OPENCLAW_GATEWAY_PORT,
OPENCLAW_GATEWAY_URL and the port passed to the gateway run --port invocation
from the resolved _DASHBOARD_PORT so the exported values and gateway process
match the baked CHAT_UI_URL.
🪄 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: b84499a2-7543-4b39-a847-6ad82765995b
📒 Files selected for processing (4)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/generate-openclaw-config.test.ts
- scripts/generate-openclaw-config.py
6cf440e to
51204ce
Compare
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/nemoclaw-start.sh`:
- Around line 901-947: The Python block in ensure_gateway_token uses a
predictable tmp filename f'{path}.tmp' which a local attacker can symlink to
cause a root write; replace that logic in the python snippet inside
ensure_gateway_token so it creates the temporary file securely in the target
directory using a safe atomic tempfile (e.g. os.mkstemp or
tempfile.NamedTemporaryFile with dir=os.path.dirname(path) and flags to avoid
following symlinks), write the JSON to that file descriptor, fsync the data and
directory, set file mode to 0o600, close the descriptor, and then
os.replace(tmp_path, path); ensure any exceptions still print the SECURITY error
and exit non‑zero. Reference the python block invoked from ensure_gateway_token
and the variables path/tmp_path when making the change.
🪄 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: 18197792-14df-4458-a483-a62a64eaa884
📒 Files selected for processing (4)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/generate-openclaw-config.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/nemoclaw-start.test.ts
- scripts/generate-openclaw-config.py
Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
51204ce to
0642197
Compare
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
…eway-port # Conflicts: # scripts/nemoclaw-start.sh # test/nemoclaw-start.test.ts
ericksoa
left a comment
There was a problem hiding this comment.
Approved after reviewing the current head, resolving the main conflict, validating the targeted tests, and confirming CodeRabbit's security threads are resolved.
Summary
Keeps the OpenClaw gateway server, generated OpenClaw config, runtime CLI environment, and OpenShell forward aligned when NemoClaw uses a non-default dashboard/gateway port.
Related Issue
Fixes #3256
Changes
gateway.portfromNEMOCLAW_DASHBOARD_PORTorCHAT_UI_URL, with port validation.OPENCLAW_GATEWAY_PORT,OPENCLAW_GATEWAY_URL, and the gateway token before auto-pair/OpenClaw CLI subprocesses run.18790across generated config and startup runtime env behavior.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Targeted checks run:
env PATH=/home/yimoj/.local/node-v22.16.0-linux-x64/bin:$PATH ./node_modules/.bin/vitest run test/nemoclaw-start.test.ts test/generate-openclaw-config.test.ts(112 passed)env PATH=/home/yimoj/.local/node-v22.16.0-linux-x64/bin:$PATH npm run typecheck:cli(passed)python3 -m py_compile scripts/generate-openclaw-config.py(passed)bash -n scripts/nemoclaw-start.sh && git diff --check(passed)docker build --build-arg BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest -t nemoclaw-production .(passed)NEMOCLAW_TEST_IMAGE=nemoclaw-production bash test/e2e-non-root-smoke.sh(2 passed, reproduces/fixes the failed CI job)docker run --rm -e NEMOCLAW_DASHBOARD_PORT=18790 nemoclaw-production openclaw --version(passed)NEMOCLAW_TEST_IMAGE=nemoclaw-production bash test/e2e-port-overrides.sh(9 passed, expected image-module skips)18789occupied and second sandbox on18790; generated config,/tmp/nemoclaw-proxy-env.sh, andopenclaw agentall used the custom gateway port successfully.Known local failures:
npx prek run --all-filesfails in theTest (CLI)hook attest/fetch-guard-patch-regression.test.tsbecause this host has a root-owned global/usr/local/lib/node_modules/openclaw; the extracted Dockerfile block cannot remove it.npm testpreviously hit the same host-global OpenClaw permission issue plus installer preflight assumptions for this linked git worktree.Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation