fix(policy): re-apply edited presets in policy-add and document tls: skip passthrough

[Dongni-Yang]

Summary

NemoClaw-side fix for the two actionable parts of #7323 (Cloudflare-fronted upstreams reset connections because the sandbox egress proxy re-originates TLS even with access: full).

The connection resets themselves originate upstream: the OpenShell egress proxy terminates and re-originates TLS for allowed HTTPS endpoints, and access: full only lifts L7 method/path rules. The supported unblock — declaring the endpoint as a raw L4 CONNECT tunnel with access: full + tls: skip — existed but was undocumented (only discoverable in a comment inside the WhatsApp channel preset). The second defect from the report is NemoClaw code and is fixed here: policy-add <preset> refused to pick up edited preset files, forcing a destructive policy-remove/policy-add cycle.

Changes

• Drift-aware policy-add <preset> (src/lib/actions/sandbox/policy-channel.ts): naming an already-applied preset now compares the preset content against the live gateway policy.
  • match → successful idempotent no-op (exit 0)
  • preset no longer matches the live policy (edited file, or the live entry was overridden/removed) → re-applies through the normal preview/confirm path, using the "would replace the current preset policy" scope preview
  • preset content or live policy unreadable → conservative failure (exit 1) with cause-specific messaging
  • name owned by a custom preset → conservative failure (guard against clobbering user-customized policies with built-in content)
• Interactive picker (src/lib/policy/index.ts): already-applied rejection now includes a hint pointing at the named re-apply path.
• Docs — new "Raw TLS Passthrough with tls: skip" recipe (docs/network-policy/customize-network-policy.mdx): when to use it (CDN/Cloudflare-fronted endpoints resetting re-originated handshakes, protocols like WhatsApp's Noise that tunnel their own crypto), a worked preset example, and a warning that L7 inspection is disabled for that endpoint.
• Docs — recipe corrections in the same page: both the new recipe and the pre-existing my-mcp recipe now apply catalog presets by name — the previously documented flow (save into nemoclaw-blueprint/policies/presets/, then policy-add --from-file the same file) self-collides with the built-in name-collision guard and cannot work as written. Also documents that the filename must match preset.name (a mismatch is a silent no-op today).
• Docs — contract updates: docs/reference/commands.mdx (policy-add exit-code contract) and docs/network-policy/integration-policy-examples.mdx (removes the now-obsolete policy-remove-first workaround).
• Tests: new 10-test suite src/lib/actions/sandbox/policy-channel-add-drift.test.ts (match/drift/absent/dry-run/confirm-declined/non-interactive/custom-guard/unreadable states) plus the updated picker pin.

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: docs-updated
• Evidence: The complete exact-head diff against main was re-reviewed and is byte-identical to the previously approved policy change (SHA-256 7ab35cacc210a4320f1e5304f9d5461443168c99ff216b4622a2e183bf201e96). The three documentation pages accurately cover matching, drifted, unreadable, and custom-owned presets plus raw TLS tradeoffs. Focused policy tests passed 10/10 and 90/90; npm run check:diff and npm run docs passed.
• Agent: Codex documentation writer subagent

Behavior notes for reviewers

• The already-applied + matching case changes from exit 1 to exit 0. No in-repo consumer (src, tests, e2e, scripts) relies on the old exit code; the only pinned assertion was the picker test, updated here.
• Drift re-apply converges the live policy to the preset content (preset overrides on key collision — same semantics as applyPreset everywhere else). This means a manual live-policy edit inside a preset-owned key is reverted by a scripted policy-add --yes of that preset; the preview/confirm path and the "no longer matches the live policy" notice make this explicit. Flagging for maintainer sign-off as a deliberate semantics choice.
• The TLS re-origination defect itself (no raw CONNECT passthrough under access: full, and the re-originated handshake that Cloudflare rejects) needs an upstream OpenShell change and is out of scope here; a root-cause comment with the workaround is posted on Sandbox egress proxy re-originates TLS even with access: full, causing Cloudflare-fronted upstreams to reset the connection #7323.

Testing

• 220/220 across the 13 touched/neighboring vitest suites; tsc -p tsconfig.cli.json clean; biome clean; npm run docs:check-agent-variants passes.
• Three adversarial local review rounds (premise/ground-truth, correctness, contract/regression, docs/security dimensions) were run before submission; the final round was dry.

Closes #7323

Signed-off-by: Dongni-Yang dongniy@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Policy preset re-runs are now drift-aware: exact matches are successful no-ops; drift or missing recorded presets are previewed and re-applied after confirmation (or automatically in non-interactive mode).
  • Added/expanded custom guidance for raw TLS passthrough using tls: skip, including effective egress scope previews and warnings about disabled L7 inspection.
• Bug Fixes
  • Unknown preset names now fail clearly; operations now conservatively fail when preset/live policy content can’t be read.
  • Improved handling of preset name conflicts with custom presets and clearer re-apply instructions.
• Documentation
  • Updated command/preset behavior docs (including policy-add outlook idempotency), MCP preset application by preset name, and tls: skip limitations.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Named policy presets now compare stored content with live gateway state. Matching presets exit successfully without changes, while drifted or absent policies re-apply through the normal preview and confirmation flow. Documentation and tests cover these behaviors and raw TLS passthrough presets.

Changes

Policy preset drift handling

Layer / File(s)	Summary
Drift-aware policy-add flow src/lib/actions/sandbox/policy-channel.ts	Named preset re-addition detects custom ownership, compares preset content with live gateway state, no-ops on matches, and re-applies drifted or absent policies.
Policy flow validation and picker messaging src/lib/actions/sandbox/policy-channel-add-drift.test.ts, src/lib/policy/index.ts, test/policies.test.ts	Tests cover drift, matching, absence, dry-run, confirmation, non-interactive execution, ownership conflicts, and read failures; picker output includes a re-apply command.
Policy preset usage documentation docs/network-policy/customize-network-policy.mdx, docs/network-policy/integration-policy-examples.mdx, docs/reference/commands.mdx	Documentation describes named preset application, drift-aware behavior, comparison failures, and raw TLS passthrough configuration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant policy-add
  participant Gateway
  participant PolicyApply
  User->>policy-add: policy-add <preset>
  policy-add->>Gateway: compare preset with live policy
  Gateway-->>policy-add: match, drift, absent, or failure
  policy-add->>User: preview and confirmation when needed
  User->>policy-add: confirm re-apply
  policy-add->>PolicyApply: apply preset and refresh context

Loading

Suggested labels: area: docs, v0.0.92

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main changes: drift-aware preset reapplication in policy-add and new tls: skip passthrough documentation.

✨ 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 dongniy/7323-policy-add-drift-docs

---

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

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-7352.docs.buildwithfern.com/nemoclaw

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 8225e7d in the dongniy/7323-policy-... branch remains at 96%, unchanged from commit 3f65c23 in the main branch.

---

_{Updated July 24, 2026 03:34 UTC}

[github-actions]

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 0 blockers · 9 warnings · 1 suggestion

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: credential-sanitization, security-posture, channels-add-remove, channels-stop-start, inference-routing, network-policy, onboard-repair, onboard-resume

1 optional E2E recommendation

• gateway-drift-preflight

Blockers

PRA-1 Blocker — Preserve custom policy-key ownership before a built-in re-apply

• Location: src/lib/actions/sandbox/policy-channel.ts:230
• Category: security
• Problem: A named built-in re-apply checks only whether a custom preset has the same preset name. A custom preset with a different name can own the built-in preset's network-policy key. When the built-in content differs from live policy, this path continues to apply and replaces that custom key.
• Impact: A confirmed built-in re-apply can replace custom egress rules. This can remove a user's intended restriction or authorize the built-in endpoint scope instead.
• Fix: Before re-applying a drifted or absent built-in preset, reject the operation when a registered custom preset owns any of its network-policy keys, unless its recorded content is the expected built-in-owned state.
• Verification: Create a sandbox registry entry with custom preset name `custom-pypi` that defines the `pypi` network-policy key, then invoke `policy-add pypi --yes` and confirm it refuses before `applyPreset`.
• Test coverage: Add a policy-channel drift test where a different-name custom preset owns the built-in preset's network-policy key; assert no gateway comparison/apply mutation occurs and the command exits non-zero.
• Evidence: `customNames.includes(preset.name)` at line 178 detects only identical preset names. After `getPresetContentGatewayState` returns `drift` or `absent`, the code reaches `applyPreset(sandboxName, answer, { suppressDisclosure: true })` at line 250. `src/lib/policy/index.ts` already has ownership-aware mutation support through `expectedExistingNetworkPolicyContent` and custom policy-key ownership logic.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/policy/index.ts (1)

1544-1550: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider threading sandboxName for a concrete re-apply command instead of the <sandbox> placeholder.

selectFromList doesn't receive sandboxName, so the guidance falls back to a literal <sandbox> placeholder. The caller (policy-channel.ts) already has the real sandbox name at the call site (policies.selectFromList(allPresets, { applied })), so threading it through would let this message show a copy-pasteable command instead of a template.

🤖 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/policy/index.ts` around lines 1544 - 1550, Thread the caller’s real
sandbox name through selectFromList and its options from policy-channel.ts, then
use that value in the already-applied preset guidance instead of the literal
<sandbox> placeholder. Preserve the existing policy-add command structure and
behavior.

src/lib/actions/sandbox/policy-channel.ts (1)

168-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Correct drift-detection logic; consider extracting it to reduce function complexity.

The custom-ownership guard, content load, and match/null/drift/absent branching correctly implement the drift contract validated by classifyPresetEntries (src/lib/policy/index.ts:568-588) and by the accompanying tests. This block does, however, push addSandboxPolicyUnlocked well past its prior branching complexity (already flagged as high-complexity in the line-range details). Consider extracting lines 172-216 into a small helper (e.g. resolveNamedPresetReapply(sandboxName, preset)) that returns either an early-exit signal or the reapplyState, keeping the outer function focused on orchestration.

♻️ Illustrative extraction

+function resolveNamedPresetReapply(
+  sandboxName: string,
+  preset: { name: string },
+): { exit: true } | { exit: false; reapplyState: "drift" | "absent" | null } {
+  const customNames = registry.getCustomPolicies(sandboxName).map((entry) => entry.name);
+  if (customNames.includes(preset.name)) {
+    console.error(`  Preset '${preset.name}' was applied as a custom preset (--from-file).`);
+    console.error(
+      `  Edit and re-apply it with --from-file, or run '${CLI_NAME} ${sandboxName} policy-remove ${preset.name}' first.`,
+    );
+    process.exit(1);
+  }
+  const appliedContent = policies.loadPresetForSandbox(sandboxName, preset.name);
+  if (!appliedContent) {
+    console.error(`  Could not read the content of preset '${preset.name}'.`);
+    process.exit(1);
+  }
+  const appliedState = policies.getPresetContentGatewayState(sandboxName, appliedContent);
+  if (appliedState === "match") {
+    console.log(
+      `  Preset '${preset.name}' is already applied and matches the live policy; nothing to do.`,
+    );
+    return { exit: true };
+  }
+  if (appliedState === null) {
+    console.error(`  Preset '${preset.name}' is already applied.`);
+    console.error(
+      "  Could not read the live sandbox policy to compare (is the sandbox gateway running?).",
+    );
+    process.exit(1);
+  }
+  console.log(
+    appliedState === "drift"
+      ? `  Preset '${preset.name}' no longer matches the live policy.`
+      : `  Preset '${preset.name}' is recorded as applied but missing from the live policy.`,
+  );
+  return { exit: false, reapplyState: appliedState };
+}

🤖 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/policy-channel.ts` around lines 168 - 216, Extract
the custom-ownership guard and preset state branching from
addSandboxPolicyUnlocked into a focused helper such as
resolveNamedPresetReapply(sandboxName, preset). Preserve the existing
custom-policy errors, content loading, match early return,
unreadable-live-policy failure, and drift/absent reapplyState behavior; have the
outer function use the helper for orchestration.

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.

Inline comments:
In `@docs/network-policy/integration-policy-examples.mdx`:
- Around line 103-104: Update the policy-add reapplication description in the
integration policy examples to state that it re-applies whenever the preset
content differs from the live policy, including live-policy overrides or removed
preset entries, not only when the preset file changes. Preserve the existing
safe no-op and exit-zero behavior when the contents already match.

---

Nitpick comments:
In `@src/lib/actions/sandbox/policy-channel.ts`:
- Around line 168-216: Extract the custom-ownership guard and preset state
branching from addSandboxPolicyUnlocked into a focused helper such as
resolveNamedPresetReapply(sandboxName, preset). Preserve the existing
custom-policy errors, content loading, match early return,
unreadable-live-policy failure, and drift/absent reapplyState behavior; have the
outer function use the helper for orchestration.

In `@src/lib/policy/index.ts`:
- Around line 1544-1550: Thread the caller’s real sandbox name through
selectFromList and its options from policy-channel.ts, then use that value in
the already-applied preset guidance instead of the literal <sandbox>
placeholder. Preserve the existing policy-add command structure and behavior.

🪄 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: bab0d24c-9b30-46dc-b9a6-0d505fdcdda0

📥 Commits

Reviewing files that changed from the base of the PR and between 5087eb4 and 117c96a.

📒 Files selected for processing (7)

• docs/network-policy/customize-network-policy.mdx
• docs/network-policy/integration-policy-examples.mdx
• docs/reference/commands.mdx
• src/lib/actions/sandbox/policy-channel-add-drift.test.ts
• src/lib/actions/sandbox/policy-channel.ts
• src/lib/policy/index.ts
• test/policies.test.ts

[Dongni-Yang]

Note on the build-sandbox-images failures — pre-existing advisory breakage, not introduced by this PR. The base image's mcporter runtime audit gate fails on advisories that reached the reviewed corpus overnight (@hono/node-server GHSA-frvp-7c67-39w9, @modelcontextprotocol/sdk) against the pinned mcporter@0.7.3 lock. Any PR that invalidates that Docker layer cache re-runs the audit against today's DB and fails; cache-riding PRs pass. Full analysis: #7354 (comment) — related in-flight fixes: #7355, #7332.

[Dongni-Yang]

The E2E / PR Gate Coordination failure shares the same pre-existing root cause as the image builds: the selected e2e run's jobs (credential-sanitization, network-policy, onboard-repair) all fail while building/onboarding the sandbox image on the Dockerfile.base OpenClaw layer — the mcporter@0.7.3 advisory breakage described above. Re-dispatching e2e will not help until that remediation (#7355 / #7332) lands; the e2e failures contain no assertion against this PR's changed behavior.

[apurvvkumaria]

Reviewed at exact head 5f26a39 after the append-only refresh from current main. The named preset drift path correctly reapplies the shipped single-key presets, including the required tls: skip field edit; focused drift and policy integration suites pass 100/100, and the merge was conflict-free with a Verified DCO commit. The hypothetical multi-key removed-key ownership case requires registry history and is safe fix-forward rather than a blocker for #7323.