harden sandbox against agent config tampering (defense-in-depth)#654
harden sandbox against agent config tampering (defense-in-depth)#654BenediktSchackenberg wants to merge 8 commits intoNVIDIA:mainfrom
Conversation
|
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:
📝 WalkthroughWalkthroughMove OpenClaw’s canonical config to an immutable system path ( Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Dockerfile / Image Build
participant Start as nemoclaw-start.sh (init)
participant FS as Filesystem (/etc, /tmp, /sandbox)
participant Gateway as OpenClaw Gateway
participant Agent as Sandbox Agent
rect rgba(200,230,255,0.5)
Build->>FS: create /etc/openclaw/openclaw.json (0444) and set /etc/openclaw (0555)
Build->>FS: create /sandbox/.openclaw/workspace and symlink .openclaw/openclaw.json -> /etc/openclaw/openclaw.json
end
rect rgba(220,255,200,0.5)
Start->>FS: copy /etc/openclaw/openclaw.json -> /tmp/openclaw/openclaw.json (runtime), mutate gateway fields
Start->>FS: update /sandbox/.openclaw/openclaw.json to point at /tmp/openclaw/openclaw.json
Start->>Gateway: start gateway using /tmp/openclaw/openclaw.json
end
rect rgba(255,230,200,0.5)
Agent->>FS: attempt write/edit `/sandbox/.openclaw/openclaw.json` or `/etc/openclaw/...` (blocked by fs perms + tool policy)
Agent->>Gateway: attempt `openclaw gateway ...` or `kill` (blocked by tool policy)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Pull request overview
Adds defense-in-depth hardening to prevent sandboxed agents from tampering with OpenClaw gateway configuration, and documents/automates workspace file backup/restore workflows.
Changes:
- Move
openclaw.jsonto/etc/openclaw/and add a compatibility symlink from/sandbox/.openclaw/openclaw.json. - Add sandbox
tool_policydeny rules for writing/editing the config and for running gateway lifecycle commands. - Add workspace backup/restore script plus new documentation pages and navigation updates.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/nemoclaw-start.sh |
Points config reads/writes at /etc/openclaw/openclaw.json and attempts to lock /sandbox/.openclaw after setup. |
Dockerfile |
Writes config under /etc/openclaw/ and symlinks it into /sandbox/.openclaw/. |
nemoclaw-blueprint/policies/openclaw-sandbox.yaml |
Adds tool-policy deny list for config tampering and gateway lifecycle commands. |
scripts/backup-workspace.sh |
New CLI helper to backup/restore workspace files via openshell sandbox download/upload. |
docs/workspace/workspace-files.md |
New concept doc describing workspace files and persistence characteristics. |
docs/workspace/backup-restore.md |
New how-to doc for manual + scripted workspace backups/restores. |
docs/reference/commands.md |
Adds warning about nemoclaw destroy deleting the PVC and workspace files. |
docs/index.md |
Adds Workspace section cards/toctree entries to docs navigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Dockerfile
Outdated
| @@ -60,9 +61,9 @@ config = { \ | |||
| 'models': [{'id': 'nemotron-3-super-120b-a12b', 'name': 'NVIDIA Nemotron 3 Super 120B', 'reasoning': False, 'input': ['text'], 'cost': {'input': 0, 'output': 0, 'cacheRead': 0, 'cacheWrite': 0}, 'contextWindow': 131072, 'maxTokens': 4096}] \ | |||
| }}} \ | |||
| }; \ | |||
| path = os.path.expanduser('~/.openclaw/openclaw.json'); \ | |||
| json.dump(config, open(path, 'w'), indent=2); \ | |||
| os.chmod(path, 0o600)" | |||
| json.dump(config, open('/etc/openclaw/openclaw.json', 'w'), indent=2); \ | |||
| os.chmod('/etc/openclaw/openclaw.json', 0o444)" \ | |||
| && ln -sf /etc/openclaw/openclaw.json /sandbox/.openclaw/openclaw.json | |||
There was a problem hiding this comment.
This Dockerfile block runs as USER sandbox, but it attempts to mkdir -p /etc/openclaw and write /etc/openclaw/openclaw.json. On a standard Linux filesystem /etc is owned by root (0755), so this will fail at build time unless you temporarily switch back to USER root (or create/chown /etc/openclaw earlier while root) before writing the config, then switch back to USER sandbox afterwards.
There was a problem hiding this comment.
Good catch — moved the entire /etc/openclaw block before USER sandbox. It now runs as root, creates the dir, writes the config, and chmod 555s the directory. Then we switch to sandbox.
scripts/nemoclaw-start.sh
Outdated
| # Config lives in /etc/openclaw/ (read-only mount, outside agent-writable zone). | ||
| # Symlink at ~/.openclaw/openclaw.json points here. See #516 item 2. | ||
| config_path = '/etc/openclaw/openclaw.json' | ||
| os.makedirs(os.path.dirname(config_path), exist_ok=True) |
There was a problem hiding this comment.
Switching config_path to /etc/openclaw/openclaw.json means this function will later try to create /etc/openclaw and rewrite the config under /etc. That conflicts with the stated goal of keeping the config read-only for agents (and with the image/policy typically treating /etc as non-writable), so startup can fail with PermissionError when the config is rewritten. If /etc/openclaw/openclaw.json is meant to be immutable, consider removing the runtime rewrite path and moving these dynamic overrides (e.g. allowedOrigins / token) to a separate writable runtime config location.
There was a problem hiding this comment.
Refactored: fix_openclaw_config now reads the immutable template from /etc/openclaw/ and writes the runtime copy to /tmp/openclaw/. Symlink at ~/.openclaw/openclaw.json points to the /tmp location. /etc stays untouched at runtime.
scripts/nemoclaw-start.sh
Outdated
| # Lock down .openclaw/ so agents can't mess with its structure. | ||
| # Subdirs (agents/, workspace/) stay writable — only the parent gets locked. | ||
| # This prevents creating new files/symlinks in .openclaw/ itself. (#516 item 5) | ||
| if [ "$(id -u)" = "0" ]; then | ||
| chown root:root /sandbox/.openclaw | ||
| chmod 555 /sandbox/.openclaw | ||
| fi |
There was a problem hiding this comment.
This lock-down only runs when id -u is 0, but the container image sets USER sandbox in the Dockerfile. In the typical case this block will never execute, leaving /sandbox/.openclaw/ writable and allowing an agent to replace the openclaw.json symlink (e.g., via exec rm/ln) to point at an attacker-controlled config. Consider enforcing the ownership/mode during image build (as root) or running this entrypoint with sufficient privileges before dropping to the sandbox user.
There was a problem hiding this comment.
Yep, this was dead code — container runs as sandbox so the id -u check never passed. Moved the lock-down into the Dockerfile where it runs as root during build. Removed the block from the start script entirely.
| - "/etc/openclaw/openclaw.json" | ||
| - tool: edit | ||
| paths: | ||
| - "**/.openclaw/openclaw.json" | ||
| - "/etc/openclaw/openclaw.json" |
There was a problem hiding this comment.
This policy keeps /etc under filesystem_policy.read_only, and now the config is also explicitly denied for write/edit. That combination implies /etc/openclaw/openclaw.json must be treated as immutable at runtime; any startup logic that rewrites the config (e.g., to set allowedOrigins from CHAT_UI_URL) will fail once this policy is applied. Please ensure runtime config mutation is removed or moved to a writable location before relying on /etc/openclaw/openclaw.json in production.
| - "/etc/openclaw/openclaw.json" | |
| - tool: edit | |
| paths: | |
| - "**/.openclaw/openclaw.json" | |
| - "/etc/openclaw/openclaw.json" | |
| - tool: edit | |
| paths: | |
| - "**/.openclaw/openclaw.json" |
There was a problem hiding this comment.
This is now consistent — the start script only reads from /etc/openclaw/ (immutable template) and writes runtime config to /tmp/openclaw/ which is in the read_write zone. No more writes to /etc at runtime, so the filesystem_policy and tool_policy work as intended.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Dockerfile (1)
53-66:⚠️ Potential issue | 🔴 CriticalThis
RUNnow requires root and will fail afterUSER sandbox.
mkdir -p /etc/openclawand the write to/etc/openclaw/openclaw.jsonare no longer permitted for the unprivilegedsandboxuser, so the image build breaks here. Move this block beforeUSER sandbox, or bracket it with a temporaryUSER root/USER sandboxswitch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 53 - 66, The Dockerfile RUN step that creates /etc/openclaw and writes /etc/openclaw/openclaw.json runs after the USER sandbox switch and thus fails; move the entire RUN block that creates and writes /etc/openclaw/openclaw.json (the multi-line RUN starting with "mkdir -p /etc/openclaw && python3 -c ...") to appear before the USER sandbox instruction, or wrap it by temporarily switching to root (insert USER root before that RUN and USER sandbox after) so the mkdir, json.dump and chmod operations on /etc/openclaw succeed.scripts/nemoclaw-start.sh (1)
24-57:⚠️ Potential issue | 🟠 MajorDon't re-lock the relocated config to
0600.This function now rewrites the shared config under
/etc, but it still ends by forcing mode0600. Once that file is root-owned, the sandbox user can no longer read it through the compatibility symlink, which undercuts the "readable but not replaceable" design and can breakopenclawinvocations run assandbox.🔧 Suggested fix
with open(config_path, 'w') as f: json.dump(cfg, f, indent=2) -os.chmod(config_path, 0o600) +os.chmod(config_path, 0o444)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/nemoclaw-start.sh` around lines 24 - 57, The script currently always forces os.chmod(config_path, 0o600) which re-locks the relocated /etc/openclaw/openclaw.json and prevents sandbox users from reading via the compatibility symlink; change this so the file mode is not unconditionally overwritten — only set restrictive permissions when creating the file (i.e., when config_path did not exist before), or preserve existing permissions otherwise. Use the existing os.path.exists(config_path) check (and/or introduce a created flag) around the json.dump and call to os.chmod so os.chmod(config_path, 0o600) runs only for newly created files and is skipped when the file already existed.
🧹 Nitpick comments (8)
docs/reference/commands.md (1)
179-180: Format the workspace filenames as code spans.This warning calls out concrete filenames, so they should use inline code formatting for consistency and scanability.
As per coding guidelines, CLI commands, file paths, flags, parameter names, and values must use inline
codeformatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/commands.md` around lines 179 - 180, Update the markdown sentence to format the concrete workspace filenames as inline code spans: wrap SOUL.md, USER.md, IDENTITY.md, AGENTS.md, MEMORY.md (and the phrase "daily memory notes" if it refers to specific file names) with backticks so they render as code; keep the rest of the sentence and the Backup and Restore link unchanged to preserve context and links.docs/workspace/backup-restore.md (3)
28-32: Add an intro sentence here, remove the bold, and punctuate the bullets.This section jumps straight into the list, the items read as sentence fragments, and the first bullet does not need emphasis. LLM pattern detected.
As per coding guidelines, sections use H2 and H3, each starting with an introductory sentence, every sentence must end with a period, and bold is reserved for UI labels, parameter names, and genuine warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/backup-restore.md` around lines 28 - 32, Add a one-sentence intro before the "## When to Back Up" list, remove the bold formatting from the "nemoclaw <name> destroy" item, and ensure each bullet is a complete sentence ending with a period (e.g., "Before running nemoclaw <name> destroy.", "Before major NemoClaw version upgrades.", "Periodically, if you've invested time customizing your agent."). Make the change in the "## When to Back Up" section.
93-98: Make these section lead-ins full sentences.
Verifying a Backupends its intro with a colon even though it introduces a code block, andNext Stepsstarts with links instead of a short intro sentence.As per coding guidelines, sections use H2 and H3, each starting with an introductory sentence, every sentence must end with a period, and colons should only introduce a list.
Also applies to: 107-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/backup-restore.md` around lines 93 - 98, Change the two section lead-ins so they are full sentences: for the "Verifying a Backup" H2 replace the trailing colon before the code block with a complete sentence (e.g., "List backed-up files to confirm completeness.") and ensure it ends with a period, and for the "Next Steps" H2 add a short introductory sentence before the links (e.g., "Follow these next steps to complete the restore process.") ensuring every sentence ends with a period; apply the same treatment to the other instance mentioned (lines 107-110) so all H2/H3 lead-ins are full sentences and do not use colons to introduce the code block or links.
71-77: Give these H3 sections real lead-in sentences.
Backupgoes straight into a code block, and theRestorelead-ins are fragments ending with colons rather than full sentences.As per coding guidelines, sections use H2 and H3, each starting with an introductory sentence, every sentence must end with a period, and colons should only introduce a list.
Also applies to: 79-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/backup-restore.md` around lines 71 - 77, The H3 sections titled "Backup" and "Restore" need full lead-in sentences before their code blocks or lists: add a one-sentence introduction (e.g., "Backup saves a copy of a workspace to a timestamped directory.") immediately after the "Backup" heading and similarly a complete sentence before the "Restore" heading and before any subsequent examples or lists; ensure every sentence ends with a period and remove trailing colons unless they introduce an actual list. Reference the "Backup" and "Restore" headings and their surrounding example blocks when making the edits.docs/index.md (1)
171-171: Use code formatting for the file names in this card.
SOUL.mdandUSER.mdare file names here, so keep them in inline code spans.As per coding guidelines, CLI commands, file paths, flags, parameter names, and values must use inline
codeformatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.md` at line 171, Update the sentence in docs/index.md so that the workspace file names are formatted as inline code: replace plain text SOUL.md and USER.md with `SOUL.md` and `USER.md` (and apply inline `code` formatting to any other file names, CLI commands, paths, flags or parameter names in that line) to follow the project's inline code formatting guideline.docs/workspace/workspace-files.md (3)
57-64: Remove the colons from these H3 titles.Both headings violate the docs title style.
As per coding guidelines, no colons in titles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/workspace-files.md` around lines 57 - 64, Update the two H3 headings "Survives: Sandbox Restart" and "Lost: Sandbox Destroy" to remove the colons so they follow docs title style (e.g., change to "Survives Sandbox Restart" and "Lost Sandbox Destroy"); locate the headings in the workspace-files.md content and edit the heading lines only, preserving the surrounding text and formatting for the PVC explanation and sandbox destroy sentence.
26-35: Add a lead-in sentence before this table.
## File Referencecurrently drops straight into the table, but new docs pages require each H2/H3 section to start with a short intro sentence.As per coding guidelines, sections use H2 and H3, each starting with an introductory sentence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/workspace-files.md` around lines 26 - 35, The "## File Reference" section currently jumps straight into the table; add a short lead-in sentence immediately below the "## File Reference" heading that briefly describes what the table lists (e.g., that it summarizes key workspace files and their purposes) so the section follows the documentation guideline requiring each H2/H3 to start with an introductory sentence; update the content around the existing table referencing the same heading and entries like `SOUL.md`, `USER.md`, `IDENTITY.md`, `AGENTS.md`, `MEMORY.md`, and `memory/`.
77-78: Drop the bold labels and em dashes in this numbered list.These callouts read like generated prose. LLM pattern detected.
As per coding guidelines, bold is reserved for UI labels, parameter names, and genuine warnings, and excessive or routine em dashes should be flagged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workspace/workspace-files.md` around lines 77 - 78, Remove the bold formatting and em dashes from the numbered list items that read "**Let the agent do it** — Ask your agent..." and "**Edit manually** — Use `openshell sandbox shell`..."; replace them with plain text labels and punctuation (for example: "Let the agent do it: Ask your agent to update..." and "Edit manually: Use `openshell sandbox shell` to open...") so the labels are not bold and the em dashes are removed.
🤖 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/workspace/workspace-files.md`:
- Around line 57-61: The sentence mentions the command "nemoclaw <name> restart"
that isn't documented elsewhere; either add documentation for the "nemoclaw
<name> restart" command in the commands list (describe usage and behavior
consistent with "openshell sandbox restart") or remove the phrase from the
"Survives: Sandbox Restart" paragraph; update references in workspace-files.md
and ensure the command string "nemoclaw <name> restart" is either documented or
removed to keep docs consistent.
In `@scripts/backup-workspace.sh`:
- Around line 75-79: The current command substitution assigning ts from ls can
fail under set -euo pipefail if BACKUP_BASE doesn't exist; add an explicit check
for the directory's existence before running the ls substitution: verify [ -d
"$BACKUP_BASE" ] (or equivalent) and call die "No backups found in
${BACKUP_BASE}/" if it doesn't exist, otherwise perform the ls | sort -r | head
-n1 assignment to ts and then keep the existing [ -n "$ts" ] || die and echo
"Using most recent backup: ${ts}". This ensures the failing ls won't abort the
script unexpectedly and preserves the existing die/error behavior.
In `@scripts/nemoclaw-start.sh`:
- Around line 183-189: The script locks /sandbox/.openclaw to 0555 which
prevents creating new top-level entries; ensure the workspace directory is
created (mkdir -p /sandbox/.openclaw/workspace) and its ownership/permissions
set appropriately (e.g., chown to the sandbox user and writable mode) before the
chown/chmod 555 lines so first-run workspace initialization can succeed;
reference the existing chown/chmod operations and the agents/ and workspace/
directories when making the change.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 53-66: The Dockerfile RUN step that creates /etc/openclaw and
writes /etc/openclaw/openclaw.json runs after the USER sandbox switch and thus
fails; move the entire RUN block that creates and writes
/etc/openclaw/openclaw.json (the multi-line RUN starting with "mkdir -p
/etc/openclaw && python3 -c ...") to appear before the USER sandbox instruction,
or wrap it by temporarily switching to root (insert USER root before that RUN
and USER sandbox after) so the mkdir, json.dump and chmod operations on
/etc/openclaw succeed.
In `@scripts/nemoclaw-start.sh`:
- Around line 24-57: The script currently always forces os.chmod(config_path,
0o600) which re-locks the relocated /etc/openclaw/openclaw.json and prevents
sandbox users from reading via the compatibility symlink; change this so the
file mode is not unconditionally overwritten — only set restrictive permissions
when creating the file (i.e., when config_path did not exist before), or
preserve existing permissions otherwise. Use the existing
os.path.exists(config_path) check (and/or introduce a created flag) around the
json.dump and call to os.chmod so os.chmod(config_path, 0o600) runs only for
newly created files and is skipped when the file already existed.
---
Nitpick comments:
In `@docs/index.md`:
- Line 171: Update the sentence in docs/index.md so that the workspace file
names are formatted as inline code: replace plain text SOUL.md and USER.md with
`SOUL.md` and `USER.md` (and apply inline `code` formatting to any other file
names, CLI commands, paths, flags or parameter names in that line) to follow the
project's inline code formatting guideline.
In `@docs/reference/commands.md`:
- Around line 179-180: Update the markdown sentence to format the concrete
workspace filenames as inline code spans: wrap SOUL.md, USER.md, IDENTITY.md,
AGENTS.md, MEMORY.md (and the phrase "daily memory notes" if it refers to
specific file names) with backticks so they render as code; keep the rest of the
sentence and the Backup and Restore link unchanged to preserve context and
links.
In `@docs/workspace/backup-restore.md`:
- Around line 28-32: Add a one-sentence intro before the "## When to Back Up"
list, remove the bold formatting from the "nemoclaw <name> destroy" item, and
ensure each bullet is a complete sentence ending with a period (e.g., "Before
running nemoclaw <name> destroy.", "Before major NemoClaw version upgrades.",
"Periodically, if you've invested time customizing your agent."). Make the
change in the "## When to Back Up" section.
- Around line 93-98: Change the two section lead-ins so they are full sentences:
for the "Verifying a Backup" H2 replace the trailing colon before the code block
with a complete sentence (e.g., "List backed-up files to confirm completeness.")
and ensure it ends with a period, and for the "Next Steps" H2 add a short
introductory sentence before the links (e.g., "Follow these next steps to
complete the restore process.") ensuring every sentence ends with a period;
apply the same treatment to the other instance mentioned (lines 107-110) so all
H2/H3 lead-ins are full sentences and do not use colons to introduce the code
block or links.
- Around line 71-77: The H3 sections titled "Backup" and "Restore" need full
lead-in sentences before their code blocks or lists: add a one-sentence
introduction (e.g., "Backup saves a copy of a workspace to a timestamped
directory.") immediately after the "Backup" heading and similarly a complete
sentence before the "Restore" heading and before any subsequent examples or
lists; ensure every sentence ends with a period and remove trailing colons
unless they introduce an actual list. Reference the "Backup" and "Restore"
headings and their surrounding example blocks when making the edits.
In `@docs/workspace/workspace-files.md`:
- Around line 57-64: Update the two H3 headings "Survives: Sandbox Restart" and
"Lost: Sandbox Destroy" to remove the colons so they follow docs title style
(e.g., change to "Survives Sandbox Restart" and "Lost Sandbox Destroy"); locate
the headings in the workspace-files.md content and edit the heading lines only,
preserving the surrounding text and formatting for the PVC explanation and
sandbox destroy sentence.
- Around line 26-35: The "## File Reference" section currently jumps straight
into the table; add a short lead-in sentence immediately below the "## File
Reference" heading that briefly describes what the table lists (e.g., that it
summarizes key workspace files and their purposes) so the section follows the
documentation guideline requiring each H2/H3 to start with an introductory
sentence; update the content around the existing table referencing the same
heading and entries like `SOUL.md`, `USER.md`, `IDENTITY.md`, `AGENTS.md`,
`MEMORY.md`, and `memory/`.
- Around line 77-78: Remove the bold formatting and em dashes from the numbered
list items that read "**Let the agent do it** — Ask your agent..." and "**Edit
manually** — Use `openshell sandbox shell`..."; replace them with plain text
labels and punctuation (for example: "Let the agent do it: Ask your agent to
update..." and "Edit manually: Use `openshell sandbox shell` to open...") so the
labels are not bold and the em dashes are removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80eba33e-e2d6-440a-ae9e-c55cb58b3ddd
📒 Files selected for processing (8)
Dockerfiledocs/index.mddocs/reference/commands.mddocs/workspace/backup-restore.mddocs/workspace/workspace-files.mdnemoclaw-blueprint/policies/openclaw-sandbox.yamlscripts/backup-workspace.shscripts/nemoclaw-start.sh
- Remove undocumented 'nemoclaw <name> restart' from workspace-files.md - Add existence check for BACKUP_BASE before listing backups - Pre-create workspace/ directory in Dockerfile before 555 lock
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Dockerfile (1)
44-46:⚠️ Potential issue | 🟠 MajorCreate the writable children as
sandboxbefore locking.openclaw.This still leaves the earlier
workspace/issue unresolved, andmkdir -pruns as root here, so/sandbox/.openclaw/agents/main/agentends up root-owned too. After Line 70 locks the parent to0555, the sandbox user can no longer createworkspace/or writeauth-profiles.jsonunderagents/main/agent, so first-run workspace setup andwrite_auth_profile()can fail.🔧 Suggested fix
-RUN mkdir -p /sandbox/.openclaw/agents/main/agent \ - && chmod 700 /sandbox/.openclaw +RUN install -d -o sandbox -g sandbox -m 755 \ + /sandbox/.openclaw/agents/main/agent \ + /sandbox/.openclaw/workspace \ + && chmod 700 /sandbox/.openclaw🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 44 - 46, Pre-create the writable child directories as the sandbox user before making /sandbox/.openclaw read-only: ensure the Dockerfile creates and/or chowns the specific writable paths (e.g., /sandbox/.openclaw/agents/main/agent and its workspace/ and auth-profiles.json parent) so they are owned by the sandbox user and writable by that user, then apply the restrictive chmod 0555 to the .openclaw parent; update the RUN sequence around the mkdir -p /sandbox/.openclaw/agents/main/agent and the later chmod 0555 to either switch to USER sandbox (or use chown sandbox:sandbox) for creating and populating workspace/ and the agent directory so write_auth_profile() and first-run workspace setup succeed.
🤖 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 25-30: The sandbox policy currently blocks edits to the template
and symlink but misses the live runtime file referenced by runtime_path (set
from template_path and runtime_dir); update the openclaw-sandbox.yaml deny rules
to include the runtime_path target (the live /tmp openclaw config) in both the
deny → write and deny → edit sections so agents cannot write or edit the live
runtime config directly.
---
Duplicate comments:
In `@Dockerfile`:
- Around line 44-46: Pre-create the writable child directories as the sandbox
user before making /sandbox/.openclaw read-only: ensure the Dockerfile creates
and/or chowns the specific writable paths (e.g.,
/sandbox/.openclaw/agents/main/agent and its workspace/ and auth-profiles.json
parent) so they are owned by the sandbox user and writable by that user, then
apply the restrictive chmod 0555 to the .openclaw parent; update the RUN
sequence around the mkdir -p /sandbox/.openclaw/agents/main/agent and the later
chmod 0555 to either switch to USER sandbox (or use chown sandbox:sandbox) for
creating and populating workspace/ and the agent directory so
write_auth_profile() and first-run workspace setup succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 164ee619-88cc-47a2-98bd-2c385b4bfd07
📒 Files selected for processing (2)
Dockerfilescripts/nemoclaw-start.sh
- Add docs/workspace/workspace-files.md explaining SOUL.md, USER.md, IDENTITY.md, AGENTS.md, MEMORY.md and their persistence behavior - Add docs/workspace/backup-restore.md with manual and scripted backup instructions - Add scripts/backup-workspace.sh for automated backup and restore - Add warning to 'nemoclaw destroy' command docs about data loss - Add Workspace section to docs index and navigation Closes NVIDIA#434 AI-assisted: Built with Claude, reviewed by human.
- Make page title generic ('Workspace Files' without enumeration)
- Fix 'Gateway Destroy' header to 'Sandbox Destroy'
- Convert admonitions to MyST syntax (:::{warning}/:::)
- Add language specifier to code block (```text)
- Secure backup base directory with mkdir -m 0700
- Fix sample output to match script output format
- Use rm -rf instead of rmdir for partial backup cleanup
- Add error handling to restore uploads
…ck parent dir defense-in-depth on top of the fs permission fix from NVIDIA#514: - add tool_policy deny rules for write/edit on openclaw.json and exec on gateway lifecycle commands (item 1) - move openclaw.json to /etc/openclaw/ with symlink, so it's outside the agent-writable /sandbox/ tree (item 2) - lock /sandbox/.openclaw/ to 555 root:root after setup, keeping agents/ and workspace/ writable (item 5) - document landlock enforce and auth defaults as future work (items 3, 4) closes NVIDIA#516 AI-assisted: Built with Claude, reviewed by human.
- Dockerfile: create /etc/openclaw and lock it down as root before switching to USER sandbox (fixes mkdir permission issue) - nemoclaw-start.sh: read immutable template from /etc/openclaw/, write runtime config to /tmp/openclaw/ (symlinked from ~/.openclaw/) - Move .openclaw/ lock-down from start script to Dockerfile (was unreachable since container runs as sandbox, not root) - Consistent with sandbox.yaml filesystem_policy: /etc is read_only, /tmp is read_write
- Remove undocumented 'nemoclaw <name> restart' from workspace-files.md - Add existence check for BACKUP_BASE before listing backups - Pre-create workspace/ directory in Dockerfile before 555 lock
BenediktSchackenberg
force-pushed
the
fix/sandbox-config-hardening
branch
from
29ac903 to
b62bde9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 143-146: The parent config directory is left world-writable (mode
1777) which breaks hardening; update the Dockerfile steps around the RUN ln -sf
/etc/openclaw/openclaw.json /sandbox/.openclaw/openclaw.json and the directory
creation that sets mode 1777 so the parent directory is non-writable (e.g., 0555
or 0755) and create a separate mutable runtime subdirectory (e.g.,
.openclaw/runtime) with the sticky/world-writable mode for ephemeral runtime
writes; ensure nemoclaw-start.sh continues to replace the symlink to the mutable
runtime path and adjust the Dockerfile sequence to create dirs, set strict mode
on the parent, then create the writable runtime subdir before creating the
symlink.
In `@docs/index.md`:
- Line 173: Wrap the file names in the sentence "Understand SOUL.md, USER.md,
and other workspace files, plus backup and restore." using inline code
formatting: replace SOUL.md and USER.md with `SOUL.md` and `USER.md` (and apply
inline code to any other workspace file names mentioned) in docs/index.md so
file paths follow the project's inline `code` style.
In `@nemoclaw-blueprint/policies/openclaw-sandbox.yaml`:
- Around line 59-65: The policy currently lists specific "commands" under the
block with tool: exec which OpenClaw does not enforce; update the policy to
either deny the exec tool entirely by adding exec to the tools.*.deny list
(i.e., reference the tool name "exec") or switch to security=allowlist mode and
configure explicit path-based allowlisting for binaries you want to permit (use
OpenClaw's tools.exec.allowlist or security=allowlist settings), and remove the
unsupported "commands" array so the intent is enforced via tools.*.deny or the
allowlist configuration instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b3bc5ad-d75e-4a8a-9344-1bc1c5f4654a
📒 Files selected for processing (8)
Dockerfiledocs/index.mddocs/reference/commands.mddocs/workspace/backup-restore.mddocs/workspace/workspace-files.mdnemoclaw-blueprint/policies/openclaw-sandbox.yamlscripts/backup-workspace.shscripts/nemoclaw-start.sh
✅ Files skipped from review due to trivial changes (3)
- docs/reference/commands.md
- docs/workspace/workspace-files.md
- docs/workspace/backup-restore.md
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/nemoclaw-start.sh
- scripts/backup-workspace.sh
…e exec commands deny
Follow-up to #514 (filesystem permission lock). This adds more layers so an agent can't tamper with its own config even if it finds a way around file permissions.
What changed:
Tool policy deny list —
write/editonopenclaw.jsonandexecon gateway lifecycle commands are now explicitly blocked in the sandbox policy. Belt-and-suspenders with the fs lock.Config moved to /etc/openclaw/ —
openclaw.jsonnow lives outside the agent-writable/sandbox/tree entirely. A symlink keeps backward compat. The agent can read it but can't replace it or the symlink.Parent directory locked —
/sandbox/.openclaw/is set toroot:root 555after setup. Subdirs (agents/,workspace/) stay writable. This prevents creating new files or replacing the symlink in the parent.Intentionally NOT done:
Fixes #516
AI-assisted: Built with Claude, reviewed by human.
Summary by CodeRabbit
New Features
Security / Behavior
Documentation