fix(onboard): guard config-sync chmod with ownership check#4206
Closed
hunglp6d wants to merge 1 commit into
Closed
fix(onboard): guard config-sync chmod with ownership check#4206hunglp6d wants to merge 1 commit into
hunglp6d wants to merge 1 commit into
Conversation
… EPERM PR NVIDIA#4054 added `chmod 700 ~/.nemoclaw` and `chmod 600 ~/.nemoclaw/config.json` to the sandbox config-sync script. Inside OpenClaw sandbox containers the `/sandbox/.nemoclaw` directory is root-owned (Dockerfile L733: root:root 1755 for DAC protection of blueprints/), so the sandbox user cannot chmod it. Under `set -euo pipefail` the EPERM becomes exit 1, crashing every OpenClaw E2E job at onboard step 7/8. Wrap both chmod calls in an ownership check: if [ "$(stat -c '%u' <path>)" = "$(id -u)" ]; then chmod ...; fi This preserves the hardening on host installs (where the user owns the dir) while skipping the chmod inside root-owned sandbox containers where prepare_filesystem already handles permissions. Hermes containers are unaffected because their Dockerfile.base chowns /sandbox to sandbox:sandbox. Fixes: NemoClaw nightly-e2e run 26425202514 (39/39 OpenClaw jobs failed) Refs: NemoClaw#4054, NemoClaw#4009 Signed-off-by: Hung Le <hple@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2 tasks
Contributor
Author
|
Closed due to a fix #4199 was merged |
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
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
Guard
chmod 700 ~/.nemoclawandchmod 600 ~/.nemoclaw/config.jsonin the sandbox config-sync script with an ownership check so the commands only run when the current user owns the target. This prevents EPERM crashes inside OpenClaw sandbox containers where/sandbox/.nemoclawis root-owned (Dockerfile L733:root:root 1755), fixing all 39 failed OpenClaw E2E jobs in the May 26 nightly run.Related Issue
Fixes #4207
Changes
chmod 700 ~/.nemoclawwithif [ "$(stat -c '%u' ~/.nemoclaw)" = "$(id -u)" ]so it only runs when the sandbox user owns the directory (host installs). Inside root-owned containers,prepare_filesystemalready handles permissions.chmod 600 ~/.nemoclaw/config.jsonwith the same ownership guard.config-sync.test.tscontinue to pass (3/3) since thechmod 700andchmod 600strings are still present inside the if-blocks.Root Cause
PR #4054 (commit
f1044ad) added unconditionalchmod 700 ~/.nemoclawandchmod 600 ~/.nemoclaw/config.jsontobuildSandboxConfigSyncScript()insrc/lib/onboard/config-sync.ts. The script runs as thesandboxuser inside OpenClaw containers where/sandbox/.nemoclawisroot:root 1755(Dockerfile L733). Underset -euo pipefail, the EPERM from the failed chmod becomes exit 1, piped throughopenshell sandbox connect, crashing every OpenClaw E2E job at onboard step 7/8.Hermes containers are unaffected because
Dockerfile.baseL74-75 chowns/sandboxtosandbox:sandbox.Validation
A focused
custom-e2e.yamlworkflow was run on a sibling branch to confirm this fix repairs the regression. The workflow re-runs only the jobs from the original nightly that this PR targets, onubuntu-latest, off the same fix commit as this PR.ci/nightly-e2e-self-hostedonhunglp6d/NemoClawcloud-e2e(covers the OpenClaw onboard path that crashes)4f9b749The validation branch is intentionally not the head of this PR — it carries an extra
.github/workflows/custom-e2e.yamlcommit that is scaffolding, not part of the fix. Re-run the validation by pushing any commit to the validation branch.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Hung Le hple@nvidia.com