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..a797a24b04 --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/030_hub_local_clients.md @@ -0,0 +1,318 @@ +# 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** — `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`, +`isWildcardHostname`, `shouldInjectApiAuthHeader` and `probeHostname`; no call site repeats +`?? port`, and none of them re-derives "does this bind need a key?". + +## 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. + +`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 +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}` | `localInferenceDestination` + `ANTHROPIC_AUTH_TOKEN` | +| `fetchClaudeCodeState` | `http://127.0.0.1:${publicPort}/api/claude-code` | `localManagementOrigin` + admin token | +| `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: + +- **`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 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 +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` 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` 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. + + 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 + 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 +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 +``` + +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/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 \ + 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`. + +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). +- 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/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/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/src/claude/desktop-3p.ts b/src/claude/desktop-3p.ts index 7163ca2fa1..ec50f765ab 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 { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; import { assertDesktop3pModelsValid } from "./desktop-3p-guard"; export interface Desktop3pModelEntry { @@ -322,9 +323,14 @@ 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`. + * + * `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", @@ -335,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") { @@ -620,8 +626,30 @@ 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 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(port, 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 33df1e8457..366017a04b 100644 --- a/src/claude/gateway-cache.ts +++ b/src/claude/gateway-cache.ts @@ -13,7 +13,7 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; -import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "../lib/service-secrets"; +import { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; import type { OcxConfig } from "../types"; export interface GatewayModelRow { @@ -24,7 +24,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; } @@ -60,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, @@ -92,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" - ? `http://127.0.0.1:${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 446485e2b7..03261c016a 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 { 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"; @@ -102,16 +103,34 @@ function isClaudeLoopbackHostname(hostname: string): boolean { || normalized === "[::1]"; } -function targetsLocalClaudeProxy(value: string | undefined, port: number): boolean { +/** + * Is this base URL one of OURS? + * + * 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[], + 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) - && effectivePort === port - && parsed.username === "" - && parsed.password === ""; + && ports.includes(effectivePort); } catch { return false; } @@ -147,7 +166,16 @@ 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 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 + : 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 @@ -188,10 +216,14 @@ 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) + && parsed.origin !== managedBaseUrl) { + 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 @@ -216,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, port!); + : targetsLocalClaudeProxy(env.ANTHROPIC_BASE_URL, ownLocalPorts, managedBaseUrl); const isOwnAdmissionToken = (value: string): boolean => ownTokens.includes(value) || isProxyAdmissionSecret(value, config); const inheritedApiKey = env.ANTHROPIC_API_KEY; @@ -265,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 @@ -335,6 +389,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 +406,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 +585,16 @@ export function buildNativeClaudeEnv( return Boolean(value && (value === PROXY_MARKER || isProxyAdmissionSecret(value, config))); }); const baseUrl = env.ANTHROPIC_BASE_URL; - if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, config.port)) { + // 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 new file mode 100644 index 0000000000..56e9b3c056 --- /dev/null +++ b/src/lib/local-destinations.ts @@ -0,0 +1,162 @@ +/** + * 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. `localInferenceDestination` — the data plane a client wire actually speaks. + * + * 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, 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 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 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 localInferenceDestination( + config: LocalInferenceConfig | undefined, + publicPort: number, +): 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), + }; +} + +/** + * 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, +): 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/*`. + * + * 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/index.ts b/src/server/index.ts index 106f34069a..e5d6b182f6 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -822,6 +822,25 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors( await handleChatCompletions(req, config, logCtx, { requestId, start, turnAdmissionLease, admission }), req, - config, + policy, )); } diff --git a/src/server/management/api-access.ts b/src/server/management/api-access.ts index 65c9502bb6..44336919ca 100644 --- a/src/server/management/api-access.ts +++ b/src/server/management/api-access.ts @@ -1,4 +1,6 @@ import type { OcxConfig } from "../../types"; +import { isWildcardHostname } from "../../codex/loopback-target"; +import { localInferenceDestination } from "../../lib/local-destinations"; import { probeHostname } from "../proxy-liveness"; export interface ApiAccessEndpoints { @@ -21,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. */ @@ -66,7 +73,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 +111,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 `${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 46f39209f9..ed428f86d7 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 { localInferenceDestination } from "../../lib/local-destinations"; import { fetchAllModels } from "../management-api"; import { predictCursorEffort } from "../models-capabilities"; import { expandCursorEffortRow, knownEffortRowIds } from "../effort-row"; @@ -54,11 +55,19 @@ 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, 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 @@ -107,7 +116,7 @@ export async function buildCursorIntegrationStatus( }, regularCursor: { installed: regular !== undefined, path: regular?.path ?? null }, gateway: { - baseUrl: `http://127.0.0.1:${port}/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 35954f035e..3461428fab 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 { localAdmissionToken, localInferenceDestination } from "../lib/local-destinations"; /** * Does the opencodex dummy marker belong in the system environment? @@ -79,9 +80,14 @@ export function writeShellEnvFile( auto?: AutoContextMode, deps: SystemEnvDeps = {}, ): void { + // 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(`http://127.0.0.1:${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 @@ -89,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 5825b2a2a1..712addbeb7 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -7,6 +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 { 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"; @@ -35,12 +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 port. */ port: number; + /** + * The host `/healthz` answers on — `probeHostname(config.hostname)` at injection time (#4236). + * + * 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. + */ + 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 }; @@ -70,7 +95,13 @@ function readTracking(): SystemEnvTracking | undefined { (name): name is string => typeof name === "string" && MANAGED_SYSTEM_ENV_NAMES.has(name), ))] : undefined; - return { ...tracking, 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; } @@ -88,18 +119,40 @@ function ownedBaseUrl(port: number): string { return `http://127.0.0.1:${port}`; } -function writeTracking(port: number, injectedKeys: string[]): 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, + // 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 }); } -function rollbackInjectedKeys(port: number, injectedKeys: string[]): void { +/** 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); +} + +/** 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 { @@ -110,7 +163,7 @@ function rollbackInjectedKeys(port: number, injectedKeys: string[]): void { } if (rollbackFailed.length > 0) { - writeTracking(port, rollbackFailed); + writeTracking(port, rollbackFailed, tracked); return; } @@ -150,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, @@ -167,15 +251,17 @@ export async function injectSystemEnv( const inject = (name: string, value: string) => { setLaunchctlEnv(name, value); if (!injectedKeys.includes(name)) injectedKeys.push(name); - writeTracking(port, injectedKeys); + writeTracking(port, injectedKeys, tracked); }; try { - inject("ANTHROPIC_BASE_URL", ownedBaseUrl(port)); + 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); @@ -190,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); + writeTracking(port, injectedKeys, tracked); } } // Lever keys (devlog 136 B6): user-wins — skip any key the user already set in the @@ -242,9 +328,9 @@ export async function injectSystemEnv( injectClaudeAgentDefs(config, windows); } catch { /* best-effort */ } - writeTracking(port, injectedKeys); + writeTracking(port, injectedKeys, tracked); } catch (error) { - rollbackInjectedKeys(port, injectedKeys); + rollbackInjectedKeys(port, injectedKeys, tracked); removeShellEnvFile(); console.error("Failed to inject system environment; rolled back launchctl changes:", error); throw error; @@ -266,7 +352,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,7 +382,13 @@ export async function cleanStaleSystemEnv(): Promise { if (!tracking) return { cleaned: false, reason: "no tracking file" }; try { - 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 cbcccf3d49..3cb0b2f3e2 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,17 @@ 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 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 } + : {}), + }, settings: { model: routedModel, reasoning: DEFAULT_REASONING, diff --git a/src/vision/routed-describe.ts b/src/vision/routed-describe.ts index fdd1575606..dc0af78218 100644 --- a/src/vision/routed-describe.ts +++ b/src/vision/routed-describe.ts @@ -19,16 +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. + * 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 { 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"; @@ -60,23 +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. */ -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}`; +/** Base URL seam for tests; production always self-fetches the resolved local destination. */ +export function routedDescribeBaseUrl( + config: Pick, +): string { + 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( @@ -84,7 +106,7 @@ export async function describeImageRouted( _detail: string | undefined, contextText: string, routedModel: string, - config: Pick, + config: Pick, settings: VisionSettings, abortSignal?: AbortSignal, baseUrlOverride?: string, @@ -98,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/structure/01_runtime.md b/structure/01_runtime.md index 7e529ea503..32cf8036c8 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -58,12 +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 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 70ad5f1ea9..8e8c458580 100644 --- a/structure/09_client-integrations.md +++ b/structure/09_client-integrations.md @@ -149,6 +149,31 @@ 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 (`localInferenceDestination`) is +`127.0.0.1` on the unauthenticated loopback listener's effective port when that listener is +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. `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 Connected `ocx claude desktop apply` reads the hub's Desktop snapshot and writes the hub origin diff --git a/tests/claude-integration/claude-cli.test.ts b/tests/claude-integration/claude-cli.test.ts index 86574b11b9..0ac94f078f 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,236 @@ 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 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({ + 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"); + }); + + 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", + 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://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 }), + 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", () => { + // 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", + 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..912c2c18af 100644 --- a/tests/clients/desktop-3p.test.ts +++ b/tests/clients/desktop-3p.test.ts @@ -394,6 +394,74 @@ 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 — 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" }, + // 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-")); + 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 }); + // 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; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(dir); + } + } + }); + + 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/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..52b003b052 --- /dev/null +++ b/tests/lib/local-destinations.test.ts @@ -0,0 +1,280 @@ +/** + * 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. + * + * 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 { + localAdmissionToken, + localInferenceDestination, + localLoopbackInferencePorts, + 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; +} + +/** 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: [], + }, +]; + +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("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, + {}, + { 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, + ) }).toEqual({ hostname, ports: [PUBLIC_PORT, 10_104] }); + } + }); + + 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(); + }); +}); + +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("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"], + [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 6cb18458aa..3db7fab1a4 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,56 @@ 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: — 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", 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, keyMode } 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 }); + // 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 820aa2c658..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", () => { @@ -71,6 +74,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..36336f9fea 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", () => { @@ -82,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/loopback-listener-admission.test.ts b/tests/server/loopback-listener-admission.test.ts index d6a88e7408..e87cc5104c 100644 --- a/tests/server/loopback-listener-admission.test.ts +++ b/tests/server/loopback-listener-admission.test.ts @@ -84,6 +84,47 @@ 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 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", () => { + // 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..7411e1559d 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 inference wires are admitted as POST only (see the dedicated test below). + { method: "GET", path: "/v1/messages" }, + { method: "GET", path: "/v1/chat/completions" }, + { 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" }, @@ -464,6 +466,55 @@ describe("unauthenticated loopback listener", () => { } }); + 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/messages/count_tokens", "/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 +980,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); 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. diff --git a/tests/server/system-env.test.ts b/tests/server/system-env.test.ts index a8c4175198..6a3d61cb7a 100644 --- a/tests/server/system-env.test.ts +++ b/tests/server/system-env.test.ts @@ -310,6 +310,160 @@ describe("system environment injection", () => { }); }); +/** + * A launchd-started `claude` is a local client (#4236): on a hub bound to a tailnet address 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 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); + + 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; 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"); + // 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"); + // 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 base URL, not the tracked port", () => { + 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", + }); + // 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("revert recognizes a bind-address injection as ours too", () => { + trackingFile = JSON.stringify({ + 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[] = []; + 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://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", () => { 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..d742345d8f 100644 --- a/tests/vision/vision-routed.test.ts +++ b/tests/vision/vision-routed.test.ts @@ -3,12 +3,15 @@ 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 } 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 +91,93 @@ describe("describeImageRouted unit", () => { } }); + /** + * The self-fetch is a local client too (#4236): on a hub bound to a tailnet address the only + * 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://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. + 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 }); + // `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", () => { expect(routedDescribeAdmissionToken({})).toBeUndefined(); expect(routedDescribeAdmissionToken({