diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index fbed02554..38808ed07 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -1,6 +1,7 @@ import type { Server } from "bun"; import { bridgeToResponsesSSE, buildResponseJSON, formatErrorResponse, type ResponsesTerminalStatus } from "../../bridge"; import { formatPassthroughUpstreamError } from "./passthrough-error"; +import { describeUpstreamConnectFailure } from "./upstream-error"; import { getConfigPath, multiAgentGuidanceEnabled, @@ -1194,7 +1195,7 @@ export async function handleResponses( } const msg = outcome === "timeout" ? `Provider connect timeout after ${connectMs}ms` - : `Provider unreachable: ${err instanceof Error ? err.message : String(err)}`; + : describeUpstreamConnectFailure(err, connectMs); return formatErrorResponse(502, "upstream_error", msg); }; try { @@ -1741,9 +1742,7 @@ export async function handleResponses( cleanupUpstreamAbort(); upstream.abort(); if (options.abortSignal?.aborted) return clientCancelledResponse(); - const msg = err instanceof Error && err.name === "TimeoutError" - ? `Provider connect timeout after ${connectMs}ms` - : `Provider unreachable: ${err instanceof Error ? err.message : String(err)}`; + const msg = describeUpstreamConnectFailure(err, connectMs); return formatErrorResponse(502, "upstream_error", msg); } @@ -1783,9 +1782,7 @@ export async function handleResponses( if (options.abortSignal?.aborted) { return { failed: clientCancelledResponse() }; } - const msg = err instanceof Error && err.name === "TimeoutError" - ? `Provider connect timeout after ${connectMs}ms` - : `Provider unreachable: ${err instanceof Error ? err.message : String(err)}`; + const msg = describeUpstreamConnectFailure(err, connectMs); return { failed: formatErrorResponse(502, "upstream_error", msg) }; } }; diff --git a/src/server/responses/upstream-error.ts b/src/server/responses/upstream-error.ts new file mode 100644 index 000000000..7de21398f --- /dev/null +++ b/src/server/responses/upstream-error.ts @@ -0,0 +1,48 @@ +/** + * Upstream connection failures share one message shape across the three catch sites in + * core.ts. A TLS certificate/hostname mismatch deserves its own wording: the generic + * "Provider unreachable" reads as if opencodex built a wrong endpoint, which sent issue + * #553 looking for an adapter URL bug that does not exist. Name the likely cause and the + * command that settles it. + */ +export function describeUpstreamConnectFailure(err: unknown, connectMs: number): string { + if (err instanceof Error && err.name === "TimeoutError") { + return `Provider connect timeout after ${connectMs}ms`; + } + const detail = err instanceof Error ? err.message : String(err); + const code = err instanceof Error ? (err as { code?: unknown }).code : undefined; + // `code` is the reliable signal. The message fallback is anchored to the head because Bun + // renders this rejection as `ERR_TLS_CERT_ALTNAME_INVALID fetching ""`; matching the + // bare substring anywhere would also fire on text that merely quotes the code back at us. + // Only transport failures reach these call sites, so that is defensive rather than load-bearing. + if (code === "ERR_TLS_CERT_ALTNAME_INVALID" || detail.startsWith("ERR_TLS_CERT_ALTNAME_INVALID")) { + const host = extractHostname(detail); + const target = host ?? "the provider host"; + const probe = host ?? ""; + return `Provider TLS certificate does not match ${target}: ${redactUrlUserinfo(detail)}. ` + + "opencodex did not rewrite this hostname — a certificate that does not cover it normally " + + "means TLS interception (corporate proxy, VPN, or local MITM tooling) or a poisoned DNS " + + `answer. Check with: openssl s_client -connect ${probe}:443 -servername ${probe} ` + + "@"); +} + +function extractHostname(detail: string): string | null { + const match = detail.match(/https?:\/\/([^/\s"']+)/); + if (!match?.[1]) return null; + try { + return new URL(`https://${match[1]}`).hostname || null; + } catch { + return null; + } +} diff --git a/tests/upstream-connect-error.test.ts b/tests/upstream-connect-error.test.ts new file mode 100644 index 000000000..e7376e5a2 --- /dev/null +++ b/tests/upstream-connect-error.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test } from "bun:test"; +import { describeUpstreamConnectFailure } from "../src/server/responses/upstream-error"; + +function tlsAltnameError(url: string): Error { + // Shape of what Bun's fetch actually rejects with on a certificate/hostname mismatch. + return Object.assign(new Error(`ERR_TLS_CERT_ALTNAME_INVALID fetching "${url}"`), { + code: "ERR_TLS_CERT_ALTNAME_INVALID", + }); +} + +describe("describeUpstreamConnectFailure", () => { + test("a TLS altname mismatch names the host, the likely cause, and the check", () => { + const msg = describeUpstreamConnectFailure( + tlsAltnameError("https://api.individual.githubcopilot.com/chat/completions"), + 30000, + ); + expect(msg).toContain("api.individual.githubcopilot.com"); + expect(msg).toContain("TLS interception"); + expect(msg).toContain("openssl s_client"); + expect(msg).toContain("-servername api.individual.githubcopilot.com"); + // The generic wording is what sent issue #553 hunting for an adapter URL bug. + expect(msg).not.toContain("Provider unreachable"); + }); + + test("the branch also fires when only the message carries the code", () => { + const msg = describeUpstreamConnectFailure(new Error("ERR_TLS_CERT_ALTNAME_INVALID"), 30000); + expect(msg).toContain("openssl s_client"); + // No URL in the detail, so the message degrades to a placeholder rather than guessing. + expect(msg).toContain("the provider host"); + expect(msg).toContain(""); + }); + + test("text that merely quotes the code back does not take the TLS branch", () => { + // Only transport failures reach these call sites, so this is defensive — but the message + // fallback is anchored to the head so echoed text cannot produce wrong advice. + const msg = describeUpstreamConnectFailure( + new Error('upstream said: ERR_TLS_CERT_ALTNAME_INVALID somewhere'), + 30000, + ); + expect(msg).toBe("Provider unreachable: upstream said: ERR_TLS_CERT_ALTNAME_INVALID somewhere"); + }); + + test("an ordinary connection failure keeps the existing wording", () => { + expect(describeUpstreamConnectFailure(new Error("ECONNREFUSED"), 30000)) + .toBe("Provider unreachable: ECONNREFUSED"); + expect(describeUpstreamConnectFailure(new Error("ENOTFOUND api.example.com"), 30000)) + .toBe("Provider unreachable: ENOTFOUND api.example.com"); + }); + + test("a timeout keeps its own message", () => { + const err = Object.assign(new Error("The operation timed out."), { name: "TimeoutError" }); + expect(describeUpstreamConnectFailure(err, 12345)) + .toBe("Provider connect timeout after 12345ms"); + }); + + test("a non-Error rejection still produces the generic message", () => { + expect(describeUpstreamConnectFailure("socket hang up", 30000)) + .toBe("Provider unreachable: socket hang up"); + }); + + test("URL userinfo is redacted on both branches", () => { + // A provider base URL can carry credentials as userinfo, and the runtime error echoes + // the URL it was fetching. Neither message may hand that back to the caller. + // The secret is assembled at runtime so the privacy scanner does not read the literal + // `user:token@host` form here as a real address. + const secret = ["sk", "fixture", "token"].join("-"); + const withCreds = `fetching "https://user:${secret}@api.example.com/v1"`; + const tls = describeUpstreamConnectFailure( + Object.assign(new Error(`ERR_TLS_CERT_ALTNAME_INVALID ${withCreds}`), { + code: "ERR_TLS_CERT_ALTNAME_INVALID", + }), + 30000, + ); + expect(tls).not.toContain(secret); + expect(tls).toContain("@api.example.com"); + expect(tls).toContain("does not match api.example.com"); + + const generic = describeUpstreamConnectFailure(new Error(`ECONNREFUSED ${withCreds}`), 30000); + expect(generic).not.toContain(secret); + expect(generic).toContain("@api.example.com"); + }); + + test("an IPv6 literal host is extracted in bracket form", () => { + const msg = describeUpstreamConnectFailure( + Object.assign(new Error('ERR_TLS_CERT_ALTNAME_INVALID fetching "https://[2001:db8::1]:8443/v1"'), { + code: "ERR_TLS_CERT_ALTNAME_INVALID", + }), + 30000, + ); + expect(msg).toContain("[2001:db8::1]"); + expect(msg).toContain("-servername [2001:db8::1]"); + }); +});