From d25cbc02a592dc6e772f6e83375c4d04c97fc347 Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 28 Aug 2026 01:45:54 +0900 Subject: [PATCH 1/6] feat(remote): add protocol metadata and runtime role --- src/config.ts | 35 ++++++++++++++ src/remote/protocol.ts | 88 ++++++++++++++++++++++++++++++++++++ src/server/index.ts | 2 + src/server/proxy-liveness.ts | 6 +++ src/types/config.ts | 4 ++ 5 files changed, 135 insertions(+) create mode 100644 src/remote/protocol.ts 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..de366d5e53 --- /dev/null +++ b/src/remote/protocol.ts @@ -0,0 +1,88 @@ +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; + } +} + +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 = managementOrigin(new URL(req.url).origin); + 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..e0e30b618d 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -210,6 +210,7 @@ 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; @@ -1041,6 +1042,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server Date: Fri, 28 Aug 2026 01:47:25 +0900 Subject: [PATCH 2/6] feat(remote): serve authenticated catalog snapshots --- src/server/auth-cors.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 0174794c6a..2bffde448c 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -403,6 +403,7 @@ export const AUTH_MATRIX: readonly ApiAuthMatrixRow[] = [ // passthrough, so the two bearer domains still never mix. `x-api-key` is still rejected. { endpoint: "/v1/responses", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, { endpoint: "/v1/chat/completions", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, + { endpoint: "/v1/catalog", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, { endpoint: "/v1/messages", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, { endpoint: "/v1/models", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, // #809: least-privilege catalog read for remote Codex clients. Same admission set as From 1bb9828a17aa1c51b553f290994d6383829e8c83 Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 28 Aug 2026 01:51:44 +0900 Subject: [PATCH 3/6] fix(remote): derive management origin from request host --- src/remote/protocol.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/remote/protocol.ts b/src/remote/protocol.ts index de366d5e53..71ee0c10b5 100644 --- a/src/remote/protocol.ts +++ b/src/remote/protocol.ts @@ -32,11 +32,21 @@ function managementOrigin(value: unknown): string | 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 = managementOrigin(new URL(req.url).origin); + const managementUrl = observedManagementOrigin(req); if (!managementUrl) throw new Error("Readiness request does not have an HTTP(S) management origin"); return { protocol: REMOTE_HUB_PROTOCOL, From 825800e359fb6e3d11029fad6976415428b0b268 Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 28 Aug 2026 01:53:01 +0900 Subject: [PATCH 4/6] test(remote): cover phase one protocol and catalog contract --- tests/api-catalog-route.test.ts | 51 ++++++--- tests/config.test.ts | 63 +++++++++++ tests/proxy-liveness.test.ts | 105 ++++++++++++++++++ tests/server-auth.test.ts | 189 +++++++++++++++++++++++++++++++- tests/server-live.test.ts | 25 ++++- 5 files changed, 415 insertions(+), 18 deletions(-) diff --git a/tests/api-catalog-route.test.ts b/tests/api-catalog-route.test.ts index 634ce3e7e1..f639d61f95 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,19 @@ describe("GET /api/catalog route (#709)", () => { expect(response?.status).toBe(404); expect(await response!.json()).toEqual({ error: "catalog not found" }); }); + + test("returns a bounded server failure for a malformed persisted catalog", async () => { + 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(500); + expect(await response!.json()).toEqual({ error: "catalog unavailable" }); + }); }); describe("GET|HEAD /v1/catalog least-privilege data-plane route (#809)", () => { 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..94db2e0854 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,6 +49,11 @@ 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 { + MAX_REMOTE_CATALOG_BYTES, + catalogDataPlaneResponse, + type SerializedCatalog, +} from "../src/server/catalog-download"; import { watchdogMs } from "./helpers/ci-watchdog"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; @@ -82,6 +88,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 +4041,168 @@ 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); + const expectedEtag = `"sha256-${createHash("sha256").update(remoteBytes).digest("base64url")}"`; + expect(remote.headers.get("etag")).toBe(expectedEtag); + expect(remote.headers.get("cache-control")).toBe("private, no-cache"); + 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"], + [{ "x-api-key": REMOTE_DATA_KEY }, 401, null], + [{ 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(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls.flat().join(" ")).not.toContain(unsafeId); + } finally { + await server.stop(true); + warnSpy.mockRestore(); + } + }); + + test("strong, weak, list, and wildcard validators return bodyless 304", async () => { + 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 }, + }); + const etag = first.headers.get("etag")!; + for (const validator of [etag, `W/${etag}`, `"stale", ${etag}`, "*"]) { + 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(304); + expect(await response.text()).toBe(""); + expect(response.headers.get("etag")).toBe(etag); + expect(response.headers.get("cache-control")).toBe("private, no-cache"); + expect(response.headers.get("x-opencodex-key-id")).toBe("remote-key"); + } + for (const validator of ['"stale"', `W/ ${etag}`, "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("cache-control")).toBe("private, no-cache"); + } + } finally { + await server.stop(true); + } + }); + + test("the remote size cap accepts exactly the bound and rejects cap plus one", async () => { + const request = new Request("http://localhost/v1/catalog"); + const policy = { hostname: "127.0.0.1" }; + const atCap: SerializedCatalog = { bytes: new Uint8Array(MAX_REMOTE_CATALOG_BYTES) }; + const overCap: SerializedCatalog = { bytes: new Uint8Array(MAX_REMOTE_CATALOG_BYTES + 1) }; + expect(catalogDataPlaneResponse(atCap, request, policy).status).toBe(200); + const rejected = catalogDataPlaneResponse(overCap, request, policy); + expect(rejected.status).toBe(503); + expect(await rejected.json()).toMatchObject({ error: { code: "catalog_too_large" } }); + }); + + 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. From 8980430c291d71077c434a48f0fc6dc8dedf8666 Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 28 Aug 2026 01:56:54 +0900 Subject: [PATCH 5/6] fix(remote): type catalog bytes over ArrayBuffer and scope the key-id warn assertion --- tests/server-auth.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 94db2e0854..dfc595e01c 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -4131,8 +4131,15 @@ describe("GET /v1/catalog remote data plane", () => { }); expect(response.status).toBe(200); expect(response.headers.get("x-opencodex-key-id")).toBeNull(); - expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy.mock.calls.flat().join(" ")).not.toContain(unsafeId); + // 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(); From 4fdd54d4613f73ae8cf3a3bee5d3835a10f3d99e Mon Sep 17 00:00:00 2001 From: jun Date: Tue, 1 Sep 2026 17:10:52 +0900 Subject: [PATCH 6/6] fix(remote): reconcile the phase-one catalog contract with the landed /v1/catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2979 landed /v1/catalog on dev while this phase was open, so the rebase had to decide, per behavior, which side wins rather than taking one branch wholesale. Taken from dev, because narrowing shipped behavior is a regression, not a hardening: GET+HEAD instead of GET-only; a 256 MiB cap with 507 instead of 32 MiB with 503, since a supported 2,000-model catalog serializes to roughly 92 MB and the smaller cap rejected valid input; a malformed persisted catalog rendering as 404 rather than 500, which keeps "your file is corrupt" indistinguishable from "you have no catalog"; and x-api-key accepted, matching /v1/models and the AUTH_MATRIX row #809 shipped. This route forwards no caller credential upstream, so x-api-key carries no extra authority — rejecting it would only 401 Anthropic-SDK clients holding a valid data credential. Taken from this phase, because dev has no equivalent: the x-opencodex-key-id echo that names which configured key was admitted. dev defined withRemoteCatalogKeyId but never called it, so multi-key attribution was dead code; it is now wired into the route, scoped to configured keys, with an unsafe id omitted rather than sanitized and its warning never repeating the id. Taken from neither, per D2 in the design unit: the route emits Cache-Control: no-store with no ETag and no 304. dev's private,no-cache plus a strong byte-derived ETag is unsafe on a body that varies by key identity — no-cache permits storage and forces revalidation, and the revalidation is what crosses identities. /api/catalog keeps its validator; it is management-authenticated, loopback-scoped, and identity-invariant. The duplicate AUTH_MATRIX row this phase added is dropped: dev already had one for /v1/catalog, and the two disagreed on x-api-key, so the matrix contradicted itself and the live-server check failed against whichever row it read first. --- src/server/auth-cors.ts | 1 - src/server/index.ts | 63 +++++++++++++++++++++++++------ tests/api-catalog-route.test.ts | 36 ++++++++++++------ tests/server-auth.test.ts | 66 ++++++++++++++++----------------- 4 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 2bffde448c..0174794c6a 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -403,7 +403,6 @@ export const AUTH_MATRIX: readonly ApiAuthMatrixRow[] = [ // passthrough, so the two bearer domains still never mix. `x-api-key` is still rejected. { endpoint: "/v1/responses", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, { endpoint: "/v1/chat/completions", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, - { endpoint: "/v1/catalog", bearer: "accepted", dedicated: "accepted", xApiKey: "rejected" }, { endpoint: "/v1/messages", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, { endpoint: "/v1/models", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, // #809: least-privilege catalog read for remote Codex clients. Same admission set as diff --git a/src/server/index.ts b/src/server/index.ts index e0e30b618d..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, @@ -214,6 +215,33 @@ 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; @@ -1123,23 +1151,34 @@ 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/tests/api-catalog-route.test.ts b/tests/api-catalog-route.test.ts index f639d61f95..f46632a079 100644 --- a/tests/api-catalog-route.test.ts +++ b/tests/api-catalog-route.test.ts @@ -87,7 +87,13 @@ describe("GET /api/catalog route (#709)", () => { expect(await response!.json()).toEqual({ error: "catalog not found" }); }); - test("returns a bounded server failure for a malformed persisted catalog", async () => { + 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"); @@ -96,8 +102,8 @@ describe("GET /api/catalog route (#709)", () => { url, loadConfig(), ); - expect(response?.status).toBe(500); - expect(await response!.json()).toEqual({ error: "catalog unavailable" }); + expect(response?.status).toBe(404); + expect(await response!.json()).toEqual({ error: "catalog not found" }); }); }); @@ -147,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"); @@ -161,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), { @@ -174,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/server-auth.test.ts b/tests/server-auth.test.ts index dfc595e01c..e4030f8a59 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -49,12 +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 { - MAX_REMOTE_CATALOG_BYTES, - catalogDataPlaneResponse, - type SerializedCatalog, -} from "../src/server/catalog-download"; - import { watchdogMs } from "./helpers/ci-watchdog"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; const previousOpencodexHome = process.env.OPENCODEX_HOME; @@ -4058,9 +4052,17 @@ describe("GET /v1/catalog remote data plane", () => { expect(remote.status).toBe(200); expect(remoteBytes).toEqual(managementBytes); expect(new TextDecoder().decode(remoteBytes)).toBe(REMOTE_CATALOG_BYTES); - const expectedEtag = `"sha256-${createHash("sha256").update(remoteBytes).digest("base64url")}"`; - expect(remote.headers.get("etag")).toBe(expectedEtag); - expect(remote.headers.get("cache-control")).toBe("private, no-cache"); + // 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); @@ -4075,7 +4077,13 @@ describe("GET /v1/catalog remote data plane", () => { const cases = [ [{ "x-opencodex-api-key": REMOTE_DATA_KEY }, 200, "remote-key"], [{ authorization: `Bearer ${REMOTE_DATA_KEY}` }, 200, "remote-key"], - [{ "x-api-key": REMOTE_DATA_KEY }, 401, null], + // 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], @@ -4146,7 +4154,12 @@ describe("GET /v1/catalog remote data plane", () => { } }); - test("strong, weak, list, and wildcard validators return bodyless 304", async () => { + 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); @@ -4154,41 +4167,24 @@ describe("GET /v1/catalog remote data plane", () => { const first = await fetch(new URL("/v1/catalog", server.url), { headers: { "x-opencodex-api-key": REMOTE_DATA_KEY }, }); - const etag = first.headers.get("etag")!; - for (const validator of [etag, `W/${etag}`, `"stale", ${etag}`, "*"]) { - 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(304); - expect(await response.text()).toBe(""); - expect(response.headers.get("etag")).toBe(etag); - expect(response.headers.get("cache-control")).toBe("private, no-cache"); - expect(response.headers.get("x-opencodex-key-id")).toBe("remote-key"); - } - for (const validator of ['"stale"', `W/ ${etag}`, "malformed"]) { + 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("cache-control")).toBe("private, no-cache"); + expect(response.headers.get("etag")).toBeNull(); + expect(response.headers.get("cache-control")).toBe("no-store"); } } finally { await server.stop(true); } }); - test("the remote size cap accepts exactly the bound and rejects cap plus one", async () => { - const request = new Request("http://localhost/v1/catalog"); - const policy = { hostname: "127.0.0.1" }; - const atCap: SerializedCatalog = { bytes: new Uint8Array(MAX_REMOTE_CATALOG_BYTES) }; - const overCap: SerializedCatalog = { bytes: new Uint8Array(MAX_REMOTE_CATALOG_BYTES + 1) }; - expect(catalogDataPlaneResponse(atCap, request, policy).status).toBe(200); - const rejected = catalogDataPlaneResponse(overCap, request, policy); - expect(rejected.status).toBe(503); - expect(await rejected.json()).toMatchObject({ error: { code: "catalog_too_large" } }); - }); - test("method and path matching stay exact ahead of the unknown-v1 guard", async () => { saveConfig(remoteCatalogConfig()); writeRemoteCatalog();