diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 39c502e170..d4f611149c 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -1639,6 +1639,8 @@ Consent applies to this provider ID in the selected `CODEX_HOME`; it does not approve other providers or other homes. Start and sync update only its `base_url` to the local proxy. Its identity, WebSocket setting, other TOML fields and history remain unchanged. Keep the proxy running while using this route. +Turning the Codex integration OFF also prevents approved-provider URL updates on +start or sync; the saved provider consent remains available when you turn it back on. This mode supports explicit Responses provider tables with `requires_openai_auth = true` and no custom credentials or headers. Non-loopback targets, authless Desktop mode diff --git a/docs-site/src/content/docs/reference/cli/agents.md b/docs-site/src/content/docs/reference/cli/agents.md index c09d2f06ae..a5c2e39e01 100644 --- a/docs-site/src/content/docs/reference/cli/agents.md +++ b/docs-site/src/content/docs/reference/cli/agents.md @@ -467,7 +467,9 @@ twice: the named range ending now, then the same range evaluated one equal windo filters apply to both reads. Human output prints current, previous, signed delta, and percent change. `--json` is `{schemaVersion,range,current,previous,delta,change,incomplete}`. The two reads are sequential, not an atomic snapshot. Custom `since`/`until` on each response must -match the requested window. +match the requested window. Both reads use the same connection and credential. If the +connection, account, or credential changes during the command, it reports an owner +mismatch and asks you to retry instead of printing a comparison. ### Page indexed request history diff --git a/src/cli/usage-compare.ts b/src/cli/usage-compare.ts index 6a081a2186..2f27bb2fb1 100644 --- a/src/cli/usage-compare.ts +++ b/src/cli/usage-compare.ts @@ -6,7 +6,8 @@ */ import { readClientConnectionState, sameClientConnectionOwner } from "../client/state"; import { fetchHubUsage } from "../client/hub-client"; -import { readServiceApiTokenState } from "../lib/service-secrets"; +import { readServiceApiTokenState, serviceApiTokenFingerprint } from "../lib/service-secrets"; +import { runningProxyUpdateHeaders } from "../oauth/login-cli"; import { redactSecretString } from "../lib/redact"; import { rangeWindow, USAGE_SURFACES } from "../usage/summary"; import type { UsageTimeWindow } from "../usage/time-range"; @@ -16,6 +17,7 @@ import { rejectArgs, RuntimeApiError, runtimeRequest, + runtimeBaseUrl, takeFlag, takeOption, takeOptionWithSyntax, @@ -174,22 +176,54 @@ function formatCompare(input: { return lines; } +// Private command-local authority; neither credentials nor fingerprints enter output. +function captureUsageOwner() { + const connection = readClientConnectionState(); + if (connection.kind === "invalid" || connection.kind === "mismatched") { + throw new Error(`Client usage unavailable: ${connection.reason}`); + } + const token = readServiceApiTokenState(); + if (connection.kind === "connected" && (token.kind !== "present" || token.fingerprint !== connection.value.tokenFingerprint)) { + throw new Error("Client usage unavailable: the enrolled data key is missing or changed; repair the client connection"); + } + const headers = runningProxyUpdateHeaders(); + const fingerprint = connection.kind === "connected" ? connection.value.tokenFingerprint + : serviceApiTokenFingerprint(headers.get("X-OpenCodex-API-Key") ?? ""); + return { connection, token, headers, fingerprint }; +} +type UsageOwner = ReturnType; + +function assertUsageOwner(owner: UsageOwner): void { + const current = readClientConnectionState(); + const expected = owner.connection; + let same = current.kind === expected.kind; + if (expected.kind === "connected") { + const token = readServiceApiTokenState(); + same = current.kind === "connected" && sameClientConnectionOwner(current.value, expected.value) + && current.value.transport === expected.value.transport + && JSON.stringify(current.value.link) === JSON.stringify(expected.value.link) + && current.value.tokenFingerprint === owner.fingerprint + && token.kind === "present" && token.fingerprint === owner.fingerprint; + } else { + same &&= serviceApiTokenFingerprint(runningProxyUpdateHeaders().get("X-OpenCodex-API-Key") ?? "") === owner.fingerprint; + } + if (!same) throw new Error("Usage owner changed during comparison; no comparison was produced. Retry for the current connection and credential."); +} + async function readUsage( suffix: string, window: UsageTimeWindow, apiKeyId: string | undefined, deps: RuntimeApiDeps, + owner: UsageOwner, ): Promise { - const connection = readClientConnectionState(); - if (connection.kind === "invalid" || connection.kind === "mismatched") { - throw new Error(`Client usage unavailable: ${connection.reason}`); - } + assertUsageOwner(owner); + const { connection, token } = owner; let result: ScopedUsage; if (connection.kind === "connected") { if (apiKeyId !== undefined) { throw new CliUsageError("--api-key-id is unavailable on connected clients; run on the hub for a selected key, or omit it for this client's usage", USAGE_COMPARE_USAGE); } - const token = readServiceApiTokenState(); if (token.kind !== "present" || token.fingerprint !== connection.value.tokenFingerprint) { throw new Error("Client usage unavailable: the enrolled data key is missing or changed; repair the client connection"); } @@ -199,18 +233,11 @@ async function readUsage( result = await fetchHubUsage(connection.value.serverUrl, token.token, new URLSearchParams(suffix.replace(/^\?/, "")), { fetchImpl: peerFetch, timeoutMs: 60_000, ...(link ? { peer: "Home" as const } : {}), }); - const current = readClientConnectionState(); - const currentToken = readServiceApiTokenState(); - if (current.kind !== "connected" || !sameClientConnectionOwner(current.value, connection.value) - || current.value.tokenFingerprint !== token.fingerprint - || currentToken.kind !== "present" || currentToken.fingerprint !== token.fingerprint) { - throw new Error("Client connection changed while reading usage; retry for the current connection"); - } } else if (apiKeyId === undefined) { - result = await runtimeRequest(`/api/usage${suffix}`, {}, deps); + result = await runtimeRequest(`/api/usage${suffix}`, { headers: owner.headers }, deps); } else { try { - result = await runtimeRequest(`/api/usage${suffix}`, { redirect: "error", credentials: "omit" }, deps); + result = await runtimeRequest(`/api/usage${suffix}`, { redirect: "error", credentials: "omit", headers: owner.headers }, deps); } catch (error) { if (error instanceof RuntimeApiError && error.code === "proxy_not_running") throw error; throw new Error("Key-scoped usage could not be read. Check runtime access and retry."); @@ -222,6 +249,7 @@ async function readUsage( if (result?.customWindow !== true || result.since !== window.since || result.until !== window.until) { throw new Error("The server did not confirm the requested custom usage window. Upgrade and restart the proxy, then retry."); } + assertUsageOwner(owner); return result; } @@ -247,8 +275,10 @@ export async function handleUsageCompareCommand(argv: string[], deps: UsageCompa if (!Number.isSafeInteger(now) || now < 0) throw new CliUsageError("compare clock is invalid", USAGE_COMPARE_USAGE); const windows = compareUsageWindows(range, now); const filters = { range, surface, provider, model, apiKeyId }; - const currentReport = await readUsage(query({ ...filters, since: windows.current.since, until: windows.current.until }), windows.current, apiKeyId, deps); - const previousReport = await readUsage(query({ ...filters, since: windows.previous.since, until: windows.previous.until }), windows.previous, apiKeyId, deps); + const owner = captureUsageOwner(); + const readDeps = owner.connection.kind === "disconnected" ? { ...deps, baseUrl: await runtimeBaseUrl(deps) } : deps; + const currentReport = await readUsage(query({ ...filters, since: windows.current.since, until: windows.current.until }), windows.current, apiKeyId, readDeps, owner); + const previousReport = await readUsage(query({ ...filters, since: windows.previous.since, until: windows.previous.until }), windows.previous, apiKeyId, readDeps, owner); const current = totalsOf(currentReport); const previous = totalsOf(previousReport); const deltaTotals: Totals = {}; diff --git a/src/codex/config-write-lock.ts b/src/codex/config-write-lock.ts index c29546853c..17c705c8ab 100644 --- a/src/codex/config-write-lock.ts +++ b/src/codex/config-write-lock.ts @@ -96,8 +96,29 @@ function acquire(configPath: string): ConfigWriteAcquireResult { throw error; } const acquired = tryAcquire(`${target.canonical}.ocx-write.lock`); - if (acquired.ok) destinations.set(acquired.handle, target); - return acquired; + if (!acquired.ok) return acquired; + try { + // A preceding writer may atomically replace the file before releasing the lock. + // Capture the inode only after acquisition, while retaining the canonical-path gate. + const current = destination(configPath); + if (current.canonical !== target.canonical || !stillHeld(acquired.handle)) { + release(acquired.handle); + return { ok: false, error: "unsafe", detail: acquired.handle.path }; + } + // Preserve caller-owned unreadable/non-regular errors, but do not mistake a + // dangling link (realpath's fallback) or a fresh replacement for proven absence. + try { + const entry = lstatSync(current.canonical, { bigint: true }); + if (entry.isSymbolicLink() || `${entry.dev}:${entry.ino}` !== current.inode) throw new ConfigWriteDestinationChanged(); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT" || current.inode !== null) throw error; + } + destinations.set(acquired.handle, current); + return acquired; + } catch (error) { + release(acquired.handle); + throw error; + } } export function withConfigWriteLock(configPath: string, run: (handle: LockHandle) => T): ConfigWriteLockOutcome { const acquired = acquire(configPath); diff --git a/src/codex/provider-routing.ts b/src/codex/provider-routing.ts index e3f153aedb..c6b171ce3b 100644 --- a/src/codex/provider-routing.ts +++ b/src/codex/provider-routing.ts @@ -2,6 +2,8 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { z } from "zod"; +import { loadConfig } from "../config"; +import { shouldSyncCodexOnStart } from "./desired-state"; import { atomicWriteFile } from "../config/atomic-write"; import { withConfigMutationLockSync } from "../config/mutation-lock"; import { getConfigDir } from "../config/paths"; @@ -10,7 +12,7 @@ import { ConfigWriteLockRefusal, publishConfigWriteTarget, watchConfigWriteTarge import { providerTableStart } from "./injected-marker"; import { CODEX_CONFIG_PATH, getCodexHome, tomlString } from "./paths"; import { rootSourceLines, sourceAssignment, sourceText } from "./toml-source-lines"; -import { cleanExternalProviderJournal } from "./inject/config-write-section"; +import { JOURNAL_PATH, removeJournal } from "./journal"; import { externalCodexModelProvider } from "./inject/config-toml"; import { readBoundedCodexConfig } from "./inject/bounded-config-reader"; import { isLoopbackHostname } from "./loopback-target"; @@ -75,7 +77,7 @@ function providerUrl(content: string, id: string, checkCredentials = true) { function inWriteSection(run: (held: LockHandle, content: string, record: RoutingRecord) => T): T { assertCodexHomeOwner(getCodexHome()); const result = withConfigWriteLockHeld(CODEX_CONFIG_PATH, undefined, held => { - watchConfigWriteTargets(held, [CODEX_CONFIG_PATH, recordPath()]); + watchConfigWriteTargets(held, [CODEX_CONFIG_PATH, recordPath(), JOURNAL_PATH]); const content = readFileSync(CODEX_CONFIG_PATH, "utf8"); return run(held, content, readRecord()); }); @@ -122,21 +124,22 @@ function accepts(fieldUrl: string, saved: Approval): boolean { /** Called by start/sync, never prompts and never broadens the approved provider set. */ export function injectApprovedCodexProvider(id: string, target: CodexRoutingTarget, beforeWrite?: () => void, validateOnly = false): false | { configChanged: boolean } { - if (!Object.hasOwn(readRecord().providers, id)) return false; - target = validateCodexRoutingTarget(target); - if (target.requiresAdmissionToken || target.desktopAuthless || target.clientCompaction || !isLoopbackHostname(new URL(target.baseUrl).hostname)) { - throw new Error("Approved custom providers support only authenticated Codex on a local loopback proxy without client compaction."); - } - if (!validateOnly) cleanExternalProviderJournal(beforeWrite); return inWriteSection((held, content, record) => { + if (!Object.hasOwn(record.providers, id)) return false; const apply = () => { - if (!Object.hasOwn(record.providers, id)) return false; + target = validateCodexRoutingTarget(target); + if (target.requiresAdmissionToken || target.desktopAuthless || target.clientCompaction || !isLoopbackHostname(new URL(target.baseUrl).hostname)) { + throw new Error("Approved custom providers support only authenticated Codex on a local loopback proxy without client compaction."); + } if (externalCodexModelProvider(content) !== id) throw new Error("Active provider changed; routing was not applied."); const field = providerUrl(content, id), saved = record.providers[id]!; if (!accepts(field.url, saved)) throw new Error("Approved provider base_url changed outside OpenCodex; revoke and approve it again."); - if (validateOnly || field.url === target.baseUrl) return { configChanged: false }; + if (validateOnly) return { configChanged: false }; beforeWrite?.(); + if (!shouldSyncCodexOnStart(loadConfig())) return false; if (readFileSync(CODEX_CONFIG_PATH, "utf8") !== content) throw new Error("Codex config changed before publication; retry sync."); + publishConfigWriteTarget(CODEX_CONFIG_PATH, held, JOURNAL_PATH, (_destination, hooks) => removeJournal(hooks)); + if (field.url === target.baseUrl) return { configChanged: false }; // Write recovery evidence first. Both the previous and candidate URL remain restorable // if publication fails or the process exits between these two atomic renames. saved.managedUrls = [...new Set([field.url, target.baseUrl])].filter(url => url !== saved.originalUrl); @@ -145,7 +148,7 @@ export function injectApprovedCodexProvider(id: string, target: CodexRoutingTarg publish(CODEX_CONFIG_PATH, field.replace(`base_url = ${tomlString(target.baseUrl)}`), held); return { configChanged: true }; }; - return beforeWrite ? withConfigMutationLockSync(apply) : apply(); + return validateOnly ? apply() : withConfigMutationLockSync(apply); }); } diff --git a/structure/cli-management.md b/structure/cli-management.md index b04bf04ba4..7e588af09d 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -121,7 +121,7 @@ Storage policy flags in `src/cli/storage.ts` preserve omitted enable/settings an `src/cli/log-view-filter.ts` owns the additive logs-filter snapshot, separate from existing follow/event transport. It fetches only the fixed log endpoint with a scan limit of at most 2000, applies GUI-equivalent local selectors, then returns the newest matches in source order without deduplicating. Model/provider matching covers resolved/served/attempt identities; protocol and conversation matching reuse their actual owners. Speed bounds accept observed value-kind metrics only. The versioned JSON view separates loaded, matched and returned counts from configured scan/output limits; JSONL is rows only. Neither a match count nor a cursor claims historical completeness or resumable search. Invalid options fail before discovery; malformed windows never become empty success. -`src/cli/usage-compare.ts` reads `GET /api/usage` twice: the named range at now, then the same range shifted back one equal window. `--range all` is refused. Both responses must acknowledge the derived custom bounds. Key-scoped reads keep redirect-error transport and echoed `apiKeyId`. Connected clients reuse the existing hub usage path and refuse caller `--api-key-id`. Default range is 30d. The two reads are sequential, not an atomic snapshot. +`src/cli/usage-compare.ts` reads `GET /api/usage` twice: the named range at now, then the same range shifted back one equal window. `--range all` is refused. Both responses must acknowledge the derived custom bounds. Key-scoped reads keep redirect-error transport and echoed `apiKeyId`. Connected clients reuse the existing hub usage path and refuse caller `--api-key-id`. Default range is 30d. A command-local snapshot pins connection mode, usage owner, destination and credential fingerprint for both reads. Each read verifies that the owner and credential still match; a change discards the comparison with a retry message. Credentials and fingerprints never enter output. The two reads are sequential, not an atomic snapshot. `src/cli/usage-model-search.ts` applies optional substring search only after the existing usage target/scope acknowledgment. A copied model list sorts stably by total tokens, searches model/provider/resolvedModel and caps at 100 by default. The report's other fields remain intact; explicit modelView metadata distinguishes a view miss from no usage. `src/cli/usage-report.ts` retains the legacy renderer when no view is supplied and renders the selected rows when one is supplied. `--top` accepts 1–1000 human model rows (default 10 without search, 100 with search), applies after search and is never sent to the API. JSON ignores `--top`, including the existing 100-row search view and its metadata. Search never becomes a server attribution query or widens connected-client/key scope. diff --git a/structure/codex-home.md b/structure/codex-home.md index 5df85bffe9..d541ba24ab 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -300,7 +300,7 @@ batches, injection, journal replay, removal, and restore/compensation read and w holding it. Injection and coordinated restore take config before SQLite N; prompt writes take prompt-store before config. Nested writers receive an explicit live handle for the same canonical config destination; implicit nesting refuses fast. Symlink aliases contend -on the real destination's lock. Each publication revalidates canonical identity and inode immediately before rename, +on the real destination's lock. Acquisition revalidates the canonical path and captures the inode after the lock is held, admitting a preceding writer's completed atomic replacement while refusing a canonical retarget before entering the section. Each publication revalidates canonical identity and inode immediately before rename, including each Windows retry; the confirmed publication hook advances that witness. Async acquisition uses a monotonic deadline, defaults to two seconds, and accepts only integer timeouts from zero to ten seconds. Zero permits one immediate attempt. diff --git a/structure/codex-provider-routing.md b/structure/codex-provider-routing.md index 5a2f2a962a..3a7d7571d1 100644 --- a/structure/codex-provider-routing.md +++ b/structure/codex-provider-routing.md @@ -8,6 +8,7 @@ lock and destination guards; recovery evidence precedes URL publication and reta previous and candidate local URL across an interrupted write. Changed foreign URLs refuse injection and survive restore. Native restore completes its existing preflight before restoring approved URLs. Foreground signal/exit cleanup delegates to the same native restore, which restores approved URLs and preserves unapproved external configurations. Partial native outcomes remain partial, and recovery failures are reported in the config artifact. Consent survives stop/restore; explicit revocation removes it. Successful URL publication returns a committed configuration-change receipt through injection/sync; validation-only and unchanged URL operations report no change, so explicit config-change restarts retain the same authority. +Approval-record inspection, stale native-journal cleanup and URL publication share one Codex write section, so a cooperating concurrent sync cannot replace the record during its bounded read. The shared lock witnesses the config inode after acquisition, so a preceding writer’s completed publication does not invalidate the next holder. URL injection rechecks the current integration intent under both the Codex write lock and config mutation lock, after the client-write guard. Integration OFF skips URL and recovery-record publication without revoking provider consent. Validation-only inspection does not acquire the config mutation lock or create its persistent coordination metadata. Only Responses/OpenAI-auth provider tables without separate credentials or headers are eligible, and only ordinary loopback routing is admitted. Authless, client-compaction and admission-token targets are refused rather than silently changing authentication policy. diff --git a/tests/cli/cli-usage-compare.test.ts b/tests/cli/cli-usage-compare.test.ts index b5123a0dd3..9ae50d303a 100644 --- a/tests/cli/cli-usage-compare.test.ts +++ b/tests/cli/cli-usage-compare.test.ts @@ -2,6 +2,13 @@ * `ocx usage compare` reads the named range, then the previous equal window. */ import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { repoRoot } from "../helpers/repo-root"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; import { handleObserveCommand } from "../../src/cli/observe"; import { compareUsageWindows } from "../../src/cli/usage-compare"; @@ -221,3 +228,87 @@ describe("ocx usage compare", () => { expect(JSON.parse(lines.join("\n")).filter.apiKeyId).toBe("Key-A"); }); }); + + +test.each(["stable-local", "stable-hub", "connect", "disconnect", "account", "credential", "admin", "transport"])( + "usage comparison keeps one owner across both windows (%s)", scenario => { + const home = mkdtempSync(join(tmpdir(), "ocx-compare-owner-")); + try { + const child = spawnSync(process.execPath, ["--eval", ` + import { spyOn } from "bun:test"; + import { writeFileSync } from "node:fs"; + import { join } from "node:path"; + import * as state from "./src/client/state"; + import { serviceApiTokenFingerprint } from "./src/lib/service-secrets"; + import { handleObserveCommand } from "./src/cli/observe"; + const scenario = ${JSON.stringify(scenario)}; + const home = process.env.OPENCODEX_HOME; + const token = "compare-fixture-key-a", nextToken = "compare-fixture-key-b"; + const client = { serverUrl: "https://hub.example.test", managementUrl: "https://manage.example.test", + managementTransport: "direct", selectedClients: ["claude"], tokenEnv: "OPENCODEX_API_AUTH_TOKEN", + apiKeyId: "account-a", tokenFingerprint: serviceApiTokenFingerprint(token), protocolVersion: 1, + connectedAt: "2026-09-01T00:00:00.000Z" }; + const connected = !["stable-local", "connect", "admin"].includes(scenario); + const write = (isClient) => writeFileSync(join(home, "config.json"), JSON.stringify({ + port: 9, providers: {}, defaultProvider: "openai", runtimeRole: isClient ? "client" : "standalone", + ...(isClient ? { client } : {}), + })); + write(connected); + writeFileSync(join(home, "service-api-token"), token); + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "compare-fixture-admin-a"; + const change = () => { + if (scenario === "account") client.apiKeyId = "account-b"; + if (scenario === "credential") { + writeFileSync(join(home, "service-api-token"), nextToken); + client.tokenFingerprint = serviceApiTokenFingerprint(nextToken); + } + if (scenario === "transport") client.transport = "hub"; + if (scenario === "admin") process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "compare-fixture-admin-b"; + write(scenario !== "disconnect"); + }; + // On the old implementation these are the first read's before/after observations, + // followed by the second read's capture. Change only after that first owner was verified. + const read = state.readClientConnectionState; + let observations = 0; + spyOn(state, "readClientConnectionState").mockImplementation(() => { + if (++observations === 3 && connected && !scenario.startsWith("stable")) change(); + return read(); + }); + const output = [], errors = [], requests = []; + console.log = (...args) => output.push(args.join(" ")); + console.error = (...args) => errors.push(args.join(" ")); + const code = await handleObserveCommand(["usage", "compare", "--json"], { + baseUrl: "http://local.test", now: () => ${NOW}, + fetchImpl: async (input, init) => { + const url = new URL(String(input)); + requests.push({ origin: url.origin, path: url.pathname, + originalCredential: new Headers(init?.headers).get("x-opencodex-api-key") === (connected ? token : "compare-fixture-admin-a") }); + if (requests.length === 1 && ["connect", "admin"].includes(scenario)) change(); + return Response.json({ schemaVersion: 1, source: "hub", scope: "client", range: "30d", surface: "all", + generatedAt: 1, customWindow: true, since: Number(url.searchParams.get("since")), until: Number(url.searchParams.get("until")), + summary: { requests: 1, totalTokens: 3, inputTokens: 2, outputTokens: 1, cachedInputTokens: 0, unpricedRequests: 1, unmeteredRequests: 0 }, + providers: [], models: [], days: [], filter: { provider: null, model: null, matched: true, comboOverlap: false } }); + }, + }); + process.stdout.write(JSON.stringify({ code, output, errors, requests })); + `], { cwd: repoRoot(), env: { ...process.env, OPENCODEX_HOME: home }, encoding: "utf8", timeout: SPAWN_BUDGET_MS - 5000 }); + expect(child.status, child.stderr).toBe(0); + const result = JSON.parse(child.stdout); + if (scenario.startsWith("stable")) { + expect(result.code).toBe(0); + expect(result.requests).toHaveLength(2); + expect(result.requests.every((request: { originalCredential: boolean }) => request.originalCredential)).toBe(true); + expect(result.requests.map((request: { path: string }) => request.path)).toEqual(Array(2).fill(scenario === "stable-hub" ? "/v1/usage" : "/api/usage")); + expect(JSON.parse(result.output.join("\n")).delta.requests).toBe(0); + } else { + expect(result.code).toBe(1); + expect(result.output).toEqual([]); + expect(result.errors.join("\n")).toContain("Usage owner changed during comparison"); + expect(result.requests).toHaveLength(1); + } + expect(child.stdout).not.toContain("compare-fixture"); + } finally { + removeTreeWithRetry(home); + } + }, SPAWN_BUDGET_MS, +); diff --git a/tests/codex-integration/codex-config-write-lock.test.ts b/tests/codex-integration/codex-config-write-lock.test.ts index 182785c420..a22de718d9 100644 --- a/tests/codex-integration/codex-config-write-lock.test.ts +++ b/tests/codex-integration/codex-config-write-lock.test.ts @@ -9,14 +9,17 @@ * handle through (`heldConfigWriteLock`) so a nested writer does not refuse * itself inside the caller's wider section. */ -import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { existsSync, mkdtempSync, readFileSync, realpathSync, renameSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; +import * as promptLock from "../../src/codex/prompt-lock"; +import { atomicWriteFile } from "../../src/config/atomic-write"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { acquireConfigWriteLock, CONFIG_WRITE_LOCKED_MESSAGE, configWriteLockPath, + publishConfigWrite, releaseConfigWriteLock, withConfigWriteLock, withConfigWriteLockHeld, @@ -1147,3 +1150,79 @@ test("failed standalone toggle restores its exact preimage", async () => { expect(fs.readFileSync(a)).toEqual(original); }); }); + + +test("acquisition witnesses the inode after a preceding writer commits and releases", () => { + const path = fixtureConfig('model = "before"\n'); + const lockPath = configWriteLockPath(path); + const acquire = promptLock.tryAcquire; + let interleaved = false; + const spy = spyOn(promptLock, "tryAcquire").mockImplementation((file, deps) => { + if (file === lockPath && !interleaved) { + interleaved = true; + const prior = acquire(file, deps); + if (!prior.ok) throw new Error("preceding writer did not acquire"); + try { + writeFileSync(path + ".next", 'model = "preceding"\n'); + renameSync(path + ".next", path); + } finally { promptLock.release(prior.handle); } + } + return acquire(file, deps); + }); + try { + const result = withConfigWriteLock(path, held => { + const next = readFileSync(path, "utf8") + "# following writer\n"; + publishConfigWrite(path, held, (destination, hooks) => atomicWriteFile(destination, next, undefined, hooks)); + return readFileSync(path, "utf8"); + }); + expect(interleaved).toBe(true); + expect(result).toEqual({ ok: true, value: 'model = "preceding"\n# following writer\n' }); + expect(existsSync(lockPath)).toBe(false); + } finally { spy.mockRestore(); } +}); + +test("acquisition refuses a canonical retarget before entering the write section", () => { + const a = fixtureConfig('model = "a"\n'), b = fixtureConfig('model = "b"\n'); + const parent = mkdtempSync(join(tmpdir(), "ocx-acquire-alias-")); roots.push(parent); + const alias = join(parent, "home"); + const aHome = realpathSync(join(a, "..")), bHome = realpathSync(join(b, "..")); + symlinkSync(aHome, alias, "junction"); + const path = join(alias, "config.toml"), lockPath = configWriteLockPath(path); + const acquire = promptLock.tryAcquire; + let retargeted = false, entered = false; + const spy = spyOn(promptLock, "tryAcquire").mockImplementation((file, deps) => { + if (file === lockPath && !retargeted) { + retargeted = true; + unlinkSync(alias); symlinkSync(bHome, alias, "junction"); + } + return acquire(file, deps); + }); + try { + expect(withConfigWriteLock(path, () => { entered = true; })).toEqual({ ok: false, error: "unsafe", detail: lockPath }); + expect(entered).toBe(false); + expect(readFileSync(a, "utf8")).toBe('model = "a"\n'); + expect(readFileSync(b, "utf8")).toBe('model = "b"\n'); + expect(existsSync(lockPath)).toBe(false); + } finally { spy.mockRestore(); } +}); + + +test("acquisition does not admit a dangling symlink as a preceding writer's deletion", () => { + const path = fixtureConfig('model = "a"\n'), lockPath = configWriteLockPath(path); + const acquire = promptLock.tryAcquire; + let changed = false, entered = false; + const spy = spyOn(promptLock, "tryAcquire").mockImplementation((file, deps) => { + if (file === lockPath && !changed) { + changed = true; + renameSync(path, path + ".original"); + symlinkSync(path + ".missing", path); + } + return acquire(file, deps); + }); + try { + expect(() => withConfigWriteLock(path, () => { entered = true; })).toThrow("destination changed"); + expect(entered).toBe(false); + expect(readFileSync(path + ".original", "utf8")).toBe('model = "a"\n'); + expect(existsSync(lockPath)).toBe(false); + } finally { spy.mockRestore(); } +}); diff --git a/tests/codex-integration/codex-provider-routing.test.ts b/tests/codex-integration/codex-provider-routing.test.ts index dc4f488e6e..b575e47f73 100644 --- a/tests/codex-integration/codex-provider-routing.test.ts +++ b/tests/codex-integration/codex-provider-routing.test.ts @@ -106,8 +106,11 @@ test("validation is read-only, revocation restores managed URL and removes futur run(` allow(); const before = readFileSync(path, "utf8"); + const { readdirSync } = await import("node:fs"); + const entries = JSON.stringify(readdirSync(process.env.OPENCODEX_HOME).sort()); await injectCodexConfig(10100, {}, { validateOnly: true }); if (readFileSync(path, "utf8") !== before) throw new Error("validation wrote config"); + if (JSON.stringify(readdirSync(process.env.OPENCODEX_HOME).sort()) !== entries) throw new Error("validation created mutation metadata"); await injectCodexConfig(10100, {}); if (await handleCodexProviderCommand(["revoke", id]) !== 0) throw new Error("revoke failed"); const result = await injectCodexConfig(10100, {}); @@ -175,13 +178,21 @@ test("reserved provider IDs cannot claim consent", () => { test("concurrent sync processes leave a complete restorable config", () => { run(` allow(); + const before = readFileSync(path, "utf8"); const jobs = [18273, 18274, 18275].map(port => Bun.spawn([process.execPath, "--eval", - 'import { injectCodexConfig } from "./src/codex/inject"; const result = await injectCodexConfig(' + port + ', {}); if (!result.success && !result.retryable) process.exit(2);' + 'import { injectCodexConfig } from "./src/codex/inject"; const result = await injectCodexConfig(' + port + ', {}); process.stdout.write(JSON.stringify(result)); if (!result.success && !result.retryable) process.exit(2);' ], { cwd: process.cwd(), env: process.env, stdout: "pipe", stderr: "pipe" })); - const exits = await Promise.all(jobs.map(job => job.exited)); - if (exits.some(code => code !== 0)) throw new Error("concurrent sync failed: " + exits); + const outcomes = await Promise.all(jobs.map(async job => ({ code: await job.exited, error: await new Response(job.stderr).text(), output: await new Response(job.stdout).text() }))); + if (outcomes.some(result => result.code !== 0)) throw new Error("concurrent sync failed: " + JSON.stringify(outcomes)); + const results = outcomes.map(outcome => JSON.parse(outcome.output)); + if (results.some(result => !result.success && result.retryable !== true)) throw new Error("non-retryable concurrent result"); + if (results.every(result => !result.success)) { + if (readFileSync(path, "utf8") !== before) throw new Error("retry-only syncs mutated config"); + const converged = await injectCodexConfig(18273, {}); + if (!converged.success || converged.configChanged !== true) throw new Error("uncontended sync did not converge: " + JSON.stringify(converged)); + } const config = Bun.TOML.parse(readFileSync(path, "utf8")); - if (![18273, 18274, 18275].some(port => config.model_providers[id].base_url === "http://127.0.0.1:" + port + "/v1")) throw new Error("no sync published"); + if (![18273, 18274, 18275].some(port => config.model_providers[id].base_url === "http://127.0.0.1:" + port + "/v1")) throw new Error("no sync published: " + JSON.stringify(outcomes)); `); run("restoreApprovedCodexProviders();"); expect(readFileSync(join(codexHome, "config.toml"), "utf8")).toBe(original); @@ -243,3 +254,54 @@ test("startup health recognizes only the exact managed URL of an approved provid if (getCodexRoutingKind() !== "custom-local") throw new Error("unrecorded URL was claimed"); `); }); + + +test.each(["already-off", "late-off"])("approved routing respects integration OFF at publication (%s)", timing => { + run(` + const { readdirSync } = await import("node:fs"); + const { getDefaultConfig, saveConfig } = await import("./src/config"); + const { injectApprovedCodexProvider } = await import("./src/codex/provider-routing"); + const { standaloneCodexRoutingTarget } = await import("./src/codex/inject/routing-target"); + const config = getDefaultConfig(); + saveConfig(config); allow(); + const record = join(process.env.OPENCODEX_HOME, readdirSync(process.env.OPENCODEX_HOME).find(name => name.startsWith("codex-provider-routing-") && name.endsWith(".json"))); + const evidence = readFileSync(record, "utf8"); + const disable = () => { + config.clientIntegrations = { codex: false }; + writeFileSync(join(process.env.OPENCODEX_HOME, "config.json"), JSON.stringify(config)); + }; + if (${JSON.stringify(timing)} === "already-off") disable(); + const result = injectApprovedCodexProvider(id, standaloneCodexRoutingTarget(18281, {}), () => { + if (${JSON.stringify(timing)} === "late-off") disable(); + }); + if (result !== false) throw new Error("integration OFF was not skipped"); + if (readFileSync(record, "utf8") !== evidence) throw new Error("skipped injection changed recovery evidence"); + if (!approvedCodexProviders().includes(id)) throw new Error("skip revoked consent"); + `); + expect(readFileSync(join(codexHome, "config.toml"), "utf8")).toBe(original); + expect(readFileSync(join(codexHome, "history.jsonl"), "utf8")).toBe("history sentinel\n"); +}); + + +test("concurrent approval replacement cannot escape injection as an unhandled read failure", () => { + run(` + const fs = await import("node:fs"); + const { spyOn } = await import("bun:test"); + allow(); + const record = join(process.env.OPENCODEX_HOME, fs.readdirSync(process.env.OPENCODEX_HOME).find(name => name.startsWith("codex-provider-routing-") && name.endsWith(".json"))); + const stat = fs.statSync; + // Reproduce a cooperating writer's atomic rename between the reader's stat and open. + const observation = spyOn(fs, "statSync").mockImplementation((file, ...options) => { + const result = stat(file, ...options); + if (file === record && !existsSync(path + ".ocx-write.lock")) { + writeFileSync(record + ".replacement", readFileSync(record)); + fs.renameSync(record + ".replacement", record); + } + return result; + }); + try { + const result = await injectCodexConfig(18282, {}); + if (!result.success && !result.retryable) throw new Error("concurrent injection was not success-or-retryable"); + } finally { observation.mockRestore(); } + `); +});