Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/lib/inference/ollama/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,12 @@ function pullOllamaModelViaHttp(model) {
body,
url,
],
{ stdio: ["ignore", "pipe", "pipe"] },
{
stdio: ["ignore", "pipe", "pipe"],
// #2616: inject NO_PROXY=localhost so the streamed pull against the
// local Ollama daemon doesn't tunnel through the user's host proxy.
env: buildSubprocessEnv(),
},
);

const readline = require("readline");
Expand Down Expand Up @@ -747,7 +752,9 @@ function unloadOllamaModels() {
const psResult = spawnSync(
"curl",
["-sS", "--max-time", "3", `http://localhost:${OLLAMA_PORT}/api/ps`],
{ encoding: "utf8" },
// #2616: env-sanitize so http_proxy=127.0.0.1:8118 (Privoxy) doesn't
// hijack this localhost probe.
{ encoding: "utf8", env: buildSubprocessEnv() },
);
if (psResult.status !== 0) return;

Expand Down Expand Up @@ -777,7 +784,8 @@ function unloadOllamaModels() {
JSON.stringify({ model: entry.name, keep_alive: 0 }),
`http://localhost:${OLLAMA_PORT}/api/generate`,
],
{ encoding: "utf8" },
// #2616: env-sanitize so http_proxy doesn't hijack the unload call.
{ encoding: "utf8", env: buildSubprocessEnv() },
);
}
} catch {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3358,7 +3358,7 @@ async function preflight(
process.exit(1);
}
console.log(" ✓ Docker is running");

require("./onboard/http-proxy-preflight").warnIfHostProxyMissesLoopback();
const optedOutGpuPassthrough =
preflightOpts.optedOutGpuPassthrough === true || preflightOpts.noGpu === true;
assertCdiNvidiaGpuSpecPresent(host, optedOutGpuPassthrough);
Expand Down
110 changes: 110 additions & 0 deletions src/lib/onboard/http-proxy-preflight.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { describe, expect, it } from "vitest";

import { redactProxyCredentials, warnIfHostProxyMissesLoopback } from "./http-proxy-preflight";

describe("redactProxyCredentials (#2616)", () => {
it("returns plain proxy URLs unchanged", () => {
expect(redactProxyCredentials("http://127.0.0.1:8118")).toBe("http://127.0.0.1:8118");
expect(redactProxyCredentials("http://corp-proxy.example.com:3128")).toBe(
"http://corp-proxy.example.com:3128",
);
});

it("redacts user:password basic-auth from proxy URLs", () => {
const redacted = redactProxyCredentials("http://alice:s3cret@proxy.example.com:3128");
expect(redacted).not.toContain("alice");
expect(redacted).not.toContain("s3cret");
expect(redacted).toContain("****");
expect(redacted).toContain("proxy.example.com:3128");
});

it("redacts username-only basic-auth", () => {
const redacted = redactProxyCredentials("http://token123@proxy.example.com:3128");
expect(redacted).not.toContain("token123");
expect(redacted).toContain("****");
});

it("falls back to regex redaction for non-URL-parseable strings", () => {
// Some users set HTTP_PROXY to malformed strings; we should still redact.
const redacted = redactProxyCredentials("//alice:s3cret@host");
expect(redacted).not.toContain("alice");
expect(redacted).not.toContain("s3cret");
});
});

describe("warnIfHostProxyMissesLoopback (#2616)", () => {
it("does not warn when no HTTP_PROXY is set", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback({}, (line) => lines.push(line));
expect(fired).toBe(false);
expect(lines).toEqual([]);
});

it("does not warn when NO_PROXY already includes localhost", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback(
{ http_proxy: "http://127.0.0.1:8118", NO_PROXY: "localhost,127.0.0.1" },
(line) => lines.push(line),
);
expect(fired).toBe(false);
expect(lines).toEqual([]);
});

it("warns when NO_PROXY only has localhost (127.0.0.1 still proxied) (CodeRabbit #3801)", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback(
{ http_proxy: "http://127.0.0.1:8118", NO_PROXY: "localhost" },
(line) => lines.push(line),
);
expect(fired).toBe(true);
expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1");
});

