Skip to content

test(shields): cover retained descriptor after hash repair - #7396

Merged
prekshivyas merged 8 commits into
NVIDIA:mainfrom
RUSHcitizen:fix/7382-guard-preserve-config
Jul 25, 2026
Merged

test(shields): cover retained descriptor after hash repair#7396
prekshivyas merged 8 commits into
NVIDIA:mainfrom
RUSHcitizen:fix/7382-guard-preserve-config

Conversation

@RUSHcitizen

@RUSHcitizen RUSHcitizen commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds regression coverage proving that an absent-hash lock replaces openclaw.json before sealing, so a descriptor opened before the lock cannot mutate the canonical config afterward. The production repair and broader recovery coverage already merged in #7467; this conflict resolution narrows #7396 to the remaining security invariant identified by automated review.

Related Issue

Follow-up coverage for #7382. The implementation merged in #7467.

Changes

  • Opens openclaw.json for writing before removing .config-hash.
  • Locks the config, then writes through the retained descriptor.
  • Verifies the canonical config and regenerated hash remain unchanged on fresh sealed inodes.

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: Test-only hardening for an existing documented repair; no user-visible behavior or interface changes.
  • 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: NemoClaw nine-category security review found no findings; the test-only diff strengthens descriptor/TOCTOU coverage without changing production behavior.
  • 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: origin/main...792ac3b581eca45d2c69d491bb2f99fd1e71e2b3 changes only test/openclaw-config-guard-absent-hash.test.ts; existing docs already describe fresh sealed config inodes and absent-hash synthesis. The focused suite passes 11/11, the combined guard suites pass 55/55, and Biome, diff check, pre-commit, commit-msg, and pre-push pass.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable
  • Supporting evidence: Not applicable

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
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — vitest run --project integration test/openclaw-config-guard.test.ts test/openclaw-config-guard-absent-hash.test.ts: 55/55 passed; the descriptor-safe follow-up passes its focused suite 11/11.
  • Applicable broad gate passed — not applicable to this one-test security regression addition.
  • 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)

Signed-off-by: Atharv Kumaria kumariaaatharv@gmail.com
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com

_force_fail_closed_lock's "no bounded pair could be captured" branch
severed both canonical config files unconditionally, conflating two
different situations: openclaw.json itself being untrustworthy (a
real tampering signal) versus only its sibling .config-hash being
missing or raced after an interrupted transition. The latter is
benign drift, but the sever path destroyed the still-healthy
openclaw.json either way, leaving the sandbox shields state
unrecoverable without a rebuild.

Attempt a single-file capture of openclaw.json via the existing
_snapshot_file identity contract (regular, single-link, bounded,
same-device, stable reads) before severing. If it passes, rebuild the
canonical pair from those bytes with _force_replace_bytes, which
never trusts the old inode and republishes onto a fresh root-owned
one, so no previously opened writable descriptor survives. Best-effort
JSON5-validate the preserved bytes; containment still succeeds if
validation fails. Only fall back to severing both canonical paths
when openclaw.json itself cannot be captured.

New coverage lives in its own file rather than growing
test/openclaw-config-guard.test.ts past the repo's test-file-size
budget; it duplicates that file's fixture/runGuard harness rather
than extracting a shared module, matching the existing split between
test/hermes-runtime-config-guard.test.ts and its -topology sibling.

Signed-off-by: Atharv Kumaria <kumariaaatharv@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The fail-closed lock path now reconstructs the canonical configuration pair when possible, validates it, and otherwise severs canonical entries. A new Vitest integration suite exercises missing-hash recovery, idempotence, symlink rejection, permissions, and cleanup.

Changes

Fail-closed lock recovery

Layer / File(s) Summary
Canonical pair recovery and fallback severing
scripts/openclaw-config-guard.py
The guard rebuilds openclaw.json and .config-hash from freshly captured bytes, validates the rebuilt JSON5 configuration, and falls back to rejected markers when capture fails.
Sandboxed lock behavior validation
test/openclaw-config-guard-fail-closed-lock.test.ts
Integration tests run the guard in controlled fixtures and verify hash reconstruction, read-only modes, idempotent locking, symlink handling, and rejected markers.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Guard as _force_fail_closed_lock
  participant Filesystem
  participant Validator as JSON5 runtime validation
  Guard->>Filesystem: Capture openclaw.json
  Guard->>Filesystem: Replace openclaw.json and .config-hash
  Filesystem-->>Guard: Return rebuilt canonical pair
  Guard->>Validator: Validate rebuilt openclaw.json
  Guard->>Filesystem: Rename canonical paths to rejected markers if capture fails
