Conversation
…iltration sentry.io is a multi-tenant SaaS — any client with a project ID can POST to any Sentry project, not just NemoClaw's. The baseline sandbox policy allowed POST to sentry.io with path '/**', which turned the host into a generic exfiltration channel: a compromised agent inside the sandbox could ship stack traces, env vars, file contents, etc. to a Sentry project controlled by an attacker via the public envelope endpoint (https://sentry.io/api/<any-project-id>/envelope/). Path-pattern restrictions cannot fix this because the project ID is part of the URL and there is no server-side allowlist of legitimate projects. This is a follow-up to #1214 (which added 'protocol: rest' for sentry.io) — that PR closed the wire-protocol gap, this PR closes the remaining HTTP-method-level gap. Changes: - nemoclaw-blueprint/policies/openclaw-sandbox.yaml: drop the 'method: POST, path: /**' allow rule for sentry.io. GET stays allowed because GET has no request body and is harmless for exfil. Side effect: Claude Code's crash telemetry to Sentry is silently dropped. That is the correct tradeoff for a sandbox whose stated goal is preventing data egress, and the sandbox already blocks many similar telemetry channels by default. - test/validate-blueprint.test.ts: walk every endpoint in network_policies, find sentry.io, and assert (a) at least one sentry.io entry exists, (b) no sentry.io entry has a POST allow rule, (c) the GET allow rule is preserved. Verified by stashing the policy fix and re-running: the test correctly fails on main with the unfixed policy, and passes with the fix in place. Closes #1437
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The path-scoped POST rules (/api/*/envelope/** and /api/*/store/**) do not prevent multi-tenant exfiltration because the Sentry project ID is part of the URL and attacker-controlled. Replace with GET-only: GET has no request body and is bounded by URL length limits, keeping read-only Sentry SDK paths (e.g. DSN config) functional. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR updates the OpenClaw sandbox network policy to change the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoclaw-blueprint/policies/openclaw-sandbox.yaml (1)
99-99: Optionally narrow the remaining GET allow.
GET "/**"still leaves the whole host reachable. If Claude Code only needs a small set of Sentry bootstrap/config routes, an explicit GET allowlist would better match the file’s deny-by-default posture and further reduce the residual egress surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nemoclaw-blueprint/policies/openclaw-sandbox.yaml` at line 99, The current rule allow: { method: GET, path: "/**" } is too broad; replace that wildcard GET allow with an explicit allowlist of only the Sentry/bootstrap/config endpoints Claude Code needs (e.g., the specific paths used for Sentry init/config), so instead of GET "/**" add individual GET rules for each required route and remove the catch-all; update the policy block containing allow: { method: GET, path: "/**" } to enumerate the precise paths to reduce egress surface and preserve the deny-by-default posture.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nemoclaw-blueprint/policies/openclaw-sandbox.yaml`:
- Line 99: The current rule allow: { method: GET, path: "/**" } is too broad;
replace that wildcard GET allow with an explicit allowlist of only the
Sentry/bootstrap/config endpoints Claude Code needs (e.g., the specific paths
used for Sentry init/config), so instead of GET "/**" add individual GET rules
for each required route and remove the catch-all; update the policy block
containing allow: { method: GET, path: "/**" } to enumerate the precise paths to
reduce egress surface and preserve the deny-by-default posture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86ffe17a-5239-4e61-927a-de4313712413
📒 Files selected for processing (1)
nemoclaw-blueprint/policies/openclaw-sandbox.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/validate-blueprint.test.ts (1)
181-189: Make the GET regression test non-vacuous on its own.If this test is run in isolation, it can pass even when
sentry.iois missing entirely. Add an explicit existence assertion here too.Suggested diff
it("regression `#1437`: sentry.io retains GET (harmless, no body for exfil)", () => { const sentryEndpoints = findEndpoints((h) => h === "sentry.io"); + expect(sentryEndpoints.length).toBeGreaterThan(0); for (const ep of sentryEndpoints) { const rules = Array.isArray(ep.rules) ? ep.rules : []; const hasGet = rules.some( (r) => r && r.allow && typeof r.allow.method === "string" && r.allow.method.toUpperCase() === "GET", ); expect(hasGet).toBe(true); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/validate-blueprint.test.ts` around lines 181 - 189, The test currently only checks that any found sentry.io endpoints allow GET, but it can pass vacuously if none are found; ensure the test asserts the endpoints exist by adding an explicit existence assertion on sentryEndpoints (e.g., expect(sentryEndpoints.length).toBeGreaterThan(0)) immediately after const sentryEndpoints = findEndpoints((h) => h === "sentry.io"); so that the loop over sentryEndpoints and the hasGet check actually validate real entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/validate-blueprint.test.ts`:
- Around line 181-189: The test currently only checks that any found sentry.io
endpoints allow GET, but it can pass vacuously if none are found; ensure the
test asserts the endpoints exist by adding an explicit existence assertion on
sentryEndpoints (e.g., expect(sentryEndpoints.length).toBeGreaterThan(0))
immediately after const sentryEndpoints = findEndpoints((h) => h ===
"sentry.io"); so that the loop over sentryEndpoints and the hasGet check
actually validate real entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d1fafa0-a397-475d-bf66-d3329a5ac28e
📒 Files selected for processing (1)
test/validate-blueprint.test.ts
ericksoa
left a comment
There was a problem hiding this comment.
Policy change is correct — blocks POST to sentry.io to close the multi-tenant exfiltration vector. GET-only is safe (no request body). Restored the regression tests that were dropped during merge conflict resolution. LGTM.
Summary
Closes #1437. Supersedes #1565.
/api/*/envelope/**and/api/*/store/**) withGET /**on thesentry.ioendpointCredit to @ColinM-sys for the original analysis and fix in #1565 (fork was unreachable behind a proxy, so we had to reopen from origin).
Test plan
sentry.ioendpoint innemoclaw-blueprint/policies/openclaw-sandbox.yamlhas only a GET rule, no POSTnpm testto confirm no test regressions🤖 Generated with Claude Code
Summary by CodeRabbit