it("warns when NO_PROXY only has 127.0.0.1 (localhost still proxied) (CodeRabbit #3801)", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback(
{ http_proxy: "http://127.0.0.1:8118", NO_PROXY: "127.0.0.1" },
(line) => lines.push(line),
);
expect(fired).toBe(true);
expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1");
});

it("warns when HTTP_PROXY is set without NO_PROXY=localhost", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback(
{ http_proxy: "http://127.0.0.1:8118" },
(line) => lines.push(line),
);
expect(fired).toBe(true);
expect(lines.join("\n")).toContain("HTTP_PROXY/http_proxy is set");
expect(lines.join("\n")).toContain("Detected proxy: http://127.0.0.1:8118");
expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1");
});

it("redacts credentials in the proxy URL it logs (CodeRabbit #3801)", () => {
const lines: string[] = [];
warnIfHostProxyMissesLoopback(
{ http_proxy: "http://alice:s3cret@proxy.example.com:3128" },
(line) => lines.push(line),
);
const joined = lines.join("\n");
expect(joined).not.toContain("alice");
expect(joined).not.toContain("s3cret");
expect(joined).toContain("****");
expect(joined).toContain("proxy.example.com:3128");
});

it("respects uppercase HTTP_PROXY too", () => {
const lines: string[] = [];
const fired = warnIfHostProxyMissesLoopback(
{ HTTP_PROXY: "http://corp-proxy:3128" },
(line) => lines.push(line),
);
expect(fired).toBe(true);
expect(lines.join("\n")).toContain("corp-proxy:3128");
});
});
56 changes: 56 additions & 0 deletions src/lib/onboard/http-proxy-preflight.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* Preflight warning when the user's shell has HTTP_PROXY set without a
* NO_PROXY=localhost,127.0.0.1 bypass. See #2616.
*
* NemoClaw's own subprocess spawn helpers (`buildSubprocessEnv`) inject
* NO_PROXY for loopback hosts, so NemoClaw-managed processes are safe. But
* any tool the user runs that respects HTTP_PROXY (curl, Node fetch,
* Python requests) inherits the user's environment and will still tunnel
* localhost traffic through the host proxy — common on macOS with Privoxy
* at 127.0.0.1:8118.
*/
export function warnIfHostProxyMissesLoopback(
env: NodeJS.ProcessEnv = process.env,
warn: (line: string) => void = (line) => console.warn(line),
): boolean {
const proxyEnv = env.HTTP_PROXY || env.http_proxy;
if (!proxyEnv) return false;
const noProxyEnv = env.NO_PROXY || env.no_proxy || "";
// Require BOTH entries — HTTP libraries match the literal hostname against
// NO_PROXY, so `NO_PROXY=localhost` alone still proxies `127.0.0.1` requests
// (and vice versa). Only suppress the warning when both are present.
const hasLocalhost = /(^|,)\s*localhost\s*(,|$)/.test(noProxyEnv);
const hasLoopback = /(^|,)\s*127\.0\.0\.1\s*(,|$)/.test(noProxyEnv);
if (hasLocalhost && hasLoopback) return false;
warn(" ⚠ HTTP_PROXY/http_proxy is set without NO_PROXY=localhost,127.0.0.1.");
warn(` Detected proxy: ${redactProxyCredentials(proxyEnv)}`);
warn(" NemoClaw injects NO_PROXY for its own subprocess spawns, but any tool you run");
warn(" that respects HTTP_PROXY (curl, Node fetch, Python requests) will still tunnel");
warn(" localhost traffic through your host proxy. To bypass loopback (see #2616):");
warn(" export NO_PROXY=localhost,127.0.0.1");
warn(" export no_proxy=localhost,127.0.0.1");
return true;
}

