fix(sandbox): keep sandbox root read-only (Fixes #2394)

[deepujain]

Summary

The sandbox image made /sandbox writable by the sandbox user, so touch /sandbox/testfile could succeed even though the sandbox root should stay read-only for normal agent use.

This keeps /sandbox owned by root with mode 0755, while preserving writable state directories that the sandbox still needs.

Changes

• Dockerfile.base: stop recursively handing /sandbox to the sandbox user; keep the root directory owned by root and writable only by root.
• Dockerfile: re-apply the root ownership/mode in the production image for compatibility with older base images.
• test/sandbox-root-permissions.test.ts: add static coverage so the image recipes do not regress back to a writable /sandbox root.

Testing

• npm run build:cli passed
• npm run typecheck:cli passed
• npm test -- test/sandbox-root-permissions.test.ts passed
• npm test was also attempted. The full suite is not green on current main in this environment; failures are in existing installer/onboard/legacy-guard tests outside this Dockerfile permission change.

Evidence it works

The new test checks that both image recipes set /sandbox to root:root and 0755, and that the base image no longer recursively assigns /sandbox itself to the sandbox user.

Fixes #2394

Signed-off-by: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

• Chores

  • Tightened sandbox directory access controls in container images by enforcing stricter top-level ownership and permissions while preserving writable subpaths, improving security and consistency across environments.

• Tests

  • Added automated checks to verify sandbox directory ownership and permission configuration are applied as intended.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 549f3f96-735f-4429-b4f8-167526d64447

📥 Commits

Reviewing files that changed from the base of the PR and between 3c57961 and 7edb864.

📒 Files selected for processing (3)

• Dockerfile
• Dockerfile.base
• test/sandbox-root-permissions.test.ts

✅ Files skipped from review due to trivial changes (1)

• test/sandbox-root-permissions.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• Dockerfile.base
• Dockerfile

---

📝 Walkthrough

Walkthrough

The PR hardens the top-level /sandbox directory by making it root-owned with mode 755 in both base and production Dockerfiles, while preserving sandbox ownership for specific subdirectories. It also adds a Vitest that asserts these ownership and permission changes in both Dockerfiles.

Changes

Cohort / File(s)	Summary
Docker base image changes Dockerfile.base	Set /sandbox to root:root and chmod 755 /sandbox instead of recursively chowning to sandbox; restore sandbox:sandbox ownership for .nemoclaw only.
Docker production image changes Dockerfile	Add explicit chown root:root /sandbox and chmod 755 /sandbox build steps to enforce root ownership and 755 permissions at build time.
Permission verification tests test/sandbox-root-permissions.test.ts	New Vitest file that loads both Dockerfiles and asserts presence/absence of chown -R sandbox:sandbox /sandbox, and presence of chown root:root /sandbox and chmod 755 /sandbox directives.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nibbled lines in Docker’s lair,

root locks the sandbox with careful care,
Subfolders safe for sandbox to keep,
No stray write can quietly creep,
Hops of joy — secure and neat! 🥕🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(sandbox): keep sandbox root read-only' accurately describes the primary change—ensuring /sandbox is read-only by enforcing root ownership and permissions.
Linked Issues check	✅ Passed	The PR implements all coding requirements from #2394: Dockerfile.base and Dockerfile now set /sandbox to root:root with mode 755, blocking sandbox user writes, and a test validates this configuration persists.
Out of Scope Changes check	✅ Passed	All changes directly address the objective of making /sandbox read-only; no unrelated modifications were introduced beyond the scope of #2394.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/sandbox-root-permissions.test.ts`:
- Around line 20-21: The current assertions use substring checks on the
dockerfile variable which allow false positives (e.g., matching "chown root:root
/sandbox/.openclaw"); update the expectations to assert the exact command lines
using anchors or equality: replace expect(dockerfile).toContain("chown root:root
/sandbox") and the chmod check with regex or string-equality assertions that
match the full command (for example use
expect(dockerfile).toMatch(/^RUN\s+chown\s+root:root\s+\/sandbox$/m) and
expect(dockerfile).toMatch(/^RUN\s+chmod\s+755\s+\/sandbox$/m)) so the tests
require the exact relock commands in the dockerfile variable.

🪄 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: 69b3ad0e-f630-4cd8-b7df-22dec581ae82

📥 Commits

Reviewing files that changed from the base of the PR and between dc02013 and 3c57961.

📒 Files selected for processing (3)

• Dockerfile
• Dockerfile.base
• test/sandbox-root-permissions.test.ts

[deepujain]

Tightened the permission regression test so it matches the exact /sandbox relock commands instead of broad substrings. Reran npm test -- test/sandbox-root-permissions.test.ts.