diff --git a/src/lib/local-destinations.ts b/src/lib/local-destinations.ts index d4699754d18..4dfb1a0469e 100644 --- a/src/lib/local-destinations.ts +++ b/src/lib/local-destinations.ts @@ -21,7 +21,8 @@ * loopback listener enabled → `127.0.0.1:`, no credential * loopback/absent `hostname` → `127.0.0.1:`, no credential * wildcard `hostname` → `127.0.0.1:`, ADMISSION CREDENTIAL REQUIRED - * anything else → `:`, credential REQUIRED + * literal non-loopback IP → `:`, credential REQUIRED + * DNS bind name → `127.0.0.1:`, credential REQUIRED (fail closed) * * A wildcard bind does answer on 127.0.0.1, which is why its origin stays loopback, but the * public listener demands data-plane admission regardless of which address received the @@ -36,6 +37,7 @@ * token: no exported client configuration may carry management authority (reviewer constraint * on #4236). */ +import { isIP } from "node:net"; import { effectiveLoopbackListenerPort, isLoopbackHostname, isWildcardHostname, shouldInjectApiAuthHeader } from "../codex/loopback-target"; import { probeHostname } from "../server/proxy-liveness"; import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "./service-secrets"; @@ -59,6 +61,31 @@ export interface LocalInferenceDestination { requiresAdmissionToken: boolean; } +/** + * Turn a bind setting into an address that is safe to combine with local credentials. + * + * Bun resolves a DNS bind name when the listener starts, but a later client lookup can receive + * a different answer. Local integrations therefore use only literal addresses; a DNS bind falls + * back to loopback and fails closed when the listener does not answer there. Operators who need + * local integrations on a DNS-bound listener can enable the dedicated loopback listener. + * + * `localhost` is deliberately NOT treated as a DNS bind here, even though it is a name. It is + * reserved to loopback by RFC 6761, so a second lookup cannot select a peer off this machine — + * the thing this function exists to prevent. Rewriting it to `127.0.0.1` would buy no safety and + * would change what every existing loopback install writes into its exported client + * configuration, which is a contract `tests/claude-integration/claude-cli.test.ts` pins. + * `isLoopbackHostname` is the existing encoding of "this name is loopback", so the two stay in + * agreement by construction rather than by a second list. + */ +export function localCredentialDestinationHostname(hostname: string | undefined): string { + const probed = probeHostname(hostname); + if (isLoopbackHostname(probed)) return probed; + const literal = probed.startsWith("[") && probed.endsWith("]") + ? probed.slice(1, -1) + : probed; + return isIP(literal) !== 0 ? probed : "127.0.0.1"; +} + /** * The one answer for "where does a local client send inference, and does it need a key?". * @@ -85,7 +112,7 @@ export function localInferenceDestination( // literal; `shouldInjectApiAuthHeader` is the existing encoding of "this bind demands a // data-plane credential", so the two stay in agreement by construction. return { - origin: `http://${probeHostname(hostname)}:${publicPort}`, + origin: `http://${localCredentialDestinationHostname(hostname)}:${publicPort}`, port: publicPort, requiresAdmissionToken: shouldInjectApiAuthHeader(config), }; @@ -146,8 +173,8 @@ const ADMISSION_TOKEN_SHAPE = /^[A-Za-z0-9._~+/=-]{8,4096}$/; * * A hub's management ingress is loopback-only and exists precisely so the operator's own * machine has a management address when the proxy listener is bound elsewhere. Everything else - * keeps dialing the public listener on the bind address it can actually reach — `probeHostname` - * turns a wildcard bind into 127.0.0.1 and brackets a bare IPv6 literal. + * keeps dialing a literal public bind address it can actually reach. DNS bind names fail closed + * to loopback because resolving them again could select a different peer after startup. * * The caller still supplies the management credential. Never write that credential into an * exported client configuration. @@ -158,5 +185,5 @@ export function localManagementOrigin( ): string { const ingress = config?.runtimeRole === "hub" ? config.hub?.managementIngress : undefined; if (ingress?.enabled) return `http://127.0.0.1:${ingress.port}`; - return `http://${probeHostname(config?.hostname)}:${publicPort}`; + return `http://${localCredentialDestinationHostname(config?.hostname)}:${publicPort}`; } diff --git a/src/server/management/api-access.ts b/src/server/management/api-access.ts index 0421824e5a6..93f5d4b952a 100644 --- a/src/server/management/api-access.ts +++ b/src/server/management/api-access.ts @@ -1,7 +1,6 @@ import type { OcxConfig } from "../../types"; import { isWildcardHostname } from "../../codex/loopback-target"; -import { localInferenceDestination } from "../../lib/local-destinations"; -import { probeHostname } from "../proxy-liveness"; +import { localCredentialDestinationHostname, localInferenceDestination } from "../../lib/local-destinations"; import { isCanonicalOpenAiForwardProvider, OPENAI_API_PROVIDER_ID, OPENAI_CODEX_PROVIDER_ID } from "../../providers/openai-tiers-destination"; import { LIVE_AUDIO_MODEL, TRANSCRIPTION_MODEL } from "../audio-upstream"; @@ -95,7 +94,7 @@ export function resolveApiAccessBaseUrl( const port = config.port ?? 10100; if (!isWildcardBindHost(config.hostname)) { - return `http://${probeHostname(config.hostname)}:${port}/v1`; + return `http://${localCredentialDestinationHostname(config.hostname)}:${port}/v1`; } const fromOrigin = opts.requestOrigin ? originBaseUrl(opts.requestOrigin) : null; @@ -140,7 +139,7 @@ export function resolveApiAccessDisplayHost( opts: BuildApiAccessEndpointsOptions = {}, ): string { if (!isWildcardBindHost(configHostname)) { - return probeHostname(configHostname); + return localCredentialDestinationHostname(configHostname); } try { return new URL(resolveApiAccessBaseUrl({ hostname: configHostname, port: 10100 }, opts)).hostname diff --git a/tests/lib/local-destinations.test.ts b/tests/lib/local-destinations.test.ts index 52b003b052a..66d853892b3 100644 --- a/tests/lib/local-destinations.test.ts +++ b/tests/lib/local-destinations.test.ts @@ -159,6 +159,46 @@ describe("localInferenceDestination", () => { .toEqual({ hostname, origin: "http://127.0.0.1:10100", requires: true }); } }); + + test("a DNS bind name fails closed to loopback for credential-bearing destinations", () => { + // Bun resolves the bind name once at listen time; a later client lookup can get a different + // answer, so a credential-bearing local destination must never re-resolve it. The data-plane + // credential stays required — the destination degrades to a socket that refuses rather than + // one that leaks the token to a rebound peer. + const destination = localInferenceDestination({ hostname: "mutable-bind.example" }, PUBLIC_PORT); + expect(destination).toEqual({ + origin: "http://127.0.0.1:10100", + port: PUBLIC_PORT, + requiresAdmissionToken: true, + }); + }); + + test("literal non-loopback IPs still compose a credential-bearing bind destination", () => { + // The fail-closed branch is only for names: a literal tailnet or LAN bind keeps its exact + // address, because a literal cannot be re-resolved to a different peer after startup. + for (const hostname of [TAILNET, "192.168.7.7", "fd7a:115c:a1e0::1", "[fd7a:115c:a1e0::1]"]) { + const destination = localInferenceDestination({ hostname }, PUBLIC_PORT); + expect(destination.requiresAdmissionToken).toBe(true); + expect(destination.origin).not.toBe("http://127.0.0.1:10100"); + } + }); + + test("localhost is a name but not a DNS bind, and keeps the address it already had", () => { + // RFC 6761 reserves `localhost` to loopback, so a second lookup cannot select a peer off + // this machine — which is the only thing the fail-closed branch above exists to prevent. + // Rewriting it to 127.0.0.1 would buy no safety and would silently change the origin every + // existing loopback install writes into its exported client configuration; the first draft + // of this change did exactly that and failed + // `ocx claude management discovery destination > a loopback or wildcard install keeps + // asking 127.0.0.1 on the public port`. + // + // Only the management origin can observe this: `localInferenceDestination` answers a + // loopback bind from its own earlier branch and never reaches the name check at all. + expect(localManagementOrigin(hub({ runtimeRole: "standalone", hostname: "localhost" }), PUBLIC_PORT)) + .toBe("http://localhost:10100"); + expect(localInferenceDestination({ hostname: "localhost" }, PUBLIC_PORT).origin) + .toBe("http://127.0.0.1:10100"); + }); }); describe("localLoopbackInferencePorts", () => { @@ -277,4 +317,12 @@ describe("localManagementOrigin", () => { .toEqual({ hostname, origin: expected }); } }); + + test("a DNS bind name fails closed to loopback rather than re-resolving for the credential", () => { + // The management origin carries the same credential boundary as inference: a name the + // client would re-resolve can point at a different peer after startup, so only literal + // bind addresses may compose a credential-bearing destination. + const config = hub({ runtimeRole: "standalone", hostname: "mutable-bind.example" }); + expect(localManagementOrigin(config, PUBLIC_PORT)).toBe("http://127.0.0.1:10100"); + }); }); diff --git a/tests/server/api-access-endpoints.test.ts b/tests/server/api-access-endpoints.test.ts index ce2a55fdbf7..fef90701359 100644 --- a/tests/server/api-access-endpoints.test.ts +++ b/tests/server/api-access-endpoints.test.ts @@ -103,6 +103,15 @@ describe("buildApiAccessEndpoints", () => { })).toBe("http://100.76.170.81:10100/v1"); }); + test("a DNS bind name fails closed to loopback for the credential-bearing base URL", () => { + // A literal bind keeps its exact address, but a name can be re-resolved to a different + // peer after startup; the generated API base URL must not send credentials through it. + expect(resolveApiAccessBaseUrl({ + hostname: "mutable-bind.example", + port: 10100, + })).toBe("http://127.0.0.1:10100/v1"); + }); + test("reflects disabled Claude inbound in API access metadata", () => { expect(buildApiAccessEndpoints({ claudeCode: { enabled: false } }).claudeCodeEnabled).toBe(false); });