/**
* Redact basic-auth credentials from a proxy URL before logging. HTTP_PROXY
* vars sometimes carry `http://user:password@proxy:3128`; logging that raw
* leaks secrets into terminal scrollback, screenshots, and support tickets.
*/
export function redactProxyCredentials(raw: string): string {
try {
const u = new URL(raw);
if (u.username || u.password) {
u.username = "****";
u.password = "";
return u.toString();
}
return raw;
} catch {
// Not a parseable URL — fall back to regex over the userinfo segment.
return raw.replace(/(\/\/)[^/@]+@/, "$1****@");
}
}
6 changes: 5 additions & 1 deletion src/lib/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,11 @@ function runCaptureEx(cmd: readonly string[], opts: Omit<CaptureOptions, "ignore
const result = spawnSync(exe, args, {
...spawnOpts,
cwd: ROOT,
env: { ...process.env, ...extraEnv },
// #2616: route via buildRunnerEnv so subprocess env is sanitized and
// NO_PROXY=localhost,127.0.0.1 is injected when HTTP_PROXY is set.
// Otherwise curl probes against localhost (Ollama validation, etc.)
// tunnel through the user's host proxy and fail with HTTP 500.
env: buildRunnerEnv(extraEnv),
stdio: ["pipe", "pipe", "pipe"],
encoding: "utf-8",
});
Expand Down
50 changes: 50 additions & 0 deletions test/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,56 @@ describe("runner env merging", () => {
);
expect(firstCall[2]?.env?.PATH).toBe("/usr/local/bin:/usr/bin");
});

it("#2616: runCaptureEx injects NO_PROXY=localhost,127.0.0.1 when http_proxy is set", () => {
// Regression for the macOS Privoxy scenario: validateOllamaModel calls
// runCaptureEx with a curl probe against http://localhost:11434. Before
// the fix, runCaptureEx merged raw process.env (including the user's
// http_proxy) and never injected NO_PROXY, so the spawned curl tunneled
// its localhost probe through Privoxy and returned HTTP 500.
const calls: SpawnCall[] = [];
const originalSpawnSync = childProcess.spawnSync;
const originalHttpProxy = process.env.http_proxy;
const originalNoProxy = process.env.NO_PROXY;
const originalNoProxyLower = process.env.no_proxy;
// @ts-expect-error — intentional partial mock for testing
childProcess.spawnSync = captureSpawnCall(calls, { status: 0, stdout: "", stderr: "" });

try {
delete require.cache[require.resolve(runnerPath)];
const { runCaptureEx } = require(runnerPath);
process.env.http_proxy = "http://127.0.0.1:8118";
delete process.env.NO_PROXY;
delete process.env.no_proxy;
runCaptureEx([
"curl",
"-sS",
"--max-time",
"3",
"http://localhost:11434/api/ps",
]);
} finally {
if (originalHttpProxy === undefined) delete process.env.http_proxy;
else process.env.http_proxy = originalHttpProxy;
if (originalNoProxy === undefined) delete process.env.NO_PROXY;
else process.env.NO_PROXY = originalNoProxy;
if (originalNoProxyLower === undefined) delete process.env.no_proxy;
else process.env.no_proxy = originalNoProxyLower;
childProcess.spawnSync = originalSpawnSync;
delete require.cache[require.resolve(runnerPath)];
}

expect(calls).toHaveLength(1);
const firstCall = requireCall(calls, 0);
const env = firstCall[2]?.env ?? {};
expect(env.http_proxy).toBe("http://127.0.0.1:8118");
// Both casings get the loopback hosts so curl, Node, Python all respect
// the bypass regardless of which one they read.
expect(env.NO_PROXY).toContain("localhost");
expect(env.NO_PROXY).toContain("127.0.0.1");
expect(env.no_proxy).toContain("localhost");
expect(env.no_proxy).toContain("127.0.0.1");
});
});

describe("shellQuote", () => {
Expand Down
Loading