fix(scripts): restore .openclaw perms after nemoclaw exec command - #6060
Merged
Conversation
openclaw doctor --fix collapses /sandbox/.openclaw from 2770 to 700 and openclaw.json from 660 to 600 when run via `nemoclaw exec`. The NEMOCLAW_CMD exec paths used bare `exec` to replace the shell process, making any post-command cleanup impossible. Replace `exec` with a regular call, capture the exit code, call normalize_mutable_config_perms to restore 2770/660, then exit with the original code. Covers both the non-root path and the root path (via STEP_DOWN_PREFIX_SANDBOX). Fixes #6047 Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedNo new commits to review since the last review. ⚙️ 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:
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:
📝 WalkthroughWalkthroughThis PR replaces Bash-local mutable OpenClaw config permission repair/baseline logic with a descriptor-safe Python normalizer ( ChangesMutable config cleanup pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant execSandbox
participant runSandboxExecCommand
participant cleanupOpenClawAfterExec
participant inspectMutableConfigPerms
participant repairMutableConfigPerms
execSandbox->>runSandboxExecCommand: start sandbox command
runSandboxExecCommand->>cleanupOpenClawAfterExec: inspect after child exit
cleanupOpenClawAfterExec->>inspectMutableConfigPerms: read mutable config state
cleanupOpenClawAfterExec->>repairMutableConfigPerms: repair when needed
runSandboxExecCommand-->>execSandbox: completion code + cleanup status
sequenceDiagram
participant run_oneshot_command
participant NEMOCLAW_CMD
participant normalize_mutable_config_perms
run_oneshot_command->>NEMOCLAW_CMD: start direct child
run_oneshot_command->>NEMOCLAW_CMD: forward TERM/INT while waiting
run_oneshot_command->>normalize_mutable_config_perms: cleanup after child exit
normalize_mutable_config_perms-->>run_oneshot_command: cleanup status
run_oneshot_command-->>run_oneshot_command: return cleanup failure or child exit code
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/nemoclaw-start.sh`:
- Around line 3981-3984: The one-shot command path in nemoclaw-start.sh is no
longer forwarding termination signals because it exits after running
NEMOCLAW_CMD without the gateway loop’s trap handling. Update the one-shot
branch around the NEMOCLAW_CMD execution so it either uses exec or installs an
explicit trap/forward-and-wait wrapper consistent with cleanup_on_signal,
ensuring SIGTERM/SIGINT from docker stop are propagated to the child before
exiting with its status.
- Around line 3981-3984: The one-shot command path in nemoclaw-start.sh can
bypass cleanup because the NEMOCLAW_CMD invocation is running under errexit, so
the exit code capture and normalize_mutable_config_perms may never run. Update
the one-shot flow around the NEMOCLAW_CMD execution so failures cannot skip the
cleanup/restore step, using the existing _nemoclaw_cmd_rc and
normalize_mutable_config_perms flow as the place to preserve. If you keep the
shell as PID 1 instead of execing the child, also ensure TERM/INT are forwarded
to the child process; otherwise restore exec and relocate the permission-fix
logic outside the post-run path.
🪄 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: d7d86225-04c1-4fc2-9e37-1488bf415ad2
📒 Files selected for processing (1)
scripts/nemoclaw-start.sh
openclaw doctor --fix collapses /sandbox/.openclaw from 2770 to 700 and openclaw.json from 660 to 600 when run via `nemoclaw exec`. The NEMOCLAW_CMD exec paths used bare `exec` to replace the shell process, making any post-command cleanup impossible. Replace `exec` with a regular call in both NEMOCLAW_CMD paths. Use `_nemoclaw_cmd_rc=0; cmd || _nemoclaw_cmd_rc=$?` to capture the exit code safely under `set -e`, call normalize_mutable_config_perms to restore 2770/660, then exit with the original code. Covers both the non-root path and the root path (via STEP_DOWN_PREFIX_SANDBOX). Add ORDER-marker tests in test/nemoclaw-start-perms.test.ts to verify the call sequence (command → normalize → exit with captured code) in both paths, including that a non-zero exit from NEMOCLAW_CMD is preserved through the normalize call. Fixes #6047 Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/nemoclaw-start-perms.test.ts (1)
16-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMarker-based source-text extraction is brittle implementation lock-in.
extractBlockslices raw bash text out ofnemoclaw-start.shusing exact literal markers (including specific indentation, e.g." if [ ${#NEMOCLAW_CMD[@]} -gt 0 ]; then\n"and"\n fi\n"). Any reformatting of the script (reindentation, comment rewording,shfmt) will throw "marker not found" and fail the test for reasons unrelated to the permission-ordering regression being verified, rather than exercising an observable behavioral contract.Since the script has no callable functions to test directly, this approach may be unavoidable, but consider isolating the two
NEMOCLAW_CMDblocks behind clearly-delimited, stable comment sentinels innemoclaw-start.shitself (e.g.# BEGIN/END NEMOCLAW_CMD_NONROOT) so the test's contract is decoupled from incidental formatting in the surrounding code. As per path instructions, tests should "prefer observable outcomes through the public boundary over source-text, private-shape ... assertions."Also applies to: 29-34, 56-61
🤖 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 `@test/nemoclaw-start-perms.test.ts` around lines 16 - 22, The test helper extractBlock is coupled to exact bash formatting, so reindentation or shfmt changes will break the test for the wrong reason. Update nemoclaw-start.sh to add stable sentinel comments around the NEMOCLAW_CMD sections, and then change extractBlock and the related assertions in nemoclaw-start-perms.test.ts to slice between those sentinels instead of brittle literal indentation markers. Keep the test focused on the ordering behavior of the NEMOCLAW_CMD blocks rather than raw source-text shape.Source: Path instructions
🤖 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 `@test/nemoclaw-start-perms.test.ts`:
- Around line 16-22: The test helper extractBlock is coupled to exact bash
formatting, so reindentation or shfmt changes will break the test for the wrong
reason. Update nemoclaw-start.sh to add stable sentinel comments around the
NEMOCLAW_CMD sections, and then change extractBlock and the related assertions
in nemoclaw-start-perms.test.ts to slice between those sentinels instead of
brittle literal indentation markers. Keep the test focused on the ordering
behavior of the NEMOCLAW_CMD blocks rather than raw source-text shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 52ecaead-99eb-4c19-81c9-d921630dc945
📒 Files selected for processing (2)
scripts/nemoclaw-start.shtest/nemoclaw-start-perms.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/nemoclaw-start.sh
…-shape asserts The codebase-growth-guardrails check bans `if (` in changed test files and caps source-shape assertion cases at 0. Move the script-block extraction into module-scope helpers (assertions there are not counted as source-shape cases) and drop the inline if/throw guards, keeping the test bodies linear. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28528148197
|
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28528151221
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28531132233
|
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28531133994
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28531609217
|
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28531611172
|
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28532745190
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Vitest E2E Target Results —
|
| Job | Result |
|---|---|
| live | |
| network-policy |
Contributor
|
✅ Action performedReview finished.
|
Contributor
|
✅ Action performedReview finished.
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
|
@coderabbitai review Growth-guard follow-up is now at |
Contributor
|
✅ Action performedReview finished.
|
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28538535223
|
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28538710787
|
cv
approved these changes
Jul 1, 2026
Contributor
|
✅ Action performedReview finished.
|
21 tasks
This was referenced Jul 1, 2026
cjagwani
added a commit
that referenced
this pull request
Jul 1, 2026
Merge origin/main into fix/5980-exec-newline-guidance. Resolved: - src/lib/actions/sandbox/exec.ts: keep main's async spawn dispatch (runSandboxExecChild + signal forwarding + OpenClaw permission cleanup, #6060); re-apply the PR's pre-dispatch multiline-argv guard on top, threaded through main's deps seam. The guard exits 2 before dispatch and never echoes argument contents. - docs/reference/commands.mdx: place the PR's agent-agnostic multiline guidance after main's AgentOnly exit-code blocks. - src/lib/actions/sandbox/exec.multiline-guard.test.ts: update the default-runner stdio-inherit test to assert main's async spawn (spawn(..., { stdio: 'inherit' })) instead of the retired spawnSync default. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apurvvkumaria
pushed a commit
that referenced
this pull request
Jul 1, 2026
## Summary Five pre-existing test failures on the `macos-vitest` CI workflow (confirmed present before #6060) traced to bash 3.x incompatibilities, a macOS UTF-8 locale issue, and a missing fixture sync. This PR fixes all five root causes. Supersedes #6137 (that PR's branch was accidentally cut from a security feature branch, pulling in unrelated files into the diff; this is a clean rebase onto main with the same two commits plus a CodeRabbit fix). ## Related Issue Investigation of CI run [28539883104](https://github.com/NVIDIA/NemoClaw/actions/runs/28539883104); failures also present in run [28534214317](https://github.com/NVIDIA/NemoClaw/actions/runs/28534214317) (before #6060), confirming #6060 is not the cause. ## Changes - **`agents/hermes/start.sh`** — three bash 3.2 compatibility fixes: - Replace bash 4.1+ named-FD `exec {var}<file` with a `{ } < file` grouped redirect; variables assigned inside `{}` remain in function scope - Replace `mapfile -d '' -t` (bash 4.x only) with `while IFS= read -r -d "" elem; do arr+=("$elem"); done` - Guard `${_HERMES_GUARD_TIMEOUT[@]}` with `${arr[@]+"${arr[@]}"}` so `set -u` does not abort the script when the array is empty (bash 3.2 treats empty `[@]` as unbound) - **`scripts/gateway-control.sh`** — add `export LC_ALL=C` so `[a-f]` character-class ranges in `case` patterns are byte-exact; macOS `en_US.UTF-8` makes `[a-f]` case-insensitive, allowing uppercase hex nonces to pass the `*[!0-9a-f]*` check - **`scripts/lib/gateway-supervisor.sh`** — same `LC_ALL=C` fix for the sourced library's nonce validation path - **`test/gateway-supervisor-control.test.ts`** — pin `NEMOCLAW_TEST_GATEWAY_CONTROL_CALLER_UID=0` in the nonce-rejection test so it does not depend on the CI runner's UID; tighten macOS bash 3.2 SIGTERM filter from broad word-match to exact `Terminated: <digits>` / `Killed: <digits>` format so unrelated stderr still fails the assertion - **`test/e2e/fixtures/redaction.ts`** — add `tvly-` Tavily token pattern missing since #6134, fixing the `e2e-redaction-parity` `Array(16)` vs `Array(17)` mismatch ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Docs not applicable — justification: shell compatibility and test fixes, no user-facing behavior change - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) — `gateway-control.sh` and `gateway-supervisor.sh` handle nonce validation - [x] Sensitive-path review completed or maintainer-approved waiver recorded — the `LC_ALL=C` fix tightens nonce validation (rejects uppercase hex on macOS that was previously accepted); `gateway_control_stop_tracked_pid` behavior is unchanged ## Verification - [x] Git hooks passed during commit and push - [x] Targeted tests pass for changed behavior - `test/gateway-supervisor-control.test.ts`: 22/22 ✓ - `test/hermes-managed-exit-authorization.test.ts`: all ✓ (was 8 failures before) - `test/e2e/support/e2e-redaction-parity.test.ts`: 3/3 ✓ - `test/hermes-gateway-supervisor-recovery.test.ts`: 41/42 (1 local flake — PID 4242 alive on dev machine; unrelated to these changes, passes on CI fresh runners) - [x] No secrets, API keys, or credentials committed **Remaining failures not addressed in this PR** (different root class, need separate investigation): - `install-preflight.test.ts` — environment-specific - `deepagents-code-tui-startup-check.test.ts` — needs investigation - `platform-parity-cloud-experimental.test.ts` — needs investigation - WSL `runtime-recovery-preload.test.ts`, `rebuild-config-hash.test.ts` — different class of failure --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved gateway nonce validation to reliably accept only lowercase hex characters, independent of locale and macOS environments. * Fixed managed gateway startup, recovery, and live-status monitoring to be compatible with older Bash versions. * Tightened controller/marker parsing to reduce incorrect authorization or status detection. * Expanded secret redaction to cover additional Tavily-shaped tokens, improving protection in logs and text output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
18 tasks
cv
added a commit
that referenced
this pull request
Jul 1, 2026
<!-- markdownlint-disable MD041 --> ## Summary This follow-up extracts the direct mutable OpenClaw repair invocation from the oversized shields module after #6060 merged. It preserves the trusted-helper command, identity validation, timeouts, and runtime behavior while documenting the existing cross-process lock and first-start recovery-baseline semantics requested by automated review. ## Related Issue Follow-up to #6060 and #6047. ## Changes - Move sandbox UID/GID lookup and the trusted mutable-config normalizer invocation into `src/lib/shields/mutable-config-repair.ts`. - Keep runtime dependency lookup compatible with the existing Vitest spies and module cache behavior. - Document why the timer-bound host lock is the correct cross-process serialization boundary for post-`exec` repair. - Document why a missing recovery baseline is expected and intentionally quiet before the first successful post-override capture. - Pin the extracted privilege boundary with focused tests for sanitized UID/GID probes, exact normalizer argv, invalid identity short-circuiting, and Docker execution failures. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: the existing shields-transition, mutable-permission, and post-`exec` cleanup suites exercise the unchanged public path, identity validation, and trusted-helper invocation. - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this is a behavior-preserving extraction plus implementation comments; the #6060 command and troubleshooting docs remain accurate. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent read-only review found no security or correctness blocker; helper path, argv, timeout, UID/GID validation, cross-process transition lock, and fail-closed behavior are unchanged from merged #6060. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ### Evidence - `npx prek run --from-ref origin/main --to-ref HEAD` passed under the repository-required `umask 022`, including the full CLI/integration coverage hook. - The normal push hook passed CLI type-checking and version synchronization. - Focused shields-transition, mutable-config-permission, extracted-repair, and post-`exec` cleanup tests passed 49/49; CLI type-checking, Biome, Python parsing, test-title style, file-size budget, and `git diff --check` passed. - The extracted-repair tests prove `stdin=false` and `sanitizeEnvironment=true` on every privileged call, reject invalid UID/GID values before normalization, assert forwarded Docker argv/options, and propagate normalizer execution failures. - The documentation-writer audit found no doc change necessary; `npm run docs` completed with 0 errors and 2 pre-existing warnings. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved mutable configuration-permission repair to run the normalization step inside a privileged sandbox, including sandbox identity discovery and validation (UID/GID) before changes are applied. - Added a time-bound execution wrapper around the normalization command to prevent long-running operations. - **Refactor** - Moved the mutable-config permission normalization into a dedicated, reusable module invoked by the existing repair workflow. - **Tests** - Added coverage for identity validation failures, error propagation, and the exact privileged command invocation sequence. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
3 tasks
ericksoa
pushed a commit
that referenced
this pull request
Jul 2, 2026
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
21 tasks
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…IDIA#6060) <!-- markdownlint-disable MD041 --> ## Summary `openclaw doctor --fix` can collapse mutable OpenClaw paths from NemoClaw's multi-UID `2770/660` contract to OpenClaw's single-user `700/600` defaults. This change restores that contract after both entrypoint one-shot commands and the documented `nemoclaw <name> exec` boundary, with deterministic child-versus-cleanup exit-status precedence. ## Related Issue Fixes NVIDIA#6047 ## Changes - Supervise entrypoint one-shot commands so `TERM` and `INT` are forwarded, the direct child is reaped, permission cleanup always runs, and the remote status is preserved when cleanup succeeds. - After public OpenShell exec returns, inspect registered OpenClaw sandboxes, repair only detected mutable-permission drift through the installed descriptor-safe normalizer while holding the timer-bound shields mutation lock, and require successful re-inspection. Hermes, custom agents, unregistered sandboxes, and active shields locks remain untouched. - Move mutable-tree normalization, baseline capture, and empty-config recovery into an installed root-trusted Python helper that operates through pinned, no-follow descriptors. - Authenticate the permanently privilege-dropped owner child with a private Unix socket, `SO_PASSCRED`, exact credentials, and `SCM_RIGHTS`; retain the exact directory/config descriptors across the privilege boundary. - Replace recovery baseline/config/hash entries with fresh inodes so hardlinks, symlink swaps, directory replacement, and inode-reuse races cannot turn root into a confused deputy. - Fail closed on missing trusted helper, unexpected ownership, unsafe links, metadata changes, malformed descriptor handoff, or incomplete verification. - Document host-side cleanup behavior, atomic recovery, failure precedence, and safe operator recovery guidance. - Add unit, integration, container, and live-target coverage for permission drift, signals, cleanup precedence, capability loss, hardlink safety, path swaps, protected symlinks, and trusted-helper selection. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent review through runtime head `85b0744310553ec4a141bdc031cdc881e8c57422` confirmed the previously reproducible cross-phase ABA, external-hardlink ownership/mode mutation, path-based recovery TOCTOU, root helper-selection, and earlier-tree hardlink-alias issues are fixed. Follow-up `64d6234a9198e14e582155ebae4c75c362373f10` changes test control flow only. The owner child is permanently privilege-dropped before recursive mutation and root performs no work without an authenticated descriptor handoff. No runtime privilege-boundary blocker remains; the same-UID child retains only authority that the sandbox user already has over its own inode. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ### Evidence - Signed commit and push hooks: repository checks, Biome, ShellCheck, Hadolint, gitleaks, CLI typecheck, full CLI test hook, source-shape budget, and test-size budget passed. - Focused host tests: the final runtime correction passed 67 CLI and 145 integration tests after the earlier focused suites; CLI type-checking passed. - Fresh production image: build passed; new E2E cases 30–30f all passed. The full script reported 40 passes and two unrelated stale base-image profile assertions. - Independent container probes passed normal repair, exact-config capture, empty-config recovery, hardlink/protected-target invariants, and the final `700/600` to `2770/660` production-image repair path. - Full CLI/coverage hooks, repository checks, source-shape budget, and test-size budget passed under the repository's expected `umask 022`. - Docs build completed with 0 errors and 2 pre-existing warnings; agent-variant synchronization and docs checks passed. - Security test-depth follow-ups remain non-blocking: malformed ancillary-message variants, an isolated missing-`CAP_SETUID` case, 16 MiB boundary/source-mutation/temp-cleanup cases, exact-image provenance when reusing an existing E2E tag, and a same-UID concurrent post-check hardlink race that cannot increase the child process's existing authority. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added safer one-shot command handling with post-command config permission cleanup. * Improved config recovery and permission repair behavior for OpenClaw environments. * Updated documentation to describe the new cleanup and recovery behavior more clearly. * **Bug Fixes** * Hardened config permission handling against symlinks, ownership mismatches, and concurrent changes. * Improved failure handling so unsafe or incomplete repairs now fail closed with clearer status reporting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Five pre-existing test failures on the `macos-vitest` CI workflow (confirmed present before NVIDIA#6060) traced to bash 3.x incompatibilities, a macOS UTF-8 locale issue, and a missing fixture sync. This PR fixes all five root causes. Supersedes NVIDIA#6137 (that PR's branch was accidentally cut from a security feature branch, pulling in unrelated files into the diff; this is a clean rebase onto main with the same two commits plus a CodeRabbit fix). ## Related Issue Investigation of CI run [28539883104](https://github.com/NVIDIA/NemoClaw/actions/runs/28539883104); failures also present in run [28534214317](https://github.com/NVIDIA/NemoClaw/actions/runs/28534214317) (before NVIDIA#6060), confirming NVIDIA#6060 is not the cause. ## Changes - **`agents/hermes/start.sh`** — three bash 3.2 compatibility fixes: - Replace bash 4.1+ named-FD `exec {var}<file` with a `{ } < file` grouped redirect; variables assigned inside `{}` remain in function scope - Replace `mapfile -d '' -t` (bash 4.x only) with `while IFS= read -r -d "" elem; do arr+=("$elem"); done` - Guard `${_HERMES_GUARD_TIMEOUT[@]}` with `${arr[@]+"${arr[@]}"}` so `set -u` does not abort the script when the array is empty (bash 3.2 treats empty `[@]` as unbound) - **`scripts/gateway-control.sh`** — add `export LC_ALL=C` so `[a-f]` character-class ranges in `case` patterns are byte-exact; macOS `en_US.UTF-8` makes `[a-f]` case-insensitive, allowing uppercase hex nonces to pass the `*[!0-9a-f]*` check - **`scripts/lib/gateway-supervisor.sh`** — same `LC_ALL=C` fix for the sourced library's nonce validation path - **`test/gateway-supervisor-control.test.ts`** — pin `NEMOCLAW_TEST_GATEWAY_CONTROL_CALLER_UID=0` in the nonce-rejection test so it does not depend on the CI runner's UID; tighten macOS bash 3.2 SIGTERM filter from broad word-match to exact `Terminated: <digits>` / `Killed: <digits>` format so unrelated stderr still fails the assertion - **`test/e2e/fixtures/redaction.ts`** — add `tvly-` Tavily token pattern missing since NVIDIA#6134, fixing the `e2e-redaction-parity` `Array(16)` vs `Array(17)` mismatch ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Docs not applicable — justification: shell compatibility and test fixes, no user-facing behavior change - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) — `gateway-control.sh` and `gateway-supervisor.sh` handle nonce validation - [x] Sensitive-path review completed or maintainer-approved waiver recorded — the `LC_ALL=C` fix tightens nonce validation (rejects uppercase hex on macOS that was previously accepted); `gateway_control_stop_tracked_pid` behavior is unchanged ## Verification - [x] Git hooks passed during commit and push - [x] Targeted tests pass for changed behavior - `test/gateway-supervisor-control.test.ts`: 22/22 ✓ - `test/hermes-managed-exit-authorization.test.ts`: all ✓ (was 8 failures before) - `test/e2e/support/e2e-redaction-parity.test.ts`: 3/3 ✓ - `test/hermes-gateway-supervisor-recovery.test.ts`: 41/42 (1 local flake — PID 4242 alive on dev machine; unrelated to these changes, passes on CI fresh runners) - [x] No secrets, API keys, or credentials committed **Remaining failures not addressed in this PR** (different root class, need separate investigation): - `install-preflight.test.ts` — environment-specific - `deepagents-code-tui-startup-check.test.ts` — needs investigation - `platform-parity-cloud-experimental.test.ts` — needs investigation - WSL `runtime-recovery-preload.test.ts`, `rebuild-config-hash.test.ts` — different class of failure --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved gateway nonce validation to reliably accept only lowercase hex characters, independent of locale and macOS environments. * Fixed managed gateway startup, recovery, and live-status monitoring to be compatible with older Bash versions. * Tightened controller/marker parsing to reduce incorrect authorization or status detection. * Expanded secret redaction to cover additional Tavily-shaped tokens, improving protection in logs and text output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary This follow-up extracts the direct mutable OpenClaw repair invocation from the oversized shields module after NVIDIA#6060 merged. It preserves the trusted-helper command, identity validation, timeouts, and runtime behavior while documenting the existing cross-process lock and first-start recovery-baseline semantics requested by automated review. ## Related Issue Follow-up to NVIDIA#6060 and NVIDIA#6047. ## Changes - Move sandbox UID/GID lookup and the trusted mutable-config normalizer invocation into `src/lib/shields/mutable-config-repair.ts`. - Keep runtime dependency lookup compatible with the existing Vitest spies and module cache behavior. - Document why the timer-bound host lock is the correct cross-process serialization boundary for post-`exec` repair. - Document why a missing recovery baseline is expected and intentionally quiet before the first successful post-override capture. - Pin the extracted privilege boundary with focused tests for sanitized UID/GID probes, exact normalizer argv, invalid identity short-circuiting, and Docker execution failures. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: the existing shields-transition, mutable-permission, and post-`exec` cleanup suites exercise the unchanged public path, identity validation, and trusted-helper invocation. - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this is a behavior-preserving extraction plus implementation comments; the NVIDIA#6060 command and troubleshooting docs remain accurate. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent read-only review found no security or correctness blocker; helper path, argv, timeout, UID/GID validation, cross-process transition lock, and fail-closed behavior are unchanged from merged NVIDIA#6060. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ### Evidence - `npx prek run --from-ref origin/main --to-ref HEAD` passed under the repository-required `umask 022`, including the full CLI/integration coverage hook. - The normal push hook passed CLI type-checking and version synchronization. - Focused shields-transition, mutable-config-permission, extracted-repair, and post-`exec` cleanup tests passed 49/49; CLI type-checking, Biome, Python parsing, test-title style, file-size budget, and `git diff --check` passed. - The extracted-repair tests prove `stdin=false` and `sanitizeEnvironment=true` on every privileged call, reject invalid UID/GID values before normalization, assert forwarded Docker argv/options, and propagate normalizer execution failures. - The documentation-writer audit found no doc change necessary; `npm run docs` completed with 0 errors and 2 pre-existing warnings. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved mutable configuration-permission repair to run the normalization step inside a privileged sandbox, including sandbox identity discovery and validation (UID/GID) before changes are applied. - Added a time-bound execution wrapper around the normalization command to prevent long-running operations. - **Refactor** - Moved the mutable-config permission normalization into a dedicated, reusable module invoked by the existing repair workflow. - **Tests** - Added coverage for identity validation failures, error propagation, and the exact privileged command invocation sequence. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
21 tasks
wscurran
added
area: security
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
5 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
openclaw doctor --fixcan collapse mutable OpenClaw paths from NemoClaw's multi-UID2770/660contract to OpenClaw's single-user700/600defaults. This change restores that contract after both entrypoint one-shot commands and the documentednemoclaw <name> execboundary, with deterministic child-versus-cleanup exit-status precedence.Related Issue
Fixes #6047
Changes
TERMandINTare forwarded, the direct child is reaped, permission cleanup always runs, and the remote status is preserved when cleanup succeeds.SO_PASSCRED, exact credentials, andSCM_RIGHTS; retain the exact directory/config descriptors across the privilege boundary.Type of Change
Quality Gates
85b0744310553ec4a141bdc031cdc881e8c57422confirmed the previously reproducible cross-phase ABA, external-hardlink ownership/mode mutation, path-based recovery TOCTOU, root helper-selection, and earlier-tree hardlink-alias issues are fixed. Follow-up64d6234a9198e14e582155ebae4c75c362373f10changes test control flow only. The owner child is permanently privilege-dropped before recursive mutation and root performs no work without an authenticated descriptor handoff. No runtime privilege-boundary blocker remains; the same-UID child retains only authority that the sandbox user already has over its own inode.Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Evidence
700/600to2770/660production-image repair path.umask 022.CAP_SETUIDcase, 16 MiB boundary/source-mutation/temp-cleanup cases, exact-image provenance when reusing an existing E2E tag, and a same-UID concurrent post-check hardlink race that cannot increase the child process's existing authority.Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes