From 195158b58cdbcb709597e53e51eb917a328bcd6f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:44:41 +0900 Subject: [PATCH 1/8] feat(server): admit the hub's own client wires on the loopback listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unauthenticated loopback listener served only the Codex data plane, so on a hub whose public listener binds a tailnet address the machine's other local clients had no socket at all: `ocx claude`, Claude Desktop and the system-env injection speak `POST /v1/messages`, while Cursor Private Inference, the routed vision helper and aside/opencode speak `POST /v1/chat/completions`. Both 404'd there, which is the other half of the "Codex works but nothing else does" report in #4236. Admit exactly those two POST routes. Both handlers already resolve admission from the receiving listener's policy view — the same resolver and the same loopback short-circuit `/v1/responses` uses — so this adds a wire, not a trust level. `/api/*`, `/healthz`, `/readyz`, the GUI and `count_tokens` still 404: local management discovery belongs to the authenticated surface, and the reviewer condition on #4236 was explicit about not widening this listener to `/api/*`. The chat-completions branch also finished its CORS with `config` rather than the request's `policy`. On the public listener those are the same object, so that is a no-op there; on the loopback listener it is the difference between CORS headers that match the admission decision above them and headers derived from a bind address that did not receive the request. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- src/server/index.ts | 17 ++++- structure/01_runtime.md | 6 +- .../loopback-listener-admission.test.ts | 35 +++++++++ .../loopback-listener-integration.test.ts | 73 ++++++++++++++++--- 4 files changed, 118 insertions(+), 13 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 106f34069a..6301f90d07 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -822,6 +822,17 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors( await handleChatCompletions(req, config, logCtx, { requestId, start, turnAdmissionLease, admission }), req, - config, + policy, )); } diff --git a/structure/01_runtime.md b/structure/01_runtime.md index 7e529ea503..a460da4541 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -58,7 +58,11 @@ uninstall still restore. `startServer` composes up to three sockets in one synchronous startup transaction: the public data listener, the optional unauthenticated data-loopback listener, and the optional hub-management -listener. The hub-management socket is enabled only by `runtimeRole: "hub"` plus +listener. The data-loopback socket serves a fixed data-plane allowlist: Responses and its compact +sibling, the native search relay, the standalone Images POSTs, `GET /v1/models`, the realtime voice +shapes, and `POST /v1/messages` plus `POST /v1/chat/completions` — the two inference wires the +host's own local clients speak. It never serves `/api/*`, `/healthz`, `/readyz`, or GUI routes, so +local management discovery has to use an authenticated surface with a management credential. The hub-management socket is enabled only by `runtimeRole: "hub"` plus `hub.managementIngress.enabled`, always binds `127.0.0.1`, and default-denies everything except GUI, session bootstrap/exchange, and `/api/*`. A failed optional bind initiates rollback of every earlier socket; normal stop joins all bound sockets before lifecycle release. The existing launchd/systemd diff --git a/tests/server/loopback-listener-admission.test.ts b/tests/server/loopback-listener-admission.test.ts index d6a88e7408..93fcb79b14 100644 --- a/tests/server/loopback-listener-admission.test.ts +++ b/tests/server/loopback-listener-admission.test.ts @@ -84,6 +84,41 @@ describe("loopback listener policy view", () => { }); }); +describe("local client inference wires on the loopback listener (#4236)", () => { + const source = readFileSync(new URL("../../src/server/index.ts", import.meta.url), "utf8"); + + test("the allowlist admits both wires as POST and nothing else about them", () => { + // The allowlist is a closure inside startServer, so this reads the entry itself. The + // integration file proves the socket behaviour; this pins the SHAPE, because "admit the + // path" and "admit the path for any method" are one character apart. + expect(source).toContain( + 'if (path === "/v1/messages" || path === "/v1/chat/completions") return req.method === "POST";', + ); + }); + + test("no /api route joins the allowlist", () => { + // Management discovery is the other destination contract (authenticated ingress). A + // reviewer condition on #4236: `/api/*` must never appear on this listener. + const start = source.indexOf("function loopbackRouteAllowed("); + const end = source.indexOf("function managementIngressRouteAllowed(", start); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + expect(source.slice(start, end)).not.toContain('"/api'); + }); + + test("the chat wire finishes CORS with the receiving listener's policy", () => { + // It is now served on the loopback listener, so the public config must not be the source + // of its CORS headers — the same rule the Anthropic routes above already follow. + const chatStart = source.indexOf('url.pathname === "/v1/chat/completions"'); + const nextRoute = source.indexOf("url.pathname === \"/v1/live\"", chatStart); + expect(chatStart).toBeGreaterThan(-1); + const branch = source.slice(chatStart, nextRoute); + expect(branch).toContain("handleChatCompletions(req, config, logCtx"); + expect(branch).toContain("req,\n policy,\n ));"); + expect(branch).not.toContain("req,\n config,\n ));"); + }); +}); + describe("loopback listener origin gate", () => { // The kernel bind stops remote TCP, but not a victim browser: an attacker page can make the // browser connect to 127.0.0.1, and that connection IS local. The Host/Origin gate is the diff --git a/tests/server/loopback-listener-integration.test.ts b/tests/server/loopback-listener-integration.test.ts index a98c6607a2..4dff1a2524 100644 --- a/tests/server/loopback-listener-integration.test.ts +++ b/tests/server/loopback-listener-integration.test.ts @@ -290,9 +290,11 @@ describe("unauthenticated loopback listener", () => { { method: "GET", path: "/" }, { method: "GET", path: "/healthz" }, { method: "GET", path: "/readyz" }, - { method: "POST", path: "/v1/chat/completions", body: '{"model":"x","messages":[]}' }, - { method: "POST", path: "/v1/messages", body: '{"model":"x","messages":[]}' }, { method: "GET", path: "/v1/opencodex/artifacts/x" }, + // The two inference wires are admitted as POST only (see the dedicated test below). + { method: "GET", path: "/v1/messages" }, + { method: "GET", path: "/v1/chat/completions" }, + { method: "POST", path: "/v1/messages/count_tokens", body: '{"model":"x","messages":[]}' }, // Voice call-create is admitted only as POST; the keyed sideband join only as an upgrade. { method: "GET", path: "/v1/live/rtc_x" }, { method: "GET", path: "/v1/realtime/calls/rtc_x" }, @@ -464,6 +466,50 @@ describe("unauthenticated loopback listener", () => { } }); + test("admits the two local client inference wires, and still refuses /api/* (#4236)", async () => { + // The hub's own local clients do not speak Responses: `ocx claude`, the system-env + // injection and Claude Desktop speak the Anthropic wire, Cursor / the vision helper / + // aside speak OpenAI chat. On a tailnet-bound hub this listener is their only local + // socket, so a 404 here is the whole "Codex works but nothing else does" defect. + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = await startLoopbackTestServer(loopbackPort); + const base = `http://127.0.0.1:${loopbackPort}`; + const publicBase = `http://127.0.0.1:${server.port}`; + try { + for (const path of ["/v1/messages", "/v1/chat/completions"]) { + const viaPublic = await fetch(`${publicBase}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + // The public listener is unchanged: a wildcard bind still demands a credential. + expect({ path, status: viaPublic.status }).toEqual({ path, status: 401 }); + + // Deliberately malformed so it fails INSIDE the handler. Neither 401 (not admitted) + // nor 404 (not on the allowlist) may come back. + const viaLoopback = await fetch(`${base}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + expect({ path, status: viaLoopback.status }).not.toEqual({ path, status: 401 }); + expect({ path, status: viaLoopback.status }).not.toEqual({ path, status: 404 }); + } + + // Management discovery is the OTHER destination contract and must not ride along: the + // CLI resolves `/api/*` through the authenticated surface with a management credential. + for (const path of ["/api/claude-code", "/api/config"]) { + const response = await fetch(`${base}${path}`, { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect({ path, status: response.status }).toEqual({ path, status: 404 }); + } + } finally { + await server.stop(true); + } + }); + test("admits POST /v1/responses and its compact sibling without a credential", async () => { const loopbackPort = await freePort(); saveConfig(baseConfig(loopbackPort)); @@ -929,18 +975,23 @@ describe("loopback companion listener", () => { const server = startServer(port); try { // Same socket semantics as the ported form, same default-deny. A companion is a bind - // address change, never an admission change — `/api/*` in particular stays unreachable - // without a management credential, and the Anthropic wire stays off this listener. - for (const path of ["/api/config", "/healthz", "/"]) { + // address change, never an admission change — `/api/*`, health and the GUI stay + // unreachable here no matter which port the listener shares. + for (const path of ["/api/config", "/api/claude-code", "/healthz", "/readyz", "/"]) { const response = await fetch(`http://127.0.0.1:${port}${path}`); expect({ path, status: response.status }).toEqual({ path, status: 404 }); } - const messages = await fetch(`http://127.0.0.1:${port}/v1/messages`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: '{"model":"x","messages":[]}', - }); - expect(messages.status).toBe(404); + // The inference wires the hub's own clients speak ARE served (#4236); malformed bodies, + // so a non-404 proves admission rather than an upstream call. + for (const path of ["/v1/messages", "/v1/chat/completions"]) { + const response = await fetch(`http://127.0.0.1:${port}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + expect({ path, status: response.status }).not.toEqual({ path, status: 404 }); + expect({ path, status: response.status }).not.toEqual({ path, status: 401 }); + } expect((await fetch(`http://127.0.0.1:${port}/v1/models`)).status).toBe(200); } finally { await server.stop(true); From 0dc12bed9f34f80c081b9840d614e0cec10715be Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:44:54 +0900 Subject: [PATCH 2/8] fix(claude): resolve a local client's two destinations instead of one port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight sites composed `http://127.0.0.1:` by hand (#4236). On a hub bound to a tailnet address that socket does not exist, so `ocx claude`, Claude Desktop, Cursor, the system-env injection, the gateway-model cache and the routed vision helper all pointed at a closed port while Codex — which already honored `unauthenticatedLoopbackListener` — worked. They are two contracts, not one substituted base URL, and `src/lib/local- destinations.ts` keeps them apart: - `localInferenceOrigin` / `localInferencePort` — the unauthenticated loopback listener's effective port when it is enabled, else the public port. That listener admits local callers with no credential. - `localManagementOrigin` — a hub's loopback management ingress when enabled, else the public bind address. The caller still sends the admin token: management auth has no loopback bypass and the unauthenticated listener serves no `/api/*`. No exported client configuration gains a credential. `fetchClaudeCodeState` uses the management resolver (its `enabled: false` answer is what makes `ocx claude` fall back to a native launch); every inference site uses the other one. A loopback or standalone install resolves to the same string it wrote before, so nothing changes there. Three details worth naming. `targetsLocalClaudeProxy` now takes a SET of ports, because the public port and the listener's port are both ours — rewriting one into the other would strip the admission token minted for it. The gateway-model cache moved with `buildClaudeEnv`, since Claude Code honors that file only while its `baseUrl` equals `ANTHROPIC_BASE_URL`. And the system-env tracking record gained an optional `clientPort`: ownership on revert is proven against the port that was injected, while liveness keeps probing the public port, because the listener serves no `/healthz` and probing it would revert a live proxy's environment. `resolveApiAccessBaseUrl` changed only in its last-resort loopback branch; every branch above it still describes the address the client actually reached. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- .../docs/reference/configuration/server.md | 13 +++- src/claude/desktop-3p.ts | 11 +++- src/claude/gateway-cache.ts | 10 ++- src/cli/claude.ts | 43 +++++++++--- src/lib/local-destinations.ts | 66 +++++++++++++++++++ src/server/management/api-access.ts | 9 ++- .../management/cursor-integration-routes.ts | 12 +++- src/server/system-env-shell.ts | 6 +- src/server/system-env.ts | 46 ++++++++++--- src/vision/plan.ts | 13 +++- src/vision/routed-describe.ts | 17 +++-- structure/09_client-integrations.md | 15 +++++ 12 files changed, 224 insertions(+), 37 deletions(-) create mode 100644 src/lib/local-destinations.ts diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index 4fbbeb9e8e..cf7237a1c1 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -195,12 +195,21 @@ already holds that loopback address, so OpenCodex refuses the pair at write time rather than failing the second bind. On those binds you do not need the listener at all — a loopback bind already admits local callers. +With a `port` set, the local integrations follow the listener: `ocx claude`, the `system-env` +injection, the Claude Desktop profile, the Cursor gateway value and the routed vision helper all +write `http://127.0.0.1:`, the same port `ocx sync` writes into Codex. Restart the +proxy after changing this field so those values are rewritten. + The listener serves only `POST /v1/responses`, its WebSocket upgrade, `POST /v1/responses/compact`, +`POST /v1/messages` (the Anthropic wire Claude Code and Claude Desktop speak), +`POST /v1/chat/completions` (the OpenAI chat wire Cursor and the vision helper speak), `POST /v1/alpha/search` (the native Codex web-search relay), `GET /v1/models`, and the realtime voice surface: the standalone WebSocket upgrades, WebRTC call creation (`POST /v1/live`, `POST /v1/realtime/calls`), and the keyed sideband join upgrades (`/v1/live/{callId}`, -`/v1/realtime/calls/{callId}`, `/v1/realtime?call_id=`). Everything else, including `/api/*` and -the dashboard, returns `404`. +`/v1/realtime/calls/{callId}`, `/v1/realtime?call_id=`). Everything else, including `/api/*`, +`/healthz`, `/readyz` and the dashboard, returns `404` — local management reads such as +`ocx claude`'s discovery call go to the authenticated management surface with a management +credential, never here. :::danger[This is an unauthenticated surface] Every process on the machine can use this listener. It spends account quota and paid provider diff --git a/src/claude/desktop-3p.ts b/src/claude/desktop-3p.ts index 7163ca2fa1..102426c5cf 100644 --- a/src/claude/desktop-3p.ts +++ b/src/claude/desktop-3p.ts @@ -22,6 +22,7 @@ import { type DesktopProfileModel, } from "./desktop-profile"; import { nativeOpenAiContextWindow, type NativeContextLimitsInput } from "../codex/catalog"; +import { localInferencePort } from "../lib/local-destinations"; import { assertDesktop3pModelsValid } from "./desktop-3p-guard"; export interface Desktop3pModelEntry { @@ -322,6 +323,9 @@ export function activeDesktop3pAlias(provider: string, modelId: string): string * channel for supports1m/tier pins and it overrides discovery anyway (no merge), so * discovery stays off for determinism. supports1m makes Desktop offer a separate 1M * row; selecting it sends the bare id + `anthropic-beta: context-1m-2025-08-07`. + * + * `port` is the LOCAL port Desktop should dial, already resolved by the caller (see + * `writeDesktop3pConfig`): on a hub that is the unauthenticated loopback listener's port. */ export function generateDesktop3pConfig( port: number, @@ -620,8 +624,13 @@ export function writeDesktop3pConfig( if (connection.kind === "connected" || inspectRemoteDesktopCleanup().kind !== "absent") { return { written: false, path: resolveDesktop3pConfigLibraryPath(), reason: "desktop_remote_store_active" }; } + // Claude Desktop runs on this machine, so it dials the unauthenticated loopback listener + // when one is enabled — on a tailnet-bound hub that is the only local socket (#4236). + // Resolved here, from the config this write already re-read, rather than in the pure + // generator: `latest.config` is the freshest answer any caller could pass in. + const localPort = localInferencePort(latest.config, port); return writeDesktop3pConfigWithGenerator(() => ( - generateDesktop3pConfig(port, nativeSlugs, routedModels, apiKey, mode, profile, nativeContextCap) + generateDesktop3pConfig(localPort, nativeSlugs, routedModels, apiKey, mode, profile, nativeContextCap) )); }), lifecycleLockDeps); } catch { return { written: false, path: resolveDesktop3pConfigLibraryPath(), reason: "desktop_lifecycle_busy_or_unsafe" }; } diff --git a/src/claude/gateway-cache.ts b/src/claude/gateway-cache.ts index 33df1e8457..4e0d70cc3c 100644 --- a/src/claude/gateway-cache.ts +++ b/src/claude/gateway-cache.ts @@ -13,6 +13,7 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; +import { localInferenceOrigin } from "../lib/local-destinations"; import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "../lib/service-secrets"; import type { OcxConfig } from "../types"; @@ -24,7 +25,12 @@ export interface GatewayModelRow { export interface GatewayModelCacheRefreshOptions { timeoutMs?: number; configDir?: string; - admissionConfig?: Pick; + /** + * Admission credential source AND local destination source: the cache file's `baseUrl` must + * equal the `ANTHROPIC_BASE_URL` the CLI is launched with or Claude Code ignores the whole + * cache, so this has to resolve the same loopback listener `buildClaudeEnv` resolves (#4236). + */ + admissionConfig?: Pick; env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; } @@ -102,7 +108,7 @@ export async function refreshGatewayModelCacheFromProxy( if (admissionToken) headers.set("x-opencodex-api-key", admissionToken); const baseUrl = typeof portOrTarget === "number" - ? `http://127.0.0.1:${portOrTarget}` + ? localInferenceOrigin(options.admissionConfig, portOrTarget) : new URL(portOrTarget.baseUrl).origin; // ?ids=cli pins the readable claude-ocx id family deterministically (audit 051 diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 446485e2b7..50a0379b2c 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -18,6 +18,7 @@ import { isProxyAdmissionSecret } from "../server/auth-cors"; import { findLiveProxy } from "../server/proxy-liveness"; import type { OcxConfig } from "../types"; import { configuredAdminToken } from "../lib/admin-secrets"; +import { localInferenceOrigin, localInferencePort, localManagementOrigin } from "../lib/local-destinations"; import { PROXY_MARKER, ownAdmissionTokens, defaultAuthDetectDeps, detectClaudeAuth, type AuthDetectDeps } from "../claude/auth-detect"; import { resolveClaudeAuthMode } from "../claude/auth-mode"; import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; @@ -102,14 +103,22 @@ function isClaudeLoopbackHostname(hostname: string): boolean { || normalized === "[::1]"; } -function targetsLocalClaudeProxy(value: string | undefined, port: number): boolean { +/** + * Is this loopback base URL one of OURS? + * + * "Ours" is a SET of ports, not one port (#4236): on a hub with an unauthenticated loopback + * listener the public port and the listener's port are both local addresses this proxy answers + * on, so a URL naming either of them was written by us. Treating the one this launch did not + * pick as a foreign proxy would strip our own admission token out of the environment. + */ +function targetsLocalClaudeProxy(value: string | undefined, ports: readonly number[]): boolean { if (!value) return false; try { const parsed = new URL(value); const effectivePort = parsed.port === "" ? 80 : Number(parsed.port); return parsed.protocol === "http:" && isClaudeLoopbackHostname(parsed.hostname) - && effectivePort === port + && ports.includes(effectivePort) && parsed.username === "" && parsed.password === ""; } catch { @@ -147,7 +156,13 @@ export function buildClaudeEnv( ): ClaudeLaunchEnv { const explicitTarget = typeof portOrTarget === "number" ? null : portOrTarget; const port = typeof portOrTarget === "number" ? portOrTarget : null; - const managedBaseUrl = explicitTarget ? new URL(explicitTarget.baseUrl).origin : `http://127.0.0.1:${port}`; + // A local launch dials the unauthenticated loopback listener whenever one is enabled — the + // only local socket a tailnet-bound hub has (#4236). Unchanged on every other topology. + const managedBaseUrl = explicitTarget + ? new URL(explicitTarget.baseUrl).origin + : localInferenceOrigin(config, port!); + // Every local port this proxy answers on, so a base URL naming either one is still ours. + const ownLocalPorts = port === null ? [] : [...new Set([port, localInferencePort(config, port)])]; const env: ClaudeLaunchEnv = { ...base }; // Step 1 — strip OUR OWN dummy from the inherited environment before anything reads // or writes the token slot. setDefault below preserves any non-empty value, so a @@ -188,10 +203,13 @@ export function buildClaudeEnv( try { const parsed = new URL(existingBaseUrl); const effectivePort = parsed.port === "" ? 80 : Number(parsed.port); + // Stale means "a port no live local listener of ours owns". With a loopback listener + // enabled that is two ports, and rewriting one of them into the other would reject a + // destination we wrote ourselves. if (parsed.protocol === "http:" && isClaudeLoopbackHostname(parsed.hostname) - && effectivePort !== port) { - const replacement = `http://127.0.0.1:${port}`; + && !ownLocalPorts.includes(effectivePort)) { + const replacement = managedBaseUrl; console.error(`⚠ Replacing stale opencodex ANTHROPIC_BASE_URL ${parsed.origin} with ${replacement}.`); env.ANTHROPIC_BASE_URL = replacement; // The credentials in this environment were paired with the destination we just @@ -219,7 +237,7 @@ export function buildClaudeEnv( const ownTokens = explicitTarget ? [explicitTarget.admissionToken] : ownAdmissionTokens(config); const targetsLocalProxy = explicitTarget ? targetsClaudeRoutingTarget(env.ANTHROPIC_BASE_URL, explicitTarget) - : targetsLocalClaudeProxy(env.ANTHROPIC_BASE_URL, port!); + : targetsLocalClaudeProxy(env.ANTHROPIC_BASE_URL, ownLocalPorts); const isOwnAdmissionToken = (value: string): boolean => ownTokens.includes(value) || isProxyAdmissionSecret(value, config); const inheritedApiKey = env.ANTHROPIC_API_KEY; @@ -335,6 +353,12 @@ export function buildClaudeEnv( * Context-window map from the RUNNING proxy's management API (warm TTL cache; the * daemon registers every selector form — audit R3#1). 3s bound + management auth header. * (no [1m] marking, conservative). + * + * This is the MANAGEMENT destination, not the inference one (#4236): `/api/claude-code` is + * never served by the unauthenticated loopback listener, so it resolves through + * `localManagementOrigin` — a hub's loopback management ingress when it has one, otherwise the + * public bind — and keeps sending the local admin token. `enabled: false` is how `ocx claude` + * decides to launch natively, so a wrong destination here silently downgrades every launch. */ export interface ClaudeCodeLiveState { contextWindows: Record; @@ -346,7 +370,7 @@ export async function fetchClaudeCodeState(config: OcxConfig, port: number, time const headers = new Headers(); const token = configuredAdminToken(); if (token) headers.set("x-opencodex-api-key", token); - const res = await fetch(`http://127.0.0.1:${port}/api/claude-code`, { + const res = await fetch(`${localManagementOrigin(config, port)}/api/claude-code`, { headers, signal: AbortSignal.timeout(timeoutMs), }); @@ -525,7 +549,10 @@ export function buildNativeClaudeEnv( return Boolean(value && (value === PROXY_MARKER || isProxyAdmissionSecret(value, config))); }); const baseUrl = env.ANTHROPIC_BASE_URL; - if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, config.port)) { + // Both local ports count as ours here too: a native launch must shed the managed destination + // whichever of them this machine's last proxy launch wrote (#4236). + const nativeLocalPorts = [...new Set([config.port, localInferencePort(config, config.port)])]; + if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, nativeLocalPorts)) { delete env.ANTHROPIC_BASE_URL; } for (const name of admissionSlots) { diff --git a/src/lib/local-destinations.ts b/src/lib/local-destinations.ts new file mode 100644 index 0000000000..54220a3e29 --- /dev/null +++ b/src/lib/local-destinations.ts @@ -0,0 +1,66 @@ +/** + * Where a process running ON THE HUB ITSELF dials the hub (#4236). + * + * There are TWO destinations here, not one base URL substituted everywhere, and conflating + * them is what broke every local integration on a tailnet-bound hub: + * + * 1. `localManagementOrigin` — authenticated management discovery/state (`/api/*`). It is + * served by the public listener and, on a hub, additionally by the loopback-only + * `hub.managementIngress`. Callers must still send a management credential: management + * authentication has no loopback bypass (structure/05), and the unauthenticated loopback + * listener deliberately does not serve `/api/*` at all. + * 2. `localInferenceOrigin` — the data plane a client wire actually speaks. When + * `unauthenticatedLoopbackListener` is enabled this is the listener's effective port, which + * admits local callers with no credential, so nothing has to export one. + * + * A hub whose public listener binds a tailnet address has no `127.0.0.1:` socket, + * which is why eight call sites hardcoding that origin all failed while Codex (which already + * honored the listener) worked. They route through here instead of repeating `?? port`: the day + * the resolution changes, a forgotten site points a client config at a closed socket. + */ +import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; +import { probeHostname } from "../server/proxy-liveness"; +import type { OcxConfig } from "../types"; + +export type LocalInferenceConfig = Pick; +export type LocalManagementConfig = Pick; + +/** + * The port a local client dials for inference: the unauthenticated loopback listener's + * effective port when it is enabled, otherwise the public port (unchanged behaviour for a + * loopback or standalone install). + */ +export function localInferencePort( + config: LocalInferenceConfig | undefined, + publicPort: number, +): number { + return effectiveLoopbackListenerPort(config, publicPort) ?? publicPort; +} + +/** `http://127.0.0.1:` — the origin every local client wire writes. */ +export function localInferenceOrigin( + config: LocalInferenceConfig | undefined, + publicPort: number, +): string { + return `http://127.0.0.1:${localInferencePort(config, publicPort)}`; +} + +/** + * The origin a local CLI dials for `/api/*`. + * + * 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. + * + * The caller still supplies the management credential. Never write that credential into an + * exported client configuration. + */ +export function localManagementOrigin( + config: LocalManagementConfig | undefined, + publicPort: number, +): 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}`; +} diff --git a/src/server/management/api-access.ts b/src/server/management/api-access.ts index 65c9502bb6..2bd0fa3fb4 100644 --- a/src/server/management/api-access.ts +++ b/src/server/management/api-access.ts @@ -1,4 +1,5 @@ import type { OcxConfig } from "../../types"; +import { localInferenceOrigin } from "../../lib/local-destinations"; import { probeHostname } from "../proxy-liveness"; export interface ApiAccessEndpoints { @@ -66,7 +67,7 @@ function originBaseUrl(raw: string): string | null { * Falls back to loopback only when no usable request context is available. */ export function resolveApiAccessBaseUrl( - config: Pick, + config: Pick, opts: BuildApiAccessEndpointsOptions = {}, ): string { const port = config.port ?? 10100; @@ -104,7 +105,11 @@ export function resolveApiAccessBaseUrl( } } - return `http://127.0.0.1:${port}/v1`; + // Last resort: a wildcard bind with no usable request context, so the only address we can + // name is loopback — and on that address the unauthenticated loopback listener, when one is + // enabled, is the port a local caller should use (#4236). The branches above are unchanged: + // a specific bind or a real request host still describes the address the CLIENT reached. + return `${localInferenceOrigin(config, port)}/v1`; } /** @deprecated Prefer resolveApiAccessBaseUrl; retained for focused host-format tests. */ diff --git a/src/server/management/cursor-integration-routes.ts b/src/server/management/cursor-integration-routes.ts index 46f39209f9..9724523a9d 100644 --- a/src/server/management/cursor-integration-routes.ts +++ b/src/server/management/cursor-integration-routes.ts @@ -14,6 +14,7 @@ import { cursorLastSeen, type CursorSeen } from "../../integrations/cursor-seen" import { detectCursorInstalls, type CursorInstall } from "../../integrations/cursor-detect"; import { loadCursorEffortTable } from "../../integrations/cursor-effort-table"; import { configuredApiAuthToken, isApiAuthRequired, jsonResponse } from "../auth-cors"; +import { localInferencePort } from "../../lib/local-destinations"; import { fetchAllModels } from "../management-api"; import { predictCursorEffort } from "../models-capabilities"; import { expandCursorEffortRow, knownEffortRowIds } from "../effort-row"; @@ -54,8 +55,13 @@ export async function buildCursorIntegrationStatus( // The port the browser reached is the one Cursor on the same machine will reach too; the // runtime record and config.port are fallbacks for a request that carries no port. const port = runtime?.port ?? (Number(ctx.url?.port) || config.port); - // Describes the public bind. A second unauthenticated loopback listener may exist, but the - // value a user pastes into Cursor must work against the bind they will actually reach. + // Cursor runs on this machine, so the gateway URL it is told to paste is the LOCAL one: the + // unauthenticated loopback listener when one is enabled (on a tailnet-bound hub there is no + // other local socket), otherwise 127.0.0.1 on the public port exactly as before (#4236). + const gatewayPort = localInferencePort(config, port ?? 10100); + // apiKeyMode still describes the public bind's admission rule: a key is never required by the + // loopback listener, but pasting one there is harmless, while omitting one on a bind that + // demands it is not. const credentialConfigured = !!configuredApiAuthToken(config) || (config.apiKeys ?? []).some(entry => !!entry.key.trim()); const apiKeyMode = isApiAuthRequired(config) || credentialConfigured ? "credential" : "placeholder"; @@ -107,7 +113,7 @@ export async function buildCursorIntegrationStatus( }, regularCursor: { installed: regular !== undefined, path: regular?.path ?? null }, gateway: { - baseUrl: `http://127.0.0.1:${port}/v1`, + baseUrl: `http://127.0.0.1:${gatewayPort}/v1`, apiKeyMode, placeholder: CURSOR_GATEWAY_PLACEHOLDER_KEY, }, diff --git a/src/server/system-env-shell.ts b/src/server/system-env-shell.ts index 35954f035e..21b7d5d638 100644 --- a/src/server/system-env-shell.ts +++ b/src/server/system-env-shell.ts @@ -7,6 +7,7 @@ import { resolveClaudeAuthMode } from "../claude/auth-mode"; import { ANTHROPIC_PARENT_ENV_SLOTS, trustedNodeLauncherContext, type AnthropicParentEnvSlot } from "../cli/launcher-context"; import type { OcxConfig } from "../types"; import { recordOwnedConfigPath } from "../lib/config-ownership"; +import { localInferenceOrigin } from "../lib/local-destinations"; /** * Does the opencodex dummy marker belong in the system environment? @@ -79,9 +80,12 @@ export function writeShellEnvFile( auto?: AutoContextMode, deps: SystemEnvDeps = {}, ): void { + // Same local destination the launchd domain gets: the unauthenticated loopback listener when + // one is enabled, otherwise the public port (#4236). The two files must not disagree, or a + // new shell and a launchd-started `claude` would dial different sockets. const lines = [ `# Generated by opencodex — do not edit manually`, - `export ANTHROPIC_BASE_URL=${shellValue(`http://127.0.0.1:${port}`)}`, + `export ANTHROPIC_BASE_URL=${shellValue(localInferenceOrigin(config, port))}`, `export CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=${shellValue("1")}`, ]; // New lever keys are CONDITIONAL exports (audit 139 R2#1): a value the user already diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 5825b2a2a1..35aad76532 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -7,6 +7,7 @@ import { PROXY_MARKER } from "../claude/auth-detect"; import { isProxyAdmissionSecret } from "./auth-cors"; import type { OcxConfig } from "../types"; import { recordOwnedConfigPath } from "../lib/config-ownership"; +import { localInferencePort } from "../lib/local-destinations"; import { providerContextCap } from "../providers/context-cap"; import { OPENAI_CODEX_PROVIDER_ID } from "../providers/openai-tiers"; export { getShellEnvFilePath, installShellHook, uninstallShellHook, claudeCodeCliInstalled, reconcileShellHook } from "./system-env-shell"; @@ -35,7 +36,18 @@ const MANAGED_SYSTEM_ENV_NAMES = new Set([ interface SystemEnvTracking { pid: number; + /** The PUBLIC port of the owning proxy: its instance identity and its /healthz address. */ port: number; + /** + * The loopback port the injected ANTHROPIC_BASE_URL names, when it is not `port` (#4236). + * + * These two separated the day local clients started honoring the unauthenticated loopback + * listener. Ownership is proven against THIS port (it is what was injected), while liveness + * is still probed on `port` — the listener serves no `/healthz`, so probing it would declare + * a perfectly live proxy stale and revert its environment. Absent in records written before + * this field existed, where the two were by definition the same. + */ + clientPort?: number; injectedAt: string; /** Keys that were actually set by injection (revert only unsets these). */ injectedKeys?: string[]; @@ -70,7 +82,8 @@ function readTracking(): SystemEnvTracking | undefined { (name): name is string => typeof name === "string" && MANAGED_SYSTEM_ENV_NAMES.has(name), ))] : undefined; - return { ...tracking, injectedKeys } as SystemEnvTracking; + const clientPort = Number.isInteger(tracking.clientPort) ? tracking.clientPort : undefined; + return { ...tracking, clientPort, injectedKeys } as SystemEnvTracking; } catch { return undefined; } @@ -88,18 +101,24 @@ function ownedBaseUrl(port: number): string { return `http://127.0.0.1:${port}`; } -function writeTracking(port: number, injectedKeys: string[]): void { +function writeTracking(port: number, injectedKeys: string[], clientPort: number = port): void { recordOwnedConfigPath(getConfigDir(), getSystemEnvTrackingPath()); mkdirSync(getConfigDir(), { recursive: true, mode: 0o700 }); writeFileSync(getSystemEnvTrackingPath(), JSON.stringify({ pid: process.pid, port, + ...(clientPort === port ? {} : { clientPort }), injectedAt: new Date().toISOString(), injectedKeys, }), { encoding: "utf8", mode: 0o600 }); } -function rollbackInjectedKeys(port: number, injectedKeys: string[]): void { +/** The base URL that was injected: the loopback listener's port when one is in play (#4236). */ +function trackedBaseUrl(tracking: Pick): string { + return ownedBaseUrl(tracking.clientPort ?? tracking.port); +} + +function rollbackInjectedKeys(port: number, injectedKeys: string[], clientPort: number = port): void { const rollbackFailed: string[] = []; for (const name of [...injectedKeys].reverse()) { try { @@ -110,7 +129,7 @@ function rollbackInjectedKeys(port: number, injectedKeys: string[]): void { } if (rollbackFailed.length > 0) { - writeTracking(port, rollbackFailed); + writeTracking(port, rollbackFailed, clientPort); return; } @@ -164,14 +183,18 @@ export async function injectSystemEnv( const injectedKeys: string[] = existingTracking ? [...(existingTracking.injectedKeys ?? SYSTEM_ENV_NAMES)] : []; + // A launchd-started `claude` is a LOCAL client: it dials the unauthenticated loopback + // listener when one is enabled, because on a tailnet-bound hub nothing answers on + // 127.0.0.1: (#4236). `port` stays the instance identity and /healthz address. + const clientPort = localInferencePort(config, port); const inject = (name: string, value: string) => { setLaunchctlEnv(name, value); if (!injectedKeys.includes(name)) injectedKeys.push(name); - writeTracking(port, injectedKeys); + writeTracking(port, injectedKeys, clientPort); }; try { - inject("ANTHROPIC_BASE_URL", ownedBaseUrl(port)); + inject("ANTHROPIC_BASE_URL", ownedBaseUrl(clientPort)); inject("CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", "1"); const markerMode = systemEnvMarkerMode(config, deps); if (markerMode === "proxy") { @@ -190,7 +213,7 @@ export async function injectSystemEnv( unsetLaunchctlEnv("ANTHROPIC_AUTH_TOKEN"); const tokenIdx = injectedKeys.indexOf("ANTHROPIC_AUTH_TOKEN"); if (tokenIdx >= 0) injectedKeys.splice(tokenIdx, 1); - writeTracking(port, injectedKeys); + writeTracking(port, injectedKeys, clientPort); } } // Lever keys (devlog 136 B6): user-wins — skip any key the user already set in the @@ -242,9 +265,9 @@ export async function injectSystemEnv( injectClaudeAgentDefs(config, windows); } catch { /* best-effort */ } - writeTracking(port, injectedKeys); + writeTracking(port, injectedKeys, clientPort); } catch (error) { - rollbackInjectedKeys(port, injectedKeys); + rollbackInjectedKeys(port, injectedKeys, clientPort); removeShellEnvFile(); console.error("Failed to inject system environment; rolled back launchctl changes:", error); throw error; @@ -266,7 +289,7 @@ export function revertSystemEnv(): RevertResult { try { const tracksBaseUrl = tracking.injectedKeys?.includes("ANTHROPIC_BASE_URL") ?? true; - if (tracksBaseUrl && launchctlGetenv("ANTHROPIC_BASE_URL") !== ownedBaseUrl(tracking.port)) { + if (tracksBaseUrl && launchctlGetenv("ANTHROPIC_BASE_URL") !== trackedBaseUrl(tracking)) { return { reverted: false, reason: "ownership mismatch" }; } @@ -296,6 +319,9 @@ export async function cleanStaleSystemEnv(): Promise { if (!tracking) return { cleaned: false, reason: "no tracking file" }; try { + // `tracking.port`, never the injected client port: `/healthz` is not on the + // unauthenticated loopback listener's allowlist, so probing that port would 404 and + // revert a live proxy's environment (#4236). const response = await fetch(`${ownedBaseUrl(tracking.port)}/healthz`, { signal: AbortSignal.timeout(1_000), }); diff --git a/src/vision/plan.ts b/src/vision/plan.ts index cbcccf3d49..3e0f25eca9 100644 --- a/src/vision/plan.ts +++ b/src/vision/plan.ts @@ -110,8 +110,8 @@ export interface VisionPlan { anthropicSidecar?: AnthropicVisionProvider; /** Namespaced "provider/model" describer for the routed backend (roadmap 180). */ routedModel?: string; - /** Loopback dispatch inputs for the routed backend. */ - routedConfig?: Pick; + /** Loopback dispatch inputs for the routed backend (the listener decides WHICH local port). */ + routedConfig?: Pick; settings: VisionSettings; maxDescriptionsPerTurn: number; } @@ -153,7 +153,14 @@ export function planVisionSidecar( return { backend: "routed", routedModel, - routedConfig: { port: config.port, ...(config.apiKeys ? { apiKeys: config.apiKeys } : {}) }, + routedConfig: { + port: config.port, + ...(config.apiKeys ? { apiKeys: config.apiKeys } : {}), + // The self-fetch has to honor the unauthenticated loopback listener (#4236). + ...(config.unauthenticatedLoopbackListener + ? { unauthenticatedLoopbackListener: config.unauthenticatedLoopbackListener } + : {}), + }, settings: { model: routedModel, reasoning: DEFAULT_REASONING, diff --git a/src/vision/routed-describe.ts b/src/vision/routed-describe.ts index fdd1575606..0bd4e52b13 100644 --- a/src/vision/routed-describe.ts +++ b/src/vision/routed-describe.ts @@ -21,9 +21,11 @@ * * Known limitation (recorded in roadmap 170): a bindHost where 127.0.0.1 * does not answer cannot reach its own loopback — same latent limitation - * gateway-cache has. + * gateway-cache has. #4236 closes it for the case that actually occurs: a hub + * with an unauthenticated loopback listener, which this helper now dials. */ import type { OcxConfig } from "../types"; +import { localInferenceOrigin } from "../lib/local-destinations"; import { signalWithTimeout, cancelBodyOnAbort } from "../lib/abort"; import { redactSecretString } from "../lib/redact"; import { sidecarEnter } from "../lib/sidecar-tracker"; @@ -71,12 +73,17 @@ export function routedDescribeAdmissionToken(config: Pick) } /** Base URL seam for tests; production always self-fetches loopback. */ -export function routedDescribeBaseUrl(config: Pick): string { +export function routedDescribeBaseUrl( + config: Pick, +): string { // config.port can be 0 (ephemeral bind, tests) or stale after a live port // override; the server records its ACTUAL bound port via setCorsOrigin at // startup, so prefer that when config carries no positive port. - const port = config.port && config.port > 0 ? String(config.port) : configuredPort(); - return `http://127.0.0.1:${port}`; + const port = config.port && config.port > 0 ? config.port : Number(configuredPort()); + // This self-fetch is a local client like any other: on a hub bound to a tailnet address the + // only socket on 127.0.0.1 is the unauthenticated loopback listener (#4236). The helper sends + // the OpenAI chat wire, which that listener now admits. + return localInferenceOrigin(config, port); } export async function describeImageRouted( @@ -84,7 +91,7 @@ export async function describeImageRouted( _detail: string | undefined, contextText: string, routedModel: string, - config: Pick, + config: Pick, settings: VisionSettings, abortSignal?: AbortSignal, baseUrlOverride?: string, diff --git a/structure/09_client-integrations.md b/structure/09_client-integrations.md index 70ad5f1ea9..a5b535a464 100644 --- a/structure/09_client-integrations.md +++ b/structure/09_client-integrations.md @@ -149,6 +149,21 @@ fingerprint-only tests are supplementary; they cannot prove the status and write Remote clients journal and restore native integrations locally while model traffic travels directly to the hub. Catalog writes occur only after protocol negotiation and full remote schema validation. The management relay is launcher-scoped and fixed to the connection's management origin. Claude/Codex launch behavior remains integration-scoped. Key rotation and recovery align both the local connection credential and the connection-owned Desktop profile before reporting completion. Disconnect restores owned Desktop settings and native integrations locally without automatic hub-key revocation or usage mirroring. Interrupted cleanup remains recoverable for the same connection; conflicts prevent a full-cleanup claim. +## Local destinations on a hub + +A client running on the proxy's own machine has two destinations, and they are resolved +separately by `src/lib/local-destinations.ts`. Inference (`localInferenceOrigin`) is +`127.0.0.1` on the unauthenticated loopback listener's effective port when that listener is +enabled, otherwise the public port; a hub bound to a tailnet or LAN address has no other local +data socket, so `ocx claude`, the `system-env` injection, the Claude Desktop profile, the Cursor +gateway value, the gateway-model cache, the routed vision self-fetch and the API-access loopback +fallback all go through that resolver rather than composing the port themselves. Management +(`localManagementOrigin`) is the hub's loopback `hub.managementIngress` when enabled, otherwise +the public bind address, and the caller supplies the management credential. Management +authentication has no loopback bypass and the data-loopback listener serves no `/api/*`, so these +two must never be collapsed into one base URL, and an admin credential must never be written into +an exported client configuration. + ## Connected Claude Desktop profiles Connected `ocx claude desktop apply` reads the hub's Desktop snapshot and writes the hub origin From 4d044d64893d8d38953a289ff071967b1f45e571 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:45:05 +0900 Subject: [PATCH 3/8] test(claude,server,vision): pin both local destinations per module One case per touched module, each proving the same three topologies: a ported listener moves the destination, a companion listener keeps the public port, and a plain loopback or standalone install is byte-identical to before. `tests/lib/local-destinations.test.ts` (new, registered in the layout manifests) holds the resolvers apart: the inference origin is always loopback and never the bind address, and the management origin never returns the listener's port even when both are configured. The per-module cases cover what a shared resolver cannot: the gateway cache's `baseUrl` must equal what `buildClaudeEnv` writes or Claude Code ignores the file; `writeDesktop3pConfig` must resolve from the config it re-reads under the mutation lock; system-env must inject the listener port while still probing `/healthz` on the public one; the vision planner must carry the listener field into the narrowed config it hands the self-fetch; and `resolveApiAccessBaseUrl` must NOT move when a real request host is available. `ocx claude` also gets the two ports treated as one family: neither of our own local ports is replaced as stale, and a native launch sheds the managed destination on either of them. PR2's "the ported form still splits the two" witness was the record of the gap this unit closes, so it now asserts the agreement. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- scripts/test-layout/layout.json | 1 + tests/claude-integration/claude-cli.test.ts | 147 ++++++++++++++++++ .../claude-gateway-cache.test.ts | 35 +++++ tests/clients/desktop-3p.test.ts | 49 ++++++ tests/fixtures/test-layout-expected.json | 1 + tests/lib/local-destinations.test.ts | 125 +++++++++++++++ .../cursor/cursor-integration-status.test.ts | 32 ++++ tests/server/api-access-endpoints.test.ts | 29 ++++ .../loopback-companion-client-targets.test.ts | 12 +- tests/server/system-env.test.ts | 67 ++++++++ tests/vision/vision-routed.test.ts | 62 +++++++- 11 files changed, 553 insertions(+), 7 deletions(-) create mode 100644 tests/lib/local-destinations.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 4aa9b3de01..032252ad37 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -798,6 +798,7 @@ "launchd-repair.test.ts": "service", "legacy-shell-compat.test.ts": "responses", "live-service-manager-guard.test.ts": "service", + "local-destinations.test.ts": "lib", "local-management-attestation.test.ts": "server", "local-management-capability.test.ts": "server", "local-management-direct-transport.test.ts": "server", diff --git a/tests/claude-integration/claude-cli.test.ts b/tests/claude-integration/claude-cli.test.ts index 86574b11b9..90a66c0362 100644 --- a/tests/claude-integration/claude-cli.test.ts +++ b/tests/claude-integration/claude-cli.test.ts @@ -6,6 +6,7 @@ import { claudeLaunchPreflight, claudeNotFoundHint, ensureProxyForClaude, + fetchClaudeCodeState, isProxyOnlyModelId, nativeModelOverride, readPickerDefaultModel, @@ -542,6 +543,152 @@ describe("ocx claude env assembly", () => { }); +/** + * Which local socket `ocx claude` dials (#4236). + * + * `ocx claude` is handed the live PUBLIC port. On a hub bound to a tailnet address nothing + * answers on `127.0.0.1:`, so the launch has to resolve the unauthenticated + * loopback listener instead — the same port `ocx sync` already writes into Codex. + */ +describe("ocx claude local inference destination", () => { + const hub = (listener?: { enabled: boolean; port?: number }) => cfg({ + hostname: "100.76.170.81", + runtimeRole: "hub", + ...(listener ? { unauthenticatedLoopbackListener: listener } : {}), + } as Partial); + + test("a ported listener moves the base URL to the listener's port", () => { + const env = buildClaudeEnv(hub({ enabled: true, port: 10104 }), 10100, {}); + expect(env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10104"); + }); + + test("a companion listener keeps the public port, which is the point of that form", () => { + const env = buildClaudeEnv(hub({ enabled: true }), 10100, {}); + expect(env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); + }); + + test("a plain loopback install is byte-identical to before", () => { + expect(buildClaudeEnv(cfg(), 10100, {}).ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); + // And a hub that enabled no listener keeps dialing the public port too: this PR changes + // where an ENABLED listener sends local clients, not whether one exists. + expect(buildClaudeEnv(hub(), 10100, {}).ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); + }); + + test("neither of our own local ports is treated as a stale foreign proxy", () => { + // The public port and the listener port are both ours. Rewriting one into the other would + // strip the admission token that was minted for it, silently downgrading the launch. + const config = cfg({ + claudeCode: { authMode: "proxy" }, + hostname: "100.76.170.81", + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + for (const origin of ["http://127.0.0.1:10100", "http://127.0.0.1:10104"]) { + const env = buildClaudeEnv(config, 10100, { ANTHROPIC_BASE_URL: origin }, {}, { + ...AUTH_PRESENT, + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"], + }); + expect({ origin, baseUrl: env.ANTHROPIC_BASE_URL }).toEqual({ origin, baseUrl: origin }); + expect({ origin, token: env.ANTHROPIC_AUTH_TOKEN }).toEqual({ origin, token: "ocx_data_this_proxy_key" }); + } + }); + + test("a genuinely foreign loopback port is replaced with the resolved destination", () => { + const env = buildClaudeEnv( + hub({ enabled: true, port: 10104 }), + 10100, + { ANTHROPIC_BASE_URL: "http://127.0.0.1:19999" }, + {}, + { ...AUTH_PRESENT, preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"] }, + ); + expect(env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10104"); + }); + + test("a native launch sheds the managed destination on either local port", () => { + const config = cfg({ + hostname: "100.76.170.81", + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + for (const origin of ["http://127.0.0.1:10100", "http://127.0.0.1:10104"]) { + const env = buildNativeClaudeEnv(config, { + ANTHROPIC_BASE_URL: origin, + ANTHROPIC_AUTH_TOKEN: "ocx_data_this_proxy_key", + }, { preBunAnthropicSlots: ["ANTHROPIC_BASE_URL", "ANTHROPIC_AUTH_TOKEN"] }); + expect({ origin, baseUrl: env.ANTHROPIC_BASE_URL }).toEqual({ origin, baseUrl: undefined }); + } + }); +}); + +/** + * The OTHER destination contract (#4236): `/api/claude-code` is management state, never served + * by the unauthenticated loopback listener, so it resolves to the authenticated surface — a + * hub's loopback management ingress when it has one — and keeps carrying the local admin token. + * `enabled: false` from this call is what makes `ocx claude` launch natively, so a wrong + * destination here downgrades every launch on the machine. + */ +describe("ocx claude management discovery destination", () => { + const previousAdminToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + const FAKE_ADMIN_TOKEN = `ocx_admin_${"t".repeat(43)}`; + + async function captureDiscovery(config: OcxConfig): Promise<{ url: string; header: string | null }> { + const realFetch = globalThis.fetch; + let seen = { url: "", header: null as string | null }; + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = FAKE_ADMIN_TOKEN; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const request = new Request(input as never, init); + seen = { url: request.url, header: request.headers.get("x-opencodex-api-key") }; + return new Response(JSON.stringify({ enabled: true, contextWindows: { "claude-x": 200_000 } }), { + headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; + try { + const state = await fetchClaudeCodeState(config, 10100); + expect(state.enabled).toBe(true); + return seen; + } finally { + globalThis.fetch = realFetch; + if (previousAdminToken === undefined) delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + else process.env.OPENCODEX_ADMIN_AUTH_TOKEN = previousAdminToken; + } + } + + test("a hub with a management ingress is asked on the ingress, not the proxy bind", async () => { + const seen = await captureDiscovery(cfg({ + hostname: "100.76.170.81", + runtimeRole: "hub", + hub: { managementIngress: { enabled: true, port: 10102 } }, + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + } as Partial)); + expect(seen.url).toBe("http://127.0.0.1:10102/api/claude-code"); + // The listener's port must NOT be used: it serves no /api/* at all. + expect(seen.url).not.toContain("10104"); + expect(seen.header).toBe(FAKE_ADMIN_TOKEN); + }); + + test("a hub without an ingress falls back to its own public bind", async () => { + const seen = await captureDiscovery(cfg({ + hostname: "100.76.170.81", + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true }, + } as Partial)); + expect(seen.url).toBe("http://100.76.170.81:10100/api/claude-code"); + expect(seen.header).toBe(FAKE_ADMIN_TOKEN); + }); + + test("a loopback or wildcard install keeps asking 127.0.0.1 on the public port", async () => { + for (const hostname of [undefined, "127.0.0.1", "localhost", "0.0.0.0"]) { + const seen = await captureDiscovery(cfg(hostname === undefined ? {} : { hostname })); + const expected = hostname === "localhost" + ? "http://localhost:10100/api/claude-code" + : "http://127.0.0.1:10100/api/claude-code"; + expect({ hostname, url: seen.url }).toEqual({ hostname, url: expected }); + } + }); +}); + describe("ocx claude Windows launch (devlog 260715_cross_platform_audit/020)", () => { test("win32 .cmd shim launches through cmd.exe with preserved arg boundaries", () => { const deps = { diff --git a/tests/claude-integration/claude-gateway-cache.test.ts b/tests/claude-integration/claude-gateway-cache.test.ts index 5d2229f111..f690cda619 100644 --- a/tests/claude-integration/claude-gateway-cache.test.ts +++ b/tests/claude-integration/claude-gateway-cache.test.ts @@ -134,6 +134,41 @@ describe("Claude Code gateway-model cache pre-write (devlog 260712 030)", () => } }); + /** + * The cache is only honored while its `baseUrl` equals the launch's ANTHROPIC_BASE_URL, so + * this writer must resolve the same local destination `buildClaudeEnv` resolves (#4236). + * Three topologies, one answer each. + */ + test("the cached baseUrl follows the unauthenticated loopback listener", async () => { + const cases = [ + { listener: { enabled: true, port: 10104 } as const, expected: "http://127.0.0.1:10104" }, + { listener: { enabled: true } as const, expected: "http://127.0.0.1:10100" }, + { listener: { enabled: false } as const, expected: "http://127.0.0.1:10100" }, + { listener: undefined, expected: "http://127.0.0.1:10100" }, + ]; + for (const { listener, expected } of cases) { + const dir = tempDir(); + let requestedUrl = ""; + const path = await refreshGatewayModelCacheFromProxy(10100, { + configDir: dir, + admissionConfig: listener === undefined ? {} : { unauthenticatedLoopbackListener: listener }, + env: {}, + fetchImpl: async input => { + requestedUrl = String(input); + return new Response(JSON.stringify({ data: [{ id: "claude-ocx-native--x" }] }), { + headers: { "content-type": "application/json" }, + }); + }, + }); + expect({ listener, url: requestedUrl }).toEqual({ + listener, + url: `${expected}/v1/models?limit=1000&ids=cli`, + }); + const body = JSON.parse(readFileSync(path!, "utf8")); + expect({ listener, baseUrl: body.baseUrl }).toEqual({ listener, baseUrl: expected }); + } + }); + test("proxy refresh uses the hardened service token file before a configured key", async () => { const dir = tempDir(); const tokenFile = join(tempDir(), "service-api-token"); diff --git a/tests/clients/desktop-3p.test.ts b/tests/clients/desktop-3p.test.ts index e2e9796e08..c69e7e4199 100644 --- a/tests/clients/desktop-3p.test.ts +++ b/tests/clients/desktop-3p.test.ts @@ -394,6 +394,55 @@ describe("Claude Desktop 3P models", () => { } }); + /** + * Claude Desktop is a LOCAL client (#4236): the gateway base URL it is given must be the + * unauthenticated loopback listener when one is enabled, because on a hub bound to a tailnet + * address `127.0.0.1:` is a closed port. Resolved inside `writeDesktop3pConfig` + * from the config it already re-reads, so every caller gets the same answer. + */ + test("the written gateway base URL follows the unauthenticated loopback listener", () => { + const cases = [ + { listener: { enabled: true, port: 10104 }, expected: "http://127.0.0.1:10104" }, + { listener: { enabled: true }, expected: "http://127.0.0.1:4096" }, + { listener: undefined, expected: "http://127.0.0.1:4096" }, + ] as const; + for (const { listener, expected } of cases) { + const dir = mkdtempSync(join(tmpdir(), "ocx-desktop-listener-")); + const previous = process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = join(dir, "ocx"); + process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR = dir; + try { + saveConfig({ + providers: { test: { adapter: "openai-chat", baseUrl: "http://127.0.0.1:1/v1", allowPrivateNetwork: true, liveModels: false, models: ["fixture-model"] } }, + defaultProvider: "test", + port: 4096, + // A non-loopback bind is what makes the companion form legal at all. + hostname: "100.76.170.81", + runtimeRole: "hub", + ...(listener ? { unauthenticatedLoopbackListener: listener } : {}), + } as OcxConfig); + expect(readConfigDiagnostics().source).toBe("file"); + const written = writeDesktop3pConfig(4096, ["gpt-5.6-sol"], [], "k", "static", undefined, undefined, { + lockPath: join(dir, "locks", "desktop.sqlite"), + }); + expect({ listener, written: written.written }).toEqual({ listener, written: true }); + const profile = JSON.parse(readFileSync(written.path, "utf8")); + const applied = profile[Object.keys(profile)[0]]; + const baseUrl = typeof profile.inferenceGatewayBaseUrl === "string" + ? profile.inferenceGatewayBaseUrl + : applied?.inferenceGatewayBaseUrl; + expect({ listener, baseUrl }).toEqual({ listener, baseUrl: expected }); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; + else process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR = previous; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(dir); + } + } + }); + test("re-applying an owned profile preserves foreign profile keys", () => { const dir = mkdtempSync(join(tmpdir(), "ocx-desktop-merge-")); const previous = process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 1287b981ca..94dc99f8a1 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -633,6 +633,7 @@ "launchd-repair.test.ts": "service", "legacy-shell-compat.test.ts": "responses", "live-service-manager-guard.test.ts": "service", + "local-destinations.test.ts": "lib", "local-management-attestation.test.ts": "server", "local-management-capability.test.ts": "server", "local-management-direct-transport.test.ts": "server", diff --git a/tests/lib/local-destinations.test.ts b/tests/lib/local-destinations.test.ts new file mode 100644 index 0000000000..557eb34526 --- /dev/null +++ b/tests/lib/local-destinations.test.ts @@ -0,0 +1,125 @@ +/** + * The two destination contracts a process on the hub's own machine has (#4236). + * + * The defect this closes: eight local integrations hardcoded `http://127.0.0.1:`, + * an address that does not exist on a hub whose listener binds a tailnet IP. The fix is NOT one + * base URL substituted everywhere (maintainer review on #4236) — management discovery and + * inference are different surfaces with different admission rules, so they get one resolver + * each and these tests hold them apart. + */ +import { describe, expect, test } from "bun:test"; +import { + localInferenceOrigin, + localInferencePort, + localManagementOrigin, +} from "../../src/lib/local-destinations"; +import type { OcxConfig } from "../../src/types"; + +const TAILNET = "100.76.170.81"; +const PUBLIC_PORT = 10_100; + +function hub(extra: Partial = {}): OcxConfig { + return { + port: PUBLIC_PORT, + hostname: TAILNET, + runtimeRole: "hub", + defaultProvider: "openai", + providers: { openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + ...extra, + } as unknown as OcxConfig; +} + +describe("localInferencePort / localInferenceOrigin", () => { + test("a ported listener is the destination; a companion listener is the public port", () => { + expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), PUBLIC_PORT)) + .toBe(10_104); + expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: true } }), PUBLIC_PORT)) + .toBe(PUBLIC_PORT); + }); + + test("no listener, a disabled listener, and no config all keep the public port", () => { + // This is the "nothing changes on a plain loopback or standalone install" guarantee: every + // call site that used to spell `http://127.0.0.1:${port}` gets that exact string back. + expect(localInferencePort(hub(), PUBLIC_PORT)).toBe(PUBLIC_PORT); + expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: false } }), PUBLIC_PORT)) + .toBe(PUBLIC_PORT); + expect(localInferencePort(undefined, PUBLIC_PORT)).toBe(PUBLIC_PORT); + expect(localInferencePort({}, PUBLIC_PORT)).toBe(PUBLIC_PORT); + }); + + test("the origin is always loopback, never the bind address", () => { + // A tailnet or LAN address in a local client's base URL is the #4236 defect in reverse: + // the client would then need an admission credential it has no way to obtain. + for (const listener of [ + undefined, + { enabled: false } as const, + { enabled: true } as const, + { enabled: true, port: 10_104 } as const, + ]) { + const origin = localInferenceOrigin( + hub(listener === undefined ? {} : { unauthenticatedLoopbackListener: listener }), + PUBLIC_PORT, + ); + expect({ listener, host: new URL(origin).hostname }).toEqual({ listener, host: "127.0.0.1" }); + expect({ listener, protocol: new URL(origin).protocol }).toEqual({ listener, protocol: "http:" }); + } + expect(localInferenceOrigin(hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), PUBLIC_PORT)) + .toBe("http://127.0.0.1:10104"); + }); +}); + +describe("localManagementOrigin", () => { + test("a hub with an enabled ingress is asked on the ingress port", () => { + expect(localManagementOrigin( + hub({ hub: { managementIngress: { enabled: true, port: 10_102 } } }), + PUBLIC_PORT, + )).toBe("http://127.0.0.1:10102"); + }); + + test("the loopback listener never answers management, so it is never used here", () => { + // `/api/*` is deliberately absent from that listener's allowlist. Resolving management to + // it would 404 every discovery call while looking like a reachable local port. + const origin = localManagementOrigin( + hub({ + hub: { managementIngress: { enabled: true, port: 10_102 } }, + unauthenticatedLoopbackListener: { enabled: true, port: 10_104 }, + }), + PUBLIC_PORT, + ); + expect(origin).toBe("http://127.0.0.1:10102"); + expect(origin).not.toContain("10104"); + }); + + test("a disabled or absent ingress falls back to the bind address and public port", () => { + expect(localManagementOrigin(hub(), PUBLIC_PORT)).toBe(`http://${TAILNET}:10100`); + expect(localManagementOrigin(hub({ hub: { managementIngress: { enabled: false } } }), PUBLIC_PORT)) + .toBe(`http://${TAILNET}:10100`); + }); + + test("an ingress only counts on a hub, because only a hub binds one", () => { + for (const runtimeRole of [undefined, "standalone", "client"] as const) { + const origin = localManagementOrigin( + hub({ runtimeRole, hostname: "127.0.0.1", hub: { managementIngress: { enabled: true, port: 10_102 } } }), + PUBLIC_PORT, + ); + expect({ runtimeRole, origin }).toEqual({ runtimeRole, origin: "http://127.0.0.1:10100" }); + } + }); + + test("loopback and wildcard binds resolve exactly as the old hardcoded string did", () => { + const cases: Array<[string | undefined, string]> = [ + [undefined, "http://127.0.0.1:10100"], + ["127.0.0.1", "http://127.0.0.1:10100"], + ["0.0.0.0", "http://127.0.0.1:10100"], + ["::", "http://127.0.0.1:10100"], + // A bare IPv6 literal has to be bracketed or the URL is unparseable. + ["fd7a:115c:a1e0::1", "http://[fd7a:115c:a1e0::1]:10100"], + ["localhost", "http://localhost:10100"], + ]; + for (const [hostname, expected] of cases) { + const config = hub({ runtimeRole: "standalone", ...(hostname === undefined ? {} : { hostname }) }); + if (hostname === undefined) delete (config as { hostname?: string }).hostname; + expect({ hostname, origin: localManagementOrigin(config, PUBLIC_PORT) }).toEqual({ hostname, origin: expected }); + } + }); +}); diff --git a/tests/providers/cursor/cursor-integration-status.test.ts b/tests/providers/cursor/cursor-integration-status.test.ts index 6cb18458aa..99c8abd553 100644 --- a/tests/providers/cursor/cursor-integration-status.test.ts +++ b/tests/providers/cursor/cursor-integration-status.test.ts @@ -11,6 +11,7 @@ import { cursorProductJsonCandidates, detectCursorInstalls, type CursorDetectDep import { parseCursorEffortTable, type CursorEffortTable } from "../../../src/integrations/cursor-effort-table"; import { cursorLastSeen, recordCursorSeen, resetCursorSeenForTests } from "../../../src/integrations/cursor-seen"; import { cursorEffortFamily } from "../../../src/server/models-capabilities"; +import { buildCursorIntegrationStatus } from "../../../src/server/management/cursor-integration-routes"; import { startServer } from "../../../src/server"; import type { OcxConfig } from "../../../src/types"; import { SERVER_BUDGET_MS } from "../../helpers/test-budget"; @@ -228,6 +229,37 @@ describe("GET /api/native-integrations/cursor", () => { } }); + /** + * The gateway URL is pasted into Cursor on THIS machine, so it is the local destination + * (#4236): the unauthenticated loopback listener when one is enabled, because on a hub bound + * to a tailnet address nothing answers on 127.0.0.1:. Called directly rather + * than through a server so the three topologies are compared without three binds. + */ + test("the gateway base URL follows the unauthenticated loopback listener", async () => { + const url = new URL("http://127.0.0.1:10100/api/native-integrations/cursor"); + const cases = [ + { listener: { enabled: true, port: 10104 } as const, expected: "http://127.0.0.1:10104/v1" }, + { listener: { enabled: true } as const, expected: "http://127.0.0.1:10100/v1" }, + { listener: undefined, expected: "http://127.0.0.1:10100/v1" }, + ]; + for (const { listener, expected } of cases) { + const config: OcxConfig = { + ...statusConfig(), + port: 10100, + hostname: "100.76.170.81", + runtimeRole: "hub", + ...(listener ? { unauthenticatedLoopbackListener: listener } : {}), + }; + const status = await buildCursorIntegrationStatus( + { config, deps: { readRuntimePort: () => undefined }, url }, + [], + ); + expect({ listener, baseUrl: status.gateway.baseUrl }).toEqual({ listener, baseUrl: expected }); + // The tailnet address must never reach a value a local app dials. + expect(status.gateway.baseUrl).not.toContain("100.76.170.81"); + } + }); + test("reports bundle effort-table provenance and unmatched model families through the server deps seam", async () => { saveConfig(statusConfig()); const server = startServer(0, { managementApi: { loadCursorEffortTable: () => fixtureEffortTable() } }); diff --git a/tests/server/api-access-endpoints.test.ts b/tests/server/api-access-endpoints.test.ts index 820aa2c658..6eead61c3f 100644 --- a/tests/server/api-access-endpoints.test.ts +++ b/tests/server/api-access-endpoints.test.ts @@ -71,6 +71,35 @@ describe("buildApiAccessEndpoints", () => { ).baseUrl).toBe("http://[2001:db8::1]:9999/v1"); }); + /** + * Only the LAST-RESORT loopback fallback honors the unauthenticated loopback listener + * (#4236). Everything above it describes the address the CLIENT reached, so a remote caller + * is never handed a port that only exists on the hub's own 127.0.0.1. + */ + test("the loopback fallback honors the loopback listener; request-derived hosts do not", () => { + const wildcard = { hostname: "0.0.0.0", port: 10100 } as const; + expect(resolveApiAccessBaseUrl({ ...wildcard, unauthenticatedLoopbackListener: { enabled: true, port: 10104 } })) + .toBe("http://127.0.0.1:10104/v1"); + // Companion form: same port as the public listener, so the string does not move. + expect(resolveApiAccessBaseUrl({ ...wildcard, unauthenticatedLoopbackListener: { enabled: true } })) + .toBe("http://127.0.0.1:10100/v1"); + expect(resolveApiAccessBaseUrl({ ...wildcard, unauthenticatedLoopbackListener: { enabled: false } })) + .toBe("http://127.0.0.1:10100/v1"); + + // A real request context still wins, on the port the caller actually used. + expect(resolveApiAccessBaseUrl( + { ...wildcard, unauthenticatedLoopbackListener: { enabled: true, port: 10104 } }, + { requestUrl: "http://192.168.1.50:10100/api/keys" }, + )).toBe("http://192.168.1.50:10100/v1"); + + // And a specific bind is described as itself: a remote client cannot dial 10104 here. + expect(resolveApiAccessBaseUrl({ + hostname: "100.76.170.81", + port: 10100, + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + })).toBe("http://100.76.170.81:10100/v1"); + }); + test("reflects disabled Claude inbound in API access metadata", () => { expect(buildApiAccessEndpoints({ claudeCode: { enabled: false } }).claudeCodeEnabled).toBe(false); }); diff --git a/tests/server/loopback-companion-client-targets.test.ts b/tests/server/loopback-companion-client-targets.test.ts index 2b24fe4350..4988c1e137 100644 --- a/tests/server/loopback-companion-client-targets.test.ts +++ b/tests/server/loopback-companion-client-targets.test.ts @@ -67,14 +67,16 @@ describe("companion hub: sync-managed and hardcoded clients agree", () => { .toBe(`${hardcodedLocalOrigin(HUB_PORT)}/v1`); }); - test("the ported form still splits the two, which is why the companion exists", () => { - // Kept as a regression witness for the issue's table: with a distinct port the sync - // writers move and the hardcoded callers do not, so they disagree by construction. + test("the ported form now agrees too: both writers land on the listener's port", () => { + // PR2 left these two disagreeing here on purpose (the companion form was the answer, and + // the call sites were untouched). #4236's local-client unit closed the gap: `ocx claude` + // resolves the listener's EFFECTIVE port, so a 10104-style hub no longer sends Claude to + // the public port while Codex goes to the listener. const config = hubConfig({ enabled: true, port: 10_104 }); const codexOrigin = new URL(standaloneCodexRoutingTarget(HUB_PORT, config).baseUrl).origin; expect(codexOrigin).toBe(hardcodedLocalOrigin(10_104)); - expect(buildClaudeEnv(config, HUB_PORT, {}).ANTHROPIC_BASE_URL).toBe(hardcodedLocalOrigin(HUB_PORT)); - expect(codexOrigin).not.toBe(hardcodedLocalOrigin(HUB_PORT)); + expect(buildClaudeEnv(config, HUB_PORT, {}).ANTHROPIC_BASE_URL).toBe(hardcodedLocalOrigin(10_104)); + expect(buildClaudeEnv(config, HUB_PORT, {}).ANTHROPIC_BASE_URL).toBe(codexOrigin); }); test("with no listener a hub keeps demanding admission on its public address", () => { diff --git a/tests/server/system-env.test.ts b/tests/server/system-env.test.ts index a8c4175198..bdd122be00 100644 --- a/tests/server/system-env.test.ts +++ b/tests/server/system-env.test.ts @@ -310,6 +310,73 @@ describe("system environment injection", () => { }); }); +/** + * A launchd-started `claude` is a local client (#4236): on a hub bound to a tailnet address the + * only socket on 127.0.0.1 is the unauthenticated loopback listener, so that is the port the + * injected ANTHROPIC_BASE_URL must name — in the launchd domain AND in the shell env file. + * The tracking record keeps the public port separately, because that is the /healthz address. + */ +describe("system environment local destination", () => { + const hubConfig = (listener?: { enabled: boolean; port?: number }): OcxConfig => ({ + ...baseConfig, + hostname: "100.76.170.81", + runtimeRole: "hub", + ...(listener ? { unauthenticatedLoopbackListener: listener } : {}), + } as OcxConfig); + + function shellEnvBody(): string { + const write = writeSpy.mock.calls.find(call => String(call[0]).includes("claude-env.sh")); + return String(write?.[1] ?? ""); + } + + test("a ported listener moves both injected destinations, not the tracked identity", async () => { + expect(await injectSystemEnv(4567, hubConfig({ enabled: true, port: 10104 }))).toEqual({ injected: true }); + expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:10104"); + expect(shellEnvBody()).toContain("export ANTHROPIC_BASE_URL='http://127.0.0.1:10104'"); + // port stays the proxy's identity and liveness address; clientPort records what was injected. + expect(JSON.parse(trackingFile!)).toMatchObject({ port: 4567, clientPort: 10104 }); + }); + + test("the companion form and a plain install are unchanged", async () => { + expect(await injectSystemEnv(4567, hubConfig({ enabled: true }))).toEqual({ injected: true }); + expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:4567"); + // No clientPort is recorded when the two are the same, so old readers see the same file. + expect(JSON.parse(trackingFile!).clientPort).toBeUndefined(); + + execFileSpy.mockClear(); + trackingFile = undefined; + expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); + expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:4567"); + expect(JSON.parse(trackingFile!).clientPort).toBeUndefined(); + }); + + test("revert proves ownership against the injected port, not the tracked one", () => { + trackingFile = JSON.stringify({ + pid: 123, port: 4567, clientPort: 10104, injectedAt: "2026-07-11T00:00:00.000Z", + }); + // What launchd actually holds is the listener's port: that IS ours. + launchctlBaseUrl = "http://127.0.0.1:10104"; + expect(revertSystemEnv()).toEqual({ reverted: true }); + }); + + test("liveness is still probed on the public port, never on the listener", async () => { + // The listener serves no /healthz, so probing it would 404 and revert a LIVE proxy's env. + trackingFile = JSON.stringify({ + pid: 123, port: 4567, clientPort: 10104, injectedAt: "2026-07-11T00:00:00.000Z", + }); + launchctlBaseUrl = "http://127.0.0.1:10104"; + const probed: string[] = []; + globalThis.fetch = mock(async (input: unknown) => { + probed.push(String(input)); + return new Response("ok"); + }) as unknown as typeof fetch; + + expect(await cleanStaleSystemEnv()).toEqual({ cleaned: false, reason: "proxy still alive" }); + expect(probed).toEqual(["http://127.0.0.1:4567/healthz"]); + expect(unlinkSpy).not.toHaveBeenCalled(); + }); +}); + describe("system environment cleanup", () => { test("revertSystemEnv unsets owned variables and deletes the tracking file", () => { trackingFile = tracking(); diff --git a/tests/vision/vision-routed.test.ts b/tests/vision/vision-routed.test.ts index aa36de25cd..f11473b8f9 100644 --- a/tests/vision/vision-routed.test.ts +++ b/tests/vision/vision-routed.test.ts @@ -3,12 +3,14 @@ import { mkdtempSync} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { saveConfig } from "../../src/config"; +import { parseRequest } from "../../src/responses/parser"; import { startServer } from "../../src/server"; -import type { OcxConfig } from "../../src/types"; -import { resetVisionDescriptionCache } from "../../src/vision"; +import type { OcxConfig, OcxProviderConfig } from "../../src/types"; +import { planVisionSidecar, resetVisionDescriptionCache } from "../../src/vision"; import { describeImageRouted, routedDescribeAdmissionToken, + routedDescribeBaseUrl, VISION_DESCRIBE_TERMINAL_HEADER, } from "../../src/vision/routed-describe"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; @@ -88,6 +90,62 @@ describe("describeImageRouted unit", () => { } }); + /** + * The self-fetch is a local client too (#4236): on a hub bound to a tailnet address the only + * socket on 127.0.0.1 is the unauthenticated loopback listener, which now admits the chat + * wire this helper speaks. Three topologies, one destination each. + */ + test("the self-fetch base URL follows the unauthenticated loopback listener", () => { + expect(routedDescribeBaseUrl({ + port: 10100, + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + })).toBe("http://127.0.0.1:10104"); + expect(routedDescribeBaseUrl({ + port: 10100, + unauthenticatedLoopbackListener: { enabled: true }, + })).toBe("http://127.0.0.1:10100"); + expect(routedDescribeBaseUrl({ + port: 10100, + unauthenticatedLoopbackListener: { enabled: false }, + })).toBe("http://127.0.0.1:10100"); + expect(routedDescribeBaseUrl({ port: 10100 })).toBe("http://127.0.0.1:10100"); + }); + + test("the vision plan carries the listener through to the self-fetch", () => { + // The planner hands `describeImageRouted` a NARROWED config. Dropping the listener there + // would leave the resolver nothing to resolve and silently restore the closed port. + const routed: OcxProviderConfig = { + adapter: "openai-chat", + baseUrl: "https://routed.test/v1", + apiKey: "routed-key", + noVisionModels: ["text-model"], + }; + const vlm: OcxProviderConfig = { adapter: "openai-chat", baseUrl: "https://vlm.test/v1", apiKey: "k" }; + const request = parseRequest({ + model: "routed/text-model", + input: [{ + type: "message", + role: "user", + content: [ + { type: "input_text", text: "look at this" }, + { type: "input_image", image_url: PNG_DATA_URL }, + ], + }], + }); + const plan = planVisionSidecar({ + port: 10100, + hostname: "100.76.170.81", + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + defaultProvider: "routed", + providers: { routed, vlm }, + visionSidecar: { enabled: true, backend: "routed", model: "vlm/qwen-vl" }, + } as unknown as OcxConfig, routed, "text-model", request); + expect(plan?.backend).toBe("routed"); + expect(plan?.routedConfig?.unauthenticatedLoopbackListener).toEqual({ enabled: true, port: 10104 }); + expect(routedDescribeBaseUrl(plan!.routedConfig!)).toBe("http://127.0.0.1:10104"); + }); + test("admission ladder: env token first, then first apiKeys entry, as x-opencodex-api-key", () => { expect(routedDescribeAdmissionToken({})).toBeUndefined(); expect(routedDescribeAdmissionToken({ From 83ce2188f6d975ff6a53d8256894d957b5717bd8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:45:11 +0900 Subject: [PATCH 4/8] docs(devlog): record the hub local-clients unit (PR3) What shipped, the two-contract decision and why the allowlist stopped where it did, the three live-config verifications, and the follow-ups left for the docs unit (restart requirement, `count_tokens`, Cursor's apiKeyMode wording). Refs #4236 Co-Authored-By: Claude Fable 5.1 --- .../030_hub_local_clients.md | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 devlog/_plan/260911_hub_single_port/030_hub_local_clients.md diff --git a/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md new file mode 100644 index 0000000000..87175be8b7 --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md @@ -0,0 +1,187 @@ +# 030 — PR3: the hub's own local clients, two destination contracts + +Unit: `devlog/_plan/260911_hub_single_port`. Stack position 3 of 4. Branch +`codex/260911-l4-hub-local-clients`, based on `codex/260911-l4-hub-loopback-companion` +(= `dev` + PR1 launchd repair + PR2 loopback companion). That base gained a review fix +(`0aca6afe4`, hub-gate conjunction + every all-zero hostname spelling) while this unit was being +written, so the branch was rebased onto it; there were no conflicts and every count below is from +the rebased tree. Issue: +lidge-jun/opencodex#4236 — the local-client follow-up table (eight hardcoded +`127.0.0.1:` sites) and the reviewer comment that they are **two** contracts. + +PR2 closed the "Codex works but nothing else does" symptom for the *companion* form by moving the +socket instead of the call sites, and recorded the rest as open work: on a **ported** listener +(`{enabled:true, port:10104}`) the hardcoded callers still dialed the public port, and +`/v1/messages` plus `/api/claude-code` were not served on the listener at all. This unit closes +both halves — separately, because they are not the same surface. + +## The two contracts + +1. **Inference** — `localInferenceOrigin(config, publicPort)`: `http://127.0.0.1:` when `unauthenticatedLoopbackListener` is enabled, otherwise + `http://127.0.0.1:`. That listener admits local callers with no credential, so + nothing has to export one. +2. **Management** — `localManagementOrigin(config, publicPort)`: `http://127.0.0.1:` on a hub with the ingress enabled, otherwise + `http://:`. The caller still sends the admin token; + management authentication has no loopback bypass (`structure/05`), and the unauthenticated + listener serves no `/api/*` — by design, not by omission. + +Both live in `src/lib/local-destinations.ts`, one small module whose header states the split, next +to the existing `local-management-*` helpers. It reuses PR2's `effectiveLoopbackListenerPort` and +`probeHostname`; no call site repeats `?? port`. + +## What shipped + +### 1. Two inference wires on the unauthenticated loopback listener + +`loopbackRouteAllowed` (`src/server/index.ts`) now admits `POST /v1/messages` (Anthropic wire: +`ocx claude`, Claude Desktop, the `system-env` injection) and `POST /v1/chat/completions` (OpenAI +chat wire: Cursor Private Inference, the routed vision helper, aside/opencode). Both handlers +already resolve admission from the RECEIVING listener's `RequestPolicyView` — the same resolver +and the same loopback short-circuit `/v1/responses` uses — so this adds a wire, not a trust level. +The allowlist comment says why, in the shape the existing entries use. + +Nothing else was added: `/api/*`, `/healthz`, `/readyz`, the GUI and +`POST /v1/messages/count_tokens` all still 404 there. + +One consistency fix rode along: the chat-completions branch finished its CORS with `config` +instead of the request's `policy`. On the public listener those are the same object, so this is a +no-op there; on the loopback listener it is the difference between CORS headers that match the +admission decision and headers derived from a bind address that did not receive the request. + +### 2. Eight call sites, one resolver each way + +| Site | Before | After | +| --- | --- | --- | +| `buildClaudeEnv` (`src/cli/claude.ts`) | `http://127.0.0.1:${publicPort}` | `localInferenceOrigin` | +| `fetchClaudeCodeState` | `http://127.0.0.1:${publicPort}/api/claude-code` | `localManagementOrigin` + admin token | +| `writeDesktop3pConfig` → `generateDesktop3pConfig` | public port | `localInferencePort(latest.config, port)` | +| `refreshGatewayModelCacheFromProxy` | public port | `localInferenceOrigin(options.admissionConfig, port)` | +| `injectSystemEnv` / `writeShellEnvFile` | public port | `localInferencePort` / `localInferenceOrigin` | +| Cursor gateway card | `http://127.0.0.1:${port}/v1` | `localInferencePort` | +| `resolveApiAccessBaseUrl` (final loopback fallback only) | `http://127.0.0.1:${port}/v1` | `localInferenceOrigin` | +| `routedDescribeBaseUrl` | public port | `localInferenceOrigin` | + +Three consequences worth naming: + +- **`targetsLocalClaudeProxy` takes a SET of ports.** The public port and the listener's port are + both ours. Treating the one this launch did not pick as a foreign proxy would strip our own + admission token out of the environment and silently downgrade the launch; the stale-replacement + branch and `buildNativeClaudeEnv`'s shedding branch both use the set. +- **The gateway-model cache had to move with `buildClaudeEnv`.** Claude Code honors that file only + while its `baseUrl` equals `ANTHROPIC_BASE_URL`; moving one without the other would have left + the picker on a stale list. +- **`system-env` tracking now records two ports.** `port` stays the owning proxy's identity and + its `/healthz` address; new optional `clientPort` records what was injected. Ownership on revert + is proven against `clientPort ?? port`, liveness is still probed on `port` — the listener serves + no `/healthz`, so probing the injected port would declare a live proxy stale and revert its + environment. Records written before this field are unchanged, and no `clientPort` is written + when the two ports are equal. + +`resolveApiAccessBaseUrl` was touched ONLY in its last-resort loopback branch. Every branch above +it describes the address the *client* reached, and a remote caller must never be handed a port +that exists only on the hub's own 127.0.0.1. + +### 3. Vision plan narrowing + +`planVisionSidecar` hands `describeImageRouted` a narrowed config (`port`, `apiKeys`). The +listener field had to join it, or the resolver would have had nothing to resolve and the +self-fetch would have silently gone back to the closed port. A test pins the narrowing itself. + +## Decisions + +- **The reviewer's constraint is the design, not a caveat.** One base URL substituted everywhere + would have meant either `/api/*` on the unauthenticated listener or an admin token in exported + client configuration. Neither happens: the management resolver never returns the listener's + port, and no exported configuration gained a credential. +- **`count_tokens` is NOT admitted.** Scope said the two wires and nothing else, so + `POST /v1/messages/count_tokens` still 404s on the listener. Claude Code degrades to local + estimation when that route is unavailable, so this is a cosmetic loss rather than a broken + launch — but it is the one obvious follow-up candidate, and a test now pins the current answer + so widening it is a deliberate act. +- **Desktop/Cursor/system-env resolve where the config is, not in the generator.** + `generateDesktop3pConfig` stays a pure generator taking "the local port to dial"; + `writeDesktop3pConfig` resolves from the config it already re-reads under the mutation lock, so + every caller gets the same answer and no caller has to be taught about listeners. +- **Cursor's `apiKeyMode` was left alone.** It still describes the public bind's admission rule. + Pasting a credential into the unauthenticated listener is harmless; omitting one on a bind that + demands it is not. Changing that copy is a GUI decision, not part of this fix. +- **A restart is required for the ported form.** Verified against the live hub: the running + pre-PR3 proxy answers `404` for `POST /v1/messages` on `127.0.0.1:10104` while + `GET /v1/models` is `200`. After this PR the same request is served, so operators on the ported + form must restart (macOS: `launchctl kickstart -k gui/$uid/com.opencodex.proxy`) before + `ocx claude` can use the listener. Noted for the PR4 docs unit. + +## Verification (exact commands, this branch) + +``` +bun run typecheck # clean +bun run privacy:scan # Privacy scan passed +bun test tests/server/loopback-listener-admission.test.ts # 31 pass +bun test tests/server/loopback-listener-integration.test.ts # 35 pass +bun test tests/lib/local-destinations.test.ts # 8 pass +bun test tests/server/loopback-companion-client-targets.test.ts \ + tests/claude-integration/claude-cli.test.ts \ + tests/claude-integration/claude-gateway-cache.test.ts \ + tests/clients/desktop-3p.test.ts # 88 pass +bun test tests/server/system-env.test.ts \ + tests/server/api-access-endpoints.test.ts \ + tests/providers/cursor/cursor-integration-status.test.ts \ + tests/vision/vision-routed.test.ts \ + tests/claude-integration/claude-system-env-auto.test.ts # 61 pass +bun test tests/claude-integration/claude-auth-detect.test.ts \ + tests/claude-integration/claude-auth-mode.test.ts \ + tests/claude-integration/claude-management-api.test.ts \ + tests/claude-integration/claude-shell-hook.test.ts \ + tests/cli/cli-management-auth.test.ts # 110 pass (with the one above) +bun test tests/clients/desktop-3p-guard.test.ts \ + tests/clients/desktop-remote-store.test.ts \ + tests/clients/sync-client-integrations.test.ts \ + tests/codex-integration/model-visibility-management-api.test.ts \ + tests/codex-integration/native-claude-desktop-toggle.test.ts # 92 pass +bun test tests/providers/cursor/cursor-effort-rows.test.ts \ + tests/providers/xai/grok-lifecycle.test.ts \ + tests/server/api-keys-routes.test.ts # 85 pass +bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 17 pass +bun test tests/ci-workflows/docs-remote-hub-claims.test.ts # 7 pass +``` + +New test file registered in `scripts/test-layout/layout.json` and +`tests/fixtures/test-layout-expected.json`: `tests/lib/local-destinations.test.ts` → `lib`. + +Updated PR2's witness case in `tests/server/loopback-companion-client-targets.test.ts`: the +"ported form still splits the two" assertion was the record of the gap this unit closes, and now +asserts the agreement instead. + +No repository-wide suite (operator instruction); hosted CI at exact head is the proof. + +### Live acceptance (read-only, this machine) + +`~/.opencodex/config.json`: `runtimeRole: hub`, `hostname: 127.0.0.1`, `port: 10100`, +`unauthenticatedLoopbackListener: {enabled:true, port:10104}`, +`hub.managementIngress: {enabled:true, port:10102}`. A read-only script (scratchpad, not +committed) loaded that config, probed liveness, and resolved both destinations — no config write, +no restart, no `repair`/`ensure`/`sync`: + +``` +live proxy port: 10100 | source: runtime +resolved management origin: http://127.0.0.1:10102 +resolved inference origin: http://127.0.0.1:10104 +fetchClaudeCodeState enabled: true | windows: 264 +ANTHROPIC_BASE_URL origin: http://127.0.0.1:10104 +``` + +`enabled: true` is the acceptance case: before this unit that call dialed +`127.0.0.1:10100/api/claude-code` — which happens to answer on THIS host because the bind is +loopback, but returns nothing on a tailnet-bound hub, and `ocx claude` then launches native. + +## Left for the rest of the stack + +- PR4 (token UX + `ocx hub invite` + `ocx status` hub block) and the docs/skill unit own the + operator-facing copy. The docs note that matters: restart after changing + `unauthenticatedLoopbackListener`, and the ko copies of + `reference/configuration/server.md` still describe only the ported form (PR2's note). +- `POST /v1/messages/count_tokens` on the listener: decide whether the Anthropic wire should be + complete there. Currently pinned as 404. +- Cursor's `apiKeyMode` wording when the resolved base URL is the unauthenticated listener. From 0e37f38394fc40c5dceed4126f1a98a1b7b4758d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:19:36 +0900 Subject: [PATCH 5/8] fix(lib,claude,server,vision): fall back to the bind address for local inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #4236, should-fix: `localInferenceOrigin` returned `http://127.0.0.1:` whenever `unauthenticatedLoopbackListener` was off, with no bind-address fallback — and `tests/lib/local-destinations.test.ts` pinned that as intended. So the first revision of this unit closed the PORTED listener form and left the reported topology (listener off, `hostname` a tailnet address) pointing all eight call sites at a dead socket, which is the case the issue is actually about. Inference now mirrors `localManagementOrigin`'s shape and returns a struct, `localInferenceDestination` → `{ origin, port, requiresAdmissionToken }`: listener enabled → `127.0.0.1:`, no credential loopback `hostname` → `127.0.0.1:`, no credential anything else → `:`, CREDENTIAL A wildcard bind keeps a loopback origin — it does answer there — but lands in the credential bucket, because the public listener demands admission regardless of which address received the request. A string cannot express that, which is why the resolver stopped returning one: a caller that cannot see `requiresAdmissionToken` cannot tell a free socket from one that will 401. `requiresAdmissionToken` IS `shouldInjectApiAuthHeader`, the predicate that already encoded this question, so the two cannot drift. Each of the eight sites then either attaches the credential or degrades out loud: - `buildClaudeEnv` puts it in the `ANTHROPIC_AUTH_TOKEN` slot it already uses for `ownAdmissionTokens`, and warns when a subscription launch cannot carry one — asserting a host token there logs a claude.ai subscriber out (#253), so the honest outcome is a warning naming the two fixes, not a silent 401. - `injectSystemEnv` / `writeShellEnvFile` inject it, or refuse the whole injection with a reason: the launchd domain is machine-wide, so a base URL that 401s every plain `claude` on the box is worse than no injection. - `writeDesktop3pConfig` writes it as the gateway api key it already accepts. - `refreshGatewayModelCacheFromProxy` and the routed vision self-fetch already sent `x-opencodex-api-key`; they now share one ladder for resolving it. - the Cursor card's `apiKeyMode` keys on the destination it just resolved, so it cannot hand the operator a URL and tell them no key is needed for it. - `resolveApiAccessBaseUrl`'s last-resort branch is wildcard-only, so its string is unchanged. The credential is the DATA-plane one — `OPENCODEX_API_AUTH_TOKEN`, the hardened service token file, then a configured `apiKeys` entry, the same ladder `standaloneCodexRoutingTarget` and the Codex provider table use — shared as `localAdmissionToken`. Never the admin token: no exported client configuration may carry management authority (reviewer constraint on #4236). That helper shape-checks the token it reads from the FILE before sending it as a credential, because a path can be pointed at something that is not a credential at all and putting that in a header leaks file contents; the env var and configured keys pass through verbatim. Two consequences inside `ocx claude`. `targetsLocalClaudeProxy` gained the resolved destination origin as a second way to be ours, or the launch would write a base URL and refuse to recognize it one line later. And the port set became `localLoopbackInferencePorts` — the ports that actually answer on 127.0.0.1 — which is EMPTY on a tailnet bind with no listener, so a leftover `http://127.0.0.1:10100` from a previous loopback-bound install is correctly rewritten instead of preserved as ours. `buildNativeClaudeEnv` keeps a wider set on purpose: shedding asks "could we have written this?", and leaving such a URL behind with its token stripped points a native launch at a dead socket. Two more review findings ride here because they are the same composition bug: - `cleanStaleSystemEnv` probed `127.0.0.1:`, which does not exist on a tailnet-bound hub, so the record was reverted on every start and the "another instance owns env" guard could never fire — while the comment and test asserted the opposite. The tracking record now carries `bindHost` (validated on read, it reaches a fetch URL) and `clientBaseUrl` in place of `clientPort`; liveness probes the bind host and the public port, ownership is proven against what was injected. Both fields are omitted when they add nothing to `port`, so a plain loopback install writes a byte-identical record. - `probeHostname` and `api-access.ts`'s `isWildcardBindHost` each knew three wildcard spellings while `isWildcardHostname` (PR2) knew every all-zero form, so `0.0.0.0.`, `::0` and `*` were composed into literal URLs that resolve to nothing. Both call the shared predicate now. - nit: `Number(configuredPort())` is `0` when `_corsOrigin` carries no explicit port, so `routedDescribeBaseUrl` could compose `http://127.0.0.1:0`. `localInferencePort` was removed rather than kept as a wrapper: a bare port cannot express a bind-address destination, so an export that returns one is a trap. `planVisionSidecar`'s narrowed config gained `hostname` for the same reason the listener field was added to it. The test table is the review's six configurations — standalone loopback, companion hub, ported hub, listener-off + non-loopback bind, wildcard bind, client role — driving both the destination and the port-set assertions, so a new branch that forgets the flag fails there rather than in production. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- src/claude/desktop-3p.ts | 39 ++- src/claude/gateway-cache.ts | 29 +-- src/cli/claude.ts | 84 +++++-- src/lib/local-destinations.ts | 134 +++++++++-- src/server/management/api-access.ts | 12 +- .../management/cursor-integration-routes.ts | 21 +- src/server/proxy-liveness.ts | 8 +- src/server/system-env-shell.ts | 20 +- src/server/system-env.ts | 134 ++++++++--- src/vision/plan.ts | 7 +- src/vision/routed-describe.ts | 74 ++++-- tests/claude-integration/claude-cli.test.ts | 116 +++++++-- tests/clients/desktop-3p.test.ts | 25 +- tests/lib/local-destinations.test.ts | 223 +++++++++++++++--- .../cursor/cursor-integration-status.test.ts | 35 ++- tests/server/api-access-endpoints.test.ts | 11 +- .../loopback-companion-client-targets.test.ts | 22 ++ tests/server/system-env.test.ts | 113 ++++++++- tests/vision/vision-routed.test.ts | 38 ++- 19 files changed, 912 insertions(+), 233 deletions(-) diff --git a/src/claude/desktop-3p.ts b/src/claude/desktop-3p.ts index 102426c5cf..ec50f765ab 100644 --- a/src/claude/desktop-3p.ts +++ b/src/claude/desktop-3p.ts @@ -22,7 +22,7 @@ import { type DesktopProfileModel, } from "./desktop-profile"; import { nativeOpenAiContextWindow, type NativeContextLimitsInput } from "../codex/catalog"; -import { localInferencePort } from "../lib/local-destinations"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; import { assertDesktop3pModelsValid } from "./desktop-3p-guard"; export interface Desktop3pModelEntry { @@ -324,11 +324,13 @@ export function activeDesktop3pAlias(provider: string, modelId: string): string * discovery stays off for determinism. supports1m makes Desktop offer a separate 1M * row; selecting it sends the bare id + `anthropic-beta: context-1m-2025-08-07`. * - * `port` is the LOCAL port Desktop should dial, already resolved by the caller (see - * `writeDesktop3pConfig`): on a hub that is the unauthenticated loopback listener's port. + * `portOrOrigin` is the LOCAL destination Desktop should dial, already resolved by the caller + * (see `writeDesktop3pConfig`): on a hub that is the unauthenticated loopback listener, and with + * no listener the bind address — which is why an ORIGIN is accepted and not only a port. A bare + * port keeps meaning `http://127.0.0.1:`, so every existing caller and test is unchanged. */ export function generateDesktop3pConfig( - port: number, + portOrOrigin: number | string, nativeSlugs: string[], routedModels: Array, apiKey = "ocx", @@ -339,7 +341,7 @@ export function generateDesktop3pConfig( const base = { inferenceProvider: "gateway", inferenceCredentialKind: "static", - inferenceGatewayBaseUrl: `http://127.0.0.1:${port}`, + inferenceGatewayBaseUrl: typeof portOrOrigin === "number" ? `http://127.0.0.1:${portOrOrigin}` : portOrOrigin, inferenceGatewayApiKey: apiKey, }; if (mode === "discovery") { @@ -625,12 +627,29 @@ export function writeDesktop3pConfig( return { written: false, path: resolveDesktop3pConfigLibraryPath(), reason: "desktop_remote_store_active" }; } // Claude Desktop runs on this machine, so it dials the unauthenticated loopback listener - // when one is enabled — on a tailnet-bound hub that is the only local socket (#4236). - // Resolved here, from the config this write already re-read, rather than in the pure - // generator: `latest.config` is the freshest answer any caller could pass in. - const localPort = localInferencePort(latest.config, port); + // when one is enabled — on such a hub that is the only credential-free local socket + // (#4236) — and otherwise the bind address, which answers but demands data-plane + // admission. Resolved here, from the config this write already re-read, rather than in + // the pure generator: `latest.config` is the freshest answer any caller could pass in. + const destination = localInferenceDestination(latest.config, port); + // Desktop can carry a credential, so it does: the key the caller passed (the first + // configured `apiKeys` entry), else the env token / hardened service token file. This is + // the DATA-PLANE secret only — an admin token must never enter an exported client + // configuration (reviewer constraint on #4236). + const gatewayKey = destination.requiresAdmissionToken + ? apiKey ?? localAdmissionToken(latest.config) + : apiKey; + if (destination.requiresAdmissionToken && !gatewayKey) { + // The placeholder the generator defaults to would 401 on this bind. Write the profile + // anyway — a reachable URL with a visible auth failure beats a dead socket — but say so. + console.error( + `⚠ Claude Desktop will dial ${destination.origin}, which requires an opencodex data-plane ` + + "credential that could not be resolved. Configure an API key or enable " + + "`unauthenticatedLoopbackListener`.", + ); + } return writeDesktop3pConfigWithGenerator(() => ( - generateDesktop3pConfig(localPort, nativeSlugs, routedModels, apiKey, mode, profile, nativeContextCap) + generateDesktop3pConfig(destination.origin, nativeSlugs, routedModels, gatewayKey, mode, profile, nativeContextCap) )); }), lifecycleLockDeps); } catch { return { written: false, path: resolveDesktop3pConfigLibraryPath(), reason: "desktop_lifecycle_busy_or_unsafe" }; } diff --git a/src/claude/gateway-cache.ts b/src/claude/gateway-cache.ts index 4e0d70cc3c..366017a04b 100644 --- a/src/claude/gateway-cache.ts +++ b/src/claude/gateway-cache.ts @@ -13,8 +13,7 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; -import { localInferenceOrigin } from "../lib/local-destinations"; -import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "../lib/service-secrets"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; import type { OcxConfig } from "../types"; export interface GatewayModelRow { @@ -30,7 +29,7 @@ export interface GatewayModelCacheRefreshOptions { * equal the `ANTHROPIC_BASE_URL` the CLI is launched with or Claude Code ignores the whole * cache, so this has to resolve the same loopback listener `buildClaudeEnv` resolves (#4236). */ - admissionConfig?: Pick; + admissionConfig?: Pick; env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; } @@ -66,19 +65,6 @@ export function writeGatewayModelCache(baseUrl: string, models: readonly Gateway } } -/** - * Hardened service-token file, the same precedence `ocx opencode` uses. A service - * install writes the admission token to disk rather than the interactive environment, - * so an interactive `ocx claude` with neither env token nor configured key would - * otherwise still get a 401 and keep a stale picker list. - */ -function serviceFileToken(env: NodeJS.ProcessEnv): string | null { - const lookup = env.OCX_API_TOKEN_FILE?.trim() - ? env - : { ...env, OCX_API_TOKEN_FILE: serviceApiTokenFilePath() }; - return loadServiceTokenFromFile(lookup as Record); -} - /** Fetch the anthropic-flavor /v1/models from the local proxy and write the cache. */ export async function refreshGatewayModelCacheFromProxy( port: number, @@ -98,17 +84,16 @@ export async function refreshGatewayModelCacheFromProxy( // request sent to its local 127.0.0.1 address. Reuse the same dedicated // credential domain as /v1/models admission; never place it in Authorization, // which can belong to an upstream provider on other data-plane surfaces. - const envToken = (options.env ?? process.env).OPENCODEX_API_AUTH_TOKEN?.trim(); - const configuredToken = options.admissionConfig?.apiKeys - ?.find(entry => entry.key.trim().length > 0) - ?.key.trim(); + // Env token, then the hardened service token file (a service install writes the admission + // token to disk rather than the interactive environment), then a configured key — one + // shared ladder, so this cannot drift from what `buildClaudeEnv` puts in the launch env. const admissionToken = typeof portOrTarget === "number" - ? envToken || serviceFileToken(options.env ?? process.env) || configuredToken + ? localAdmissionToken(options.admissionConfig, options.env ?? process.env) : portOrTarget.admissionToken; if (admissionToken) headers.set("x-opencodex-api-key", admissionToken); const baseUrl = typeof portOrTarget === "number" - ? localInferenceOrigin(options.admissionConfig, portOrTarget) + ? localInferenceDestination(options.admissionConfig, portOrTarget).origin : new URL(portOrTarget.baseUrl).origin; // ?ids=cli pins the readable claude-ocx id family deterministically (audit 051 diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 50a0379b2c..03261c016a 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -18,7 +18,7 @@ import { isProxyAdmissionSecret } from "../server/auth-cors"; import { findLiveProxy } from "../server/proxy-liveness"; import type { OcxConfig } from "../types"; import { configuredAdminToken } from "../lib/admin-secrets"; -import { localInferenceOrigin, localInferencePort, localManagementOrigin } from "../lib/local-destinations"; +import { localAdmissionToken, localInferenceDestination, localLoopbackInferencePorts, localManagementOrigin } from "../lib/local-destinations"; import { PROXY_MARKER, ownAdmissionTokens, defaultAuthDetectDeps, detectClaudeAuth, type AuthDetectDeps } from "../claude/auth-detect"; import { resolveClaudeAuthMode } from "../claude/auth-mode"; import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; @@ -104,23 +104,33 @@ function isClaudeLoopbackHostname(hostname: string): boolean { } /** - * Is this loopback base URL one of OURS? + * Is this base URL one of OURS? * - * "Ours" is a SET of ports, not one port (#4236): on a hub with an unauthenticated loopback - * listener the public port and the listener's port are both local addresses this proxy answers - * on, so a URL naming either of them was written by us. Treating the one this launch did not - * pick as a foreign proxy would strip our own admission token out of the environment. + * Two ways to be ours (#4236), because a hub has two shapes of local destination: + * + * - a SET of loopback ports, not one port: with an unauthenticated loopback listener the + * public port and the listener's port are both addresses this proxy answers on at + * 127.0.0.1, so a URL naming either of them was written by us. Treating the one this launch + * did not pick as a foreign proxy would strip our own admission token out of the + * environment. On a tailnet bind with no listener that set is EMPTY, so a leftover + * `http://127.0.0.1:` is correctly seen as stale rather than as ours. + * - the resolved destination origin itself, which on such a bind is the bind address. Without + * this arm the launch would write a base URL and then refuse to recognize it one line later. */ -function targetsLocalClaudeProxy(value: string | undefined, ports: readonly number[]): boolean { +function targetsLocalClaudeProxy( + value: string | undefined, + ports: readonly number[], + ownOrigin?: string, +): boolean { if (!value) return false; try { const parsed = new URL(value); + if (parsed.username !== "" || parsed.password !== "") return false; + if (ownOrigin !== undefined && parsed.origin === ownOrigin) return true; const effectivePort = parsed.port === "" ? 80 : Number(parsed.port); return parsed.protocol === "http:" && isClaudeLoopbackHostname(parsed.hostname) - && ports.includes(effectivePort) - && parsed.username === "" - && parsed.password === ""; + && ports.includes(effectivePort); } catch { return false; } @@ -157,12 +167,15 @@ export function buildClaudeEnv( const explicitTarget = typeof portOrTarget === "number" ? null : portOrTarget; const port = typeof portOrTarget === "number" ? portOrTarget : null; // A local launch dials the unauthenticated loopback listener whenever one is enabled — the - // only local socket a tailnet-bound hub has (#4236). Unchanged on every other topology. + // only credential-free local socket a tailnet-bound hub has (#4236). With the listener OFF + // the destination is the BIND address, which is reachable but demands data-plane admission; + // the resolver says which of the two this is instead of every caller guessing. + const destination = port === null ? null : localInferenceDestination(config, port); const managedBaseUrl = explicitTarget ? new URL(explicitTarget.baseUrl).origin - : localInferenceOrigin(config, port!); - // Every local port this proxy answers on, so a base URL naming either one is still ours. - const ownLocalPorts = port === null ? [] : [...new Set([port, localInferencePort(config, port)])]; + : destination!.origin; + // Every port this proxy answers on at 127.0.0.1, so a base URL naming any of them is ours. + const ownLocalPorts = port === null ? [] : localLoopbackInferencePorts(config, port); const env: ClaudeLaunchEnv = { ...base }; // Step 1 — strip OUR OWN dummy from the inherited environment before anything reads // or writes the token slot. setDefault below preserves any non-empty value, so a @@ -208,7 +221,8 @@ export function buildClaudeEnv( // destination we wrote ourselves. if (parsed.protocol === "http:" && isClaudeLoopbackHostname(parsed.hostname) - && !ownLocalPorts.includes(effectivePort)) { + && !ownLocalPorts.includes(effectivePort) + && parsed.origin !== managedBaseUrl) { const replacement = managedBaseUrl; console.error(`⚠ Replacing stale opencodex ANTHROPIC_BASE_URL ${parsed.origin} with ${replacement}.`); env.ANTHROPIC_BASE_URL = replacement; @@ -234,10 +248,19 @@ export function buildClaudeEnv( // the user's Claude login. Resolve the mode before adding any proxy-owned credential: // subscription launches must keep their OAuth, while proxy launches may use the // admission key or dummy marker (see server/claude-messages.ts). - const ownTokens = explicitTarget ? [explicitTarget.admissionToken] : ownAdmissionTokens(config); + // A bind that demands admission needs a credential the machine can actually present, which + // is wider than `config.apiKeys`: the service installs its data-plane secret as + // `OPENCODEX_API_AUTH_TOKEN` / the hardened token file, and that is the ladder the Codex + // provider table already uses. Never the admin token (reviewer constraint on #4236). + const hostAdmissionToken = destination?.requiresAdmissionToken === true + ? localAdmissionToken(config) + : undefined; + const ownTokens = explicitTarget + ? [explicitTarget.admissionToken] + : [...new Set([...(hostAdmissionToken ? [hostAdmissionToken] : []), ...ownAdmissionTokens(config)])]; const targetsLocalProxy = explicitTarget ? targetsClaudeRoutingTarget(env.ANTHROPIC_BASE_URL, explicitTarget) - : targetsLocalClaudeProxy(env.ANTHROPIC_BASE_URL, ownLocalPorts); + : targetsLocalClaudeProxy(env.ANTHROPIC_BASE_URL, ownLocalPorts, managedBaseUrl); const isOwnAdmissionToken = (value: string): boolean => ownTokens.includes(value) || isProxyAdmissionSecret(value, config); const inheritedApiKey = env.ANTHROPIC_API_KEY; @@ -283,6 +306,19 @@ export function buildClaudeEnv( if (!env.ANTHROPIC_AUTH_TOKEN && !hasUserApiKey && targetsLocalProxy && resolved.markerMode === "proxy") { env.ANTHROPIC_AUTH_TOKEN = PROXY_MARKER; } + // Degrade out loud rather than hand Claude Code a destination that 401s (#4236). A + // subscription launch deliberately carries no host token — asserting one logs a claude.ai + // subscriber out (#253) — so on a bind that demands admission the honest outcome is a + // warning naming the two fixes, not a silent refusal at the first request. + if (destination?.requiresAdmissionToken === true && targetsLocalProxy) { + const carried = env.ANTHROPIC_AUTH_TOKEN?.trim(); + if (!hasUserApiKey && (!carried || carried === PROXY_MARKER)) { + console.error( + `⚠ ${managedBaseUrl} requires an opencodex data-plane credential and this launch carries none — ` + + "requests will be refused. Enable `unauthenticatedLoopbackListener` or bind the proxy to loopback.", + ); + } + } const finalAuthToken = env.ANTHROPIC_AUTH_TOKEN; const hostOwnsAuthentication = targetsLocalProxy && !hasUserApiKey @@ -549,10 +585,16 @@ export function buildNativeClaudeEnv( return Boolean(value && (value === PROXY_MARKER || isProxyAdmissionSecret(value, config))); }); const baseUrl = env.ANTHROPIC_BASE_URL; - // Both local ports count as ours here too: a native launch must shed the managed destination - // whichever of them this machine's last proxy launch wrote (#4236). - const nativeLocalPorts = [...new Set([config.port, localInferencePort(config, config.port)])]; - if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, nativeLocalPorts)) { + // Shedding asks a DIFFERENT question than the stale-replacement branch above, so it uses a + // wider set (#4236). There the question is "is this inherited URL a live destination of + // ours?" and a port nothing answers on must be rewritten. Here it is "could we have written + // this?" — and the answer is yes for the public port on any topology, because an earlier + // config on this machine may have been loopback-bound. Leaving such a URL in place with its + // admission token stripped (the loop below always strips it) would point a native launch at a + // dead socket with no credential, which is strictly worse than shedding one port too many. + const nativeLocalPorts = [...new Set([config.port, ...localLoopbackInferencePorts(config, config.port)])]; + const nativeOwnOrigin = localInferenceDestination(config, config.port).origin; + if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, nativeLocalPorts, nativeOwnOrigin)) { delete env.ANTHROPIC_BASE_URL; } for (const name of admissionSlots) { diff --git a/src/lib/local-destinations.ts b/src/lib/local-destinations.ts index 54220a3e29..56e9b3c056 100644 --- a/src/lib/local-destinations.ts +++ b/src/lib/local-destinations.ts @@ -9,42 +9,138 @@ * `hub.managementIngress`. Callers must still send a management credential: management * authentication has no loopback bypass (structure/05), and the unauthenticated loopback * listener deliberately does not serve `/api/*` at all. - * 2. `localInferenceOrigin` — the data plane a client wire actually speaks. When - * `unauthenticatedLoopbackListener` is enabled this is the listener's effective port, which - * admits local callers with no credential, so nothing has to export one. + * 2. `localInferenceDestination` — the data plane a client wire actually speaks. * - * A hub whose public listener binds a tailnet address has no `127.0.0.1:` socket, - * which is why eight call sites hardcoding that origin all failed while Codex (which already - * honored the listener) worked. They route through here instead of repeating `?? port`: the day - * the resolution changes, a forgotten site points a client config at a closed socket. + * Both resolvers have the SAME three-branch shape, because the bind address is the thing that + * decides. The first review of this module got that wrong for inference: it returned + * `http://127.0.0.1:` unconditionally whenever the loopback listener was off, so a + * hub with `hostname: ` and no listener handed all eight call sites a socket that + * does not exist. The bind address has to be the fallback, exactly as it already was for + * management: + * + * 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 + * + * 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 + * request — so it lands in the same credential bucket as a tailnet bind. That is why the + * resolver returns a STRUCT rather than a string: a caller that cannot see + * `requiresAdmissionToken` cannot tell a free socket from one that will 401, and the only + * honest answers are "attach the data-plane credential" or "say so in a log line". + * + * The credential in question is the DATA-PLANE one — `OPENCODEX_API_AUTH_TOKEN`, the hardened + * service token file, or a configured `apiKeys` entry, the same ladder + * `standaloneCodexRoutingTarget` / the Codex provider table already uses. Never the admin + * token: no exported client configuration may carry management authority (reviewer constraint + * on #4236). */ -import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; +import { effectiveLoopbackListenerPort, isLoopbackHostname, isWildcardHostname, shouldInjectApiAuthHeader } from "../codex/loopback-target"; import { probeHostname } from "../server/proxy-liveness"; +import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "./service-secrets"; import type { OcxConfig } from "../types"; -export type LocalInferenceConfig = Pick; +export type LocalInferenceConfig = Pick; export type LocalManagementConfig = Pick; +export interface LocalInferenceDestination { + /** Origin a local client wire dials, e.g. `http://127.0.0.1:10104`. */ + origin: string; + /** Port component of `origin`. */ + port: number; + /** + * Does the listener at `origin` demand `x-opencodex-api-key`? + * + * False only for the unauthenticated loopback listener and a genuinely loopback public bind. + * A caller that cannot attach a credential must log that it is degrading rather than write a + * destination that answers 401. + */ + requiresAdmissionToken: boolean; +} + /** - * The port a local client dials for inference: the unauthenticated loopback listener's - * effective port when it is enabled, otherwise the public port (unchanged behaviour for a - * loopback or standalone install). + * The one answer for "where does a local client send inference, and does it need a key?". + * + * Kept in one place so the day the resolution changes a forgotten site cannot point a client + * config at a closed socket — which is precisely the defect this module exists to close. */ -export function localInferencePort( +export function localInferenceDestination( config: LocalInferenceConfig | undefined, publicPort: number, -): number { - return effectiveLoopbackListenerPort(config, publicPort) ?? publicPort; +): LocalInferenceDestination { + const listenerPort = effectiveLoopbackListenerPort(config, publicPort); + if (listenerPort !== null) { + // Always bound to 127.0.0.1 and admits local callers with no credential, which is what + // lets an exported client configuration stay credential-free. + return { origin: `http://127.0.0.1:${listenerPort}`, port: listenerPort, requiresAdmissionToken: false }; + } + const hostname = config?.hostname; + // A loopback bind keeps the byte-identical string every call site wrote before this module + // existed, including the `localhost`/`::1` spellings that `probeHostname` would preserve. + if (isLoopbackHostname(hostname)) { + return { origin: `http://127.0.0.1:${publicPort}`, port: publicPort, requiresAdmissionToken: false }; + } + // `probeHostname` turns every all-zero spelling into 127.0.0.1 and brackets a bare IPv6 + // 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}`, + port: publicPort, + requiresAdmissionToken: shouldInjectApiAuthHeader(config), + }; } -/** `http://127.0.0.1:` — the origin every local client wire writes. */ -export function localInferenceOrigin( +/** + * Every port this proxy's data plane answers on at 127.0.0.1 — the set an inherited + * `http://127.0.0.1:` base URL must hit to count as one of OURS. + * + * On a tailnet bind with no loopback listener the set is EMPTY, and that is the point: a + * leftover `http://127.0.0.1:10100` from a previous loopback-bound install is a dead socket + * there, so `ocx claude` must replace it rather than preserve it as its own destination. + */ +export function localLoopbackInferencePorts( config: LocalInferenceConfig | undefined, publicPort: number, -): string { - return `http://127.0.0.1:${localInferencePort(config, publicPort)}`; +): number[] { + const ports: number[] = []; + // A wildcard bind owns loopback on the public port too, so a URL naming it is still ours. + if (isLoopbackHostname(config?.hostname) || isWildcardHostname(config?.hostname)) ports.push(publicPort); + const listenerPort = effectiveLoopbackListenerPort(config, publicPort); + if (listenerPort !== null && !ports.includes(listenerPort)) ports.push(listenerPort); + return ports; +} + +/** + * The DATA-PLANE admission credential this host can present to its own public listener. + * + * Same ladder the Codex provider table and `ocx opencode` already use — environment token, + * hardened service token file, first configured `apiKeys` entry — and deliberately NOT the + * admin token, which must never leave the management surface. `undefined` means the caller has + * nothing to attach and has to degrade loudly. + */ +export function localAdmissionToken( + config: Pick | undefined, + env: NodeJS.ProcessEnv = process.env, +): string | undefined { + const envToken = env.OPENCODEX_API_AUTH_TOKEN?.trim(); + if (envToken) return envToken; + const lookup = env.OCX_API_TOKEN_FILE?.trim() + ? env + : { ...env, OCX_API_TOKEN_FILE: serviceApiTokenFilePath() }; + const fileToken = loadServiceTokenFromFile(lookup as Record)?.trim(); + // Shape-check the FILE candidate only. The env var and `apiKeys` are values an operator set + // deliberately and pass through verbatim; a path, by contrast, can be pointed at or replaced + // by something that is not a credential at all, and sending that as one leaks file contents + // into a request header. Configured keys are never checked, so no existing key can be broken. + if (fileToken && ADMISSION_TOKEN_SHAPE.test(fileToken)) return fileToken; + const configured = config?.apiKeys?.find(entry => entry.key.trim().length > 0)?.key.trim(); + return configured || undefined; } +/** An admission credential is an opaque printable token — never JSON, a path, or multi-line. */ +const ADMISSION_TOKEN_SHAPE = /^[A-Za-z0-9._~+/=-]{8,4096}$/; + /** * The origin a local CLI dials for `/api/*`. * diff --git a/src/server/management/api-access.ts b/src/server/management/api-access.ts index 2bd0fa3fb4..44336919ca 100644 --- a/src/server/management/api-access.ts +++ b/src/server/management/api-access.ts @@ -1,5 +1,6 @@ import type { OcxConfig } from "../../types"; -import { localInferenceOrigin } from "../../lib/local-destinations"; +import { isWildcardHostname } from "../../codex/loopback-target"; +import { localInferenceDestination } from "../../lib/local-destinations"; import { probeHostname } from "../proxy-liveness"; export interface ApiAccessEndpoints { @@ -22,9 +23,14 @@ export type BuildApiAccessEndpointsOptions = { requestOrigin?: string | null; }; +/** + * Wildcard bind scope, shared with `probeHostname` and the loopback-companion gate rather than + * re-spelled here: a third list of three spellings is how `0.0.0.0.` and `::0` ended up treated + * as specific bind addresses on one side and wildcards on the other. + */ function isWildcardBindHost(hostname: string | undefined): boolean { const trimmed = (hostname ?? "").trim(); - return !trimmed || trimmed === "0.0.0.0" || trimmed === "::" || trimmed === "[::]"; + return !trimmed || isWildcardHostname(trimmed); } /** Bracket bare IPv6 literals for URL authority composition. */ @@ -109,7 +115,7 @@ export function resolveApiAccessBaseUrl( // name is loopback — and on that address the unauthenticated loopback listener, when one is // enabled, is the port a local caller should use (#4236). The branches above are unchanged: // a specific bind or a real request host still describes the address the CLIENT reached. - return `${localInferenceOrigin(config, port)}/v1`; + return `${localInferenceDestination(config, port).origin}/v1`; } /** @deprecated Prefer resolveApiAccessBaseUrl; retained for focused host-format tests. */ diff --git a/src/server/management/cursor-integration-routes.ts b/src/server/management/cursor-integration-routes.ts index 9724523a9d..ed428f86d7 100644 --- a/src/server/management/cursor-integration-routes.ts +++ b/src/server/management/cursor-integration-routes.ts @@ -14,7 +14,7 @@ import { cursorLastSeen, type CursorSeen } from "../../integrations/cursor-seen" import { detectCursorInstalls, type CursorInstall } from "../../integrations/cursor-detect"; import { loadCursorEffortTable } from "../../integrations/cursor-effort-table"; import { configuredApiAuthToken, isApiAuthRequired, jsonResponse } from "../auth-cors"; -import { localInferencePort } from "../../lib/local-destinations"; +import { localInferenceDestination } from "../../lib/local-destinations"; import { fetchAllModels } from "../management-api"; import { predictCursorEffort } from "../models-capabilities"; import { expandCursorEffortRow, knownEffortRowIds } from "../effort-row"; @@ -56,15 +56,18 @@ export async function buildCursorIntegrationStatus( // runtime record and config.port are fallbacks for a request that carries no port. const port = runtime?.port ?? (Number(ctx.url?.port) || config.port); // Cursor runs on this machine, so the gateway URL it is told to paste is the LOCAL one: the - // unauthenticated loopback listener when one is enabled (on a tailnet-bound hub there is no - // other local socket), otherwise 127.0.0.1 on the public port exactly as before (#4236). - const gatewayPort = localInferencePort(config, port ?? 10100); - // apiKeyMode still describes the public bind's admission rule: a key is never required by the - // loopback listener, but pasting one there is harmless, while omitting one on a bind that - // demands it is not. + // unauthenticated loopback listener when one is enabled, and otherwise the bind address on the + // public port — 127.0.0.1 for a loopback or wildcard bind exactly as before, and the tailnet + // or LAN address on a hub, where no loopback socket exists to paste (#4236). + const gateway = localInferenceDestination(config, port ?? 10100); + // apiKeyMode describes the admission rule of the destination just resolved, which on the + // loopback listener is "no key needed" and on every other form is "a key is required". + // Pasting one into the listener is harmless; omitting one on a bind that demands it is not. const credentialConfigured = !!configuredApiAuthToken(config) || (config.apiKeys ?? []).some(entry => !!entry.key.trim()); - const apiKeyMode = isApiAuthRequired(config) || credentialConfigured ? "credential" : "placeholder"; + const apiKeyMode = gateway.requiresAdmissionToken || isApiAuthRequired(config) || credentialConfigured + ? "credential" + : "placeholder"; const limits = nativeContextLimits(config); // Same visibility rules as the raw /v1/models list Cursor will read: disabled models and @@ -113,7 +116,7 @@ export async function buildCursorIntegrationStatus( }, regularCursor: { installed: regular !== undefined, path: regular?.path ?? null }, gateway: { - baseUrl: `http://127.0.0.1:${gatewayPort}/v1`, + baseUrl: `${gateway.origin}/v1`, apiKeyMode, placeholder: CURSOR_GATEWAY_PLACEHOLDER_KEY, }, diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index df7d8d7281..045e4cddf7 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -10,6 +10,7 @@ * Lives outside cli.ts (which dispatches argv at module top level) so tests can import it. */ import { loadConfig } from "../config"; +import { isWildcardHostname } from "../codex/loopback-target"; import { readAlivePid, readRuntimePort, verifyPidIdentity } from "../config/process-state"; import { directLocalHttpFetch } from "./direct-local-http"; @@ -82,10 +83,15 @@ export interface LiveProxy { /** * Host to probe for a given bind hostname: wildcards answer on IPv4 loopback, and raw * IPv6 addresses must be bracketed or the composed URL is invalid. + * + * The wildcard test is `isWildcardHostname`, not a list of spellings. This function used to + * know exactly three (`0.0.0.0`, `::`, `[::]`) while the bind-scope predicate knew every + * all-zero form, so `ocx` composed `http://0.0.0.0.:10100` or `http://*:10100` — unreachable + * URLs — for a config the server itself treated as a wildcard bind. One predicate, both sides. */ export function probeHostname(hostname: string | undefined): string { const trimmed = (hostname ?? "").trim(); - if (!trimmed || trimmed === "0.0.0.0" || trimmed === "::" || trimmed === "[::]") return "127.0.0.1"; + if (!trimmed || isWildcardHostname(trimmed)) return "127.0.0.1"; if (trimmed.startsWith("[") && trimmed.endsWith("]")) return trimmed; return trimmed.includes(":") ? `[${trimmed}]` : trimmed; } diff --git a/src/server/system-env-shell.ts b/src/server/system-env-shell.ts index 21b7d5d638..3461428fab 100644 --- a/src/server/system-env-shell.ts +++ b/src/server/system-env-shell.ts @@ -7,7 +7,7 @@ import { resolveClaudeAuthMode } from "../claude/auth-mode"; import { ANTHROPIC_PARENT_ENV_SLOTS, trustedNodeLauncherContext, type AnthropicParentEnvSlot } from "../cli/launcher-context"; import type { OcxConfig } from "../types"; import { recordOwnedConfigPath } from "../lib/config-ownership"; -import { localInferenceOrigin } from "../lib/local-destinations"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; /** * Does the opencodex dummy marker belong in the system environment? @@ -80,12 +80,14 @@ export function writeShellEnvFile( auto?: AutoContextMode, deps: SystemEnvDeps = {}, ): void { - // Same local destination the launchd domain gets: the unauthenticated loopback listener when - // one is enabled, otherwise the public port (#4236). The two files must not disagree, or a - // new shell and a launchd-started `claude` would dial different sockets. + // Same local destination the launchd domain gets, resolved through the same resolver rather + // than re-derived: the unauthenticated loopback listener when one is enabled, otherwise the + // bind address (#4236). The two files must not disagree, or a new shell and a launchd-started + // `claude` would dial different sockets. + const destination = localInferenceDestination(config, port); const lines = [ `# Generated by opencodex — do not edit manually`, - `export ANTHROPIC_BASE_URL=${shellValue(localInferenceOrigin(config, port))}`, + `export ANTHROPIC_BASE_URL=${shellValue(destination.origin)}`, `export CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=${shellValue("1")}`, ]; // New lever keys are CONDITIONAL exports (audit 139 R2#1): a value the user already @@ -93,7 +95,13 @@ export function writeShellEnvFile( const conditional = (name: string, value: string) => `[ -z "\${${name}+x}" ] && export ${name}=${shellValue(value)}`; if (systemEnvMarkerMode(config, deps) === "proxy") { - if (config.apiKeys?.length) { + // On a bind that demands data-plane admission the credential may live only in + // `OPENCODEX_API_AUTH_TOKEN` or the hardened service token file, so the same ladder the + // launchd injection uses applies here. Never the admin token (reviewer constraint on #4236). + const hostAdmissionToken = destination.requiresAdmissionToken ? localAdmissionToken(config) : undefined; + if (hostAdmissionToken) { + lines.push(`export ANTHROPIC_AUTH_TOKEN=${shellValue(hostAdmissionToken)}`); + } else if (config.apiKeys?.length) { lines.push(`export ANTHROPIC_AUTH_TOKEN=${shellValue(config.apiKeys[0].key)}`); } else { lines.push(conditional("ANTHROPIC_AUTH_TOKEN", PROXY_MARKER)); diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 35aad76532..712addbeb7 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -7,7 +7,8 @@ import { PROXY_MARKER } from "../claude/auth-detect"; import { isProxyAdmissionSecret } from "./auth-cors"; import type { OcxConfig } from "../types"; import { recordOwnedConfigPath } from "../lib/config-ownership"; -import { localInferencePort } from "../lib/local-destinations"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; +import { probeHostname } from "./proxy-liveness"; import { providerContextCap } from "../providers/context-cap"; import { OPENAI_CODEX_PROVIDER_ID } from "../providers/openai-tiers"; export { getShellEnvFilePath, installShellHook, uninstallShellHook, claudeCodeCliInstalled, reconcileShellHook } from "./system-env-shell"; @@ -36,23 +37,35 @@ const MANAGED_SYSTEM_ENV_NAMES = new Set([ interface SystemEnvTracking { pid: number; - /** The PUBLIC port of the owning proxy: its instance identity and its /healthz address. */ + /** The PUBLIC port of the owning proxy: its instance identity and its /healthz port. */ port: number; /** - * The loopback port the injected ANTHROPIC_BASE_URL names, when it is not `port` (#4236). + * The host `/healthz` answers on — `probeHostname(config.hostname)` at injection time (#4236). * - * These two separated the day local clients started honoring the unauthenticated loopback - * listener. Ownership is proven against THIS port (it is what was injected), while liveness - * is still probed on `port` — the listener serves no `/healthz`, so probing it would declare - * a perfectly live proxy stale and revert its environment. Absent in records written before - * this field existed, where the two were by definition the same. + * Without it the liveness probe went to `127.0.0.1:`, which does not exist on a + * tailnet-bound hub: every probe failed, the record was reverted on every start, and the + * "another instance owns env" guard below could never fire. Absent (⇒ 127.0.0.1) in records + * written by a build that only ever bound loopback. */ - clientPort?: number; + bindHost?: string; + /** + * The exact ANTHROPIC_BASE_URL that was injected, when it is not `http://127.0.0.1:`. + * + * The injected destination and the proxy's own `/healthz` address separated the day local + * clients started honoring the unauthenticated loopback listener, and separated again for a + * non-loopback bind. Ownership on revert is proven against THIS value — it is what was + * written — while liveness is probed on `bindHost`/`port`, because the loopback listener + * serves no `/healthz` and probing the injected port would declare a live proxy stale. + */ + clientBaseUrl?: string; injectedAt: string; /** Keys that were actually set by injection (revert only unsets these). */ injectedKeys?: string[]; } +/** Bounded host shape: this value is interpolated into a probe URL, so it is validated on read. */ +const BIND_HOST_PATTERN = /^[A-Za-z0-9.\-_]{1,253}$|^\[[0-9A-Fa-f:.]{2,45}\]$/; + type SystemEnvResult = { injected: boolean; reason?: string }; type RevertResult = { reverted: boolean; reason?: string }; type CleanupResult = { cleaned: boolean; reason?: string }; @@ -82,8 +95,13 @@ function readTracking(): SystemEnvTracking | undefined { (name): name is string => typeof name === "string" && MANAGED_SYSTEM_ENV_NAMES.has(name), ))] : undefined; - const clientPort = Number.isInteger(tracking.clientPort) ? tracking.clientPort : undefined; - return { ...tracking, clientPort, injectedKeys } as SystemEnvTracking; + const bindHost = typeof tracking.bindHost === "string" && BIND_HOST_PATTERN.test(tracking.bindHost) + ? tracking.bindHost + : undefined; + const clientBaseUrl = typeof tracking.clientBaseUrl === "string" && /^https?:\/\/[^\s/]+$/.test(tracking.clientBaseUrl) + ? tracking.clientBaseUrl + : undefined; + return { ...tracking, bindHost, clientBaseUrl, injectedKeys } as SystemEnvTracking; } catch { return undefined; } @@ -101,24 +119,40 @@ function ownedBaseUrl(port: number): string { return `http://127.0.0.1:${port}`; } -function writeTracking(port: number, injectedKeys: string[], clientPort: number = port): void { +/** What the tracking record has to remember beyond the public port, or nothing on a pure loopback install. */ +interface TrackedDestination { + /** `probeHostname(config.hostname)` — where `/healthz` answers. */ + bindHost: string; + /** The injected ANTHROPIC_BASE_URL. */ + clientBaseUrl: string; +} + +function writeTracking(port: number, injectedKeys: string[], tracked?: TrackedDestination): void { recordOwnedConfigPath(getConfigDir(), getSystemEnvTrackingPath()); mkdirSync(getConfigDir(), { recursive: true, mode: 0o700 }); writeFileSync(getSystemEnvTrackingPath(), JSON.stringify({ pid: process.pid, port, - ...(clientPort === port ? {} : { clientPort }), + // Both fields are omitted when they carry no information beyond `port`, so a plain + // loopback install keeps writing the byte-identical record it always did. + ...(tracked && tracked.bindHost !== "127.0.0.1" ? { bindHost: tracked.bindHost } : {}), + ...(tracked && tracked.clientBaseUrl !== ownedBaseUrl(port) ? { clientBaseUrl: tracked.clientBaseUrl } : {}), injectedAt: new Date().toISOString(), injectedKeys, }), { encoding: "utf8", mode: 0o600 }); } -/** The base URL that was injected: the loopback listener's port when one is in play (#4236). */ -function trackedBaseUrl(tracking: Pick): string { - return ownedBaseUrl(tracking.clientPort ?? tracking.port); +/** The base URL that was injected: the loopback listener or the bind address, not always loopback (#4236). */ +function trackedBaseUrl(tracking: Pick): string { + return tracking.clientBaseUrl ?? ownedBaseUrl(tracking.port); } -function rollbackInjectedKeys(port: number, injectedKeys: string[], clientPort: number = port): void { +/** Where the owning proxy's `/healthz` answers — the BIND address, never the injected port (#4236). */ +function trackedHealthzOrigin(tracking: Pick): string { + return `http://${tracking.bindHost ?? "127.0.0.1"}:${tracking.port}`; +} + +function rollbackInjectedKeys(port: number, injectedKeys: string[], tracked?: TrackedDestination): void { const rollbackFailed: string[] = []; for (const name of [...injectedKeys].reverse()) { try { @@ -129,7 +163,7 @@ function rollbackInjectedKeys(port: number, injectedKeys: string[], clientPort: } if (rollbackFailed.length > 0) { - writeTracking(port, rollbackFailed, clientPort); + writeTracking(port, rollbackFailed, tracked); return; } @@ -169,8 +203,39 @@ export async function injectSystemEnv( await cleanStaleSystemEnv(); + // A launchd-started `claude` is a LOCAL client: it dials the unauthenticated loopback + // listener when one is enabled, because on a tailnet-bound hub nothing answers on + // 127.0.0.1: (#4236); with no listener the destination is the BIND address, + // which answers but demands data-plane admission. `port` stays the instance identity. + const destination = localInferenceDestination(config, port); + const tracked: TrackedDestination = { + bindHost: probeHostname(config.hostname), + clientBaseUrl: destination.origin, + }; + const markerMode = systemEnvMarkerMode(config, deps); + // The data-plane credential this host can present to its own public listener: env token, + // hardened service token file, then a configured key. Never the admin token. + const hostAdmissionToken = destination.requiresAdmissionToken ? localAdmissionToken(config) : undefined; + // The launchd domain is machine-wide, so injecting a destination that will 401 every plain + // `claude` is worse than not injecting at all. Degrade out loud instead: a subscription + // launch cannot carry a host token (asserting one logs a claude.ai subscriber out, #253), + // and with no resolvable credential there is nothing to carry either way. + if (destination.requiresAdmissionToken && (markerMode !== "proxy" || !hostAdmissionToken)) { + console.error( + `⚠ Skipping system-environment injection: ${destination.origin} requires an opencodex data-plane ` + + "credential that this launch cannot supply. Enable `unauthenticatedLoopbackListener` or bind the " + + "proxy to loopback.", + ); + return { injected: false, reason: "local inference requires a data-plane credential" }; + } + const currentBaseUrl = launchctlGetenv("ANTHROPIC_BASE_URL"); - if (currentBaseUrl && !/^http:\/\/127\.0\.0\.1:\d+$/.test(currentBaseUrl)) { + // Ours is loopback in every topology but one: with the listener off and a non-loopback bind + // the value WE wrote names the bind address, and rejecting it as "user has custom" would make + // every subsequent start refuse to refresh its own injection. + if (currentBaseUrl + && !/^http:\/\/127\.0\.0\.1:\d+$/.test(currentBaseUrl) + && currentBaseUrl !== destination.origin) { return { injected: false, reason: "user has custom ANTHROPIC_BASE_URL" }; } // After stale cleanup, if a tracking file still exists with a DIFFERENT port, @@ -183,22 +248,20 @@ export async function injectSystemEnv( const injectedKeys: string[] = existingTracking ? [...(existingTracking.injectedKeys ?? SYSTEM_ENV_NAMES)] : []; - // A launchd-started `claude` is a LOCAL client: it dials the unauthenticated loopback - // listener when one is enabled, because on a tailnet-bound hub nothing answers on - // 127.0.0.1: (#4236). `port` stays the instance identity and /healthz address. - const clientPort = localInferencePort(config, port); const inject = (name: string, value: string) => { setLaunchctlEnv(name, value); if (!injectedKeys.includes(name)) injectedKeys.push(name); - writeTracking(port, injectedKeys, clientPort); + writeTracking(port, injectedKeys, tracked); }; try { - inject("ANTHROPIC_BASE_URL", ownedBaseUrl(clientPort)); + inject("ANTHROPIC_BASE_URL", destination.origin); inject("CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", "1"); - const markerMode = systemEnvMarkerMode(config, deps); if (markerMode === "proxy") { - if (config.apiKeys?.length) { + if (hostAdmissionToken) { + // The guard above proved this exists whenever the destination demands one. + inject("ANTHROPIC_AUTH_TOKEN", hostAdmissionToken); + } else if (config.apiKeys?.length) { inject("ANTHROPIC_AUTH_TOKEN", config.apiKeys[0].key); } else if (launchctlGetenv("ANTHROPIC_AUTH_TOKEN") === undefined) { inject("ANTHROPIC_AUTH_TOKEN", PROXY_MARKER); @@ -213,7 +276,7 @@ export async function injectSystemEnv( unsetLaunchctlEnv("ANTHROPIC_AUTH_TOKEN"); const tokenIdx = injectedKeys.indexOf("ANTHROPIC_AUTH_TOKEN"); if (tokenIdx >= 0) injectedKeys.splice(tokenIdx, 1); - writeTracking(port, injectedKeys, clientPort); + writeTracking(port, injectedKeys, tracked); } } // Lever keys (devlog 136 B6): user-wins — skip any key the user already set in the @@ -265,9 +328,9 @@ export async function injectSystemEnv( injectClaudeAgentDefs(config, windows); } catch { /* best-effort */ } - writeTracking(port, injectedKeys, clientPort); + writeTracking(port, injectedKeys, tracked); } catch (error) { - rollbackInjectedKeys(port, injectedKeys, clientPort); + rollbackInjectedKeys(port, injectedKeys, tracked); removeShellEnvFile(); console.error("Failed to inject system environment; rolled back launchctl changes:", error); throw error; @@ -319,10 +382,13 @@ export async function cleanStaleSystemEnv(): Promise { if (!tracking) return { cleaned: false, reason: "no tracking file" }; try { - // `tracking.port`, never the injected client port: `/healthz` is not on the - // unauthenticated loopback listener's allowlist, so probing that port would 404 and - // revert a live proxy's environment (#4236). - const response = await fetch(`${ownedBaseUrl(tracking.port)}/healthz`, { + // The BIND address and `tracking.port`, never the injected destination: `/healthz` is not + // on the unauthenticated loopback listener's allowlist, so probing that port would 404 and + // revert a live proxy's environment (#4236). Nor is it always 127.0.0.1 — a tailnet-bound + // hub has no loopback socket at all, and probing one there failed every single time, which + // reverted the record on every start and made the "another instance owns env" guard above + // unreachable. + const response = await fetch(`${trackedHealthzOrigin(tracking)}/healthz`, { signal: AbortSignal.timeout(1_000), }); if (response.ok) return { cleaned: false, reason: "proxy still alive" }; diff --git a/src/vision/plan.ts b/src/vision/plan.ts index 3e0f25eca9..3cb0b2f3e2 100644 --- a/src/vision/plan.ts +++ b/src/vision/plan.ts @@ -111,7 +111,7 @@ export interface VisionPlan { /** Namespaced "provider/model" describer for the routed backend (roadmap 180). */ routedModel?: string; /** Loopback dispatch inputs for the routed backend (the listener decides WHICH local port). */ - routedConfig?: Pick; + routedConfig?: Pick; settings: VisionSettings; maxDescriptionsPerTurn: number; } @@ -156,7 +156,10 @@ export function planVisionSidecar( routedConfig: { port: config.port, ...(config.apiKeys ? { apiKeys: config.apiKeys } : {}), - // The self-fetch has to honor the unauthenticated loopback listener (#4236). + // The self-fetch has to honor the unauthenticated loopback listener AND, with no + // listener, the bind address — so BOTH fields the destination resolver reads have to + // survive the narrowing or it silently resolves to the wrong local socket (#4236). + ...(config.hostname ? { hostname: config.hostname } : {}), ...(config.unauthenticatedLoopbackListener ? { unauthenticatedLoopbackListener: config.unauthenticatedLoopbackListener } : {}), diff --git a/src/vision/routed-describe.ts b/src/vision/routed-describe.ts index 0bd4e52b13..dc0af78218 100644 --- a/src/vision/routed-describe.ts +++ b/src/vision/routed-describe.ts @@ -19,18 +19,18 @@ * admission secret in a forwardable header is a forwarding hazard). Loopback * binds require no token at all (resolveApiAuth admits loopback). * - * Known limitation (recorded in roadmap 170): a bindHost where 127.0.0.1 - * does not answer cannot reach its own loopback — same latent limitation - * gateway-cache has. #4236 closes it for the case that actually occurs: a hub - * with an unauthenticated loopback listener, which this helper now dials. + * Destination (#4236): the unauthenticated loopback listener when one is + * enabled, otherwise the BIND address — the former roadmap-170 limitation + * ("a bindHost where 127.0.0.1 does not answer cannot reach its own + * loopback") is closed by resolving through `localInferenceDestination` + * rather than composing 127.0.0.1 by hand. */ import type { OcxConfig } from "../types"; -import { localInferenceOrigin } from "../lib/local-destinations"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; import { signalWithTimeout, cancelBodyOnAbort } from "../lib/abort"; import { redactSecretString } from "../lib/redact"; import { sidecarEnter } from "../lib/sidecar-tracker"; -import { configuredApiAuthToken, configuredPort } from "../server/auth-cors"; -import { loadServiceTokenFromFile } from "../lib/service-secrets"; +import { configuredPort } from "../server/auth-cors"; import type { DescribeOutcome, VisionSettings } from "./describe"; export const VISION_DESCRIBE_TERMINAL_HEADER = "x-opencodex-vision-describe"; @@ -62,28 +62,43 @@ function validateImageUrl(url: string): string | null { return "unsupported image URL scheme (expected data: or https:)"; } -/** The admission ladder: env token, service token file, first configured API key. */ +/** + * The admission ladder: env token, hardened service token file, first configured API key. + * + * Shared with every other local client through `localAdmissionToken` so the credential this + * self-fetch presents cannot drift from the one the Codex provider table and the Claude launch + * env carry. Never the admin token. + */ export function routedDescribeAdmissionToken(config: Pick): string | undefined { - const envToken = configuredApiAuthToken(); - if (envToken) return envToken; - const fileToken = loadServiceTokenFromFile(process.env); - if (fileToken) return fileToken; - const first = config.apiKeys?.[0]?.key?.trim(); - return first || undefined; + return localAdmissionToken(config); } -/** Base URL seam for tests; production always self-fetches loopback. */ +/** Base URL seam for tests; production always self-fetches the resolved local destination. */ export function routedDescribeBaseUrl( - config: Pick, + config: Pick, ): string { - // config.port can be 0 (ephemeral bind, tests) or stale after a live port - // override; the server records its ACTUAL bound port via setCorsOrigin at - // startup, so prefer that when config carries no positive port. - const port = config.port && config.port > 0 ? config.port : Number(configuredPort()); - // This self-fetch is a local client like any other: on a hub bound to a tailnet address the - // only socket on 127.0.0.1 is the unauthenticated loopback listener (#4236). The helper sends - // the OpenAI chat wire, which that listener now admits. - return localInferenceOrigin(config, port); + return routedDescribeDestination(config).origin; +} + +/** + * The local destination this self-fetch dials, and whether it needs a credential. + * + * This is a local client like any other: the unauthenticated loopback listener when one is + * enabled, otherwise the bind address — on a tailnet-bound hub there is no loopback socket at + * all (#4236). The helper sends the OpenAI chat wire, which that listener now admits. + */ +function routedDescribeDestination( + config: Pick, +) { + // config.port can be 0 (ephemeral bind, tests) or stale after a live port override; the + // server records its ACTUAL bound port via setCorsOrigin at startup, so prefer that when + // config carries no positive port. `configuredPort()` is itself `0` when `_corsOrigin` has no + // explicit port (a default-port origin), so the literal default has to backstop it or the + // composed URL names port 0 and the self-fetch cannot connect. + const port = config.port && config.port > 0 + ? config.port + : Number(configuredPort()) || 10_100; + return localInferenceDestination(config, port); } export async function describeImageRouted( @@ -91,7 +106,7 @@ export async function describeImageRouted( _detail: string | undefined, contextText: string, routedModel: string, - config: Pick, + config: Pick, settings: VisionSettings, abortSignal?: AbortSignal, baseUrlOverride?: string, @@ -105,6 +120,15 @@ export async function describeImageRouted( }; const admission = routedDescribeAdmissionToken(config); if (admission) headers["x-opencodex-api-key"] = admission; + // A bind that demands admission with no resolvable credential would return 401 with a body + // the caller reports as a describe failure; naming the cause once is the difference between + // "vision is broken" and a fixable configuration note. + if (!admission && !baseUrlOverride && routedDescribeDestination(config).requiresAdmissionToken) { + console.warn( + "[vision] routed describe has no opencodex data-plane credential for " + + `${routedDescribeBaseUrl(config)} — the self-fetch will be refused.`, + ); + } const requestBody = { model: routedModel, diff --git a/tests/claude-integration/claude-cli.test.ts b/tests/claude-integration/claude-cli.test.ts index 90a66c0362..0ac94f078f 100644 --- a/tests/claude-integration/claude-cli.test.ts +++ b/tests/claude-integration/claude-cli.test.ts @@ -547,8 +547,11 @@ describe("ocx claude env assembly", () => { * Which local socket `ocx claude` dials (#4236). * * `ocx claude` is handed the live PUBLIC port. On a hub bound to a tailnet address nothing - * answers on `127.0.0.1:`, so the launch has to resolve the unauthenticated - * loopback listener instead — the same port `ocx sync` already writes into Codex. + * answers on `127.0.0.1:`, so the launch resolves the unauthenticated loopback + * listener instead — the same port `ocx sync` already writes into Codex. With NO listener the + * destination is the BIND address: reachable, but it demands a data-plane credential, so the + * launch has to carry one or say out loud that it cannot. The first round of this change + * returned loopback unconditionally and these tests pinned that as intended. */ describe("ocx claude local inference destination", () => { const hub = (listener?: { enabled: boolean; port?: number }) => cfg({ @@ -569,31 +572,108 @@ describe("ocx claude local inference destination", () => { test("a plain loopback install is byte-identical to before", () => { expect(buildClaudeEnv(cfg(), 10100, {}).ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); - // And a hub that enabled no listener keeps dialing the public port too: this PR changes - // where an ENABLED listener sends local clients, not whether one exists. - expect(buildClaudeEnv(hub(), 10100, {}).ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); }); - test("neither of our own local ports is treated as a stale foreign proxy", () => { - // The public port and the listener port are both ours. Rewriting one into the other would - // strip the admission token that was minted for it, silently downgrading the launch. + test("with the listener OFF the destination is the bind address, not a dead loopback port", () => { + // The #4236 topology itself. `127.0.0.1:10100` does not exist on this hub, so returning it + // — which the first round of this change did — is a guaranteed connect failure. The bind + // address answers, and the admission token the launch carries is what makes it usable. const config = cfg({ claudeCode: { authMode: "proxy" }, hostname: "100.76.170.81", runtimeRole: "hub", - unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], } as Partial); - for (const origin of ["http://127.0.0.1:10100", "http://127.0.0.1:10104"]) { - const env = buildClaudeEnv(config, 10100, { ANTHROPIC_BASE_URL: origin }, {}, { - ...AUTH_PRESENT, - preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"], - }); - expect({ origin, baseUrl: env.ANTHROPIC_BASE_URL }).toEqual({ origin, baseUrl: origin }); - expect({ origin, token: env.ANTHROPIC_AUTH_TOKEN }).toEqual({ origin, token: "ocx_data_this_proxy_key" }); + const env = buildClaudeEnv(config, 10100, {}, {}, AUTH_PRESENT); + expect(env.ANTHROPIC_BASE_URL).toBe("http://100.76.170.81:10100"); + // `targetsLocalClaudeProxy` has to recognize the value we just wrote, or the launch would + // refuse to attach the credential to its own destination one line later. + expect(env.ANTHROPIC_AUTH_TOKEN).toBe("ocx_data_this_proxy_key"); + }); + + test("a wildcard bind keeps loopback but still carries the credential it demands", () => { + const config = cfg({ + claudeCode: { authMode: "proxy" }, + hostname: "0.0.0.0", + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + const env = buildClaudeEnv(config, 10100, {}, {}, AUTH_PRESENT); + expect(env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10100"); + expect(env.ANTHROPIC_AUTH_TOKEN).toBe("ocx_data_this_proxy_key"); + }); + + test("a subscription launch on a bind that demands admission degrades out loud", () => { + // Asserting a host token would log a claude.ai subscriber out (#253), so the launch cannot + // carry one — and a silent 401 on the first request is the failure this warning replaces. + const config = cfg({ + claudeCode: { authMode: "subscription" }, + hostname: "100.76.170.81", + runtimeRole: "hub", + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + const errors: string[] = []; + const realError = console.error; + console.error = (...args: unknown[]) => { errors.push(args.map(String).join(" ")); }; + try { + const env = buildClaudeEnv(config, 10100, {}, {}, AUTH_PRESENT); + expect(env.ANTHROPIC_BASE_URL).toBe("http://100.76.170.81:10100"); + expect(env.ANTHROPIC_AUTH_TOKEN).toBeUndefined(); + } finally { + console.error = realError; + } + expect(errors.some(line => + line.includes("http://100.76.170.81:10100") && line.includes("data-plane credential"), + )).toBe(true); + }); + + test("both live local ports are ours; a port nothing answers on is not", () => { + // On a LOOPBACK or wildcard bind the public port and the listener port BOTH answer on + // 127.0.0.1, so a URL naming either was written by us. Rewriting one into the other would + // strip the admission token minted for it and silently downgrade the launch. + for (const hostname of ["127.0.0.1", "0.0.0.0"]) { + const config = cfg({ + claudeCode: { authMode: "proxy" }, + hostname, + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + for (const origin of ["http://127.0.0.1:10100", "http://127.0.0.1:10104"]) { + const env = buildClaudeEnv(config, 10100, { ANTHROPIC_BASE_URL: origin }, {}, { + ...AUTH_PRESENT, + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"], + }); + expect({ hostname, origin, baseUrl: env.ANTHROPIC_BASE_URL }).toEqual({ hostname, origin, baseUrl: origin }); + expect({ hostname, origin, token: env.ANTHROPIC_AUTH_TOKEN }) + .toEqual({ hostname, origin, token: "ocx_data_this_proxy_key" }); + } } }); + test("on a tailnet bind the public port is NOT ours on loopback, so it is replaced", () => { + // The counterpart of the case above, and the reason the set is computed from the bind scope + // instead of from "our two port numbers": nothing answers on 127.0.0.1:10100 here, so an + // inherited value naming it is stale and must be rewritten to the listener. + const config = cfg({ + claudeCode: { authMode: "proxy" }, + hostname: "100.76.170.81", + runtimeRole: "hub", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], + } as Partial); + const env = buildClaudeEnv(config, 10100, { ANTHROPIC_BASE_URL: "http://127.0.0.1:10100" }, {}, { + ...AUTH_PRESENT, + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"], + }); + expect(env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10104"); + // Still ours: the listener admits without a credential, but the launch keeps the one it has. + const ported = buildClaudeEnv(config, 10100, { ANTHROPIC_BASE_URL: "http://127.0.0.1:10104" }, {}, { + ...AUTH_PRESENT, + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL"], + }); + expect(ported.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:10104"); + }); + test("a genuinely foreign loopback port is replaced with the resolved destination", () => { const env = buildClaudeEnv( hub({ enabled: true, port: 10104 }), @@ -606,6 +686,10 @@ describe("ocx claude local inference destination", () => { }); test("a native launch sheds the managed destination on either local port", () => { + // Shedding asks a WIDER question than replacement: "could we have written this?". The + // public port qualifies on every topology, because an earlier config on this machine may + // have been loopback-bound — and leaving such a URL behind with its token stripped (which + // always happens) would point the native launch at a dead socket with no credential. const config = cfg({ hostname: "100.76.170.81", runtimeRole: "hub", diff --git a/tests/clients/desktop-3p.test.ts b/tests/clients/desktop-3p.test.ts index c69e7e4199..912c2c18af 100644 --- a/tests/clients/desktop-3p.test.ts +++ b/tests/clients/desktop-3p.test.ts @@ -397,14 +397,17 @@ describe("Claude Desktop 3P models", () => { /** * Claude Desktop is a LOCAL client (#4236): the gateway base URL it is given must be the * unauthenticated loopback listener when one is enabled, because on a hub bound to a tailnet - * address `127.0.0.1:` is a closed port. Resolved inside `writeDesktop3pConfig` - * from the config it already re-reads, so every caller gets the same answer. + * address `127.0.0.1:` is a closed port — and with NO listener it must be the + * bind address, which is the case the first round of this change still wrote as loopback. + * Resolved inside `writeDesktop3pConfig` from the config it already re-reads, so every caller + * gets the same answer. */ test("the written gateway base URL follows the unauthenticated loopback listener", () => { const cases = [ { listener: { enabled: true, port: 10104 }, expected: "http://127.0.0.1:10104" }, { listener: { enabled: true }, expected: "http://127.0.0.1:4096" }, - { listener: undefined, expected: "http://127.0.0.1:4096" }, + // No listener on a tailnet-bound hub: the bind address, not a closed loopback port. + { listener: undefined, expected: "http://100.76.170.81:4096" }, ] as const; for (const { listener, expected } of cases) { const dir = mkdtempSync(join(tmpdir(), "ocx-desktop-listener-")); @@ -433,6 +436,12 @@ describe("Claude Desktop 3P models", () => { ? profile.inferenceGatewayBaseUrl : applied?.inferenceGatewayBaseUrl; expect({ listener, baseUrl }).toEqual({ listener, baseUrl: expected }); + // The exported profile carries the DATA-PLANE key it was handed and nothing more: a + // management credential must never enter a client configuration (review on #4236). + const apiKey = typeof profile.inferenceGatewayApiKey === "string" + ? profile.inferenceGatewayApiKey + : applied?.inferenceGatewayApiKey; + expect({ listener, apiKey }).toEqual({ listener, apiKey: "k" }); } finally { if (previous === undefined) delete process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; else process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR = previous; @@ -443,6 +452,16 @@ describe("Claude Desktop 3P models", () => { } }); + test("generateDesktop3pConfig accepts a resolved origin as well as a bare port", () => { + // The pure generator gained the origin form because a bind-address destination is not + // expressible as a port. A bare port still means `http://127.0.0.1:`, so every other + // caller and every existing expectation is unchanged. + const byPort = generateDesktop3pConfig(4096, ["gpt-5.6-sol"], [], "k") as Record; + const byOrigin = generateDesktop3pConfig("http://100.76.170.81:4096", ["gpt-5.6-sol"], [], "k") as Record; + expect(byPort.inferenceGatewayBaseUrl).toBe("http://127.0.0.1:4096"); + expect(byOrigin.inferenceGatewayBaseUrl).toBe("http://100.76.170.81:4096"); + }); + test("re-applying an owned profile preserves foreign profile keys", () => { const dir = mkdtempSync(join(tmpdir(), "ocx-desktop-merge-")); const previous = process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; diff --git a/tests/lib/local-destinations.test.ts b/tests/lib/local-destinations.test.ts index 557eb34526..52b003b052 100644 --- a/tests/lib/local-destinations.test.ts +++ b/tests/lib/local-destinations.test.ts @@ -6,11 +6,23 @@ * base URL substituted everywhere (maintainer review on #4236) — management discovery and * inference are different surfaces with different admission rules, so they get one resolver * each and these tests hold them apart. + * + * The FIRST round of that fix got inference half-right and these tests pinned the bug as + * intended: `localInferenceOrigin` returned `127.0.0.1:` whenever the loopback + * listener was off, with no bind-address fallback, so the exact topology the issue is about — + * listener off, `hostname` a tailnet address — still handed all eight sites a dead socket. Both + * resolvers now have the same three-branch shape, and the inference one additionally reports + * whether its destination demands a data-plane credential, because "reachable" and "will be + * admitted" are different questions and a string cannot answer the second. + * + * The six configurations below are the review's (a)–(f). Every one of them is a shape a real + * `config.json` can hold, and each lands in a different branch. */ import { describe, expect, test } from "bun:test"; import { - localInferenceOrigin, - localInferencePort, + localAdmissionToken, + localInferenceDestination, + localLoopbackInferencePorts, localManagementOrigin, } from "../../src/lib/local-destinations"; import type { OcxConfig } from "../../src/types"; @@ -29,42 +41,175 @@ function hub(extra: Partial = {}): OcxConfig { } as unknown as OcxConfig; } -describe("localInferencePort / localInferenceOrigin", () => { - test("a ported listener is the destination; a companion listener is the public port", () => { - expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), PUBLIC_PORT)) - .toBe(10_104); - expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: true } }), PUBLIC_PORT)) - .toBe(PUBLIC_PORT); - }); +/** The review's six configurations, each in the branch it is supposed to reach. */ +const CONFIGURATIONS: Array<{ + label: string; + config: OcxConfig; + origin: string; + requiresAdmissionToken: boolean; + loopbackPorts: number[]; +}> = [ + { + // (a) standalone loopback — the shape that must stay byte-identical to the pre-#4236 string. + label: "standalone loopback", + config: hub({ runtimeRole: "standalone", hostname: "127.0.0.1" }), + origin: "http://127.0.0.1:10100", + requiresAdmissionToken: false, + loopbackPorts: [PUBLIC_PORT], + }, + { + // (b) companion hub (PR2): the listener's effective port IS the public port. + label: "companion hub", + config: hub({ unauthenticatedLoopbackListener: { enabled: true } }), + origin: "http://127.0.0.1:10100", + requiresAdmissionToken: false, + loopbackPorts: [PUBLIC_PORT], + }, + { + // (c) ported hub: the only form whose resolved string actually moves. + label: "ported hub", + config: hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), + origin: "http://127.0.0.1:10104", + requiresAdmissionToken: false, + loopbackPorts: [10_104], + }, + { + // (d) THE DEFECT: listener off, non-loopback bind. Nothing answers on 127.0.0.1 here, so the + // destination is the bind address and it demands a data-plane credential. + label: "hub with the listener off and a non-loopback bind", + config: hub(), + origin: `http://${TAILNET}:10100`, + requiresAdmissionToken: true, + loopbackPorts: [], + }, + { + // (e) wildcard bind: loopback DOES answer, but the public listener still demands admission + // regardless of which address received the request. Reachable ≠ admitted. + label: "wildcard bind", + config: hub({ hostname: "0.0.0.0" }), + origin: "http://127.0.0.1:10100", + requiresAdmissionToken: true, + loopbackPorts: [PUBLIC_PORT], + }, + { + // (f) client role: the role decides management, never inference. A client's own local data + // plane is resolved by the same bind-address rule as anything else. + label: "client role", + config: hub({ runtimeRole: "client" }), + origin: `http://${TAILNET}:10100`, + requiresAdmissionToken: true, + loopbackPorts: [], + }, +]; - test("no listener, a disabled listener, and no config all keep the public port", () => { - // This is the "nothing changes on a plain loopback or standalone install" guarantee: every - // call site that used to spell `http://127.0.0.1:${port}` gets that exact string back. - expect(localInferencePort(hub(), PUBLIC_PORT)).toBe(PUBLIC_PORT); - expect(localInferencePort(hub({ unauthenticatedLoopbackListener: { enabled: false } }), PUBLIC_PORT)) - .toBe(PUBLIC_PORT); - expect(localInferencePort(undefined, PUBLIC_PORT)).toBe(PUBLIC_PORT); - expect(localInferencePort({}, PUBLIC_PORT)).toBe(PUBLIC_PORT); +describe("localInferenceDestination", () => { + for (const expected of CONFIGURATIONS) { + test(`${expected.label} resolves to ${expected.origin}`, () => { + const actual = localInferenceDestination(expected.config, PUBLIC_PORT); + expect({ label: expected.label, ...actual }).toEqual({ + label: expected.label, + origin: expected.origin, + port: Number(new URL(expected.origin).port), + requiresAdmissionToken: expected.requiresAdmissionToken, + }); + }); + } + + test("a credential is demanded exactly when the destination is not a credential-free socket", () => { + // This is the invariant the struct exists to carry: the only two free sockets are the + // unauthenticated loopback listener and a genuinely loopback public bind. Asserting it as a + // set, rather than per-case, is what makes a new branch that forgets the flag fail here. + const free = CONFIGURATIONS.filter(c => !c.requiresAdmissionToken).map(c => c.label); + expect(free).toEqual(["standalone loopback", "companion hub", "ported hub"]); }); - test("the origin is always loopback, never the bind address", () => { - // A tailnet or LAN address in a local client's base URL is the #4236 defect in reverse: - // the client would then need an admission credential it has no way to obtain. - for (const listener of [ + test("no listener, a disabled listener, and no config at all keep the public port on loopback", () => { + // The "nothing changes on a plain loopback or standalone install" guarantee: every call + // site that used to spell `http://127.0.0.1:${port}` gets that exact string back. + for (const config of [ undefined, - { enabled: false } as const, - { enabled: true } as const, - { enabled: true, port: 10_104 } as const, - ]) { - const origin = localInferenceOrigin( - hub(listener === undefined ? {} : { unauthenticatedLoopbackListener: listener }), + {}, + { hostname: "127.0.0.1" }, + { hostname: "localhost" }, + { hostname: "::1" }, + { hostname: "127.0.0.1", unauthenticatedLoopbackListener: { enabled: false } }, + ] as Array[0]>) { + expect({ config, ...localInferenceDestination(config, PUBLIC_PORT) }).toEqual({ + config, + origin: "http://127.0.0.1:10100", + port: PUBLIC_PORT, + requiresAdmissionToken: false, + }); + } + }); + + test("a bare IPv6 bind is bracketed, or the composed URL is unparseable", () => { + const destination = localInferenceDestination({ hostname: "fd7a:115c:a1e0::1" }, PUBLIC_PORT); + expect(destination.origin).toBe("http://[fd7a:115c:a1e0::1]:10100"); + expect(new URL(destination.origin).hostname).toBe("[fd7a:115c:a1e0::1]"); + }); + + test("every all-zero bind spelling is a wildcard, not a hostname to dial", () => { + // `probeHostname` used to know three spellings while the bind-scope predicate knew all of + // them, so these composed `http://0.0.0.0.:10100` and `http://*:10100` — URLs that connect + // to nothing — for configs the server itself treats as wildcard binds. + for (const hostname of ["0.0.0.0", "0.0.0.0.", "00.0.0.000", "::", "[::]", "::0", "0::", "*", "0"]) { + const destination = localInferenceDestination({ hostname }, PUBLIC_PORT); + expect({ hostname, origin: destination.origin, requires: destination.requiresAdmissionToken }) + .toEqual({ hostname, origin: "http://127.0.0.1:10100", requires: true }); + } + }); +}); + +describe("localLoopbackInferencePorts", () => { + for (const expected of CONFIGURATIONS) { + test(`${expected.label} answers on ${JSON.stringify(expected.loopbackPorts)} at 127.0.0.1`, () => { + expect({ label: expected.label, ports: localLoopbackInferencePorts(expected.config, PUBLIC_PORT) }) + .toEqual({ label: expected.label, ports: expected.loopbackPorts }); + }); + } + + test("a loopback or wildcard bind with a ported listener owns BOTH ports", () => { + // This is why the set exists: `ocx claude` must not rewrite one of its own destinations + // into the other and strip the admission token minted for it. + for (const hostname of ["127.0.0.1", "0.0.0.0"]) { + expect({ hostname, ports: localLoopbackInferencePorts( + hub({ hostname, unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), PUBLIC_PORT, - ); - expect({ listener, host: new URL(origin).hostname }).toEqual({ listener, host: "127.0.0.1" }); - expect({ listener, protocol: new URL(origin).protocol }).toEqual({ listener, protocol: "http:" }); + ) }).toEqual({ hostname, ports: [PUBLIC_PORT, 10_104] }); } - expect(localInferenceOrigin(hub({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }), PUBLIC_PORT)) - .toBe("http://127.0.0.1:10104"); + }); + + test("a tailnet bind with no listener owns NOTHING on loopback", () => { + // The set is empty on purpose: a leftover `http://127.0.0.1:10100` from a previous + // loopback-bound install is a dead socket there, and treating it as ours would preserve it. + expect(localLoopbackInferencePorts(hub(), PUBLIC_PORT)).toEqual([]); + }); +}); + +describe("localAdmissionToken", () => { + const config = hub({ + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_configured", createdAt: "2026-01-01T00:00:00Z" }], + } as unknown as Partial); + + test("the environment token wins, then the configured key", () => { + expect(localAdmissionToken(config, { OPENCODEX_API_AUTH_TOKEN: " ocx_data_from_env " })) + .toBe("ocx_data_from_env"); + // An empty token file path is still a lookup that finds nothing, so the configured key wins. + expect(localAdmissionToken(config, { OCX_API_TOKEN_FILE: "/nonexistent/ocx-token" })) + .toBe("ocx_data_configured"); + expect(localAdmissionToken(undefined, { OCX_API_TOKEN_FILE: "/nonexistent/ocx-token" })) + .toBeUndefined(); + }); + + test("the admin token is never a candidate", () => { + // The reviewer constraint on #4236: no exported client configuration may carry management + // authority. The ladder reads the DATA-plane variable, so an admin token in the environment + // contributes nothing even when it is the only credential present. + expect(localAdmissionToken({ apiKeys: [] }, { + OPENCODEX_ADMIN_AUTH_TOKEN: `ocx_admin_${"t".repeat(43)}`, + OCX_API_TOKEN_FILE: "/nonexistent/ocx-token", + })).toBeUndefined(); }); }); @@ -106,20 +251,30 @@ describe("localManagementOrigin", () => { } }); - test("loopback and wildcard binds resolve exactly as the old hardcoded string did", () => { + test("both resolvers agree on how a bind address becomes a dialable authority", () => { + // Management has always had the bind-address fallback; inference now has the same one. The + // two must not disagree about a wildcard, a trailing dot, or a bare IPv6 literal, or `ocx + // claude` would discover state on one host and send inference to another. const cases: Array<[string | undefined, string]> = [ [undefined, "http://127.0.0.1:10100"], ["127.0.0.1", "http://127.0.0.1:10100"], ["0.0.0.0", "http://127.0.0.1:10100"], + ["0.0.0.0.", "http://127.0.0.1:10100"], ["::", "http://127.0.0.1:10100"], + ["::0", "http://127.0.0.1:10100"], + ["*", "http://127.0.0.1:10100"], // A bare IPv6 literal has to be bracketed or the URL is unparseable. ["fd7a:115c:a1e0::1", "http://[fd7a:115c:a1e0::1]:10100"], - ["localhost", "http://localhost:10100"], + [TAILNET, `http://${TAILNET}:10100`], ]; for (const [hostname, expected] of cases) { const config = hub({ runtimeRole: "standalone", ...(hostname === undefined ? {} : { hostname }) }); if (hostname === undefined) delete (config as { hostname?: string }).hostname; expect({ hostname, origin: localManagementOrigin(config, PUBLIC_PORT) }).toEqual({ hostname, origin: expected }); + // `localhost` and `::1` are the one documented divergence: inference pins the literal + // 127.0.0.1 to keep the legacy string byte-identical, so they are excluded above. + expect({ hostname, origin: localInferenceDestination(config, PUBLIC_PORT).origin }) + .toEqual({ hostname, origin: expected }); } }); }); diff --git a/tests/providers/cursor/cursor-integration-status.test.ts b/tests/providers/cursor/cursor-integration-status.test.ts index 99c8abd553..3db7fab1a4 100644 --- a/tests/providers/cursor/cursor-integration-status.test.ts +++ b/tests/providers/cursor/cursor-integration-status.test.ts @@ -232,17 +232,20 @@ describe("GET /api/native-integrations/cursor", () => { /** * The gateway URL is pasted into Cursor on THIS machine, so it is the local destination * (#4236): the unauthenticated loopback listener when one is enabled, because on a hub bound - * to a tailnet address nothing answers on 127.0.0.1:. Called directly rather - * than through a server so the three topologies are compared without three binds. + * to a tailnet address nothing answers on 127.0.0.1: — and the BIND address when + * there is no listener, because then nothing local answers at all. The first round of this + * change returned loopback in that case, which is a value the operator would paste and watch + * fail to connect. Called directly rather than through a server so the topologies are compared + * without one bind each. */ test("the gateway base URL follows the unauthenticated loopback listener", async () => { const url = new URL("http://127.0.0.1:10100/api/native-integrations/cursor"); const cases = [ - { listener: { enabled: true, port: 10104 } as const, expected: "http://127.0.0.1:10104/v1" }, - { listener: { enabled: true } as const, expected: "http://127.0.0.1:10100/v1" }, - { listener: undefined, expected: "http://127.0.0.1:10100/v1" }, + { listener: { enabled: true, port: 10104 } as const, expected: "http://127.0.0.1:10104/v1", keyMode: "credential" }, + { listener: { enabled: true } as const, expected: "http://127.0.0.1:10100/v1", keyMode: "credential" }, + { listener: undefined, expected: "http://100.76.170.81:10100/v1", keyMode: "credential" }, ]; - for (const { listener, expected } of cases) { + for (const { listener, expected, keyMode } of cases) { const config: OcxConfig = { ...statusConfig(), port: 10100, @@ -255,11 +258,27 @@ describe("GET /api/native-integrations/cursor", () => { [], ); expect({ listener, baseUrl: status.gateway.baseUrl }).toEqual({ listener, baseUrl: expected }); - // The tailnet address must never reach a value a local app dials. - expect(status.gateway.baseUrl).not.toContain("100.76.170.81"); + // apiKeyMode now describes the admission rule of the destination just resolved, so the + // card cannot tell an operator to paste a URL and omit the key that URL requires. + expect({ listener, apiKeyMode: status.gateway.apiKeyMode }).toEqual({ listener, apiKeyMode: keyMode }); } }); + test("a loopback bind with no credential still offers the placeholder", async () => { + // The other direction of the same rule: nothing about the destination demands a key here, + // so the card must not start asking for one. + const url = new URL("http://127.0.0.1:10100/api/native-integrations/cursor"); + const config: OcxConfig = { ...statusConfig(), port: 10100, hostname: "127.0.0.1", apiKeys: [] }; + const status = await buildCursorIntegrationStatus( + { config, deps: { readRuntimePort: () => undefined }, url }, + [], + ); + expect(status.gateway).toMatchObject({ + baseUrl: "http://127.0.0.1:10100/v1", + apiKeyMode: "placeholder", + }); + }); + test("reports bundle effort-table provenance and unmatched model families through the server deps seam", async () => { saveConfig(statusConfig()); const server = startServer(0, { managementApi: { loadCursorEffortTable: () => fixtureEffortTable() } }); diff --git a/tests/server/api-access-endpoints.test.ts b/tests/server/api-access-endpoints.test.ts index 6eead61c3f..f1c6f7fa6d 100644 --- a/tests/server/api-access-endpoints.test.ts +++ b/tests/server/api-access-endpoints.test.ts @@ -43,10 +43,13 @@ describe("buildApiAccessEndpoints", () => { }); test("wildcard binds fall back to loopback only without request context", () => { - expect(buildApiAccessEndpoints({ hostname: "0.0.0.0", port: 10100 }).baseUrl) - .toBe("http://127.0.0.1:10100/v1"); - expect(buildApiAccessEndpoints({ hostname: "::", port: 10100 }).baseUrl) - .toBe("http://127.0.0.1:10100/v1"); + // Every all-zero spelling, not the three this file used to know: `0.0.0.0.`, `::0` and `*` + // are wildcard binds the server treats as such, and describing them as literal hostnames + // published `http://0.0.0.0.:10100` — a URL that resolves to nothing — to the GUI. + for (const hostname of ["0.0.0.0", "0.0.0.0.", "00.0.0.000", "::", "[::]", "::0", "0::", "*", "0", ""]) { + expect({ hostname, baseUrl: buildApiAccessEndpoints({ hostname, port: 10100 }).baseUrl }) + .toEqual({ hostname, baseUrl: "http://127.0.0.1:10100/v1" }); + } }); test("wildcard binds publish the request host instead of 127.0.0.1", () => { diff --git a/tests/server/loopback-companion-client-targets.test.ts b/tests/server/loopback-companion-client-targets.test.ts index 4988c1e137..36336f9fea 100644 --- a/tests/server/loopback-companion-client-targets.test.ts +++ b/tests/server/loopback-companion-client-targets.test.ts @@ -84,4 +84,26 @@ describe("companion hub: sync-managed and hardcoded clients agree", () => { expect(target.baseUrl).toBe(`http://${TAILNET_ADDRESS}:${HUB_PORT}/v1`); expect(target.requiresAdmissionToken).toBe(true); }); + + test("with no listener EVERY local writer agrees on that address, Claude included", () => { + // This is the case the first round of the local-clients fix got wrong. `ocx sync` already + // wrote the bind address here, while the hardcoded callers wrote `127.0.0.1:10100` — a + // closed port — so the two destination contracts disagreed on exactly the topology the + // issue is about. They resolve through the same rule now. + const config = { ...hubConfig(undefined), apiKeys: [ + { id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }, + ] } as OcxConfig; + const expected = `http://${TAILNET_ADDRESS}:${HUB_PORT}`; + + const codexTarget = standaloneCodexRoutingTarget(HUB_PORT, config); + expect(new URL(codexTarget.baseUrl).origin).toBe(expected); + expect(opencodeProxyBaseUrl(HUB_PORT, config.hostname, config)).toBe(`${expected}/v1`); + + const env = buildClaudeEnv({ ...config, claudeCode: { authMode: "proxy" } } as OcxConfig, HUB_PORT, {}); + expect(env.ANTHROPIC_BASE_URL).toBe(expected); + // And the credential is the same DATA-plane one Codex is told to send, never the admin + // token: `tokenEnv` on the Codex side, the configured `apiKeys` entry on this side. + expect(codexTarget.tokenEnv).toBe("OPENCODEX_API_AUTH_TOKEN"); + expect(env.ANTHROPIC_AUTH_TOKEN).toBe("ocx_data_this_proxy_key"); + }); }); diff --git a/tests/server/system-env.test.ts b/tests/server/system-env.test.ts index bdd122be00..6a3d61cb7a 100644 --- a/tests/server/system-env.test.ts +++ b/tests/server/system-env.test.ts @@ -312,15 +312,25 @@ describe("system environment injection", () => { /** * A launchd-started `claude` is a local client (#4236): on a hub bound to a tailnet address the - * only socket on 127.0.0.1 is the unauthenticated loopback listener, so that is the port the + * only credential-free socket is the unauthenticated loopback listener, so that is the port the * injected ANTHROPIC_BASE_URL must name — in the launchd domain AND in the shell env file. - * The tracking record keeps the public port separately, because that is the /healthz address. + * + * The tracking record keeps three separate facts because they genuinely separate here: `port` is + * the owning proxy's identity, `bindHost` is where its `/healthz` answers, and `clientBaseUrl` is + * what was injected. The first round of this change recorded only a `clientPort` and probed + * `127.0.0.1:` for liveness — an address that does not exist on this hub, so every + * probe failed, the record was reverted on every start, and the "another instance owns env" guard + * could never fire. */ describe("system environment local destination", () => { const hubConfig = (listener?: { enabled: boolean; port?: number }): OcxConfig => ({ ...baseConfig, + // Pinned rather than detected: the bind-address branch only injects when opencodex owns + // authentication, so an ambient subscription on the test host would hide the case. + claudeCode: { systemEnv: true, authMode: "proxy" }, hostname: "100.76.170.81", runtimeRole: "hub", + apiKeys: [{ id: "k1", name: "local", key: "ocx_data_this_proxy_key", createdAt: "2026-01-01T00:00:00Z" }], ...(listener ? { unauthenticatedLoopbackListener: listener } : {}), } as OcxConfig); @@ -333,36 +343,84 @@ describe("system environment local destination", () => { expect(await injectSystemEnv(4567, hubConfig({ enabled: true, port: 10104 }))).toEqual({ injected: true }); expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:10104"); expect(shellEnvBody()).toContain("export ANTHROPIC_BASE_URL='http://127.0.0.1:10104'"); - // port stays the proxy's identity and liveness address; clientPort records what was injected. - expect(JSON.parse(trackingFile!)).toMatchObject({ port: 4567, clientPort: 10104 }); + // port stays the proxy's identity; bindHost is its /healthz host; clientBaseUrl is what + // was injected. All three differ on this hub, which is why all three are recorded. + expect(JSON.parse(trackingFile!)).toMatchObject({ + port: 4567, + bindHost: "100.76.170.81", + clientBaseUrl: "http://127.0.0.1:10104", + }); + }); + + test("with the listener OFF the bind address is injected, with the credential it demands", async () => { + // The #4236 topology. `127.0.0.1:4567` does not exist here, so the first round's answer was + // a dead socket in the machine-wide launchd domain. + expect(await injectSystemEnv(4567, hubConfig())).toEqual({ injected: true }); + const commands = launchctlCommands(); + expect(commands).toContain("launchctl setenv ANTHROPIC_BASE_URL http://100.76.170.81:4567"); + expect(commands).toContain("launchctl setenv ANTHROPIC_AUTH_TOKEN ocx_data_this_proxy_key"); + expect(shellEnvBody()).toContain("export ANTHROPIC_BASE_URL='http://100.76.170.81:4567'"); + expect(shellEnvBody()).toContain("export ANTHROPIC_AUTH_TOKEN='ocx_data_this_proxy_key'"); + expect(JSON.parse(trackingFile!)).toMatchObject({ + port: 4567, + bindHost: "100.76.170.81", + clientBaseUrl: "http://100.76.170.81:4567", + }); + }); + + test("a destination that demands a credential nobody can supply is not injected at all", async () => { + // The launchd domain is machine-wide: a base URL that 401s every plain `claude` on the box + // is worse than no injection, so this degrades with a reason instead. + const noCredential = { ...hubConfig(), apiKeys: [] } as OcxConfig; + expect(await injectSystemEnv(4567, noCredential)) + .toEqual({ injected: false, reason: "local inference requires a data-plane credential" }); + expect(launchctlCommands().some(command => command.includes("setenv ANTHROPIC_BASE_URL"))).toBe(false); }); test("the companion form and a plain install are unchanged", async () => { expect(await injectSystemEnv(4567, hubConfig({ enabled: true }))).toEqual({ injected: true }); expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:4567"); - // No clientPort is recorded when the two are the same, so old readers see the same file. - expect(JSON.parse(trackingFile!).clientPort).toBeUndefined(); + // clientBaseUrl is omitted when it carries nothing beyond `port`; bindHost is still recorded, + // because /healthz does NOT answer on loopback here. + expect(JSON.parse(trackingFile!).clientBaseUrl).toBeUndefined(); + expect(JSON.parse(trackingFile!).bindHost).toBe("100.76.170.81"); execFileSpy.mockClear(); trackingFile = undefined; expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:4567"); - expect(JSON.parse(trackingFile!).clientPort).toBeUndefined(); + // A plain loopback install writes the byte-identical record it always did: no new fields. + const plain = JSON.parse(trackingFile!); + expect(plain.clientBaseUrl).toBeUndefined(); + expect(plain.bindHost).toBeUndefined(); }); - test("revert proves ownership against the injected port, not the tracked one", () => { + test("revert proves ownership against the injected base URL, not the tracked port", () => { trackingFile = JSON.stringify({ - pid: 123, port: 4567, clientPort: 10104, injectedAt: "2026-07-11T00:00:00.000Z", + pid: 123, port: 4567, bindHost: "100.76.170.81", + clientBaseUrl: "http://127.0.0.1:10104", injectedAt: "2026-07-11T00:00:00.000Z", }); // What launchd actually holds is the listener's port: that IS ours. launchctlBaseUrl = "http://127.0.0.1:10104"; expect(revertSystemEnv()).toEqual({ reverted: true }); }); - test("liveness is still probed on the public port, never on the listener", async () => { - // The listener serves no /healthz, so probing it would 404 and revert a LIVE proxy's env. + test("revert recognizes a bind-address injection as ours too", () => { trackingFile = JSON.stringify({ - pid: 123, port: 4567, clientPort: 10104, injectedAt: "2026-07-11T00:00:00.000Z", + pid: 123, port: 4567, bindHost: "100.76.170.81", + clientBaseUrl: "http://100.76.170.81:4567", injectedAt: "2026-07-11T00:00:00.000Z", + }); + launchctlBaseUrl = "http://100.76.170.81:4567"; + expect(revertSystemEnv()).toEqual({ reverted: true }); + }); + + test("liveness is probed on the BIND host and the public port, never on the listener", async () => { + // Two separate errors the first round made: the listener serves no /healthz (so probing its + // port 404s and reverts a LIVE proxy's env), and 127.0.0.1 is not where this proxy listens + // (so probing it failed every time and reverted on every start). + trackingFile = JSON.stringify({ + pid: 123, port: 4567, bindHost: "100.76.170.81", + clientBaseUrl: "http://127.0.0.1:10104", injectedAt: "2026-07-11T00:00:00.000Z", }); launchctlBaseUrl = "http://127.0.0.1:10104"; const probed: string[] = []; @@ -372,9 +430,38 @@ describe("system environment local destination", () => { }) as unknown as typeof fetch; expect(await cleanStaleSystemEnv()).toEqual({ cleaned: false, reason: "proxy still alive" }); - expect(probed).toEqual(["http://127.0.0.1:4567/healthz"]); + expect(probed).toEqual(["http://100.76.170.81:4567/healthz"]); expect(unlinkSpy).not.toHaveBeenCalled(); }); + + test("a record with no bindHost still probes loopback, so old records are read unchanged", async () => { + trackingFile = JSON.stringify({ pid: 123, port: 4567, injectedAt: "2026-07-11T00:00:00.000Z" }); + launchctlBaseUrl = "http://127.0.0.1:4567"; + const probed: string[] = []; + globalThis.fetch = mock(async (input: unknown) => { + probed.push(String(input)); + return new Response("ok"); + }) as unknown as typeof fetch; + + expect(await cleanStaleSystemEnv()).toEqual({ cleaned: false, reason: "proxy still alive" }); + expect(probed).toEqual(["http://127.0.0.1:4567/healthz"]); + }); + + test("a tampered bindHost cannot become a probe URL", async () => { + // This field is interpolated into a fetch URL, so the shape is validated on read. A record + // carrying a path, a scheme, or whitespace falls back to loopback instead of being dialed. + for (const bindHost of ["evil.example.com/../x", "http://evil.example.com", "a b", ""]) { + trackingFile = JSON.stringify({ pid: 123, port: 4567, bindHost, injectedAt: "2026-07-11T00:00:00.000Z" }); + launchctlBaseUrl = "http://127.0.0.1:4567"; + const probed: string[] = []; + globalThis.fetch = mock(async (input: unknown) => { + probed.push(String(input)); + return new Response("ok"); + }) as unknown as typeof fetch; + expect(await cleanStaleSystemEnv()).toEqual({ cleaned: false, reason: "proxy still alive" }); + expect({ bindHost, probed }).toEqual({ bindHost, probed: ["http://127.0.0.1:4567/healthz"] }); + } + }); }); describe("system environment cleanup", () => { diff --git a/tests/vision/vision-routed.test.ts b/tests/vision/vision-routed.test.ts index f11473b8f9..d742345d8f 100644 --- a/tests/vision/vision-routed.test.ts +++ b/tests/vision/vision-routed.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { saveConfig } from "../../src/config"; +import { configuredPort, setCorsOrigin } from "../../src/server/auth-cors"; import { parseRequest } from "../../src/responses/parser"; import { startServer } from "../../src/server"; import type { OcxConfig, OcxProviderConfig } from "../../src/types"; @@ -92,25 +93,51 @@ describe("describeImageRouted unit", () => { /** * The self-fetch is a local client too (#4236): on a hub bound to a tailnet address the only - * socket on 127.0.0.1 is the unauthenticated loopback listener, which now admits the chat - * wire this helper speaks. Three topologies, one destination each. + * credential-free socket is the unauthenticated loopback listener, which now admits the chat + * wire this helper speaks — and with no listener the destination is the bind address, because + * nothing answers on loopback at all. One destination per topology. */ test("the self-fetch base URL follows the unauthenticated loopback listener", () => { expect(routedDescribeBaseUrl({ port: 10100, + hostname: "100.76.170.81", unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, })).toBe("http://127.0.0.1:10104"); expect(routedDescribeBaseUrl({ port: 10100, + hostname: "100.76.170.81", unauthenticatedLoopbackListener: { enabled: true }, })).toBe("http://127.0.0.1:10100"); + // Listener off on a tailnet bind: the bind address, not a closed loopback port. expect(routedDescribeBaseUrl({ port: 10100, + hostname: "100.76.170.81", unauthenticatedLoopbackListener: { enabled: false }, - })).toBe("http://127.0.0.1:10100"); + })).toBe("http://100.76.170.81:10100"); expect(routedDescribeBaseUrl({ port: 10100 })).toBe("http://127.0.0.1:10100"); }); + test("a config with no usable port falls back to the default, never to port 0", () => { + // `_corsOrigin` carries no explicit port until the server records one, so `configuredPort()` + // returns "" and an unguarded `Number(configuredPort())` composed `http://127.0.0.1:0` — a + // URL that connects to nothing. Pin that exact state rather than whatever a sibling test's + // server left behind in this module-level global. + const restore = configuredPort(); + try { + // A recorded port is used as-is. + setCorsOrigin(4321); + expect(routedDescribeBaseUrl({ port: 0 })).toBe("http://127.0.0.1:4321"); + // An ephemeral bind that has not recorded one yet yields "0", whose Number() is 0. + setCorsOrigin(0); + expect(Number(configuredPort())).toBe(0); + const url = new URL(routedDescribeBaseUrl({ port: 0 })); + expect(url.port).not.toBe("0"); + expect(url.href).toBe("http://127.0.0.1:10100/"); + } finally { + setCorsOrigin(Number(restore) || 10_100); + } + }); + test("the vision plan carries the listener through to the self-fetch", () => { // The planner hands `describeImageRouted` a NARROWED config. Dropping the listener there // would leave the resolver nothing to resolve and silently restore the closed port. @@ -143,7 +170,12 @@ describe("describeImageRouted unit", () => { } as unknown as OcxConfig, routed, "text-model", request); expect(plan?.backend).toBe("routed"); expect(plan?.routedConfig?.unauthenticatedLoopbackListener).toEqual({ enabled: true, port: 10104 }); + // `hostname` has to survive the narrowing for the same reason: with no listener it is the + // ONLY field that distinguishes a reachable destination from a closed loopback port. + expect(plan?.routedConfig?.hostname).toBe("100.76.170.81"); expect(routedDescribeBaseUrl(plan!.routedConfig!)).toBe("http://127.0.0.1:10104"); + expect(routedDescribeBaseUrl({ ...plan!.routedConfig!, unauthenticatedLoopbackListener: undefined })) + .toBe("http://100.76.170.81:10100"); }); test("admission ladder: env token first, then first apiKeys entry, as x-opencodex-api-key", () => { From 6d555ffada01da8da484d7c3084dac13df602dda Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:19:50 +0900 Subject: [PATCH 6/8] feat(server): admit POST /v1/messages/count_tokens on the loopback listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first revision withheld it on a scope argument and pinned the 404 so widening it would be deliberate. Review took that option: scope is not a confinement argument here. `count_tokens` spends no provider quota, reaches no stored credential, and returns a count computed from the request body the caller already holds. Withholding it bought nothing — the same caller may POST the entire conversation to `/v1/messages` on this socket — while costing Claude Code its server-side count, which it silently replaces with a local estimate. So the Anthropic wire is complete on the listener now. `/api/*`, `/healthz`, `/readyz` and the GUI stay 404, which is the boundary that actually matters and the one the reviewer conditioned on. The pinned 404 test became a pinned reachability test in the same loop as the other two wires — `not 404` and `not 401`, because either alone stays green if the route is silently dropped from the allowlist — and the `GET` form is still in the denied list, so "admit the path" cannot quietly become "admit the path for any method". Refs #4236 Co-Authored-By: Claude Fable 5.1 --- src/server/index.ts | 9 +++++++++ tests/server/loopback-listener-admission.test.ts | 8 +++++++- tests/server/loopback-listener-integration.test.ts | 13 +++++++++---- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 6301f90d07..e5d6b182f6 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -833,6 +833,14 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { describe("local client inference wires on the loopback listener (#4236)", () => { const source = readFileSync(new URL("../../src/server/index.ts", import.meta.url), "utf8"); - test("the allowlist admits both wires as POST and nothing else about them", () => { + test("the allowlist admits all three wires as POST and nothing else about them", () => { // The allowlist is a closure inside startServer, so this reads the entry itself. The // integration file proves the socket behaviour; this pins the SHAPE, because "admit the // path" and "admit the path for any method" are one character apart. expect(source).toContain( 'if (path === "/v1/messages" || path === "/v1/chat/completions") return req.method === "POST";', ); + // `count_tokens` completes the Anthropic wire: no provider quota, no stored credential, and + // a count the caller could compute from the body it already holds. Withholding it only cost + // Claude Code its server-side count; the boundary that matters is `/api/*` below. + expect(source).toContain( + 'if (path === "/v1/messages/count_tokens") return req.method === "POST";', + ); }); test("no /api route joins the allowlist", () => { diff --git a/tests/server/loopback-listener-integration.test.ts b/tests/server/loopback-listener-integration.test.ts index 4dff1a2524..7411e1559d 100644 --- a/tests/server/loopback-listener-integration.test.ts +++ b/tests/server/loopback-listener-integration.test.ts @@ -291,10 +291,10 @@ describe("unauthenticated loopback listener", () => { { method: "GET", path: "/healthz" }, { method: "GET", path: "/readyz" }, { method: "GET", path: "/v1/opencodex/artifacts/x" }, - // The two inference wires are admitted as POST only (see the dedicated test below). + // The inference wires are admitted as POST only (see the dedicated test below). { method: "GET", path: "/v1/messages" }, { method: "GET", path: "/v1/chat/completions" }, - { method: "POST", path: "/v1/messages/count_tokens", body: '{"model":"x","messages":[]}' }, + { method: "GET", path: "/v1/messages/count_tokens" }, // Voice call-create is admitted only as POST; the keyed sideband join only as an upgrade. { method: "GET", path: "/v1/live/rtc_x" }, { method: "GET", path: "/v1/realtime/calls/rtc_x" }, @@ -466,18 +466,23 @@ describe("unauthenticated loopback listener", () => { } }); - test("admits the two local client inference wires, and still refuses /api/* (#4236)", async () => { + test("admits the local client inference wires, and still refuses /api/* (#4236)", async () => { // The hub's own local clients do not speak Responses: `ocx claude`, the system-env // injection and Claude Desktop speak the Anthropic wire, Cursor / the vision helper / // aside speak OpenAI chat. On a tailnet-bound hub this listener is their only local // socket, so a 404 here is the whole "Codex works but nothing else does" defect. + // + // `count_tokens` completes the Anthropic wire. It spends no provider quota and reaches no + // stored credential, so withholding it bought no confinement — the same caller may POST the + // entire conversation to `/v1/messages` on this socket — while costing Claude Code its + // server-side count. const loopbackPort = await freePort(); saveConfig(baseConfig(loopbackPort)); const server = await startLoopbackTestServer(loopbackPort); const base = `http://127.0.0.1:${loopbackPort}`; const publicBase = `http://127.0.0.1:${server.port}`; try { - for (const path of ["/v1/messages", "/v1/chat/completions"]) { + for (const path of ["/v1/messages", "/v1/messages/count_tokens", "/v1/chat/completions"]) { const viaPublic = await fetch(`${publicBase}${path}`, { method: "POST", headers: { "content-type": "application/json" }, From 59c45c1504579166af0571d62dad1bf7d7278aea Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:19:50 +0900 Subject: [PATCH 7/8] docs(structure,devlog): record the local-destination review round MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `structure/01_runtime.md`: the three-socket paragraph was one run-on block; split into four, and the data-loopback allowlist now names `count_tokens` alongside the two other wires. `structure/09_client-integrations.md`: states that `fetchClaudeCodeState` is the management resolver's caller — it sends the admin token to the hub's management ingress, or without one to the bind address, both host-local and never exported — and records the inference resolver's credential contract, including that a wildcard or tailnet bind demands the data-plane ladder and never the admin token. Devlog 030 gains a "Review round" section: the six findings, why the missing bind-address fallback left the reported topology broken while the ported form worked, what each of the eight call sites now does with `requiresAdmissionToken`, the tracking-record change, the `count_tokens` reversal, and the exact commands and counts at the pushed head. The two former follow-ups are closed; the GUI API-access copy on a credential-demanding destination is recorded as PR4's. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- .../030_hub_local_clients.md | 171 ++++++++++++++---- structure/01_runtime.md | 27 ++- structure/09_client-integrations.md | 28 ++- 3 files changed, 175 insertions(+), 51 deletions(-) diff --git a/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md index 87175be8b7..ef04633e06 100644 --- a/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md +++ b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md @@ -17,19 +17,26 @@ both halves — separately, because they are not the same surface. ## The two contracts -1. **Inference** — `localInferenceOrigin(config, publicPort)`: `http://127.0.0.1:` when `unauthenticatedLoopbackListener` is enabled, otherwise - `http://127.0.0.1:`. That listener admits local callers with no credential, so - nothing has to export one. +1. **Inference** — `localInferenceDestination(config, publicPort)` → `{ origin, port, + requiresAdmissionToken }`. `http://127.0.0.1:` with no credential when + `unauthenticatedLoopbackListener` is enabled; `http://127.0.0.1:` with no + credential on a loopback bind; otherwise `http://:` AND a + data-plane credential is required (a wildcard bind lands here too — it answers on 127.0.0.1, + but the public listener still demands admission). 2. **Management** — `localManagementOrigin(config, publicPort)`: `http://127.0.0.1:` on a hub with the ingress enabled, otherwise `http://:`. The caller still sends the admin token; management authentication has no loopback bypass (`structure/05`), and the unauthenticated listener serves no `/api/*` — by design, not by omission. +The two share the same fallback shape on purpose. See "Review round" below: the first revision of +this unit gave inference no bind-address fallback at all, which left the exact topology the issue +is about still broken. + Both live in `src/lib/local-destinations.ts`, one small module whose header states the split, next -to the existing `local-management-*` helpers. It reuses PR2's `effectiveLoopbackListenerPort` and -`probeHostname`; no call site repeats `?? port`. +to the existing `local-management-*` helpers. It reuses PR2's `effectiveLoopbackListenerPort`, +`isWildcardHostname`, `shouldInjectApiAuthHeader` and `probeHostname`; no call site repeats +`?? port`, and none of them re-derives "does this bind need a key?". ## What shipped @@ -42,8 +49,8 @@ already resolve admission from the RECEIVING listener's `RequestPolicyView` — and the same loopback short-circuit `/v1/responses` uses — so this adds a wire, not a trust level. The allowlist comment says why, in the shape the existing entries use. -Nothing else was added: `/api/*`, `/healthz`, `/readyz`, the GUI and -`POST /v1/messages/count_tokens` all still 404 there. +`POST /v1/messages/count_tokens` joined them in the review round (below). `/api/*`, `/healthz`, +`/readyz` and the GUI still 404 there. One consistency fix rode along: the chat-completions branch finished its CORS with `config` instead of the request's `policy`. On the public listener those are the same object, so this is a @@ -54,14 +61,14 @@ admission decision and headers derived from a bind address that did not receive | Site | Before | After | | --- | --- | --- | -| `buildClaudeEnv` (`src/cli/claude.ts`) | `http://127.0.0.1:${publicPort}` | `localInferenceOrigin` | +| `buildClaudeEnv` (`src/cli/claude.ts`) | `http://127.0.0.1:${publicPort}` | `localInferenceDestination` + `ANTHROPIC_AUTH_TOKEN` | | `fetchClaudeCodeState` | `http://127.0.0.1:${publicPort}/api/claude-code` | `localManagementOrigin` + admin token | -| `writeDesktop3pConfig` → `generateDesktop3pConfig` | public port | `localInferencePort(latest.config, port)` | -| `refreshGatewayModelCacheFromProxy` | public port | `localInferenceOrigin(options.admissionConfig, port)` | -| `injectSystemEnv` / `writeShellEnvFile` | public port | `localInferencePort` / `localInferenceOrigin` | -| Cursor gateway card | `http://127.0.0.1:${port}/v1` | `localInferencePort` | -| `resolveApiAccessBaseUrl` (final loopback fallback only) | `http://127.0.0.1:${port}/v1` | `localInferenceOrigin` | -| `routedDescribeBaseUrl` | public port | `localInferenceOrigin` | +| `writeDesktop3pConfig` → `generateDesktop3pConfig` | public port | `localInferenceDestination` + gateway api key | +| `refreshGatewayModelCacheFromProxy` | public port | `localInferenceDestination` + `x-opencodex-api-key` | +| `injectSystemEnv` / `writeShellEnvFile` | public port | `localInferenceDestination` + `ANTHROPIC_AUTH_TOKEN`, or a skip with a reason | +| Cursor gateway card | `http://127.0.0.1:${port}/v1` | `localInferenceDestination` + `apiKeyMode` | +| `resolveApiAccessBaseUrl` (final loopback fallback only) | `http://127.0.0.1:${port}/v1` | `localInferenceDestination` | +| `routedDescribeBaseUrl` | public port | `localInferenceDestination` + `x-opencodex-api-key` | Three consequences worth naming: @@ -72,12 +79,14 @@ Three consequences worth naming: - **The gateway-model cache had to move with `buildClaudeEnv`.** Claude Code honors that file only while its `baseUrl` equals `ANTHROPIC_BASE_URL`; moving one without the other would have left the picker on a stale list. -- **`system-env` tracking now records two ports.** `port` stays the owning proxy's identity and - its `/healthz` address; new optional `clientPort` records what was injected. Ownership on revert - is proven against `clientPort ?? port`, liveness is still probed on `port` — the listener serves - no `/healthz`, so probing the injected port would declare a live proxy stale and revert its - environment. Records written before this field are unchanged, and no `clientPort` is written - when the two ports are equal. +- **`system-env` tracking records three facts.** `port` is the owning proxy's identity, new + optional `bindHost` is where its `/healthz` answers, and new optional `clientBaseUrl` is what was + injected. Ownership on revert is proven against `clientBaseUrl ?? http://127.0.0.1:`; + liveness is probed at `bindHost`/`port`, because the loopback listener serves no `/healthz` (so + probing the injected port would declare a live proxy stale) and 127.0.0.1 is not where a + tailnet-bound hub listens (so probing it failed every time). Both fields are omitted when they + carry nothing beyond `port`, so a plain loopback install writes a byte-identical record. + `bindHost` is interpolated into a probe URL, so its shape is validated on read. `resolveApiAccessBaseUrl` was touched ONLY in its last-resort loopback branch. Every branch above it describes the address the *client* reached, and a remote caller must never be handed a port @@ -95,26 +104,82 @@ self-fetch would have silently gone back to the closed port. A test pins the nar would have meant either `/api/*` on the unauthenticated listener or an admin token in exported client configuration. Neither happens: the management resolver never returns the listener's port, and no exported configuration gained a credential. -- **`count_tokens` is NOT admitted.** Scope said the two wires and nothing else, so - `POST /v1/messages/count_tokens` still 404s on the listener. Claude Code degrades to local - estimation when that route is unavailable, so this is a cosmetic loss rather than a broken - launch — but it is the one obvious follow-up candidate, and a test now pins the current answer - so widening it is a deliberate act. +- **`count_tokens` was not admitted in the first revision, and is now.** See "Review round": the + argument for withholding it was scope, and scope is not a confinement argument when the same + caller may POST the whole conversation to `/v1/messages` on the same socket. - **Desktop/Cursor/system-env resolve where the config is, not in the generator.** `generateDesktop3pConfig` stays a pure generator taking "the local port to dial"; `writeDesktop3pConfig` resolves from the config it already re-reads under the mutation lock, so every caller gets the same answer and no caller has to be taught about listeners. -- **Cursor's `apiKeyMode` was left alone.** It still describes the public bind's admission rule. - Pasting a credential into the unauthenticated listener is harmless; omitting one on a bind that - demands it is not. Changing that copy is a GUI decision, not part of this fix. +- **Cursor's `apiKeyMode` now describes the destination it resolved.** It used to describe the + public bind's admission rule, which was harmless only while the card always showed a loopback + URL. Once the card can show the bind address, "here is the URL" and "you need no key for it" + would be a contradiction, so the flag reads `gateway.requiresAdmissionToken` as well. - **A restart is required for the ported form.** Verified against the live hub: the running pre-PR3 proxy answers `404` for `POST /v1/messages` on `127.0.0.1:10104` while `GET /v1/models` is `200`. After this PR the same request is served, so operators on the ported form must restart (macOS: `launchctl kickstart -k gui/$uid/com.opencodex.proxy`) before `ocx claude` can use the listener. Noted for the PR4 docs unit. +## Review round (second revision, same branch — new commits, no rewrite) + +Six findings, all on the shape of the resolution rather than on the split itself. + +1. **`localInferenceOrigin` had no bind-address fallback** (should-fix). With the listener OFF and + `hostname` a tailnet address, all eight sites still got `http://127.0.0.1:` — a + dead socket — and `tests/lib/local-destinations.test.ts` pinned that as intended. This is the + topology #4236 is *about*, so the unit closed the ported form and left the reported one open. + The resolver now mirrors `localManagementOrigin`'s shape and returns + `{ origin, port, requiresAdmissionToken }`; `requiresAdmissionToken` is + `shouldInjectApiAuthHeader`, the predicate that already encoded exactly this question, so the + two cannot drift. Every call site then either attaches the data-plane credential — the + `OPENCODEX_API_AUTH_TOKEN` / hardened service-token-file / `apiKeys` ladder, shared as + `localAdmissionToken` with `refreshGatewayModelCacheFromProxy` and + `routedDescribeAdmissionToken`, and **never the admin token** — or degrades with a log line + naming the destination and the two fixes. `injectSystemEnv` degrades hardest: it returns + `{ injected: false, reason }` rather than write a machine-wide base URL that 401s every plain + `claude`, because a subscription launch cannot carry a host token at all (#253). + `targetsLocalClaudeProxy` gained the destination origin as a second way to be ours, and the + port set became `localLoopbackInferencePorts` — the ports that actually answer on 127.0.0.1, + which is EMPTY on a tailnet bind with no listener, so a leftover `http://127.0.0.1:10100` is + correctly rewritten instead of preserved. `buildNativeClaudeEnv` keeps a wider set: shedding + asks "could we have written this?", and leaving such a URL behind with its token stripped is + worse than shedding one port too many. +2. **`cleanStaleSystemEnv` probed an address that does not exist** (should-fix). It dialed + `127.0.0.1:`, so on a tailnet-bound hub every liveness probe failed, the record + was reverted on every start, and the "another instance owns env" guard could never fire — while + the comment and test asserted the opposite. The tracking record now carries `bindHost`, the + probe uses it, and the record's `clientPort` was replaced by the full `clientBaseUrl` (the field + shipped in this PR only, so nothing released reads it). +3. **`probeHostname` knew three wildcard spellings** while `isWildcardHostname` (PR2) knew every + all-zero form, so `0.0.0.0.`, `::0` and `*` were composed into literal URLs that resolve to + nothing. `probeHostname` and `api-access.ts`'s `isWildcardBindHost` both call the shared + predicate now; the (e) tests enumerate nine spellings. +4. **`Number(configuredPort())` could be `0`** in `routedDescribeBaseUrl` when `_corsOrigin` has no + explicit port, composing `http://127.0.0.1:0`. Guarded with `|| 10_100`. +5. **`POST /v1/messages/count_tokens` is admitted on the loopback listener.** It spends no provider + quota and reaches no stored credential, and withholding it bought no confinement — the same + caller may POST the entire conversation to `/v1/messages` on that socket — while costing Claude + Code its server-side count. The pinned 404 test became a pinned reachability test and the + allowlist comment carries the argument. `/api/*`, `/healthz`, `/readyz` and the GUI stay 404. +6. **Docs**: `structure/01_runtime.md`'s socket paragraph was split into four; `structure/09` now + states that `fetchClaudeCodeState` sends the admin token to the management ingress or, without + one, to the bind address — host-local, never exported — and records the inference resolver's + credential contract. + +One hardening rode along with (1). `localAdmissionToken` shape-checks the token it reads from the +service token *file* (`/^[A-Za-z0-9._~+/=-]{8,4096}$/`) before sending it as a credential. A path +can be pointed at or replaced by something that is not a credential at all, and putting that in a +request header leaks file contents. The environment variable and configured `apiKeys` pass through +verbatim, so no existing key can be broken by the check. + +`localInferencePort` was removed rather than kept as a wrapper: a bare port cannot express a +bind-address destination, so an exported convenience that returns one is a trap. + ## Verification (exact commands, this branch) +First revision (counts superseded by the review round below, kept as the record): + ``` bun run typecheck # clean bun run privacy:scan # Privacy scan passed @@ -147,6 +212,46 @@ bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 17 pass bun test tests/ci-workflows/docs-remote-hub-claims.test.ts # 7 pass ``` +Review round, at the pushed head: + +``` +bun run typecheck # clean +bun run privacy:scan # Privacy scan passed +bun test tests/lib/local-destinations.test.ts \ + tests/server/loopback-listener-admission.test.ts \ + tests/server/loopback-listener-integration.test.ts \ + tests/server/system-env.test.ts \ + tests/claude-integration/claude-cli.test.ts \ + tests/clients/desktop-3p.test.ts \ + tests/server/api-access-endpoints.test.ts \ + tests/providers/cursor/cursor-integration-status.test.ts \ + tests/vision/vision-routed.test.ts \ + tests/server/loopback-companion-client-targets.test.ts # 241 pass +bun test tests/claude-integration/claude-gateway-cache.test.ts \ + tests/claude-integration/claude-system-env-auto.test.ts \ + tests/claude-integration/claude-shell-hook.test.ts \ + tests/claude-integration/claude-management-api.test.ts \ + tests/clients/desktop-3p-guard.test.ts \ + tests/clients/desktop-remote-store.test.ts \ + tests/clients/sync-client-integrations.test.ts \ + tests/codex-integration/native-claude-desktop-toggle.test.ts # 122 pass +bun test tests/server/proxy-liveness.test.ts \ + tests/codex-integration/codex-inject.test.ts \ + tests/codex-integration/codex-inject-integration.test.ts \ + tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 215 pass +bun test tests/cli/cli-management-auth.test.ts \ + tests/claude-integration/claude-auth-detect.test.ts \ + tests/claude-integration/claude-auth-mode.test.ts \ + tests/server/api-keys-routes.test.ts \ + tests/providers/cursor/cursor-effort-rows.test.ts \ + tests/ci-workflows/docs-remote-hub-claims.test.ts # 125 pass +``` + +`tests/lib/local-destinations.test.ts` now enumerates the review's six configurations — standalone +loopback, companion hub, ported hub, listener-off + non-loopback bind, wildcard bind, client role — +as one table driving both the destination and the loopback-port-set assertions, so a new branch +that forgets `requiresAdmissionToken` fails there rather than in production. + New test file registered in `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`: `tests/lib/local-destinations.test.ts` → `lib`. @@ -182,6 +287,8 @@ loopback, but returns nothing on a tailnet-bound hub, and `ocx claude` then laun operator-facing copy. The docs note that matters: restart after changing `unauthenticatedLoopbackListener`, and the ko copies of `reference/configuration/server.md` still describe only the ported form (PR2's note). -- `POST /v1/messages/count_tokens` on the listener: decide whether the Anthropic wire should be - complete there. Currently pinned as 404. -- Cursor's `apiKeyMode` wording when the resolved base URL is the unauthenticated listener. +- Both former follow-ups are closed in the review round above: `count_tokens` is admitted, and + Cursor's `apiKeyMode` now keys on the resolved destination. +- Open, and deliberately not in this unit: `resolveApiAccessBaseUrl` describes the GUI's API-access + panel, and on a credential-demanding destination the panel's copy is the PR4/docs decision, not a + resolver change. diff --git a/structure/01_runtime.md b/structure/01_runtime.md index a460da4541..32cf8036c8 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -58,16 +58,23 @@ uninstall still restore. `startServer` composes up to three sockets in one synchronous startup transaction: the public data listener, the optional unauthenticated data-loopback listener, and the optional hub-management -listener. The data-loopback socket serves a fixed data-plane allowlist: Responses and its compact -sibling, the native search relay, the standalone Images POSTs, `GET /v1/models`, the realtime voice -shapes, and `POST /v1/messages` plus `POST /v1/chat/completions` — the two inference wires the -host's own local clients speak. It never serves `/api/*`, `/healthz`, `/readyz`, or GUI routes, so -local management discovery has to use an authenticated surface with a management credential. The hub-management socket is enabled only by `runtimeRole: "hub"` plus -`hub.managementIngress.enabled`, always binds `127.0.0.1`, and default-denies everything except GUI, -session bootstrap/exchange, and `/api/*`. A failed optional bind initiates rollback of every earlier -socket; normal stop joins all bound sockets before lifecycle release. The existing launchd/systemd -installer remains the service owner and continues loading the data token from `service-api-token`; -hub mode adds no service-manager fork and no token-bearing unit/plist field. +listener. + +The data-loopback socket serves a fixed data-plane allowlist: Responses and its compact sibling, +the native search relay, the standalone Images POSTs, `GET /v1/models`, the realtime voice shapes, +and the Anthropic and OpenAI chat wires the host's own local clients speak — `POST /v1/messages`, +`POST /v1/messages/count_tokens`, and `POST /v1/chat/completions`. It never serves `/api/*`, +`/healthz`, `/readyz`, or GUI routes, so local management discovery has to use an authenticated +surface with a management credential. + +The hub-management socket is enabled only by `runtimeRole: "hub"` plus +`hub.managementIngress.enabled`, always binds `127.0.0.1`, and default-denies everything except +GUI, session bootstrap/exchange, and `/api/*`. + +A failed optional bind initiates rollback of every earlier socket; normal stop joins all bound +sockets before lifecycle release. The existing launchd/systemd installer remains the service owner +and continues loading the data token from `service-api-token`; hub mode adds no service-manager +fork and no token-bearing unit/plist field. [Decision Log] - 목적과 의도: Give a headless hub a browser management ingress without widening its data plane or trusting spoofable forwarding headers on the public listener. diff --git a/structure/09_client-integrations.md b/structure/09_client-integrations.md index a5b535a464..8e8c458580 100644 --- a/structure/09_client-integrations.md +++ b/structure/09_client-integrations.md @@ -152,17 +152,27 @@ Remote clients journal and restore native integrations locally while model traff ## Local destinations on a hub A client running on the proxy's own machine has two destinations, and they are resolved -separately by `src/lib/local-destinations.ts`. Inference (`localInferenceOrigin`) is +separately by `src/lib/local-destinations.ts`. Inference (`localInferenceDestination`) is `127.0.0.1` on the unauthenticated loopback listener's effective port when that listener is -enabled, otherwise the public port; a hub bound to a tailnet or LAN address has no other local -data socket, so `ocx claude`, the `system-env` injection, the Claude Desktop profile, the Cursor -gateway value, the gateway-model cache, the routed vision self-fetch and the API-access loopback -fallback all go through that resolver rather than composing the port themselves. Management +enabled, otherwise the public port on the bind address — `127.0.0.1` for a loopback or wildcard +bind, and the tailnet or LAN address otherwise, where no loopback data socket exists at all. So +`ocx claude`, the `system-env` injection, the Claude Desktop profile, the Cursor gateway value, +the gateway-model cache, the routed vision self-fetch and the API-access loopback fallback all go +through that resolver rather than composing the port themselves. Management (`localManagementOrigin`) is the hub's loopback `hub.managementIngress` when enabled, otherwise -the public bind address, and the caller supplies the management credential. Management -authentication has no loopback bypass and the data-loopback listener serves no `/api/*`, so these -two must never be collapsed into one base URL, and an admin credential must never be written into -an exported client configuration. +the public bind address, and the caller supplies the management credential. `fetchClaudeCodeState` +is that resolver's caller: it sends the local admin token to the management ingress, or — with no +ingress — to the bind address, and either destination is host-local and never reaches an exported +client configuration. Management authentication has no loopback bypass and the data-loopback +listener serves no `/api/*`, so these two must never be collapsed into one base URL, and an admin +credential must never be written into an exported client configuration. + +Both resolvers share the same fallback shape, and the inference one additionally reports whether +its destination demands data-plane admission: the loopback listener and a genuinely loopback bind +need no credential, while a wildcard or tailnet bind does. Each caller either attaches that +credential — the `OPENCODEX_API_AUTH_TOKEN` / service-token-file / `apiKeys` ladder, never the +admin token — or logs that it is degrading. Composing `http://127.0.0.1:` by hand is +what produced a dead socket on a tailnet-bound hub in the first place. ## Connected Claude Desktop profiles From 0fd205f0658c7658c963a5b15d53bb9c4830ecf6 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:21:09 +0900 Subject: [PATCH 8/8] test(server): the translated wires now prove Reserve admission on the listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught the two translated-wire cases in `tests/server/reserve-ingress.test.ts` asserting a local 404 for `POST /v1/messages` and `POST /v1/chat/completions`. That 404 came from the loopback listener's allowlist, and the test said so: "this local 404 does NOT prove admission propagation inside the translated handler." Both wires are served there now, so the request reaches the handler and gives the answer the 404 was standing in for — the same one the Responses transport already gives on that listener. Loopback admission makes Reserve eligible, so the turn is refused `429` behind a WHAM probe with nothing reaching the upstream, while the public listener's `dedicated` admission is not eligible and forwards the caller's own credential with no probe at all. The describe block's invariant is unchanged and now better covered: eligibility is decided by the RECEIVING listener's admission, not by the dial address — both requests leave from 127.0.0.1 with the same credential and only the socket differs. One stale comment corrected in the same file: chat IS served by the secondary listener now, so the `primaryLoopback` fixture is used because it proves the terminal refusal inside the handler, not because chat is unreachable elsewhere. Refs #4236 Co-Authored-By: Claude Fable 5.1 --- .../030_hub_local_clients.md | 24 +++++++++++++++++ tests/server/reserve-ingress.test.ts | 26 ++++++++++++++----- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md index ef04633e06..a797a24b04 100644 --- a/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md +++ b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md @@ -162,6 +162,18 @@ Six findings, all on the shape of the resolution rather than on the split itself caller may POST the entire conversation to `/v1/messages` on that socket — while costing Claude Code its server-side count. The pinned 404 test became a pinned reachability test and the allowlist comment carries the argument. `/api/*`, `/healthz`, `/readyz` and the GUI stay 404. + + Widening the allowlist also turned a weaker assertion in + `tests/server/reserve-ingress.test.ts` into a real one. Its two translated-wire cases asserted a + local 404 and said outright that it "does NOT prove admission propagation inside the translated + handler" — the 404 came from the allowlist, not from the handler. With the wires served, those + requests reach the handler and give the same answer the Responses transport already gives on + that listener: loopback admission makes Reserve eligible, so the turn is refused 429 behind a + WHAM probe with nothing reaching the upstream, while the public listener's `dedicated` admission + is not eligible and forwards the caller's own credential. The describe block's invariant — + eligibility is decided by the RECEIVING listener, not the dial address — is now proven on four + transports instead of two. One stale comment in the same file ("chat is intentionally not served + by the secondary listener") was corrected. 6. **Docs**: `structure/01_runtime.md`'s socket paragraph was split into four; `structure/09` now states that `fetchClaudeCodeState` sends the admin token to the management ingress or, without one, to the bind address — host-local, never exported — and records the inference resolver's @@ -227,6 +239,18 @@ bun test tests/lib/local-destinations.test.ts \ tests/providers/cursor/cursor-integration-status.test.ts \ tests/vision/vision-routed.test.ts \ tests/server/loopback-companion-client-targets.test.ts # 241 pass +bun test tests/server/reserve-ingress.test.ts # 32 pass +bun test tests/cli/cli-export-command.test.ts \ + tests/cli/hub-gated-local-clients.test.ts \ + tests/clients/integrations-writer.test.ts \ + tests/codex-integration/codex-desired-state.test.ts \ + tests/codex-integration/reserve-auth-context.test.ts \ + tests/codex-integration/reserve-catalog.test.ts \ + tests/codex-integration/reserve-dispatch.test.ts \ + tests/codex-integration/reserve-helper-boundary.test.ts \ + tests/providers/xai/grok-sync.test.ts \ + tests/server/management-client-config-route.test.ts \ + tests/server/reserve-claude-policy.test.ts # 245 pass bun test tests/claude-integration/claude-gateway-cache.test.ts \ tests/claude-integration/claude-system-env-auto.test.ts \ tests/claude-integration/claude-shell-hook.test.ts \ diff --git a/tests/server/reserve-ingress.test.ts b/tests/server/reserve-ingress.test.ts index 5ca09a58de..f46e7686f7 100644 --- a/tests/server/reserve-ingress.test.ts +++ b/tests/server/reserve-ingress.test.ts @@ -175,18 +175,32 @@ describe("Reserve eligibility trusts receiving-listener admission", () => { } finally { await fixture.close(); } }, SERVER_BUDGET_MS); - test.each(["chat", "messages"] as const)("translated %s: public has no Reserve WHAM; local allowlist refuses", async transport => { + test.each(["chat", "messages"] as const)("translated %s: public has no Reserve WHAM; local listener does", async transport => { + // The invariant is the one this whole describe block is about: eligibility is decided by the + // RECEIVING listener's admission, not by the dial address. Both requests below leave from + // 127.0.0.1 with the same credential; only the socket differs. + // + // This case used to assert a local 404, because the unauthenticated loopback listener did not + // serve the translated wires at all — and said so: "this local 404 does NOT prove admission + // propagation inside the translated handler." It is served now (#4236, the hub's own local + // clients speak these two wires and nothing else answers them on a tailnet-bound hub), so the + // weaker assertion is replaced by the one the 404 was standing in for: the same 429-behind-a- + // WHAM-probe answer the Responses transport already gives on this listener. const fixture = await reserveIngressFixture(); try { const before = snapshot(fixture.counters); const publicResult = await fixture.request("public", transport, "gpt-reserve", headers("dedicated")); + // Public admission is `dedicated`, so Reserve is not eligible and the caller's own + // forwarded credential reaches inference with no WHAM probe at all. expect(publicResult.status).toBe(200); expect(delta(fixture.counters, before)).toMatchObject({ wham: 0, inference: 1 }); const localBefore = snapshot(fixture.counters); const localResult = await fixture.request("local", transport, "gpt-reserve", headers("dedicated")); - expect(localResult.status).toBe(404); - expect(delta(fixture.counters, localBefore)).toEqual({ wham: 0, credential: 0, tokenRead: 0, inference: 0 }); - // This local 404 does NOT prove admission propagation inside the translated handler. + // Loopback admission IS eligible, so the handler probes Reserve availability and refuses + // the turn rather than spending the account — and no request reaches the upstream. + expect(localResult.status).toBe(429); + expect(localResult.text).toContain("Reserve"); + expect(delta(fixture.counters, localBefore)).toMatchObject({ wham: 1, inference: 0 }); fixture.assertConfigUnchanged(); } finally { await fixture.close(); } }, SERVER_BUDGET_MS); @@ -199,8 +213,8 @@ describe("terminal routed vision helpers cannot spend Reserve", () => { ["chat", "openai/gpt-reserve"], ["chat", "main/gpt-reserve"], ["responses", "openai/gpt-reserve"], ["responses", "main/gpt-reserve"], ] as const)("%s %s refuses before credential enrichment", async (transport, model) => { - // Chat is intentionally not served by the secondary listener; use an actual primary - // loopback bind so this tests the handler, not the secondary listener's 404 allowlist. + // A primary LOOPBACK bind, so the terminal refusal is proven inside the handler on a + // request the public listener admitted as loopback — not by any secondary-listener gate. const fixture = await reserveIngressFixture({ primaryLoopback: true }); try { fixture.allow(); // A permission denial must not accidentally make this test green.