fix(blueprint): default OpenClaw fetch guard to sandbox proxy - #5713
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
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:
📝 WalkthroughWalkthroughDockerfile Patch 4 is rewritten to target ChangesPatch 4: resolveGuardedFetchMode sandbox default
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Selective E2E Results — ❌ Some jobs failedRun: 28182597555
|
Selective E2E Results — ✅ All requested jobs passedRun: 28182597555
|
Selective E2E Results — ❌ Some jobs failedRun: 28182597555
|
Selective E2E Results — ❌ Some jobs failedRun: 28182597555
|
1 similar comment
Selective E2E Results — ❌ Some jobs failedRun: 28182597555
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28424214692
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28425869865
|
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28427201204
|
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28427258935
|
Signed-off-by: San Dang <sdang@nvidia.com>
Maintainer status — hard deferCurrent head: Exact-head verification is green:
The PR is still not merge-ready because both exact-head review advisors report
Sequencing is also unresolved with #5595, which upgrades OpenClaw to 2026.6.9 and overlaps the same Dockerfile and fetch-guard classifier tests. The landing order and exact 2026.6.9 patch shape must be coordinated, followed by a fresh advisor/CI pass. Status: defer; do not merge until the advisor gates and #5595 sequencing are resolved or explicitly adjudicated by a maintainer. |
Signed-off-by: San Dang <sdang@nvidia.com>
|
Maintainer security gate for exact head fa2120f: do not merge into v0.0.71. The pinned openclaw@2026.5.27 artifact contains 93 fetchWithSsrFGuard calls. 86 use a plain object with no mode or dispatcher policy, 4 more can dynamically omit mode, and only 3 are always explicit. This PR globally reclassifies omitted-mode calls from STRICT to TRUSTED_ENV_PROXY inside the sandbox. That removes resolvePinnedHostnameWithPolicy, private/reserved-address rejection, and DNS pinning from paths that include webhooks, downloads, media, provider endpoints, browser/CDP, and messaging APIs. The current tests assert zero pinned-DNS resolution and cover only literal blocked hosts; they do not cover a public-looking name resolving or rebinding to loopback, link-local, metadata, or private space. This is also a regression from current main: the PR replaces the existing strict-preserving managed-proxy Patch 4 and narrow cron Patch 6. It should retain those mainline patches. Any additional exception must be reduced to audited callsite-specific trusted mode for demonstrably operator-controlled endpoints, with exact 2026.5.27 published-dist proof and negative DNS-answer/rebinding tests. PR #5595 later ports the strict-preserving design to 2026.6.9 and must not be mechanically combined with this broad rewrite. No CI, approval, or E2E result can override this source-level SSRF boundary failure. The branch needs a security-preserving redesign before further release consideration. |
…proxy (#6051) Reverts #5713, merged by accident <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox proxy handling so unconfigured strict network requests are routed correctly in OpenShell environments. * Added safer cron preflight behavior to avoid breaking DNS-pinned strict-mode runs in proxy-only sandboxes. * **Chores** * Removed an unused runtime verification step and simplified the build process. * Updated related automation and test coverage to match the new proxy and patching behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cv@lixo.org>
…#5713) ## Summary Broadens the OpenClaw compatibility patch so `resolveGuardedFetchMode()` defaults omitted guarded-fetch modes to `trusted_env_proxy` inside OpenShell sandboxes. This avoids local DNS lookups for bare `fetchWithSsrFGuard` callers while preserving explicit modes and strict behavior outside `OPENSHELL_SANDBOX=1`. ## Related Issue Related: NVIDIA#396 (fetch-guard/local DNS portion only; Gemini policy/docs remain out of scope) Fixes NVIDIA#4687 Related: NVIDIA#5129 Related: NVIDIA#5592 ## Changes - Patch OpenClaw's central guarded-fetch mode resolver in the sandbox image instead of rewriting individual call sites. - Remove the deprecated `proxy: "env"` compatibility branch from the patched resolver and drop the old cron-preflight call-site patch. - Cover additional bare `fetchWithSsrFGuard` users, including the Mattermost/OpenClaw call site discussed in NVIDIA#5592. - Update fetch-guard patch regression tests for sandbox defaults, explicit-mode precedence, out-of-sandbox strict behavior, and removal of the narrower Patch 6 rewrite. ## Localized compatibility patch review - **Invalid state:** OpenClaw 2026.5.27 resolves an omitted guarded-fetch mode to strict DNS pinning. In an OpenShell proxy-only sandbox, that performs local DNS before proxy dispatch, so sandbox-only and proxy-routed names fail before policy enforcement. - **Source boundary:** NemoClaw consumes the pinned compiled OpenClaw distribution in the sandbox image; it does not own the upstream resolver source. The image build therefore classifies and patches the reviewed `resolveGuardedFetchMode()` shape centrally and fails closed if that shape drifts. - **Source-fix constraint:** The durable fix belongs in OpenClaw: either select `trusted_env_proxy` for OpenShell sandboxes at the resolver boundary or make every sandbox-sensitive caller pass that mode explicitly. A permanent NemoClaw fork of the third-party runtime is out of scope. - **Regression proof:** `test/fetch-guard-patch-regression.test.ts` verifies the reviewed resolver shape, explicit-mode precedence, sandbox-only default, strict behavior outside the sandbox, removal of the deprecated opt-in, representative SSRF denials, and fail-closed drift handling. The exact merged head passes all 23 focused tests. - **Removal condition:** Remove Patch 4 when the pinned OpenClaw release provides an equivalent OpenShell-aware default, or when every sandbox-sensitive caller explicitly selects trusted proxy mode and the runtime acceptance checks pass without this patch. ## Security boundary and E2E Inspection of the pinned OpenClaw 2026.5.27 implementation confirms that explicit modes still override the default, hostname validation runs before proxy dispatcher construction, loopback/metadata/private/special-use IPv4 and IPv6 literals remain rejected, and redirects are handled manually and revalidated on each hop. DNS-resolution enforcement intentionally moves to the OpenShell L7 proxy and network-policy boundary only for omitted-mode calls inside the sandbox. A unified exact-head E2E dispatch covers `cron-preflight-inference-local`, `inference-routing`, `network-policy`, and `common-egress-agent`: https://github.com/NVIDIA/NemoClaw/actions/runs/28424214692 ## Type of Change - [x] 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) ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal commit and push hooks passed on the current head - [x] Targeted test passes: 23/23 - [x] Tests added or updated for changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Exact-head unified E2E is passing (in progress) --- Signed-off-by: San Dang <sdang@nvidia.com> --------- Signed-off-by: San Dang <sdang@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
…proxy (NVIDIA#6051) Reverts NVIDIA#5713, merged by accident <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox proxy handling so unconfigured strict network requests are routed correctly in OpenShell environments. * Added safer cron preflight behavior to avoid breaking DNS-pinned strict-mode runs in proxy-only sandboxes. * **Chores** * Removed an unused runtime verification step and simplified the build process. * Updated related automation and test coverage to match the new proxy and patching behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cv@lixo.org>
Summary
Broadens the OpenClaw compatibility patch so
resolveGuardedFetchMode()defaults omitted guarded-fetch modes totrusted_env_proxyinside OpenShell sandboxes. This avoids local DNS lookups for barefetchWithSsrFGuardcallers while preserving explicit modes and strict behavior outsideOPENSHELL_SANDBOX=1.Related Issue
Related: #396 (fetch-guard/local DNS portion only; Gemini policy/docs remain out of scope)
Fixes #4687
Related: #5129
Related: #5592
Changes
proxy: "env"compatibility branch from the patched resolver and drop the old cron-preflight call-site patch.fetchWithSsrFGuardusers, including the Mattermost/OpenClaw call site discussed in feat(messaging): onboard Mattermost for OpenClaw #5592.Localized compatibility patch review
resolveGuardedFetchMode()shape centrally and fails closed if that shape drifts.trusted_env_proxyfor OpenShell sandboxes at the resolver boundary or make every sandbox-sensitive caller pass that mode explicitly. A permanent NemoClaw fork of the third-party runtime is out of scope.test/fetch-guard-patch-regression.test.tsverifies the reviewed resolver shape, explicit-mode precedence, sandbox-only default, strict behavior outside the sandbox, removal of the deprecated opt-in, representative SSRF denials, and fail-closed drift handling. The exact merged head passes all 23 focused tests.Security boundary and E2E
Inspection of the pinned OpenClaw 2026.5.27 implementation confirms that explicit modes still override the default, hostname validation runs before proxy dispatcher construction, loopback/metadata/private/special-use IPv4 and IPv6 literals remain rejected, and redirects are handled manually and revalidated on each hop. DNS-resolution enforcement intentionally moves to the OpenShell L7 proxy and network-policy boundary only for omitted-mode calls inside the sandbox.
A unified exact-head E2E dispatch covers
cron-preflight-inference-local,inference-routing,network-policy, andcommon-egress-agent: https://github.com/NVIDIA/NemoClaw/actions/runs/28424214692Type of Change
Verification
Verifiedin GitHubSigned-off-by: San Dang sdang@nvidia.com