feat(policy): add opt-in allow-all network posture#5092
Open
larroy wants to merge 1 commit into
Open
Conversation
Add an opt-in "allow-all" posture that lets the sandbox reach any public host, alongside a scoped relaxation of the catch-all host guardrail. - New openclaw-sandbox-allow-all.yaml: a single catch-all host: "*" base policy (ports 80/443) with prominent danger warnings. SSRF and private-network blocking still apply. - New "allow-all" tier (tiers.yaml): swaps the base policy instead of layering presets. Selectable via NEMOCLAW_POLICY_TIER=allow-all at create time or interactively post-create. - shields down --policy allow-all applies the catch-all to a running sandbox, reusing the permissive runtime FS-union path. - Guardrail relaxed, not removed: validate-configs permits bare wildcard hosts only in *-allow-all.yaml files; "*" stays rejected in all normal presets/base policies and IP catch-alls (0.0.0.0/0, ::/0) stay rejected everywhere (preserves NVIDIA#1445 intent). - Deny-by-default remains the shipped default; allow-all is opt-in only. - Docs: tier reference + security best-practices warnings. Note: bare host: "*" support in the OpenShell L7 proxy must be verified on a live sandbox; the policy file documents the enforcement: audit fallback if the proxy rejects "*". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds a new "Allow All" network policy tier to NemoClaw that disables egress filtering via a catch-all wildcard host endpoint. The tier is configured as an opt-in, development-only alternative to the default deny-by-default baseline, with comprehensive validation exemptions, runtime application support, onboarding integration, and test coverage. ChangesAllow-All Network Policy Tier
Sequence DiagramsequenceDiagram
participant CLI
participant Onboarding
participant PolicySelection
participant PolicyLib
participant OpenshellAPI
participant Sandbox
CLI->>Onboarding: create sandbox with NEMOCLAW_POLICY_TIER=allow-all
Onboarding->>Onboarding: resolve tier → allow-all
Onboarding->>Onboarding: switch basePolicyPath to ALLOW_ALL_POLICY_PATH
Onboarding->>PolicySelection: setupPoliciesWithSelection with applyAllowAllPolicy
PolicySelection->>PolicySelection: tierName === "allow-all" detected
PolicySelection->>PolicySelection: onSelection([]) - skip presets
PolicySelection->>Sandbox: wait for sandbox ready
Sandbox-->>PolicySelection: ready
PolicySelection->>PolicyLib: applyAllowAllPolicy(sandboxName)
PolicyLib->>OpenshellAPI: resolve openshell binary
PolicyLib->>Sandbox: fetch live policy YAML
Sandbox-->>PolicyLib: current policy state
PolicyLib->>PolicyLib: buildRuntimePermissivePolicy (merge live + allow-all)
PolicyLib->>OpenshellAPI: openshell policy set --wait (apply runtime policy)
OpenshellAPI->>Sandbox: apply policy (host: "*")
PolicyLib->>PolicyLib: cleanupTempDir
PolicyLib-->>PolicySelection: allow-all applied
PolicySelection-->>Onboarding: complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![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: 5
🧹 Nitpick comments (2)
docs/reference/network-policies.mdx (1)
72-76: ⚡ Quick winSplit this warning into one sentence per line and remove the extra em dash.
LLM pattern detected. This paragraph packs multiple sentences onto single source lines, and it uses more than one em dash in the same paragraph.
As per coding guidelines, "One sentence per line in source" and "Excessive em dashes. One per paragraph is fine; multiple per paragraph ... should be flagged."
🤖 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/reference/network-policies.mdx` around lines 72 - 76, The Warning block and the following paragraph pack multiple sentences on single source lines and use multiple em dashes; edit the Warning paragraph and the line starting "Allow All has no presets..." so each sentence is on its own source line, remove the extra em dash so only one em dash appears in the Warning paragraph, and preserve the existing content/meaning (references to `nemoclaw-blueprint/policies/openclaw-sandbox-allow-all.yaml`, `NEMOCLAW_POLICY_TIER=allow-all`, and the CLI `nemoclaw sandbox:shields:down <name> --policy allow-all` should remain unchanged).Source: Coding guidelines
docs/security/best-practices.mdx (1)
107-109: ⚡ Quick winReflow this warning to one sentence per line and replace the clause colon.
This block keeps several sentences on one source line, and the colon after "selected sandbox" is general punctuation rather than a list introducer.
As per coding guidelines, "One sentence per line in source" and "Colons should only introduce a list."
🤖 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/best-practices.mdx` around lines 107 - 109, Reflow the Warning block so each sentence is on its own source line and remove the colon after "selected sandbox" by replacing it with a comma (or reword to avoid a colon); specifically edit the Warning block text inside the docs/security/best-practices.mdx Warning section so that sentences like the Allow All policy description, the SSRF/private-network note, and the final advisory are each on separate lines and the phrase "for the selected sandbox:" becomes "for the selected sandbox," (or an equivalent clause without a colon).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/reference/network-policies.mdx`:
- Line 70: The doc currently contradicts itself: the new table row "Allow All" /
`allow-all` says it swaps the base policy but the paragraph above still claims
the baseline policy is always applied; update the surrounding tier description
and any nearby paragraphs (the text around the tiers table and the
baseline-policy description) to reflect that selecting the `allow-all` tier
replaces/swaps the base policy rather than applying it in addition, so the page
consistently documents `allow-all` as the only documented behavior that disables
the baseline filtering.
In `@nemoclaw-blueprint/policies/tiers.yaml`:
- Around line 53-56: The tier "allow-all" description promises unrestricted
outbound access but the actual policy in openclaw-sandbox-allow-all.yaml only
permits host: "*" on ports 80 and 443; update the "allow-all" tier copy to
explicitly call out the 80/443 restriction (e.g., append that outbound is
limited to ports 80 and 443) or alternatively remove/adjust the port restriction
in openclaw-sandbox-allow-all.yaml so the description matches the policy; be
sure to edit the "name: allow-all" tier description string to reflect the chosen
fix.
In `@scripts/validate-configs.ts`:
- Around line 220-223: The helper isAllowAllPolicyFile currently keys the
wildcard exemption on the basename only, letting files like
presets/slack-allow-all.yaml bypass checks; update it to scope the exemption by
path/schema kind instead of basename by verifying the file's full relative path
or directory (e.g., ensure the file resides in the dedicated base policy
directory or matches the exact allowed base policy path used by discoverTargets)
before returning true; adjust the same logic used at the other occurrence (lines
mentioned around 363-367) so only the dedicated allow-all base policy can use a
bare "*" and presets or other policy locations are not exempted.
In `@src/lib/onboard.ts`:
- Around line 3374-3379: The code reads recordedTier via
registry.getSandbox(sandboxName)?.policyTier after the recreate/prune path has
already removed the sandbox, so recreated sandboxes lose their previous
"allow-all" tier; change the flow to capture the sandbox's prior policyTier
before the prune/delete runs (e.g. fetch and store the value earlier where the
sandbox still exists), then use that stored value instead of calling
registry.getSandbox(...) when computing recordedTier and allowAllAtCreate (which
influences basePolicyPath and policies.ALLOW_ALL_POLICY_PATH); ensure the stored
priorTier is threaded into the logic that sets allowAllAtCreate/basePolicyPath
so recreated sandboxes retain the recorded tier defaults.
- Around line 3367-3383: Extract the allow-all base policy resolution logic from
onboard.ts into a helper in the existing policy helper/module (e.g., add a
function like resolveBasePolicyPath(agent, sandboxName) in the policies helper)
so onboard.ts no longer contains the inline env/recordedTier/allowAllAtCreate
logic; the new helper should: compute basePolicyPath = (agent &&
agentOnboard.getAgentPolicyPath(agent)) || defaultPolicyPath, read recordedTier
via registry.getSandbox(sandboxName)?.policyTier, read envTier from
process.env.NEMOCLAW_POLICY_TIER, determine allowAllAtCreate and, if true,
return policies.ALLOW_ALL_POLICY_PATH and the decision message (or a boolean)
otherwise return the normal basePolicyPath; then update onboard.ts to call this
new helper (replacing basePolicyPath, recordedTier, envTier, and
allowAllAtCreate usage) and remove the inline block so behavior (including the
console message about allow-all) remains identical while reducing onboard.ts
size.
---
Nitpick comments:
In `@docs/reference/network-policies.mdx`:
- Around line 72-76: The Warning block and the following paragraph pack multiple
sentences on single source lines and use multiple em dashes; edit the Warning
paragraph and the line starting "Allow All has no presets..." so each sentence
is on its own source line, remove the extra em dash so only one em dash appears
in the Warning paragraph, and preserve the existing content/meaning (references
to `nemoclaw-blueprint/policies/openclaw-sandbox-allow-all.yaml`,
`NEMOCLAW_POLICY_TIER=allow-all`, and the CLI `nemoclaw sandbox:shields:down
<name> --policy allow-all` should remain unchanged).
In `@docs/security/best-practices.mdx`:
- Around line 107-109: Reflow the Warning block so each sentence is on its own
source line and remove the colon after "selected sandbox" by replacing it with a
comma (or reword to avoid a colon); specifically edit the Warning block text
inside the docs/security/best-practices.mdx Warning section so that sentences
like the Allow All policy description, the SSRF/private-network note, and the
final advisory are each on separate lines and the phrase "for the selected
sandbox:" becomes "for the selected sandbox," (or an equivalent clause without a
colon).
🪄 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: 582ee336-36a4-4ced-9916-e3974f8b93c5
📒 Files selected for processing (13)
docs/reference/network-policies.mdxdocs/security/best-practices.mdxnemoclaw-blueprint/policies/openclaw-sandbox-allow-all.yamlnemoclaw-blueprint/policies/tiers.yamlscripts/validate-configs.tssrc/commands/sandbox/shields/down.tssrc/lib/onboard.tssrc/lib/onboard/policy-selection.tssrc/lib/policy/index.tssrc/lib/shields/index.tstest/policy-selection-allow-all.test.tstest/policy-tiers.test.tstest/validate-configs-dangerous-hosts.test.ts
| | Restricted | None | Base sandbox only. No third-party network access beyond inference and core agent tooling. | | ||
| | Balanced (default) | `npm`, `pypi`, `huggingface`, `brew`, `brave when supported` | Full dev tooling and web search for agents that support web search. No messaging platform access. | | ||
| | Open | `npm`, `pypi`, `huggingface`, `brew`, `brave when supported`, `slack`, `discord`, `telegram`, `wechat` (experimental), `whatsapp` (experimental), `jira`, `outlook` | Broad access across third-party services including messaging and productivity. | | ||
| | Allow All | None — swaps the base policy | Catch-all egress to any public host (no network filtering). Opt-in for trusted dev/testing only. | |
Contributor
There was a problem hiding this comment.
Update the surrounding tier description so allow-all is the only documented behavior.
This new table row and warning say allow-all swaps the base policy, but the paragraph above still says the baseline policy is always applied regardless of tier. Please align those statements so the page does not describe two different runtime behaviors.
Also applies to: 72-76
🤖 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/reference/network-policies.mdx` at line 70, The doc currently
contradicts itself: the new table row "Allow All" / `allow-all` says it swaps
the base policy but the paragraph above still claims the baseline policy is
always applied; update the surrounding tier description and any nearby
paragraphs (the text around the tiers table and the baseline-policy description)
to reflect that selecting the `allow-all` tier replaces/swaps the base policy
rather than applying it in addition, so the page consistently documents
`allow-all` as the only documented behavior that disables the baseline
filtering.
Comment on lines
+53
to
+56
| - name: allow-all | ||
| label: Allow All (no egress filtering — dev/testing only) | ||
| description: Catch-all egress to any public host. Swaps the base policy for the allow-all catch-all; presets do not apply. Maximum scope — you accept full responsibility. | ||
| presets: [] |
Contributor
There was a problem hiding this comment.
Call out the 80/443 limit in the tier copy.
The selector text reads like unrestricted outbound access, but openclaw-sandbox-allow-all.yaml still only permits host: "*" on ports 80 and 443. As written, the UI copy promises broader reach than the policy actually grants.
🤖 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 `@nemoclaw-blueprint/policies/tiers.yaml` around lines 53 - 56, The tier
"allow-all" description promises unrestricted outbound access but the actual
policy in openclaw-sandbox-allow-all.yaml only permits host: "*" on ports 80 and
443; update the "allow-all" tier copy to explicitly call out the 80/443
restriction (e.g., append that outbound is limited to ports 80 and 443) or
alternatively remove/adjust the port restriction in
openclaw-sandbox-allow-all.yaml so the description matches the policy; be sure
to edit the "name: allow-all" tier description string to reflect the chosen fix.
Comment on lines
+220
to
+223
| function isAllowAllPolicyFile(file: string): boolean { | ||
| const base = file.replaceAll("\\", "/").split("/").pop() ?? ""; | ||
| return /-allow-all\.ya?ml$/.test(base); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't key the wildcard exemption off basename alone.
discoverTargets() also validates every preset under nemoclaw-blueprint/policies/presets/. With the current helper, a preset like slack-allow-all.yaml would bypass the dangerous-host guard purely because of its filename, even though this PR's contract is that only the dedicated allow-all base policy may use bare *.
Suggested direction
+const ALLOW_ALL_POLICY_FILES = new Set([
+ "nemoclaw-blueprint/policies/openclaw-sandbox-allow-all.yaml",
+]);
+
function isAllowAllPolicyFile(file: string): boolean {
- const base = file.replaceAll("\\", "/").split("/").pop() ?? "";
- return /-allow-all\.ya?ml$/.test(base);
+ return ALLOW_ALL_POLICY_FILES.has(file.replaceAll("\\", "/"));
}If you want this to stay extensible, the important part is scoping the exemption by path/schema kind, not by basename.
Also applies to: 363-367
🤖 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 `@scripts/validate-configs.ts` around lines 220 - 223, The helper
isAllowAllPolicyFile currently keys the wildcard exemption on the basename only,
letting files like presets/slack-allow-all.yaml bypass checks; update it to
scope the exemption by path/schema kind instead of basename by verifying the
file's full relative path or directory (e.g., ensure the file resides in the
dedicated base policy directory or matches the exact allowed base policy path
used by discoverTargets) before returning true; adjust the same logic used at
the other occurrence (lines mentioned around 363-367) so only the dedicated
allow-all base policy can use a bare "*" and presets or other policy locations
are not exempted.
Comment on lines
+3367
to
+3383
| let basePolicyPath = (agent && agentOnboard.getAgentPolicyPath(agent)) || defaultPolicyPath; | ||
| // Allow-all is not a preset bundle — it swaps the base policy for the | ||
| // catch-all (host: "*"). At create time the tier is known only when it comes | ||
| // from the environment (non-interactive) or a recorded recreate; interactive | ||
| // fresh onboards choose the tier post-create and apply it via | ||
| // setupPoliciesWithSelection. Swapping the base here means an env/recreate | ||
| // allow-all sandbox boots permissive from the very first request. | ||
| const recordedTier = registry.getSandbox(sandboxName)?.policyTier ?? null; | ||
| const envTier = (process.env.NEMOCLAW_POLICY_TIER || "").trim().toLowerCase(); | ||
| const allowAllAtCreate = | ||
| envTier === "allow-all" || (recordedTier === "allow-all" && !envTier); | ||
| if (allowAllAtCreate) { | ||
| basePolicyPath = policies.ALLOW_ALL_POLICY_PATH; | ||
| console.log( | ||
| " Policy tier 'allow-all': booting with catch-all egress (no network filtering).", | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move the new allow-all onboarding glue behind a helper to get the guardrail green.
CI is already failing because src/lib/onboard.ts exceeded its growth budget, and these inline allow-all additions are the new surface. Hoisting the base-policy resolution and extra dependency wiring into an existing policy helper/module should preserve behavior while bringing this file back under the guardrail.
As per pipeline failures, src/lib/onboard.ts is currently blocked by the codebase growth guardrail.
Also applies to: 6342-6358
🤖 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.ts` around lines 3367 - 3383, Extract the allow-all base
policy resolution logic from onboard.ts into a helper in the existing policy
helper/module (e.g., add a function like resolveBasePolicyPath(agent,
sandboxName) in the policies helper) so onboard.ts no longer contains the inline
env/recordedTier/allowAllAtCreate logic; the new helper should: compute
basePolicyPath = (agent && agentOnboard.getAgentPolicyPath(agent)) ||
defaultPolicyPath, read recordedTier via
registry.getSandbox(sandboxName)?.policyTier, read envTier from
process.env.NEMOCLAW_POLICY_TIER, determine allowAllAtCreate and, if true,
return policies.ALLOW_ALL_POLICY_PATH and the decision message (or a boolean)
otherwise return the normal basePolicyPath; then update onboard.ts to call this
new helper (replacing basePolicyPath, recordedTier, envTier, and
allowAllAtCreate usage) and remove the inline block so behavior (including the
console message about allow-all) remains identical while reducing onboard.ts
size.
Source: Pipeline failures
Comment on lines
+3374
to
+3379
| const recordedTier = registry.getSandbox(sandboxName)?.policyTier ?? null; | ||
| const envTier = (process.env.NEMOCLAW_POLICY_TIER || "").trim().toLowerCase(); | ||
| const allowAllAtCreate = | ||
| envTier === "allow-all" || (recordedTier === "allow-all" && !envTier); | ||
| if (allowAllAtCreate) { | ||
| basePolicyPath = policies.ALLOW_ALL_POLICY_PATH; |
Contributor
There was a problem hiding this comment.
Read the previous policy tier before the recreate flow clears the registry entry.
On the recreate path, Line 3264 already removes the sandbox from the registry before this lookup runs, so registry.getSandbox(sandboxName)?.policyTier is null here. A sandbox that previously used allow-all therefore reboots with the default base policy unless NEMOCLAW_POLICY_TIER is set again, which breaks the intended first-boot behavior for recreated allow-all sandboxes. Capture the prior tier before prune/delete and thread that value into this decision instead.
Based on PR objectives, recreated allow-all sandboxes are supposed to boot with the recorded tier’s base policy.
🤖 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.ts` around lines 3374 - 3379, The code reads recordedTier via
registry.getSandbox(sandboxName)?.policyTier after the recreate/prune path has
already removed the sandbox, so recreated sandboxes lose their previous
"allow-all" tier; change the flow to capture the sandbox's prior policyTier
before the prune/delete runs (e.g. fetch and store the value earlier where the
sandbox still exists), then use that stored value instead of calling
registry.getSandbox(...) when computing recordedTier and allowAllAtCreate (which
influences basePolicyPath and policies.ALLOW_ALL_POLICY_PATH); ensure the stored
priorTier is threaded into the logic that sets allowAllAtCreate/basePolicyPath
so recreated sandboxes retain the recorded tier defaults.
wscurran
added
area: policy
Contributor
|
✨ |
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
2 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
Adds an opt-in "allow-all" network posture that lets a sandbox reach any public host, alongside a scoped relaxation of the catch-all host guardrail. Deny-by-default remains the shipped default; allow-all activates only when explicitly selected.
Related Issue
Changes
nemoclaw-blueprint/policies/openclaw-sandbox-allow-all.yaml: a single catch-allhost: "*"base policy (ports 80/443) with prominent danger warnings. SSRF/private-network blocking still applies.allow-alltier intiers.yamlthat swaps the base policy instead of layering presets. Selectable viaNEMOCLAW_POLICY_TIER=allow-allat create time or interactively post-create.sandbox:shields:down --policy allow-allapplies the catch-all to a running sandbox, reusing the permissive runtime FS-union path (resolveAllowAllPolicyPath,applyAllowAllPolicy).scripts/validate-configs.ts): bare wildcard hosts (*/*:port) are permitted only in*-allow-all.yamlfiles;*stays rejected in all normal presets/base policies, and IP catch-alls (0.0.0.0/0,::/0) stay rejected everywhere (preserves No Pre-Commit Validation Hook for Network Policy YAML Changes - IssueFinder - SN 21 #1445 intent).Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Pedro Larroy pedro.larroy.lists@gmail.com
Summary by CodeRabbit
New Features
NEMOCLAW_POLICY_TIER=allow-allenvironment variable ornemoclaw sandbox:shields:down <name> --policy allow-allcommand.Documentation