fix(sandbox): guard openclaw configure and export gateway token (#1114)#1305
Merged
ericksoa merged 11 commits intoNVIDIA:mainfrom Apr 7, 2026
Merged
Conversation
Permission fixes: - Add /sandbox/.openclaw-data/credentials to the writable state layout in Dockerfile.base (mkdir + symlink), following the same pattern as agents, extensions, workspace, and other existing directories - This resolves EACCES errors when WhatsApp/Telegram bridges attempt to mkdir /sandbox/.openclaw/credentials at runtime Gateway token export: - Add export_gateway_token() to nemoclaw-start.sh that reads gateway.auth.token from openclaw.json and exports it as OPENCLAW_GATEWAY_TOKEN - Persist the token to ~/.bashrc and ~/.profile using the same marker-based idempotent pattern as the proxy config, so interactive sessions (openshell sandbox connect) also see the variable - Called in both root and non-root startup paths Security properties preserved: - /sandbox/.openclaw remains root-owned, chmod 755 (read-only for sandbox user) - openclaw.json remains chmod 444 (immutable) - New credentials symlink is chown -h root:root like all other symlinks - No new attack surface — credentials dir follows identical pattern to 10 existing writable subdirectories Fixes NVIDIA#1114 Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
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 Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as Startup Script
participant Reader as Python Token Reader
participant Env as Runtime Env
participant Profiles as User Profile Files
Startup->>Reader: call _read_gateway_token() (reads /sandbox/.openclaw/openclaw.json)
Reader-->>Startup: return token (string or empty)
alt token non-empty
Startup->>Env: export OPENCLAW_GATEWAY_TOKEN
Startup->>Profiles: insert/replace marker block with export line in .bashrc/.profile
else token empty
Startup->>Profiles: remove marker block from .bashrc/.profile
end
Startup->>Profiles: insert configure guard function into .bashrc/.profile (via markers)
Startup->>Startup: continue to configure_messaging_channels()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@scripts/nemoclaw-start.sh`:
- Around line 130-132: The script currently returns early when the token is
empty, leaving previously inserted marker blocks in users' shell rc files and
causing stale credentials to persist; update the logic around the token check
(the block that currently does "if [ -z \"$token\" ]; then return; fi") to first
remove any existing marker blocks from the shell rc files (e.g., .bashrc,
.profile) before returning — implement a small cleanup routine that detects the
unique start/end markers used by the script and deletes the enclosed lines (use
the same marker names the script inserts), then return; apply the same change to
the other token-handling section referenced (the block around lines 144-156) so
both places remove markers when token is missing.
🪄 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: Pro
Run ID: 39659444-ffd6-4178-803e-b9e04d2789df
📒 Files selected for processing (2)
Dockerfile.basescripts/nemoclaw-start.sh
wscurran
added
bug
Contributor
|
✨ Thanks for submitting this pull request, which proposes a way to fix EACCES errors in sandboxes by adding the credentials directory to the writable state layout. |
When the gateway token is empty or revoked, remove previously written marker blocks from .bashrc/.profile to prevent stale credentials from persisting in interactive sessions. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/nemoclaw-start.sh (1)
118-170: Well-structured idempotent token management with proper stale credential cleanup.The function correctly:
- Extracts the gateway token from the verified config file using a safe Python heredoc
- Removes stale marker blocks when the token is empty (lines 133-147), preventing revoked tokens from persisting in interactive sessions
- Persists non-empty tokens idempotently using the same marker pattern as proxy config
The block-removal awk logic appears in both the empty-token path (lines 140-141) and the update path (lines 161-162). Consider extracting this into a small helper function to reduce duplication, similar to
_write_proxy_snippet.
,♻️ Optional: Extract block-removal helper
+_remove_marker_block() { + local file="$1" begin="$2" end="$3" + if [ -f "$file" ] && grep -qF "$begin" "$file" 2>/dev/null; then + local tmp + tmp="$(mktemp)" + awk -v b="$begin" -v e="$end" \ + '$0==b{s=1;next} $0==e{s=0;next} !s' "$file" >"$tmp" + cat "$tmp" >"$file" + rm -f "$tmp" + return 0 + fi + return 1 +} + export_gateway_token() { # ... token extraction ... local marker_begin="# nemoclaw-gateway-token begin" local marker_end="# nemoclaw-gateway-token end" if [ -z "$token" ]; then for rc_file in "${_SANDBOX_HOME}/.bashrc" "${_SANDBOX_HOME}/.profile"; do - if [ -f "$rc_file" ] && grep -qF "$marker_begin" "$rc_file" 2>/dev/null; then - local tmp - tmp="$(mktemp)" - awk -v b="$marker_begin" -v e="$marker_end" \ - '$0==b{s=1;next} $0==e{s=0;next} !s' "$rc_file" >"$tmp" - cat "$tmp" >"$rc_file" - rm -f "$tmp" - fi + _remove_marker_block "$rc_file" "$marker_begin" "$marker_end" || true done return fi # ... rest of function uses _remove_marker_block similarly ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 118 - 170, The duplicate awk-based block-removal logic in export_gateway_token (used in the empty-token branch and the update branch) should be extracted into a small helper (e.g., _remove_marker_block or similar) that accepts an rc file path and the marker_begin/marker_end values; replace both inline copies with calls to that helper and reuse the same pattern used by _write_proxy_snippet to keep behavior identical (create tmp via mktemp, write filtered content, overwrite rc file, and cleanup).
🤖 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 118-170: The duplicate awk-based block-removal logic in
export_gateway_token (used in the empty-token branch and the update branch)
should be extracted into a small helper (e.g., _remove_marker_block or similar)
that accepts an rc file path and the marker_begin/marker_end values; replace
both inline copies with calls to that helper and reuse the same pattern used by
_write_proxy_snippet to keep behavior identical (create tmp via mktemp, write
filtered content, overwrite rc file, and cleanup).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f2f1ef9-d247-44c1-804a-08d1944a33c1
📒 Files selected for processing (1)
scripts/nemoclaw-start.sh
Remove the Dockerfile.base credentials directory changes to avoid overlap with PR NVIDIA#1126 which already addresses that fix. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dongni-Yang
changed the title
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Resolve conflict in scripts/nemoclaw-start.sh: keep both export_gateway_token (this branch) and validate_openclaw_symlinks + harden_openclaw_symlinks (upstream). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/nemoclaw-start.sh (2)
121-129: Consider using context manager for file handling.The inline Python uses
open()without explicit close. While Python's GC will handle it, usingwith open()is the idiomatic pattern and avoids resource warnings in stricter environments.♻️ Suggested improvement
token="$( python3 - <<'PYTOKEN' import json try: - cfg = json.load(open('/sandbox/.openclaw/openclaw.json')) + with open('/sandbox/.openclaw/openclaw.json') as f: + cfg = json.load(f) print(cfg.get('gateway', {}).get('auth', {}).get('token', '')) except Exception: print('') PYTOKEN )"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 121 - 129, The inline Python heredoc that calls json.load(open('/sandbox/.openclaw/openclaw.json')) should use a context manager to ensure the file is closed: replace the direct open(...) usage in the try block inside the python3 - <<'PYTOKEN' heredoc with a with open('/sandbox/.openclaw/openclaw.json') as f: and then call json.load(f) (keeping the existing try/except and print logic intact) so the file is explicitly closed even on errors.
118-129: Token extraction duplicated withprint_dashboard_urls.The Python snippet for reading
gateway.auth.tokenis nearly identical to lines 262-274. If this pattern is used again, consider extracting a shell helper function that returns the token, but this is low priority given only two usage sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 118 - 129, Duplicate Python token-extraction logic appears in export_gateway_token and print_dashboard_urls; create a single helper (e.g., get_gateway_token) that runs the Python snippet once and returns the token, then call that helper from export_gateway_token and print_dashboard_urls to remove the duplicated snippet and centralize token-reading logic.
🤖 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 121-129: The inline Python heredoc that calls
json.load(open('/sandbox/.openclaw/openclaw.json')) should use a context manager
to ensure the file is closed: replace the direct open(...) usage in the try
block inside the python3 - <<'PYTOKEN' heredoc with a with
open('/sandbox/.openclaw/openclaw.json') as f: and then call json.load(f)
(keeping the existing try/except and print logic intact) so the file is
explicitly closed even on errors.
- Around line 118-129: Duplicate Python token-extraction logic appears in
export_gateway_token and print_dashboard_urls; create a single helper (e.g.,
get_gateway_token) that runs the Python snippet once and returns the token, then
call that helper from export_gateway_token and print_dashboard_urls to remove
the duplicated snippet and centralize token-reading logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dfd9554f-2c5a-44f9-a1c1-78e23c876be0
📒 Files selected for processing (1)
scripts/nemoclaw-start.sh
Address CodeRabbit review: use `with open()` context manager and deduplicate the token-reading Python snippet shared by export_gateway_token and print_dashboard_urls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…1114) Running `openclaw configure` inside the sandbox fails with a cryptic EACCES because Landlock enforces read-only on /sandbox/.openclaw/ at the kernel level. The atomic-write temp file lands in the locked directory and is rejected. Install a shell function wrapper in .bashrc/.profile that intercepts `openclaw configure` and prints a clear message directing users to `nemoclaw onboard --resume` on the host instead. Fixes NVIDIA#1114 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dongni-Yang
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/nemoclaw-start.sh (1)
130-173: Please add a regression test for the rc-file marker updates.These new paths rewrite
~/.bashrcand~/.profile, but the providedtest/nemoclaw-start.test.jscoverage still only asserts startup flow and diagnostics. A temp-home test for insert/replace/remove behavior would keep the stale-token cleanup and configure guard from drifting again.Also applies to: 175-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 130 - 173, Add a regression test for export_gateway_token that verifies insert/replace/remove behavior on rc files: create a temp home and set _SANDBOX_HOME to it, create .bashrc and .profile with and without existing marker blocks and varying write permissions, then invoke the export_gateway_token function (or source the script and call export_gateway_token) with _read_gateway_token stubbed to return a token and then to return empty; assert that when a token is returned the marker block is inserted or replaced with the new OPENCLAW_GATEWAY_TOKEN snippet, and when empty any existing marker blocks are removed. Add assertions in test/nemoclaw-start.test.js to cover both branches that use the tmp-file+awk replacement (existing marker present) and the append branch (no marker but writable), ensuring the marker text and token value are exactly as expected and removed on token absence.
🤖 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/nemoclaw-start.sh`:
- Around line 155-158: The snippet currently injects the raw token into a
sourced shell fragment (variables snippet, token, marker_begin, marker_end),
which can break or allow code execution if token contains quotes, dollars,
backticks or newlines; fix by shell-escaping the token before embedding it—e.g.,
produce an escaped_token from token using a safe shell-escaping method (printf
'%q' or base64-encode the value and decode at runtime) and then use that
escaped_token when building snippet between marker_begin and marker_end so the
resulting export OPENCLAW_GATEWAY_TOKEN assignment is always syntactically safe.
- Around line 136-149: The cleanup branch that strips marker blocks (the for
loop using _SANDBOX_HOME and marker_begin/marker_end) fails to remove the token
from the current environment, so children like NEMOCLAW_CMD can still inherit a
stale OPENCLAW_GATEWAY_TOKEN; fix by unsetting OPENCLAW_GATEWAY_TOKEN in that
branch (e.g., call unset OPENCLAW_GATEWAY_TOKEN before the return) so the
variable is removed from the current process environment before spawning any
children.
---
Nitpick comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 130-173: Add a regression test for export_gateway_token that
verifies insert/replace/remove behavior on rc files: create a temp home and set
_SANDBOX_HOME to it, create .bashrc and .profile with and without existing
marker blocks and varying write permissions, then invoke the
export_gateway_token function (or source the script and call
export_gateway_token) with _read_gateway_token stubbed to return a token and
then to return empty; assert that when a token is returned the marker block is
inserted or replaced with the new OPENCLAW_GATEWAY_TOKEN snippet, and when empty
any existing marker blocks are removed. Add assertions in
test/nemoclaw-start.test.js to cover both branches that use the tmp-file+awk
replacement (existing marker present) and the append branch (no marker but
writable), ensuring the marker text and token value are exactly as expected and
removed on token absence.
🪄 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: Pro
Run ID: d76bcc9c-8987-4fdd-af3e-55cd1d9c6a56
📒 Files selected for processing (1)
scripts/nemoclaw-start.sh
Address CodeRabbit review: - Unset stale OPENCLAW_GATEWAY_TOKEN when token is empty so child processes do not inherit a revoked value. - Shell-escape token with single-quote wrapping before embedding in .bashrc/.profile to prevent injection via quotes/dollars/backticks. - Add regression tests for export_gateway_token and install_configure_guard (marker idempotency, escaping, call sites). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The install_configure_guard function contains a heredoc with a nested openclaw() shell function. The previous regex stopped at the first word-character line inside the heredoc. Match up to the next top-level function definition instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
3 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.
Summary
Fixes the four issues reported in #1114 — EACCES permission errors and
missing gateway token when running inside the NemoClaw sandbox.
Issue mapping
EACCES: open '/sandbox/.openclaw/openclaw.json.*.tmp'install_configure_guard— interceptsopenclaw configurewith a clear error and directs users tonemoclaw onboard --resumeon the hostEACCES: mkdir '/sandbox/.openclaw/credentials'.openclaw-data/)Root cause (issues 1 & 2)
OpenClaw's
configurecommand performs atomic writes — it creates a tempfile (
openclaw.json.PID.UUID.tmp) in the same directory as the config.Since
/sandbox/.openclaw/is Landlock read-only at the kernel level,file creation is rejected with EACCES. This is by design: the sandbox
config is intentionally immutable at runtime.
Rather than weakening Landlock (security regression), we intercept the
command in the sandbox shell and guide users to the correct host-side
workflow.
Changes
1.
install_configure_guard()— Writes a shell function wrapper to.bashrc/.profilethat interceptsopenclaw configureand prints:All other
openclawsubcommands pass through to the real binary.2.
export_gateway_token()— Readsgateway.auth.tokenfromopenclaw.jsonand exports it asOPENCLAW_GATEWAY_TOKEN, sointeractive sessions (
openshell sandbox connect) can authenticatewith the gateway. Persists to
.bashrc/.profileusing idempotentmarker blocks and cleans stale tokens on revocation.
3.
_read_gateway_token()helper — Shared Python snippet used byboth
export_gateway_tokenandprint_dashboard_urls(deduplication,uses
with open()context manager).All three are called in both root and non-root startup paths.
Security properties preserved
/sandbox/.openclawremains root-owned, Landlock read-onlyopenclaw.jsonremains chmod 444 (immutable)command openclawbypass preserves all non-configure functionalityFixes #1114
Signed-off-by: Dongni Yang dongniy@nvidia.com
Co-authored-by: Claude Sonnet 4.6 noreply@anthropic.com
Co-authored-by: Claude Opus 4.6 (1M context) noreply@anthropic.com