docs(security): clarify mutable-default config posture - #3407
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThis PR updates documentation and code comments across NemoClaw to clarify sandbox filesystem policies, introduce shields commands for host-side config lockdown, and explain how ChangesNemoClaw filesystem policy and shields command documentation
Sequence Diagram(s)sequenceDiagram
participant Host
participant Onboarding as nemoclaw onboard
participant Sandbox
participant Config as openclaw.json
Host->>Onboarding: timeout/heartbeat/inference settings
Onboarding->>Config: generate at sandbox creation
Sandbox->>Config: writable for runtime state
Note over Sandbox,Config: in-sandbox edits not supported
Host->>Onboarding: nemoclaw onboard --resume
Onboarding->>Config: regenerate with new values
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryPi Semantic E2E AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/security/best-practices.md (1)
265-266: ⚡ Quick winSplit independent clauses into separate sentences.
Line 265 uses a semicolon to join two independent clauses.
The docs style guide requires periods to separate sentences.As per coding guidelines, "Colons should only introduce a list. Flag colons used as general punctuation between clauses."
The same principle applies to semicolons.Suggested revision
-Landlock itself stays fixed at sandbox creation; `shields up` does not rewrite the Landlock policy. +Landlock itself stays fixed at sandbox creation. +`shields up` does not rewrite the Landlock policy.🤖 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 `@docs/security/best-practices.md` around lines 265 - 266, The sentence "Landlock itself stays fixed at sandbox creation; `shields up` does not rewrite the Landlock policy." joins two independent clauses with a semicolon — split them into two sentences by replacing the semicolon with a period and capitalizing "`shields up`" so it reads "Landlock itself stays fixed at sandbox creation. `Shields up` does not rewrite the Landlock policy." Ensure the revised wording follows the docs style guide about using periods instead of semicolons and avoiding colons/semicolons as general punctuation.docs/deployment/sandbox-hardening.md (1)
115-115: ⚡ Quick winSplit independent clauses into separate sentences.
Line 115 uses a semicolon to join two independent clauses.
The docs style guide requires periods to separate sentences.Suggested revision
-That host-side command layers root ownership, read-only modes, and the immutable flag where available; it does not change the Landlock policy after sandbox creation. +That host-side command layers root ownership, read-only modes, and the immutable flag where available. +It does not change the Landlock policy after sandbox creation.As per coding guidelines, semicolons should not be used as general punctuation between clauses in NemoClaw documentation.
🤖 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 `@docs/deployment/sandbox-hardening.md` at line 115, The sentence "That host-side command layers root ownership, read-only modes, and the immutable flag where available; it does not change the Landlock policy after sandbox creation." uses a semicolon between two independent clauses—replace the semicolon with a period and capitalize the start of the second clause so it becomes two sentences (e.g., end the first clause after "available." and start "It does not change the Landlock policy after sandbox creation.")..agents/skills/nemoclaw-user-configure-inference/references/switch-inference-providers.md (1)
152-153: ⚡ Quick winUse active voice and separate sentences.
Line 152 uses passive voice ("are overwritten by").
The docs style guide requires active voice.
Additionally, line 153 contains two sentences on the same line.Suggested revision
-NemoClaw-managed build-time defaults, and direct file edits are overwritten by a -rebuild. Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value. +NemoClaw-managed build-time defaults, and rebuilding overwrites direct file edits. +Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value.As per coding guidelines, active voice is required in NemoClaw documentation, and each sentence must appear on its own line.
🤖 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 @.agents/skills/nemoclaw-user-configure-inference/references/switch-inference-providers.md around lines 152 - 153, Rewrite the two-sentence fragment that currently reads "NemoClaw-managed build-time defaults, and direct file edits are overwritten by a rebuild. Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value." into active voice and put each sentence on its own line; e.g., change the passive clause "are overwritten by a rebuild" to an active form such as "A rebuild overwrites NemoClaw-managed build-time defaults and direct file edits." and then place "Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value." on the following line so each sentence is separate.docs/inference/switch-inference-providers.md (1)
176-177: ⚡ Quick winUse active voice and separate sentences.
Line 176 uses passive voice ("are overwritten by").
The docs style guide requires active voice.
Additionally, line 177 contains two sentences on the same line.Suggested revision
-NemoClaw-managed build-time defaults, and direct file edits are overwritten by a -rebuild. Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value. +NemoClaw-managed build-time defaults, and rebuilding overwrites direct file edits. +Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value.As per coding guidelines, active voice is required in NemoClaw documentation, and each sentence must appear on its own line.
🤖 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 `@docs/inference/switch-inference-providers.md` around lines 176 - 177, The current sentence "NemoClaw-managed build-time defaults, and direct file edits are overwritten by a rebuild. Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value." uses passive voice and combines multiple sentences on one line; rewrite it in active voice and put each sentence on its own line. For example, replace it with an active sentence such as "A rebuild overwrites NemoClaw-managed build-time defaults and direct file edits." on one line, and keep the second instruction "Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new value." on the next line so each sentence stands alone.
🤖 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
@.agents/skills/nemoclaw-user-configure-inference/references/switch-inference-providers.md:
- Around line 152-153: Rewrite the two-sentence fragment that currently reads
"NemoClaw-managed build-time defaults, and direct file edits are overwritten by
a rebuild. Rebuild the sandbox via `nemoclaw onboard --resume` to apply a new
value." into active voice and put each sentence on its own line; e.g., change
the passive clause "are overwritten by a rebuild" to an active form such as "A
rebuild overwrites NemoClaw-managed build-time defaults and direct file edits."
and then place "Rebuild the sandbox via `nemoclaw onboard --resume` to apply a
new value." on the following line so each sentence is separate.
In `@docs/deployment/sandbox-hardening.md`:
- Line 115: The sentence "That host-side command layers root ownership,
read-only modes, and the immutable flag where available; it does not change the
Landlock policy after sandbox creation." uses a semicolon between two
independent clauses—replace the semicolon with a period and capitalize the start
of the second clause so it becomes two sentences (e.g., end the first clause
after "available." and start "It does not change the Landlock policy after
sandbox creation.").
In `@docs/inference/switch-inference-providers.md`:
- Around line 176-177: The current sentence "NemoClaw-managed build-time
defaults, and direct file edits are overwritten by a rebuild. Rebuild the
sandbox via `nemoclaw onboard --resume` to apply a new value." uses passive
voice and combines multiple sentences on one line; rewrite it in active voice
and put each sentence on its own line. For example, replace it with an active
sentence such as "A rebuild overwrites NemoClaw-managed build-time defaults and
direct file edits." on one line, and keep the second instruction "Rebuild the
sandbox via `nemoclaw onboard --resume` to apply a new value." on the next line
so each sentence stands alone.
In `@docs/security/best-practices.md`:
- Around line 265-266: The sentence "Landlock itself stays fixed at sandbox
creation; `shields up` does not rewrite the Landlock policy." joins two
independent clauses with a semicolon — split them into two sentences by
replacing the semicolon with a period and capitalizing "`shields up`" so it
reads "Landlock itself stays fixed at sandbox creation. `Shields up` does not
rewrite the Landlock policy." Ensure the revised wording follows the docs style
guide about using periods instead of semicolons and avoiding colons/semicolons
as general punctuation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e797cd43-89cd-4116-929e-64b396d4bb6a
📒 Files selected for processing (14)
.agents/skills/nemoclaw-user-configure-inference/references/switch-inference-providers.md.agents/skills/nemoclaw-user-configure-security/references/best-practices.md.agents/skills/nemoclaw-user-deploy-remote/references/sandbox-hardening.md.agents/skills/nemoclaw-user-reference/references/troubleshooting.mdagents/hermes/Dockerfile.basedocs/deployment/sandbox-hardening.mddocs/inference/switch-inference-providers.mddocs/reference/troubleshooting.mddocs/security/best-practices.mdscripts/nemoclaw-start.shsrc/lib/actions/root-help.tstest/e2e-gateway-isolation.shtest/e2e/test-sandbox-survival.shtest/root-help.test.ts
Contributor
7 tasks
miyoungc
pushed a commit
that referenced
this pull request
May 28, 2026
…4296) ## Summary The sandbox-hardening filesystem-access table listed `/sandbox/.nemoclaw` as read-write, matching the Landlock policy. In a default sandbox the parent directory is `root:root` mode `1755`, so the sandbox user cannot create files directly under it — reporters observed `touch /sandbox/.nemoclaw/foo` returning "Permission denied" and read the table as wrong. This PR clarifies the row and adds a note that the Access column reflects Landlock only. ## Related Issue Fixes #3690 ## Changes - `docs/deployment/sandbox-hardening.mdx`: rewrite the `/sandbox/.nemoclaw` row to call out the DAC restriction and list the actually writable children (`state/`, `migration/`, `snapshots/`, `staging/`, `config.json`). - Add a paragraph below the table explaining that Landlock declaration is one of two gates; POSIX ownership/permissions additionally control actual write success. ## Type of Change - [x] Doc only (prose changes, no code sample modifications) ## Verification - [x] `npx prek run --files docs/deployment/sandbox-hardening.mdx` passes (full `--all-files` blocked by unrelated `src/ext/wechat/login.test.ts` tsc miss on dist; pre-push pushed with `--no-verify`) - [ ] `npm test` — not run; docs-only - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings — not run locally; relying on docs-to-skills prek hook (passed) and CI - [x] Doc pages follow the style guide The three already-stale rows in the table (`/sandbox`, `/sandbox/.openclaw`, `/sandbox/.openclaw-data`) called out in #3690 were corrected in prior PRs (#3407 mutable-default posture, #3548 fern migration); this PR addresses the remaining row. The reporter's "doc never mentions shields up/down" concern is intentionally not addressed here: shields are experimental and excluded from user-facing docs per `docs/.docs-skip`. --- Signed-off-by: Tony Luo <xialuo@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified sandbox filesystem layout and permission semantics: explicitly lists which sandbox subpaths are writable, notes parent/blueprint directories remain root-owned, explains that the access column reflects policy declarations (Landlock) while actual writes still require POSIX/DAC permissions, and adds an example showing direct file creation may still fail. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4296?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Tony Luo <xialuo@nvidia.com>
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4296?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
wscurran
added
area: docs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
shields upadds host-side DAC/chattr lockdown for sensitive workloadsopenclaw.jsonis not described as immutable in the default sandbox/sandbox is Landlock read-onlycomments from startup/test/Hermes scaffoldingValidation
npx vitest run test/root-help.test.tsnpm run build:clinpm run typecheck:clipython3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --dry-runbash test/e2e/e2e-cloud-experimental/check-docs.sh --only-links --local-only docs/security/best-practices.md docs/deployment/sandbox-hardening.md docs/inference/switch-inference-providers.md docs/reference/troubleshooting.mduv run --group docs sphinx-build -W -b html docs docs/_build/htmlnode bin/nemoclaw.js help | rg -n 'Reconfiguration|Agent config|host-side|shields up|read-only|Landlock enforced'Issue Notes
/sandboxand.openclawwrites are expected in mutable-default mode; this PR fixes the stale docs/root-help wording that said otherwise.shields up / shields down / shields statuscommands #3230 by documenting shields commands and the mutable-default vs lockdown split; it does not add the full runtime mutability matrix requested there.Summary by CodeRabbit
Documentation
/sandboxdirectories remain writable by design and do not indicate Landlock failures.nemoclaw onboardfor proper reconfiguration.shields up/down/statuscommands for host-side config lockdown without recreating the sandbox.Tests