fix(security): bind Ollama to localhost instead of 0.0.0.0#1140
fix(security): bind Ollama to localhost instead of 0.0.0.0#1140fdzdev wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
📝 WalkthroughWalkthroughPlatform-specific Ollama binding configuration introduced during onboarding and validation. macOS binds to 127.0.0.1, other Linux systems use 0.0.0.0, and WSL passes no override. Error messages and documentation updated to reflect these platform distinctions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
I'm worried this change may break the current local Ollama flow on Linux. A few places in the repo currently assume sandbox/container reachability requires Ollama to listen on
So the premise here (that If that premise has now been validated across supported setups, I think we need to update the validation logic/docs/tests in the same PR, and ideally add a regression test proving a container can still reach Without that, I'd be hesitant to merge this as-is because it looks likely to regress local inference reachability. |
On macOS, Docker Desktop routes host-gateway through the VM, so 127.0.0.1 is reachable from containers — bind to localhost to avoid exposing the Ollama API to the LAN (CWE-668, NVBUG 6014821). On Linux, containers access the host via the Docker bridge IP, so 0.0.0.0 binding is still required for container reachability. - Platform-aware OLLAMA_HOST in onboard.js general and macOS paths - Platform-aware error message in local-inference.js validation - Updated spark-install.md docs to clarify Linux-only requirement - Updated test-gpu-e2e.sh comments for platform context - Added regression tests for platform-specific error messages Made-with: Cursor
ba19399 to
946e5bb
Compare
|
Reworked in 946e5bb — you're right, 127.0.0.1 breaks container reachability on Linux. The fix is now platform-aware: 127.0.0.1 on macOS (Docker Desktop routes host-gateway through the VM) and 0.0.0.0 on Linux (Docker bridge requires it). Updated the validation error message, docs, E2E test comments, and added regression tests for both platforms. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 2645-2650: The code sets ollamaEnv to "OLLAMA_HOST=0.0.0.0:11434"
for non-WSL Linux which exposes the API off-host; change the non-WSL branch that
currently checks process.platform to use a loopback bind (e.g.,
"OLLAMA_HOST=127.0.0.1:11434") instead of 0.0.0.0 so Linux gets the same
hardening as macOS; update the ollamaEnv assignment (the variable ollamaEnv and
the surrounding isWsl() / process.platform logic) to default to 127.0.0.1 unless
an explicit configuration/flag requires 0.0.0.0.
🪄 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: Pro
Run ID: 72db8f4e-8ae4-4108-9e19-acef5662f5bf
📒 Files selected for processing (6)
bin/lib/local-inference.jsbin/lib/onboard.jsscripts/setup.shspark-install.mdtest/e2e/test-gpu-e2e.shtest/local-inference.test.js
✅ Files skipped from review due to trivial changes (2)
- test/e2e/test-gpu-e2e.sh
- spark-install.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/setup.sh
bin/lib/local-inference.js
Outdated
| process.platform === "darwin" | ||
| ? "Local Ollama is responding on localhost, but containers cannot reach http://host.openshell.internal:11434. Restart Docker Desktop and ensure host networking is enabled." | ||
| : "Local Ollama is responding on localhost, but containers cannot reach http://host.openshell.internal:11434. Ensure Ollama listens on 0.0.0.0:11434 (not 127.0.0.1) so sandboxes can reach it via the Docker bridge.", |
There was a problem hiding this comment.
WSL gets the wrong remediation here.
Line 101 sends every non-macOS host down the 0.0.0.0 guidance, but bin/lib/onboard.js Lines 2645-2651 intentionally skip any OLLAMA_HOST override on WSL. If container reachability fails there, this message tells WSL users to expose Ollama unnecessarily instead of troubleshooting the Docker/WSL path. Please add a WSL-specific branch, or pass the actual startup mode into validateLocalProvider, so the guidance matches how Ollama was launched.
| let ollamaEnv = ""; | ||
| if (!isWsl()) { | ||
| ollamaEnv = | ||
| process.platform === "darwin" | ||
| ? "OLLAMA_HOST=127.0.0.1:11434 " | ||
| : "OLLAMA_HOST=0.0.0.0:11434 "; |
There was a problem hiding this comment.
Non-WSL Linux still publishes Ollama off-host.
Line 2650 keeps the default Linux launch path on OLLAMA_HOST=0.0.0.0:11434. A normal nemoclaw onboard on Linux therefore still exposes the Ollama API beyond the host, so this hardening only lands for macOS right now. If Linux is meant to be covered too, this branch needs to stop forcing 0.0.0.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/onboard.js` around lines 2645 - 2650, The code sets ollamaEnv to
"OLLAMA_HOST=0.0.0.0:11434" for non-WSL Linux which exposes the API off-host;
change the non-WSL branch that currently checks process.platform to use a
loopback bind (e.g., "OLLAMA_HOST=127.0.0.1:11434") instead of 0.0.0.0 so Linux
gets the same hardening as macOS; update the ollamaEnv assignment (the variable
ollamaEnv and the surrounding isWsl() / process.platform logic) to default to
127.0.0.1 unless an explicit configuration/flag requires 0.0.0.0.
Resolved conflicts: - bin/lib/local-inference.js: take main's dist/ shim (implementation moved to src/lib/local-inference.ts) - bin/lib/onboard.js: keep PR's platform-aware Ollama binding (127.0.0.1 on macOS, 0.0.0.0 on Linux, WSL2 default) for install-ollama; add main's new vllm handler block - src/lib/local-inference.ts: update container reachability error message to be platform-aware (mirrors new onboard.js binding logic) - src/lib/local-inference.test.ts: update test to match platform-aware error message - test/e2e/test-gpu-e2e.sh: keep main's NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE check; update comment to clarify Linux-only 0.0.0.0 requirement - scripts/setup.sh, test/local-inference.test.js: accept main's deletion (setup.sh superseded; test moved to TypeScript) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 2751-2764: The Ollama bind-selection (the computation of ollamaEnv
using isWsl() and process.platform === "darwin") must be reused for both launch
paths so macOS doesn't get OLLAMA_HOST=0.0.0.0:11434; update the code that runs
Ollama (the run(...) call in the non-install branch where selected.key ===
"ollama") to compute and use the same ollamaEnv logic (or call a small helper
function) as the install-ollama path instead of hardcoding 0.0.0.0, referencing
the existing isWsl, ollamaEnv variable logic and the run(...) invocation to
locate where to change.
In `@src/lib/local-inference.ts`:
- Around line 122-124: The current platform branch uses only darwin vs else and
incorrectly directs WSL users to Linux advice; update the conditional that
checks process.platform (the ternary producing the two hint strings) to detect
WSL (e.g., check process.env.WSL_DISTRO_NAME or process.platform === "linux" &&
/microsoft/i.test(require("os").release())) and add a third branch for WSL with
a specific hint: explain that OLLAMA_HOST should be left unset on WSL because
0.0.0.0 breaks host-gateway reachability and point them to restart Docker
Desktop or use appropriate WSL host networking workarounds (mirroring the intent
in bin/lib/onboard.js). Ensure the darwin and non-WSL linux messages remain
unchanged.
🪄 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: Pro
Run ID: 0b808fec-4392-4e0b-9cc2-c4cdc6567bb1
📒 Files selected for processing (5)
bin/lib/onboard.jsspark-install.mdsrc/lib/local-inference.test.tssrc/lib/local-inference.tstest/e2e/test-gpu-e2e.sh
✅ Files skipped from review due to trivial changes (3)
- spark-install.md
- src/lib/local-inference.test.ts
- test/e2e/test-gpu-e2e.sh
| // On macOS, Docker Desktop routes host-gateway through the VM so | ||
| // 127.0.0.1 is reachable from containers — bind to localhost to | ||
| // avoid exposing Ollama to the LAN (CWE-668, NVBUG 6014821). | ||
| // On Linux, containers access the host via the Docker bridge IP | ||
| // so 0.0.0.0 is required for reachability. | ||
| // On WSL2, the default binding works without override. | ||
| let ollamaEnv = ""; | ||
| if (!isWsl()) { | ||
| ollamaEnv = | ||
| process.platform === "darwin" | ||
| ? "OLLAMA_HOST=127.0.0.1:11434 " | ||
| : "OLLAMA_HOST=0.0.0.0:11434 "; | ||
| } | ||
| run(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true }); |
There was a problem hiding this comment.
Reuse the bind-selection logic in both Ollama launch paths.
Line 2334 makes install-ollama macOS-only, so the non-darwin branch here is dead. Meanwhile Line 2697 in the regular selected.key === "ollama" flow still launches a stopped Ollama with OLLAMA_HOST=0.0.0.0:11434 on macOS, which reintroduces the LAN exposure this PR is trying to remove.
💡 Suggested fix
+function getOllamaServeEnvPrefix() {
+ if (isWsl()) return "";
+ return process.platform === "darwin"
+ ? "OLLAMA_HOST=127.0.0.1:11434 "
+ : "OLLAMA_HOST=0.0.0.0:11434 ";
+}
...
- const ollamaEnv = isWsl() ? "" : "OLLAMA_HOST=0.0.0.0:11434 ";
+ const ollamaEnv = getOllamaServeEnvPrefix();
run(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true });
...
- let ollamaEnv = "";
- if (!isWsl()) {
- ollamaEnv =
- process.platform === "darwin"
- ? "OLLAMA_HOST=127.0.0.1:11434 "
- : "OLLAMA_HOST=0.0.0.0:11434 ";
- }
+ const ollamaEnv = getOllamaServeEnvPrefix();
run(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/onboard.js` around lines 2751 - 2764, The Ollama bind-selection (the
computation of ollamaEnv using isWsl() and process.platform === "darwin") must
be reused for both launch paths so macOS doesn't get OLLAMA_HOST=0.0.0.0:11434;
update the code that runs Ollama (the run(...) call in the non-install branch
where selected.key === "ollama") to compute and use the same ollamaEnv logic (or
call a small helper function) as the install-ollama path instead of hardcoding
0.0.0.0, referencing the existing isWsl, ollamaEnv variable logic and the
run(...) invocation to locate where to change.
| process.platform === "darwin" | ||
| ? "Local Ollama is responding on localhost, but containers cannot reach http://host.openshell.internal:11434. Restart Docker Desktop and ensure host networking is enabled." | ||
| : "Local Ollama is responding on localhost, but containers cannot reach http://host.openshell.internal:11434. Ensure Ollama listens on 0.0.0.0:11434 (not 127.0.0.1) so sandboxes can reach it via the Docker bridge.", |
There was a problem hiding this comment.
Handle WSL separately in the Ollama reachability hint.
This darwin/else split still sends WSL users to the Linux advice, but bin/lib/onboard.js intentionally leaves OLLAMA_HOST unset on WSL because 0.0.0.0 breaks host-gateway reachability there. If this validation fails on WSL, the message now points users at the wrong remediation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/local-inference.ts` around lines 122 - 124, The current platform
branch uses only darwin vs else and incorrectly directs WSL users to Linux
advice; update the conditional that checks process.platform (the ternary
producing the two hint strings) to detect WSL (e.g., check
process.env.WSL_DISTRO_NAME or process.platform === "linux" &&
/microsoft/i.test(require("os").release())) and add a third branch for WSL with
a specific hint: explain that OLLAMA_HOST should be left unset on WSL because
0.0.0.0 breaks host-gateway reachability and point them to restart Docker
Desktop or use appropriate WSL host networking workarounds (mirroring the intent
in bin/lib/onboard.js). Ensure the darwin and non-WSL linux messages remain
unchanged.
Summary
OLLAMA_HOST=0.0.0.0:11434with127.0.0.1:11434in all three launch sites (CWE-668, NVBUG 6014821)0.0.0.0exposes the Ollama API to the entire LAN, enabling DNS rebinding and unauthorized model accesshost-gatewaywhich routes to loopback, so127.0.0.1is sufficient for sandbox connectivityTest plan
nemoclaw onboardwith Ollama on Linux — Ollama binds to127.0.0.1:11434, not0.0.0.0host-gateway/host.docker.internalnemoclaw onboardon macOS (brew install path) — same localhost bindingSummary by CodeRabbit
Bug Fixes
Documentation
Tests