From 20b8c70e051d08e44a5c291b5686be2b56ec5143 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 22 Sep 2026 05:08:30 +0900 Subject: [PATCH 01/10] fix(service): combine token-binding, qualified-localhost bind, and WSL ownership state Combines three fork PRs touching service guards: bind reused-token hardening to the validated file (#502), normalize qualified localhost binds (#458), and accept legacy WSL ownership state (#262), rebased onto current dev. bun test: service-secrets + service-auth-qualified-localhost + codex-home-wsl + server-auth-localhost-bind 21 pass; server-auth + windows-deploy-close-regressions 117 pass with 2 env-flaky (stalled-400 fails on clean dev baseline; catalog admission fails only in full-file ordering, passes isolated and on baseline) (cherry picked from commit 6c9c3d89cf5e1d499000398ffe4152164c2296a3) --- scripts/test-layout/layout.json | 2 + src/lib/service-secrets.ts | 95 ++++++++++++++++++- src/server/index.ts | 5 +- src/service.ts | 2 +- src/service/guards.ts | 81 ++++++++-------- src/service/state.ts | 13 ++- structure/codex-home.md | 5 +- .../codex-integration/codex-home-wsl.test.ts | 25 ++++- tests/fixtures/test-layout-expected.json | 2 + tests/helpers/server-auth-config.ts | 18 ++++ .../server/server-auth-localhost-bind.test.ts | 42 ++++++++ tests/server/server-auth.test.ts | 18 +--- .../service-auth-qualified-localhost.test.ts | 69 ++++++++++++++ tests/service/service-secrets.test.ts | 46 +++++++++ .../windows-deploy-close-regressions.test.ts | 2 +- 15 files changed, 355 insertions(+), 70 deletions(-) create mode 100644 tests/helpers/server-auth-config.ts create mode 100644 tests/server/server-auth-localhost-bind.test.ts create mode 100644 tests/service/service-auth-qualified-localhost.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 0cbb0fc7b38..8ef9cb21959 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1354,6 +1354,7 @@ "self-launch-argv.test.ts": "lib", "server-403-permission-e2e.test.ts": "server", "server-agent-task-recovery-replay.test.ts": "server", + "server-auth-localhost-bind.test.ts": "server", "server-auth.test.ts": "server", "server-background-lifecycle.test.ts": "server", "server-clickjacking-headers.test.ts": "server", @@ -1381,6 +1382,7 @@ "server-xai-oauth-401-replay.test.ts": "server", "server-xai-responses-streaming.test.ts": "server", "service-ownership-compatibility.test.ts": "service", + "service-auth-qualified-localhost.test.ts": "service", "service-ownership-handover.test.ts": "service", "service-ownership-state.test.ts": "service", "service-probe-docker.test.ts": "service", diff --git a/src/lib/service-secrets.ts b/src/lib/service-secrets.ts index 7dbd58a8991..926bd21cc73 100644 --- a/src/lib/service-secrets.ts +++ b/src/lib/service-secrets.ts @@ -1,8 +1,8 @@ import { createHash } from "node:crypto"; -import { closeSync, existsSync, fsyncSync, lstatSync, openSync, readFileSync, unlinkSync } from "node:fs"; +import { closeSync, constants, existsSync, fchmodSync, fstatSync, fsyncSync, lstatSync, openSync, readFileSync, readSync, unlinkSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config"; -import { atomicWriteFile } from "../config/atomic-write"; +import { atomicWriteFile, atomicWriteFileNoFollow } from "../config/atomic-write"; const MAX_SERVICE_API_TOKEN_BYTES = 4096; @@ -49,6 +49,97 @@ export function readServiceApiTokenState(): ServiceApiTokenState { } } +/** + * Validate and tighten a reused service token without applying permissions to a + * pathname that may have been replaced since validation. + * + * The token is read off the opened descriptor β€” never off the path a second + * time β€” and once it validates, it is REPUBLISHED through the no-follow atomic + * writer rather than hardened in place. Windows ACL tooling is pathname-based, + * so an in-place harden there could still land on a substituted entry; the + * republish instead replaces whatever entry sits at the path with a freshly + * hardened owner-only file holding the same token. On return the path names + * that file, which is the contract `origin: "file"` reports. On POSIX the + * opened descriptor is also fchmod'd first, so a token-bearing inode a race + * moved aside is still tightened wherever its entry ended up. + * + * Callers must run this under `withConfigMutationLockSync`: client-key rotation + * replaces the token under that lock, and a republish outside it could rename a + * stale token back over a committed rotation. + */ +export function hardenReusedServiceApiToken( + validate: (token: string) => void, +): ServiceApiTokenState { + const path = serviceApiTokenFilePath(); + // O_NOFOLLOW refuses a symlinked entry and O_NONBLOCK keeps a FIFO (or other + // blocking node) from stalling the open before fstat can reject it. Windows + // omits both flags, so there the descriptor is bound to its entry by the + // lstat/fstat identity comparison below. + const flags = process.platform === "win32" + ? constants.O_RDONLY + : constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK; + let fd: number | undefined; + try { + fd = openSync(path, flags); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT") return { kind: "absent" }; + if (code === "ELOOP") return { kind: "unsafe", reason: "service token path is not a bounded regular file" }; + return { kind: "unsafe", reason: "service token path could not be inspected" }; + } + try { + const stat = fstatSync(fd, { bigint: true }); + if (!stat.isFile() || stat.size > BigInt(MAX_SERVICE_API_TOKEN_BYTES)) { + return { kind: "unsafe", reason: "service token path is not a bounded regular file" }; + } + if (process.platform === "win32") { + let entry; + try { + entry = lstatSync(path, { bigint: true }); + } catch { + return { kind: "unsafe", reason: "service token path could not be inspected" }; + } + if (entry.isSymbolicLink() || !entry.isFile() || entry.dev !== stat.dev || entry.ino !== stat.ino) { + return { kind: "unsafe", reason: "service token path is not a bounded regular file" }; + } + } + // Bound the read as well as the stat: a file that grows past the cap after + // fstat is unsafe, not something to buffer whole. + const bytes = Buffer.alloc(MAX_SERVICE_API_TOKEN_BYTES + 1); + let length = 0; + try { + while (length < bytes.length) { + const count = readSync(fd, bytes, length, bytes.length - length, null); + if (!count) break; + length += count; + } + } catch { + return { kind: "unsafe", reason: "service token file could not be read" }; + } + if (length > MAX_SERVICE_API_TOKEN_BYTES) { + return { kind: "unsafe", reason: "service token path is not a bounded regular file" }; + } + const token = bytes.subarray(0, length).toString("utf8").trim(); + if (!token) return { kind: "unsafe", reason: "service token file is empty" }; + validate(token); + if (process.platform !== "win32") { + // Best-effort matches the previous repair behavior: the descriptor binds + // the chmod to the regular file opened above even if its directory entry + // moved, so the validated inode is never left loose under another name. + try { fchmodSync(fd, 0o600); } catch { /* best-effort */ } + } + // The descriptor's work ends here β€” and must: Windows refuses to rename over + // a file this process still holds open, so the republish cannot run while it + // is held. + closeSync(fd); + fd = undefined; + atomicWriteFileNoFollow(path, `${token}\n`); + return { kind: "present", token, fingerprint: serviceApiTokenFingerprint(token) }; + } finally { + if (fd !== undefined) closeSync(fd); + } +} + export function writeServiceApiTokenFile(token: string): PersistedServiceApiToken { const value = token.trim(); if (!value || /[\r\n\0]/.test(value) || Buffer.byteLength(value) > MAX_SERVICE_API_TOKEN_BYTES) { diff --git a/src/server/index.ts b/src/server/index.ts index 61fbcec7d8a..7dba95c8f8a 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -320,12 +320,13 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe const listenPort = port ?? config.port ?? 10100; setCorsOrigin(listenPort); - // Canonicalize an explicit "localhost" bind to IPv4 so it matches the injected base_url (which + // Canonicalize an explicit "localhost" bind (including its fully-qualified spelling) to IPv4 + // so it matches the injected base_url (which // resolves localhostβ†’127.0.0.1): on Windows `localhost` resolves ::1-first, but the injected URL // is 127.0.0.1, so binding literal "localhost" would reintroduce the F4 refusal. Wildcards // (0.0.0.0/::) and specific hosts are left untouched so intentional exposure is preserved. const configuredHost = config.hostname?.trim(); - const bindHost = !configuredHost || /^localhost$/i.test(configuredHost) ? "127.0.0.1" : configuredHost; + const bindHost = !configuredHost || /^localhost\.?$/i.test(configuredHost) ? "127.0.0.1" : configuredHost; // Unauthenticated loopback listener (#1102). Off unless explicitly enabled. // A port-less enabled entry is the companion form: same port as the public listener, on diff --git a/src/service.ts b/src/service.ts index 79cfff6efda..8be7805d0ad 100644 --- a/src/service.ts +++ b/src/service.ts @@ -7,7 +7,7 @@ */ export type { ServiceBackend, ServiceInstallState, ServiceStateEvidence, ServiceStateResolution, ServiceOwner, ServiceOwnership, ServiceOwnershipSubject, ServiceOwnershipResolution, ServiceStateSwapDeps, RecordServiceOwnerRequest, RecordServiceOwnerDeps, ReleaseServiceOwnerDeps, RemoveServiceStateDeps } from "./service/state"; -export { SERVICE_MANAGED_ENV, SERVICE_OWNERSHIP_PROTOCOL_VERSION, SERVICE_OWNERSHIP_MINIMUM_CLI_VERSION, stableLauncherEntry, serviceLogPath, serviceStatePaths, serviceStatePathsForOpenCodexHome, parseServiceInstallState, parseServiceOwnership, inspectServiceStateEvidence, resolveServiceState, currentServiceHomes, serviceHomeMatches, readServiceBackend, serviceReinstallArgs, serviceInstallArgs, ServiceStateConflictError, ServiceOwnershipSubjectMismatchError, ServiceOwnershipSubjectUnknownError, ServiceTakeoverCompatibilityChangedError, swapServiceInstallState, removeServiceInstallStateRecords, serviceOwnership, resolveServiceOwnership, sameServiceOwnershipSubject, desktopOwnsService, ownershipGrantedTo, recordServiceOwner, releaseServiceOwner } from "./service/state"; +export { SERVICE_MANAGED_ENV, SERVICE_OWNERSHIP_PROTOCOL_VERSION, SERVICE_OWNERSHIP_MINIMUM_CLI_VERSION, stableLauncherEntry, serviceLogPath, serviceStatePaths, serviceStatePathsForOpenCodexHome, parseServiceInstallState, parseServiceOwnership, inspectServiceStateEvidence, resolveServiceState, currentServiceHomes, serviceHomeMatches, serviceCodexHomeMatchesInstall, readServiceBackend, serviceReinstallArgs, serviceInstallArgs, ServiceStateConflictError, ServiceOwnershipSubjectMismatchError, ServiceOwnershipSubjectUnknownError, ServiceTakeoverCompatibilityChangedError, swapServiceInstallState, removeServiceInstallStateRecords, serviceOwnership, resolveServiceOwnership, sameServiceOwnershipSubject, desktopOwnsService, ownershipGrantedTo, recordServiceOwner, releaseServiceOwner } from "./service/state"; export type { OwnershipMutationLeaseOptions, OwnershipMutationLease } from "./service/ownership-mutation-lease.mjs"; export { acquireOwnershipMutationLease, withOwnershipMutationLease } from "./service/ownership-mutation-lease.mjs"; export type { ManagingCliRole, ManagingCliObservation, RegisteredManagingCliInvocation, ServiceTakeoverCompatibilityInput, ServiceTakeoverCompatibility } from "./service/ownership-compatibility"; diff --git a/src/service/guards.ts b/src/service/guards.ts index 3afb9b25f99..d052c427eb6 100644 --- a/src/service/guards.ts +++ b/src/service/guards.ts @@ -1,7 +1,8 @@ import { execSync } from "node:child_process"; import { chmodSync, existsSync, mkdirSync, writeFileSync } from "node:fs"; import { getConfigDir, loadConfig } from "../config"; -import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; +import { withConfigMutationLockSync } from "../config/mutation-lock"; +import { hardenReusedServiceApiToken, readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; import { tokenCollidesWithAdmin } from "../lib/admin-secrets"; import { randomBytes } from "node:crypto"; import { hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl"; @@ -9,8 +10,9 @@ import { recordOwnedConfigPath } from "../lib/config-ownership"; import { isTestHomeGuardArmed } from "../lib/test-home-guard"; import { diagnoseService } from "./diagnostics"; import type { ServiceDiagnostic } from "./diagnostics"; -import { currentCodexHome, currentOpenCodexHome, normalizePathForCompare, readServiceInstallState } from "./state"; +import { currentCodexHome, currentOpenCodexHome, normalizePathForCompare, readServiceInstallState, serviceCodexHomeMatchesInstall } from "./state"; import { resolveCodexSqliteHome } from "../codex/paths"; +import { isLoopbackHostname } from "../codex/loopback-target"; import { win32 } from "node:path"; /** @@ -46,9 +48,7 @@ export function assertServiceEnvironmentMatchesInstall(): void { const state = readServiceInstallState(); if (!state) return; const actualCodexHome = currentCodexHome(); - const expected = normalizePathForCompare(state.codexHome); - const actual = normalizePathForCompare(actualCodexHome); - if (expected !== actual) { + if (!serviceCodexHomeMatchesInstall(state.codexHome)) { throw new ServiceOwnershipError( `Service was installed with CODEX_HOME=${state.codexHome}, but current CODEX_HOME=${actualCodexHome}. ` + "Run the service command from the same Codex home so native Codex restore updates the correct config.", @@ -73,11 +73,6 @@ export function assertServiceEnvironmentMatchesInstall(): void { } } -function isLoopbackHostname(hostname: string | undefined): boolean { - const normalized = (hostname ?? "127.0.0.1").trim().toLowerCase(); - return normalized === "" || normalized === "localhost" || normalized === "127.0.0.1" || normalized === "::1" || normalized === "[::1]"; -} - /** * The `ocx` command a user should rerun for the service state they actually have. * @@ -228,42 +223,42 @@ function persistServiceApiToken(token: string): string { * connected to a hub the same file holds that hub's issued client key, which must not be * overwritten by a local install. * + * Provisioning runs inside the cross-process config mutation lock: client-key rotation + * replaces `service-api-token` and records the new fingerprint under the same lock, so a + * reuse republish or a fresh write here can never interleave with a committed rotation and + * silently roll its bytes back. + * * The PATH is logged; the value never is, and never reaches argv, a unit file or a plist. */ export function writeServiceApiTokenFile(): ProvisionedServiceApiToken | null { - const token = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); - if (token) { - // Last line of defence: every install/repair path funnels through here, so a - // collision cannot reach disk regardless of which caller ran (#2696). - assertNotAdminToken(token); - const path = persistServiceApiToken(token); - console.log(`πŸ” Data-plane token taken from OPENCODEX_API_AUTH_TOKEN and stored at ${path} (owner-only).`); - return { path, origin: "env" }; - } - if (isLoopbackHostname(loadConfig().hostname)) return null; - const existing = readServiceApiTokenState(); - if (existing.kind === "present") { - // The collision check is NOT only for the env branch. A file that already holds the admin - // token -- hand-pasted before #2696, or written by the very incident this unit closes -- - // was silently accepted here, so `ocx status` reported `present (file)` and the hub - // crash-looped at boot with no command pointing at the cause. - const path = serviceApiTokenFilePath(); - assertNotAdminToken(existing.token, process.env, "file"); - // `readServiceApiTokenState` accepts any bounded regular file, so a reused token may well - // be group- or world-readable. Tighten it on the way through rather than claiming - // "owner-only" about a mode nobody checked; best-effort, since a non-owner cannot chmod - // and failing the install over it would be worse than the loose mode. - try { chmodSync(path, 0o600); } catch { /* best-effort */ } - if (process.platform === "win32") hardenSecretPath(path, { required: false }); - // No log line: repair/restart hit this on every run and an unconditional notice about a - // credential file trains operators to ignore the one that matters. - return { path, origin: "file" }; - } - if (existing.kind === "unsafe") throw new Error(`${existing.reason}: ${serviceApiTokenFilePath()}`); - const path = persistServiceApiToken(randomBytes(32).toString("hex")); - console.log(`πŸ” Provisioned an owner-only data-plane token at ${path}; nothing needs to be exported by hand.`); - console.log(" Remote machines get their own per-client key β€” run 'ocx hub invite' instead of copying this file."); - return { path, origin: "generated" }; + return withConfigMutationLockSync(() => { + const token = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); + if (token) { + // Last line of defence: every install/repair path funnels through here, so a + // collision cannot reach disk regardless of which caller ran (#2696). + assertNotAdminToken(token); + const path = persistServiceApiToken(token); + console.log(`πŸ” Data-plane token taken from OPENCODEX_API_AUTH_TOKEN and stored at ${path} (owner-only).`); + return { path, origin: "env" }; + } + if (isLoopbackHostname(loadConfig().hostname)) return null; + const existing = hardenReusedServiceApiToken(token => assertNotAdminToken(token, process.env, "file")); + if (existing.kind === "present") { + // The collision check is NOT only for the env branch. A file that already holds the admin + // token -- hand-pasted before #2696, or written by the very incident this unit closes -- + // was silently accepted here, so `ocx status` reported `present (file)` and the hub + // crash-looped at boot with no command pointing at the cause. + const path = serviceApiTokenFilePath(); + // No log line: repair/restart hit this on every run and an unconditional notice about a + // credential file trains operators to ignore the one that matters. + return { path, origin: "file" }; + } + if (existing.kind === "unsafe") throw new Error(`${existing.reason}: ${serviceApiTokenFilePath()}`); + const path = persistServiceApiToken(randomBytes(32).toString("hex")); + console.log(`πŸ” Provisioned an owner-only data-plane token at ${path}; nothing needs to be exported by hand.`); + console.log(" Remote machines get their own per-client key β€” run 'ocx hub invite' instead of copying this file."); + return { path, origin: "generated" }; + }); } export function sh(cmd: string): string { diff --git a/src/service/state.ts b/src/service/state.ts index dbd2520b07b..4964f947d8c 100644 --- a/src/service/state.ts +++ b/src/service/state.ts @@ -3,7 +3,7 @@ import { homedir } from "node:os"; import { delimiter, dirname, isAbsolute, join, posix, resolve, win32 } from "node:path"; import { expandUserPath, getConfigDir } from "../config"; import { atomicWriteFileStreamed } from "../config/atomic-write"; -import { resolveCodexHomeDir, type CodexHomeDeps } from "../codex/home"; +import { isWslRuntime, resolveCodexHomeDir, type CodexHomeDeps } from "../codex/home"; import { resolveCodexSqliteHome } from "../codex/paths"; import { durableBunRuntime, type BunRuntimeSource, type DurableBunRuntime } from "../lib/bun-runtime"; import { WINSW_SHA256, WINSW_VERSION } from "../lib/winsw"; @@ -857,6 +857,17 @@ export function serviceHomeMatches(a: string, b: string): boolean { return normalizePathForCompare(a) === normalizePathForCompare(b); } +/** Accept the Linux default written by service versions predating WSL home discovery. */ +export function serviceCodexHomeMatchesInstall(recordedHome: string, deps: CodexHomeDeps = {}): boolean { + const actualHome = currentCodexHome(deps); + if (serviceHomeMatches(recordedHome, actualHome)) return true; + + const env = deps.env ?? process.env; + if (env.CODEX_HOME?.trim() || !isWslRuntime(deps)) return false; + const legacyDefault = join((deps.homedir ?? homedir)(), ".codex"); + return serviceHomeMatches(recordedHome, legacyDefault); +} + /** Single accessor for backend-sensitive service code β€” v1/legacy state maps to scheduler. */ export function readServiceBackend(): ServiceBackend { return readServiceInstallState()?.backend === "native" ? "native" : "scheduler"; diff --git a/structure/codex-home.md b/structure/codex-home.md index 47177b946ae..0f8cdc7a3df 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -78,8 +78,9 @@ treat it as destructive, not as an upgrade or restart command. Service install-state ownership uses this same resolver. In WSL, an unset `CODEX_HOME` may resolve to the single discoverable Windows Desktop home; recording Linux `~/.codex` instead would make a later repair or uninstall look foreign even though the service and runtime were started from the -same environment. An explicit `CODEX_HOME` remains authoritative, and existing foreign ownership -records are never migrated implicitly. +same environment. Ownership checks therefore accept that exact legacy Linux-home record when WSL +now discovers a Windows home. An explicit `CODEX_HOME` remains authoritative, and other foreign +ownership records are never migrated implicitly. > Decision record: [ADR-0006](decisions/ADR-0006-codex-home.md) diff --git a/tests/codex-integration/codex-home-wsl.test.ts b/tests/codex-integration/codex-home-wsl.test.ts index 671eb4a5a6c..b0e523eb970 100644 --- a/tests/codex-integration/codex-home-wsl.test.ts +++ b/tests/codex-integration/codex-home-wsl.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { wslAutomountRoot, listWslWindowsCodexHomes } from "../../src/codex/home"; import { isWindowsInteropDir } from "../../src/codex/shim"; -import { currentServiceHomes } from "../../src/service"; +import { currentServiceHomes, serviceCodexHomeMatchesInstall } from "../../src/service"; describe("wsl.conf automount root", () => { test("defaults to /mnt when wsl.conf is absent or silent", () => { @@ -63,4 +63,27 @@ describe("wsl.conf automount root", () => { expect(homes.codexHome).toBe(windowsCodexHome); expect(homes.codexHome).not.toBe("/home/example/.codex"); }); + + test("service ownership accepts the legacy Linux fallback when WSL now discovers Windows Codex", () => { + const usersRoot = ["/mnt/c", "Users"].join("/"); + const windowsCodexHome = [usersRoot, "windows-user", ".codex"].join("/"); + const deps = { + env: { WSL_DISTRO_NAME: "Ubuntu" }, + platform: "linux", + homedir: () => "/home/example", + usersRoot, + existsSync: (path: string) => path === usersRoot + || path === `${windowsCodexHome}/config.toml`, + readdirSync: () => ["windows-user"], + statSync: (() => ({ isDirectory: () => true })) as never, + realpathSync: (path: string) => path, + }; + + expect(serviceCodexHomeMatchesInstall("/home/example/.codex", deps)).toBe(true); + expect(serviceCodexHomeMatchesInstall("/home/other/.codex", deps)).toBe(false); + expect(serviceCodexHomeMatchesInstall("/home/example/.codex", { + ...deps, + env: { ...deps.env, CODEX_HOME: windowsCodexHome }, + })).toBe(false); + }); }); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index c69955e4208..921c807b9ce 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1183,6 +1183,7 @@ "self-launch-argv.test.ts": "lib", "server-403-permission-e2e.test.ts": "server", "server-agent-task-recovery-replay.test.ts": "server", + "server-auth-localhost-bind.test.ts": "server", "server-auth.test.ts": "server", "server-background-lifecycle.test.ts": "server", "server-clickjacking-headers.test.ts": "server", @@ -1210,6 +1211,7 @@ "server-xai-oauth-401-replay.test.ts": "server", "server-xai-responses-streaming.test.ts": "server", "service-ownership-compatibility.test.ts": "service", + "service-auth-qualified-localhost.test.ts": "service", "service-ownership-handover.test.ts": "service", "service-ownership-state.test.ts": "service", "service-probe-docker.test.ts": "service", diff --git a/tests/helpers/server-auth-config.ts b/tests/helpers/server-auth-config.ts new file mode 100644 index 00000000000..a0593293389 --- /dev/null +++ b/tests/helpers/server-auth-config.ts @@ -0,0 +1,18 @@ +import type { OcxConfig } from "../../src/types"; + +export function serverAuthConfig(hostname?: string): OcxConfig { + return { + port: 10100, + hostname, + defaultProvider: "openai", + providers: { + openai: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + apiKey: "sk-secret-value", + headers: { "X-Custom": "provider-secret" }, + defaultModel: "gpt-test", + }, + }, + }; +} diff --git a/tests/server/server-auth-localhost-bind.test.ts b/tests/server/server-auth-localhost-bind.test.ts new file mode 100644 index 00000000000..123bad3b896 --- /dev/null +++ b/tests/server/server-auth-localhost-bind.test.ts @@ -0,0 +1,42 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../../src/config"; +import { startServer } from "../../src/server"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { serverAuthConfig as config } from "../helpers/server-auth-config"; + +const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; +const previousOpencodexHome = process.env.OPENCODEX_HOME; +const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-localhost-")); +let isolatedCodexHome: IsolatedCodexHome | null = null; + +beforeEach(() => { + isolatedCodexHome = installIsolatedCodexHome("ocx-server-auth-codex-"); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; +}); + +afterEach(() => { + if (previousApiToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiToken; + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); +}); + +describe("server local API auth", () => { + test("fully-qualified localhost binds to the same IPv4 target generated for clients", async () => { + saveConfig(config("localhost.")); + const server = startServer(0); + try { + expect(server.hostname).toBe("127.0.0.1"); + } finally { + await server.stop(true); + } + }); +}); diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index aec00ee0be2..a6b17ca30de 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -55,6 +55,7 @@ import { resetDebugSettingsForTests, setDebugSettings } from "../../src/lib/debu import { watchdogMs } from "../helpers/ci-watchdog"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { deferredResetSseUpstream } from "../helpers/deferred-reset-sse-upstream"; +import { serverAuthConfig as config } from "../helpers/server-auth-config"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; const previousOpencodexHome = process.env.OPENCODEX_HOME; const originalGlobalFetch = globalThis.fetch; @@ -71,23 +72,6 @@ const originalGlobalWebSocket = globalThis.WebSocket; const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-")); let isolatedCodexHome: IsolatedCodexHome | null = null; -function config(hostname?: string): OcxConfig { - return { - port: 10100, - hostname, - defaultProvider: "openai", - providers: { - openai: { - adapter: "openai-chat", - baseUrl: "https://api.example.test/v1", - apiKey: "sk-secret-value", - headers: { "X-Custom": "provider-secret" }, - defaultModel: "gpt-test", - }, - }, - }; -} - 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"; diff --git a/tests/service/service-auth-qualified-localhost.test.ts b/tests/service/service-auth-qualified-localhost.test.ts new file mode 100644 index 00000000000..324563d21eb --- /dev/null +++ b/tests/service/service-auth-qualified-localhost.test.ts @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../../src/config"; +import { assertServiceAuthEnvironment, writeServiceApiTokenFile } from "../../src/service"; +import { serviceApiTokenFilePath } from "../../src/lib/service-secrets"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +/** + * Fully-qualified `localhost.` is the same bind as `localhost`: the server canonicalizes both + * to 127.0.0.1, so the service guards must classify it as loopback too. When the private copy + * of that predicate in src/service/guards.ts did not strip the trailing dot, `localhost.` took + * the remote-bind path β€” install demanded a usable data-plane token file for a listener that + * requires no admission credential at all. Lives beside service.test.ts because that file is + * at its committed size cap in tests/fixtures/file-size-baseline.json. + */ +const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-service-auth-qualified-localhost-")); +const previousOpenCodexHome = process.env.OPENCODEX_HOME; +const previousApiAuthToken = process.env.OPENCODEX_API_AUTH_TOKEN; + +function installConfig(hostname: string): void { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + saveConfig({ + port: 10100, + hostname, + providers: { openai: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" } }, + defaultProvider: "openai", + } as OcxConfig); +} + +afterEach(() => { + if (previousOpenCodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpenCodexHome; + if (previousApiAuthToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiAuthToken; + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); +}); + +describe("service install auth preflight", () => { + test("a fully-qualified loopback bind needs no data-plane token and provisions none", () => { + for (const hostname of ["localhost", "localhost."]) { + installConfig(hostname); + + expect(() => assertServiceAuthEnvironment()).not.toThrow(); + // Loopback installs create no credential: admission is not required, and on a + // hub-connected machine this file holds the hub's issued client key instead. + expect(writeServiceApiTokenFile()).toBeNull(); + expect(existsSync(serviceApiTokenFilePath())).toBe(false); + } + }); + + test("an unusable token file on a fully-qualified loopback bind does not block install", () => { + for (const hostname of ["localhost", "localhost."]) { + installConfig(hostname); + // Empty-after-trim reads as "unsafe" β€” the state a remote bind refuses at preflight. + writeFileSync(serviceApiTokenFilePath(), "\n", "utf8"); + + expect(() => assertServiceAuthEnvironment()).not.toThrow(); + // The writer leaves the file for the operator rather than throwing or replacing it. + expect(writeServiceApiTokenFile()).toBeNull(); + expect(readFileSync(serviceApiTokenFilePath(), "utf8")).toBe("\n"); + } + }); +}); diff --git a/tests/service/service-secrets.test.ts b/tests/service/service-secrets.test.ts index ef43c39aca2..acb04a051c4 100644 --- a/tests/service/service-secrets.test.ts +++ b/tests/service/service-secrets.test.ts @@ -1,17 +1,23 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { execFileSync } from "node:child_process"; import * as nodeFs from "node:fs"; import { + chmodSync, existsSync, lstatSync, mkdtempSync, + readFileSync, + renameSync, readdirSync, rmSync, + statSync, symlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { + hardenReusedServiceApiToken, readServiceApiTokenState, readTokenBackupState, removeOrphanTokenBackup, @@ -76,6 +82,46 @@ describe("startup data-plane token resolution", () => { }); describe("service API token ownership", () => { + test("hardens the validated token descriptor rather than a replacement pathname", () => { + if (process.platform === "win32") return; + const path = serviceApiTokenFilePath(); + const openedToken = join(home, "opened-token"); + const victim = join(home, "victim"); + writeFileSync(path, "ocx_data_original\n", { mode: 0o644 }); + writeFileSync(victim, "executable\n", { mode: 0o755 }); + chmodSync(path, 0o644); + chmodSync(victim, 0o755); + + const state = hardenReusedServiceApiToken(token => { + expect(token).toBe("ocx_data_original"); + renameSync(path, openedToken); + symlinkSync(victim, path); + }); + + expect(state).toMatchObject({ kind: "present", token: "ocx_data_original" }); + expect(statSync(openedToken).mode & 0o777).toBe(0o600); + expect(statSync(victim).mode & 0o777).toBe(0o755); + // Identity at return: the swap during validation is replaced, so the path + // the caller reports names a hardened file holding the validated token β€” + // never the substituted symlink. + expect(lstatSync(path).isSymbolicLink()).toBe(false); + expect(lstatSync(path).mode & 0o777).toBe(0o600); + expect(readFileSync(path, "utf8").trim()).toBe("ocx_data_original"); + }); + + test("a non-regular token path is unsafe rather than blocking the open", () => { + if (process.platform === "win32") return; + const path = serviceApiTokenFilePath(); + execFileSync("mkfifo", [path]); + + const state = hardenReusedServiceApiToken(() => { + throw new Error("validation must not run for a non-regular token path"); + }); + + expect(state.kind).toBe("unsafe"); + if (state.kind === "unsafe") expect(state.reason).toContain("regular file"); + }); + test("writes only the exact owner path through an atomic owner-only replacement", () => { const token = "ocx_data_0123456789abcdef0123456789abcdef01234567"; const persisted = writeServiceApiTokenFile(token); diff --git a/tests/windows/windows-deploy-close-regressions.test.ts b/tests/windows/windows-deploy-close-regressions.test.ts index b65925b29be..91e9088d8a5 100644 --- a/tests/windows/windows-deploy-close-regressions.test.ts +++ b/tests/windows/windows-deploy-close-regressions.test.ts @@ -81,7 +81,7 @@ describe("server bind canonicalizes explicit localhost but preserves wildcards ( const src = read("src/server/index.ts"); test("literal localhost binds to 127.0.0.1; 0.0.0.0/:: exposure is untouched", () => { expect(src).toContain("const configuredHost = config.hostname?.trim();"); - expect(src).toContain('!configuredHost || /^localhost$/i.test(configuredHost) ? "127.0.0.1"'); + expect(src).toContain('!configuredHost || /^localhost\\.?$/i.test(configuredHost) ? "127.0.0.1"'); // Must not blanket-rewrite the PUBLIC bind host β€” that would break intentional 0.0.0.0 // exposure, which is the regression this guards. // From 178dab4aaba41074cb29f6ad4202e52f77dafb1f Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Tue, 22 Sep 2026 08:13:50 +0900 Subject: [PATCH 02/10] docs(service): record the unsafe-file contract on reused-token hardening (cherry picked from commit 07f5285ba9e508f55b0adc210d58ecf6b9361122) --- src/lib/service-secrets.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib/service-secrets.ts b/src/lib/service-secrets.ts index 926bd21cc73..97ffe111dc1 100644 --- a/src/lib/service-secrets.ts +++ b/src/lib/service-secrets.ts @@ -63,6 +63,12 @@ export function readServiceApiTokenState(): ServiceApiTokenState { * opened descriptor is also fchmod'd first, so a token-bearing inode a race * moved aside is still tightened wherever its entry ended up. * + * + * Return contract vs `readServiceApiTokenState`: an empty or malformed token file + * reports `unsafe` here and is never written β€” the path-based pre-check may still + * pass the install on loopback while this writer deliberately leaves the file + * untouched. Only `absent` permits a fresh write; anything unreadable stays as-is. + * * Callers must run this under `withConfigMutationLockSync`: client-key rotation * replaces the token under that lock, and a republish outside it could rename a * stale token back over a committed rotation. From 8a0361b8a495eaf769f8b91b6c6e6c247c9f7b5e Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 20 Sep 2026 17:51:11 +0900 Subject: [PATCH 03/10] Take a fresh task listing for the second startup ownership decision (cherry picked from commit 9705b1d03d224495b56fc01cb72c8d5362698125) --- src/server/index.ts | 9 ++++----- .../codex-service-manager-probe-hardening.test.ts | 15 +++++++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 7dba95c8f8a..1b846660ec8 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -255,10 +255,10 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe const resolveServiceHomes = deps.resolveServiceHomes ?? currentServiceHomes; let startupOwnershipHomes: ReturnType | null = null; let startupOwnershipStatePaths: readonly string[] | null = null; - // #2923: both synchronous startup ownership decisions keep their fresh, - // race-sensitive targeted task query. Only the expensive fallback listing is - // shared, and only while that targeted result stays byte-for-byte unchanged. - // Runtime ownership retries below intentionally omit this startup-local memo. + // #2923: retain a successful fallback listing only within the first startup + // ownership decision. A targeted query's bytes are not a Task Scheduler state + // generation, so the later race-sensitive decision must take a fresh listing. + // Runtime ownership retries below intentionally omit this startup-local memo too. const startupWindowsTaskListingCache = createWindowsTaskListingCache(); try { const homes = resolveServiceHomes(); @@ -555,7 +555,6 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe deps, startupOwnershipHomes, startupOwnershipStatePaths, - startupWindowsTaskListingCache, ); const preparedNativeMainLifecycle = nativeOwnership.ownership !== "foreign" && startupOwnershipHomes !== null diff --git a/tests/codex-integration/codex-service-manager-probe-hardening.test.ts b/tests/codex-integration/codex-service-manager-probe-hardening.test.ts index 2f9e3b6bfab..99d259c5485 100644 --- a/tests/codex-integration/codex-service-manager-probe-hardening.test.ts +++ b/tests/codex-integration/codex-service-manager-probe-hardening.test.ts @@ -213,7 +213,7 @@ describe("Windows ownership probe hardening regressions", () => { expect(result.kind).toBe("unknown"); }); - test("one startup keeps two targeted queries but shares one unchanged full listing (#2923)", async () => { + test("a startup refreshes the full listing when a task appears after the second targeted snapshot", async () => { const codexHome = join(home, "codex"); mkdirSync(codexHome, { recursive: true }); process.env.CODEX_HOME = codexHome; @@ -228,15 +228,22 @@ describe("Windows ownership probe hardening regressions", () => { let targetedQueries = 0; let fullListings = 0; + let taskRegistered = false; const runRaw: RawProbeRunner = (file, args) => { if (!file.toLowerCase().endsWith("schtasks.exe")) return raw(1, "", "unexpected executable"); if (args.includes("/xml")) { targetedQueries += 1; + // Model a localized targeted query that took its absent snapshot before + // a concurrent installer committed the task, then returned unchanged + // opaque bytes. Only the following fresh listing can observe the task. + if (targetedQueries === 2) taskRegistered = true; return { status: 1, stdout: Buffer.alloc(0), stderr: GBK_TASK_NOT_FOUND, timedOut: false, spawnFailed: false }; } if (args.includes("/fo")) { fullListings += 1; - return raw(0, '"\\SomeOtherTask","N/A","Ready"\r\n'); + return raw(0, taskRegistered + ? '"\\opencodex-proxy","N/A","Ready"\r\n' + : '"\\SomeOtherTask","N/A","Ready"\r\n'); } return raw(1, "", "unexpected query"); }; @@ -262,9 +269,9 @@ describe("Windows ownership probe hardening regressions", () => { }, }); try { - expect(ownerships.slice(0, 2)).toEqual(["owned", "owned"]); + expect(ownerships.slice(0, 2)).toEqual(["owned", "unknown"]); expect(targetedQueries).toBe(2); - expect(fullListings).toBe(1); + expect(fullListings).toBe(2); } finally { await server.stop(true); } From 7b619b0dd12ee36659c4579eb9bc8b2c5b1bfc36 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 21 Sep 2026 01:24:17 +0900 Subject: [PATCH 04/10] fix(server): retain workflow slots for streaming turns (cherry picked from commit 8ab2881b0acaae1de5aecf0b2dc7fcb76c5aec2b) --- src/server/index.ts | 4 +--- src/server/lifecycle.ts | 8 +++++++ structure/transports/responses.md | 2 ++ .../active-registry-admission.test.ts | 24 ++++++++++++++++++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 1b846660ec8..26a68f1a034 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -516,16 +516,14 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe // still unreadable to a browser dashboard -- which made exposing it pointless. return withCors(workflowDecisionRefusalResponse(workflow, undefined, refusalLog), req, policy); } - const releaseWorkflow = (): void => { if (workflow?.admitted) workflow.lease.release(); }; + if (workflow?.admitted) lease.attach(workflow.lease); let response: Response; try { response = await runAdmittedBodyWork(req, policy, config.maxInboundBodyBytes, () => work(lease), refusalLog); } catch (error) { - releaseWorkflow(); lease.release(); throw error; } - releaseWorkflow(); if (!lease.isTransferred()) { lease.release(); } diff --git a/src/server/lifecycle.ts b/src/server/lifecycle.ts index fc757cc0670..e36bc460682 100644 --- a/src/server/lifecycle.ts +++ b/src/server/lifecycle.ts @@ -35,6 +35,7 @@ export const MAX_ACTIVE_SESSION_LANES = 64; export const SESSION_LANE_ID_BYTES = 32; const turnGate = createAdmissionGate("active_turns", MAX_ACTIVE_TURNS); export interface ActiveTurnLease extends AdmissionLease { + attach(lease: AdmissionLease): void; bindAbortController(ac: AbortController): void; beginCodexAccountSelection(): CodexAccountSelectionAdmission; isTransferred(): boolean; @@ -190,10 +191,15 @@ export function tryAdmitTurn(sessionLaneId?: string): ActiveTurnLease | null { } } const controllers = new Set(); + const attachedLeases = new Set(); let active = true; let transferred = false; let nativeMainClaimed = false; const lease: ActiveTurnLease = { + attach(attachedLease) { + if (!active) attachedLease.release(); + else attachedLeases.add(attachedLease); + }, bindAbortController(ac) { knownTurnControllers.add(ac); if (!active) { @@ -238,6 +244,8 @@ export function tryAdmitTurn(sessionLaneId?: string): ActiveTurnLease | null { if (activeTurns.get(controller) === lease) activeTurns.delete(controller); } controllers.clear(); + for (const attachedLease of attachedLeases) attachedLease.release(); + attachedLeases.clear(); nativeMainTurns.delete(lease); if (opaqueSessionLaneId) { const currentRefCount = activeSessionLaneRefCounts.get(opaqueSessionLaneId); diff --git a/structure/transports/responses.md b/structure/transports/responses.md index a82e8f8cb08..17205cc8e22 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -69,6 +69,8 @@ repository state. Consequently, the active-turn and session-lane gates are concu limits, the translator budget is a live retained-byte limit, the response-state caps are cache retention limits, and the stall watchdog is a silence limit. None is a cumulative continuation or semantic no-progress budget. +Active-turn admission owns workflow admission, so both remain held until a streaming body finishes +or is cancelled. > Decision record: [ADR-0031](../decisions/ADR-0031-responses-http-sse.md) diff --git a/tests/codex-integration/active-registry-admission.test.ts b/tests/codex-integration/active-registry-admission.test.ts index 88bbeb5790c..16b375b9872 100644 --- a/tests/codex-integration/active-registry-admission.test.ts +++ b/tests/codex-integration/active-registry-admission.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { MAX_ACTIVE_TURNS, abortAndReleaseAllTurns, activeRegistryMetrics, trackStreamLifetime, tryAdmitTurn, unregisterTurn } from "../../src/server/lifecycle"; +import { workflowBudgetSnapshot } from "../../src/lib/workflow-budget"; import { MAX_TRACKED_CODEX_WEBSOCKETS, getTrackedCodexWebSocketCountForAccount, @@ -125,14 +126,20 @@ describe("active registry admission", () => { try { const response = await fetch(new URL("/v1/responses", server.url), { method: "POST", - headers: { "content-type": "application/json" }, + headers: { + "content-type": "application/json", + "x-codex-parent-thread-id": "stream-root", + "thread-id": "stream-child", + }, body: JSON.stringify({ model: "fixture/model", input: "hello", stream: true }), }); expect(response.status).toBe(200); expect(activeRegistryMetrics().activeTurns.active).toBe(before + 1); + expect(workflowBudgetSnapshot("stream-root")?.active).toBe(1); settle(); expect(await response.text()).toBe("chunk"); expect(activeRegistryMetrics().activeTurns.active).toBe(before); + expect(workflowBudgetSnapshot("stream-root")?.active).toBe(0); } finally { settle?.(); await server.stop(true); @@ -177,6 +184,21 @@ describe("active registry admission", () => { expect(activeRegistryMetrics().activeTurns.active).toBe(before); }); + test("a transferred turn retains attached admission until its stream settles", async () => { + const lease = tryAdmitTurn()!; + let attachedReleases = 0; + lease.attach({ release() { attachedReleases += 1; } }); + const source = new ReadableStream({ pull() {} }); + const tracked = trackStreamLifetime(source, new AbortController(), undefined, lease); + + expect(lease.isTransferred()).toBe(true); + expect(attachedReleases).toBe(0); + await tracked.cancel(); + expect(attachedReleases).toBe(1); + lease.release(); + expect(attachedReleases).toBe(1); + }); + test("the 256-turn gate bounds concurrency, not sequential continuation count", () => { const before = activeRegistryMetrics().activeTurns.active; for (let index = 0; index <= MAX_ACTIVE_TURNS; index += 1) { From 5091fa4a62e26019bd8b72626b43d26dd37f8bb6 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 02:28:28 +0900 Subject: [PATCH 05/10] test(server): own CORS fixture lifetime and isolate host probes The management CORS test can exhaust its 5s body limit during Windows startup, before its local finally runs. Teardown then removes the home while the real proxy still owns its spend SQLite transaction. Give the CORS fixture an abort signal, tracked body and memoized listener stop. Settle those before restoring seams and homes, retain the existing producer/ACL drains, and await failed-start rollback when setup throws. Consume both HTTP response bodies and prove cancellation releases the actual spend owner. Keep real token ACL creation, HTTP admission and CORS decoration. Isolate native Codex sync, host service ownership and settings-only runtime/service diagnostic projections from this fixture, whose assertions concern management CORS headers. Restore its scoped runtime spy after shutdown. No timeout, skip, ACL policy, production source or dependency change. --- tests/server/server-auth.test.ts | 162 ++++++++++++++++++++++++++----- 1 file changed, 136 insertions(+), 26 deletions(-) diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index e7bf1bd9d6b..2133304bf69 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -1,4 +1,4 @@ -import { waitForNativeMainStartupGate } from "../../src/codex/native-profile-startup"; +import { flushNativeMainStartupReleases, waitForNativeMainStartupGate } from "../../src/codex/native-profile-startup"; import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { createHash } from "node:crypto"; import { logsFromApiBody } from "../helpers/logs-api"; @@ -22,6 +22,14 @@ import { recordCodexUpstreamOutcome, } from "../../src/codex/routing"; import { loadConfig, saveConfig } from "../../src/config"; +import { flushConfigDirHardeningForTests } from "../../src/config/paths"; +import { flushWindowsSecretAclReapsBeforeRemoval } from "../../src/lib/windows-secret-acl"; +import { closeRequestHistoryIndex } from "../../src/routing/history/indexer"; +import { clearHealthHistoryCacheForTests } from "../../src/routing/health"; +import { stopServerListener } from "../../src/server/lifecycle"; +import { spendLedgerOwnerSnapshot } from "../../src/lib/spend-ledger-owner"; +import * as codexRuntime from "../../src/codex/runtime"; +import { deriveStartupHealth } from "../../src/codex/autostart-health"; import { clearUpstreamHostHealth, getUpstreamHostHealth, recordUpstreamHostFailure, upstreamHostHealthKey } from "../../src/codex/upstream-host-health"; import { deriveProviderPresets } from "../../src/providers/derive"; import { MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; @@ -36,6 +44,7 @@ import { rootFallbackPayload, safeConfigDTO, startServer, + waitForFailedStartRollback, } from "../../src/server"; import { clearRequestLogsForTests, getRequestLogEntries } from "../../src/server/request-log"; import { setRelayPlatformForTests } from "../../src/server/responses/passthrough-delivery"; @@ -72,6 +81,38 @@ const originalGlobalWebSocket = globalThis.WebSocket; // isolation convention already used by tests/helpers/isolated-codex-home.ts. const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-")); let isolatedCodexHome: IsolatedCodexHome | null = null; +let managementCorsFixture: ReturnType | null = null; +let restoreCorsRuntime: (() => void) | null = null; + +/** A runner timeout does not cancel its async body or execute its local finally first. */ +function ownManagementCorsServer(server: ReturnType) { + const abort = new AbortController(); + let body: Promise | undefined; + let closing: Promise | undefined; + return { + server, + signal: abort.signal, + run(work: () => Promise): Promise { + abort.signal.throwIfAborted(); + body = work().catch(error => { + if (closing && abort.signal.aborted && error instanceof Error && error.name === "AbortError") return; + throw error; + }); + return body; + }, + close(): Promise { + return closing ??= (async () => { + abort.abort(); + // Start the actual listener/lifecycle stop while the canceled request settles. + // The existing stop helper memoizes this promise for repeated teardown callers. + const stopped = Promise.allSettled([stopServerListener(server)]); + await Promise.allSettled(body ? [body] : []); + const [result] = await stopped; + if (result.status === "rejected") throw result.reason; + })(); + }, + }; +} function config(hostname?: string): OcxConfig { return { @@ -170,15 +211,20 @@ beforeEach(() => { isolatedCodexHome = installIsolatedCodexHome("ocx-server-auth-codex-"); }); -afterEach(() => { +afterEach(async () => { + try { + if (managementCorsFixture) { + await managementCorsFixture.close(); + managementCorsFixture = null; + } + } finally { + restoreCorsRuntime?.(); + restoreCorsRuntime = null; + } globalThis.fetch = originalGlobalFetch; globalThis.WebSocket = originalGlobalWebSocket; if (previousApiToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiToken; - if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = previousOpencodexHome; - isolatedCodexHome?.restore(); - isolatedCodexHome = null; clearCodexUpstreamHealth(); clearThreadAccountMap(); clearAccountNeedsReauth("pool-a"); @@ -187,6 +233,18 @@ afterEach(() => { resetCodexModelEntitlementCacheForTests(); resetDebugSettingsForTests(); resetDebugLogBufferForTests(); + // These producers and the process-wide SQLite index can outlive a stopped listener. + // Drain/close them under the fixture home before restoring paths or removing its files. + await flushNativeMainStartupReleases(); + await flushConfigDirHardeningForTests(); + clearHealthHistoryCacheForTests(); + closeRequestHistoryIndex(); + await flushWindowsSecretAclReapsBeforeRemoval(TEST_DIR); + if (isolatedCodexHome) await flushWindowsSecretAclReapsBeforeRemoval(isolatedCodexHome.path); + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); }); @@ -1330,30 +1388,82 @@ describe("server local API auth", () => { } }); - test("management CORS echoes validated loopback Origin and covers delegated codex-auth responses", async () => { - if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); - mkdirSync(TEST_DIR, { recursive: true }); - process.env.OPENCODEX_HOME = TEST_DIR; - saveConfig(config("127.0.0.1")); + describe("management CORS fixture", () => { + beforeEach(async () => { + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ ...config("127.0.0.1"), clientIntegrations: { codex: false } }); + // Real config/token ACL preparation belongs to fixture readiness, not the CORS + // response deadline. Keep the production startup and the ordinary 5s test limit. + // Neither management endpoint exercises native Codex synchronization or the + // developer's installed service. Keep those external owners outside this fixture. + const runtime = spyOn(codexRuntime, "resolveCodexRuntime").mockReturnValue({ + runtime: { command: "codex-fixture", version: null, source: "fallback" }, failures: [], + }); + restoreCorsRuntime = () => runtime.mockRestore(); + try { + managementCorsFixture = ownManagementCorsServer(startServer(0, { + inspectNativeCodexOwnership: ownedServiceHomeInspection("management CORS sandbox"), + managementApi: { + // /api/settings projects runtime/service diagnostics, but their host probes + // are not CORS behavior. Keep admission, routing and response decoration real. + getCachedStartupHealth: async () => deriveStartupHealth({ + routingKind: "native", autostartEnabled: false, serviceInstalled: false, + serviceViable: false, serviceEnabled: false, serviceRunning: false, + serviceStale: false, serviceConflict: false, serviceSupported: true, + shimInstalled: false, shimHealthy: false, platform: process.platform, + }), + }, + })); + } catch (error) { + await waitForFailedStartRollback(error); + throw error; + } + }); - const server = startServer(0); - const origin = `http://127.0.0.1:${server.port}`; - try { - const settings = await fetch(new URL("/api/settings", server.url), { - headers: managementHeaders({ origin }), + test("management CORS echoes validated loopback Origin and covers delegated codex-auth responses", async () => { + const fixture = managementCorsFixture!; + const origin = `http://127.0.0.1:${fixture.server.port}`; + await fixture.run(async () => { + const settings = await fetch(new URL("/api/settings", fixture.server.url), { + headers: managementHeaders({ origin }), signal: fixture.signal, + }); + expect(settings.status).toBe(200); + expect(settings.headers.get("access-control-allow-origin")).toBe(origin); + expect(settings.headers.get("vary")).toContain("Origin"); + await settings.text(); + + const active = await fetch(new URL("/api/codex-auth/active", fixture.server.url), { + headers: managementHeaders({ origin }), signal: fixture.signal, + }); + expect(active.status).toBe(200); + expect(active.headers.get("access-control-allow-origin")).toBe(origin); + await active.text(); }); - expect(settings.status).toBe(200); - expect(settings.headers.get("access-control-allow-origin")).toBe(origin); - expect(settings.headers.get("vary")).toContain("Origin"); + }); - const active = await fetch(new URL("/api/codex-auth/active", server.url), { - headers: managementHeaders({ origin }), + test("management CORS fixture cancellation settles its body and releases the real spend lease", async () => { + const fixture = managementCorsFixture!; + let aborted = false; + let settled = false; + const entered = Promise.withResolvers(); + const work = fixture.run(async () => { + const response = abortableSseUpstream("data: pending\n\n", fixture.signal, () => { aborted = true; }); + const reading = response.text(); + entered.resolve(); + try { await reading; } finally { settled = true; } }); - expect(active.status).toBe(200); - expect(active.headers.get("access-control-allow-origin")).toBe(origin); - } finally { - await server.stop(true); - } + await entered.promise; + expect(spendLedgerOwnerSnapshot().ownership).toBe("held"); + const stopped = fixture.close(); + expect(fixture.close()).toBe(stopped); + await stopped; + await work; + expect(aborted).toBe(true); + expect(settled).toBe(true); + expect(spendLedgerOwnerSnapshot().ownership).toBe("unheld"); + }); }); test("non-loopback management API allows same-origin GUI requests with API token", async () => { From 3832fa52012f4ae108fdcdddc1c2af201911c1b0 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 02:52:52 +0900 Subject: [PATCH 06/10] refactor(test): extract server auth fixture ownership Move CORS setup, request/server lifetime, scoped runtime restoration, authenticated headers and fixture-state drains into a sibling test helper. Keep server-auth.test.ts within its existing 4589-line ratchet cap without changing the baseline or reducing assertions. The CORS and real spend-lease cancellation bodies are unchanged. Preserve the real token ACL path, production authentication/CORS behavior, setup rollback, memoized shutdown, body settlement and existing test deadlines. --- tests/helpers/server-auth-fixture.ts | 126 +++++++++++++++++++++++++++ tests/server/server-auth.test.ts | 115 +++--------------------- 2 files changed, 136 insertions(+), 105 deletions(-) create mode 100644 tests/helpers/server-auth-fixture.ts diff --git a/tests/helpers/server-auth-fixture.ts b/tests/helpers/server-auth-fixture.ts new file mode 100644 index 00000000000..d3a2681630c --- /dev/null +++ b/tests/helpers/server-auth-fixture.ts @@ -0,0 +1,126 @@ +import { spyOn } from "bun:test"; +import { existsSync, mkdirSync } from "node:fs"; +import { flushNativeMainStartupReleases } from "../../src/codex/native-profile-startup"; +import * as codexRuntime from "../../src/codex/runtime"; +import { deriveStartupHealth } from "../../src/codex/autostart-health"; +import { clearAccountNeedsReauth, clearAccountQuota } from "../../src/codex/auth-api"; +import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../../src/codex/routing"; +import { resetCodexModelEntitlementCacheForTests } from "../../src/codex/model-entitlements"; +import { saveConfig } from "../../src/config"; +import { flushConfigDirHardeningForTests } from "../../src/config/paths"; +import { configuredAdminToken } from "../../src/lib/admin-secrets"; +import { resetDebugLogBufferForTests } from "../../src/lib/debug-log-buffer"; +import { resetDebugSettingsForTests } from "../../src/lib/debug-settings"; +import { flushWindowsSecretAclReapsBeforeRemoval } from "../../src/lib/windows-secret-acl"; +import { clearHealthHistoryCacheForTests } from "../../src/routing/health"; +import { closeRequestHistoryIndex } from "../../src/routing/history/indexer"; +import { startServer, waitForFailedStartRollback } from "../../src/server"; +import { stopServerListener } from "../../src/server/lifecycle"; +import type { OcxConfig } from "../../src/types"; +import { ownedServiceHomeInspection } from "./owned-service-home-inspection"; +import { removeTreeWithRetry } from "./remove-tree"; + +export function managementHeaders(initial?: HeadersInit): Headers { + const token = configuredAdminToken(); + if (!token) throw new Error("management token was not initialized"); + const headers = new Headers(initial); + headers.set("x-opencodex-api-key", token); + return headers; +} + +/** A runner timeout does not cancel its async body or execute its local finally first. */ +function ownManagementCorsServer(server: ReturnType, restoreRuntime: () => void) { + const abort = new AbortController(); + let body: Promise | undefined; + let closing: Promise | undefined; + return { + server, + signal: abort.signal, + run(work: () => Promise): Promise { + abort.signal.throwIfAborted(); + body = work().catch(error => { + if (closing && abort.signal.aborted && error instanceof Error && error.name === "AbortError") return; + throw error; + }); + return body; + }, + close(): Promise { + return closing ??= (async () => { + try { + abort.abort(); + // Start the actual listener/lifecycle stop while the canceled request settles. + // The existing stop helper memoizes this promise for repeated teardown callers. + const stopped = Promise.allSettled([stopServerListener(server)]); + await Promise.allSettled(body ? [body] : []); + const [result] = await stopped; + if (result.status === "rejected") throw result.reason; + } finally { + restoreRuntime(); + } + })(); + }, + }; +} + +export type ManagementCorsFixture = ReturnType; + +/** Prepare real auth/ACL state while isolating diagnostics unrelated to CORS behavior. */ +export async function startManagementCorsFixture( + configDir: string, + fixtureConfig: OcxConfig, +): Promise { + if (existsSync(configDir)) removeTreeWithRetry(configDir); + mkdirSync(configDir, { recursive: true }); + process.env.OPENCODEX_HOME = configDir; + saveConfig({ ...fixtureConfig, clientIntegrations: { codex: false } }); + // Real config/token ACL preparation belongs to fixture readiness, not the CORS + // response deadline. Keep the production startup and the ordinary 5s test limit. + // Neither management endpoint exercises native Codex synchronization or the + // developer's installed service. Keep those external owners outside this fixture. + const runtime = spyOn(codexRuntime, "resolveCodexRuntime").mockReturnValue({ + runtime: { command: "codex-fixture", version: null, source: "fallback" }, failures: [], + }); + try { + const server = startServer(0, { + inspectNativeCodexOwnership: ownedServiceHomeInspection("management CORS sandbox"), + managementApi: { + // /api/settings projects runtime/service diagnostics, but their host probes + // are not CORS behavior. Keep admission, routing and response decoration real. + getCachedStartupHealth: async () => deriveStartupHealth({ + routingKind: "native", autostartEnabled: false, serviceInstalled: false, + serviceViable: false, serviceEnabled: false, serviceRunning: false, + serviceStale: false, serviceConflict: false, serviceSupported: true, + shimInstalled: false, shimHealthy: false, platform: process.platform, + }), + }, + }); + return ownManagementCorsServer(server, () => runtime.mockRestore()); + } catch (error) { + try { + await waitForFailedStartRollback(error); + } finally { + runtime.mockRestore(); + } + throw error; + } +} + +/** Reset fixture evidence and settle producers while the caller still owns both homes. */ +export async function settleServerAuthFixture(configDir: string, codexHome?: string): Promise { + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + clearAccountNeedsReauth("pool-a"); + clearAccountNeedsReauth("pool-b"); + clearAccountQuota(); + resetCodexModelEntitlementCacheForTests(); + resetDebugSettingsForTests(); + resetDebugLogBufferForTests(); + // These producers and the process-wide SQLite index can outlive a stopped listener. + // Drain/close them before the caller restores paths or removes the files. + await flushNativeMainStartupReleases(); + await flushConfigDirHardeningForTests(); + clearHealthHistoryCacheForTests(); + closeRequestHistoryIndex(); + await flushWindowsSecretAclReapsBeforeRemoval(configDir); + if (codexHome) await flushWindowsSecretAclReapsBeforeRemoval(codexHome); +} diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index 2133304bf69..7ba62cbb017 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -1,4 +1,4 @@ -import { flushNativeMainStartupReleases, waitForNativeMainStartupGate } from "../../src/codex/native-profile-startup"; +import { waitForNativeMainStartupGate } from "../../src/codex/native-profile-startup"; import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { createHash } from "node:crypto"; import { logsFromApiBody } from "../helpers/logs-api"; @@ -22,14 +22,8 @@ import { recordCodexUpstreamOutcome, } from "../../src/codex/routing"; import { loadConfig, saveConfig } from "../../src/config"; -import { flushConfigDirHardeningForTests } from "../../src/config/paths"; -import { flushWindowsSecretAclReapsBeforeRemoval } from "../../src/lib/windows-secret-acl"; -import { closeRequestHistoryIndex } from "../../src/routing/history/indexer"; -import { clearHealthHistoryCacheForTests } from "../../src/routing/health"; -import { stopServerListener } from "../../src/server/lifecycle"; import { spendLedgerOwnerSnapshot } from "../../src/lib/spend-ledger-owner"; -import * as codexRuntime from "../../src/codex/runtime"; -import { deriveStartupHealth } from "../../src/codex/autostart-health"; +import { settleServerAuthFixture, managementHeaders, startManagementCorsFixture, type ManagementCorsFixture } from "../helpers/server-auth-fixture"; import { clearUpstreamHostHealth, getUpstreamHostHealth, recordUpstreamHostFailure, upstreamHostHealthKey } from "../../src/codex/upstream-host-health"; import { deriveProviderPresets } from "../../src/providers/derive"; import { MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; @@ -44,7 +38,6 @@ import { rootFallbackPayload, safeConfigDTO, startServer, - waitForFailedStartRollback, } from "../../src/server"; import { clearRequestLogsForTests, getRequestLogEntries } from "../../src/server/request-log"; import { setRelayPlatformForTests } from "../../src/server/responses/passthrough-delivery"; @@ -61,8 +54,8 @@ import { SYSTEM_RESTART_CAPABILITY_VERSION } from "../../src/lib/system-restart- import { LOCAL_PROVIDER_RELOAD_CAPABILITY_VERSION } from "../../src/lib/local-provider-reload-contract"; import { GUI_PAIR_CAPABILITY_VERSION } from "../../src/lib/gui-pair-capability"; 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 { getDebugLogEntries } from "../../src/lib/debug-log-buffer"; +import { setDebugSettings } from "../../src/lib/debug-settings"; import { watchdogMs } from "../helpers/ci-watchdog"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { deferredResetSseUpstream } from "../helpers/deferred-reset-sse-upstream"; @@ -81,38 +74,7 @@ const originalGlobalWebSocket = globalThis.WebSocket; // isolation convention already used by tests/helpers/isolated-codex-home.ts. const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-")); let isolatedCodexHome: IsolatedCodexHome | null = null; -let managementCorsFixture: ReturnType | null = null; -let restoreCorsRuntime: (() => void) | null = null; - -/** A runner timeout does not cancel its async body or execute its local finally first. */ -function ownManagementCorsServer(server: ReturnType) { - const abort = new AbortController(); - let body: Promise | undefined; - let closing: Promise | undefined; - return { - server, - signal: abort.signal, - run(work: () => Promise): Promise { - abort.signal.throwIfAborted(); - body = work().catch(error => { - if (closing && abort.signal.aborted && error instanceof Error && error.name === "AbortError") return; - throw error; - }); - return body; - }, - close(): Promise { - return closing ??= (async () => { - abort.abort(); - // Start the actual listener/lifecycle stop while the canceled request settles. - // The existing stop helper memoizes this promise for repeated teardown callers. - const stopped = Promise.allSettled([stopServerListener(server)]); - await Promise.allSettled(body ? [body] : []); - const [result] = await stopped; - if (result.status === "rejected") throw result.reason; - })(); - }, - }; -} +let managementCorsFixture: ManagementCorsFixture | null = null; function config(hostname?: string): OcxConfig { return { @@ -147,14 +109,6 @@ function writeRemoteCatalog(): void { 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"); - const headers = new Headers(initial); - headers.set("x-opencodex-api-key", token); - return headers; -} - const canonicalDirect = { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", @@ -212,35 +166,15 @@ beforeEach(() => { }); afterEach(async () => { - try { - if (managementCorsFixture) { - await managementCorsFixture.close(); - managementCorsFixture = null; - } - } finally { - restoreCorsRuntime?.(); - restoreCorsRuntime = null; + if (managementCorsFixture) { + await managementCorsFixture.close(); + managementCorsFixture = null; } globalThis.fetch = originalGlobalFetch; globalThis.WebSocket = originalGlobalWebSocket; if (previousApiToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiToken; - clearCodexUpstreamHealth(); - clearThreadAccountMap(); - clearAccountNeedsReauth("pool-a"); - clearAccountNeedsReauth("pool-b"); - clearAccountQuota(); - resetCodexModelEntitlementCacheForTests(); - resetDebugSettingsForTests(); - resetDebugLogBufferForTests(); - // These producers and the process-wide SQLite index can outlive a stopped listener. - // Drain/close them under the fixture home before restoring paths or removing its files. - await flushNativeMainStartupReleases(); - await flushConfigDirHardeningForTests(); - clearHealthHistoryCacheForTests(); - closeRequestHistoryIndex(); - await flushWindowsSecretAclReapsBeforeRemoval(TEST_DIR); - if (isolatedCodexHome) await flushWindowsSecretAclReapsBeforeRemoval(isolatedCodexHome.path); + await settleServerAuthFixture(TEST_DIR, isolatedCodexHome?.path); if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = previousOpencodexHome; isolatedCodexHome?.restore(); @@ -1390,36 +1324,7 @@ describe("server local API auth", () => { describe("management CORS fixture", () => { beforeEach(async () => { - if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); - mkdirSync(TEST_DIR, { recursive: true }); - process.env.OPENCODEX_HOME = TEST_DIR; - saveConfig({ ...config("127.0.0.1"), clientIntegrations: { codex: false } }); - // Real config/token ACL preparation belongs to fixture readiness, not the CORS - // response deadline. Keep the production startup and the ordinary 5s test limit. - // Neither management endpoint exercises native Codex synchronization or the - // developer's installed service. Keep those external owners outside this fixture. - const runtime = spyOn(codexRuntime, "resolveCodexRuntime").mockReturnValue({ - runtime: { command: "codex-fixture", version: null, source: "fallback" }, failures: [], - }); - restoreCorsRuntime = () => runtime.mockRestore(); - try { - managementCorsFixture = ownManagementCorsServer(startServer(0, { - inspectNativeCodexOwnership: ownedServiceHomeInspection("management CORS sandbox"), - managementApi: { - // /api/settings projects runtime/service diagnostics, but their host probes - // are not CORS behavior. Keep admission, routing and response decoration real. - getCachedStartupHealth: async () => deriveStartupHealth({ - routingKind: "native", autostartEnabled: false, serviceInstalled: false, - serviceViable: false, serviceEnabled: false, serviceRunning: false, - serviceStale: false, serviceConflict: false, serviceSupported: true, - shimInstalled: false, shimHealthy: false, platform: process.platform, - }), - }, - })); - } catch (error) { - await waitForFailedStartRollback(error); - throw error; - } + managementCorsFixture = await startManagementCorsFixture(TEST_DIR, config("127.0.0.1")); }); test("management CORS echoes validated loopback Origin and covers delegated codex-auth responses", async () => { From da9bf75042bf306ff1e2d04352f3e4593642b9de Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 03:26:56 +0900 Subject: [PATCH 07/10] test: drain localhost bind fixture before home removal --- .../server/server-auth-localhost-bind.test.ts | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/tests/server/server-auth-localhost-bind.test.ts b/tests/server/server-auth-localhost-bind.test.ts index 123bad3b896..8bd910a333c 100644 --- a/tests/server/server-auth-localhost-bind.test.ts +++ b/tests/server/server-auth-localhost-bind.test.ts @@ -3,23 +3,41 @@ import { existsSync, mkdirSync, mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { saveConfig } from "../../src/config"; -import { startServer } from "../../src/server"; +import { startServer, waitForFailedStartRollback } from "../../src/server"; +import { stopServerListener } from "../../src/server/lifecycle"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { serverAuthConfig as config } from "../helpers/server-auth-config"; +import { settleServerAuthFixture } from "../helpers/server-auth-fixture"; +import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; const previousOpencodexHome = process.env.OPENCODEX_HOME; const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-localhost-")); let isolatedCodexHome: IsolatedCodexHome | null = null; +let server: ReturnType | null = null; -beforeEach(() => { +beforeEach(async () => { isolatedCodexHome = installIsolatedCodexHome("ocx-server-auth-codex-"); mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; + // Binding a hostname does not exercise native client synchronization. Keep real + // listener/auth/ACL setup while excluding the host's installed service identity. + saveConfig({ ...config("localhost."), clientIntegrations: { codex: false } }); + try { + server = startServer(0, { + inspectNativeCodexOwnership: ownedServiceHomeInspection("localhost bind sandbox"), + }); + } catch (error) { + await waitForFailedStartRollback(error); + throw error; + } }); -afterEach(() => { +afterEach(async () => { + if (server) await stopServerListener(server); + server = null; + await settleServerAuthFixture(TEST_DIR, isolatedCodexHome?.path); if (previousApiToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiToken; if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; @@ -31,12 +49,6 @@ afterEach(() => { describe("server local API auth", () => { test("fully-qualified localhost binds to the same IPv4 target generated for clients", async () => { - saveConfig(config("localhost.")); - const server = startServer(0); - try { - expect(server.hostname).toBe("127.0.0.1"); - } finally { - await server.stop(true); - } + expect(server!.hostname).toBe("127.0.0.1"); }); }); From 434bdf52194d1e5222a5654cb4f66ee9f60f04fd Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 03:18:01 +0900 Subject: [PATCH 08/10] test(server): own non-loopback management fixture lifetime Reuse the owned management server fixture for the non-loopback settings test, keeping its real 0.0.0.0 listener, LAN Host/Origin, missing-token rejection, authenticated response and CORS assertion. Prepare real token/ACL state before the HTTP body, isolate only unrelated host diagnostic projections, track cancellation and consume both response bodies. Teardown settles the body and actual listener/spend owner before restoring homes. Rename the helper to its shared management-server role without changing policy, timeouts, assertions or the file-size baseline. --- tests/helpers/server-auth-fixture.ts | 18 ++++---- tests/server/server-auth.test.ts | 67 +++++++++++++--------------- 2 files changed, 40 insertions(+), 45 deletions(-) diff --git a/tests/helpers/server-auth-fixture.ts b/tests/helpers/server-auth-fixture.ts index d3a2681630c..ab1bda1c825 100644 --- a/tests/helpers/server-auth-fixture.ts +++ b/tests/helpers/server-auth-fixture.ts @@ -29,7 +29,7 @@ export function managementHeaders(initial?: HeadersInit): Headers { } /** A runner timeout does not cancel its async body or execute its local finally first. */ -function ownManagementCorsServer(server: ReturnType, restoreRuntime: () => void) { +function ownManagementServer(server: ReturnType, restoreRuntime: () => void) { const abort = new AbortController(); let body: Promise | undefined; let closing: Promise | undefined; @@ -62,18 +62,18 @@ function ownManagementCorsServer(server: ReturnType, restore }; } -export type ManagementCorsFixture = ReturnType; +export type ManagementServerFixture = ReturnType; -/** Prepare real auth/ACL state while isolating diagnostics unrelated to CORS behavior. */ -export async function startManagementCorsFixture( +/** Prepare real auth/ACL state while isolating unrelated host diagnostic projections. */ +export async function startManagementServerFixture( configDir: string, fixtureConfig: OcxConfig, -): Promise { +): Promise { if (existsSync(configDir)) removeTreeWithRetry(configDir); mkdirSync(configDir, { recursive: true }); process.env.OPENCODEX_HOME = configDir; saveConfig({ ...fixtureConfig, clientIntegrations: { codex: false } }); - // Real config/token ACL preparation belongs to fixture readiness, not the CORS + // Real config/token ACL preparation belongs to fixture readiness, not the HTTP // response deadline. Keep the production startup and the ordinary 5s test limit. // Neither management endpoint exercises native Codex synchronization or the // developer's installed service. Keep those external owners outside this fixture. @@ -82,10 +82,10 @@ export async function startManagementCorsFixture( }); try { const server = startServer(0, { - inspectNativeCodexOwnership: ownedServiceHomeInspection("management CORS sandbox"), + inspectNativeCodexOwnership: ownedServiceHomeInspection("management HTTP sandbox"), managementApi: { // /api/settings projects runtime/service diagnostics, but their host probes - // are not CORS behavior. Keep admission, routing and response decoration real. + // are not auth/CORS behavior. Keep admission, routing and response decoration real. getCachedStartupHealth: async () => deriveStartupHealth({ routingKind: "native", autostartEnabled: false, serviceInstalled: false, serviceViable: false, serviceEnabled: false, serviceRunning: false, @@ -94,7 +94,7 @@ export async function startManagementCorsFixture( }), }, }); - return ownManagementCorsServer(server, () => runtime.mockRestore()); + return ownManagementServer(server, () => runtime.mockRestore()); } catch (error) { try { await waitForFailedStartRollback(error); diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index febf802a2bb..5078135e136 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -23,7 +23,7 @@ import { } from "../../src/codex/routing"; import { loadConfig, saveConfig } from "../../src/config"; import { spendLedgerOwnerSnapshot } from "../../src/lib/spend-ledger-owner"; -import { settleServerAuthFixture, managementHeaders, startManagementCorsFixture, type ManagementCorsFixture } from "../helpers/server-auth-fixture"; +import { settleServerAuthFixture, managementHeaders, startManagementServerFixture, type ManagementServerFixture } from "../helpers/server-auth-fixture"; import { clearUpstreamHostHealth, getUpstreamHostHealth, recordUpstreamHostFailure, upstreamHostHealthKey } from "../../src/codex/upstream-host-health"; import { deriveProviderPresets } from "../../src/providers/derive"; import { MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; @@ -75,7 +75,7 @@ const originalGlobalWebSocket = globalThis.WebSocket; // isolation convention already used by tests/helpers/isolated-codex-home.ts. const TEST_DIR = mkdtempSync(join(tmpdir(), "ocx-server-auth-")); let isolatedCodexHome: IsolatedCodexHome | null = null; -let managementCorsFixture: ManagementCorsFixture | null = null; +let managementFixture: ManagementServerFixture | null = null; 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"; @@ -150,9 +150,9 @@ beforeEach(() => { }); afterEach(async () => { - if (managementCorsFixture) { - await managementCorsFixture.close(); - managementCorsFixture = null; + if (managementFixture) { + await managementFixture.close(); + managementFixture = null; } globalThis.fetch = originalGlobalFetch; globalThis.WebSocket = originalGlobalWebSocket; @@ -1308,11 +1308,11 @@ describe("server local API auth", () => { describe("management CORS fixture", () => { beforeEach(async () => { - managementCorsFixture = await startManagementCorsFixture(TEST_DIR, config("127.0.0.1")); + managementFixture = await startManagementServerFixture(TEST_DIR, config("127.0.0.1")); }); test("management CORS echoes validated loopback Origin and covers delegated codex-auth responses", async () => { - const fixture = managementCorsFixture!; + const fixture = managementFixture!; const origin = `http://127.0.0.1:${fixture.server.port}`; await fixture.run(async () => { const settings = await fetch(new URL("/api/settings", fixture.server.url), { @@ -1333,7 +1333,7 @@ describe("server local API auth", () => { }); test("management CORS fixture cancellation settles its body and releases the real spend lease", async () => { - const fixture = managementCorsFixture!; + const fixture = managementFixture!; let aborted = false; let settled = false; const entered = Promise.withResolvers(); @@ -1355,38 +1355,33 @@ describe("server local API auth", () => { }); }); - test("non-loopback management API allows same-origin GUI requests with API token", async () => { - if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); - mkdirSync(TEST_DIR, { recursive: true }); - process.env.OPENCODEX_HOME = TEST_DIR; - process.env.OPENCODEX_API_AUTH_TOKEN = "local-secret"; - saveConfig({ - ...config("0.0.0.0"), - port: 0, + describe("non-loopback management fixture", () => { + beforeEach(async () => { + process.env.OPENCODEX_API_AUTH_TOKEN = "local-secret"; + managementFixture = await startManagementServerFixture(TEST_DIR, { ...config("0.0.0.0"), port: 0 }); }); - const server = startServer(0); - const origin = `http://lan.example.test:${server.port}`; - try { - const missing = await fetch(`http://127.0.0.1:${server.port}/api/settings`, { - headers: { - host: `lan.example.test:${server.port}`, - origin, - }, - }); - expect(missing.status).toBe(401); + test("non-loopback management API allows same-origin GUI requests with API token", async () => { + const fixture = managementFixture!; + const server = fixture.server; + const origin = `http://lan.example.test:${server.port}`; + await fixture.run(async () => { + const missing = await fetch(`http://127.0.0.1:${server.port}/api/settings`, { + headers: { host: `lan.example.test:${server.port}`, origin }, + signal: fixture.signal, + }); + expect(missing.status).toBe(401); + await missing.text(); - const ok = await fetch(`http://127.0.0.1:${server.port}/api/settings`, { - headers: managementHeaders({ - host: `lan.example.test:${server.port}`, - origin, - }), + const ok = await fetch(`http://127.0.0.1:${server.port}/api/settings`, { + headers: managementHeaders({ host: `lan.example.test:${server.port}`, origin }), + signal: fixture.signal, + }); + expect(ok.status).toBe(200); + expect(ok.headers.get("access-control-allow-origin")).toBe(origin); + await ok.text(); }); - expect(ok.status).toBe(200); - expect(ok.headers.get("access-control-allow-origin")).toBe(origin); - } finally { - await server.stop(true); - } + }); }); test("websocket upgrade rejects hostile Origin even with a valid API token", async () => { From 085d1dd2e8cdc88374e2eefd7b970cb9f7bf1ce3 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:02:37 +0900 Subject: [PATCH 09/10] test(server): seed current config for localhost bind fixture --- tests/helpers/server-auth-fixture.ts | 11 +++++++++++ tests/server/server-auth-localhost-bind.test.ts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/helpers/server-auth-fixture.ts b/tests/helpers/server-auth-fixture.ts index ab1bda1c825..473eff61a4c 100644 --- a/tests/helpers/server-auth-fixture.ts +++ b/tests/helpers/server-auth-fixture.ts @@ -8,10 +8,12 @@ import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../../src/codex import { resetCodexModelEntitlementCacheForTests } from "../../src/codex/model-entitlements"; import { saveConfig } from "../../src/config"; import { flushConfigDirHardeningForTests } from "../../src/config/paths"; +import { migrateSubagentModels } from "../../src/config/subagent-models"; import { configuredAdminToken } from "../../src/lib/admin-secrets"; import { resetDebugLogBufferForTests } from "../../src/lib/debug-log-buffer"; import { resetDebugSettingsForTests } from "../../src/lib/debug-settings"; import { flushWindowsSecretAclReapsBeforeRemoval } from "../../src/lib/windows-secret-acl"; +import { projectOpenAiTierMigration } from "../../src/providers/openai-tiers"; import { clearHealthHistoryCacheForTests } from "../../src/routing/health"; import { closeRequestHistoryIndex } from "../../src/routing/history/indexer"; import { startServer, waitForFailedStartRollback } from "../../src/server"; @@ -20,6 +22,15 @@ import type { OcxConfig } from "../../src/types"; import { ownedServiceHomeInspection } from "./owned-service-home-inspection"; import { removeTreeWithRetry } from "./remove-tree"; +/** Seed a current installation without replaying unrelated upgrade writes at startup. */ +export function currentServerFixtureConfig(config: OcxConfig): OcxConfig { + // Use the production projections, not hand-maintained version flags. The caller + // still publishes once through saveConfig with real file/directory ACL hardening. + const current = projectOpenAiTierMigration(config).config; + migrateSubagentModels(current); + return current; +} + export function managementHeaders(initial?: HeadersInit): Headers { const token = configuredAdminToken(); if (!token) throw new Error("management token was not initialized"); diff --git a/tests/server/server-auth-localhost-bind.test.ts b/tests/server/server-auth-localhost-bind.test.ts index 8bd910a333c..b8c0ba47ec1 100644 --- a/tests/server/server-auth-localhost-bind.test.ts +++ b/tests/server/server-auth-localhost-bind.test.ts @@ -8,7 +8,7 @@ import { stopServerListener } from "../../src/server/lifecycle"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { serverAuthConfig as config } from "../helpers/server-auth-config"; -import { settleServerAuthFixture } from "../helpers/server-auth-fixture"; +import { currentServerFixtureConfig, settleServerAuthFixture } from "../helpers/server-auth-fixture"; import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; @@ -23,7 +23,7 @@ beforeEach(async () => { process.env.OPENCODEX_HOME = TEST_DIR; // Binding a hostname does not exercise native client synchronization. Keep real // listener/auth/ACL setup while excluding the host's installed service identity. - saveConfig({ ...config("localhost."), clientIntegrations: { codex: false } }); + saveConfig(currentServerFixtureConfig({ ...config("localhost."), clientIntegrations: { codex: false } })); try { server = startServer(0, { inspectNativeCodexOwnership: ownedServiceHomeInspection("localhost bind sandbox"), From a12b2ad38deafdcd9672e8fea597d5f7a679aa78 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:29:05 +0900 Subject: [PATCH 10/10] docs(runtime): consolidate Remote Workspace contract summary --- structure/runtime.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/structure/runtime.md b/structure/runtime.md index 8fdffda6775..cbcb7199933 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -382,9 +382,7 @@ Connected `ocx usage` reads `/v1/usage` through `src/client/hub-client.ts`, usin The client usage read requires HTTPS or loopback HTTP before adding the enrolled credential, and sets request `cache: "no-store"`; the hub response also forbids caching. -The shared atomic replacement publisher also identifies explicit Remote Workspace file writes as `remote-workspace`; its isolated owner and support limits are documented in [Remote Workspace](remote-workspace.md). - -Remote Workspace uses a separate, explicitly enabled server surface with structural WebSocket callbacks and awaited per-server cleanup; [its contract](remote-workspace.md) owns that integration. +The shared atomic replacement publisher identifies explicit Remote Workspace file writes as `remote-workspace`. Remote Workspace's separate, explicitly enabled server surface uses structural WebSocket callbacks and awaited per-server cleanup; [its contract](remote-workspace.md) owns that integration and documents its isolated owner and support limits. Chat helper admission in `src/server/responses/request-sidecar-auth.ts` follows the [deferred stored-main contract](providers/openai-tiers.md): only a needed Direct OpenAI helper