-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(opencode): read /api/models with the admin token, not the admission key #4317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ import { spawn } from "node:child_process"; | |
| import { existsSync, readFileSync } from "node:fs"; | ||
| import { homedir } from "node:os"; | ||
| import { dirname, join } from "node:path"; | ||
| import { loadConfig } from "../config"; | ||
| import { getConfigDir, loadConfig } from "../config"; | ||
| import { | ||
| OPENCODE_API_KEY_ENV, | ||
| OPENCODE_CONFIG_SCHEMA, | ||
|
|
@@ -40,9 +40,15 @@ import type { | |
| OpencodeV2ProviderBlock, | ||
| } from "../clients/config-export"; | ||
| import { filterCatalogVisibleModels, visibleNativeSlugs } from "../codex/catalog"; | ||
| import { isLoopbackHostname } from "../codex/loopback-target"; | ||
| import { configuredAdminToken } from "../lib/admin-secrets"; | ||
| import { localManagementOrigin } from "../lib/local-destinations"; | ||
| import { LOCAL_MANAGEMENT_READ_PATHS } from "../lib/local-management-capability"; | ||
| import { commandInvocation } from "../lib/win-exec"; | ||
| import { loadServiceTokenFromFile, serviceApiTokenFilePath } from "../lib/service-secrets"; | ||
| import { providerCodexAccountMode } from "../providers/registry"; | ||
| import { directLocalHttpFetch } from "../server/direct-local-http"; | ||
| import { fetchBoundLocalManagementRead } from "../server/local-management-read-client"; | ||
| import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; | ||
| import type { OcxConfig } from "../types"; | ||
| import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; | ||
|
|
@@ -306,16 +312,95 @@ function opencodeBlocks( | |
| /** Default deadline for authenticated GET /api/models during `ocx opencode` launch. */ | ||
| export const OPENCODE_PROXY_MODELS_TIMEOUT_MS = 8_000; | ||
|
|
||
| /** Fetch the live model catalog from a running proxy's management API. */ | ||
| /** | ||
| * Fetch the live model catalog from a running proxy management API. | ||
| * | ||
| * `GET /api/models` sits behind `requireManagementAuth`, so `managementToken` has to be a | ||
| * management credential. It is deliberately NOT the data-plane admission key `buildOpencodeEnv` | ||
| * hands the child process, and two boundaries keep it local: | ||
| * | ||
| * 1. Destination — the resolved origin must be loopback. `probeHostname` already normalises every | ||
| * wildcard spelling to 127.0.0.1 and brackets bare IPv6 literals, so the supported | ||
| * wildcard/IPv4/IPv6 listener cases keep working, while a non-loopback bind is refused before | ||
| * any token-bearing request exists. | ||
| * 2. Transport — `directLocalHttpFetch` never consults proxy environment variables, never follows | ||
| * a redirect, and drops proxy headers. A global `fetch` can do all three, so the management | ||
| * credential does not travel through one. | ||
| * | ||
| * When the live proxy is process-attested (`live.source === "runtime"`) the read goes through the | ||
| * single-use local management capability instead, so no reusable credential leaves this process at | ||
| * all. A proxy that does not recognise that capability yet (an older build) falls back to the | ||
| * loopback token read below. | ||
| */ | ||
| export interface OpencodeProxyModelsDeps { | ||
| fetchImpl?: typeof fetch; | ||
| timeoutMs?: number; | ||
| /** | ||
| * `/api/*` origin for this read, normally `localManagementOrigin(config, live.port)`, which | ||
| * prefers a hub loopback management ingress. Defaults to the identity-probed proxy record. | ||
| */ | ||
| origin?: string; | ||
| /** Capability-read seam; defaults to the real single-use capability client. */ | ||
| boundRead?: typeof fetchBoundLocalManagementRead; | ||
| } | ||
|
|
||
| /** True when `origin` is a plain-HTTP loopback destination this process may carry a token to. */ | ||
| export function isLocalManagementOrigin(origin: string): boolean { | ||
| try { | ||
| const url = new URL(origin); | ||
| return url.protocol === "http:" && !url.username && !url.password && isLoopbackHostname(url.hostname); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function opencodeProxyModelRows(response: Response, text: string): OpencodeProxyModelRow[] { | ||
| let body: unknown = null; | ||
| if (text) { | ||
| try { body = JSON.parse(text); } | ||
| catch { body = text; } | ||
| } | ||
| if (!response.ok) { | ||
| const message = body && typeof body === "object" && typeof (body as Record<string, unknown>).error === "string" | ||
| ? (body as Record<string, string>).error | ||
| : `Management request failed (${response.status})`; | ||
| throw new Error(message); | ||
| } | ||
| if (!Array.isArray(body)) { | ||
| throw new Error("Management API returned an unexpected /api/models payload."); | ||
| } | ||
| return body as OpencodeProxyModelRow[]; | ||
| } | ||
|
|
||
| export async function fetchOpencodeProxyModels( | ||
| live: LiveProxy, | ||
| apiKey: string, | ||
| deps: { fetchImpl?: typeof fetch; timeoutMs?: number } = {}, | ||
| managementToken: string, | ||
| deps: OpencodeProxyModelsDeps = {}, | ||
| ): Promise<OpencodeProxyModelRow[]> { | ||
| const baseUrl = `http://${probeHostname(live.hostname)}:${live.port}`; | ||
| const fetchImpl = deps.fetchImpl ?? fetch; | ||
| const attestedOrigin = `http://${probeHostname(live.hostname)}:${live.port}`; | ||
| const origin = deps.origin ?? attestedOrigin; | ||
| if (!isLocalManagementOrigin(origin)) { | ||
| throw new Error( | ||
| `Refusing to send the opencodex management credential to ${origin}: it is not a loopback address. ` | ||
| + "Bind the proxy to loopback, or enable the hub management ingress, so this host has a local /api/* address.", | ||
| ); | ||
| } | ||
| const target = new URL(origin); | ||
| // The capability is bound to the attested pid AND to the port the request arrives on, so it can | ||
| // only be presented to the proxy listener that minted it, never to a separate management ingress. | ||
| if (live.source === "runtime" | ||
| && target.port === String(live.port) | ||
| && target.hostname === new URL(attestedOrigin).hostname) { | ||
| const read = await (deps.boundRead ?? fetchBoundLocalManagementRead)( | ||
| live, | ||
| LOCAL_MANAGEMENT_READ_PATHS.models, | ||
| { timeoutMs: deps.timeoutMs ?? OPENCODE_PROXY_MODELS_TIMEOUT_MS }, | ||
| ); | ||
| if (read.kind === "response") return opencodeProxyModelRows(read.response, await read.response.text()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a new AGENTS.md reference: src/AGENTS.md:L17-L17 Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Fall back after a capability authentication rejection. At Handle the expected 🤖 Prompt for AI Agents |
||
| } | ||
| const fetchImpl = deps.fetchImpl ?? directLocalHttpFetch; | ||
| const headers = new Headers({ Accept: "application/json" }); | ||
| const token = apiKey.trim(); | ||
| const token = managementToken.trim(); | ||
| if (token) headers.set("X-OpenCodex-API-Key", token); | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), deps.timeoutMs ?? OPENCODE_PROXY_MODELS_TIMEOUT_MS); | ||
|
|
@@ -335,7 +420,7 @@ export async function fetchOpencodeProxyModels( | |
| let text: string; | ||
| try { | ||
| response = await Promise.race([ | ||
| fetchImpl(`${baseUrl}/api/models`, { | ||
| fetchImpl(`${target.origin}/api/models`, { | ||
| headers, | ||
| signal: controller.signal, | ||
| }), | ||
|
|
@@ -352,21 +437,7 @@ export async function fetchOpencodeProxyModels( | |
| } finally { | ||
| clearTimeout(timeout); | ||
| } | ||
| let body: unknown = null; | ||
| if (text) { | ||
| try { body = JSON.parse(text); } | ||
| catch { body = text; } | ||
| } | ||
| if (!response.ok) { | ||
| const message = body && typeof body === "object" && typeof (body as Record<string, unknown>).error === "string" | ||
| ? (body as Record<string, string>).error | ||
| : `Management request failed (${response.status})`; | ||
| throw new Error(message); | ||
| } | ||
| if (!Array.isArray(body)) { | ||
| throw new Error("Management API returned an unexpected /api/models payload."); | ||
| } | ||
| return body as OpencodeProxyModelRow[]; | ||
| return opencodeProxyModelRows(response, text); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -602,6 +673,26 @@ export function opencodeApiKey(config: OcxConfig, env: OpencodeLaunchEnv = proce | |
| return config.apiKeys?.[0]?.key || "ocx"; | ||
| } | ||
|
|
||
| /** | ||
| * Credential for the launcher's `GET /api/models` read. | ||
| * | ||
| * That route is a management route, so `requireManagementAuth` only admits the admin credential — | ||
| * the data-plane admission key {@link opencodeApiKey} returns for the child process is refused there | ||
| * with `opencodex admin token required`. Prefer the configured admin token, the same credential every | ||
| * other headless management caller sends (`runningProxyUpdateHeaders`), and keep the admission key as | ||
| * the fallback for a host that has no admin token configured. | ||
| * | ||
| * The destination and transport are constrained by {@link fetchOpencodeProxyModels}: the credential | ||
| * only ever reaches a loopback `/api/*` origin, and an attested proxy answers the same read over a | ||
| * single-use capability that needs no reusable credential at all. | ||
| */ | ||
| export function opencodeManagementToken(config: OcxConfig, env: OpencodeLaunchEnv = process.env): string { | ||
| // Name the directory instead of passing `undefined`: an explicit `env` may describe a different | ||
| // OPENCODEX_HOME than this process, and the admin token file is read from that directory. | ||
| const configDir = env.OPENCODEX_HOME?.trim() || getConfigDir(); | ||
| return configuredAdminToken(configDir, env) ?? opencodeApiKey(config, env); | ||
| } | ||
|
|
||
| async function ensureProxyForOpencode(config: OcxConfig): Promise<LiveProxy | null> { | ||
| const live = await findLiveProxy(); | ||
| if (live) return live; | ||
|
|
@@ -650,9 +741,13 @@ export async function cmdOpencode(args: string[]): Promise<number> { | |
| } | ||
|
|
||
| const apiKey = opencodeApiKey(startupConfig); | ||
| const managementToken = opencodeManagementToken(startupConfig); | ||
| let proxyModels: OpencodeProxyModelRow[]; | ||
| try { | ||
| proxyModels = await fetchOpencodeProxyModels(live, apiKey); | ||
| proxyModels = await fetchOpencodeProxyModels(live, managementToken, { | ||
| // A hub reaches its own management API through the loopback ingress, not the public bind. | ||
| origin: localManagementOrigin(startupConfig, live.port), | ||
| }); | ||
|
Comment on lines
+747
to
+750
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛡️ Codex Security Review · Automatically triggered
On a shared host, when the proxy is stopped and runtime state is absent, another local user can bind its loopback port and answer Useful? React with 👍 / 👎. |
||
| } catch (error) { | ||
| const reason = error instanceof Error ? error.message : String(error); | ||
| console.error(`❌ Could not fetch the model catalog from the proxy: ${reason}`); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,13 @@ writes the resulting local Desktop configuration. No admin token, hub-profile up | |||||||||
| alias regeneration is part of this flow. Unsupported old hubs, invalid snapshots and unavailable | ||||||||||
| Desktop models fail apply without a local-catalog or loopback fallback. | ||||||||||
|
|
||||||||||
| Local launchers read hub management state only through an authenticated, loopback-only origin. | ||||||||||
| `src/cli/opencode.ts` applies that rule to its `GET /api/models` catalogue read: the management | ||||||||||
|
Comment on lines
+21
to
+22
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Scope the loopback claim to OpenCode.
-Local launchers read hub management state only through an authenticated, loopback-only origin.
+The OpenCode launcher reads its model catalogue only through an authenticated, loopback-only origin.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| credential is refused a non-loopback destination before any request is built, the transport is the | ||||||||||
| direct local one (no proxy environment variables and no redirects), and the admission key handed to | ||||||||||
| the child process carries no management authority. An attested proxy answers that read over a | ||||||||||
| single-use local read capability instead of the credential itself. | ||||||||||
|
|
||||||||||
| Date-shaped Desktop IDs can overlap genuine native model IDs. When available discovery and | ||||||||||
| mapping evidence cannot resolve one, Messages and count-tokens return HTTP 503 with the fixed | ||||||||||
| `desktop_model_mapping_unavailable` error rather than classifying it as invalid. Unknown legacy hash aliases | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -191,6 +191,17 @@ not an authentication or entitlement decision. | |||||||||||||||
|
|
||||||||||||||||
| Routed Responses continuations whose local replay state is missing resolve their recovery decision from the selected wire protocol, not the model name; the contract lives in [Responses transport](transports/responses.md). | ||||||||||||||||
|
|
||||||||||||||||
| `src/cli/opencode.ts` reads its model catalogue from the authenticated management route | ||||||||||||||||
| `GET /api/models`, so it presents the management credential (`configuredAdminToken`: | ||||||||||||||||
| `OPENCODEX_ADMIN_AUTH_TOKEN`, then the hardened `admin-api-token` file) and keeps the data-plane | ||||||||||||||||
| admission key for the child process environment alone. The read is loopback-only — the resolved | ||||||||||||||||
|
Comment on lines
+195
to
+197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Document conditional credential use.
Rewrite this sentence so it does not state that the management credential is always presented or that the admission key is used only for the child process. Proposed wording- so it presents the management credential (`configuredAdminToken`: `OPENCODEX_ADMIN_AUTH_TOKEN`, then the hardened `admin-api-token` file) and keeps the data-plane
- admission key for the child process environment alone.
+ so it prefers the management credential (`configuredAdminToken`: `OPENCODEX_ADMIN_AUTH_TOKEN`, then the hardened `admin-api-token` file).
+ If no management credential exists, it falls back to the admission key for this read. An attested proxy can
+ use the single-use capability instead, while the child process continues to receive the admission key.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| `/api/*` origin must be a loopback address, with `probeHostname` normalizing the wildcard and IPv6 | ||||||||||||||||
| spellings — and it travels over `directLocalHttpFetch`, which ignores proxy environment variables | ||||||||||||||||
| and never follows a redirect. A process-attested proxy answers the same read over the single-use | ||||||||||||||||
| local management capability for `/api/models` (`src/lib/local-management-capability.ts`) rather than | ||||||||||||||||
| a reusable credential; a proxy without that capability falls back to the loopback token read. See | ||||||||||||||||
| [Config surface](config.md). | ||||||||||||||||
|
|
||||||||||||||||
| ## Remote Hub hardening ownership | ||||||||||||||||
|
|
||||||||||||||||
| `src/remote/protocol.ts` owns pure interval/feature negotiation. `src/remote/hub-state.ts` owns the `GET|HEAD /v1/hub-state` contract, its caps, and the parser both sides share. `src/client/hub-client.ts` owns bounded, schema-validated remote catalog consumption, hub-state reads, and key-id probes; `src/client/hub-state.ts` owns the resolution and the owner-stamped 0600 cache, and a failed read reports "unavailable" rather than degrading to the client's own local provider and login state. `src/client/hub-relay.ts` is a fixed-authority management relay with URL, header, body, redirect, and stream bounds. The public data listener remains the direct client→hub path; the loopback management ingress never serves data-plane routes. | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the effective token-file directory.
When
OPENCODEX_HOMEis set,src/cli/opencode.tspasses that directory toconfiguredAdminToken, soadmin-api-tokenis not necessarily read from~/.opencodex. State that~/.opencodexis the default and thatOPENCODEX_HOMEoverrides it.Otherwise, users with a custom OpenCodex home can place the management token in a path that the launcher does not read.
Proposed wording
As per path instructions: keep paths and configuration keys synchronized with the repository.
🤖 Prompt for AI Agents
Source: Path instructions