diff --git a/src/config.ts b/src/config.ts index c24bcb4f0e..7f373e57c5 100644 --- a/src/config.ts +++ b/src/config.ts @@ -3126,6 +3126,26 @@ export function resolveEnvValue(value: string | undefined): string | undefined { return value; } +const warnedProxyConfigDiscards = new Set<"proxy" | "noProxy" | "noProxyElements">(); + +function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements"): void { + if (warnedProxyConfigDiscards.has(kind)) return; + warnedProxyConfigDiscards.add(kind); + if (kind === "proxy") { + console.warn( + "⚠️ config.json proxy was discarded because it is not a non-empty resolved string — configured proxy routing is disabled; existing proxy environment variables remain authoritative, otherwise outbound requests use direct egress", + ); + } else if (kind === "noProxy") { + console.warn( + "⚠️ config.json noProxy was discarded because it is not a string, string array, or resolved environment reference — existing NO_PROXY and loopback bypasses remain", + ); + } else { + console.warn( + "⚠️ config.json noProxy contains invalid elements — invalid elements were ignored; valid entries, existing NO_PROXY, and loopback bypasses remain", + ); + } +} + /** * Mirror `config.proxy` into HTTP(S)_PROXY env vars so Bun's native fetch routes every outbound * provider call through the proxy — no per-callsite changes (verified: Bun honors these plus @@ -3134,8 +3154,18 @@ export function resolveEnvValue(value: string | undefined): string | undefined { * that makes outbound provider requests (server start, catalog sync). */ export function applyProxyEnv(config: OcxConfig): void { - const proxy = resolveEnvValue(config.proxy); - if (!proxy) return; + // `proxy` and `noProxy` are not declared in the top-level schema, which ends in + // `.passthrough()`, so whatever is on disk arrives here verbatim. A non-string value + // reached string-only methods and threw out of this function, and it runs once per + // process entry point — the failure was a startup crash, not a degraded proxy. Ignore + // malformed values with a privacy-safe warning instead: they cannot express a routing + // intent, and refusing to start is a worse answer than starting without them. + const rawProxy = config.proxy; + const proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; + if (!proxy) { + if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); + return; + } if (!process.env.HTTP_PROXY?.trim() && !process.env.http_proxy?.trim()) process.env.HTTP_PROXY = proxy; if (!process.env.HTTPS_PROXY?.trim() && !process.env.https_proxy?.trim()) process.env.HTTPS_PROXY = proxy; const existing = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; @@ -3144,7 +3174,20 @@ export function applyProxyEnv(config: OcxConfig): void { // Configured entries first, then loopback: loopback is unconditional, so appending it last // keeps it present even when the operator lists a loopback host themselves. const raw = config.noProxy; - const configured = (Array.isArray(raw) ? raw : (resolveEnvValue(raw) ?? "").split(",")) + let configuredEntries: string[]; + if (Array.isArray(raw)) { + // One unusable element must not discard the operator's other entries. + if (raw.some(entry => typeof entry !== "string")) warnProxyConfigDiscardOnce("noProxyElements"); + configuredEntries = raw.filter((entry): entry is string => typeof entry === "string"); + } else if (typeof raw === "string") { + const resolved = resolveEnvValue(raw); + if (raw && resolved === undefined) warnProxyConfigDiscardOnce("noProxy"); + configuredEntries = (resolved ?? "").split(","); + } else { + if (raw !== undefined) warnProxyConfigDiscardOnce("noProxy"); + configuredEntries = []; + } + const configured = configuredEntries .map(entry => entry.trim()) .filter(Boolean); for (const host of [...configured, "localhost", "127.0.0.1", "::1", "[::1]"]) { diff --git a/tests/proxy-env.test.ts b/tests/proxy-env.test.ts index f58ba4055d..bf7ccaa467 100644 --- a/tests/proxy-env.test.ts +++ b/tests/proxy-env.test.ts @@ -24,6 +24,71 @@ function configWithProxy(proxy?: string, noProxy?: string | string[]): OcxConfig return { proxy, noProxy, providers: {} } as unknown as OcxConfig; } +// The top-level config schema ends in `.passthrough()` and declares neither `proxy` nor +// `noProxy`, so these shapes survive validation and reach applyProxyEnv verbatim. +function configWithRawProxy(proxy: unknown, noProxy?: unknown): OcxConfig { + return { proxy, noProxy, providers: {} } as unknown as OcxConfig; +} + +describe("applyProxyEnv with values the schema does not constrain", () => { + test("warns once per discarded proxy setting without exposing its raw value", () => { + const secret = "raw-proxy-credential-sentinel-2947"; + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const invalidProxy = { secret }; + applyProxyEnv(configWithRawProxy(invalidProxy)); + applyProxyEnv(configWithRawProxy(invalidProxy)); + expect(process.env.HTTP_PROXY).toBeUndefined(); + expect(process.env.HTTPS_PROXY).toBeUndefined(); + + const invalidNoProxy = { secret }; + applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", invalidNoProxy)); + applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", invalidNoProxy)); + expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1]"); + + const invalidElement = { secret }; + applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", ["internal.example", invalidElement])); + applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", ["internal.example", invalidElement])); + expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1],internal.example"); + } finally { + console.warn = originalWarn; + } + + expect(warnings).toHaveLength(3); + expect(warnings[0]).toContain("config.json proxy was discarded"); + expect(warnings[0]).toContain("direct egress"); + expect(warnings[1]).toContain("config.json noProxy was discarded"); + expect(warnings[1]).toContain("existing NO_PROXY and loopback bypasses remain"); + expect(warnings[2]).toContain("config.json noProxy contains invalid elements"); + expect(warnings[2]).toContain("invalid elements were ignored"); + expect(warnings.join("\n")).not.toContain(secret); + }); + + // applyProxyEnv runs at every process entry point that makes outbound requests, so a + // throw here is a startup crash rather than a degraded proxy. + test("a non-string proxy does not throw and sets no proxy env", () => { + expect(() => applyProxyEnv(configWithRawProxy(42))).not.toThrow(); + expect(process.env.HTTP_PROXY).toBeUndefined(); + expect(process.env.HTTPS_PROXY).toBeUndefined(); + }); + + test("a non-string noProxy does not throw and keeps loopback exclusions", () => { + expect(() => applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", 42))).not.toThrow(); + expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1]"); + }); + + test.each([ + ["a number", 42], + ["null", null], + ["an object", { a: 1 }], + ])("keeps the operator's usable noProxy entries when the array also holds %s", (_label, bad) => { + expect(() => applyProxyEnv(configWithRawProxy("http://proxy.corp:8080", ["internal.example", bad]))).not.toThrow(); + expect(process.env.NO_PROXY).toBe("internal.example,localhost,127.0.0.1,::1,[::1]"); + }); +}); + describe("applyProxyEnv", () => { test("no-op when config.proxy is unset", () => { process.env.NO_PROXY = "operator-owned.example";