fix(policy): refuse to apply a preset when the live policy could not be read

[latenighthackathon]

Summary

When policy get succeeds but returns output NemoClaw cannot parse (an error or status line, or otherwise unexpected content), policy-add and the onboard preset apply rebuild the policy from only the preset being applied and write that back, silently dropping every other preset already in place along with the filesystem and process rules. A full gateway outage is safe, since the follow-up policy set also fails and nothing is written; the damage happens when the gateway is healthy enough to accept the write but returned a degraded read. The matching remove command already refuses to act on a policy it could not read; the apply paths did not.

Related Issue

Fixes #4586

Changes

• applyPresetContent and applyPresets now stop and report when the live policy read comes back non-empty but unparseable, instead of overwriting it. A genuinely empty read, which is the normal case for a fresh sandbox with no policy yet, still applies the first preset as before. This matches the guard the remove path already had.
• Added regression tests in test/policies.test.ts for the degraded-read abort (both functions) and the fresh-sandbox happy path.

Type of Change

• Code change (feature, bug fix, or refactor)

Verification

• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed

Ran npm run build:cli, npm run typecheck:cli, npx @biomejs/biome lint, and the full policies suite (154/154). The new tests fail before the change and pass after.

Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

Summary by CodeRabbit

• Bug Fixes

  • Preset application now aborts when the current sandbox policy cannot be read or validated, preventing accidental overwrites during error conditions.

• Tests

  • Added regression tests ensuring preset application fails safely when the live sandbox policy is unreadable and succeeds for a fresh sandbox.

[copy-pr-bot]

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]

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: e7ce658c-02ce-4e48-8235-82c730463795

📥 Commits

Reviewing files that changed from the base of the PR and between 1ad657e and 56610a5.

📒 Files selected for processing (2)

• src/lib/policy/index.ts
• test/policies.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/lib/policy/index.ts
• test/policies.test.ts

---

📝 Walkthrough

Walkthrough

applyPresetContent and applyPresets now detect when a captured policy read is non-empty but fails parsing and abort the apply (log an error and return false) instead of overwriting the sandbox policy. Regression tests for issue #4586 cover degraded and empty read cases.

Changes

Policy Safety Guard for Preset Application

Layer / File(s)	Summary
Add validation guards to preset application functions src/lib/policy/index.ts	applyPresetContent and applyPresets now check runCapture output and parseCurrentPolicy; if the raw output is non-empty but parsing yields an empty/invalid policy, they log an error and return false without applying presets.
Regression tests for issue #4586 test/policies.test.ts	New tests with a mocked OpenShell binary and stubbed registry functions verify that degraded policy get output causes applyPresetContent and applyPresets to return false (and log unreadable-policy error), while genuinely empty reads allow applying presets (true and "Applied preset" log).

Sequence Diagram(s)

sequenceDiagram
  participant applyPreset as applyPresetContent/applyPresets
  participant OpenShell as OpenShell (runCapture / `policy get`)
  participant Parser as parseCurrentPolicy
  participant Registry as registry.addCustomPolicy / policy set

  applyPreset->>OpenShell: runCapture("policy get") -> rawOutput
  OpenShell-->>applyPreset: rawOutput
  applyPreset->>Parser: parseCurrentPolicy(rawOutput)
  Parser-->>applyPreset: parsedPolicy (empty when unreadable)
  alt parsedPolicy non-empty
    applyPreset->>Registry: merge & policy set (apply preset)
    Registry-->>applyPreset: success
  else parsedPolicy empty but rawOutput non-empty
    applyPreset-->>applyPreset: log error "could not read current policy"
    applyPreset-->>Registry: abort (return false)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4576: Changes in src/lib/policy/index.ts related to returning false under specific failure conditions during preset application.

Suggested labels

fix, enhancement: policy

Suggested reviewers

• cv

Poem

🐰 I hopped into the sandbox light,
Found tangled reads that hid from sight.
So I stitched a guard, soft and bright —
Presets safe through stormy night.
✨

🚥 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 and concisely summarizes the main change: refusing to apply presets when the live policy cannot be read.
Linked Issues check	✅ Passed	The PR implementation fully addresses all objectives from issue #4586: guards against applying presets when policy reads fail parseCurrentPolicy, distinguishes empty from degraded reads, and preserves fresh sandbox behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #4586: modifications to applyPresetContent and applyPresets logic, plus comprehensive regression tests verifying the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[wscurran]

✨ Thanks for submitting this detailed PR about refusing to apply a preset when the live policy could not be read. This proposes a fix for a bug in the NemoClaw CLI that prevents silent overwriting of existing presets and policy rules.

---

Related open issues:

• #4586 policy-add and onboard preset apply overwrite the live policy when policy get returns degraded exit-0 output

[prekshivyas]

LGTM — mirrors the existing remove-path guard in applyPresetContent / applyPresets, narrow scope, and the new regression tests cover both apply functions plus the fresh-sandbox happy path. CI green on 1ad657e, no CodeRabbit threads.