Skip to content

fix(onboard): preserve root-owned config sync dirs#4199

Merged
cv merged 1 commit into
mainfrom
fix/openclaw-config-sync-perms
May 26, 2026
Merged

fix(onboard): preserve root-owned config sync dirs#4199
cv merged 1 commit into
mainfrom
fix/openclaw-config-sync-perms

Conversation

@ericksoa
Copy link
Copy Markdown
Contributor

@ericksoa ericksoa commented May 25, 2026

Summary

  • keep ~/.nemoclaw directory hardening for user-owned config dirs
  • skip directory chmod when the config dir is owned by another UID, which preserves OpenClaw's root-owned /sandbox/.nemoclaw layout
  • keep config.json restricted to 0600
  • add shell-level regression coverage for owned and non-owned config directories

Root Cause

PR #4054 changed the step-7 sandbox config sync script from writing ~/.nemoclaw/config.json to unconditionally running chmod 700 ~/.nemoclaw first. In the OpenClaw sandbox image, /sandbox/.nemoclaw is intentionally root-owned with mode 1755, while /sandbox/.nemoclaw/config.json is sandbox-owned and writable. The directory chmod fails under set -euo pipefail, so openshell sandbox connect <sandbox> exits 1 during:

[7/8] Setting up OpenClaw inside sandbox

This restores compatibility with that image contract without reverting the file-permission hardening from #4054.

Evidence

Validation

  • npm ci --include=dev --ignore-scripts
  • npx vitest run src/lib/onboard/config-sync.test.ts --reporter=verbose
  • npx @biomejs/biome check src/lib/onboard/config-sync.ts src/lib/onboard/config-sync.test.ts
  • git diff --check
  • npm run build:cli
  • npm run typecheck:cli

Summary by CodeRabbit

  • Refactor

    • Improved NemoClaw configuration directory permission handling to be more robust in different user ownership scenarios.
    • Enhanced sandbox configuration script to conditionally apply permissions based on directory ownership.
  • Tests

    • Added comprehensive integration tests for configuration directory permission management and ownership verification.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

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: 34676ae3-2305-4970-87da-f3084391e708

📥 Commits

Reviewing files that changed from the base of the PR and between 4f9b749 and 1ada225.

📒 Files selected for processing (2)
  • src/lib/onboard/config-sync.test.ts
  • src/lib/onboard/config-sync.ts

📝 Walkthrough

Walkthrough

The PR updates NemoClaw's sandbox config-sync script to use $HOME-relative paths with ownership-aware permission handling, replacing hardcoded ~/.nemoclaw with dynamic $HOME-based directory creation. Permission changes are now conditional on the directory owner matching the current user. Tests add helpers to execute the script in isolated environments and verify both the permission-setting and permission-skipping behaviors.

Changes

NemoClaw Config Permission Safety

Layer / File(s) Summary
Shell script NemoClaw config update
src/lib/onboard/config-sync.ts
The generated bash script switches from hardcoded ~/.nemoclaw to a $HOME-based path (defaulting to /sandbox), computes the directory's UID at runtime, applies chmod 700 conditionally only when the owner matches the current user, and sets config.json permissions to 600.
Test infrastructure and assertion updates
src/lib/onboard/config-sync.test.ts
Adds spawnSync import and Unix-specific helpers to write fake id/stat commands to a temp bin directory, run the generated script with overridden HOME and PATH, and compute file mode bits. Existing script assertions are updated to match the new $HOME-based paths and ownership-aware permission logic.
Integration tests for permission handling
src/lib/onboard/config-sync.test.ts
Two new Unix-only tests execute the generated script against temporary directory structures: one asserts that NemoClaw config directories and files are tightened to user-only permissions when owned by the current user, and another simulates a different owner (via fake id/stat) and asserts permissions remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4054: Both PRs update NemoClaw config permissioning logic to enforce owner-only modes and adjust corresponding tests accordingly.

Suggested labels

fix, security, v0.0.51

Poem

🐰 A config directory, now safe and sound,
With ownership checks all around,
$HOME paths replace the old way,
Permissions enforced, come what may—
The sandbox stays locked, hooray!

🚥 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 directly addresses the main fix: preventing chmod of root-owned config directories. It accurately reflects the core change from the PR objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/openclaw-config-sync-perms

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@github-actions
Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Top item: No actionable code-review findings

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: openclaw-onboard-security-posture-e2e, cloud-e2e
Optional E2E: rebuild-openclaw-e2e, ubuntu-repo-cloud-openclaw

Dispatch hint: openclaw-onboard-security-posture-e2e,cloud-e2e

Auto-dispatched E2E: openclaw-onboard-security-posture-e2e, cloud-e2e via nightly-e2e.yaml at 1ada2256399f114e412f4b5d561411886940b88fnightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • openclaw-onboard-security-posture-e2e (high): Runs a full OpenClaw install/onboard/inference path with non-root-host security posture assertions, directly exercising the changed sandbox config-sync script and the permission-sensitive class this PR modifies.
  • cloud-e2e (high): Baseline full user journey install → onboard → sandbox verification → live inference. This is required because config-sync runs during onboard and writes provider selection needed for OpenClaw runtime behavior.