Loading

Suggested labels: area: security

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is relevant and specific to the new fail-closed lock behavior and its test coverage, though it emphasizes tests more than the code fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections match; severity counts match.

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
  • security-posture

Workflow run details

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

@wscurran wscurran added area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior labels Jul 23, 2026
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for the fix. This preserves openclaw.json when it passes its own identity contract, avoiding the destructive rollback when only .config-hash is missing. Maintainers will review the sandbox shields logic and security implications.


Related open issues:


Related open PRs:


Related open issues:

@prekshivyas prekshivyas self-assigned this Jul 25, 2026
@apurvvkumaria
apurvvkumaria self-requested a review July 25, 2026 05:57
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas prekshivyas changed the title fix(shields): preserve openclaw.json when only .config-hash is missing test(shields): cover retained descriptor after hash repair Jul 25, 2026
Comment thread test/openclaw-config-guard-absent-hash.test.ts Fixed
Comment thread test/openclaw-config-guard-absent-hash.test.ts Fixed
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Comment thread test/openclaw-config-guard-absent-hash.test.ts Fixed
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Comment thread test/openclaw-config-guard-absent-hash.test.ts Fixed
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Comment thread test/openclaw-config-guard-absent-hash.test.ts Fixed
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

@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.

Reviewed final test-only salvage at 792ac3b. The production fix was superseded by #7467; this PR now preserves only the retained writable-descriptor regression coverage. Focused tests pass 11/11, combined guard suites pass 55/55, all 41 current checks are green, the documentation review is no-docs-needed, DCO is present, and all commits are verified. Security review: PASS for secrets, input validation, authorization, dependencies, error handling/logging, cryptography/data protection, configuration, security testing, and holistic system security. No actionable findings remain.

@prekshivyas
prekshivyas merged commit 6e8d21a into NVIDIA:main Jul 25, 2026
41 checks passed
@cv cv mentioned this pull request Jul 26, 2026
23 tasks
apurvvkumaria pushed a commit that referenced this pull request Jul 27, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the
exact `## v0.0.96` heading.
The entry reconciles all 90 first-parent commits since v0.0.95 with all
92 merged PRs in the live `v0.0.96` label ledger and groups the
user-visible changes by operator journey.

## Changes

