feat(policy): add safe common egress defaults#4768
Conversation
PR Review AdvisorFindings: 2 needs attention, 5 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
|
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:
📝 WalkthroughWalkthroughAdds read-only ChangesPolicy Presets and Tier Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: Dispatch required scenario E2E:
Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/policies.test.ts (1)
1589-1603: ⚡ Quick winAdd explicit guards against
access: fulland wildcard binaries in this new safety test.This test validates methods, but it won’t catch a future widening to
access: fullor binary/**. Add direct assertions so the regression fails fast.Proposed test hardening
it("weather and public-reference presets stay read-only and narrowly client-scoped", () => { for (const preset of ["weather", "public-reference"]) { const content = requirePresetContent(policies.loadPreset(preset)); + const parsed = YAML.parse(content) as { + network_policies?: Record<string, { binaries?: Array<{ path?: string }> }>; + }; + const binaryPaths = Object.values(parsed.network_policies ?? {}) + .flatMap((policy) => policy.binaries ?? []) + .map((entry) => entry.path ?? ""); + expect(content).toContain("protocol: rest"); expect(content).toContain("method: GET"); expect(content).toContain("method: HEAD"); + expect(content).not.toContain("access: full"); expect(content).not.toContain("method: POST"); expect(content).not.toContain("method: PUT"); expect(content).not.toContain("method: PATCH"); expect(content).not.toContain("method: DELETE"); + expect(binaryPaths).not.toContain("/**"); expect(content).toContain("/usr/local/bin/node"); expect(content).toContain("/opt/hermes/.venv/bin/python"); expect(content).toContain("/usr/bin/curl"); } });🤖 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/policies.test.ts` around lines 1589 - 1603, The test "weather and public-reference presets stay read-only and narrowly client-scoped" currently checks allowed HTTP methods and specific binaries but doesn't assert against expanding privileges; update the test that iterates presets from policies.loadPreset(preset) and content = requirePresetContent(...) to also assert that the preset content does NOT contain "access: full" and does NOT contain wildcard/broad binary paths such as "/**" or other wildcard patterns (e.g. "/*") so any regression to full access or wildcard binaries fails immediately.
🤖 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.
Nitpick comments:
In `@test/policies.test.ts`:
- Around line 1589-1603: The test "weather and public-reference presets stay
read-only and narrowly client-scoped" currently checks allowed HTTP methods and
specific binaries but doesn't assert against expanding privileges; update the
test that iterates presets from policies.loadPreset(preset) and content =
requirePresetContent(...) to also assert that the preset content does NOT
contain "access: full" and does NOT contain wildcard/broad binary paths such as
"/**" or other wildcard patterns (e.g. "/*") so any regression to full access or
wildcard binaries fails immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 36b8b1df-8c49-4119-a075-645b54cbe1a2
📒 Files selected for processing (10)
nemoclaw-blueprint/policies/presets/public-reference.yamlnemoclaw-blueprint/policies/presets/weather.yamlnemoclaw-blueprint/policies/tiers.yamlsrc/lib/onboard/hermes-managed-tools.tssrc/lib/onboard/policy-selection.tstest/e2e/test-common-egress-agent-e2e.shtest/onboard-policy-suggestions.test.tstest/policies.test.tstest/policy-tiers-onboard.test.tstest/policy-tiers.test.ts
Selective E2E Results — ✅ All requested jobs passedRun: 26955344390
|
Selective E2E Results — ✅ All requested jobs passedRun: 26955772183
|
Selective E2E Results — ✅ All requested jobs passedRun: 26955916534
|
Selective E2E Results — ❌ Some jobs failedRun: 26956028685
|
Selective E2E Results — ❌ Some jobs failedRun: 26957675088
|
Selective E2E Results — ✅ All requested jobs passedRun: 26957826438
|
Selective E2E Results — ❌ Some jobs failedRun: 26957800405
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 27006005488
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 27008152348
|
Selective E2E Results — ✅ All requested jobs passedRun: 27008581953
|
## Summary - Adds the `v0.0.60` section to `docs/about/release-notes.mdx` using the dev announcement from discussion #4877. - Fills the source-doc gaps found during release-prep review across inference, policy tiers, command behavior, security boundaries, Hermes dashboard/tooling, runtime context, and troubleshooting. - Refreshes generated agent skills under `.agents/skills/` from the current Fern docs output and upgrades Fern from `5.44.3` to `5.45.0`. ## Source summary - #4037 -> `docs/reference/architecture.mdx`, `docs/about/how-it-works.mdx`, `docs/about/release-notes.mdx`: Documents system-only runtime context that stays out of visible chat. - #4875 -> `docs/reference/architecture.mdx`, `docs/about/how-it-works.mdx`, `docs/about/release-notes.mdx`: Documents try-first sandbox network/filesystem guidance and clearer failure classification. - #4788 -> `docs/security/best-practices.mdx`, `docs/about/release-notes.mdx`: Documents shared OpenClaw device-approval policy for startup and connect. - #4768 -> `docs/reference/network-policies.mdx`, `docs/network-policy/integration-policy-examples.mdx`, `docs/get-started/quickstart.mdx`, `docs/get-started/quickstart-hermes.mdx`, `docs/reference/commands.mdx`: Documents `weather`, `public-reference`, and Hermes managed-tool gateway preset behavior. - #3788 and #4864 -> `docs/reference/network-policies.mdx`, `docs/reference/commands.mdx`: Documents non-interactive policy-tier fail-fast behavior and interactive prompt fallback. - #4756 and #4866 -> `docs/reference/commands.mdx`: Documents env-aware default sandbox resolution for `list`, `status`, and `tunnel` commands. - #4320 -> `docs/reference/commands.mdx`: Documents `$$nemoclaw tunnel status` behavior. - #4328 -> `docs/reference/commands.mdx`: Documents line-scoped policy preset descriptions in `policy-list`. - #4580 and #4748 -> `docs/reference/architecture.mdx`: Documents package-managed OpenShell gateway service and Docker-driver gateway-marker behavior. - #4598 -> `docs/manage-sandboxes/lifecycle.mdx`: Documents concurrent gateway/dashboard cleanup isolation by sandbox name and port. - #4777 -> `docs/reference/troubleshooting.mdx`: Documents Docker GPU patch rollback behavior. - #4610 -> `docs/reference/troubleshooting.mdx`, `docs/reference/commands.mdx`: Keeps mutable OpenClaw config permission guidance aligned and removes skipped experimental wording. - #4868 -> `docs/reference/commands.mdx`: Keeps `.dockerignore` handling for custom `onboard --from <Dockerfile>` contexts in generated skills. - #4870 -> `docs/reference/commands.mdx`, `docs/manage-sandboxes/runtime-controls.mdx`: Documents `NEMOCLAW_MINIMAL_BOOTSTRAP` and generated skill coverage. - #4641 -> `docs/inference/inference-options.mdx`, `docs/reference/troubleshooting.mdx`: Documents local NVIDIA NIM platform-digest pulls and served-model id adoption. - #4810 and #4867 -> `docs/inference/inference-options.mdx`: Documents stable NGC managed-vLLM image lineage and DGX Station DeepSeek V4 Flash coverage. - #4852 -> `docs/inference/use-local-inference.mdx`, `docs/reference/troubleshooting.mdx`: Documents Ollama model fit filtering, 16K context floor, cold-load retry, and failed-model exclusion. - #4847 -> `docs/inference/switch-inference-providers.mdx`: Documents API-family sync, Hermes `api_mode`, and Bedrock Runtime exception. - #4800 -> `docs/inference/tool-calling-reliability.mdx`: Documents Nemotron managed-inference native tool-search fallback. - #4333 -> `docs/inference/switch-inference-providers.mdx`: Documents interactive multimodal input prompting. - #4086 -> `docs/reference/troubleshooting.mdx`: Keeps proxy bypass normalization in generated troubleshooting coverage. - #4811 and #4855 -> `docs/get-started/quickstart-hermes.mdx`: Documents prebuilt Hermes dashboard assets and TUI recovery without runtime rebuilds. - #4854 -> `docs/inference/switch-inference-providers.mdx`, `docs/reference/commands.mdx`: Documents Hermes proxy API-key placeholder preservation during inference switches. - #4248 -> `docs/manage-sandboxes/messaging-channels.mdx`, `.agents/skills/`: Keeps messaging enrollment behavior aligned with manifest-hook implementation. - #4771 -> `docs/security/best-practices.mdx`, `docs/security/credential-storage.mdx`: Documents Hermes placeholder-only secret boundary for sandbox-visible runtime files. - #4787 -> `docs/security/best-practices.mdx`, `docs/about/release-notes.mdx`: Documents expanded memory scanner examples for OpenAI project keys and Slack app-level tokens. - #4848 -> `docs/reference/commands.mdx`: Documents OpenClaw skill install mirroring into the agent home directory. - #4790 -> `docs/about/release-notes.mdx`: Uses the prior release-prep structure and generated `.agents/skills/` refresh as the template for this release. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ skills/ --prefix nemoclaw-user --doc-platform fern-mdx --dry-run` - `npm run docs` - `git diff --check` - skip-term scan across `docs/`, `.agents/skills/`, and `skills/` - `npm run build:cli` - `npm run typecheck:cli` - Commit and pre-push hook suites, including markdownlint, gitleaks, env-var docs gate, docs-to-skills verification, and skills YAML tests <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * DeepSeek-V4-Flash now available as default inference model for DGX Station. * Hermes dashboard improved with dedicated port and OAuth-authenticated tool gateway selection. * Added weather and public-reference policy presets for expanded agent capabilities. * Enhanced Ollama model selection with GPU memory filtering and automatic retry for timeouts. * **Bug Fixes** * Improved policy tier validation to prevent invalid configurations. * Better sandbox cleanup scoping by port to prevent conflicts across deployments. * Added GPU patch failure recovery with automatic rollback. * **Documentation** * Expanded troubleshooting guides for inference, security, and sandbox lifecycle. * Added .dockerignore best practices for custom deployments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
weatherandpublic-referencepresets with host/path/method-scoped public API egressweatherin balanced/open defaults andpublic-referencein open defaultsCloses #4767
Fixes #4814
Related
Tests
npm run build:clinpx vitest run test/policy-tiers.test.ts test/policy-tiers-onboard.test.ts test/onboard-policy-suggestions.test.ts test/policies.test.ts test/validate-config-schemas.test.ts(302 passed)npx vitest run test/policies.test.ts test/validate-config-schemas.test.ts(229 passed after preset binary-scope update)npm run validate:configsbash -n test/e2e/test-common-egress-agent-e2e.sh && shellcheck test/e2e/test-common-egress-agent-e2e.shE2E status
test/e2e/test-common-egress-agent-e2e.shexercises the allowed common-egress paths through agent tool use for OpenClaw balanced/open and Hermes open.Summary by CodeRabbit
New Features
Behavior
Tests
Chores