From 02f7cc6bbe6a424b426a328bd886677f7e7848e9 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:55:16 +0900 Subject: [PATCH 1/2] fix(local): fail closed DNS bind names for credential-bearing destinations probeHostname returns a DNS bind name unchanged, and the credential-bearing destination composers (localInferenceDestination, localManagementOrigin, resolveApiAccessBaseUrl) then embed that name in a URL the client dials with credentials attached. A hostname that re-resolves at dial time is a DNS-rebinding exfiltration path: the credential leaves for whatever the name resolves to then, not what it resolved to at compose time. localCredentialDestinationHostname keeps literal IPs as-is and fails closed to 127.0.0.1 for DNS names, so credential-bearing destinations only ever target a literal address. Display-only hosts keep the resolved name. Tests pin the fail-closed behavior for inference, management-origin, and API-access base URLs, plus the literal non-loopback IP path. --- src/lib/local-destinations.ts | 28 ++++++++++++++++---- src/server/management/api-access.ts | 7 +++-- tests/lib/local-destinations.test.ts | 31 +++++++++++++++++++++++ tests/server/api-access-endpoints.test.ts | 9 +++++++ 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/lib/local-destinations.ts b/src/lib/local-destinations.ts index d4699754d18..330eaa42206 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,22 @@ 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. + */ +export function localCredentialDestinationHostname(hostname: string | undefined): string { + const probed = probeHostname(hostname); + 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 +103,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 +164,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 +176,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..8bd2f770dbc 100644 --- a/tests/lib/local-destinations.test.ts +++ b/tests/lib/local-destinations.test.ts @@ -159,6 +159,29 @@ 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"); + } + }); }); describe("localLoopbackInferencePorts", () => { @@ -277,4 +300,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); }); From e6635e2c33229ab77d3e186631132d251a77fa29 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 18 Sep 2026 18:47:43 +0900 Subject: [PATCH 2/2] fix(local): keep localhost out of the DNS fail-closed branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fail-closed branch treated every non-literal bind name as a DNS bind, including `localhost`, and rewrote it to 127.0.0.1. That failed `ocx claude management discovery destination > a loopback or wildcard install keeps asking 127.0.0.1 on the public port`, which pins that a `localhost` install keeps writing `http://localhost:` into its exported client configuration. The rewrite also bought nothing. RFC 6761 reserves `localhost` to loopback, so a second lookup cannot select a peer off this machine — the only outcome the fail-closed branch exists to prevent. `isLoopbackHostname` is already the encoding of "this name is loopback" in this module, so the carve-out reuses it rather than growing a second list. Also rebased onto current `dev`. Co-authored-by: luvs01 --- src/lib/local-destinations.ts | 9 +++++++++ tests/lib/local-destinations.test.ts | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/lib/local-destinations.ts b/src/lib/local-destinations.ts index 330eaa42206..4dfb1a0469e 100644 --- a/src/lib/local-destinations.ts +++ b/src/lib/local-destinations.ts @@ -68,9 +68,18 @@ export interface LocalInferenceDestination { * 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; diff --git a/tests/lib/local-destinations.test.ts b/tests/lib/local-destinations.test.ts index 8bd2f770dbc..66d853892b3 100644 --- a/tests/lib/local-destinations.test.ts +++ b/tests/lib/local-destinations.test.ts @@ -182,6 +182,23 @@ describe("localInferenceDestination", () => { 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", () => {