Skip to content

fix(openclaw): fail unsafe config restore fallbacks#5177

Merged
jyaunches merged 8 commits into
mainfrom
followup/5174-openclaw-restore-feedback
Jun 11, 2026
Merged

fix(openclaw): fail unsafe config restore fallbacks#5177
jyaunches merged 8 commits into
mainfrom
followup/5174-openclaw-restore-feedback

Conversation

@jyaunches

@jyaunches jyaunches commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Follow-up to PR #5174 addressing PR Review Advisor feedback that unsafe fallback paths could wholesale restore sanitized backup openclaw.json over fresh runtime-owned config.

Changes:

  • fail OpenClaw openclaw.json restore when current rebuilt config is missing/unreadable or either JSON parse fails
  • move restore input/read/source-of-truth policy into a focused helper module
  • keep current provider/plugin entries for matching keys while preserving backup-only custom entries, including absent current entry-map coverage
  • add regression tests for missing current config, invalid current JSON, invalid backup JSON, provider/plugin edge cases, and fake-SSH restore behavior

Validation:

  • npm run build:cli
  • npx vitest run src/lib/state/openclaw-config-merge.test.ts src/lib/state/openclaw-config-restore-input.test.ts
  • npx vitest run test/snapshot.test.ts -t "backs up and restores openclaw.json settings|excludes tar-failed" --testTimeout=15000
  • npm run checks -- --changed

Summary by CodeRabbit

  • New Features

    • Selective-merge for OpenClaw restores: prefers current runtime-generated config, preserves backup-only entries, and recomputes/stores sandbox config hash on successful restores.
  • Bug Fixes

    • Prevent stale backup data from overwriting rebuilt/runtime-managed OpenClaw settings; restore now fails if a safe merge cannot be constructed.
  • Tests

    • Added extensive tests for merge logic, restore-input generation, sandbox restore flows, and snapshot/ssh restore scenarios.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 3 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 0 still apply, 1 new item found

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: src/lib/state/openclaw-config-restore-input.ts: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The module comment explicitly documents the invalid state, source-fix constraint, and removal condition; the remaining gap is runtime negative coverage for the fail-closed behavior.
  • Source-of-truth review needed: src/lib/state/sandbox.ts: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: restoreStateFile returns false when buildStateFileRestoreInput returns null and restoreSandboxState then pushes spec.path to failedFiles, but current tests do not directly assert that branch.
  • Add runtime negative coverage for fail-closed openclaw.json restore (src/lib/state/sandbox.ts:1540): The helper-level fail-closed cases are covered, but the changed runtime restore contract is only indirectly exercised. Because this path protects runtime-owned OpenClaw config and credentials from unsafe fallback restores, a fake-SSH/runtime test should prove that restoreSandboxState records openclaw.json in failedFiles and leaves the current file untouched when the selective merge cannot be built.
    • Recommendation: Add targeted fake-SSH restore tests for missing current openclaw.json, invalid current JSON, invalid backed-up JSON, and symlink/non-regular current openclaw.json, asserting restore.success is false, failedFiles contains openclaw.json, and the current sandbox file is preserved.
    • Evidence: src/lib/state/openclaw-config-restore-input.test.ts covers buildOpenClawConfigRestoreInput(null) and JSON parse failures, while test/openclaw-config-snapshot.test.ts covers the successful fake-SSH restore/hash-refresh path. The runtime failure branch flows through buildStateFileRestoreInput returning null, restoreStateFile returning false, and restoreSandboxState pushing spec.path to failedFiles, but that behavior is not directly asserted.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — restoreSandboxState marks openclaw.json failed and leaves current openclaw.json unchanged when the current rebuilt config is missing or the read command exits 2.. The main behavior changes runtime sandbox restore semantics for a security-sensitive OpenClaw config file. Unit tests cover the helper and merge logic, and one fake-SSH test covers the successful restore/hash-refresh path, but negative runtime restore behavior remains unproven.
  • **Runtime validation** — restoreSandboxState marks openclaw.json failed and leaves current openclaw.json unchanged when the current rebuilt config is invalid JSON.. The main behavior changes runtime sandbox restore semantics for a security-sensitive OpenClaw config file. Unit tests cover the helper and merge logic, and one fake-SSH test covers the successful restore/hash-refresh path, but negative runtime restore behavior remains unproven.
  • **Runtime validation** — restoreSandboxState marks openclaw.json failed and leaves current openclaw.json unchanged when the backed-up openclaw.json is invalid JSON.. The main behavior changes runtime sandbox restore semantics for a security-sensitive OpenClaw config file. Unit tests cover the helper and merge logic, and one fake-SSH test covers the successful restore/hash-refresh path, but negative runtime restore behavior remains unproven.
  • **Runtime validation** — restoreSandboxState marks openclaw.json failed when buildOpenClawConfigReadCommand rejects a symlinked or non-regular current openclaw.json.. The main behavior changes runtime sandbox restore semantics for a security-sensitive OpenClaw config file. Unit tests cover the helper and merge logic, and one fake-SSH test covers the successful restore/hash-refresh path, but negative runtime restore behavior remains unproven.
  • **Runtime validation** — If intended by the ownership contract, mergeOpenClawRestoredConfig preserves current runtime-owned top-level models/plugins settings when the backup contains stale values outside providers/entries.. The main behavior changes runtime sandbox restore semantics for a security-sensitive OpenClaw config file. Unit tests cover the helper and merge logic, and one fake-SSH test covers the successful restore/hash-refresh path, but negative runtime restore behavior remains unproven.
  • **Add runtime negative coverage for fail-closed openclaw.json restore** — Add targeted fake-SSH restore tests for missing current openclaw.json, invalid current JSON, invalid backed-up JSON, and symlink/non-regular current openclaw.json, asserting restore.success is false, failedFiles contains openclaw.json, and the current sandbox file is preserved.
  • **Acceptance clause:** No linked issue acceptance clauses were provided in the deterministic context. — add test evidence or identify existing coverage. linkedIssues is empty for PR fix(openclaw): fail unsafe config restore fallbacks #5177, so there are no issue/comment clauses to map literally. PR-body claims were treated as untrusted supporting context rather than acceptance criteria.
  • **Acceptance clause:** PR body claim: "fail OpenClaw openclaw.json restore when current rebuilt config is missing/unreadable or either JSON parse fails" — add test evidence or identify existing coverage. buildOpenClawConfigRestoreInput fails on missing currentContents and JSON.parse errors; unit tests cover missing current, invalid current JSON, and invalid backup JSON. Runtime restore failure accounting for unreadable/symlinked/missing current config is not directly tested.
