From 3195b5209e5dd15c25d5c605f6cf1d85132fce32 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 18:39:24 +0900 Subject: [PATCH 1/8] fix(codex): honor integration opt-out for approved providers Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- .../content/docs/guides/codex-integration.md | 2 ++ src/codex/provider-routing.ts | 5 +++- structure/codex-provider-routing.md | 1 + .../codex-provider-routing.test.ts | 29 +++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 39c502e170f..d4f611149c4 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/src/codex/provider-routing.ts b/src/codex/provider-routing.ts index e3f153aedb3..4e11c625788 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"; @@ -136,6 +138,7 @@ export function injectApprovedCodexProvider(id: string, target: CodexRoutingTarg 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 }; beforeWrite?.(); + if (!shouldSyncCodexOnStart(loadConfig())) return false; if (readFileSync(CODEX_CONFIG_PATH, "utf8") !== content) throw new Error("Codex config changed before publication; retry sync."); // Write recovery evidence first. Both the previous and candidate URL remain restorable // if publication fails or the process exits between these two atomic renames. @@ -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 withConfigMutationLockSync(apply); }); } diff --git a/structure/codex-provider-routing.md b/structure/codex-provider-routing.md index 5a2f2a962a7..8c6e9f8e9fc 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. +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. 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/codex-integration/codex-provider-routing.test.ts b/tests/codex-integration/codex-provider-routing.test.ts index dc4f488e6e8..1dd3c8ce55e 100644 --- a/tests/codex-integration/codex-provider-routing.test.ts +++ b/tests/codex-integration/codex-provider-routing.test.ts @@ -243,3 +243,32 @@ 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(); + let calls = 0; + const result = injectApprovedCodexProvider(id, standaloneCodexRoutingTarget(18281, {}), () => { + // The first guard belongs to stale-journal cleanup; the second is URL publication. + if (++calls === 2 && ${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"); +}); From 104aee9f35f0ed006f2b259c5394f03371b1e66c Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 18:41:02 +0900 Subject: [PATCH 2/8] fix(cli): bind usage comparison windows to one owner Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- .../src/content/docs/reference/cli/agents.md | 4 +- src/cli/usage-compare.ts | 64 +++++++++---- structure/cli-management.md | 2 +- tests/cli/cli-usage-compare.test.ts | 91 +++++++++++++++++++ 4 files changed, 142 insertions(+), 19 deletions(-) diff --git a/docs-site/src/content/docs/reference/cli/agents.md b/docs-site/src/content/docs/reference/cli/agents.md index c09d2f06ae8..a5c2e39e01d 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 6a081a2186d..2f27bb2fb15 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/structure/cli-management.md b/structure/cli-management.md index b04bf04ba43..7e588af09df 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/tests/cli/cli-usage-compare.test.ts b/tests/cli/cli-usage-compare.test.ts index b5123a0dd30..9ae50d303a4 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, +); From a22ef2287c2e468eb961f60a6a33a7f1d42c5187 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 18:45:14 +0900 Subject: [PATCH 3/8] fix(codex): serialize approved-provider recovery reads Keep approval inspection, stale journal cleanup and URL publication in one write section so a concurrent atomic record replacement cannot escape as an unhandled bounded-read failure. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- src/codex/provider-routing.ts | 18 +++++------ structure/codex-provider-routing.md | 2 +- .../codex-provider-routing.test.ts | 32 ++++++++++++++++--- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/codex/provider-routing.ts b/src/codex/provider-routing.ts index 4e11c625788..20a3b441639 100644 --- a/src/codex/provider-routing.ts +++ b/src/codex/provider-routing.ts @@ -12,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"; @@ -77,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()); }); @@ -124,22 +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) => { 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); diff --git a/structure/codex-provider-routing.md b/structure/codex-provider-routing.md index 8c6e9f8e9fc..d432d7a7dd3 100644 --- a/structure/codex-provider-routing.md +++ b/structure/codex-provider-routing.md @@ -8,7 +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. -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. +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. 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. 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/codex-integration/codex-provider-routing.test.ts b/tests/codex-integration/codex-provider-routing.test.ts index 1dd3c8ce55e..c3cc8571d40 100644 --- a/tests/codex-integration/codex-provider-routing.test.ts +++ b/tests/codex-integration/codex-provider-routing.test.ts @@ -178,8 +178,8 @@ test("concurrent sync processes leave a complete restorable config", () => { 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);' ], { 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() }))); + if (outcomes.some(result => result.code !== 0)) throw new Error("concurrent sync failed: " + JSON.stringify(outcomes)); 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"); `); @@ -260,10 +260,8 @@ test.each(["already-off", "late-off"])("approved routing respects integration OF writeFileSync(join(process.env.OPENCODEX_HOME, "config.json"), JSON.stringify(config)); }; if (${JSON.stringify(timing)} === "already-off") disable(); - let calls = 0; const result = injectApprovedCodexProvider(id, standaloneCodexRoutingTarget(18281, {}), () => { - // The first guard belongs to stale-journal cleanup; the second is URL publication. - if (++calls === 2 && ${JSON.stringify(timing)} === "late-off") disable(); + 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"); @@ -272,3 +270,27 @@ test.each(["already-off", "late-off"])("approved routing respects integration OF 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(); } + `); +}); From 97a80456bca853ee74862dc5af7d8d16430c08d5 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 18:47:56 +0900 Subject: [PATCH 4/8] test(codex): verify retry-only sync rounds converge safely Concurrent fail-fast lock contenders can all return retryable conflicts. Assert that such a round preserves the preimage, then require one uncontended sync to publish before checking crash-restorable output. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- .../codex-provider-routing.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/codex-integration/codex-provider-routing.test.ts b/tests/codex-integration/codex-provider-routing.test.ts index c3cc8571d40..a935f5b373e 100644 --- a/tests/codex-integration/codex-provider-routing.test.ts +++ b/tests/codex-integration/codex-provider-routing.test.ts @@ -175,13 +175,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 outcomes = await Promise.all(jobs.map(async job => ({ code: await job.exited, error: await new Response(job.stderr).text() }))); + 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); From 8f80ef273cb5214766fdac22e8e3b9704364bdba Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 19:26:20 +0900 Subject: [PATCH 5/8] fix(codex): keep provider validation free of mutation metadata Only real approved-provider writes acquire the config mutation lock. Read-only validation and unapproved provider inspection retain the Codex file lock without creating a persistent config mutation database. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- src/codex/provider-routing.ts | 4 ++-- structure/codex-provider-routing.md | 2 +- tests/codex-integration/codex-provider-routing.test.ts | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/codex/provider-routing.ts b/src/codex/provider-routing.ts index 20a3b441639..c6b171ce3b5 100644 --- a/src/codex/provider-routing.ts +++ b/src/codex/provider-routing.ts @@ -125,8 +125,8 @@ 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 } { 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."); @@ -148,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 withConfigMutationLockSync(apply); + return validateOnly ? apply() : withConfigMutationLockSync(apply); }); } diff --git a/structure/codex-provider-routing.md b/structure/codex-provider-routing.md index d432d7a7dd3..4524ad2bf21 100644 --- a/structure/codex-provider-routing.md +++ b/structure/codex-provider-routing.md @@ -8,7 +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. 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. +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. 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/codex-integration/codex-provider-routing.test.ts b/tests/codex-integration/codex-provider-routing.test.ts index a935f5b373e..b575e47f73e 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, {}); From 94b11a44ca160943889486c04f2f19abf1a36b4f Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 19:45:16 +0900 Subject: [PATCH 6/8] fix(codex): witness configuration inode after lock acquisition A preceding writer can atomically replace config.toml and release between destination lookup and lock acquisition. Observe its committed inode under the acquired lock while retaining canonical-retarget and live-handle checks. Keep strict publication guards for changes after acquisition. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- src/codex/config-write-lock.ts | 17 ++++- structure/codex-home.md | 2 +- structure/codex-provider-routing.md | 2 +- .../codex-config-write-lock.test.ts | 62 ++++++++++++++++++- 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/codex/config-write-lock.ts b/src/codex/config-write-lock.ts index c29546853c6..a851bc940bd 100644 --- a/src/codex/config-write-lock.ts +++ b/src/codex/config-write-lock.ts @@ -96,8 +96,21 @@ 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 }; + } + 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/structure/codex-home.md b/structure/codex-home.md index 5df85bffe9a..d541ba24ab5 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 4524ad2bf21..3a7d7571d1b 100644 --- a/structure/codex-provider-routing.md +++ b/structure/codex-provider-routing.md @@ -8,7 +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. 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. +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/codex-integration/codex-config-write-lock.test.ts b/tests/codex-integration/codex-config-write-lock.test.ts index 182785c4204..9c6dd6ac6cc 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,58 @@ 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(); } +}); From fba19e6524e7a0c9cf3bb3650bb8dcc04a3c9d5b Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 19:46:46 +0900 Subject: [PATCH 7/8] fix(codex): reject unsafe entries during acquisition admission Validate regular-file identity while admitting the post-acquisition inode so a dangling symlink cannot be mistaken for a preceding writer deleting config.toml. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- src/codex/config-write-lock.ts | 1 + .../codex-config-write-lock.test.ts | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/codex/config-write-lock.ts b/src/codex/config-write-lock.ts index a851bc940bd..43e2608a04c 100644 --- a/src/codex/config-write-lock.ts +++ b/src/codex/config-write-lock.ts @@ -101,6 +101,7 @@ function acquire(configPath: string): ConfigWriteAcquireResult { // 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); + canonicalFileIdentity(current.canonical); if (current.canonical !== target.canonical || !stillHeld(acquired.handle)) { release(acquired.handle); return { ok: false, error: "unsafe", detail: acquired.handle.path }; diff --git a/tests/codex-integration/codex-config-write-lock.test.ts b/tests/codex-integration/codex-config-write-lock.test.ts index 9c6dd6ac6cc..8beb1a97446 100644 --- a/tests/codex-integration/codex-config-write-lock.test.ts +++ b/tests/codex-integration/codex-config-write-lock.test.ts @@ -1205,3 +1205,24 @@ test("acquisition refuses a canonical retarget before entering the write section 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("canonical configuration target is not the observed regular file"); + expect(entered).toBe(false); + expect(readFileSync(path + ".original", "utf8")).toBe('model = "a"\n'); + expect(existsSync(lockPath)).toBe(false); + } finally { spy.mockRestore(); } +}); From 6b1015e85ddd3d9b7776160f686684c6a2b56286 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 16 Oct 2026 19:56:36 +0900 Subject: [PATCH 8/8] fix(codex): preserve caller-specific unreadable config errors Keep post-acquisition inode and dangling-link validation without rejecting stable non-regular entries before prompt-layer consumers can return their existing config_unreadable refusal. Co-authored-by: xianlubird Co-authored-by: Matt Van Horn --- src/codex/config-write-lock.ts | 9 ++++++++- tests/codex-integration/codex-config-write-lock.test.ts | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/codex/config-write-lock.ts b/src/codex/config-write-lock.ts index 43e2608a04c..17c705c8ab8 100644 --- a/src/codex/config-write-lock.ts +++ b/src/codex/config-write-lock.ts @@ -101,11 +101,18 @@ function acquire(configPath: string): ConfigWriteAcquireResult { // 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); - canonicalFileIdentity(current.canonical); 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) { diff --git a/tests/codex-integration/codex-config-write-lock.test.ts b/tests/codex-integration/codex-config-write-lock.test.ts index 8beb1a97446..a22de718d95 100644 --- a/tests/codex-integration/codex-config-write-lock.test.ts +++ b/tests/codex-integration/codex-config-write-lock.test.ts @@ -1220,7 +1220,7 @@ test("acquisition does not admit a dangling symlink as a preceding writer's dele return acquire(file, deps); }); try { - expect(() => withConfigWriteLock(path, () => { entered = true; })).toThrow("canonical configuration target is not the observed regular file"); + 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);