Optional E2E

  • rebuild-openclaw-e2e (high): Useful adjacent coverage for the rebuild/resume path called out in config-sync.ts comments, ensuring OpenClaw state survives and post-rebuild inference still works after config permission changes.
  • ubuntu-repo-cloud-openclaw (high): Scenario-runner alternative/adjacent check for repo-current cloud OpenClaw onboarding with smoke, inference, credentials, and baseline-onboarding suites.

New E2E recommendations

  • sandbox config permissions (medium): Existing E2E coverage exercises onboarding, but does not appear to assert /sandbox/.nemoclaw/config.json mode 600, /sandbox/.nemoclaw directory mode behavior, or the foreign-owned-directory no-chmod case added by this PR.
    • Suggested test: Add an E2E validation step after OpenClaw onboard that inspects /sandbox/.nemoclaw/config.json and /sandbox/.nemoclaw metadata from inside the sandbox, and add a targeted regression fixture for a pre-existing non-current-user-owned .nemoclaw directory if OpenShell can create that state.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: openclaw-onboard-security-posture-e2e,cloud-e2e

@github-actions
Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26422388069
Target ref: 1ada2256399f114e412f4b5d561411886940b88f
Workflow ref: main
Requested jobs: cloud-onboard-e2e,tunnel-lifecycle-e2e
Summary: 0 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ⚠️ cancelled
tunnel-lifecycle-e2e ⚠️ cancelled

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26422445180
Target ref: 1ada2256399f114e412f4b5d561411886940b88f
Workflow ref: main
Requested jobs: openclaw-onboard-security-posture-e2e,cloud-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-e2e ✅ success
openclaw-onboard-security-posture-e2e ✅ success

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 26422654135
Target ref: 1ada2256399f114e412f4b5d561411886940b88f
Workflow ref: main
Requested jobs: cloud-onboard-e2e,tunnel-lifecycle-e2e
Summary: 0 passed, 2 failed, 0 skipped

Job Result
cloud-onboard-e2e ❌ failure
tunnel-lifecycle-e2e ❌ failure

Failed jobs: cloud-onboard-e2e, tunnel-lifecycle-e2e. Check run artifacts for logs.

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 26430878242
Target ref: 1ada2256399f114e412f4b5d561411886940b88f
Workflow ref: main
Requested jobs: all (no filter)
Summary: 47 passed, 3 failed, 2 skipped

Job Result
bedrock-runtime-compatible-anthropic-e2e ✅ success
brave-search-e2e ✅ success
channels-add-remove-e2e ✅ success
channels-stop-start-e2e ❌ failure
cloud-e2e ✅ success
cloud-inference-e2e ✅ success
cloud-onboard-e2e ❌ failure
credential-migration-e2e ✅ success
credential-sanitization-e2e ✅ success
device-auth-health-e2e ✅ success
diagnostics-e2e ✅ success
docs-validation-e2e ✅ success
double-onboard-e2e ✅ success
gpu-double-onboard-e2e ⏭️ skipped
gpu-e2e ⏭️ skipped
hermes-discord-e2e ✅ success
hermes-e2e ✅ success
hermes-inference-switch-e2e ✅ success
hermes-onboard-security-posture-e2e ✅ success
hermes-slack-e2e ✅ success
inference-routing-e2e ✅ success
issue-2478-crash-loop-recovery-e2e ✅ success
issue-3600-gpu-proof-optional-e2e ✅ success
kimi-inference-compat-e2e ✅ success
launchable-smoke-e2e ✅ success
messaging-compatible-endpoint-e2e ✅ success
messaging-providers-e2e ✅ success
network-policy-e2e ✅ success
onboard-negative-paths-e2e ✅ success
onboard-repair-e2e ✅ success
onboard-resume-e2e ✅ success
openclaw-discord-pairing-e2e ✅ success
openclaw-inference-switch-e2e ✅ success
openclaw-onboard-security-posture-e2e ✅ success
openclaw-slack-pairing-e2e ✅ success
openclaw-tui-chat-correlation-e2e ❌ failure
openshell-gateway-upgrade-e2e ✅ success
overlayfs-autofix-e2e ✅ success
rebuild-hermes-e2e ✅ success
rebuild-hermes-stale-base-e2e ✅ success
rebuild-openclaw-e2e ✅ success
runtime-overrides-e2e ✅ success
sandbox-operations-e2e ✅ success
sandbox-survival-e2e ✅ success
shields-config-e2e ✅ success
skill-agent-e2e ✅ success
snapshot-commands-e2e ✅ success
state-backup-restore-e2e ✅ success
telegram-injection-e2e ✅ success
token-rotation-e2e ✅ success
tunnel-lifecycle-e2e ✅ success
upgrade-stale-sandbox-e2e ✅ success

Failed jobs: channels-stop-start-e2e, cloud-onboard-e2e, openclaw-tui-chat-correlation-e2e. Check run artifacts for logs.

@cv cv added the v0.0.51 Release target label May 26, 2026
@cv cv merged commit cb11ca8 into main May 26, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.51 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants