Skip to content

fix(shields): unlock idempotently when config already mutable (#7430) - #7629

Merged
prekshivyas merged 17 commits into
mainfrom
fix/7430-clone-restore-shields-guard
Jul 31, 2026
Merged

fix(shields): unlock idempotently when config already mutable (#7430)#7629
prekshivyas merged 17 commits into
mainfrom
fix/7430-clone-restore-shields-guard

Conversation

@yimoj

@yimoj yimoj commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

nemoclaw <sandbox> shields down failed on the first attempt on DGX Spark/Station and macOS clone/restore (and freshly onboarded) sandboxes with OpenClaw config guard unlock [config-not-locked] ... restart seal requires the exact shields-locked directory posture. On those platforms an in-sandbox OpenClaw reconciler re-permissions the config back to the sandbox-owned mutable posture after a host shields lock returns, and a freshly onboarded or snapshot-restored sandbox boots mutable before the lock settles, so the first host shields down ran unlock against an already-mutable config. The config guard's unlock transition required the exact shields-locked posture and rejected it; the rollback then re-locked the config, so a retry succeeded. After this change unlock is idempotent: an already-mutable config is a no-op success, mirroring the already-locked short-circuit in the lock branch.

Related Issue

Fixes #7430

Changes

  • scripts/openclaw-config-guard.py: in _transition, the unlock branch now returns a no-op success when the config already holds the exact mutable posture. After the raw config/hash pair is captured, _verify_mutable_posture revalidates the sandbox-owned parent, config directory, and both files before returning, so directory drift during capture fails closed. A genuinely locked or drifted posture still takes the strict path. No new abstraction, configuration, or fallback is introduced.
  • test/openclaw-config-guard.test.ts: regression coverage verifies idempotent success for the exact mutable posture, fail-closed rejection when the config directory drifts during pair capture, and that a rejected 0600 unlock leaves the invalid mode and file identities unchanged before fixture cleanup restores the mutable posture.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: internal config-guard reliability fix; makes an existing command succeed on first attempt in a clone/restore/fresh corner case that no user-facing page documents. Documentation-writer subagent confirmed no-docs-needed.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex maintainer security review PASS at exact head 5b5cee9ef across all nine categories; exact diff fingerprint ccedc3a08262471a16265d0e9782eec7a9d8c5d8eaaacb99ca178528854f9269. The change removes the reported TOCTOU gap by capturing the pair and then revalidating the complete mutable posture; focused race and rejected-mode coverage passes.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Exact head 5b5cee9ef310dd316064e8546fda6fe0b2467f13 preserves a byte-identical two-file diff with regression-only coverage proving that a rejected unlock preserves an invalid 0600 mode and always restores the test fixture during cleanup. The production contract is unchanged: unlock is idempotent only for the exact mutable posture and fails closed on drift. docs/manage-sandboxes/runtime-controls.mdx, docs/reference/commands.mdx, docs/reference/troubleshooting.mdx, and docs/security/tcb-boundary.mdx already describe the mutable 2770/660 contract, unsupported 700/600 drift, shields transitions, and fail-closed config-guard boundary.
  • Agent: Codex

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable — exact-head 5b5cee9ef repository hooks passed; the push hook's CLI typecheck and package/tag sync passed
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — the config-guard suite passed 45/45 with repository Python 3.11 on the preceding feature-identical exact head 210f7c802; exact current-main head 5b5cee9ef keeps the same two-file fingerprint, and build/validation/typecheck/hooks pass. Fresh GitHub CI is required for the exact-head Linux rerun
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: not applicable (single guard-branch change with focused coverage)
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Real-hardware E2E (reporter workflow, worktree CLI)

macOS (Apple M4 Pro, arm64, macOS 26.5.2, OpenShell 0.0.85, Colima Docker 29.2.1) — exact head 210f7c802

Worktree CLI built from a fresh clone of this branch on the host (./bin/nemoclaw.js --versionnemoclaw v0.0.96-54-g210f7c8). Reporter workflow: onboarded sandbox sb-7430snapshot restore clone → shields down.

Failing precondition captured raw on the clone before any shields command — config already mutable while shields state is not down:

/sandbox                            755  sandbox:sandbox
/sandbox/.openclaw                 2770  sandbox:sandbox
/sandbox/.openclaw/openclaw.json    660  sandbox:sandbox
/sandbox/.openclaw/.config-hash     660  sandbox:sandbox

Pre-fix (baked guard without this change) — reproduces:

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430 snapshot restore v1 --to sb-7430-clone --yes
State restored into 'sb-7430-clone', but gateway pairing could not be verified.

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430-clone shields down --timeout 1m --policy permissive
  Unlocking openclaw config (/sandbox/.openclaw/openclaw.json)...
  ERROR: Config not unlocked: OpenClaw config guard unlock [config-not-locked] /sandbox/.openclaw: restart seal requires the exact shields-locked directory posture
  Re-run `nemoclaw sb-7430-clone shields down` after correcting file ownership.

The failed unlock's rollback re-locks the config (/sandbox 1775 root:sandbox, /sandbox/.openclaw 755 root:root, openclaw.json 444 root:root), which is exactly why a retry appears to succeed and masks the defect. The post-fix run therefore uses a fresh clone, not a retry.

Post-fix (this change's guard, first attempt on a fresh clone) — resolved:

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430 snapshot restore v1 --to sb-7430-clone2 --yes
State restored into 'sb-7430-clone2', but gateway pairing could not be verified.

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430-clone2 shields down --timeout 1m --policy permissive
  Unlocking openclaw config (/sandbox/.openclaw/openclaw.json)...
  Config unlocked for sb-7430-clone2 (auto-lockdown in: 1m)
  Sandbox is in default (mutable) state.
exit 0

Config stays at the mutable target (/sandbox/.openclaw 2770 sandbox:sandbox, openclaw.json and .config-hash 660 sandbox:sandbox) — the idempotent branch changed nothing.

Locked transition and strict path unaffected (same clone, same head):

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430-clone2 shields up
  Lockdown active for sb-7430-clone2
exit 0
# posture: /sandbox 1775 root:sandbox | /sandbox/.openclaw 755 root:root | openclaw.json 444 root:root

$ NEMOCLAW_GATEWAY_PORT=8091 ./bin/nemoclaw.js sb-7430-clone2 shields down --timeout 1m --policy permissive
  Config unlocked for sb-7430-clone2 (auto-lockdown in: 1m)
exit 0
# posture: /sandbox 755 sandbox:sandbox | /sandbox/.openclaw 2770 sandbox:sandbox | openclaw.json 660 sandbox:sandbox

A genuinely locked config still takes the strict _verify_locked_posture path and unlocks normally; only the already-mutable posture short-circuits.

DGX Spark (horizon-46, NVIDIA GB10, aarch64, Ubuntu 24.04, OpenShell 0.0.85, Docker 29.2.1)

Same reporter workflow on the reporter's platform, worktree CLI, failing state = config mutable (2770 sandbox:sandbox) + host shields state UP (shieldsDown:false):

  • Baked (unfixed) guard: nemoclaw sb-7430-clone shields down --timeout 2m --policy permissiveERROR: ... [config-not-locked] ... (reproduces).
  • Fixed guard (hot-patched into the container): same command → Config unlocked for sb-7430-clone (auto-lockdown in: 2m); config stays 2770 sandbox:sandbox.

Both platforms accept the OpenShell-supervised PID-1 topology on current main (the reporter's v0.0.91 requires NemoClaw PID 1 no longer fires); the remaining first-attempt failure was this config-not-locked idempotency gap.

Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Made unlock fully idempotent when the configuration already has the expected mutable permissions.
    • Prevented legitimate “shields down”/already-unlocked states from incorrectly failing with a “config not locked” error, without changing existing permissions.
  • Tests

    • Added scenarios verifying unlock succeeds with the correct mutable posture and that generated artifacts remain unchanged.
    • Expanded mutable-directory drift coverage to ensure failures report the expected error code and leave artifacts intact.

On DGX Spark/Station and macOS an in-sandbox OpenClaw reconciler
re-permissions the config back to the sandbox-owned mutable posture
after a host shields lock returns, and a freshly onboarded or
snapshot-restored sandbox boots mutable before the lock settles. The
config guard's `unlock` transition required the exact shields-locked
posture, so the first `nemoclaw <sandbox> shields down` on a
clone/restore (or fresh) sandbox failed with `config-not-locked` even
though the config already held the mutable target posture; the rollback
then re-locked it, so a retry succeeded.

Make `unlock` idempotent, mirroring the already-locked short-circuits in
the lock branch: when the config already holds the exact mutable posture,
verify the mutable files and return a no-op success. Security-safe — it
returns early only for the sandbox-owned mutable target, so nothing
locked is unlocked.

Reproduced and verified on a real DGX Spark (GB10, arm64, OpenShell
0.0.85) with the worktree CLI: before the fix `shields down` returned
`[config-not-locked]`; after, it returns "Config unlocked".

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-code-quality

github-code-quality Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 5b5cee9 in the fix/7430-clone-resto... branch remains at 96%, unchanged from commit 57f73a5 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 5b5cee9 in the fix/7430-clone-resto... branch remains at 81%, unchanged from commit 57f73a5 in the main branch.

Show a code coverage summary of the most impacted files.
File main 57f73a5 fix/7430-clone-resto... 5b5cee9 +/-
src/lib/onboard/docker-cdi.ts 80% 70% -10%
src/lib/onboard...host-anchors.ts 94% 90% -4%
src/lib/onboard/preflight.ts 80% 80% 0%
src/lib/onboard...al-inference.ts 91% 93% +2%
src/lib/actions...orchestrator.ts 83% 87% +4%
src/lib/onboard...box-gpu-mode.ts 92% 97% +5%
src/lib/actions...confirmation.ts 69% 79% +10%
src/lib/actions...ocker-health.ts 65% 82% +17%
src/lib/onboard...box-prebuild.ts 74% 92% +18%
src/lib/onboard...flight-ports.ts 33% 67% +34%

Updated July 31, 2026 17:02 UTC

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The OpenClaw config guard now treats unlock as a successful no-op when the configuration is already mutable. Tests verify preserved artifact identity and detection of mutable-posture drift.

Changes

OpenClaw unlock transition

Layer / File(s) Summary
Mutable unlock handling
scripts/openclaw-config-guard.py, test/openclaw-config-guard.test.ts
The unlock path revalidates already-mutable configurations without locked-posture checks. Tests verify successful no-op behavior, unchanged config artifacts, and config-not-mutable reporting when permissions or directory posture drift.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant ConfigGuard
  participant ConfigFilesystem
  CLI->>ConfigGuard: request unlock
  ConfigGuard->>ConfigFilesystem: snapshot config and hash
  ConfigGuard->>ConfigFilesystem: verify mutable posture
  ConfigGuard-->>CLI: success or config-not-mutable issue
Loading

Possibly related issues

  • NVIDIA/NemoClaw#7538 — The change directly addresses unlock behavior for configurations already in the mutable posture.

Suggested labels: v0.0.97

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change matches #7430 by allowing shields down to succeed on already-mutable sandboxes while still failing closed on drift.
Out of Scope Changes check ✅ Passed The code and test changes stay focused on config-guard unlock idempotency and regression coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making the shields unlock transition idempotent when the configuration is already mutable.
✨ 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/7430-clone-restore-shields-guard

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/openclaw-config-guard.test.ts (1)

417-424: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert that the complete config pair remains unchanged.

This test only verifies the directory mode. Capture openclaw.json and .config-hash bytes and modes before runGuard, then compare them afterward; otherwise a regression that rewrites configuration data or file permissions could still pass.

As per path instructions, this test should prove behavioral confidence; the PR objective also requires the mutable configuration to remain unchanged.

Proposed test strengthening
-    const { configDir } = fixture();
+    const { configDir, configPath, hashPath } = fixture();
+    const before = {
+      config: fs.readFileSync(configPath),
+      hash: fs.readFileSync(hashPath),
+      configMode: mode(configPath),
+      hashMode: mode(hashPath),
+      dirMode: mode(configDir),
+    };
     const r = runGuard("unlock", configDir);
     expect(r.status, JSON.stringify(r.lines)).toBe(0);
     expect(r.lines.at(-1)).toMatchObject({ action: "unlock", status: "ok" });
-    expect(mode(configDir)).toBe(0o2770);
+    expect(fs.readFileSync(configPath)).toEqual(before.config);
+    expect(fs.readFileSync(hashPath)).toEqual(before.hash);
+    expect(mode(configPath)).toBe(before.configMode);
+    expect(mode(hashPath)).toBe(before.hashMode);
+    expect(mode(configDir)).toBe(before.dirMode);
🤖 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/openclaw-config-guard.test.ts` around lines 417 - 424, Strengthen the
idempotent unlock test around runGuard("unlock", configDir) by capturing the
bytes and permission modes of both openclaw.json and .config-hash before
execution, then asserting all four values remain unchanged afterward; retain the
existing directory-mode and successful unlock assertions.

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/openclaw-config-guard.test.ts`:
- Around line 417-424: Strengthen the idempotent unlock test around
runGuard("unlock", configDir) by capturing the bytes and permission modes of
both openclaw.json and .config-hash before execution, then asserting all four
values remain unchanged afterward; retain the existing directory-mode and
successful unlock assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c2564c6a-d3fc-4e0f-b98e-45d3bab86f30

📥 Commits

Reviewing files that changed from the base of the PR and between a47ddd8 and 88511bc.

📒 Files selected for processing (2)
  • scripts/openclaw-config-guard.py
  • test/openclaw-config-guard.test.ts

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported 1 fewer blocker, the same number of warnings, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

2 optional E2E recommendations
  • shields-config
  • gateway-guard-recovery

Blockers

PRA-1 Blocker — Validate the config/hash pair before idempotent unlock succeeds

  • Location: scripts/openclaw-config-guard.py:3475
  • Category: correctness
  • Problem: The new exact-mutable-posture branch captures files with `_snapshot_raw_pair` and returns after mode and ownership checks. Unlike the former unlock path, it does not validate that `.config-hash` names and hashes the captured `openclaw.json` bytes, so unlock now reports success for a stable but corrupted pair.
  • Impact: A successful shields-down operation can leave a malformed or mismatched config integrity record in the mutable target state, rather than rejecting the invalid configuration as the previous unlock path did.
  • Fix: Use `_snapshot_pair` in the idempotent mutable branch, or explicitly validate the captured pair before `_verify_mutable_posture` and returning.
  • Verification: Set a mismatched `.config-hash` in an exact mutable fixture, invoke `unlock`, and compare its result with the prior `_snapshot_pair` validation contract.
  • Test coverage: Add a mutable-posture unlock test with a malformed or mismatched `.config-hash` record. Assert unlock rejects it with the established validation error and does not change either file.
  • Evidence: `_snapshot_pair` at scripts/openclaw-config-guard.py:2189-2213 calls `_validate_hash_record`; `_snapshot_raw_pair` at lines 2216-2240 does not. The new unlock branch at scripts/openclaw-config-guard.py:3475 captures with `_snapshot_raw_pair` and returns after `_verify_mutable_posture`. The changed test at test/openclaw-config-guard.test.ts:414-436 covers modes, identity, and directory drift but not a malformed or mismatched hash record.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

#7430)

Address CodeRabbit review on #7629: the idempotent-unlock regression test
verified only the config directory mode. Also assert the openclaw.json and
.config-hash file modes stay sandbox-owned 0660 so a regression that
re-permissions config files during the no-op unlock cannot pass. Kept within
the 1500-line test-file-size budget by dropping the redundant result-shape
assertion.

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @coderabbitai — addressed in 33b70e9. The idempotent-unlock test now asserts all three modes (.openclaw dir plus openclaw.json and .config-hash) stay sandbox-owned mutable (2770/0660), so a regression that re-permissions config files during the no-op unlock is caught. The config bytes are unchanged by construction (the new branch runs _verify_mutable_files then return, with no write on the idempotent path), and the full byte-diff assertion was omitted only to stay within the 1500-line test-file-size-budget for this file.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git show --stat --oneline 33b70e9c8
git show --format=fuller --find-renames --find-copies -- \
  scripts/openclaw-config-guard.py test/openclaw-config-guard.test.ts 33b70e9c8

ast-grep outline scripts/openclaw-config-guard.py --items all --match '_transition|_verify_mutable_files'
ast-grep outline test/openclaw-config-guard.test.ts --items all --match 'unlock|mutable'

rg -n -C 6 '_is_mutable_dir_posture|_verify_mutable_files|def _transition' scripts/openclaw-config-guard.py
rg -n -C 8 'idempotent|already.*mutable|openclaw\.json|\.config-hash|0o2770|0o660' test/openclaw-config-guard.test.ts

Length of output: 49138


@yimoj Verified in 33b70e9c8: the regression test now covers the directory and both config files’ mutable modes, and the idempotent branch only snapshots/verifies before returning. The omitted byte-diff assertion is reasonable given that control flow.

🐇 ✅

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security/correctness review at exact head 33b70e9c88bf98bf70d2052b4bddbb2a23974b78.

Blocking finding: the idempotent unlock path checks the mutable directory posture before _snapshot_raw_pair(), then validates only file metadata. Because the tree is already sandbox-owned and writable, a concurrent sandbox process can change either directory during the pair capture. The branch still returns success even though the final posture is no longer the exact mutable target. I reproduced this by injecting a config-directory chmod to 0700 after the snapshot: _transition("unlock", ...) returned successfully with final mode 0700.

Please capture the pair and then revalidate the full mutable posture, for example with _verify_mutable_posture(opened, pair, identity), and add a regression that injects parent or config directory drift during capture. This also resolves advisor warning PRA-1.

Gate evidence:

  • Product scope: PASS. This repairs existing Shields behavior linked to issue #7430 and adds no supported surface.
  • Contributor compliance: PASS. The PR body has the author DCO declaration and all 3 commits are GitHub Verified.
  • CI: PASS for the current PR SHA; all 53 current checks are green, including E2E / PR Gate.
  • Focused verification: PASS, 45/45 openclaw-config-guard tests at this head.
  • Current-main compatibility: NOT READY. The PR records base a47ddd896e, while current main is a8748ab1af. A synthetic merge is clean, but the branch must be refreshed and its new SHA rechecked before approval.

Nine-category security review:

  1. Secrets and credentials: PASS — no secrets or credential handling changed.
  2. Input validation and sanitization: PASS — descriptor-safe file validation remains in place.
  3. Authentication and authorization: PASS — root and startup authorization paths are unchanged.
  4. Dependencies: PASS — no dependency changes.
  5. Error handling and logging: PASS — no sensitive logging added.
  6. Cryptography and data protection: PASS — no cryptographic behavior changed.
  7. Configuration and security defaults: PASS — no ports, privileges, or defaults changed.
  8. Security testing: WARNING — the new writable-tree race is not covered.
  9. System security: FAIL — the reproduced TOCTOU lets unlock report success after the verified directory posture has drifted.

Comment on lines +3464 to +3476
if _is_mutable_dir_posture(opened, identity):
# Unlock is idempotent, mirroring the already-locked short-circuits in
# the lock branch above. A config already in the exact mutable posture
# is the unlock target: on some platforms (DGX Spark/Station, macOS) an
# in-sandbox OpenClaw reconciler re-permissions the config back to
# sandbox-owned mutable *after* a host lock returns, and a freshly
# onboarded or snapshot-restored sandbox boots mutable before the lock
# settles. Requiring the locked posture there rejected a legitimate
# `shields down` with config-not-locked even though the config already
# holds the mutable target posture. Verify the exact mutable file
# posture and treat the transition as a no-op instead of failing.
_verify_mutable_files(opened, _snapshot_raw_pair(opened), identity)
return

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directory check happens before _snapshot_raw_pair(), but this tree is already sandbox-writable. If a sandbox process changes the parent or config directory during capture, _verify_mutable_files() still passes and this branch returns success without rechecking the directory. I reproduced _transition("unlock", ...) succeeding with final config-directory mode 0700. Capture the pair, then validate the full mutable posture (for example _verify_mutable_posture(opened, pair, identity)) and add a directory-drift regression.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed at exact signed/Verified head 32c1b9698. The already-mutable unlock path now captures with _snapshot_raw_pair() and then calls _verify_mutable_posture(opened, pair, identity) before returning. The regression mutates the config directory to 0700 during capture and requires fail-closed config-not-mutable; the companion rejected-mode coverage also proves a 0600 file remains unchanged. The focused guard suite passes 45/45 and exact-range hooks pass. Leaving this thread open for your re-review.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact head update: aec2516 is now current-main synced and GitHub Verified; the feature diff is unchanged. The full mutable-posture revalidation and directory-drift regression remain in place. Exact-head guard tests pass 45/45 under the repository Python 3.11 environment, exact-range hooks pass, and the fresh WSL, macOS, CLI, CodeQL, and E2E gates are green. Leaving the thread open for your re-review.

@cv cv added the v0.0.97 label Jul 27, 2026
@apurvvkumaria apurvvkumaria self-assigned this Jul 27, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Comment thread test/openclaw-config-guard.test.ts Fixed
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/openclaw-config-guard.test.ts`:
- Around line 427-434: Update the invalid-permissions loop in the unlock guard
test to assert that mode(invalidPath) remains 0o600 after the rejected run. Move
restoration of invalidPath to 0o660 into a finally block so cleanup occurs even
if the assertion or runGuard call fails, while preserving the existing identity
and status assertions.
🪄 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: 58b4efc7-bbce-4406-9ed6-a404b96ca447

📥 Commits

Reviewing files that changed from the base of the PR and between fcf5b17 and 67ec5af.

📒 Files selected for processing (1)
  • test/openclaw-config-guard.test.ts

Comment thread test/openclaw-config-guard.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
cv pushed a commit that referenced this pull request Jul 28, 2026
<!-- markdownlint-disable MD041 -->
## Summary
<!-- 1-3 plain sentences: what changes and why. Describe
before-and-after behavior when it applies. Follow the NemoClaw Writing
Guide: https://github.com/NVIDIA/NemoClaw/blob/main/WRITING.md. Do not
add unrelated prose cleanup. -->
Replace the mutable NodeSource bootstrap in both WSL workflows with
checksum-verified official Node.js 22.23.1 archives. This removes the
shared installer failure affecting current PR lanes while keeping the
runtime on the supported, security-current Node 22 LTS line.

## Changes
<!-- List concrete changes. If this adds an abstraction, configuration,
fallback, migration, or compatibility path, name its current requirement
and consumer, explain why a direct change is insufficient, and identify
the test that protects it. -->
- Install official Node.js 22.23.1 archives for `x64` and `arm64`, with
exact SHA-256 pins, HTTPS-only bounded downloads, temporary-directory
cleanup, and fail-closed architecture handling.
- Verify each archive before extraction and assert the installed runtime
version.
- Protect both workflow consumers with
`test/platform-vitest-main-workflow.test.ts`, including
architecture-to-digest bindings and the repository's approved security
source-shape contract.
- Address the same WSL bootstrap failure observed on
[#7590](https://github.com/NVIDIA/NemoClaw/actions/runs/30312091098/job/90131121904),
[#7629](https://github.com/NVIDIA/NemoClaw/actions/runs/30311922489/job/90131122471),
[#7603](https://github.com/NVIDIA/NemoClaw/actions/runs/30311696352/job/90131123359),
and
[#7626](https://github.com/NVIDIA/NemoClaw/actions/runs/30312801807/job/90132378380).

## 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
<!-- Check one tests line and one docs line. Check other lines when
applicable. Add every requested justification or approval reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This changes internal CI
bootstrap behavior only; public installation requirements remain
unchanged.
- [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: Codex security review
PASS across all nine categories on exact diff fingerprint
`2981cff1a79a0c0acfaf5b0a8e55353809ec472c0565fa1f168996f92107b7ae`.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review
<!-- Required for code and documentation changes after the changes and
applicable validation are complete. Keep one review checkbox and one
instance of each visible or hidden field. For Evidence, list changed
documentation paths. For documentation-only changes, also state that the
writing rules and documentation style were reviewed. For other results,
explain why no documentation change is needed or why the review is
blocked. For Agent, use a consistent product and surface name, such as
Codex Desktop, Codex CLI, Claude Code, or Cursor. After committing all
review changes, put `git rev-parse --short HEAD` and `git rev-parse
--short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review
and refresh that metadata after any new commit. This receipt is advisory
during the data-collection pilot. -->
- [x] Documentation writer subagent reviewed the completed changes
- Result: `no-docs-needed`
- Evidence: Exact-head review confirmed the change is limited to CI
bootstrap workflows and their executable contract; public Node.js
requirements do not change.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 062779c -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence
<!-- Required only when scripts/prepare-dgx-station-host.sh changes.
Maintainers must review the linked evidence before approving or merging.
This is human-reviewed evidence, not authenticated hardware provenance.
Exceptional bypasses use existing repository governance and must be
documented on the PR. -->
- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run --project integration
test/platform-vitest-main-workflow.test.ts` (3 passed); both embedded
WSL installers pass `bash -n`.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **CI Improvements**
* Standardized Node.js 22 installation across WSL workflows with a
pinned version and architecture-specific downloads.
* Added archive checksum verification, version validation, cleanup, and
explicit failure handling.

* **Tests**
* Added coverage to verify secure, deterministic Node.js installation
behavior in both WSL workflows.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria and others added 3 commits July 27, 2026 17:55
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@cjagwani cjagwani added v0.0.98 and removed v0.0.97 labels Jul 28, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery labels Jul 28, 2026
@wscurran wscurran added area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression platform: dgx-spark Affects DGX Spark hardware or workflows platform: dgx-station Affects DGX Station hardware or workflows platform: macos Affects macOS, including Apple Silicon labels Jul 28, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@cjagwani

Copy link
Copy Markdown
Collaborator

Babysitting status for exact head 210f7c8 (plain comment; no Changes Requested review from me): the later commits appear to address the earlier mutable-posture race, and all 53 current check contexts are green. Approval is still blocked because the branch is based on c4c020c rather than current main da1b103, maintainer edits are disabled, and the prior review has not been cleared on current-base evidence. Please refresh; I will re-review the next quiet head.

@cjagwani

Copy link
Copy Markdown
Collaborator

Correction to my prior handoff: conflict-free base refreshes are explicitly waived. Please do not merge main solely for base currency; preserving exact-head evidence is preferred unless GitHub reports a real conflict or reviewed behavior requires a change. The substantive blocker or missing evidence described in the earlier handoff remains, but base age by itself is not a blocker. This is a plain coordination comment, not Changes Requested.

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head approval for 210f7c8 after the quiet-window review. The prior mutable-posture race is resolved: the idempotent unlock path captures the raw config/hash pair, then revalidates sandbox ownership and exact modes on both live directory descriptors and both files before returning. The injected config-directory drift regression fails closed. The exact focused suite passes 45/45, Python compilation passes, all 53 GitHub checks are green, all 14 commits are Verified, and no unresolved major findings remain. Nine-category security review is PASS: no credential, authorization, dependency, logging, cryptographic, policy, or privilege expansion; descriptor-bound validation and immutable/append-flag checks remain intact. GitHub reports no conflict, and the user waiver applies to base-only refreshes.

@cjagwani

Copy link
Copy Markdown
Collaborator

@prekshivyas The blocking review is attached to old head 33b70e9c8; exact head 210f7c802 closes the mutable-posture TOCTOU window by revalidating after the raw-pair snapshot, with focused regression coverage and green exact-head CI. Could you re-review or dismiss the stale Changes Requested state? I am continuing to babysit this PR through terminal state.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@prekshivyas
prekshivyas merged commit 8e90c7f into main Jul 31, 2026
72 of 74 checks passed
@prekshivyas
prekshivyas deleted the fix/7430-clone-restore-shields-guard branch July 31, 2026 17:06
@senthilr-nv senthilr-nv mentioned this pull request Aug 1, 2026
23 tasks
senthilr-nv added a commit that referenced this pull request Aug 1, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical dated changelog entry for `v0.0.100` so the
maintainer release plan can verify the pre-tag documentation
prerequisite. The entry summarizes the user-facing changes merged since
`v0.0.99` and links to the relevant guides.

## Changes

- Add `docs/changelog/2026-07-31.mdx` with the exact `## v0.0.100`
heading.
- Cover restored OpenClaw pairing, transactional replacement, Deep
Agents Code, onboarding recovery, lifecycle cleanup, Hermes builds, host
provenance, documentation, and trusted E2E evidence.
- Distinguish active Docker and Kubernetes runtime-bundle enforcement
from the still-inactive managed shared-state transaction foundation.

## Source Coverage

The release entry maps the doc-impacting merged PRs in the
`v0.0.99..main` release range to `docs/changelog/2026-07-31.mdx`: #8021,
#8024, #7973, #8028, #7947, #7788, #7884, #8023, #7969, #8020, #7989,
#8000, #7907, #7942, #7567, #8013, #7955, #8017, #8014, #8015, #7629,
#7644, #7821, #7971, and #7991.

PR #7974 was reviewed after the final rebase and excluded because it
changes internal maintainer-skill attribution policy and tests only; it
does not change a user-facing product or documentation surface.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: the
changelog contract test validates the dated entry, version heading, SPDX
form, and route constraints.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-07-31.mdx`; exact-head review passed
for `6093f44f`; writing rules and documentation style reviewed; `npx
vitest run test/changelog-docs.test.ts` passed 6/6; `npm run docs`
passed with zero Fern errors and two generic Fern upgrade notices.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 6093f44 -->
<!-- docs-review-agents-blob-sha: 3dd7c24 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; no DGX Station host script changed.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run validate:pr` passed after refreshing `origin/main` when hooks
were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 6/6 at `6093f44f`.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Not applicable to a dated
prose-only release entry.
- [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) —
validation passed with zero errors; Fern emitted two generic upgrade
notices.
- [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)
— the changelog entry has the required parser-safe MDX SPDX header;
dated changelog entries intentionally do not use page frontmatter.

---

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.100.
* Documented improvements to restore pairing, sandbox replacement,
onboarding recovery, lifecycle cleanup, runtime handling, build support,
host readiness, and end-to-end validation.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression platform: dgx-spark Affects DGX Spark hardware or workflows platform: dgx-station Affects DGX Station hardware or workflows platform: macos Affects macOS, including Apple Silicon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Linux][Security] config guard PID 1 ownership check rejects legitimate shields up/down from host CLI on clone/restore sandboxes

7 participants