fix(image): default OpenClaw sandbox to non-root - #7890
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:
📝 WalkthroughWalkthroughThe Docker image now defaults to ChangesNon-root container execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Docker
participant nemoclaw_start
participant E2E_Runtime
participant Workflow_Validator
Docker->>nemoclaw_start: Start image as sandbox
nemoclaw_start->>E2E_Runtime: Run non-root smoke checks
Docker->>nemoclaw_start: Start image with --user root
nemoclaw_start->>E2E_Runtime: Emit root mode and run command as sandbox
Workflow_Validator->>E2E_Runtime: Validate matrix-specific entrypoint expectation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
Updated the root-path coverage, OpenClaw security-posture assertion, and UID-isolation documentation. The production-image smoke and gateway-isolation suites pass. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/security/best-practices.mdx`:
- Around line 578-584: Update the security guidance near the “Never run as root”
recommendation to distinguish OpenShell’s default sandbox user from the explicit
direct-runtime root override. State that OpenShell should use sandbox by
default, while direct runtimes may use root only when separate gateway and agent
UIDs are required, keeping the surrounding security recommendation consistent.
🪄 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: 2d81b3c3-dd44-47bb-afbe-6a86dbdcd625
📒 Files selected for processing (7)
.github/workflows/e2e.yamldocs/deployment/install-openclaw-plugins.mdxdocs/security/best-practices.mdxtest/e2e-gateway-isolation.shtest/e2e-non-root-smoke.shtest/e2e/support/security-posture-workflow-boundary.test.tstools/e2e/security-posture-workflow-boundary.mts
7bb2703 to
e60e529
Compare
|
Rebased onto current main. Focused tests and |
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/e2e-non-root-smoke.sh`:
- Around line 103-115: Update Test 3 in the root override assertion to require a
stable root-mode marker emitted by nemoclaw-start, rather than relying on the
absence of the free-form “Running as non-root” message. Keep the child command’s
UID check as a separate assertion, and make the test pass only when both the
root-only marker and expected sandbox UID are present.
🪄 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: 7896ff63-d81c-446a-a732-110ad29173df
📒 Files selected for processing (8)
.github/workflows/e2e.yamlDockerfiledocs/deployment/install-openclaw-plugins.mdxdocs/security/best-practices.mdxtest/e2e-gateway-isolation.shtest/e2e-non-root-smoke.shtest/e2e/support/security-posture-workflow-boundary.test.tstools/e2e/security-posture-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/security/best-practices.mdx
e60e529 to
a0562b0
Compare
wscurran
added
area: packaging
|
✨ Thanks for the PR. This fixes OpenShell compatibility by defaulting the OpenClaw sandbox image to the Related open issues: Related open issues: |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed exact head a0562b0. The OpenClaw image now defaults to the non-root sandbox user required by OpenShell, while direct runtimes retain an explicit root override that enters the existing privileged supervisor path and steps agent commands back down. Workflow, isolation, smoke, plugin-image, and operator documentation changes are consistent. I found no blocking security, compatibility, or regression defect. The failed E2E gate is a trusted-verdict timeout, not an attributable test failure.
Signed-off-by: Deepak Jain <deepujain@gmail.com>
Signed-off-by: Deepak Jain <deepujain@gmail.com>
Signed-off-by: Deepak Jain <deepujain@gmail.com>
Signed-off-by: Deepak Jain <deepujain@gmail.com>
fa5987a to
8091faf
Compare
|
Rebased on current main. The image build, focused non-root checks, and full PR validation pass, and all commits are verified. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Synced with latest main. Focused non-root checks and PR validation pass; CI is rerunning. |
|
Synced with current main while preserving the reviewed history. CodeRabbit and guardrails are clean; E2E and advisor checks are rerunning. |
|
Synced with today's main. The reviewed history remains verified; CodeRabbit and fast gates pass, with advisors and E2E still running. |
|
Synced with current main. All commits remain verified, approval and CodeRabbit are intact, and E2E is running. The failed primary advisor lane stopped before analysis and reported no findings. |
5 participants
Summary
The OpenClaw sandbox image now defaults to the
sandboxuser so OpenShell can accept its OCI metadata. Root-only image setup still completes during the build, and direct container runtimes can explicitly override the runtime user to root when gateway and agent UID separation is required.Related Issue
Fixes #7882
Changes
sandboxafter all root-only build steps.no-new-privilegesstartup, and explicit root override against the built production image.USER sandboxin the custom OpenClaw plugin example.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailabledocker build --progress=plain -t nemoclaw-7882-e2e .NEMOCLAW_TEST_IMAGE=nemoclaw-7882-e2e bash test/e2e-non-root-smoke.sh(4 passed)NEMOCLAW_TEST_IMAGE=nemoclaw-7882-e2e bash test/e2e-gateway-isolation.sh(45 passed)npx vitest run --project e2e-support test/e2e/support/security-posture-workflow-boundary.test.ts test/e2e/support/security-posture.test.ts(5 passed)test/nemoclaw-start.test.tsintegration selection (4 passed)npm run build:cli,npm run typecheck:cli, and pre-push gates passednpm run docsbuilds without warnings (doc changes only)Signed-off-by: Deepak Jain deepujain@gmail.com
Summary by CodeRabbit
Bug Fixes
sandboxuser by default, while still supporting explicitrootoverrides when required.Tests
Documentation
sandboxafter required root setup.