fix(policy): remove telegram + discord from base sandbox policy#2415
Conversation
📝 WalkthroughWalkthroughRemoved Telegram and Discord network policy entries from the OpenClaw sandbox baseline and added regression tests ensuring messaging providers (Telegram, Discord, Slack) are not present in the base network policy; baseline now documents that messaging endpoints are opt-in via presets. ChangesBaseline network policy & tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
✨ Thanks for submitting this pull request that proposes a way to fix a bug that causes the base sandbox policy to silently grant egress to Telegram and Discord. This identifies a bug and proposes a change to remove these messaging channels from the baseline policy, addressing a previously-fixed behavior that was reintroduced. Related open PRs: |
6f1fe0f to
c47fdd3
Compare
…NVIDIA#2180) After a fresh `nemoclaw onboard` with Balanced tier defaults, where the user explicitly skipped messaging channels in step [5/8] and did not tick Discord in step [8/8], `nemoclaw <name> policy-list` reports: ● discord — Discord API, gateway, and CDN access (active on gateway, missing from local state) The sandbox can reach `discord.com`, `gateway.discord.gg`, and `cdn.discordapp.com` without the user having opted in — the base sandbox policy's own header says "deny by default, allow only what's needed for core functionality," which messaging to third-party IM platforms is not. Same story for `api.telegram.org`. This is a regression, not a new bug: - NVIDIA#1705 (2026-04-09, 77051cc) removed pre-allowed `telegram` and `discord` from `nemoclaw-blueprint/policies/openclaw-sandbox.yaml` for exactly this reason. - NVIDIA#1700 (2026-04-14, 855924f), an unrelated npm_registry PR, was rebased on a branch that predated NVIDIA#1705 and re-added both entries during conflict resolution. The CodeRabbit release notes on NVIDIA#1700 even called this out as "Added network policy entries to enable controlled Telegram and Discord messaging access" — it landed along with the legitimate npm_registry tightening and has been in the baseline since. Once a key like `discord` exists in the gateway's loaded policy, `policies.getGatewayPresets()` detects the Discord preset as active (it matches on key presence), and `policy-list` then renders the misleading "active on gateway, missing from local state" line. - Re-remove `telegram` and `discord` entries from `nemoclaw-blueprint/policies/openclaw-sandbox.yaml`. Messaging endpoints are only reachable if the user selects the matching channel in step [5/8] and the corresponding preset is applied on top of the baseline in step [8/8]. Leave a comment block at the removed location referencing NVIDIA#1705, NVIDIA#2180, and the preset path so the next merge-conflict resolution does not casually re-add them. - Add three regression tests in `test/validate-blueprint.test.ts` mirroring the existing `NVIDIA#1583` GitHub pattern: - `regression NVIDIA#2180: base policy does not silently grant Telegram access` - `regression NVIDIA#2180: base policy does not silently grant Discord access` - `regression NVIDIA#2180: base policy does not silently grant Slack access` (guard against the same merge pattern re-adding Slack even though it was never in the baseline historically) Each asserts both the key absence in `network_policies` and the absence of any host-matching endpoint anywhere in the base policy, so a rename can't smuggle the grant back in. - `npx vitest run test/validate-blueprint.test.ts test/policies.test.ts` — 124 tests pass including the 3 new regressions. - `npx vitest run test/validate-configs-dangerous-hosts.test.ts test/onboard.test.ts` — 160 tests pass, no onboard flow regressions. Touches only the base policy YAML and the blueprint validator. Does not modify any preset (presets/telegram.yaml, presets/discord.yaml, presets/slack.yaml) — users who enabled messaging via onboard still get the same preset applied on top of baseline and retain the same endpoint access. The permissive variant (openclaw-sandbox-permissive.yaml), used for `--dangerously-skip-permissions`, is intentionally unchanged. Fixes NVIDIA#2180. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
c47fdd3 to
054392c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoclaw-blueprint/policies/openclaw-sandbox.yaml (1)
175-181: Run targeted network policy E2E for this change class.Given this is a baseline egress policy edit, run the
network-policy-e2ejob on the PR branch to re-validate deny-by-default, whitelist behavior, hot-reload, and SSRF protections before merge. As per coding guidelines, "E2E test recommendation: network-policy-e2e — deny-by-default, whitelist, hot-reload, SSRF".🤖 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/openclaw-sandbox.yaml` around lines 175 - 181, This PR touches the baseline egress policy (the openclaw-sandbox.yaml baseline comment block) so run the network-policy-e2e job on the PR branch to validate deny-by-default, whitelist behavior, hot-reload, and SSRF protections; trigger the network-policy-e2e pipeline (job name: network-policy-e2e) against this branch and attach the results to the PR, ensuring the e2e passes before merging to confirm the baseline egress change is safe.
🤖 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 `@nemoclaw-blueprint/policies/openclaw-sandbox.yaml`:
- Around line 175-181: This PR touches the baseline egress policy (the
openclaw-sandbox.yaml baseline comment block) so run the network-policy-e2e job
on the PR branch to validate deny-by-default, whitelist behavior, hot-reload,
and SSRF protections; trigger the network-policy-e2e pipeline (job name:
network-policy-e2e) against this branch and attach the results to the PR,
ensuring the e2e passes before merging to confirm the baseline egress change is
safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 78e3572c-5e3f-4a3a-88ba-1555155cc2cd
📒 Files selected for processing (2)
nemoclaw-blueprint/policies/openclaw-sandbox.yamltest/validate-blueprint.test.ts
## Summary - Bump the docs release metadata to `0.0.37`. - Document release-prep updates for messaging policy presets, sandbox runtime utilities, and the GPU CDI troubleshooting path. - Refresh generated `nemoclaw-user-*` skills from the updated docs. ## Source summary - #3159 -> `docs/reference/troubleshooting.md`: Documents the GPU CDI preflight warning and remediation for `nvidia.com/gpu=all` gateway start failures. - #2415 -> `docs/reference/network-policies.md`, `docs/manage-sandboxes/messaging-channels.md`, `docs/network-policy/customize-network-policy.md`: Clarifies that Telegram, Discord, and Slack egress comes from opt-in messaging presets, not the baseline policy. - #3091 -> `docs/deployment/sandbox-hardening.md`, `docs/network-policy/customize-network-policy.md`: Documents the retained sandbox utilities `vi`, `jq`, and `dos2unix` while keeping host-side policy files as the durable source of truth. ## Test plan - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user` - `make docs` - `npm run build:cli` - `npm run typecheck:cli` - Commit and pre-push hooks: markdownlint, docs-to-skills verification, gitleaks, commitlint, CLI typecheck ## Skipped - #3193 and #3191 matched `docs/.docs-skip` entries for experimental shields/config paths. - #3200 and #3183 were test-only fixes. - #3189 and #3163 were internal documentation/refactor changes with no public docs impact. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified which utilities remain in the sandbox runtime for lightweight inspection and cleanup * Noted that messaging endpoints (Discord, Slack, Telegram) are not in the baseline policy and that channel presets are applied during onboarding * Added GPU passthrough troubleshooting for gateway startup * Updated release/version bump and release-prep workflow guidance, including Discord preset description updates <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
The base sandbox policy (
nemoclaw-blueprint/policies/openclaw-sandbox.yaml) was silently granting every sandbox egress toapi.telegram.org,discord.com,gateway.discord.gg, andcdn.discordapp.com— regardless of whether the user enabled the telegram/discord messaging channel in step [5/8] or ticked the preset in step [8/8] of onboard. This is a regression of a previously-fixed behavior.After a fresh onboard that explicitly skipped messaging channels and did not tick Discord in the Balanced-tier preset picker,
nemoclaw <name> policy-listreports:Problem
Regression history:
77051cc8) — removedtelegramanddiscordfrom baseline "for exactly this reason." Explicitly called it a data exfiltration vector.855924fd) — an unrelated npm_registry PR that was rebased on a branch predating fix(security): remove pre-allowed messaging from base sandbox policy #1705. The CodeRabbit release notes on fix(policy): restrict baseline npm_registry to openclaw binary only #1700 even flagged it: "Added network policy entries to enable controlled Telegram and Discord messaging access." The messaging entries came back as part of that PR's conflict-resolution and have been in the baseline since.Once
discordexists as a key in the gateway's loaded policy,policies.getGatewayPresets()detects the Discord preset as active (it matches on key presence).nemoclaw policy-listthen renders the misleading "active on gateway, missing from local state" line for a preset the user never selected — and the egress is actually enforced, so it's a real capability grant, not just a display bug.Test plan
npx vitest run test/validate-blueprint.test.ts test/policies.test.ts— 124 tests pass, including the three new regression tests that guard against this specific re-add pattern for telegram, discord, and slack.npx vitest run test/validate-configs-dangerous-hosts.test.ts test/onboard.test.ts— 160 tests pass. No onboard-flow regressions.presets/{telegram,discord}.yaml(no preset YAML changes in this PR).openclaw-sandbox-permissive.yaml(used for--dangerously-skip-permissions) unchanged — permissive users still get pre-allowed messaging as before.Fixes #2180.
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit
Bug Fixes
Tests