- Add the parser-safe dated MDX changelog entry for v0.0.96 with
root-absolute links to the focused user guides.
- Source summary:
- [#7194](#7194) ->
`docs/changelog/2026-07-25.mdx`: Document persistent baseline network
policy exclusions and their inspection, rebuild, and snapshot behavior.
- [#7188](#7188),
[#7427](#7427), and
[#7546](#7546) ->
`docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference
routing, keyless loopback endpoints, and provider-marker isolation.
- [#7238](#7238) ->
`docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider
identifier validation before state writes or OpenShell calls, with
bounded terminal-safe rejection previews.
- [#7319](#7319),
[#7274](#7274),
[#7528](#7528),
[#7353](#7353), and
[#7560](#7560) ->
`docs/changelog/2026-07-25.mdx`: Document the managed default gateway
service, onboarding readiness, and container-runtime identity
safeguards.
- [#7349](#7349),
[#7498](#7498),
[#7406](#7406),
[#7196](#7196),
[#7559](#7559),
[#7421](#7421),
[#7510](#7510),
[#7295](#7295), and
[#7565](#7565) ->
`docs/changelog/2026-07-25.mdx`: Document gateway-scoped status,
lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and
fail-closed CLI prompt and command output.
- [#7591](#7591) ->
`docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP
tool-name discovery, its bounded and names-only contract, probe
interaction, and rebuild requirement.
- [#7305](#7305),
[#7480](#7480),
[#7471](#7471),
[#7365](#7365), and
[#7541](#7541) ->
`docs/changelog/2026-07-25.mdx`: Document installer version checks,
version-tag reporting, license guidance, WSL Ollama selection, and DGX
Station vLLM detection.
- [#7482](#7482),
[#7466](#7466),
[#7208](#7208),
[#7434](#7434), and
[#7586](#7586) ->
`docs/changelog/2026-07-25.mdx`: Document Ollama resource details,
reasoning precedence, Hermes onboarding behavior, and preserved managed
Hermes BuildKit failures.

- [#6830](#6830),
[#7492](#7492),
[#7563](#7563), and
[#7582](#7582) ->
`docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw
production lock, fixed managed-image dependencies, immutable Hermes base
adoption, and Hermes image-size reduction.
- [#7505](#7505),
[#7530](#7530),
[#7547](#7547),
[#7508](#7508),
[#7548](#7548),
[#7549](#7549),
[#7537](#7537),
[#7534](#7534),
[#7515](#7515),
[#7511](#7511),
[#7551](#7551),
[#7562](#7562),
[#7575](#7575),
[#7496](#7496),
[#7594](#7594),
[#7595](#7595), and
[#7599](#7599) ->
`docs/changelog/2026-07-25.mdx`: Summarize release validation, transient
and bounded dispatch reconciliation, exact pre-tag qualification,
identity revalidation, npm-audit retry, sharding, image reuse, timeout,
telemetry, and workflow-hardening changes.
- Reconciled without separate changelog prose:
- [#7539](#7539),
[#7526](#7526),
[#7507](#7507),
[#7506](#7506),
[#7519](#7519),
[#7516](#7516),
[#7396](#7396),
[#7254](#7254),
[#7583](#7583),
[#7596](#7596), and
[#7598](#7598): Test-harness or
fixture-only changes.
- [#7403](#7403),
[#7161](#7161),
[#6877](#6877),
[#7531](#7531),
[#7525](#7525),
[#7522](#7522),
[#7536](#7536),
[#7552](#7552),
[#7566](#7566),
[#7553](#7553),
[#7561](#7561),
[#7577](#7577),
[#7569](#7569),
[#7585](#7585),
[#7584](#7584),
[#7592](#7592),
[#7580](#7580),
[#7571](#7571),
[#7517](#7517),
[#7589](#7589),
[#7402](#7402),
[#7558](#7558),
[#7544](#7544), and
[#7601](#7601): Dependency,
internal recovery, validation, contributor-workflow, E2E optimization,
telemetry, or CI trust changes with no separate user-facing release
claim.
- [#7556](#7556),
[#7573](#7573),
[#7576](#7576), and
[#7578](#7578): Experimental
repository-maintainer conflict automation with no canonical user
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:
`test/changelog-docs.test.ts` validates dated changelog structure,
version headings, and published links.
- [ ] 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: Reviewed `docs/changelog/2026-07-25.mdx` at exact head
`0f5dedb47` against 90 first-parent release commits and 92 merged PRs
labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version
heading, literal CLI names, writing style, skip terms, all 20
root-absolute published links, and the accepted #7591 opt-in
authenticated discovery bounds. #7544, #7599, and #7601 remain internal
or CI-only release-ledger entries. Changelog tests passed 6/6, the docs
build passed with 0 errors and two pre-existing Fern warnings, and `npm
run check:diff` plus the final diff check passed.
- Agent: Codex Desktop documentation-writer subagent
<!-- docs-review-head-sha: 0f5dedb -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## 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 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 — `npx vitest run
test/changelog-docs.test.ts`: 6/6 passed.
- [ ] 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 this
prose-only changelog 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) — the
build passed with 0 errors and 2 existing Fern warnings; the
published-route check passed.
- [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)
— native changelog files use the required parser-safe MDX SPDX comment
and no frontmatter.

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


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

* **New Features**
* Persistent network policy exclusions with consistent restore/exclusion
reporting across rebuilds/snapshots.
* Opt-in MCP tool discovery via `mcp status --tools` with bounded,
redacted authenticated traffic.
* Improved HTTPS inference switching for custom endpoints and refreshed
onboarding/model menu details.
* Refined OpenShell gateway defaults for port `8080`, including more
reliable readiness checks.
* **Bug Fixes**
* Prevent incorrect provider/model restoration after compatible-provider
update failures.
* Preserve managed MCP state after exec loss and tighten gateway/doctor
status scoping.
* **Tests**
* Stronger, fail-closed release validation with hardened
evidence/artifact handoff and bounded timeouts/retries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: security Security controls, permissions, secrets, or hardening bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants