fix(openclaw): confine host-gateway web_fetch carve-out#4325
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR replaces Dockerfile Patch 2b hostname-validator rewrites with a web-fetch guard rewrite that conditionally injects ChangesOpenShell Fetch-Guard Patch Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Selective E2E Results — ✅ All requested jobs passedRun: 26523238086
|
Selective E2E Results — ✅ All requested jobs passedRun: 26523296507
|
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas This is an automated advisory review. A human maintainer must make the final merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
281-309: Run a container-backed E2E pass for this patch.This block rewrites compiled OpenClaw JS inside the image, so the fixture tests are helpful but not sufficient for the final sandbox artifact. I’d run the recommended nightly E2E subset on the built image before merge.
As per coding guidelines, "Dockerfile: This file affects the sandbox container image. Layer ordering, permissions, and baked config changes are only testable with a real container build."
🤖 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 `@Dockerfile` around lines 281 - 309, Patch 2b mutates compiled OpenClaw JS inside the image (the block that patches files found by web_guard_files looking for fetchWithWebToolsNetworkGuard(params) and injects OPENSHELL_SANDBOX host-gateway logic around withTrustedEnvProxyGuardedFetchMode(resolved)/fetchWithSsrFGuard), so run a container-backed E2E pass on the built image before merging: build the Dockerfile image, run the recommended nightly E2E subset against that container, verify the patch was applied (look for the nemoclaw marker and the OPENSHELL_SANDBOX host openshell.internal hostname check), confirm web_fetch/trusted-proxy behavior and that no runtime errors occur, and only merge if the E2E tests pass.
🤖 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 `@Dockerfile`:
- Around line 301-307: The fail-closed detector populates web_fetch_proxy_refs
by grepping for the wrong proxy symbol (it looks for useTrustedEnvProxy and a
specific withTrustedEnvProxyGuardedFetchMode(resolved) call), so change the grep
in the web_fetch_proxy_refs assignment to also include the real runtime symbol
useEnvProxy (and any other variant like useTrustedEnvProxy if desired) so the
check correctly detects remaining proxy callsites; update the grep pattern in
the web_fetch_proxy_refs assignment (the line that sets web_fetch_proxy_refs
using grep -RIlE --include='*.js' ...) to match useEnvProxy and the existing
withTrustedEnvProxyGuardedFetchMode(resolved) token, and keep the existing
behavior of printing OC_VERSION, OC_DIST and invoking patch_fail if matches are
found.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 281-309: Patch 2b mutates compiled OpenClaw JS inside the image
(the block that patches files found by web_guard_files looking for
fetchWithWebToolsNetworkGuard(params) and injects OPENSHELL_SANDBOX host-gateway
logic around withTrustedEnvProxyGuardedFetchMode(resolved)/fetchWithSsrFGuard),
so run a container-backed E2E pass on the built image before merging: build the
Dockerfile image, run the recommended nightly E2E subset against that container,
verify the patch was applied (look for the nemoclaw marker and the
OPENSHELL_SANDBOX host openshell.internal hostname check), confirm
web_fetch/trusted-proxy behavior and that no runtime errors occur, and only
merge if the E2E tests pass.
🪄 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: a648c922-faf1-4212-84e2-cb4f8ffafe84
📒 Files selected for processing (3)
Dockerfilescripts/generate-openclaw-config.pytest/fetch-guard-patch-regression.test.ts
Selective E2E Results — ❌ Some jobs failedRun: 26523364300
|
Selective E2E Results — ❌ Some jobs failedRun: 26525054507
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26526524270
|
Selective E2E Results — ❌ Some jobs failedRun: 26527083695
|
Selective E2E Results — ❌ Some jobs failedRun: 26527962124
|
Selective E2E Results — ❌ Some jobs failedRun: 26530299117
|
Selective E2E Results — ✅ All requested jobs passedRun: 26530366040
|
Selective E2E Results — ❌ Some jobs failedRun: 26530463243
|
Selective E2E Results — ✅ All requested jobs passedRun: 26536918913
|
Selective E2E Results — ✅ All requested jobs passedRun: 26537485546
|
Selective E2E Results — ❌ Some jobs failedRun: 26537635717
|
## Summary Refreshes the NemoClaw documentation for the v0.0.54 release and regenerates user skills from the Fern MDX source. Also keeps the Fern CLI pin current so local docs checks use the upgraded Fern version. ## Related Issue <!-- No single related issue. This is release-prep documentation catch-up. --> ## Changes - #4403 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document Telegram, Discord, and Slack post-rebuild bridge verification and summarize channel activation fixes. - #4222 -> `docs/about/release-notes.mdx`: Include Slack generated channel enablement in the v0.0.54 messaging summary. - #4346 -> `docs/get-started/windows-preparation.mdx`, `docs/about/release-notes.mdx`: Document safer Windows bootstrap behavior for Ubuntu first-run and Docker Desktop WSL integration. - #4416 -> `docs/inference/use-local-inference.mdx`, `docs/about/release-notes.mdx`: Document the Docker Desktop WSL requirement for Windows-host Ollama. - #4442 -> `docs/about/release-notes.mdx`: Summarize the optional NemoHermes native web dashboard and related environment variables. - #4426 -> `docs/about/release-notes.mdx`: Summarize copy-paste recovery hints for invalid sandbox names and missing NVIDIA API keys. - #4459 -> `docs/about/release-notes.mdx`: Summarize the Linuxbrew prefix fix for sandbox Homebrew usage. - #4450 -> `docs/about/release-notes.mdx`: Summarize `/nemoclaw` slash command startup activation. - #4468 -> `docs/about/release-notes.mdx`: Summarize scope-upgrade approval recovery. - #4325 -> `docs/about/release-notes.mdx`: Summarize the narrowed `web_fetch` host-gateway allowance. - #4474 -> `docs/about/release-notes.mdx`: Summarize Hermes Provider smoke-check behavior for OAuth versus Nous API key setup. - Refresh generated `.agents/skills/nemoclaw-user-*` references from `docs/` and update `fern/fern.config.json` to Fern `5.41.2`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Optional NemoHermes native web dashboard (configurable port and TUI) * GPU memory cleanup now unloads Ollama models when switching providers or stopping services * **Bug Fixes** * Improved sandbox name validation with suggested slug recovery * Windows-host Ollama now requires Docker Desktop WSL integration and exits with remediation guidance when unsupported * **Documentation** * Clarified quickstart/onboard flow, installer TTY/non‑TTY guidance, Hermes Docker prerequisites, sandbox hardening, and channels add rebuild checks <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4539?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Validation
Follow-up to #4288 PR review advisor feedback.
Summary by CodeRabbit
Documentation
Tests
Chores