Since last review details

Current findings:

  • Source-of-truth review needed: src/lib/state/openclaw-config-restore-input.ts: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The module comment explicitly documents the invalid state, source-fix constraint, and removal condition; the remaining gap is runtime negative coverage for the fail-closed behavior.
  • Source-of-truth review needed: src/lib/state/sandbox.ts: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: restoreStateFile returns false when buildStateFileRestoreInput returns null and restoreSandboxState then pushes spec.path to failedFiles, but current tests do not directly assert that branch.
  • Add runtime negative coverage for fail-closed openclaw.json restore (src/lib/state/sandbox.ts:1540): The helper-level fail-closed cases are covered, but the changed runtime restore contract is only indirectly exercised. Because this path protects runtime-owned OpenClaw config and credentials from unsafe fallback restores, a fake-SSH/runtime test should prove that restoreSandboxState records openclaw.json in failedFiles and leaves the current file untouched when the selective merge cannot be built.
    • Recommendation: Add targeted fake-SSH restore tests for missing current openclaw.json, invalid current JSON, invalid backed-up JSON, and symlink/non-regular current openclaw.json, asserting restore.success is false, failedFiles contains openclaw.json, and the current sandbox file is preserved.
    • Evidence: src/lib/state/openclaw-config-restore-input.test.ts covers buildOpenClawConfigRestoreInput(null) and JSON parse failures, while test/openclaw-config-snapshot.test.ts covers the successful fake-SSH restore/hash-refresh path. The runtime failure branch flows through buildStateFileRestoreInput returning null, restoreStateFile returning false, and restoreSandboxState pushing spec.path to failedFiles, but that behavior is not directly asserted.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Failed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-advisor-raw-output.txt

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Failed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-scenario-advisor-raw-output.txt

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 27306740873
Target ref: 69bfdaa7b1620e73030d64a5aa7d10b8b5815307
Workflow ref: main
Requested jobs: rebuild-openclaw-e2e,snapshot-commands-e2e
Summary: 1 passed, 1 failed, 0 skipped

Job Result
rebuild-openclaw-e2e ❌ failure
snapshot-commands-e2e ✅ success

