refactor(shields): isolate mutable config repair

[cv]

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

• 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: 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
• Docs not applicable — justification: this is a behavior-preserving extraction plus implementation comments; the fix(scripts): restore .openclaw perms after nemoclaw exec command #6060 command and troubleshooting docs remain accurate.
• 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: 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 fix(scripts): restore .openclaw perms after nemoclaw exec command #6060.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
• Targeted tests pass for changed behavior
• Full npm test passes (broad runtime changes only)
• 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)

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

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.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 082cd4c2-a75f-45d1-9b67-6b2a7eaf8f28

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Extracts mutable OpenClaw config normalization into src/lib/shields/mutable-config-repair.ts, rewires src/lib/shields/index.ts to use it, adds tests for the new helper, and updates comments in sandbox cleanup and Python baseline recovery code.

Changes

Mutable Config Normalization Extraction

Layer / File(s)	Summary
New normalization module src/lib/shields/mutable-config-repair.ts	Adds helpers to run privileged Docker sandbox commands, resolve sandbox UID/GID via /usr/bin/id with numeric validation, and exports normalizeMutableOpenClawConfig to invoke the trusted normalizer script.
Wire and validate the extracted helper src/lib/shields/index.ts, src/lib/shields/mutable-config-repair.test.ts, src/lib/shields/openclaw-transition.test.ts	Imports normalizeMutableOpenClawConfig from the new module, removes the in-file normalizer implementation and its path constant, and adds tests covering identity validation, argv construction, timeout wrapping, and error propagation.
Comment clarifications src/lib/actions/sandbox/exec.ts, scripts/lib/normalize_mutable_config_perms.py	Adds explanatory comments to cleanupOpenClawAfterExec and lock_recovery_baseline without changing executable behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: area: sandbox

Suggested reviewers: jyaunches, brandonpelfrey

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main refactor: extracting mutable config repair out of the shields module.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/pr6060-advisor-followup

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: shields-config
Optional E2E: sandbox-operations

Dispatch hint: shields-config

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• shields-config (high): Focused live coverage for this change: it runs install/onboard, exercises the real nemoclaw <sandbox> exec doctor path that tightens OpenClaw config permissions, verifies host-side repair restores 2770/660 mutable config posture, and then validates shields up/down locking, drift detection, audit, and auto-restore behavior against a live sandbox.

Optional E2E

• sandbox-operations (high): Useful adjacent confidence for real sandbox exec/list/status/recovery behavior, but not merge-blocking because the code changes are concentrated in shields mutable-config repair and shields-config directly covers the affected permission-repair contract.

New E2E recommendations

• mutable config repair concurrency and watchdog (medium): Existing shields-config proves the single-process live repair path, but there is no focused live E2E that forces a hung normalizer or concurrent nemoclaw <sandbox> exec cleanup/repair invocations to validate the timeout wrapper and cross-process shields transition lock behavior under contention.
  • Suggested test: Add a live mutable-config-repair stress test that creates doctor-tightened permissions, launches concurrent exec cleanup repairs, optionally injects a slow/hung normalizer fixture, and asserts final .openclaw ownership/modes plus fail-closed timeout reporting.

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: shields-config

[github-actions]

E2E Target Recommendation

Required E2E targets: shields-config
Optional E2E targets: ubuntu-repo-cloud-openclaw

Dispatch required E2E targets:

• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=shields-config

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

• shields-config: Exercises the live OpenClaw shields/config boundary affected by the mutable config repair refactor and normalizer timeout changes, including nemoclaw <sandbox> exec running openclaw doctor --fix and verifying the mutable .openclaw permissions are repaired in a real sandbox.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=shields-config

Optional E2E targets

• ubuntu-repo-cloud-openclaw: Adjacent baseline OpenClaw live target to catch broader regressions from changes in the shields coordinator import path and OpenClaw sandbox lifecycle, beyond the focused shields-config repair scenario.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Relevant changed files

• scripts/lib/normalize_mutable_config_perms.py
• src/lib/actions/sandbox/exec.ts
• src/lib/shields/index.ts
• src/lib/shields/mutable-config-repair.ts

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/pr6060-advisor... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/pr6060-advisor... a32d292	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the codex/pr6060-advisor... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/pr6060-advisor... a32d292	+/-
src/lib/shields...nsition-lock.ts	—	86%	—
src/lib/actions...dbox/rebuild.ts	—	80%	—
src/lib/actions...all/run-plan.ts	—	80%	—
src/lib/state/o...oard-session.ts	—	80%	—
src/lib/state/sandbox.ts	—	72%	—
src/lib/shields/index.ts	—	70%	—
src/lib/onboard/preflight.ts	—	69%	—
src/lib/onboard...er-gpu-patch.ts	—	59%	—
src/lib/actions...licy-channel.ts	—	58%	—
src/lib/onboard.ts	—	20%	—

---

_{Updated July 01, 2026 21:59 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 5 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Runtime validation
• PRA-T5 Add or justify test follow-up: Acceptance clause

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Add or identify targeted live-sandbox validation that `nemoclaw <sandbox> exec` followed by cleanup repairs doctor-tightened OpenClaw `.openclaw` permissions back to the mutable contract without starting a shields transition.. Changed code touches privileged Docker/OpenShell sandbox repair, file ownership/mode normalization, shields posture locking, and process timeouts. Unit coverage is strong for the extracted boundary and existing post-exec cleanup flow, but live behavior depends on a real sandbox, Docker exec, /usr/bin/timeout availability/semantics, file ownership, and shields state.
• PRA-T2 Runtime validation — Add or identify targeted live-sandbox validation that post-exec cleanup refuses to weaken or repair mutable OpenClaw config permissions while shields posture is locked.. Changed code touches privileged Docker/OpenShell sandbox repair, file ownership/mode normalization, shields posture locking, and process timeouts. Unit coverage is strong for the extracted boundary and existing post-exec cleanup flow, but live behavior depends on a real sandbox, Docker exec, /usr/bin/timeout availability/semantics, file ownership, and shields state.
• PRA-T3 Runtime validation — Add or identify targeted live-sandbox validation that the privileged mutable-config normalizer runs with sanitized interpreter/loader environment and `/usr/bin/python3 -I`.. Changed code touches privileged Docker/OpenShell sandbox repair, file ownership/mode normalization, shields posture locking, and process timeouts. Unit coverage is strong for the extracted boundary and existing post-exec cleanup flow, but live behavior depends on a real sandbox, Docker exec, /usr/bin/timeout availability/semantics, file ownership, and shields state.
• PRA-T4 Runtime validation — Add or identify targeted live-sandbox validation that a hung or slow `normalize_mutable_config_perms.py` invocation is terminated by `/usr/bin/timeout --signal=TERM --kill-after=5s 15s` before the 25s host Docker timeout releases the shields transition lock.. Changed code touches privileged Docker/OpenShell sandbox repair, file ownership/mode normalization, shields posture locking, and process timeouts. Unit coverage is strong for the extracted boundary and existing post-exec cleanup flow, but live behavior depends on a real sandbox, Docker exec, /usr/bin/timeout availability/semantics, file ownership, and shields state.
• PRA-T5 Acceptance clause — No deterministic linked issue clauses were provided for this PR. — add test evidence or identify existing coverage. The validation context reports linkedIssues: []; PR-body references to fix(scripts): restore .openclaw perms after nemoclaw exec command #6060/[DGX Spark][Sandbox] openclaw doctor --fix collapses /sandbox/.openclaw permissions, breaking gateway group-writable contract #6047 were treated as untrusted evidence only and not converted into acceptance requirements.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 2 suggestions · 4 test follow-ups
Since last review: 5 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Add explicit test for 'root' username output from id command
• PRA-1 In-scope improvement: Add explicit test for 'root' username output from id command in src/lib/shields/mutable-config-repair.test.ts:154
• PRA-2 In-scope improvement: Consider deduplicating privileged exec helpers with index.ts in src/lib/shields/mutable-config-repair.ts:40

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Improvement	tests	src/lib/shields/mutable-config-repair.test.ts:154	Add a test case in mutable-config-repair.test.ts mocking dockerExecFileSync to return 'root\n' and expecting 'invalid UID' error, similar to existing invalid UID/GID tests.
PRA-2	Improvement	security	src/lib/shields/mutable-config-repair.ts:40	Optional: extract shared privileged exec helpers to a common module (e.g., sandbox/privileged-exec.ts) or import from index.ts. Not required for merge — current duplication is small and isolated.

Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

• None.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Add explicit test for 'root' username output from id command

• Location: src/lib/shields/mutable-config-repair.test.ts:154
• Category: tests
• Problem: The sandboxIdentityId function validates UID/GID with regex /^[1-9][0-9]*$/ which rejects numeric '0' and non-numeric strings. In some container environments, `id -u sandbox` could return the username 'root' instead of numeric '0'. While the regex would reject 'root' (good defense-in-depth), no test explicitly verifies this case.
• Impact: If id command returns 'root' instead of '0', the current test suite doesn't verify the rejection path. Low risk since regex already rejects it, but explicit test improves confidence.
• Suggested action: Add a test case in mutable-config-repair.test.ts mocking dockerExecFileSync to return 'root\n' and expecting 'invalid UID' error, similar to existing invalid UID/GID tests.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check mutable-config-repair.test.ts for a test mocking dockerExecFileSync returning 'root\n' and expecting throw with 'invalid UID'
• Missing regression test: Add test case: 'rejects root username from id command before the GID or normalizer runs'
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: mutable-config-repair.ts:27-33 shows regex validation; mutable-config-repair.test.ts:72-110 shows existing invalid UID/GID tests

PRA-2 Improvement — Consider deduplicating privileged exec helpers with index.ts

• Location: src/lib/shields/mutable-config-repair.ts:40
• Category: security
• Problem: mutable-config-repair.ts defines runPrivileged and privilegedExecCapture which duplicate the pattern from index.ts (privilegedSandboxExec / privilegedSandboxExecCapture). Current duplication is minimal (69 lines total in new module) and keeps module self-contained for testing. No security or correctness impact.
• Impact: Minor code duplication across two modules. If privileged exec logic changes (timeout, env sanitization), both places need updates.
• Suggested action: Optional: extract shared privileged exec helpers to a common module (e.g., sandbox/privileged-exec.ts) or import from index.ts. Not required for merge — current duplication is small and isolated.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare mutable-config-repair.ts:14-33 with index.ts:265-280 for runPrivileged/privilegedExecCapture vs privilegedSandboxExec/privilegedSandboxExecCapture
• Missing regression test: N/A — code quality improvement only
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Both modules construct docker exec argv via privilegedExecModule.privilegedSandboxExecArgv with same stdio/timeout options

Simplification opportunities: 1 possible cut, net -20 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-2 shrink (src/lib/shields/mutable-config-repair.ts:40): runPrivileged and privilegedExecCapture functions in mutable-config-repair.ts (lines 14-33)
  • Replacement: Import and reuse privilegedSandboxExec / privilegedSandboxExecCapture from index.ts or a new shared helper
  • Net: -20 lines
  • Safety boundary: Must preserve sanitizeEnvironment=true and stdin=false on all calls; must not change timeout behavior (15s identity, 25s normalizer)

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Add explicit test for 'root' username output from id command in mutable-config-repair.test.ts. Unit tests cover all extracted module boundaries (argv construction, UID/GID validation, error propagation). Runtime/sandbox/infrastructure paths (normalize_mutable_config_perms.py descriptor-based mutation, privileged docker exec, cross-process lock integration) need behavioral runtime validation in live sandbox/container environment.
• PRA-T2 Runtime validation — Integration test for normalize_mutable_config_perms.py with live sandbox container verifying descriptor-based chmod/chown/setgid. Unit tests cover all extracted module boundaries (argv construction, UID/GID validation, error propagation). Runtime/sandbox/infrastructure paths (normalize_mutable_config_perms.py descriptor-based mutation, privileged docker exec, cross-process lock integration) need behavioral runtime validation in live sandbox/container environment.
• PRA-T3 Runtime validation — Multi-process race test for withTimerBoundShieldsMutationLock under concurrent CLI invocations. Unit tests cover all extracted module boundaries (argv construction, UID/GID validation, error propagation). Runtime/sandbox/infrastructure paths (normalize_mutable_config_perms.py descriptor-based mutation, privileged docker exec, cross-process lock integration) need behavioral runtime validation in live sandbox/container environment.
• PRA-T4 Add explicit test for 'root' username output from id command — Add a test case in mutable-config-repair.test.ts mocking dockerExecFileSync to return 'root\n' and expecting 'invalid UID' error, similar to existing invalid UID/GID tests.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Add explicit test for 'root' username output from id command

• Location: src/lib/shields/mutable-config-repair.test.ts:154
• Category: tests
• Problem: The sandboxIdentityId function validates UID/GID with regex /^[1-9][0-9]*$/ which rejects numeric '0' and non-numeric strings. In some container environments, `id -u sandbox` could return the username 'root' instead of numeric '0'. While the regex would reject 'root' (good defense-in-depth), no test explicitly verifies this case.
• Impact: If id command returns 'root' instead of '0', the current test suite doesn't verify the rejection path. Low risk since regex already rejects it, but explicit test improves confidence.
• Suggested action: Add a test case in mutable-config-repair.test.ts mocking dockerExecFileSync to return 'root\n' and expecting 'invalid UID' error, similar to existing invalid UID/GID tests.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check mutable-config-repair.test.ts for a test mocking dockerExecFileSync returning 'root\n' and expecting throw with 'invalid UID'
• Missing regression test: Add test case: 'rejects root username from id command before the GID or normalizer runs'
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: mutable-config-repair.ts:27-33 shows regex validation; mutable-config-repair.test.ts:72-110 shows existing invalid UID/GID tests

PRA-2 Improvement — Consider deduplicating privileged exec helpers with index.ts

• Location: src/lib/shields/mutable-config-repair.ts:40
• Category: security
• Problem: mutable-config-repair.ts defines runPrivileged and privilegedExecCapture which duplicate the pattern from index.ts (privilegedSandboxExec / privilegedSandboxExecCapture). Current duplication is minimal (69 lines total in new module) and keeps module self-contained for testing. No security or correctness impact.
• Impact: Minor code duplication across two modules. If privileged exec logic changes (timeout, env sanitization), both places need updates.
• Suggested action: Optional: extract shared privileged exec helpers to a common module (e.g., sandbox/privileged-exec.ts) or import from index.ts. Not required for merge — current duplication is small and isolated.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare mutable-config-repair.ts:14-33 with index.ts:265-280 for runPrivileged/privilegedExecCapture vs privilegedSandboxExec/privilegedSandboxExecCapture
• Missing regression test: N/A — code quality improvement only
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Both modules construct docker exec argv via privilegedExecModule.privilegedSandboxExecArgv with same stdio/timeout options

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[cv]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

Vitest E2E Target Results — ✅ All requested jobs passed

Run: 28542337844
Workflow ref: codex/pr6060-advisor-followup
Requested targets: (default — all supported)
Requested jobs: shields-config
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
shields-config	✅ success

[cv]

Maintainer follow-up on exact head a32d292:

• The four previously listed Nemotron required findings were already resolved on the prior head; that sticky body itself marks each one RESOLVED.
• The optional PRA-5 point is now addressed with an explicit rationale for rejecting UID/GID 0.
• A fresh security review identified a host-timeout orphaning risk in the privileged mutable-config normalizer. This head adds an in-sandbox GNU timeout watchdog: TERM after 15s, KILL after 5s, with a 25s outer Docker timeout so the remote process group ends before the shields transition lock can be released.
• The watchdog failure path remains fail-closed, and absolute argv, isolated Python mode, sanitized privileged execution, non-root identity validation, and descriptor-safe normalization are preserved.

Validation: full pre-commit CLI/integration coverage and pre-push CLI typecheck passed; focused behavior tests passed 49/49; security call-chain tests passed 63/63; Biome and git diff checks passed. Independent runtime probes confirmed the process group is gone before the outer timeout. Commit a32d292 is signed, DCO-signed-off, and GitHub Verified.

@coderabbitai review

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd . src/lib/shields -e ts | head -50
echo "---mutable-config-repair.ts---"
cat -n src/lib/shields/mutable-config-repair.ts 2>/dev/null || true
echo "---exec.ts relevant excerpt---"
rg -n "timeout|TERM|KILL|watchdog" src/lib/actions/sandbox/exec.ts 2>/dev/null || true
echo "---normalize_mutable_config_perms.py---"
cat -n scripts/lib/normalize_mutable_config_perms.py 2>/dev/null || true

Length of output: 62936

---

@cv Thanks for the detailed follow-up. I inspected src/lib/shields/mutable-config-repair.ts on this head and can confirm the watchdog wiring matches your description:

• MUTABLE_CONFIG_NORMALIZER_WATCHDOG prepends /usr/bin/timeout --signal=TERM --kill-after=5s 15s ahead of the python3 -I invocation, and MUTABLE_CONFIG_NORMALIZER_HOST_TIMEOUT_MS = 25000 is passed as the outer dockerExecFileSync timeout. Worst case in-sandbox teardown (TERM at 15s + 5s KILL grace = 20s) leaves a comfortable margin under the 25s host timeout, so the remote process group should reliably be gone before the harder Docker-side timeout fires and before the shields transition lock is released.
• The PRA-5 rationale is now explicit in sandboxIdentityId(): rejecting any UID/GID that isn't strictly positive, with the comment noting this keeps the ownership target non-root so the privileged repair path can't become a confused-deputy vector.
• The absolute argv, -I isolated Python mode, privilegedSandboxExecArgv(..., false, true) sanitized privileged execution, and non-root identity validation are all intact and match the earlier resolved state.
• scripts/lib/normalize_mutable_config_perms.py's lock_recovery_baseline still only adds the explanatory comment on the pre-capture FileNotFoundError path — no logic changed there, consistent with your summary.

Everything checks out against the description. I'll kick off a fresh review pass on this head now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

Vitest E2E Target Results — ✅ All requested jobs passed

Run: 28549995589
Workflow ref: codex/pr6060-advisor-followup
Requested targets: (default — all supported)
Requested jobs: shields-config
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
shields-config	✅ success

[cv]

Advisor follow-up disposition for exact head a32d292:

• Runtime validation: required shields-config run 28549995589 passed on this head. It exercises the real sandbox exec cleanup, descriptor-safe chmod/chown normalization, mutable permission recovery, and shields locked/unlocked posture.
• Watchdog validation: exact argv and 15s/5s/25s budgets are pinned by unit/integration tests. Independent probes against the full PR runtime image verified GNU timeout availability and confirmed both Python parent and descendant exit before the outer deadline. Adding a production-only stall hook would broaden the privileged surface for no user-visible acceptance gain.
• The suggested root username case is not a distinct reachable id(1) behavior: /usr/bin/id -u always emits a numeric UID unless -n is supplied. The regex rejection branch is already covered by UID 0 and nonnumeric GID cases.
• The helper deduplication suggestion is intentionally deferred. index.ts imports mutable-config-repair.ts, so importing its helpers back would create a cycle; extracting another shared module would expand this security follow-up and couple call sites with intentionally different timeout budgets. The current helpers are small, local, and explicitly preserve sanitized privileged execution.
• Transition serialization already has lock contention/reentrancy/fail-closed coverage; this patch specifically prevents the remote worker from outliving that lock.
• Acceptance evidence is inherited from the fix(scripts): restore .openclaw perms after nemoclaw exec command #6060 follow-up for [DGX Spark][Sandbox] openclaw doctor --fix collapses /sandbox/.openclaw permissions, breaking gateway group-writable contract #6047 and is covered by the exact-head CI and live E2E above.

Both advisors now recommend merge_as_is with zero required findings and zero warnings. CodeRabbit is green with no review threads; all latest exact-head check runs are passing. The only remaining branch-protection gate is the requested human approval.