diff --git a/src/config.ts b/src/config.ts index 11d88af91d..c77e1507ff 100644 --- a/src/config.ts +++ b/src/config.ts @@ -74,6 +74,7 @@ import { type FastWire, type ProviderCostOverlay, } from "./types"; +import type { OcxRuntimeRole } from "./types/config"; import { OPENAI_CODEX_PROVIDER_ID } from "./providers/openai-tiers"; import { modelAutoCompactTokenLimitsConfigError } from "./providers/auto-compact-budget"; import { fastWireDeclarationError, hasFastWireCapabilityConflict } from "./providers/fastwire"; @@ -861,8 +862,13 @@ const agentTaskRecoverySchema = z.object({ cacheEntries: z.number().int().min(1).max(512).optional(), }).strict(); +const runtimeRoleSchema = z.enum(["standalone", "hub", "client"]); + const configSchema = z.object({ port: z.number().int().min(0).max(65535).default(10100), + // A malformed hand edit must disable only remote-role behavior, not discard + // providers or data-plane keys. Live writes are rejected explicitly below. + runtimeRole: runtimeRoleSchema.optional().catch(undefined), managementUsageMaxReadBytes: z.number().int().positive().default(64 * 1024 * 1024), // Invalid hand edits disable only this opt-in circuit. Live writes remain strict. upstreamHostCircuitThreshold: z.number().int() @@ -1706,6 +1712,18 @@ function warnDegradedAgentTaskRecovery(rawParsed: unknown): void { if (warning) console.warn(`⚠️ config.json ${warning}. Other settings were preserved.`); } +function malformedRuntimeRoleWarning(rawParsed: unknown): string | null { + const raw = rawConfigRecord(rawParsed); + if (!raw || !Object.hasOwn(raw, "runtimeRole") || raw.runtimeRole === undefined) return null; + if (runtimeRoleSchema.safeParse(raw.runtimeRole).success) return null; + return 'runtimeRole ignored: expected "standalone", "hub", or "client"; falling back to "standalone"'; +} + +function warnDegradedRuntimeRole(rawParsed: unknown): void { + const warning = malformedRuntimeRoleWarning(rawParsed); + if (warning) console.warn(`⚠️ config.json ${warning}. Other settings were preserved.`); +} + type NativeSubagentPersistedField = "injectionModel" | "injectionEffort" | "syncCodexSubagentDefaults"; function rawConfigRecord(rawParsed: unknown): Record | null { @@ -1859,6 +1877,7 @@ export function loadConfig(): OcxConfig { warnDegradedCodexAccountPicker(parsed); warnDegradedUpstreamHostCircuitThreshold(parsed); warnDegradedAgentTaskRecovery(parsed); + warnDegradedRuntimeRole(parsed); return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); } // Schema validation failed — merge defaults into the raw object instead of @@ -1883,6 +1902,7 @@ export function loadConfig(): OcxConfig { warnDegradedCodexAccountPicker(parsed); warnDegradedUpstreamHostCircuitThreshold(parsed); warnDegradedAgentTaskRecovery(parsed); + warnDegradedRuntimeRole(parsed); return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); } // Still failing, but if every complaint is about one or more named entries @@ -1903,6 +1923,7 @@ export function loadConfig(): OcxConfig { warnDegradedCodexAccountPicker(parsed); warnDegradedUpstreamHostCircuitThreshold(parsed); warnDegradedAgentTaskRecovery(parsed); + warnDegradedRuntimeRole(parsed); return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); } } @@ -2003,6 +2024,8 @@ function validFileConfigDiagnostics(config: OcxConfig, rawParsed: unknown): Conf if (hostCircuitWarning) warnings.push(hostCircuitWarning); const recoveryWarning = malformedAgentTaskRecoveryWarning(rawParsed); if (recoveryWarning) warnings.push(recoveryWarning); + const runtimeRoleWarning = malformedRuntimeRoleWarning(rawParsed); + if (runtimeRoleWarning) warnings.push(runtimeRoleWarning); if (syncDisabledReason) { warnings.push(`syncCodexSubagentDefaults ignored: ${syncDisabledReason}`); } @@ -2094,6 +2117,13 @@ function agentTaskRecoveryError(value: unknown): string | null { return `schema_invalid: agentTaskRecovery${field ? `.${field}` : ""}: ${issue?.message ?? "invalid configuration"}`; } +function runtimeRoleError(value: unknown): string | null { + const raw = rawConfigRecord(value); + if (!raw || !Object.hasOwn(raw, "runtimeRole") || raw.runtimeRole === undefined) return null; + if (runtimeRoleSchema.safeParse(raw.runtimeRole).success) return null; + return 'schema_invalid: runtimeRole: must be one of "standalone", "hub", or "client"'; +} + /** * Same reasoning as {@link blankHostnameError}, and more urgent: the read path degrades a * malformed selection-order map to undefined, which on a write would drop every entry the @@ -2211,6 +2241,7 @@ export function validateConfigCandidate(value: unknown): { ok: true; config: Ocx ?? codexAccountPickerEnabledError(value) ?? emptyCompletionRetryError(value) ?? oauthOpenBrowserError(value) + ?? runtimeRoleError(value) ?? loopbackListenerPortError(value); if (boundaryError) return { ok: false, error: boundaryError }; const result = configSchema.safeParse(value); @@ -3123,6 +3154,10 @@ export function multiAgentGuidanceEnabled( return config.multiAgentGuidanceEnabled !== false; } +export function runtimeRole(config: Pick): OcxRuntimeRole { + return config.runtimeRole ?? "standalone"; +} + export function getDefaultConfig(): OcxConfig { // Fresh-install default: works out of the box with Codex's ChatGPT OAuth (no API key). // gpt-* requests forward the caller's incoming OAuth headers to the ChatGPT backend. diff --git a/src/remote/protocol.ts b/src/remote/protocol.ts new file mode 100644 index 0000000000..71ee0c10b5 --- /dev/null +++ b/src/remote/protocol.ts @@ -0,0 +1,98 @@ +import type { OcxConfig } from "../types/config"; + +export const REMOTE_HUB_PROTOCOL = 1; +export const MINIMUM_REMOTE_CLIENT_PROTOCOL = 1; + +export interface RemoteReadyMetadata { + protocol: number; + minimumClientProtocol: number; + managementUrl: string; +} + +export type RemoteProtocolCompatibility = + | { ok: true; metadata: RemoteReadyMetadata } + | { ok: false; reason: "invalid" | "hub-too-new" | "hub-too-old"; message: string }; + +const INVALID_REMOTE_PROTOCOL_MESSAGE = + "OpenCodex hub returned invalid remote protocol metadata; upgrade or repair ocx on the hub."; + +function positiveSafeInteger(value: unknown): value is number { + return typeof value === "number" && Number.isSafeInteger(value) && value > 0; +} + +function managementOrigin(value: unknown): string | null { + if (typeof value !== "string") return null; + try { + const parsed = new URL(value); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return null; + if (parsed.username || parsed.password || parsed.pathname !== "/" || parsed.search || parsed.hash) return null; + return parsed.origin; + } catch { + return null; + } +} + +function observedManagementOrigin(req: Request): string | null { + try { + const requestUrl = new URL(req.url); + const host = req.headers.get("Host") ?? requestUrl.host; + return managementOrigin(`${requestUrl.protocol}//${host}`); + } catch { + return null; + } +} + +export function readyProtocolMetadata(config: OcxConfig, req: Request): RemoteReadyMetadata { + // Phase 2 will consult config.hub.managementPublicOrigin here. Keeping the + // parameter now fixes the consumer signature without changing Phase 1 behavior. + void config; + const managementUrl = observedManagementOrigin(req); + if (!managementUrl) throw new Error("Readiness request does not have an HTTP(S) management origin"); + return { + protocol: REMOTE_HUB_PROTOCOL, + minimumClientProtocol: MINIMUM_REMOTE_CLIENT_PROTOCOL, + managementUrl, + }; +} + +export function parseRemoteReadyMetadata(value: unknown): RemoteReadyMetadata | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const raw = value as Record; + if (!positiveSafeInteger(raw.protocol) || !positiveSafeInteger(raw.minimumClientProtocol)) return null; + if (raw.minimumClientProtocol > raw.protocol) return null; + const parsedManagementOrigin = managementOrigin(raw.managementUrl); + if (!parsedManagementOrigin) return null; + return { + protocol: raw.protocol, + minimumClientProtocol: raw.minimumClientProtocol, + managementUrl: parsedManagementOrigin, + }; +} + +export function checkRemoteProtocolCompatibility( + value: unknown, + client: { protocol: number; minimumHubProtocol: number } = { + protocol: REMOTE_HUB_PROTOCOL, + minimumHubProtocol: MINIMUM_REMOTE_CLIENT_PROTOCOL, + }, +): RemoteProtocolCompatibility { + const metadata = parseRemoteReadyMetadata(value); + if (!metadata || !positiveSafeInteger(client.protocol) || !positiveSafeInteger(client.minimumHubProtocol)) { + return { ok: false, reason: "invalid", message: INVALID_REMOTE_PROTOCOL_MESSAGE }; + } + if (client.protocol < metadata.minimumClientProtocol) { + return { + ok: false, + reason: "hub-too-new", + message: `OpenCodex hub requires remote protocol ${metadata.minimumClientProtocol}; this client supports protocol ${client.protocol}. Upgrade ocx on this client.`, + }; + } + if (metadata.protocol < client.minimumHubProtocol) { + return { + ok: false, + reason: "hub-too-old", + message: `OpenCodex hub provides remote protocol ${metadata.protocol}; this client requires at least ${client.minimumHubProtocol}. Upgrade ocx on the hub.`, + }; + } + return { ok: true, metadata }; +} diff --git a/src/server/index.ts b/src/server/index.ts index 18e4e5254a..35c7361be8 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -155,6 +155,7 @@ import { resolveApiAuth, resolveResponsesApiAuth, requestPolicyView, + type DataPlaneAdmission, type RequestPolicyView, safeConfigDTO, setCorsOrigin, @@ -210,9 +211,37 @@ import { type PackageTreeIntegrityGuard, } from "../lib/package-tree-integrity"; import { detectInstall } from "../update/index"; +import { readyProtocolMetadata } from "../remote/protocol"; export const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024; const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0; + +// Header-safe by construction: a key id reaches a response header, so anything outside this +// class could inject a header break or a control character into a response we control. +const REMOTE_CATALOG_KEY_ID_PATTERN = /^[A-Za-z0-9._-]{1,64}$/; + +/** + * Name WHICH configured credential was admitted, so a multi-key operator can attribute a + * catalog read. + * + * Scoped to configured keys on purpose: an environment token or a loopback bind has no key + * to name, and emitting one anyway would invent an attribution that does not exist. 200 only + * — this route emits no validator and therefore never answers 304. + * + * An id that fails the header-safe pattern is omitted rather than sanitized, with one warning + * that does NOT repeat the id: logging the offending value is how a malformed id becomes a + * log-injection vector instead of a dropped header. + */ +function withRemoteCatalogKeyId(response: Response, admission: DataPlaneAdmission): Response { + if (response.status !== 200 || admission.kind !== "configured") return response; + if (!REMOTE_CATALOG_KEY_ID_PATTERN.test(admission.keyId)) { + console.warn("[remote-catalog] configured API key id is not header-safe; omitting x-opencodex-key-id"); + return response; + } + response.headers.set("x-opencodex-key-id", admission.keyId); + return response; +} + const LIVE_SIDEBAND_PENDING_MAX = 32; const LIVE_SIDEBAND_PENDING_BYTES_MAX = 1024 * 1024; const LIVE_SIDEBAND_CLOSE_FALLBACK_MS = 1_000; @@ -1041,6 +1070,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server = { "content-type": "application/json", - // Identity-varying content behind a credential: never let a shared cache keep it. - "cache-control": "private, no-cache", + // Identity-varying content behind a credential: never let a shared cache keep it, + // and never hand out a validator it could revalidate with. `no-cache` alone does + // not prevent storage — it forces revalidation, and the revalidation is exactly + // what would cross identities here, because this body varies by key type and key + // id while the ETag would be derived from bytes alone. A store keyed on URL plus + // validator could then serve one credential's representation to another. Proving + // an identity-partitioned cache key across every intermediary in the path is a + // much larger commitment than the bandwidth a 304 saves on this payload, so this + // route declines the trade: no-store, no ETag, no 304. + // + // GET /api/catalog keeps its validator. That route is management-authenticated + // and loopback-scoped, and its representation does not vary by data-key identity. + "cache-control": "no-store", }; - if (serialized.etag) headers.ETag = serialized.etag; const version = await persistedCodexVersion(); if (version) headers["x-opencodex-codex-version"] = version; - // Conditional GET: a client that already holds these bytes re-validates cheaply. - const ifNoneMatch = req.headers.get("if-none-match")?.trim(); - if (serialized.etag && ifNoneMatch && ifNoneMatch === serialized.etag) { - return withCors(new Response(null, { status: 304, headers }), req, policy); - } + // No conditional handling: with no validator emitted, an If-None-Match on this route + // can only have been guessed or copied from elsewhere, and honoring it would + // reintroduce the cross-identity path above. Every request gets the full body. if (serialized.bytes !== undefined) headers["content-length"] = String(serialized.bytes); // HEAD returns identical status and headers with no body. - return withCors( - new Response(req.method === "HEAD" ? null : serialized.body, { status: 200, headers }), - req, - policy, + return withRemoteCatalogKeyId( + withCors( + new Response(req.method === "HEAD" ? null : serialized.body, { status: 200, headers }), + req, + policy, + ), + admission, ); } diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 04dbd50930..412cf2709d 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -269,6 +269,12 @@ interface ReadyzBody { pid?: unknown; port?: unknown; status?: unknown; + // Remote protocol metadata is intentionally additive here. Ordinary + // readiness remains compatible with legacy standalone servers; `ocx connect` + // validates these fields separately in src/remote/protocol.ts. + protocol?: unknown; + minimumClientProtocol?: unknown; + managementUrl?: unknown; } export interface ReadinessProbeResult { diff --git a/src/types/config.ts b/src/types/config.ts index 53fca2809b..07ba0cc4f3 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -244,8 +244,12 @@ export interface OcxConfigRebaseProvenance { deletedTopLevelKeys: string[]; } +export type OcxRuntimeRole = "standalone" | "hub" | "client"; + export interface OcxConfig { port: number; + /** Runtime topology role. Absence preserves the historical standalone behavior. */ + runtimeRole?: OcxRuntimeRole; /** Opt in to one identical-turn retry when a Responses completion has no text or tool call. */ emptyCompletionRetry?: boolean; /** diff --git a/tests/api-catalog-route.test.ts b/tests/api-catalog-route.test.ts index 634ce3e7e1..f46632a079 100644 --- a/tests/api-catalog-route.test.ts +++ b/tests/api-catalog-route.test.ts @@ -10,6 +10,7 @@ import type { OcxConfig } from "../src/types"; const TEST_DIR = join(import.meta.dir, `.tmp-api-catalog-route-${process.pid}`); const previousOpencodexHome = process.env.OPENCODEX_HOME; let isolatedCodexHome: IsolatedCodexHome | null = null; +const CATALOG_FIXTURE_BYTES = '{"models":[{"slug":"mock/test-model","display_name":"Mock Test","description":"fixture","priority":1,"visibility":"list","base_instructions":"You are a helpful coding assistant.","input_modalities":["text"]}]}'; beforeEach(() => { if (previousOpencodexHome === undefined) mkdirSync(TEST_DIR, { recursive: true }); @@ -39,18 +40,7 @@ afterEach(() => { describe("GET /api/catalog route (#709)", () => { test("returns the on-disk catalog and omits sync runtime probes for version hint", async () => { isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-"); - const catalog = { - models: [{ - slug: "mock/test-model", - display_name: "Mock Test", - description: "fixture", - priority: 1, - visibility: "list", - base_instructions: "You are a helpful coding assistant.", - input_modalities: ["text"], - }], - }; - writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), JSON.stringify(catalog)); + writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), CATALOG_FIXTURE_BYTES); const url = new URL("http://localhost/api/catalog"); const response = await handleManagementAPI( @@ -59,10 +49,32 @@ describe("GET /api/catalog route (#709)", () => { loadConfig(), ); expect(response?.status).toBe(200); - expect(await response!.json()).toEqual(catalog); + expect(await response!.text()).toBe(CATALOG_FIXTURE_BYTES); expect(response!.headers.get("x-opencodex-codex-version")).toBeNull(); }); + test("preserves the persisted Codex version header after serializer extraction", async () => { + isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-version-"); + writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), CATALOG_FIXTURE_BYTES); + writeFileSync(join(TEST_DIR, "codex-runtime.json"), JSON.stringify({ + version: 1, + command: "/fixture/codex", + source: "configured", + selectedVersion: "0.150.0", + updatedAt: "2026-08-28T00:00:00.000Z", + })); + + const url = new URL("http://localhost/api/catalog"); + const response = await handleManagementAPI( + new ManagementRequest(url, { headers: managementHeaders() }), + url, + loadConfig(), + ); + expect(response?.status).toBe(200); + expect(response!.headers.get("x-opencodex-codex-version")).toBe("0.150.0"); + expect(await response!.text()).toBe(CATALOG_FIXTURE_BYTES); + }); + test("returns 404 when the catalog file is missing", async () => { isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-missing-"); const url = new URL("http://localhost/api/catalog"); @@ -74,6 +86,25 @@ describe("GET /api/catalog route (#709)", () => { expect(response?.status).toBe(404); expect(await response!.json()).toEqual({ error: "catalog not found" }); }); + + test("renders a malformed persisted catalog as absent rather than leaking the parse failure", async () => { + // The management route deliberately collapses unreadable, absent, and malformed + // into one 404. An earlier revision of this phase threw on malformed JSON and + // asserted 500 here, which distinguishes "your catalog file is corrupt" from + // "you have no catalog" to any caller that can reach the route. The shared + // serializer returns `{ body: null }` for all three so no route can accidentally + // reintroduce that distinction. + isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-malformed-"); + writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), '{"models":'); + const url = new URL("http://localhost/api/catalog"); + const response = await handleManagementAPI( + new ManagementRequest(url, { headers: managementHeaders() }), + url, + loadConfig(), + ); + expect(response?.status).toBe(404); + expect(await response!.json()).toEqual({ error: "catalog not found" }); + }); }); describe("GET|HEAD /v1/catalog least-privilege data-plane route (#809)", () => { @@ -122,9 +153,12 @@ describe("GET|HEAD /v1/catalog least-privilege data-plane route (#809)", () => { expect(res.status).toBe(200); const body = await res.text(); expect(JSON.parse(body)).toEqual(catalogFixture); - expect(res.headers.get("cache-control")).toBe("private, no-cache"); - const etag = res.headers.get("etag"); - expect(etag).toBeTruthy(); + // No validator on this plane: the body varies by key identity, so a shared strong + // ETag would let a store revalidate one credential's representation for another. + // `no-cache` did not prevent that — it permits storage and forces revalidation, and + // the revalidation is the crossing. See the note in src/server/index.ts. + expect(res.headers.get("cache-control")).toBe("no-store"); + expect(res.headers.get("etag")).toBeNull(); // The whole point of the shared serializer: the two planes must not drift. const mgmtUrl = new URL("http://localhost/api/catalog"); @@ -136,12 +170,16 @@ describe("GET|HEAD /v1/catalog least-privilege data-plane route (#809)", () => { expect(mgmt?.status).toBe(200); expect(await mgmt!.text()).toBe(body); - // Conditional GET re-validates without resending the payload. + // A conditional request cannot succeed here, because no validator was ever handed + // out to build one from. Even a client that guesses the management route's ETag gets + // the full body rather than a 304. + const mgmtEtag = mgmt!.headers.get("etag"); + expect(mgmtEtag).toBeTruthy(); const revalidated = await fetch(new URL("/v1/catalog", server.url), { - headers: { "x-opencodex-api-key": DATA_KEY, "if-none-match": etag! }, + headers: { "x-opencodex-api-key": DATA_KEY, "if-none-match": mgmtEtag! }, }); - expect(revalidated.status).toBe(304); - expect(await revalidated.text()).toBe(""); + expect(revalidated.status).toBe(200); + expect(await revalidated.text()).toBe(body); // HEAD is the same status and headers with no body. const head = await fetch(new URL("/v1/catalog", server.url), { @@ -149,7 +187,8 @@ describe("GET|HEAD /v1/catalog least-privilege data-plane route (#809)", () => { headers: { "x-opencodex-api-key": DATA_KEY }, }); expect(head.status).toBe(200); - expect(head.headers.get("etag")).toBe(etag); + expect(head.headers.get("etag")).toBeNull(); + expect(head.headers.get("cache-control")).toBe("no-store"); expect(await head.text()).toBe(""); } finally { await server.stop(true); diff --git a/tests/config.test.ts b/tests/config.test.ts index 62d3f6d782..de026c8dee 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -22,6 +22,7 @@ import { readRuntimePort, removePid, removeRuntimePort, + runtimeRole, ocxStartProcessCacheSizeForTests, setOcxStartProcessCacheForTests, setProcessCommandLineExecForTests, @@ -115,6 +116,68 @@ function writeAccountNamespaceConfig( } describe("opencodex config defaults", () => { + test("runtime role is absent-by-default and resolves to standalone", () => { + const defaults = getDefaultConfig(); + expect(Object.hasOwn(defaults, "runtimeRole")).toBe(false); + expect(runtimeRole(defaults)).toBe("standalone"); + writeConfig(defaults); + const before = readFileSync(getConfigPath(), "utf8"); + expect(runtimeRole(loadConfig())).toBe("standalone"); + expect(readFileSync(getConfigPath(), "utf8")).toBe(before); + }); + + test("runtime role accepts the three explicit contract values", () => { + for (const role of ["standalone", "hub", "client"] as const) { + expect(validateConfigCandidate({ ...getDefaultConfig(), runtimeRole: role })).toMatchObject({ + ok: true, + config: { runtimeRole: role }, + }); + } + }); + + test("runtime role rejects malformed live candidates", () => { + for (const runtimeRole of ["server", "", 1, null]) { + expect(validateConfigCandidate({ ...getDefaultConfig(), runtimeRole })).toMatchObject({ + ok: false, + error: expect.stringContaining("runtimeRole"), + }); + } + }); + + test("a malformed persisted runtime role preserves providers and API keys", () => { + const invalidRole = "future-secret-shaped-role"; + writeConfig({ + port: 12345, + runtimeRole: invalidRole, + defaultProvider: "custom", + providers: { custom: { adapter: "openai-chat", baseUrl: "https://example.test/v1", apiKey: "upstream-secret" } }, + apiKeys: [{ id: "key-1", name: "default", key: "ocx_persisted", createdAt: "2026-08-28T00:00:00.000Z" }], + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + const loaded = loadConfig(); + const diagnostics = readConfigDiagnostics(); + expect(runtimeRole(loaded)).toBe("standalone"); + expect(loaded.runtimeRole).toBeUndefined(); + expect(loaded).toMatchObject({ + port: 12345, + defaultProvider: "custom", + providers: { custom: { baseUrl: "https://example.test/v1", apiKey: "upstream-secret" } }, + apiKeys: [expect.objectContaining({ id: "key-1", key: "ocx_persisted" })], + }); + expect(diagnostics).toMatchObject({ + source: "file", + error: null, + warnings: [expect.stringContaining("runtimeRole ignored")], + }); + expect(backupNames()).toEqual([]); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls.flat().join(" ")).not.toContain(invalidRole); + } finally { + warnSpy.mockRestore(); + } + }); + test("malformed classifier config is normalized at load, even with subagentEffort absent (#1697)", () => { // normalizePersistedClaudeCode used to be reached only through a subagentEffort short-circuit, // so a config whose ONLY defect was elsewhere in claudeCode was never normalized. These diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 7fd806fd62..7a00da7077 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -11,6 +11,12 @@ import { proxyIdentityAt, validateReadyzBody, } from "../src/server/proxy-liveness"; +import { + checkRemoteProtocolCompatibility, + parseRemoteReadyMetadata, + readyProtocolMetadata, +} from "../src/remote/protocol"; +import { getDefaultConfig } from "../src/config"; function healthz(body: unknown, status = 200): Response { return new Response(JSON.stringify(body), { status }); @@ -530,6 +536,18 @@ describe("validateReadyzBody strict contract", () => { expect(validateReadyzBody(VALID_BODY, 10100)).toEqual({ ready: true, status: "ready", pid: 4242, port: 10100 }); }); + test("accepts additive remote protocol and unknown future fields without weakening identity", () => { + const additive = { + ...VALID_BODY, + protocol: 1, + minimumClientProtocol: 1, + managementUrl: "https://hub.example.test", + futureCapability: { enabled: true }, + }; + expect(validateReadyzBody(additive, 10100)).toEqual({ ready: true, status: "ready", pid: 4242, port: 10100 }); + expect(validateReadyzBody({ ...additive, service: "foreign" }, 10100)).toBeNull(); + }); + test("accepts pending/failed bodies as not-ready with the same fixed status", () => { expect(validateReadyzBody({ ...VALID_BODY, status: "pending" }, 10100)).toEqual({ ready: false, status: "pending", pid: 4242, port: 10100 }); expect(validateReadyzBody({ ...VALID_BODY, status: "failed" }, 10100)).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); @@ -613,6 +631,93 @@ describe("validateReadyzBody strict contract", () => { }); }); +describe("remote readiness protocol metadata", () => { + const metadata = { + protocol: 1, + minimumClientProtocol: 1, + managementUrl: "https://hub.example.test", + }; + const invalidMessage = "OpenCodex hub returned invalid remote protocol metadata; upgrade or repair ocx on the hub."; + + test("parses required fields, canonicalizes the origin, and ignores additive fields", () => { + expect(parseRemoteReadyMetadata({ + ...metadata, + managementUrl: "https://hub.example.test:443/", + future: true, + })).toEqual(metadata); + }); + + test("builds one stable shape for standalone, hub, and client roles", () => { + for (const runtimeRole of ["standalone", "hub", "client"] as const) { + expect(readyProtocolMetadata( + { ...getDefaultConfig(), runtimeRole }, + new Request("https://hub.example.test/readyz"), + )).toEqual(metadata); + } + }); + + test("uses the observed Host and ignores forwarding headers", () => { + expect(readyProtocolMetadata(getDefaultConfig(), new Request("http://127.0.0.1/readyz", { + headers: { + Host: "hub.example.test:8443", + Forwarded: "host=attacker.test;proto=https", + "X-Forwarded-Host": "attacker.test", + "X-Forwarded-Proto": "https", + }, + }))).toEqual({ + protocol: 1, + minimumClientProtocol: 1, + managementUrl: "http://hub.example.test:8443", + }); + }); + + test("classifies a hub that requires a newer client with the exact message", () => { + expect(checkRemoteProtocolCompatibility({ ...metadata, protocol: 2, minimumClientProtocol: 2 })).toEqual({ + ok: false, + reason: "hub-too-new", + message: "OpenCodex hub requires remote protocol 2; this client supports protocol 1. Upgrade ocx on this client.", + }); + }); + + test("classifies a hub below the client floor with the exact message", () => { + expect(checkRemoteProtocolCompatibility(metadata, { protocol: 2, minimumHubProtocol: 2 })).toEqual({ + ok: false, + reason: "hub-too-old", + message: "OpenCodex hub provides remote protocol 1; this client requires at least 2. Upgrade ocx on the hub.", + }); + }); + + test("malformed metadata is invalid, never a version mismatch", () => { + const malformed = [ + { ...metadata, protocol: 0 }, + { minimumClientProtocol: 1, managementUrl: metadata.managementUrl }, + { protocol: 1, managementUrl: metadata.managementUrl }, + { ...metadata, protocol: "1" }, + { ...metadata, protocol: Number.MAX_SAFE_INTEGER + 1 }, + { ...metadata, minimumClientProtocol: 2 }, + { ...metadata, managementUrl: "https://hub.example.test/path" }, + { ...metadata, managementUrl: "https://hub.example.test/?query=1" }, + { ...metadata, managementUrl: "https://hub.example.test/#fragment" }, + { ...metadata, managementUrl: "https://user@hub.example.test" }, + ]; + for (const value of malformed) { + expect(parseRemoteReadyMetadata(value)).toBeNull(); + expect(checkRemoteProtocolCompatibility(value)).toEqual({ + ok: false, + reason: "invalid", + message: invalidMessage, + }); + } + }); + + test("accepts an additive protocol level when the v1 intervals intersect", () => { + expect(checkRemoteProtocolCompatibility({ ...metadata, protocol: 2 })).toEqual({ + ok: true, + metadata: { ...metadata, protocol: 2 }, + }); + }); +}); + // ── probeReadiness: strict HTTP + contract enforcement ───────────────────────── function readyz(body: unknown, status = 200): Response { diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index ac913d4d33..e4030f8a59 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -1,5 +1,6 @@ import { waitForNativeMainStartupGate } from "../src/codex/native-profile-startup"; -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { createHash } from "node:crypto"; import { logsFromApiBody } from "./helpers/logs-api"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { request as httpRequest } from "node:http"; @@ -48,7 +49,6 @@ import { LOCAL_PROVIDER_RELOAD_CAPABILITY_VERSION } from "../src/lib/local-provi import { resetCodexModelEntitlementCacheForTests } from "../src/codex/model-entitlements"; import { getDebugLogEntries, resetDebugLogBufferForTests } from "../src/lib/debug-log-buffer"; import { resetDebugSettingsForTests, setDebugSettings } from "../src/lib/debug-settings"; - import { watchdogMs } from "./helpers/ci-watchdog"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; const previousOpencodexHome = process.env.OPENCODEX_HOME; @@ -82,6 +82,22 @@ function config(hostname?: string): OcxConfig { }; } +const REMOTE_CATALOG_BYTES = '{"models":[{"slug":"fixture/model","display_name":"Fixture Model","priority":1,"visibility":"list","base_instructions":"Fixture instructions","input_modalities":["text"]}]}'; +const REMOTE_DATA_KEY = "ocx_data_remote_catalog"; + +function remoteCatalogConfig(keyId = "remote-key"): OcxConfig { + return { + ...config("0.0.0.0"), + port: 0, + apiKeys: [{ id: keyId, name: "remote", key: REMOTE_DATA_KEY, createdAt: "2026-08-28T00:00:00.000Z" }], + }; +} + +function writeRemoteCatalog(): void { + if (!isolatedCodexHome) throw new Error("isolated Codex home is not installed"); + writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), REMOTE_CATALOG_BYTES); +} + function managementHeaders(initial?: HeadersInit): Headers { const token = configuredAdminToken(); if (!token) throw new Error("management token was not initialized"); @@ -4019,3 +4035,177 @@ describe("server local API auth", () => { } }); }); + +describe("GET /v1/catalog remote data plane", () => { + test("management and data-plane routes return byte-identical catalog bodies", async () => { + saveConfig(remoteCatalogConfig()); + writeRemoteCatalog(); + const server = startServer(0); + try { + const management = await fetch(new URL("/api/catalog", server.url), { headers: managementHeaders() }); + const remote = await fetch(new URL("/v1/catalog", server.url), { + headers: { "x-opencodex-api-key": REMOTE_DATA_KEY }, + }); + const managementBytes = new Uint8Array(await management.arrayBuffer()); + const remoteBytes = new Uint8Array(await remote.arrayBuffer()); + expect(management.status).toBe(200); + expect(remote.status).toBe(200); + expect(remoteBytes).toEqual(managementBytes); + expect(new TextDecoder().decode(remoteBytes)).toBe(REMOTE_CATALOG_BYTES); + // Management ETag spelling is hex, per the shipped catalogEtag() in + // src/server/catalog-download.ts. An earlier revision of this phase used a + // "sha256-" spelling from its own serializer, which no longer exists. + const expectedEtag = `"${createHash("sha256").update(remoteBytes).digest("hex")}"`; + // The bytes are identical across planes, but the caching contract is not: the + // management route may carry a validator because its representation does not vary by + // data-key identity, while this one must not. Asserting the management ETag here keeps + // the byte-identity claim honest without implying the remote route offers one. + expect(management.headers.get("etag")).toBe(expectedEtag); + expect(remote.headers.get("etag")).toBeNull(); + expect(remote.headers.get("cache-control")).toBe("no-store"); + expect(remote.headers.get("x-opencodex-key-id")).toBe("remote-key"); + } finally { + await server.stop(true); + } + }); + + test("admission accepts configured dedicated and bearer keys, and rejects every foreign class", async () => { + saveConfig(remoteCatalogConfig()); + writeRemoteCatalog(); + const server = startServer(0); + try { + const cases = [ + [{ "x-opencodex-api-key": REMOTE_DATA_KEY }, 200, "remote-key"], + [{ authorization: `Bearer ${REMOTE_DATA_KEY}` }, 200, "remote-key"], + // Accepted, matching /v1/models and the AUTH_MATRIX row this route shipped with in + // #809. An earlier revision of this phase rejected x-api-key here for least-privilege + // reasons, but this route forwards no caller credential upstream, so the header + // carries no extra authority — and rejecting it 401s Anthropic-SDK clients holding a + // perfectly valid data credential. The narrowing was a behavior regression against + // shipped code, not a hardening. + [{ "x-api-key": REMOTE_DATA_KEY }, 200, "remote-key"], + [{ authorization: "Bearer foreign-key" }, 401, null], + [{ authorization: `Bearer ${configuredAdminToken() ?? "missing-admin"}` }, 401, null], + [{ "x-opencodex-api-key": REMOTE_DATA_KEY, origin: "https://attacker.test" }, 403, null], + [{}, 401, null], + ] as const; + for (const [headers, status, keyId] of cases) { + const response = await fetch(new URL("/v1/catalog", server.url), { headers }); + expect(response.status).toBe(status); + expect(response.headers.get("x-opencodex-key-id")).toBe(keyId); + } + } finally { + await server.stop(true); + } + }); + + test("environment-token and loopback admission never emit a configured key id", async () => { + process.env.OPENCODEX_API_AUTH_TOKEN = "environment-catalog-token"; + saveConfig(remoteCatalogConfig()); + writeRemoteCatalog(); + const remote = startServer(0); + try { + const response = await fetch(new URL("/v1/catalog", remote.url), { + headers: { "x-opencodex-api-key": "environment-catalog-token" }, + }); + expect(response.status).toBe(200); + expect(response.headers.get("x-opencodex-key-id")).toBeNull(); + } finally { + await remote.stop(true); + } + + const loopbackConfig = remoteCatalogConfig(); + loopbackConfig.hostname = "127.0.0.1"; + saveConfig(loopbackConfig); + const loopback = startServer(0); + try { + const response = await fetch(new URL("/v1/catalog", loopback.url)); + expect(response.status).toBe(200); + expect(response.headers.get("x-opencodex-key-id")).toBeNull(); + } finally { + await loopback.stop(true); + } + }); + + test("an unsafe configured key id is omitted with one id-free warning", async () => { + const unsafeId = "unsafe key id"; + saveConfig(remoteCatalogConfig(unsafeId)); + writeRemoteCatalog(); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/catalog", server.url), { + headers: { "x-opencodex-api-key": REMOTE_DATA_KEY }, + }); + expect(response.status).toBe(200); + expect(response.headers.get("x-opencodex-key-id")).toBeNull(); + // Other subsystems (config repair, provider migration) may warn during startup; + // this contract is about the remote-catalog warning specifically: exactly one, + // and it never echoes the unsafe id. + const remoteCatalogWarns = warnSpy.mock.calls + .map(call => call.map(String).join(" ")) + .filter(line => line.includes("[remote-catalog]")); + expect(remoteCatalogWarns).toHaveLength(1); + expect(remoteCatalogWarns[0]).not.toContain(unsafeId); + expect(warnSpy.mock.calls.flat().map(String).join(" ")).not.toContain(unsafeId); + } finally { + await server.stop(true); + warnSpy.mockRestore(); + } + }); + + test("no conditional request can elicit a 304, and no validator is offered to build one from", async () => { + // The response body varies by key identity, so a shared strong validator would let a + // store revalidate one identity's representation for another. The route therefore + // carries no ETag at all: there is nothing for a client to send back, and every + // If-None-Match spelling — including ones that would match a validator if one existed — + // gets the full body. An earlier revision of this phase asserted the opposite here. + saveConfig(remoteCatalogConfig()); + writeRemoteCatalog(); + const server = startServer(0); + try { + const first = await fetch(new URL("/v1/catalog", server.url), { + headers: { "x-opencodex-api-key": REMOTE_DATA_KEY }, + }); + expect(first.status).toBe(200); + expect(first.headers.get("etag")).toBeNull(); + expect(first.headers.get("cache-control")).toBe("no-store"); + + for (const validator of ['"sha256-anything"', 'W/"sha256-anything"', '"stale", "other"', "*", "malformed"]) { + const response = await fetch(new URL("/v1/catalog", server.url), { + headers: { "x-opencodex-api-key": REMOTE_DATA_KEY, "if-none-match": validator }, + }); + expect(response.status).toBe(200); + expect(await response.text()).toBe(REMOTE_CATALOG_BYTES); + expect(response.headers.get("etag")).toBeNull(); + expect(response.headers.get("cache-control")).toBe("no-store"); + } + } finally { + await server.stop(true); + } + }); + + test("method and path matching stay exact ahead of the unknown-v1 guard", async () => { + saveConfig(remoteCatalogConfig()); + writeRemoteCatalog(); + const server = startServer(0); + try { + for (const [path, method] of [ + ["/v1/catalog", "POST"], + ["/v1/catalog/", "GET"], + ["/v1/does-not-exist", "GET"], + ] as const) { + const response = await fetch(new URL(path, server.url), { + method, + headers: { "x-opencodex-api-key": REMOTE_DATA_KEY }, + }); + expect(response.status).toBe(404); + expect(response.headers.get("content-type")).toContain("application/json"); + expect(await response.json()).toMatchObject({ error: { code: "not_found" } }); + expect(response.headers.get("x-opencodex-key-id")).toBeNull(); + } + } finally { + await server.stop(true); + } + }); +}); diff --git a/tests/server-live.test.ts b/tests/server-live.test.ts index a92e4ec0a0..53418c4627 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -121,6 +121,14 @@ function forwardConfig(): OcxConfig { } as OcxConfig; } +function expectReadyProtocolMetadata(body: Record, managementUrl: string): void { + expect(body).toMatchObject({ + protocol: 1, + minimumClientProtocol: 1, + managementUrl, + }); +} + function multipartLiveBody( sdp = "v=0", session: Record | null = { model: "gpt-live" }, @@ -1231,13 +1239,17 @@ describe("GET /readyz", () => { try { const pending = await fetch(new URL("/readyz", server.url)); expect(pending.status).toBe(503); - expect(((await pending.json()) as { status: string }).status).toBe("pending"); + const pendingBody = (await pending.json()) as Record; + expect(pendingBody.status).toBe("pending"); + expectReadyProtocolMetadata(pendingBody, new URL(server.url).origin); await runStartupReadinessSync(gate, async () => outcome); const settled = await fetch(new URL("/readyz", server.url)); expect(settled.status).toBe(expectedHttp); - expect(((await settled.json()) as { status: string }).status).toBe(expectedStatus); + const settledBody = (await settled.json()) as Record; + expect(settledBody.status).toBe(expectedStatus); + expectReadyProtocolMetadata(settledBody, new URL(server.url).origin); } finally { await server.stop(true); } @@ -1268,8 +1280,11 @@ describe("GET /readyz", () => { expect(typeof readyzBody.uptime).toBe("number"); expect(typeof readyzBody.pid).toBe("number"); expect(typeof readyzBody.port).toBe("number"); + expectReadyProtocolMetadata(readyzBody, new URL(base).origin); // Sanitization: the body must never carry sync diagnostics, paths, or warnings. - expect(Object.keys(readyzBody).sort()).toEqual(["pid", "port", "service", "status", "uptime", "version"]); + expect(Object.keys(readyzBody).sort()).toEqual([ + "managementUrl", "minimumClientProtocol", "pid", "port", "protocol", "service", "status", "uptime", "version", + ]); expect(JSON.stringify(readyzBody)).not.toContain("warning"); expect(JSON.stringify(readyzBody)).not.toContain("path"); } finally { @@ -1517,7 +1532,9 @@ describe("GET /readyz while draining", () => { const drainRes = await fetch(new URL("/readyz", base)); expect(drainRes.status).toBe(503); expect(drainRes.headers.get("retry-after")).toBe("1"); - expect(((await drainRes.json()) as { status: string }).status).toBe("pending"); + const drainBody = (await drainRes.json()) as Record; + expect(drainBody.status).toBe("pending"); + expectReadyProtocolMetadata(drainBody, new URL(base).origin); // The gate itself is untouched — draining is a listener state, not a gate // transition, so the startup-sync ownership contract is preserved.