Failed jobs: rebuild-openclaw-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27307992758
Target ref: 69bfdaa7b1620e73030d64a5aa7d10b8b5815307
Workflow ref: main
Requested jobs: rebuild-openclaw-e2e,channels-add-remove-e2e,channels-stop-start-e2e,messaging-providers-e2e
Summary: 0 passed, 0 failed, 0 skipped

Job Result
channels-add-remove-e2e ⚠️ cancelled
channels-stop-start-e2e ⚠️ cancelled
messaging-providers-e2e ⚠️ cancelled
rebuild-openclaw-e2e ⚠️ cancelled

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d2b94144-ef52-4167-9456-c3cfa3b464b1

📥 Commits

Reviewing files that changed from the base of the PR and between c7e96f1 and 9f4fd59.

📒 Files selected for processing (1)
  • test/snapshot.test.ts
💤 Files with no reviewable changes (1)
  • test/snapshot.test.ts

📝 Walkthrough

Walkthrough

Refactors OpenClaw openclaw.json restoration: adds an entry-map merge helper, a sandbox-aware restore-input builder that reads current config over SSH and merges selectively, integrates the builder into sandbox restore (failing on unsafe merges), and expands tests covering policies and end-to-end backup/restore.

Changes

OpenClaw config restore refactor

Layer / File(s) Summary
Entry-map merge helper
src/lib/state/openclaw-config-merge.ts, src/lib/state/openclaw-config-merge.test.ts
mergeOpenClawEntryMap combines backup and current entry maps with current values winning. mergeOpenClawModels and mergeOpenClawPlugins now use this helper for providers and entries respectively instead of inline spread logic.
Restore-input builder with SSH read
src/lib/state/openclaw-config-restore-input.ts, src/lib/state/openclaw-config-restore-input.test.ts
New module defines merge policy (shouldMergeOpenClawConfigStateFile), constructs SSH commands for safe remote reads (buildOpenClawConfigReadCommand), reads current config via spawnSync, parses both configs, merges via mergeOpenClawRestoredConfig, and returns formatted JSON buffer or error. Tests validate policy boundaries and error handling.
Sandbox restore integration
src/lib/state/sandbox.ts
Refactors buildStateFileRestoreInput to return `Buffer
End-to-end and snapshot test updates
test/openclaw-config-snapshot.test.ts, test/snapshot-gateway-guard.test.ts, test/snapshot.test.ts
Adds openclaw-config-snapshot.test.ts validating full backup/restore cycle with secrets sanitized and runtime state preserved. Existing snapshot tests add SSH mocking for openclaw.json handling, updated fake openshell behavior, and explicit timeouts. Some legacy/duplicate OpenClaw test blocks were removed or reshaped.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5174: Both PRs modify src/lib/state/openclaw-config-merge.ts's merge logic for models.providers and plugins.entries; this PR refines entry-map merge behavior and adds targeted tests.

Suggested labels

integration: openclaw, area: sandbox, bug-fix, v0.0.63

Suggested reviewers

  • cv
  • prekshivyas

Poem

🐰 I hopped through backup lines and SSH streams,
Merged maps with care so runtime rules the dreams.
Tests frisk and pass, the hash aligns just so,
Stale data shrugged off — fresh config gets to glow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(openclaw): fail unsafe config restore fallbacks' directly and clearly summarizes the main change: preventing unsafe fallback behavior in OpenClaw config restoration.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 followup/5174-openclaw-restore-feedback

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

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 27308242519
Target ref: followup/5174-openclaw-restore-feedback
Requested jobs: rebuild-openclaw-e2e,channels-add-remove-e2e,channels-stop-start-e2e,messaging-providers-e2e
Summary: 3 passed, 1 failed, 0 skipped

Job Result
channels-add-remove-e2e ✅ success
channels-stop-start-e2e ✅ success
messaging-providers-e2e ✅ success
rebuild-openclaw-e2e ❌ failure

Failed jobs: rebuild-openclaw-e2e. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27311545942
Target ref: followup/5174-openclaw-restore-feedback
Requested jobs: rebuild-openclaw-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
rebuild-openclaw-e2e ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27311705063
Target ref: c7e96f176e1e6665b8f2d84f81bea93ed53a311e
Workflow ref: main
Requested jobs: snapshot-commands-e2e,rebuild-openclaw-e2e,state-backup-restore-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
rebuild-openclaw-e2e ⚠️ cancelled
snapshot-commands-e2e ✅ success
state-backup-restore-e2e ⚠️ cancelled

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27312231293
Target ref: 9f4fd594fb28ac204db00696542db5d7a959f909
Workflow ref: main
Requested jobs: snapshot-commands-e2e,rebuild-openclaw-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
rebuild-openclaw-e2e ⚠️ cancelled
snapshot-commands-e2e ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27312583423
Target ref: f9e04358cfa4f0e4bb20920bfd3018a137ef37df
Workflow ref: main
Requested jobs: snapshot-commands-e2e,rebuild-openclaw-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
rebuild-openclaw-e2e ✅ success
snapshot-commands-e2e ✅ success

@jyaunches jyaunches merged commit 0f99ee5 into main Jun 11, 2026
39 checks passed
@jyaunches jyaunches deleted the followup/5174-openclaw-restore-feedback branch June 11, 2026 00:19
miyoungc added a commit that referenced this pull request Jun 11, 2026
## Summary
- Add the v0.0.63 release-note section using the published development
note as source context.
- Update source docs for sandbox recovery, OpenClaw config restore
safety, managed vLLM selection, Slack Socket Mode conflict handling, and
host diagnostics.
- Refresh generated `nemoclaw-user-*` skills from the updated Fern MDX
docs.
- Update the release-doc refresh skill so post-release docs for version
`n` look up the matching announcement discussion and use the `n+1` patch
release label.
- Fix CLI/docs parity by avoiding a `--from <Dockerfile>` flag mention
inside the `upgrade-sandboxes` command section.

## Source summary
- #5034 -> `docs/reference/troubleshooting.mdx`,
`docs/about/release-notes.mdx`: Document safer stale-sandbox recovery
through `rebuild --yes` before recreating from scratch.
- #5091 -> `docs/reference/troubleshooting.mdx`,
`docs/about/release-notes.mdx`: Document Docker-driver post-reboot
recovery from OpenShell container labels.
- #5101, #5174, #5177 -> `docs/manage-sandboxes/backup-restore.mdx`,
`docs/about/release-notes.mdx`: Document OpenClaw `openclaw.json`
preservation, merge behavior, and fail-safe restore handling.
- #5102 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`,
`docs/manage-sandboxes/lifecycle.mdx`, `docs/about/release-notes.mdx`:
Document `upgrade-sandboxes` image-fingerprint drift detection.
- #4201 -> `docs/reference/troubleshooting.mdx`,
`docs/about/release-notes.mdx`: Document the installer diagnostic for
unexpected Docker daemon access outside the `docker` group.
- #5038 -> `docs/inference/inference-options.mdx`,
`docs/inference/use-local-inference.mdx`,
`docs/about/release-notes.mdx`: Document the interactive managed-vLLM
model picker and non-interactive override behavior.
- #5040, #5041 -> `docs/reference/troubleshooting.mdx`,
`docs/about/release-notes.mdx`: Document Ollama auth-proxy recovery and
host DNS preflight diagnostics.
- #4986, #5039 -> `docs/manage-sandboxes/messaging-channels.mdx`,
`docs/about/release-notes.mdx`: Document Slack validation and duplicate
Slack Socket Mode sandbox handling.
- #4981, #5168 -> `docs/about/release-notes.mdx`: Capture Hermes gateway
secret-guard and wrapped-argv startup hardening in the release surface.
- Follow-up ->
`.agents/skills/nemoclaw-contributor-update-docs/SKILL.md`: Record the
post-release docs workflow, discussion-announcement lookup, and
next-patch release label rule.
- Follow-up -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`: Reword custom Dockerfile
sandbox text so CLI parity does not treat `--from` as an
`upgrade-sandboxes` flag.

## Verification
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix
nemoclaw-user --doc-platform fern-mdx`
- `npm run docs`
- `npm run build:cli`
- `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli`
- Skip-term scan for `docs/.docs-skip` blocked terms across generated
user skills

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

## Summary by CodeRabbit

* **Documentation**
* Enhanced local inference setup with interactive model selection
prompts and environment variable overrides
* Improved sandbox upgrade detection using build fingerprints and
version checks
* Clarified configuration restore behavior preserving user settings
during rebuild/restore
  * Added gateway authentication as fifth security layer
  * Expanded Slack messaging validation with live credential checking
* Enhanced troubleshooting guidance for Docker access, DNS issues, and
sandbox recovery
* Updated release notes for v0.0.63 featuring sandbox recovery and
inference improvements

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants