fix(rebuild): preserve external policy authority - #10116
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesPolicy authority lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to This change makes rebuilds preserve and revalidate external policy authority, but the current head still contains paths that can bypass those checks, accept legacy external targets, or leave a live unregistered sandbox after rollback; several affected tests also do not typecheck or load, and required validation gates are incomplete. These correctness, cleanup, and merge-readiness issues make the PR high risk and not ready to merge until fixed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant RebuildPipeline
participant PolicyAuthority
participant MCPBridge
participant Shields
RebuildPipeline->>PolicyAuthority: qualify and record authority
RebuildPipeline->>MCPBridge: prepare and restore with validator
MCPBridge->>PolicyAuthority: validate before and after mutations
RebuildPipeline->>Shields: persist or rebind recovery state
Shields->>PolicyAuthority: validate authority-bound recovery
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 450887f in the TypeScript / code-coverage/cliThe overall line coverage in commit 450887f in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
… into codex/9833-rebuild-authority Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
| revalidatePolicyRequirements?(operation: string): void; | ||
| } | ||
|
|
||
| function policyAuthorityCheckedRunner( |
<!-- markdownlint-disable MD041 --> ## Outcome Hermes GPU creation retains NemoClaw policy authority when OpenShell applies the reviewed GPU baseline enrichment or changes the policy during the same verified create transaction. Completed sandboxes, external policy mutations, and arbitrary filesystem additions remain unable to refresh a receipt. ## Reason The fallback and compatibility-only Hermes GPU jobs reached the exact sandbox, then rejected policy ownership. The compatibility route did not recognize the reviewed GPU enrichment. A later product-owned container transition could also advance the policy identity after the first receipt was recorded. The broader seven-job report contains three distinct root causes. This PR claims only the product-owned GPU receipt-transition cause. PR #10398 owns a different fallback cleanup failure. PRs #10116 and #10119 own external-policy preservation. ### Related issues Part of #9833. Relates to #10155. ## Changes - Recognize the reviewed native and compatibility GPU enrichment shapes while preserving route-specific `/proc` requirements. - Refresh a managed receipt only through the in-progress verified create transaction after exact identity and stable policy verification pass again. - Update the durable pending checkpoint with compare-and-set against the prior checkpoint before later effects continue. - Reject receipt refresh for completed sandbox mutations, routes without GPU injection, and arbitrary added filesystem paths. - Preserve the Hermes portable native-policy verifier on the renamed shared enrichment predicate. ## Verification - `npx vitest run --project cli src/lib/onboard/sandbox-create/policy-creation-receipt.test.ts` — 29 tests passed. - Focused CLI suites for receipt verification, orchestration, GPU identity gates, GPU create, policy mutation authority, and registry transitions — 9 files and 288 tests passed. - `tsc -p tsconfig.cli.json` — passed. - `tsx scripts/checks/run.mts` — repository checks passed. - File-local formatting, Oxlint, whitespace, and ignored-file hooks — passed. - `git diff --check` — passed. - The diff contains no secrets, API keys, or credentials. ## Review notes E2E root cause: `sandbox create / GPU policy receipt verification / reviewed compatibility enrichment and verified create transition rejected` Source run: https://github.com/NVIDIA/NemoClaw/actions/runs/33111985611 (run 33111985611, attempt 1) Failed jobs: - Hermes GPU fallback: https://github.com/NVIDIA/NemoClaw/actions/runs/33111985611/job/98657665722 - Hermes GPU compatibility-only: https://github.com/NVIDIA/NemoClaw/actions/runs/33111985611/job/98657665787 Signatures: - The live base policy does not match the policy supplied by the create transaction. - The creation receipt no longer matches the live sandbox policy before the runtime patch. Scope: one root cause. Security boundary: refresh requires the active verified create boundary, exact sandbox lifecycle identity, stable sandbox-scoped policy, an allowlisted GPU enrichment, and checkpoint compare-and-set. External mutations and completed-sandbox policy drift remain denied. Independent sensitive-path review remains required. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved GPU policy validation across native and compatibility routes, including supported proxy-only access. - Strengthened post-creation checks for sandbox identity, reservations, checkpoints, and policy consistency. - Prevented unexpected policy registration changes during active sandbox creation. - Ensured mismatched managed policy registrations are rejected with clearer error handling. - **Tests** - Expanded coverage for GPU enrichment, proxy-only access, and policy registration revalidation. - Added coverage confirming mismatched managed registrations are rejected rather than refreshed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
PR Review Advisor finished for commit |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Closing because the accepted product decision in #10514 supersedes the persisted policy-authority design implemented here. OpenShell is now the sole durable policy source; NemoClaw must not retain or replay a second authority record across rebuilds. #10515 implements that semantic cutover while preserving immutable sandbox identity, provider identity, credential targeting, and fail-closed destructive-operation checks. Any still-relevant tests or recovery fixes from this branch should be reassessed against #10515 rather than carrying the superseded authority model forward. Thank you for the substantial work on this path. |
Summary
Sandbox rebuild now binds one recorded OpenShell policy authority through preflight, delete, recreate, restore, and recovery. A changed or externally managed policy stops NemoClaw policy mutation and success publication.
Related Issue
Part of #9833
Changes
Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes