Skip to content

fix(policy): disclose full messaging-preset egress before channels apply - #7187

Merged
cv merged 20 commits into
mainfrom
fix/7179-disclose-preset-egress-before-channels
Jul 19, 2026
Merged

fix(policy): disclose full messaging-preset egress before channels apply#7187
cv merged 20 commits into
mainfrom
fix/7179-disclose-preset-egress-before-channels

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Every path that newly applies a messaging-preset policy now discloses the full effective egress from the exact YAML being applied — host, port, protocol/TLS/access mode, per-rule methods and paths, and binaries — before any mutation. Before this change, channels add, channels start, channels add --dry-run, onboarding widening, and rebuild reconciliation short-circuited before the host-only preview that only policy-add printed, so the WhatsApp preset silently opened a narrowly scoped GET raw.githubusercontent.com/WhiskeySockets/Baileys/master/src/Defaults/index.ts fetch that its description did not name.

Related Issue

Fixes #7179

Changes

  • Add a structured renderPresetScope / logPresetScope in src/lib/policy/preset-scope-render.ts that walks the parsed YAML network_policies.*.endpoints and prints host, port, protocol/TLS/access mode, per-rule methods and paths, and the declared binary allowlist. Derived from the exact YAML — description prose is not treated as the disclosure.
  • Replace the host-only widening log inside applyPresetContent and applyPresets in src/lib/policy/index.ts with the structured render. This is the single choke point every apply path traverses, so channels add/start, onboarding widening via syncPresetSelection, and rebuild reconciliation now all disclose without further caller changes.
  • Emit the structured scope explicitly in channels add --dry-run and applyExternalPreset (--from-file / --from-dir) inside src/lib/actions/sandbox/policy-channel.ts, because both short-circuit before the choke point. Replace the host-only preview in policy-add with the same structured render.
  • Update the WhatsApp preset descriptions in src/lib/messaging/channels/whatsapp/policy/openclaw.yaml and hermes.yaml to name the narrowly scoped Baileys protocol-version fetch. Advisory only; the structured render remains the disclosure.
  • New tests: src/lib/policy/preset-scope-render.test.ts covers full-access tunnels, REST rules, exact paths, wildcards, binaries, empty and malformed YAML; src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts drives addSandboxChannel with the WhatsApp preset and asserts the narrow Baileys path is disclosed before the "would enable channel" line.
  • Update existing tests and fixtures for the new phrasing: test/policies.test.ts, test/policy-channel-agent-resolution.test.ts, test/package-contract/cli/policy-dispatch.test.ts, test/e2e/live/mcp-bridge-reliability.ts, test/e2e/support/mcp-bridge-reliability.test.ts, src/lib/actions/sandbox/policy-channel-policy.test.ts. Ratchet the test/policies.test.ts legacy line budget down to match its trimmed size.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: the new disclosure surfaces alongside existing policy-add output that has never been separately documented; user-facing behaviour reads the same YAML the preset already ships.
  • 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:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • 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/policy/preset-scope-render.test.ts src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts src/lib/actions/sandbox/policy-channel-policy.test.ts test/policies.test.ts test/policy-channel-agent-resolution.test.ts test/e2e/support/mcp-bridge-reliability.test.ts — 134 passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features
    • Expanded “effective egress” scope previews across onboarding, preset application, and channel workflows, including endpoint/rule/binary details and preset-state messages like “no new egress”.
    • Improved dry-run/preview output and disclosure timing for channel add/start.
  • Bug Fixes
    • Hardened scope rendering to escape terminal control characters and handle malformed/empty policy entries safely.
  • Documentation
    • Updated CLI reference to clarify when finalized policy scope/egress scope disclosures are printed during common onboarding and policy actions.
  • Tests
    • Updated and added unit/integration/e2e coverage for new disclosure text, ordering, no-op behavior, and escaping.
  • Chores
    • Updated CI test size budget.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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

This change adds structured effective-egress rendering for preset YAML, integrates disclosure into policy, channel, and onboarding flows, updates WhatsApp descriptions, and revises related tests and documentation.

Changes

Preset scope disclosure

