fix(proxy): inject NO_PROXY=localhost,127.0.0.1 when HTTP_PROXY is set (#2616)

[Dongni-Yang]

Summary

On hosts with a system HTTP proxy (Privoxy, corporate CONNECT proxy, Cursor's bundled proxy), NemoClaw spawned the Ollama daemon and its :11435 auth proxy with HTTP_PROXY inherited from the shell but no NO_PROXY set. Loopback requests — the auth-proxy health probe, ollama pull, and macOS curl's token validation — were tunnelled through the proxy and failed with:

HTTP 500 — Internal Privoxy Error
Privoxy could not connect to the host: 127.0.0.1:11435

The root cause is two call sites that used { ...process.env } directly instead of the shared buildSubprocessEnv() allowlist, combined with buildSubprocessEnv() not guarding against a proxy-without-NO_PROXY environment.

Related Issue

Fixes #2616

How the fix works

Call site	Before	After
spawnOllamaAuthProxy()	{ ...process.env, token, ports } — full env including secrets	buildSubprocessEnv({ token, ports }) — allowlisted env + NO_PROXY injected
pullOllamaModel()	{ ...process.env } — full env	buildSubprocessEnv() — allowlisted env + NO_PROXY injected
buildSubprocessEnv()	forwards HTTP_PROXY with no NO_PROXY guard	calls withLocalNoProxy() which appends localhost,127.0.0.1 to both NO_PROXY and no_proxy when any proxy var is present

withLocalNoProxy() is additive: it appends only the missing entries so an existing NO_PROXY=corp.internal becomes corp.internal,localhost,127.0.0.1, preserving outbound proxy functionality for external registries like registry.ollama.ai.

Changes

• src/lib/subprocess-env.ts: add exported withLocalNoProxy(env) helper; call it at the end of buildSubprocessEnv(). When HTTP_PROXY, HTTPS_PROXY, http_proxy, or https_proxy is present, appends localhost and 127.0.0.1 to both NO_PROXY and no_proxy if either entry is missing.
• nemoclaw/src/lib/subprocess-env.ts: mirror of above (kept in sync per existing policy).
• src/lib/onboard-ollama-proxy.ts: add require("./subprocess-env") import; switch spawnOllamaAuthProxy() from { ...process.env, ... } to buildSubprocessEnv({ ... }) (also fixes the fix(security): replace process.env spread with allowlist in blueprint runner #1874 secret-leakage risk for this spawn); switch pullOllamaModel() from { ...process.env } to buildSubprocessEnv().
• src/lib/subprocess-env.test.ts (new): 11 unit tests covering withLocalNoProxy (no proxy → no-op, HTTP_PROXY/HTTPS_PROXY/http_proxy each trigger injection, partial NO_PROXY is extended, full NO_PROXY is unchanged) and buildSubprocessEnv integration (injection on proxy set, augmentation of existing NO_PROXY, no injection when no proxy, extra vars preserved).

Type of Change

• 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

• npx prek run --all-files passes
• npm test passes (subprocess-env: 11/11 new tests pass; full suite clean)
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• make docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

AI Disclosure

• AI-assisted — tool: Claude Code

---

Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Fixed HTTP proxy configuration to ensure localhost and loopback traffic properly bypass proxy routing when proxy environment variables are configured.

• Tests

  • Added comprehensive test coverage for proxy bypass functionality in subprocess environment setup.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds a withLocalNoProxy utility function that ensures localhost (127.0.0.1 and localhost) are included in the NO_PROXY environment variable when HTTP proxy variables are set in spawned subprocesses. The function is integrated into buildSubprocessEnv and applied to subprocess spawning in the Ollama onboarding flow.

Changes

Cohort / File(s)	Summary
NO_PROXY Injection Utility nemoclaw/src/lib/subprocess-env.ts, src/lib/subprocess-env.ts	Adds new withLocalNoProxy(env) function that mutates environment to append localhost and 127.0.0.1 to NO_PROXY/no_proxy when HTTP proxy variables are detected. Updates buildSubprocessEnv to invoke this function before returning the constructed environment.
Subprocess Spawning Integration src/lib/onboard-ollama-proxy.ts	Replaces direct process.env spread in auth-proxy and ollama pull subprocess spawning with calls to buildSubprocessEnv, allowing proxy environment configuration and NO_PROXY injection to be applied consistently.
Test Coverage src/lib/subprocess-env.test.ts	New test suite verifying withLocalNoProxy correctly handles missing proxy variables (no mutation), missing NO_PROXY entries (sets both NO_PROXY and no_proxy), existing NO_PROXY preservation (appends only missing loopback hosts), and deduplication of loopback addresses. Includes integration tests for buildSubprocessEnv with proxy scenarios and extra environment variable overrides.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A proxy once blocked localhost's door,
So subprocesses wandered (and crashed) forevermore.
But NO_PROXY whispers to loopback with care,
Now 127.0.0.1 can go anywhere! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: injecting NO_PROXY=localhost,127.0.0.1 when HTTP_PROXY is set to fix proxy routing issues.
Linked Issues check	✅ Passed	The PR fully addresses issue #2616 requirements: adds withLocalNoProxy() helper, updates buildSubprocessEnv() to inject NO_PROXY, and fixes both call sites (spawnOllamaAuthProxy and pullOllamaModel).
Out of Scope Changes check	✅ Passed	All changes are directly related to issue #2616: subprocess environment handling for NO_PROXY injection, affected call site updates, and comprehensive test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/subprocess-env.test.ts (1)

28-33: Optional: add a lowercase https_proxy test case for full branch coverage.

withLocalNoProxy checks lowercase https_proxy on Line 59 in src/lib/subprocess-env.ts, but this suite currently validates lowercase http_proxy only.

➕ Suggested test addition

   it("adds localhost and 127.0.0.1 when lowercase http_proxy is set", () => {
     const env: Record<string, string> = { http_proxy: "http://proxy:8888" };
     withLocalNoProxy(env);
     expect(env.NO_PROXY).toBe("localhost,127.0.0.1");
     expect(env.no_proxy).toBe("localhost,127.0.0.1");
   });
+
+  it("adds localhost and 127.0.0.1 when lowercase https_proxy is set", () => {
+    const env: Record<string, string> = { https_proxy: "http://proxy:8888" };
+    withLocalNoProxy(env);
+    expect(env.NO_PROXY).toBe("localhost,127.0.0.1");
+    expect(env.no_proxy).toBe("localhost,127.0.0.1");
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/subprocess-env.test.ts` around lines 28 - 33, Add a parallel test
that mirrors the existing "adds localhost and 127.0.0.1 when lowercase
http_proxy is set" case but uses a lowercase https_proxy environment variable
instead; call withLocalNoProxy(env) and assert that both env.NO_PROXY and
env.no_proxy equal "localhost,127.0.0.1". This ensures the branch in
withLocalNoProxy that checks for lowercase https_proxy is covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/subprocess-env.test.ts`:
- Around line 28-33: Add a parallel test that mirrors the existing "adds
localhost and 127.0.0.1 when lowercase http_proxy is set" case but uses a
lowercase https_proxy environment variable instead; call withLocalNoProxy(env)
and assert that both env.NO_PROXY and env.no_proxy equal "localhost,127.0.0.1".
This ensures the branch in withLocalNoProxy that checks for lowercase
https_proxy is covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e90ef77b-d766-4d41-99ac-7948bbaa2349

📥 Commits

Reviewing files that changed from the base of the PR and between 7c5174e and 659d2d7.

📒 Files selected for processing (4)

• nemoclaw/src/lib/subprocess-env.ts
• src/lib/onboard-ollama-proxy.ts
• src/lib/subprocess-env.test.ts
• src/lib/subprocess-env.ts

[Dongni-Yang]

Local macOS Verification — #2616

Validated the fix end-to-end on Apple Silicon macOS (Darwin 25.0.0) after merge.

Setup

• Ollama 0.22.0 (brew install ollama) with qwen2.5:0.5b pulled
• Fake Privoxy on :8118 (Python one-liner, no real Privoxy needed)
• Built both the pre-fix (7c5174e) and fix (659d2d7) commits locally from ~/nemoclaw-fix

Bug confirmed on 7c5174e (pre-fix)

With HTTP_PROXY=http://127.0.0.1:8118 set and NO_PROXY unset, the subprocess spawned with { ...process.env } routed the loopback probe through the proxy:
[proxy] CONNECT 127.0.0.1:11435 HTTP/1.1 → 500 Privoxy Error

Fix verified on 659d2d7

After switching to buildSubprocessEnv() (which calls withLocalNoProxy() automatically), the fake Privoxy terminal stayed completely silent — curl connected directly to
127.0.0.1:11435, bypassing the proxy.

Buggy build hits proxy? YES ✗
Fixed build hits proxy? NO ✓

Repro result: PASS — bug reproduced and fix verified.