feat(policy): support persistent baseline network policy exclusions#7194
Merged
Conversation
Add a distinct baseline-exclusion operation so an operator can remove an exact agent baseline network policy entry and keep it removed across rebuild, accepting reduced and unsupported functionality in exchange for tighter egress. The exclusion previews every endpoint, method/path rule, and binary it removes, requires explicit acknowledgement even non-interactively, binds the approval to a digest of the reviewed entry, and fails closed on create and rebuild when a release changes or removes that entry. The selection is recorded in the sandbox registry and replayed through the create-intent policy generation, so live policy, registry intent, and rebuild output agree. Exclusions surface in policy list and sandbox status; managed_inference stays protected pending product direction. Also canonicalise the policy command group to the two-token grammar (policy add, policy remove, ...), keeping the hyphenated policy-add forms as hidden legacy aliases so existing scripts keep working while help, completion, display, and advisory hints advertise the canonical form. Fixes #7178 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR adds digest-bound baseline network-policy exclusions with ChangesBaseline exclusion lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7194.docs.buildwithfern.com/nemoclaw |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit f490578 in the TypeScript / code-coverage/cliThe overall coverage in commit f490578 in the Show a code coverage summary of the most impacted files.
Updated |
laitingsheng
added
area: policy
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts (1)
126-146: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winForward exclusions through this resolver fixture.
The mock discards
input.baselineExclusionsand always returns[], so handler tests cannot detect a dropped replay request. Model this field from the input and add a non-empty propagation case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts` around lines 126 - 146, The resolveCreateIntent fixture must preserve baselineExclusions from its input instead of always returning an empty array. Update the input type and returned policy in resolveCreateIntent to forward input.baselineExclusions, then add a propagation case using non-empty exclusions to verify replay requests are not dropped.
🧹 Nitpick comments (1)
test/package-contract/cli/public-argv-translation.test.ts (1)
97-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest every alias through argv translation.
The
Object.keys(SANDBOX_LEGACY_ROUTE_ALIASES)assertion locks the test to private metadata, while onlypolicy-addproves legacy behavior. Table-drive canonical and legacy argv cases for every supported alias instead.As per path instructions, prefer observable public-boundary outcomes over private-shape assertions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/package-contract/cli/public-argv-translation.test.ts` around lines 97 - 111, Update the test around sandboxRouteTokens to remove the private Object.keys(SANDBOX_LEGACY_ROUTE_ALIASES) assertion and table-drive argv translation cases for every supported canonical and legacy alias. Assert each case through the public argv translation boundary, covering add, explain, get, list, and remove, including their legacy hyphenated forms and expected two-token routes.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard.ts`:
- Around line 2298-2300: Update the onboarding rebuild flow around
getBaselineExclusions to retain complete exclusion records, including
acknowledgement metadata, rather than mapping them to only key and digest. Carry
these full entries into the later registration call after the old registry row
is removed, so each rebuild replays the approvals. Add a regression test
covering two consecutive destructive rebuilds.
In `@src/lib/policy/index.ts`:
- Around line 857-873: Update excludeBaselineEntry to check the boolean result
from registry.addBaselineExclusion after applying the policy; return false when
the registry call fails, and return true only after the exclusion is
successfully recorded.
In `@src/lib/state/registry-normalization.ts`:
- Around line 23-25: Update the normalization logic around appliedAgentVersion
in src/lib/state/registry-normalization.ts:23-25 to retain both string values
and valid null values, including during registry load and disk serialization. In
src/lib/state/registry-normalization.test.ts:122-160, add coverage confirming an
entry with appliedAgentVersion: null round-trips unchanged.
---
Outside diff comments:
In `@src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts`:
- Around line 126-146: The resolveCreateIntent fixture must preserve
baselineExclusions from its input instead of always returning an empty array.
Update the input type and returned policy in resolveCreateIntent to forward
input.baselineExclusions, then add a propagation case using non-empty exclusions
to verify replay requests are not dropped.
---
Nitpick comments:
In `@test/package-contract/cli/public-argv-translation.test.ts`:
- Around line 97-111: Update the test around sandboxRouteTokens to remove the
private Object.keys(SANDBOX_LEGACY_ROUTE_ALIASES) assertion and table-drive argv
translation cases for every supported canonical and legacy alias. Assert each
case through the public argv translation boundary, covering add, explain, get,
list, and remove, including their legacy hyphenated forms and expected two-token
routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5fcab3f2-bb95-420c-be3f-6205ea08247e
📒 Files selected for processing (53)
docs/reference/network-policies.mdxscripts/checks/openshell-policy-mutation-read.mtssrc/commands/sandbox/policy/exclude.tssrc/commands/sandbox/policy/mutate.test.tssrc/commands/sandbox/policy/restore.tssrc/lib/actions/root-help.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/actions/sandbox/exec-policy-hint-rendering.test.tssrc/lib/actions/sandbox/exec-policy-hint-rendering.tssrc/lib/actions/sandbox/policy-channel-baseline.test.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/actions/sandbox/rebuild-finalization.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/actions/sandbox/rebuild-restore-phase.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/cli/command-registry.tssrc/lib/cli/public-argv-translation.tssrc/lib/cli/public-dispatch.tssrc/lib/cli/public-display-defaults.tssrc/lib/cli/public-route-metadata.tssrc/lib/domain/policy-channel.tssrc/lib/messaging/channels/telegram/hooks/status-health-eval.tssrc/lib/onboard.tssrc/lib/onboard/dashboard.tssrc/lib/onboard/initial-policy-baseline-exclusion.test.tssrc/lib/onboard/initial-policy.tssrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.tssrc/lib/onboard/machine/handlers/sandbox-test-fixtures.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/policy-tier-suppression.tssrc/lib/onboard/sandbox-create-intent-resolution.tssrc/lib/onboard/sandbox-create-intent-types.tssrc/lib/onboard/sandbox-create-intent.tssrc/lib/onboard/sandbox-create-plan-materialization.tssrc/lib/onboard/sandbox-create-plan.test.tssrc/lib/policy/baseline-exclusion.test.tssrc/lib/policy/baseline-exclusion.tssrc/lib/policy/context.test.tssrc/lib/policy/context.tssrc/lib/policy/failure-classifier.test.tssrc/lib/policy/index.tssrc/lib/sandbox/policy-command-support.tssrc/lib/sandbox/whatsapp-diagnostics.tssrc/lib/state/registry-normalization.test.tssrc/lib/state/registry-normalization.tssrc/lib/state/registry.tstest/cli/dispatch-basics.test.tstest/onboard-pre-destructive-intent.test.tstest/package-contract/cli/command-registry.test.tstest/package-contract/cli/public-argv-translation.test.tstest/policies.test.tstest/policy-explain-cli.test.ts
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
previously requested changes
Jul 19, 2026
apurvvkumaria
left a comment
apurvvkumaria
left a comment
Collaborator
There was a problem hiding this comment.
I pushed 3bc5c32 to address the mechanical/data-integrity findings (durable full exclusion records, checked persistence, drift recheck, null round-trip, status JSON coverage, canonical CLI docs, and regressions). Two correctness gaps still block approval:
-
Destructive resume is not transaction-safe. applySandboxResumeDecision can remove the durable registry row before replacement sandbox creation/registration succeeds. If creation then fails, the only persisted baselineExclusions record is gone, so a later retry can silently recreate the removed baseline route. Preserve the exclusion receipt durably until replacement registration commits (or restore the old row on failure), and add a failed-create/resume regression.
-
The PR says it fixes #7178, whose acceptance criteria require exclusions and their impact in policy explain, snapshot/rebuild summaries, and diagnostics. The current implementation exposes them in policy list and status --json, but those other required surfaces still do not consume baselineExclusions. Complete those surfaces with tests, or narrow the linked issue/scope so the PR does not close unmet acceptance criteria.
Please keep the protected managed_inference behavior and the exact live-entry digest recheck from the pushed commit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.mdx`:
- Line 1657: Split the affected Markdown/MDX prose so each sentence occupies its
own source line: in docs/reference/commands.mdx at lines 1657, 1709, and 3592;
docs/reference/enterprise-readiness.mdx at lines 68-69, placing each table-cell
sentence on its own line; docs/reference/platform-support.mdx at line 135; and
docs/reference/troubleshooting.mdx at line 1348. Preserve the existing wording
and formatting while applying the one-sentence-per-line convention.
In `@test/package-contract/cli/public-argv-translation.test.ts`:
- Around line 96-125: Update both test titles in the policy routing tests— the
parameterized test covering canonical and legacy spellings and the test covering
canonical two-token subcommands—to append the applicable issue suffix “(`#7178`)”.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e6e3608f-e4a9-459f-9415-df318dac9408
📒 Files selected for processing (37)
ci/platform-matrix.jsondocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/get-started/quickstart.mdxdocs/manage-sandboxes/backup-restore.mdxdocs/manage-sandboxes/runtime-controls.mdxdocs/manage-sandboxes/set-up-slack.mdxdocs/network-policy/customize-network-policy.mdxdocs/network-policy/integration-policy-examples.mdxdocs/reference/cli-selection-guide.mdxdocs/reference/commands.mdxdocs/reference/enterprise-readiness.mdxdocs/reference/network-policies.mdxdocs/reference/platform-support.mdxdocs/reference/troubleshooting.mdxsrc/commands/sandbox/policy/restore.tssrc/lib/actions/sandbox/policy-channel-baseline.test.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/onboard.tssrc/lib/onboard/initial-policy-baseline-exclusion.test.tssrc/lib/onboard/machine/handlers/sandbox-test-fixtures.tssrc/lib/onboard/machine/handlers/sandbox.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-create-intent-resolution.tssrc/lib/onboard/sandbox-create-intent-types.tssrc/lib/onboard/sandbox-create-plan.test.tssrc/lib/onboard/sandbox-registration.test.tssrc/lib/onboard/sandbox-registration.tssrc/lib/policy/baseline-exclusion-persistence.test.tssrc/lib/policy/baseline-exclusion.test.tssrc/lib/policy/baseline-exclusion.tssrc/lib/policy/index.tssrc/lib/sandbox/policy-command-support.tssrc/lib/state/registry-normalization.test.tssrc/lib/state/registry-normalization.tssrc/lib/state/registry.tstest/package-contract/cli/public-argv-translation.test.tstest/sandbox-status-json-stdout.test.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- src/lib/onboard/sandbox-create-intent-resolution.ts
- src/commands/sandbox/policy/restore.ts
- src/lib/onboard/initial-policy-baseline-exclusion.test.ts
- src/lib/state/registry-normalization.ts
- src/lib/state/registry-normalization.test.ts
- src/lib/onboard/machine/handlers/sandbox.ts
- src/lib/onboard/sandbox-create-plan.test.ts
- docs/reference/network-policies.mdx
- src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts
- src/lib/state/registry.ts
- src/lib/actions/sandbox/policy-channel-baseline.test.ts
- src/lib/policy/index.ts
- src/lib/policy/baseline-exclusion.test.ts
- src/lib/policy/baseline-exclusion.ts
- src/lib/sandbox/policy-command-support.ts
…eline exclusions Restore the removed registry row when replacement sandbox creation fails during a destructive resume, so an approved baseline exclusion is not silently dropped on a later retry. Also disclose exclusions and their support impact in `policy explain`, rebuild summaries, and `doctor` diagnostics, and cover the untested resolveCreateIntent fixture gap. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/doctor.ts`:
- Around line 357-378: Update baselineExclusionDoctorChecks to distinguish all
three baseline states: matching digests should remain informational as actively
excluded, differing non-null digests should remain warnings requiring
re-approval, and a null currentDigest should report the exclusion as no longer
in the baseline and inert. Ensure the null state does not claim dependent
features remain unsupported or include the re-approval warning intended for
changed entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c56464ee-361e-4266-90f9-b3c9708277a6
📒 Files selected for processing (16)
docs/manage-sandboxes/backup-restore.mdxsrc/lib/actions/sandbox/doctor-flow.test.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/policy-explain.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/onboard.tssrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/handlers/sandbox-destructive-resume-rollback.test.tssrc/lib/onboard/machine/handlers/sandbox-resume.test.tssrc/lib/onboard/machine/handlers/sandbox-resume.tssrc/lib/onboard/machine/handlers/sandbox-test-fixtures.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/policy/context.test.tssrc/lib/policy/context.tssrc/lib/policy/failure-classifier.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/manage-sandboxes/backup-restore.mdx
- src/lib/onboard/machine/core-flow-phases.test.ts
- src/lib/actions/sandbox/rebuild-post-restore-phase.ts
- src/lib/onboard.ts
Fix the doc round-trip test still asserting the legacy hyphenated `policy-get` heading now that the canonical CLI grammar is two-token, and replace branching mock implementations with lookup tables to keep new test files free of added conditionals. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/actions/sandbox/snapshot.test.ts (1)
220-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new snapshot output path with a behavior test.
This mock keeps existing tests passing, but no test verifies that a successful snapshot prints the exclusion key and support-impact summary. Add one with
getBaselineExclusionsreturning an entry and assert the CLI output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/snapshot.test.ts` at line 220, Add a behavior test in the snapshot test suite for a successful snapshot using the mocked getBaselineExclusions function. Return a non-empty exclusion entry from the mock, execute the snapshot path, and assert that CLI output includes both the exclusion key and the support-impact summary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/doctor.ts`:
- Around line 373-376: Update the hint logic in restoreBaselineEntry handling in
src/lib/actions/sandbox/doctor.ts#L373-L376 to branch on currentDigest === null:
provide a restore-only hint for removed baseline entries, while preserving the
restore-and-re-review instructions for changed entries. Update
docs/reference/network-policies.mdx#L309-L317 to separately document
changed-entry re-approval and removed-entry stale-record cleanup.
---
Nitpick comments:
In `@src/lib/actions/sandbox/snapshot.test.ts`:
- Line 220: Add a behavior test in the snapshot test suite for a successful
snapshot using the mocked getBaselineExclusions function. Return a non-empty
exclusion entry from the mock, execute the snapshot path, and assert that CLI
output includes both the exclusion key and the support-impact summary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e4930bca-915e-4a8a-8dd7-1b0043c75e3c
📒 Files selected for processing (26)
docs/reference/commands.mdxdocs/reference/network-policies.mdxdocs/reference/troubleshooting.mdxsrc/lib/actions/sandbox/doctor-flow.test.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/actions/sandbox/policy-explain.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/actions/sandbox/snapshot-baseline-exclusion-summary.test.tssrc/lib/actions/sandbox/snapshot-baseline-exclusion-summary.tssrc/lib/actions/sandbox/snapshot.test.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-text.tssrc/lib/onboard/machine/handlers/sandbox-destructive-resume-rollback.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/policy/baseline-exclusion-persistence.test.tssrc/lib/policy/baseline-exclusion.tssrc/lib/policy/context.test.tssrc/lib/policy/context.tssrc/lib/policy/index.tssrc/lib/state/registry-normalization.test.tssrc/lib/state/registry-normalization.tstest/package-contract/cli/public-argv-translation.test.tstest/policy-explain-cli.test.tstest/support/status-flow-test-harness.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/lib/actions/sandbox/policy-explain.test.ts
- src/lib/actions/sandbox/rebuild-post-restore-phase.ts
- src/lib/actions/sandbox/doctor-flow.test.ts
- src/lib/policy/context.test.ts
- src/lib/onboard/machine/handlers/sandbox.ts
- src/lib/policy/baseline-exclusion.ts
- src/lib/policy/index.ts
- docs/reference/troubleshooting.mdx
- src/lib/policy/context.ts
- src/lib/actions/sandbox/policy-channel.ts
- docs/reference/commands.mdx
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
doctor's drift hint told operators to review and re-approve a baseline key with `policy exclude --dry-run` even when the release removed the key entirely, where that command fails with "Unknown baseline entry". Branch the hint on whether the key still exists so a removed entry only gets the restore-and-clear instruction, and split the matching docs section into changed-entry vs removed-entry recovery paths. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
laitingsheng
added
v0.0.87
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
dismissed
their stale review
July 19, 2026 12:34
Addressed on exact head 882b9c9: destructive-resume exclusion recovery is transaction-safe, all required policy/rebuild/diagnostic surfaces are covered, and removed baseline keys now use restore-only guidance across doctor, runtime errors, and docs.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
|
v0.0.89 disposition: deferred at the product-scope gate. The branch is refreshed, the mechanical conflict preserves both provider selection and baseline-exclusion replay, focused tests/typecheck/hooks pass, and the missing successful-snapshot disclosure test plus command-reference sentence are now added. I am not approving while #7178 remains open with |
Co-authored-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Collaborator
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/security/openclaw-2026.6.10-dependency-review.md (1)
83-89: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep each sentence on one source line.
Lines 83-89 split sentences across multiple Markdown lines.
Suggested edit
-This section is a point-in-time record of the remediation shipped for the -2026.6.10 runtime. The current 2026.7.1 path installs the reviewed core archive -directly because its core graph already contains the fixed versions, and keeps -only the version-scoped Slack and Microsoft Teams Axios remediation plus the -diagnostics Jaeger remediation added for the 2026.7.1 archive. See -[`openclaw-2026.7.1-dependency-review.md`](./openclaw-2026.7.1-dependency-review.md) -for the active source and validation boundary. +This section is a point-in-time record of the remediation shipped for the 2026.6.10 runtime. +The current 2026.7.1 path installs the reviewed core archive directly because its core graph already contains the fixed versions, and keeps only the version-scoped Slack and Microsoft Teams Axios remediation plus the diagnostics Jaeger remediation added for the 2026.7.1 archive. +See [`openclaw-2026.7.1-dependency-review.md`](./openclaw-2026.7.1-dependency-review.md) for the active source and validation boundary.As per coding guidelines, “Keep one sentence per line in Markdown and MDX source files.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/security/openclaw-2026.6.10-dependency-review.md` around lines 83 - 89, Reformat the introductory paragraph in the 2026.6.10 dependency review so each sentence remains on a single Markdown source line. Preserve the wording, paragraph structure, and link references while joining the currently wrapped sentence fragments.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/security/openclaw-2026.6.10-dependency-review.md`:
- Around line 83-89: Reformat the introductory paragraph in the 2026.6.10
dependency review so each sentence remains on a single Markdown source line.
Preserve the wording, paragraph structure, and link references while joining the
currently wrapped sentence fragments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d0c1b9b5-d3c2-4ad6-b6e3-67f990d73757
📒 Files selected for processing (49)
ci/platform-matrix.jsondocs/deployment/deploy-to-headless-server.mdxdocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/get-started/quickstart.mdxdocs/manage-sandboxes/backup-restore.mdxdocs/monitoring/manage-deepagents-trace-export.mdxdocs/monitoring/set-up-deepagents-trace-export.mdxdocs/monitoring/verify-deepagents-trace-export.mdxdocs/network-policy/apply-policy-presets.mdxdocs/network-policy/change-baseline-network-policy.mdxdocs/network-policy/configure-raw-tls-passthrough.mdxdocs/network-policy/create-custom-policy-presets.mdxdocs/network-policy/customize-network-policy.mdxdocs/network-policy/explain-network-policy-to-agents.mdxdocs/network-policy/integration-policy-examples.mdxdocs/network-policy/replace-live-network-policy.mdxdocs/network-policy/set-up-gmail-with-an-app-password.mdxdocs/reference/commands.mdxdocs/reference/enterprise-readiness.mdxdocs/reference/network-policies.mdxdocs/reference/platform-support.mdxdocs/reference/troubleshooting.mdxdocs/security/openclaw-2026.6.10-dependency-review.mdpackage.jsonsrc/lib/actions/sandbox/dcode-probe-test-fixture.tssrc/lib/actions/sandbox/policy-channel-add-drift.test.tssrc/lib/actions/sandbox/policy-channel-baseline.test.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-local-provider-recreate.test.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/actions/sandbox/rebuild-preflight-guards.tssrc/lib/actions/sandbox/rebuild-preflight-phase.tssrc/lib/actions/sandbox/restore-gateway-pairing.test.tssrc/lib/actions/sandbox/restore-gateway-pairing.tssrc/lib/actions/sandbox/snapshot-auto-create-failure.test.tssrc/lib/actions/sandbox/snapshot-restore-lifecycle.test.tssrc/lib/actions/sandbox/snapshot-restore-test-fixture.tssrc/lib/actions/sandbox/snapshot.test.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/actions/sandbox/status-flow.test.tssrc/lib/agent/onboard.tssrc/lib/onboard.tssrc/lib/onboard/authoritative-rebuild-target.test.tssrc/lib/onboard/authoritative-rebuild-target.tssrc/lib/onboard/dashboard.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- ci/platform-matrix.json
- src/lib/onboard/dashboard.ts
- src/lib/onboard/authoritative-rebuild-target.ts
- docs/get-started/quickstart.mdx
- src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts
- docs/reference/platform-support.mdx
- src/lib/agent/onboard.ts
- docs/reference/enterprise-readiness.mdx
- src/lib/actions/sandbox/snapshot-restore-lifecycle.test.ts
- src/lib/onboard/authoritative-rebuild-target.test.ts
- src/lib/actions/sandbox/rebuild-local-provider-recreate.test.ts
- src/lib/actions/sandbox/rebuild-post-restore-phase.ts
- src/lib/actions/sandbox/status-flow.test.ts
- package.json
- docs/manage-sandboxes/backup-restore.mdx
- src/lib/actions/sandbox/policy-channel-baseline.test.ts
- docs/reference/network-policies.mdx
- src/lib/actions/sandbox/snapshot.test.ts
- src/lib/onboard.ts
- docs/reference/troubleshooting.mdx
- src/lib/actions/sandbox/snapshot-restore-test-fixture.ts
- src/lib/actions/sandbox/snapshot.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.ts
- src/lib/actions/sandbox/policy-channel.ts
- docs/reference/commands.mdx
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/policy-preset-picker.test.ts (1)
14-19: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse an ESM import for the source module in this root-level test.
test/policy-preset-picker.test.tsstill loadssrc/lib/policy/index.tsthroughcreateRequireat lines 14-19; switch this to a direct ESM source import or an ESM-compatible test seam.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/policy-preset-picker.test.ts` around lines 14 - 19, Replace the createRequire-based loading of the policies module in policy-preset-picker.test.ts with a direct ESM import of src/lib/policy/index.ts, or use an existing ESM-compatible test seam; keep the imported policies API unchanged for the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/policy-preset-picker.test.ts`:
- Around line 14-19: Replace the createRequire-based loading of the policies
module in policy-preset-picker.test.ts with a direct ESM import of
src/lib/policy/index.ts, or use an existing ESM-compatible test seam; keep the
imported policies API unchanged for the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f9b009fc-1a89-4385-8131-4949b011b566
📒 Files selected for processing (11)
docs/get-started/quickstart.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/doctor-flow.test.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/policy/index.tstest/openclaw-dependency-review.test.tstest/package-contract/cli/policy-prompt-eof.test.tstest/policies.test.tstest/policy-preset-picker.test.tstest/pr-workflow-contract.test.ts
💤 Files with no reviewable changes (1)
- test/policies.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/get-started/quickstart.mdx
- src/lib/actions/sandbox/doctor.ts
- src/lib/actions/sandbox/doctor-flow.test.ts
- src/lib/policy/index.ts
- src/lib/actions/sandbox/policy-channel.ts
- docs/reference/commands.mdx
Collaborator
|
Exact-head automated-review disposition for
No actionable automated-review finding remains. Required CI and exact-pair E2E gates are tracked separately and must still pass before merge. |
Collaborator
|
Exact-pair E2E blocker for head
This is not a branch conflict or a product-test failure. Repository policy makes the exact-pair gate terminal on this head, so it cannot be rerun or waived for merge. An upstream gate/environment fix must land first; then this PR needs a fresh exact head and the full exact-head evidence/gate cycle. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Operators could not express a persistent, explicit exclusion from an agent's built-in baseline network policy:
policy removeonly touches presets, and a rawopenshell policy setnarrowing is not rebuild-stable. This adds a distinct baseline-exclusion operation so an operator can remove an exact baseline entry (for example Hermesnous_research) and keep it removed across rebuild, accepting reduced and unsupported functionality in exchange for tighter egress.Related Issue
Fixes #7178
Changes
policy excludeandpolicy restore: preview the exact removed scope, require explicit acknowledgement for mutation, apply the change to the live policy, and persist or clear the reviewed intent.ajv8.18.0 from development-only to runtime dependencies so installed packages enforce the same boundary.pypicollision.managed_inferenceprotected at both command and durable-replay boundaries because it carries the required managed inference route.policy add,policy remove, and so on), while keeping hyphenated forms as hidden compatibility aliases.Maintainer decision
policy exclude <key> --dry-runpreviews exact scope; mutation requires explicit acknowledgement;policy restore <key>restores the current baseline entry.managed_inference.baselineExclusionsloads unchanged. Because this feature has not shipped, an exclusion record without the released schema version and agent owner is rejected rather than guessed or migrated.Product-scope evidence is issue #7178 and the recorded maintainer decision above. This product decision is separate from GitHub
mergeStateStatus, CI, and exact-head review state.Type of Change
Quality Gates
1f6ddfc8dagainst basef0f23ade5; all nine categories passed with no findings. This is agent evidence only; no human exact-head or sensitive-path review is claimed. Historical human review covered an earlier head.Documentation Writer Review
no-docs-neededf490578c93844bc9ef86036a34e73b4391e46d31. The head commit is empty and only retriggers CI. The substantive pairing retry change remains accurately covered bydocs/manage-sandboxes/backup-restore.mdx, which already states that cross-sandbox restore establishes and authenticates OpenClaw gateway pairing and documents failure recovery. Focused suites passed 32/32 and 8/8; CLI typecheck and standard CI passed. No docs build is required because no documentation changed.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable1f6ddfc8d: conflict-adjacent CLI/integration suites 161/161; compiled package contracts 318/318 in a stable single-worker lane; documentation contracts 12/12; command-registry, EOF, and picker contracts 47/47; E2E semantic phase-plan validation passed.npm run docsbuilds without warnings (doc changes only) — not a documentation-only PR; exact-head build passed with 0 errors and 2 pre-existing warnings.Additional exact-head evidence:
1f6ddfc8dd636cf26703b49c483ebcdba34f69c5, basef0f23ade57d7d4fb0bc8644f88b6d9525901d7c0.708dc6253792eb3d38fd82cf258ef610deaa0914and the 147-file changed-file list are identical to the preceding reviewed pair. The latest upstream commit had zero file overlap.prekpre-commit,commitlint, andprekpre-push passed againstupstream/main..HEADunder Node 22.19.0.Verified.Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
policy excludeandpolicy restorefor individual baseline network-policy entries, with--dry-runpreviews, acknowledgement safeguards, and durable replay across rebuilds and snapshot restores.policy list,policy explain, sandboxstatus,doctor, and rebuild/snapshot flows, including repair-required guidance and fail-closed drift handling.policy <verb>syntax (add/list/remove/explain/exclude/restore); legacy hyphenated spellings remain supported.