Layer / File(s) Summary
Scope extraction and rendering
src/lib/policy/preset-scope-render.ts, src/lib/policy/preset-scope-render.test.ts
Parses policies, endpoints, rules, and binaries into sanitized effective-egress lines and tests malformed, empty, L4, REST, multi-policy, and hostile-input cases.
Policy application and state classification
src/lib/policy/index.ts, test/policy-preset-noop-disclosure.test.ts
Classifies presets as absent, drift, or match, logs state-specific scope, and skips policy mutation when the merged policy is unchanged.
Channel and onboarding disclosure
src/lib/actions/sandbox/policy-channel.ts, src/lib/onboard/*, test/channels-add-preset.test.ts, src/lib/actions/sandbox/*test.ts
Discloses channel scope before prompts or mutations and passes initial policy disclosure through sandbox plan materialization with cleanup on callback failure.
Validation and output contracts
test/*, docs/reference/commands.mdx, src/lib/messaging/channels/whatsapp/policy/*, ci/test-file-size-budget.json
Updates fixtures, documentation, descriptions, diagnostics, assertions, and file-size budgeting for effective-egress output and ordering.

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

Sequence Diagram(s)

sequenceDiagram
  participant ChannelCommand
  participant PolicyModule
  participant ScopeRenderer
  participant Console
  participant OpenShell
  ChannelCommand->>PolicyModule: load and classify preset YAML
  PolicyModule->>ScopeRenderer: render effective scope
  ScopeRenderer-->>Console: emit endpoints, rules, and binaries
  ChannelCommand->>Console: prompt or report dry-run action
  PolicyModule->>OpenShell: apply merged policy when changed
Loading

Possibly related PRs

Suggested labels: area: policy, area: messaging, bug-fix, feature, v0.0.78

Suggested reviewers: cv, ericksoa, jyaunches, apurvvkumaria

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: full egress disclosure before channel preset application.
Linked Issues check ✅ Passed The changes implement pre-mutation disclosure across channels add/start, policy-add, onboarding, and rebuild, matching #7179.
Out of Scope Changes check ✅ Passed No clear out-of-scope code changes stand out; the docs, tests, and helper updates all support the disclosure feature.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7179-disclose-preset-egress-before-channels

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

@github-code-quality

github-code-quality Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/7179-disclose-pr... branch remains at 80%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main bb5ec40 fix/7179-disclose-pr... 5b894f5 +/-
src/lib/onboard...box-prebuild.ts 88% 69% -19%
src/lib/inferen...er-lifecycle.ts 71% 65% -6%
src/lib/platform.ts 89% 84% -5%
src/lib/actions...licy-channel.ts 79% 79% 0%
src/lib/policy/index.ts 56% 58% +2%
src/lib/actions...eway-restart.ts 90% 93% +3%
src/lib/onboard...host-anchors.ts 90% 94% +4%
src/lib/domain/.../connect-env.ts 89% 97% +8%
src/lib/policy/...ateway-state.ts 56% 73% +17%
src/lib/policy/...scope-render.ts 0% 93% +93%

Updated July 19, 2026 15:36 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@laitingsheng laitingsheng added area: policy Network policy, egress rules, presets, or sandbox policy bug-fix PR fixes a bug or regression labels Jul 19, 2026
Comment thread src/lib/policy/preset-scope-render.test.ts Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/policy-channel-scope-disclosure.test.ts`:
- Line 42: Update the test cleanup around NEMOCLAW_NON_INTERACTIVE to snapshot
its prior environment value before each test and restore that exact value in
afterEach, deleting it only when it was originally unset. Remove the
unconditional deletion so environment state cannot leak between tests.

In `@src/lib/policy/index.ts`:
- Line 939: Detect whether preset application produces an effective policy delta
before calling logPresetScope: update the single-preset path at
src/lib/policy/index.ts lines 939-939 and the batch path at lines 1081-1082 to
suppress or adjust the disclosure when all incoming policies already exist,
while preserving the current disclosure for newly added policy; add regression
coverage for a fully existing preset.

In `@src/lib/policy/preset-scope-render.test.ts`:
- Line 39: Update the renderPresetScope test suite title in the describe block
to include the linked issue suffix “(`#7179`)”, while leaving the child test
titles unchanged.

In `@test/package-contract/cli/policy-dispatch.test.ts`:
- Around line 252-264: Update the loadPresetFromFile mock used by this test to
return the parsed content of a-good.yaml, preserving the existing bad-file
branch. Read the provided path with fs.readFileSync(p, "utf8") and parse it, or
return an equivalent preset object containing the fixture’s network_policies
endpoint structure so the renderer sees the declared endpoint.
🪄 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: 84b0f957-04b8-412a-8e90-56087a660cbb

📥 Commits

Reviewing files that changed from the base of the PR and between 78e0e4e and a2b6296.

📒 Files selected for processing (14)
  • ci/test-file-size-budget.json
  • src/lib/actions/sandbox/policy-channel-policy.test.ts
  • src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/messaging/channels/whatsapp/policy/hermes.yaml
  • src/lib/messaging/channels/whatsapp/policy/openclaw.yaml
  • src/lib/policy/index.ts
  • src/lib/policy/preset-scope-render.test.ts
  • src/lib/policy/preset-scope-render.ts
  • test/e2e/live/mcp-bridge-reliability.ts
  • test/e2e/support/mcp-bridge-reliability.test.ts
  • test/package-contract/cli/policy-dispatch.test.ts
  • test/policies.test.ts
  • test/policy-channel-agent-resolution.test.ts

Comment thread src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts Outdated
Comment thread src/lib/policy/index.ts Outdated
Comment thread src/lib/policy/preset-scope-render.test.ts Outdated
Comment thread test/package-contract/cli/policy-dispatch.test.ts
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

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: cloud-onboard, credential-sanitization, security-posture, channels-add-remove, channels-stop-start, inference-routing, network-policy, onboard-repair, onboard-resume

1 optional E2E recommendation
  • mcp-bridge

Workflow run details

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

@apurvvkumaria apurvvkumaria self-assigned this Jul 19, 2026
Comment thread src/lib/policy/preset-scope-render.ts Outdated
Comment thread src/lib/actions/sandbox/policy-channel.ts Outdated

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on exact head a2b62969816e5859ffa4be47bd63bf76d15ee83d.

There are four merge blockers:

  1. The new renderer prints untrusted custom-preset YAML fields without terminal-safe escaping. I reproduced loadPresetFromFile() accepting an ESC-bearing host and renderPresetScope() returning literal \u001b[2J\u001b[H bytes, so a malicious preset can erase/spoof the audit block or prompt before the warning is shown. See the inline comment.
  2. The promised path coverage/order is incomplete. Actual non-QR channels add prompts and mutates gateway/registry state before the disclosure; channels start --dry-run has no disclosure and non-dry start mutates the stored plan first. Fresh onboarding is also not covered: prepareInitialSandboxPolicy() merges selected presets into the create-time policy, and onboard.ts creates the sandbox after logging only preset names; the new renderer is never called before that mutation. Rebuild can likewise create with those presets and emit the scope only during a later re-apply. Please exercise actual add/start, fresh interactive and headless onboarding, and rebuild ordering—not only the add dry-run stub.
  3. The existing no-op finding is valid: applyPresetContent() and applyPresets() unconditionally say egress “would be opened” even when the same effective policy is already live. This is especially visible during rebuild replay. Compare effective policy state and suppress or accurately label no-op/replacement cases.
  4. Required checks are red. The package-contract test fails exactly as CodeRabbit reported (a-good's loader stub returns a flat host, so the renderer reports no endpoints). The CodeQL required check also reports one high alert in the new test assertion; although that alert appears to be a test-only false positive, it must be resolved/dismissed before merge. Protected E2E was not dispatched because prerequisite CI failed.

The environment-restoration and issue-suffix review threads are also valid cleanup items.

Independent exact-head verification:

  • Build completed during dependency installation.
  • CLI focused tests: 35/35 passed.
  • Integration policy tests: 94/94 passed.
  • E2E support tests: 5/5 passed.
  • Package-contract policy test: 13/14 passed; the same assertion failed locally and in CI.
  • WhatsApp OpenClaw and Hermes renderings accurately include all five endpoints, the exact Baileys GET path, and both Node binaries.
  • Commit is GitHub Verified and DCO is green; the PR is mechanically mergeable but review/CI blocked.

laitingsheng and others added 2 commits July 19, 2026 08:50
…fix substring-URL CodeQL alert

The --from-dir package-contract test's loadPresetFromFile stub synthesized
minimal fake content instead of reading the real on-disk fixture, so the
new egress disclosure assertion could never see the endpoint's port. The
substring host:port check in preset-scope-render.test.ts also tripped a
CodeQL incomplete-URL-substring-sanitization alert; switched to exact line
membership. Also restores NEMOCLAW_NON_INTERACTIVE via vi.stubEnv/
unstubAllEnvs instead of a raw delete, and adds the missing (#7179) suffix
to a new describe title per project convention.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

apurvvkumaria and others added 2 commits July 19, 2026 03:33
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…ation path per #7179

Strip terminal control characters from every rendered preset field so a
crafted --from-file/--from-dir preset cannot forge or erase the egress
disclosure block before a user reviews it.

Suppress the disclosure's "would be opened" wording when a preset's
declared policies already match the sandbox's live policy, via a new
presetIntroducesNewEgress delta check shared by applyPresetContent,
applyPresets, and mergePresetNamesIntoPolicy. This also gives onboarding's
create-time preset merge (interactive and headless) a full audit block
before the sandbox is created, and rebuild's preset replay already routes
through the same functions.

Add disclosure to channels start --dry-run (previously silent) and make
channels add --dry-run no-op aware via a new logSandboxPresetScopeIfNew
helper, so a dry-run preview cannot claim new egress for an already-applied
preset.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 1643: Update the effective-scope description near renderPresetScope to
explicitly include TLS and enforcement when those endpoint fields are rendered,
alongside host, port, access, and protocol settings. Verify the wording against
renderPresetScope and its source fields so the documentation accurately lists
the complete disclosed scope.

In `@src/lib/actions/sandbox/policy-channel.ts`:
- Around line 1452-1465: Extract the duplicated preset load, policy-key
validation, error/exit handling, and discloseChannelPresetScope flow into a
shared helper, then call it from both addSandboxChannelUnlocked and this
disabled-check block. Preserve the existing null/empty validation semantics and
error wording while removing the duplicate inline logic.

In `@src/lib/onboard.ts`:
- Around line 2722-2729: Move the inline discloseInitialSandboxPolicy callback
logic into an exported helper in the InitialSandboxPolicy implementation at
initial-policy.ts, accepting injected logging and file-reading dependencies.
Update onboard.ts to wire the helper directly as
initialPolicy.discloseInitialSandboxPolicy, preserving the existing empty-preset
guard and policy scope logging behavior.

In `@test/package-contract/cli/policy-dispatch.test.ts`:
- Around line 212-234: Rename the test case around the visible behavior
exercised by runPolicyAddExternal: it validates sanitized hostile custom-preset
rendering during --dry-run --yes, not disclosure ordering before an interactive
prompt. Alternatively, add a separate interactive test that explicitly verifies
the disclosure scope appears before credential collection, while keeping this
test focused on dry-run output.
🪄 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: b7f104dc-f3e2-46df-85c4-cb3d52c6640e

📥 Commits

Reviewing files that changed from the base of the PR and between a7b18a7 and 4cef61d.

📒 Files selected for processing (14)
  • docs/reference/commands.mdx
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/onboard.ts
  • src/lib/onboard/sandbox-create-intent-types.ts
  • src/lib/onboard/sandbox-create-plan-materialization.ts
  • src/lib/onboard/sandbox-create-plan.test.ts
  • src/lib/policy/index.ts
  • src/lib/policy/preset-scope-render.test.ts
  • src/lib/policy/preset-scope-render.ts
  • test/channels-add-preset.test.ts
  • test/package-contract/cli/policy-dispatch.test.ts
  • test/policy-preset-noop-disclosure.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts
  • src/lib/policy/preset-scope-render.test.ts

Comment thread docs/reference/commands.mdx Outdated
Comment thread src/lib/actions/sandbox/policy-channel.ts Outdated
Comment thread src/lib/onboard.ts Outdated
Comment thread test/package-contract/cli/policy-dispatch.test.ts Outdated
apurvvkumaria and others added 3 commits July 19, 2026 03:46
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Exact-head advisor triage for feed1fa82efdfc4af065b013ff48c86efbe62a75: both advisor model jobs ended after partial analysis, while the publish job successfully preserved the one informational warning. The warning concerns the pre-existing mutable Baileys master URL; the PR diff only changes its description and does not introduce or widen that rule. The underlying hardening opportunity is tracked separately in #7197. I am classifying the advisor red jobs as partial-analysis infrastructure failures rather than a #7187 code regression; ordinary CI is green, and I will allow the current E2E gate coordination to finish before any one-time failed-job retry.

…before-channels' into fix/7179-disclose-preset-egress-before-channels

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@apurvvkumaria
apurvvkumaria dismissed their stale review July 19, 2026 11:54

Dismissed as stale after exact-head verification at feed1fa: all requested correctness/security fixes are present, no review threads remain unresolved, all ordinary CI is green, and the one-time advisor retry completed successfully in both lanes. Protected E2E coordination remains the only active gate.

…before-channels' into fix/7179-disclose-preset-egress-before-channels

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/policy-channel-remove-flow.test.ts`:
- Around line 71-85: Update the dry-run test around startSandboxChannel to
explicitly mock runner.runCapture with the expected failure or
unavailable-result behavior, ensuring policies.getPresetContentGatewayState
follows the “live delta unavailable” branch without invoking a real subprocess.
Preserve the existing assertions for scope output, dry-run ordering, and skipped
apply/update/rebuild/exit operations.
🪄 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: fd3ffed2-2943-4034-b69c-4e216e2344bb

📥 Commits

Reviewing files that changed from the base of the PR and between feed1fa and 990d0c4.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/policy-channel-remove-flow.test.ts
  • src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/sandbox/policy-channel-scope-disclosure.test.ts

Comment thread src/lib/actions/sandbox/policy-channel-remove-flow.test.ts
Deliberately stub runner.runCapture to a clean empty policy instead of
relying on the real subprocess call failing incidentally, so the
disclosure state is a deterministic "absent" rather than an accidental
"live delta unavailable".

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

I completed an exact-head maintainer audit and found one actionable blocker: the required pre-mutation egress disclosure was printed a second time when the preset was applied.

Fixed in a835d7a31 by allowing callers that already disclosed the finalized scope to suppress only the duplicate apply-time rendering. Direct policy helpers retain their existing disclosure behavior, and policy application/registry updates still run normally.

Validation on this revision:

  • 241 focused CLI, integration, and package-contract tests passed
  • CLI type-check and build passed
  • scoped repository hooks and secret scan passed
  • signed DCO commit is GitHub Verified
  • original-author credit preserved with Co-authored-by: Tinson Lai <tinsonl@nvidia.com>

The PR remains mergeable with zero unresolved review threads. Fresh-head CI and both automated advisors are now running; protected E2E will wait for that exact-head gate.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/channels-add-preset.test.ts (1)

213-224: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Scope the getPresetContentGatewayState mock to absent-state cases.

Returning "absent" for every test can bypass matching, drift, and no-op behavior, allowing these tests to pass without exercising the relevant policy classification. Use fixture-specific state or override this mock only where absence is the intended precondition.

🤖 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/channels-add-preset.test.ts` around lines 213 - 224, Update the
getPresetContentGatewayState mock in the test setup to return fixture-specific
policy states, and only return "absent" in tests whose precondition requires
absence. Ensure matching, drift, and no-op cases exercise their intended
classifications instead of sharing the unconditional mock.

Source: Path instructions

🧹 Nitpick comments (1)
src/lib/actions/sandbox/policy-channel-refresh.test.ts (1)

124-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer asserting disclosure behavior over the internal suppression flag.

These expectations lock tests to the applyPreset call shape instead of proving the user-visible contract. Assert that finalized scope is disclosed exactly once and before mutation, while retaining only the arguments needed to verify the public flow.

  • src/lib/actions/sandbox/policy-channel-refresh.test.ts#L124-L126: replace the exact options-object assertion with an observable no-duplicate-disclosure assertion.
  • src/lib/actions/sandbox/policy-channel-refresh.test.ts#L154-L156: apply the same behavioral assertion to the failure path.
  • test/channels-add-preset.test.ts#L371-L373: verify disclosure/application ordering rather than the internal options object.
  • test/channels-add-preset.test.ts#L405-L408: retain the scope-order assertion and remove the exact suppression flag requirement.
  • test/channels-add-preset.test.ts#L423-L425: verify the failed WhatsApp path does not duplicate disclosure.
  • test/channels-add-preset.test.ts#L521-L523: verify rollback behavior and disclosure ordering.
  • test/channels-add-preset.test.ts#L673-L675: verify observable Slack application behavior.
  • test/channels-add-preset.test.ts#L685-L687: verify observable Slack application behavior.
  • test/channels-add-preset.test.ts#L697-L699: verify observable Slack failure behavior.
🤖 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-refresh.test.ts` around lines 124 -
126, Replace internal suppressDisclosure/options-object assertions with
observable disclosure and application behavior. In
src/lib/actions/sandbox/policy-channel-refresh.test.ts:124-126 and :154-156,
assert finalized scope disclosure occurs exactly once before mutation, including
the failure path. In test/channels-add-preset.test.ts:371-373, :405-408,
:423-425, :521-523, :673-675, :685-687, and :697-699, verify
disclosure/application ordering, scope order, rollback, and failed
WhatsApp/Slack paths without requiring the internal suppression flag; retain
only arguments needed to prove the public flow.

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.

Outside diff comments:
In `@test/channels-add-preset.test.ts`:
- Around line 213-224: Update the getPresetContentGatewayState mock in the test
setup to return fixture-specific policy states, and only return "absent" in
tests whose precondition requires absence. Ensure matching, drift, and no-op
cases exercise their intended classifications instead of sharing the
unconditional mock.

---

Nitpick comments:
In `@src/lib/actions/sandbox/policy-channel-refresh.test.ts`:
- Around line 124-126: Replace internal suppressDisclosure/options-object
assertions with observable disclosure and application behavior. In
src/lib/actions/sandbox/policy-channel-refresh.test.ts:124-126 and :154-156,
assert finalized scope disclosure occurs exactly once before mutation, including
the failure path. In test/channels-add-preset.test.ts:371-373, :405-408,
:423-425, :521-523, :673-675, :685-687, and :697-699, verify
disclosure/application ordering, scope order, rollback, and failed
WhatsApp/Slack paths without requiring the internal suppression flag; retain
only arguments needed to prove the public flow.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e0386a22-15bc-47ab-8168-7cd958be66a2

📥 Commits

Reviewing files that changed from the base of the PR and between ab1e3c9 and a835d7a.

📒 Files selected for processing (7)
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel-policy.test.ts
  • src/lib/actions/sandbox/policy-channel-refresh.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/policy/index.ts
  • test/channels-add-preset.test.ts
  • test/policy-preset-noop-disclosure.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • test/policy-preset-noop-disclosure.test.ts
  • src/lib/actions/sandbox/policy-channel-policy.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/policy/index.ts

@cv

cv commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Addressed the preset-disclosure security warning in signed commit 40144dd00: all YAML-derived scope fields now pass through canonical full secret and URL-credential redaction before terminal escaping. Verification: 57 focused policy/disclosure tests passed; npm run typecheck:cli passed; normal commit and pre-push hooks passed. Documentation review found no user-facing docs change needed.

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer sweep: approved after current-base CI passed all 53 checks, selected live E2E passed across OpenClaw and Hermes, the full preset scope is disclosed before mutation, live policy state is revalidated before suppressing disclosure, and all YAML-derived terminal output is credential/query redacted and escaped. Product scope is established by #7179. I did not merge.

@cv
cv merged commit 3c4ea5a into main Jul 19, 2026
54 checks passed
@cv
cv deleted the fix/7179-disclose-preset-egress-before-channels branch July 19, 2026 16:18
@ericksoa ericksoa mentioned this pull request Jul 20, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 20, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw `v0.0.89` before
the release plan captures the tag commit.
The entry summarizes the user-visible Station preparation, inference,
recovery, policy-disclosure, and CLI-containment changes merged since
`v0.0.88`.

## Changes

- Add `docs/changelog/2026-07-20.mdx` with the exact `## v0.0.89`
release heading, parser-safe SPDX comment, summary, and detailed
bullets.
- Link each shipped theme to the most specific published OpenClaw
documentation routes.
- Keep internal E2E, advisory-registry, and review-workflow refactors
out of the user-facing release summary.

Source summary:

- #7214, #7241, #7237, #7223, #7204, #7202, #7183, and #7090 ->
`docs/changelog/2026-07-20.mdx`: Summarize qualified DGX Station
identity, package-state, PackageKit, DKMS, and reboot-handoff fixes.
- #7242, #7221, #7186, #7164, and #6874 ->
`docs/changelog/2026-07-20.mdx`: Summarize inference endpoint
provenance, provider attachment, managed vLLM cleanup and progress, and
Ollama selection guidance.
- #7225, #7216, #7192, #7136, #7096, and #6910 ->
`docs/changelog/2026-07-20.mdx`: Summarize sandbox readiness, recovery
guidance, rebuilt model routing, durable Hermes state, and gateway
restart behavior.
- #7187 -> `docs/changelog/2026-07-20.mdx`: Summarize complete effective
messaging-preset egress disclosure before policy mutation.
- #7218, #7165, and #7184 -> `docs/changelog/2026-07-20.mdx`: Summarize
structured output containment, terminal-safe route display, and Hermes
forward cleanup.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR adds release-history
prose only; the focused changelog contract test validates its required
structure and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:

## Verification

- [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 check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `mise exec node@22.23.1 -- npx vitest
run test/changelog-docs.test.ts` (6 passed).
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only release
entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — `mise
exec node@22.23.1 -- npm run docs` completed with 0 errors and 2
existing site-wide warnings.
- [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)
— the native changelog entry uses the required parser-safe MDX SPDX
comment and does not use frontmatter.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Expanded DGX Station installation support for qualified GB300 and
OTA-upgraded environments.
* Preserved selected inference providers, endpoints, model pins, and
tuning settings during sandbox creation and rebuilds.
* Improved sandbox recovery by validating availability and stability
before restarting services.
* Added clearer policy output showing the complete effective messaging
egress configuration.

* **Bug Fixes**
* Hardened status and inference route displays by sanitizing terminal
control characters.
* Improved Hermes uninstall behavior by stopping detached dashboard
forwarding.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
miyoungc added a commit that referenced this pull request Jul 21, 2026
<!-- markdownlint-disable MD041 -->
## Summary

This PR documents the full YAML-derived `policy-add --dry-run`
disclosure near the preset preview workflow.
It also defines the generic Ubuntu package-state boundary for DGX
Station preparation.
This post-release documentation follow-up does not change a dated
changelog entry.

## Changes

- [#7187](#7187) ->
`docs/network-policy/integration-policy-examples.mdx`: Explain that the
preview uses the exact preset YAML and discloses endpoint, HTTP rule,
and binary scope.
- [#7241](#7241) ->
`docs/get-started/dgx-station-preparation.mdx`: Document that NemoClaw
accepts and quiesces an idle PackageKit daemon.
- [#7202](#7202) ->
`docs/get-started/dgx-station-preparation.mdx`: Document the fail-closed
package transaction, lock, package-state, and query boundary before
mutation.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR changes prose only.
Existing source tests own the documented behavior, and focused
documentation guards pass.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable.
- Station profile/scenario: Documentation only.
- Result: No host behavior changed.
- Supporting evidence: Source behavior verified against merged PRs #7241
and #7202 and their focused tests.

## Verification

- [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 check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts test/station-doc-ownership.test.ts
test/policy-roundtrip-docs.test.ts`: 3 files and 12 tests passed.
- [ ] Applicable broad gate passed — Not applicable to this doc-only
change.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — The
build passed with 0 errors. Fern reported the existing light-mode accent
contrast warning.
- [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)

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Documentation**
- Clarified Ubuntu preparation behavior when PackageKit, APT, or dpkg
activity is detected, including conditions that stop preparation before
changes are made.
- Expanded network policy preview guidance to explain that output
reflects the maintained preset configuration and includes hosts, ports,
access rules, protocols, TLS and enforcement settings, HTTP methods and
paths, and binary allowlists.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: policy Network policy, egress rules, presets, or sandbox policy bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disclose full messaging-preset egress before channels apply it

4 participants