diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index bc0a5653ad3..be5dd4b70af 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1356,6 +1356,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", @@ -1383,6 +1384,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..97ffe111dc1 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,103 @@ 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. + * + * + * 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. + */ +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..26a68f1a034 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(); @@ -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 @@ -515,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(); } @@ -554,7 +553,6 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe deps, startupOwnershipHomes, startupOwnershipStatePaths, - startupWindowsTaskListingCache, ); const preparedNativeMainLifecycle = nativeOwnership.ownership !== "foreign" && startupOwnershipHomes !== null 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/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 80694162258..6fae969b733 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/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 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) { 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/codex-integration/codex-service-manager-probe-hardening.test.ts b/tests/codex-integration/codex-service-manager-probe-hardening.test.ts index cc342e7d5b4..f468a8ff510 100644 --- a/tests/codex-integration/codex-service-manager-probe-hardening.test.ts +++ b/tests/codex-integration/codex-service-manager-probe-hardening.test.ts @@ -219,7 +219,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; @@ -234,15 +234,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"); }; @@ -276,9 +283,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); } diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 81214295a00..f1a2abdba94 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1185,6 +1185,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", @@ -1212,6 +1213,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/helpers/server-auth-fixture.ts b/tests/helpers/server-auth-fixture.ts new file mode 100644 index 00000000000..473eff61a4c --- /dev/null +++ b/tests/helpers/server-auth-fixture.ts @@ -0,0 +1,137 @@ +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 { 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"; +import { stopServerListener } from "../../src/server/lifecycle"; +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"); + 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 ownManagementServer(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 ManagementServerFixture = ReturnType; + +/** Prepare real auth/ACL state while isolating unrelated host diagnostic projections. */ +export async function startManagementServerFixture( + 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 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. + const runtime = spyOn(codexRuntime, "resolveCodexRuntime").mockReturnValue({ + runtime: { command: "codex-fixture", version: null, source: "fallback" }, failures: [], + }); + try { + const server = startServer(0, { + inspectNativeCodexOwnership: ownedServiceHomeInspection("management HTTP sandbox"), + managementApi: { + // /api/settings projects runtime/service diagnostics, but their host probes + // 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, + serviceStale: false, serviceConflict: false, serviceSupported: true, + shimInstalled: false, shimHealthy: false, platform: process.platform, + }), + }, + }); + return ownManagementServer(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-localhost-bind.test.ts b/tests/server/server-auth-localhost-bind.test.ts new file mode 100644 index 00000000000..b8c0ba47ec1 --- /dev/null +++ b/tests/server/server-auth-localhost-bind.test.ts @@ -0,0 +1,54 @@ +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, 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 { currentServerFixtureConfig, 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(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(currentServerFixtureConfig({ ...config("localhost."), clientIntegrations: { codex: false } })); + try { + server = startServer(0, { + inspectNativeCodexOwnership: ownedServiceHomeInspection("localhost bind sandbox"), + }); + } catch (error) { + await waitForFailedStartRollback(error); + throw error; + } +}); + +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; + 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 () => { + expect(server!.hostname).toBe("127.0.0.1"); + }); +}); diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index e7bf1bd9d6b..5078135e136 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -22,6 +22,8 @@ import { recordCodexUpstreamOutcome, } from "../../src/codex/routing"; import { loadConfig, saveConfig } from "../../src/config"; +import { spendLedgerOwnerSnapshot } from "../../src/lib/spend-ledger-owner"; +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"; @@ -52,11 +54,12 @@ 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"; +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; @@ -72,23 +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; - -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", - }, - }, - }; -} +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"; @@ -106,14 +93,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", @@ -170,23 +149,20 @@ beforeEach(() => { isolatedCodexHome = installIsolatedCodexHome("ocx-server-auth-codex-"); }); -afterEach(() => { +afterEach(async () => { + if (managementFixture) { + await managementFixture.close(); + managementFixture = 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; + await settleServerAuthFixture(TEST_DIR, isolatedCodexHome?.path); if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = previousOpencodexHome; isolatedCodexHome?.restore(); isolatedCodexHome = null; - clearCodexUpstreamHealth(); - clearThreadAccountMap(); - clearAccountNeedsReauth("pool-a"); - clearAccountNeedsReauth("pool-b"); - clearAccountQuota(); - resetCodexModelEntitlementCacheForTests(); - resetDebugSettingsForTests(); - resetDebugLogBufferForTests(); if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); }); @@ -1330,64 +1306,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 () => { + managementFixture = await startManagementServerFixture(TEST_DIR, config("127.0.0.1")); + }); - 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 = 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), { + 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 = managementFixture!; + 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 () => { - 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 () => { 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. //