Commit 3606cb4
authored
fix(policy): remove a preset the gateway enforces without a local record (#9306)
<!-- 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. -->
`policy list` reports a preset as active when either the local registry
or the live gateway holds it, but `policy remove` consulted the registry
alone and exited non-zero with `Preset 'X' is not applied.`, so the one
state `policy list` exists to flag — active on gateway, missing from
local state — was the only state with no removal path. `policy remove`
now builds a single removable set from both sources and uses it for the
named-preset guard and the interactive picker alike, and it
distinguishes a gateway it could not query from a gateway that does not
hold the preset. Applying a built-in preset to a sandbox that is missing
from the registry no longer returns success in silence, which closes one
way that divergence is created.
## Related Issue
<!-- Fixes #NNN or Closes #NNN. Remove this section if none. -->
Fixes #9295
## 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. -->
- `removeSandboxPolicyUnlocked` in
`src/lib/actions/sandbox/policy-channel.ts` derives one `removable` set
from `getAppliedPresets` and `getGatewayPresets`, and both the
named-preset guard and the `selectForRemoval` picker read it. Computing
it once is what keeps the two entry points from disagreeing, which is
the shape of the reported defect. `removePreset` already reconciles a
gateway-held preset without a registry entry, so the CLI guard was the
only obstacle. Covered by `removes a preset the gateway enforces but the
registry never recorded`, `offers a gateway-only preset in the removal
picker`, and `lists a preset both sources hold only once in the removal
picker`.
- A `null` from `getGatewayPresets` means the gateway could not be
queried, not that the preset is absent, so the command falls back to the
registry and names the evidence it had instead of asserting the preset
is not applied. Covered by `names the unreachable gateway when it
refuses on local state alone`; the unchanged refusal path is covered by
`refuses a preset neither the registry nor the gateway holds`.
- The built-in arm of `applyPresetContent` in `src/lib/policy/index.ts`
warns when the sandbox has no registry entry instead of returning `true`
in silence. It keeps the successful gateway mutation, because a built-in
preset stays discoverable from the gateway and is now removable; the
custom arm still returns `false`, because a custom preset is
discoverable only through the registry. Covered by `warns but keeps the
mutation when a built-in preset cannot be recorded locally`.
- `src/lib/actions/sandbox/policy-channel-refresh.test.ts` and
`policy-channel-lock.test.ts` gain a `getGatewayPresets` stub. Without
it the new gateway read in `policy remove` would make those unit tests
spawn a real `openshell policy get`.
- `docs/network-policy/apply-policy-presets.mdx` documents removing a
preset the gateway enforces without a local record, the
unreachable-gateway behavior, and the unrecorded-sandbox case for
maintained and custom presets. `docs/reference/commands.mdx` updates the
`policy add` and `policy remove` reference sections to match.
## Type of Change
- [ ] Code change (feature, bug fix, or refactor)
- [x] 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:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] 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
<!-- 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: `docs-updated`
- Evidence: `docs/network-policy/apply-policy-presets.mdx`,
`docs/reference/commands.mdx`. The review returned request-changes. Its
blocking finding was that the persistence paragraph stated the built-in
outcome without scoping it, so a reader applying a custom preset with
`--from-file` to an unregistered sandbox would expect a gateway-active
preset when the command in fact exits non-zero and the preset appears
nowhere. It also found the quoted `policy list` row indented two spaces
where `formatPolicyListPresetRow` emits four, no command block before
the `Expected output:` lead-in, the gateway named without its OpenShell
qualifier on first use, an unreachable-gateway sentence that did not
scope its message to the named form, and both reference sections in
`docs/reference/commands.mdx` left describing the previous behavior. All
findings were verified against source and applied.
- Agent: Claude Code
<!-- docs-review-head-sha: 7ebf1a6 -->
<!-- docs-review-agents-blob-sha: b9fb6a9 -->
## 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 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 src/lib/actions/sandbox/policy-channel-policy.test.ts
src/lib/actions/sandbox/policy-channel-refresh.test.ts
src/lib/actions/sandbox/policy-channel-lock.test.ts
src/lib/actions/sandbox/policy-channel-list.test.ts` — 4 files, 81 tests
passed; `npx vitest run --project integration test/policies.test.ts` — 1
file, 76 tests passed. Reverting only the two changed source files fails
5 of the 6 new cases, so they guard the behavior rather than restate it;
the sixth is the preserved refusal path and passes either way by design.
`npm run typecheck:cli` and `npm run lint` are clean. The two `fern
check` warnings are pre-existing — rebuilding with the doc changes
stashed reports the same two.
- [ ] 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
- [x] `npm run docs` builds without warnings (doc changes only)
- [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)
---
<!-- 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: Tinson Lai <tinsonl@nvidia.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Policy removal now detects presets enforced by the gateway, including
presets missing from local state.
- Interactive removal lists combine locally recorded and
gateway-reported presets.
- Built-in presets can be applied successfully even when local registry
recording is unavailable, with a warning.
- **Bug Fixes**
- Improved handling and diagnostics when gateway state cannot be queried
or presets exist in neither source.
- **Documentation**
- Updated policy command and preset guides to explain gateway-only
presets, warnings, and offline removal behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>1 parent 9b7f391 commit 3606cb4
8 files changed
Lines changed: 152 additions & 7 deletions
File tree
- docs
- network-policy
- reference
- src/lib
- actions/sandbox
- policy
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
| 7 | + | |
| 8 | + | |
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
114 | 140 | | |
115 | 141 | | |
116 | 142 | | |
117 | 143 | | |
118 | 144 | | |
119 | 145 | | |
120 | 146 | | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
121 | 152 | | |
122 | 153 | | |
123 | 154 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2510 | 2510 | | |
2511 | 2511 | | |
2512 | 2512 | | |
| 2513 | + | |
| 2514 | + | |
2513 | 2515 | | |
2514 | 2516 | | |
2515 | 2517 | | |
| |||
2590 | 2592 | | |
2591 | 2593 | | |
2592 | 2594 | | |
2593 | | - | |
| 2595 | + | |
2594 | 2596 | | |
2595 | 2597 | | |
2596 | 2598 | | |
| |||
2604 | 2606 | | |
2605 | 2607 | | |
2606 | 2608 | | |
2607 | | - | |
| 2609 | + | |
| 2610 | + | |
| 2611 | + | |
2608 | 2612 | | |
2609 | 2613 | | |
2610 | 2614 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| 54 | + | |
54 | 55 | | |
55 | 56 | | |
56 | 57 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| 49 | + | |
49 | 50 | | |
50 | 51 | | |
51 | 52 | | |
| |||
108 | 109 | | |
109 | 110 | | |
110 | 111 | | |
| 112 | + | |
111 | 113 | | |
112 | 114 | | |
113 | 115 | | |
| |||
420 | 422 | | |
421 | 423 | | |
422 | 424 | | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
423 | 480 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
| 95 | + | |
95 | 96 | | |
96 | 97 | | |
97 | 98 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1981 | 1981 | | |
1982 | 1982 | | |
1983 | 1983 | | |
| 1984 | + | |
| 1985 | + | |
| 1986 | + | |
| 1987 | + | |
| 1988 | + | |
| 1989 | + | |
1984 | 1990 | | |
| 1991 | + | |
| 1992 | + | |
1985 | 1993 | | |
1986 | 1994 | | |
1987 | 1995 | | |
| |||
1995 | 2003 | | |
1996 | 2004 | | |
1997 | 2005 | | |
1998 | | - | |
| 2006 | + | |
1999 | 2007 | | |
| 2008 | + | |
| 2009 | + | |
| 2010 | + | |
2000 | 2011 | | |
2001 | 2012 | | |
2002 | 2013 | | |
| |||
2009 | 2020 | | |
2010 | 2021 | | |
2011 | 2022 | | |
2012 | | - | |
| 2023 | + | |
2013 | 2024 | | |
2014 | 2025 | | |
2015 | 2026 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2131 | 2131 | | |
2132 | 2132 | | |
2133 | 2133 | | |
| 2134 | + | |
| 2135 | + | |
| 2136 | + | |
| 2137 | + | |
| 2138 | + | |
| 2139 | + | |
| 2140 | + | |
| 2141 | + | |
| 2142 | + | |
| 2143 | + | |
2134 | 2144 | | |
2135 | 2145 | | |
2136 | 2146 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
737 | 737 | | |
738 | 738 | | |
739 | 739 | | |
740 | | - | |
| 740 | + | |
741 | 741 | | |
742 | 742 | | |
743 | 743 | | |
744 | 744 | | |
| 745 | + | |
745 | 746 | | |
746 | 747 | | |
747 | 748 | | |
| |||
750 | 751 | | |
751 | 752 | | |
752 | 753 | | |
| 754 | + | |
753 | 755 | | |
754 | 756 | | |
755 | 757 | | |
| |||
768 | 770 | | |
769 | 771 | | |
770 | 772 | | |
| 773 | + | |
771 | 774 | | |
772 | 775 | | |
773 | 776 | | |
| |||
776 | 779 | | |
777 | 780 | | |
778 | 781 | | |
| 782 | + | |
779 | 783 | | |
780 | 784 | | |
781 | 785 | | |
| |||
811 | 815 | | |
812 | 816 | | |
813 | 817 | | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
814 | 844 | | |
815 | 845 | | |
816 | 846 | | |
| |||
0 commit comments