From 7eb648620a0a4cdfa8bca708744f41b0777ead5d Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 3 Sep 2026 15:58:52 +0800 Subject: [PATCH 1/5] feat(config): audit persisted config mutations and attribute durable writers Rebased+squashed continuation of PR 2351 (config-mutation-audit). Adds write-ahead audit markers, redacted path/detail snapshots, exact persisted-bytes verification, principal-gated audit reads, and explicit ConfigMutationSource attribution for durable config writers. Also validates transientRetryOn5xx at the provider write boundary. --- src/cli/claude-desktop.ts | 21 +- src/cli/config-command.ts | 4 +- src/cli/dispatch.ts | 4 +- src/cli/index.ts | 2 +- src/cli/init.ts | 2 +- src/cli/models.ts | 4 +- src/cli/provider.ts | 10 +- src/cli/v2.ts | 4 +- src/client/state.ts | 6 +- src/codex/account-lifecycle.ts | 2 +- src/codex/auth-api.ts | 29 +- src/codex/convergence.ts | 2 +- src/codex/desired-state.ts | 25 +- src/codex/plan-from-token.ts | 2 +- src/codex/routing.ts | 16 +- src/config-mutation-audit.ts | 622 ++++++++ src/config.ts | 208 ++- src/lib/redact.ts | 6 +- src/oauth/index.ts | 2 +- src/oauth/login-cli.ts | 2 +- src/providers/api-keys.ts | 8 +- src/providers/key-failover.ts | 2 +- src/providers/key-store.ts | 10 +- src/server/auth-cors.ts | 5 + src/server/index.ts | 4 +- .../management/agent-settings-routes.ts | 31 +- src/server/management/combo-routes.ts | 4 +- src/server/management/config-routes.ts | 24 +- .../management/native-integration-routes.ts | 8 +- src/server/management/oauth-account-routes.ts | 24 +- src/server/management/provider-routes.ts | 16 +- .../management/routing-profile-routes.ts | 4 +- src/server/subagent-models-startup.ts | 11 +- src/storage/policy.ts | 21 +- structure/02_config-and-codex-home.md | 34 + structure/05_gui-and-management-api.md | 1 + tests/cli/cli-provider.test.ts | 8 + tests/config-mutation-audit-boundary.test.ts | 33 + tests/config-mutation-audit.test.ts | 1310 +++++++++++++++++ .../management-provider-validation.test.ts | 75 + tests/server/server-management-auth.test.ts | 14 + 41 files changed, 2474 insertions(+), 146 deletions(-) create mode 100644 src/config-mutation-audit.ts create mode 100644 tests/config-mutation-audit-boundary.test.ts create mode 100644 tests/config-mutation-audit.test.ts diff --git a/src/cli/claude-desktop.ts b/src/cli/claude-desktop.ts index 3d4ad70852..f58d889833 100644 --- a/src/cli/claude-desktop.ts +++ b/src/cli/claude-desktop.ts @@ -1,6 +1,6 @@ import { readFileSync, writeFileSync } from "node:fs"; import { resolve } from "node:path"; -import { loadConfig, saveConfigPreservingClaudeCode } from "../config"; +import { loadConfig, saveConfigPreservingClaudeCode, type ConfigMutationSource } from "../config"; import { setIntegrationEnabled } from "../codex/desired-state"; import { DESKTOP_FAMILIES, @@ -45,15 +45,17 @@ export async function applyProfile( profile: DesktopProfile, mode: Desktop3pConfigMode, deps: ApplyProfileDeps = {}, + source?: ConfigMutationSource, ): Promise<{ ok: boolean; path: string; reason?: string; warning?: string }> { // Explicit apply is an enable action. Persist intent before any Desktop write // so a process crash cannot leave a gateway profile that startup immediately removes. - const desired = setIntegrationEnabled("claude-desktop", true); + const applySource = source ?? { surface: "cli", detail: "ocx claude desktop apply" }; + const desired = setIntegrationEnabled("claude-desktop", true, applySource); if (!desired.ok) return { ok: false, path: "", reason: desired.message }; const config = loadConfig(); const state = await buildClaudeDesktopState(config, profile); config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: state.profile }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, applySource); const live = await (deps.findLiveProxyImpl ?? findLiveProxy)(); if (live) { // #859: the Desktop alias reverse-map is process-local. Applying through the @@ -195,7 +197,7 @@ export async function handleClaudeDesktopCommand(argv: string[], deps: ApplyProf if (!state.models.some(model => model.route === route && model.available)) throw new Error(`현재 사용할 수 없는 모델입니다: ${route}`); const profile = moveDesktopRoute(state.profile, route, familyRaw, flags.includes("--default")); config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: profile }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "cli", detail: "ocx claude desktop move" }); console.log(`${route} 모델을 ${familyRaw} 그룹으로 옮겼습니다.`); return 0; } @@ -206,7 +208,7 @@ export async function handleClaudeDesktopCommand(argv: string[], deps: ApplyProf if (route && !state.models.some(model => model.route === route && model.available)) throw new Error(`현재 사용할 수 없는 모델입니다: ${route}`); const profile = setDesktopFamilyDefault(state.profile, familyRaw, route); config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: profile }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "cli", detail: "ocx claude desktop default" }); console.log(`${familyRaw} 기본 모델을 ${route ?? "없음"}으로 지정했습니다.`); return 0; } @@ -225,9 +227,14 @@ export async function handleClaudeDesktopCommand(argv: string[], deps: ApplyProf const profile = parseDesktopProfile(JSON.parse(readFileSync(resolve(source), "utf8"))); const reconciled = (await buildClaudeDesktopState(config, profile)).profile; config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: reconciled }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "cli", detail: "ocx claude desktop import" }); if (flags.includes("--apply")) { - const result = await applyProfile(reconciled, "static", deps); + const result = await applyProfile( + reconciled, + "static", + deps, + { surface: "cli", detail: "ocx claude desktop import" }, + ); if (!result.ok) { console.error(`프로필은 저장했지만 Desktop 적용에 실패했습니다: ${result.reason ?? "unknown error"}`); return 1; } if (result.warning) console.warn(`⚠️ ${result.warning}`); } diff --git a/src/cli/config-command.ts b/src/cli/config-command.ts index cd33fefd74..6514547438 100644 --- a/src/cli/config-command.ts +++ b/src/cli/config-command.ts @@ -168,7 +168,7 @@ export async function handleConfigCommand(argv: string[]): Promise { } Object.assign(fresh, config); return { changed: JSON.stringify(fresh) !== before, value: undefined }; - }); + }, { surface: "cli", detail: `ocx config ${action}` }); if (outcome.status === "unavailable") { throw new Error(outcome.reason === "conflict" ? "config changed while applying this update; retry" @@ -206,7 +206,7 @@ export async function handleConfigCommand(argv: string[]): Promise { if (!path) throw new CliUsageError("import path is required", USAGE); if (!yes) throw new CliUsageError("import requires --yes", USAGE); rejectArgs(args, USAGE); - saveConfig(validate(loadInput(path))); + saveConfig(validate(loadInput(path)), { surface: "cli", detail: "ocx config import" }); printData({ ok: true, source: path }, wantsJson, [`Imported config from ${path}. Restart or run ocx sync if needed.`]); return; } diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 48a1f44be3..e5f0cdea73 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -97,7 +97,7 @@ const commandRunners: Record = { if (!live) { return emitBack(false, "No running proxy found. Run 'ocx start' — it injects opencodex automatically.", 1); } - const desired = setIntegrationEnabled("codex", true); + const desired = setIntegrationEnabled("codex", true, { surface: "cli", detail: "ocx restore back" }); if (!desired.ok) { return emitBack(false, `Codex desired state was not saved (${desired.reason}).`, desired.reason === "conflict" ? 2 : 1); } @@ -111,7 +111,7 @@ const commandRunners: Record = { const target = collectOrcaCodexHomeDiagnostic(); return emitBack(true, `Plain \`codex\` now routes through opencodex in ${target.effectiveCodexHome} (undo with: ocx restore).`, 0); } - const desired = setIntegrationEnabled("codex", false); + const desired = setIntegrationEnabled("codex", false, { surface: "cli", detail: "ocx restore" }); if (!desired.ok) { if (restoreJson) { // Machine-readable contract: every restore --json outcome emits one diff --git a/src/cli/index.ts b/src/cli/index.ts index 06478ba3a3..c4a6f83cb9 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -215,7 +215,7 @@ async function chooseListenPort( } if (shouldPersistSelectedPort(config.port, selected, preferred, options)) { config.port = selected; - saveConfig(config); + saveConfig(config, { surface: "cli", detail: "ocx start (port selection)" }); } return selected; } catch (err) { diff --git a/src/cli/init.ts b/src/cli/init.ts index 72ad3c1b70..2426a930c1 100644 --- a/src/cli/init.ts +++ b/src/cli/init.ts @@ -171,7 +171,7 @@ export async function runInit(): Promise { modelDiscovery: { newModelPolicy: "off" }, }; - saveConfig(config); + saveConfig(config, { surface: "cli", detail: "ocx init" }); // Init writes a fresh config, so a stale pre-migration backup from a previous // installation would make the next `ocx start` crash on a stale-backup // collision (issue #257). But only a STALE backup (unparseable, or already a diff --git a/src/cli/models.ts b/src/cli/models.ts index 0f91796408..194d5e5a6a 100644 --- a/src/cli/models.ts +++ b/src/cli/models.ts @@ -256,7 +256,7 @@ async function handleCustomAdd(args: string[]): Promise { addedAt: new Date().toISOString(), }; config.customModels = [...existing, entry]; - saveConfig(config); + saveConfig(config, { surface: "cli", detail: "ocx models add" }); await syncCustomModelsIfLive(); console.log(`Added custom model ${slug} (${entry.id}).`); } @@ -325,7 +325,7 @@ async function handleCustomRemove(args: string[]): Promise { const next = existing.filter((_, modelIndex) => modelIndex !== index); config.customModels = next.length > 0 ? next : undefined; - saveConfig(config); + saveConfig(config, { surface: "cli", detail: "ocx models remove" }); await syncCustomModelsIfLive(); console.log(`Removed custom model ${routedSlug(model.provider, model.modelId)}.`); } diff --git a/src/cli/provider.ts b/src/cli/provider.ts index 6795b3db52..43b1fe14fe 100644 --- a/src/cli/provider.ts +++ b/src/cli/provider.ts @@ -61,7 +61,7 @@ function maskSecret(value: string): string { // Validation helper (F1 fix: validate before saveConfig) // --------------------------------------------------------------------------- -function validateAndSave(config: ReturnType): void { +function validateAndSave(config: ReturnType, detail = "ocx provider set"): void { if (!config.providers || Object.keys(config.providers).length === 0) { console.error("Error: config would have no providers. Aborting."); process.exit(1); @@ -70,7 +70,7 @@ function validateAndSave(config: ReturnType): void { console.error(`Error: defaultProvider "${config.defaultProvider}" does not exist in providers. Aborting.`); process.exit(1); } - saveConfig(config); + saveConfig(config, { surface: "cli", detail }); } // --------------------------------------------------------------------------- @@ -225,7 +225,7 @@ async function handleAdd(args: string[]): Promise { if (allowPrivateNetwork) provConfig.allowPrivateNetwork = true; if (setDefault) config.defaultProvider = name; - validateAndSave(config); + validateAndSave(config, "ocx provider add"); if (wantsJson) { console.log(JSON.stringify({ @@ -317,7 +317,7 @@ function handleRemove(args: string[]): void { delete config.providers[name]; const droppedCustomModels = dropProviderCustomModels(config, name); - validateAndSave(config); + validateAndSave(config, "ocx provider remove"); if (wantsJson) { @@ -411,7 +411,7 @@ function handleSetDefault(args: string[]): void { } config.defaultProvider = name; - validateAndSave(config); + validateAndSave(config, "ocx provider set-default"); if (wantsJson) { diff --git a/src/cli/v2.ts b/src/cli/v2.ts index b1e63b0c5c..fab0d4872e 100644 --- a/src/cli/v2.ts +++ b/src/cli/v2.ts @@ -205,7 +205,7 @@ export async function cmdV2(args: string[], deps: V2CliDeps = {}, findPort?: () } if (modeArg === "default") deleteConfigTopLevelKey(cfg, "multiAgentMode"); else cfg.multiAgentMode = modeArg as "v1" | "v2"; - saveConfig(cfg); + saveConfig(cfg, { surface: "cli", detail: "ocx v2 mode" }); try { const sync = deps.sync ?? (await import("../codex/sync")).syncModelsToCodex; await sync(findPort ? await findPort() : undefined); @@ -235,7 +235,7 @@ export async function cmdV2(args: string[], deps: V2CliDeps = {}, findPort?: () } if (next) cfg.keepNativeChatGptOnV1 = true; else deleteConfigTopLevelKey(cfg, "keepNativeChatGptOnV1"); - saveConfig(cfg); + saveConfig(cfg, { surface: "cli", detail: "ocx v2 keep-native-v1" }); try { const sync = deps.sync ?? (await import("../codex/sync")).syncModelsToCodex; await sync(findPort ? await findPort() : undefined); diff --git a/src/client/state.ts b/src/client/state.ts index 4711586d09..d5810efe3a 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -139,7 +139,7 @@ export function commitClientConnection( config.client = structuredClone(state); } return { changed: !unchanged, value: undefined }; - }); + }, { surface: "cli", detail: "ocx connect: commit client connection" }); if (outcome.status === "committed" || outcome.status === "unchanged") return outcome.status; if (outcome.status === "unavailable" && outcome.reason === "missing") { // First ocx run on a fresh machine: ocx connect is the expected first command in @@ -150,7 +150,7 @@ export function commitClientConnection( const seeded = getDefaultConfig(); seeded.runtimeRole = "client"; seeded.client = structuredClone(state); - saveConfig(seeded); + saveConfig(seeded, { surface: "cli", detail: "ocx connect: commit client connection" }); return "committed"; } throw new Error(`client state commit unavailable: ${"reason" in outcome ? outcome.reason : "unknown"}`); @@ -169,7 +169,7 @@ export function clearClientConnection( deleteConfigTopLevelKey(config, "client"); deleteConfigTopLevelKey(config, "runtimeRole"); return { changed: true, value: "committed" as const }; - }); + }, { surface: "cli", detail: "ocx disconnect: clear client connection" }); if (outcome.status === "unavailable") return "conflict"; return outcome.value; } diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 75f748a805..0098fb97c4 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -158,7 +158,7 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): try { // Persist first for durable configs. Destructive cleanup below must never run for a // deletion that failed to commit. Transient configs intentionally skip this write. - saveConfigPreservingClaudeCode(runtimeConfig); + saveConfigPreservingClaudeCode(runtimeConfig, { surface: "internal", detail: "account lifecycle: remove account" }); } catch (error) { restoreRuntimeConfig(runtimeConfig, previousConfig); try { diff --git a/src/codex/auth-api.ts b/src/codex/auth-api.ts index 865711ba86..a17eba7fd6 100644 --- a/src/codex/auth-api.ts +++ b/src/codex/auth-api.ts @@ -4,6 +4,7 @@ import { mutatePersistedConfig, saveConfigPreservingClaudeCode, withConfigMutationLockSync, + type ConfigMutationSource, } from "../config"; import { codexAccountLogLabel, withCodexAccountLogLabel } from "./account-label"; import { @@ -609,8 +610,8 @@ function getRuntimeConfig(config: OcxConfig): OcxConfig { return isRuntimeConfig(config) ? config : loadConfig(); } -function saveRuntimeConfig(sourceConfig: OcxConfig, nextConfig: OcxConfig): void { - saveConfigPreservingClaudeCode(nextConfig); +function saveRuntimeConfig(sourceConfig: OcxConfig, nextConfig: OcxConfig, source: ConfigMutationSource): void { + saveConfigPreservingClaudeCode(nextConfig, source); if (sourceConfig === nextConfig || !isRuntimeConfig(sourceConfig)) return; for (const key of Object.keys(sourceConfig) as Array) { delete sourceConfig[key]; @@ -663,7 +664,7 @@ function persistNewCodexAccount( const namespaceAdded = tracksPickerNamespaces && appendDefaultCodexAccountNamespace(runtimeConfig, addedAccount); pickerVisibilityChanged = namespaceAdded || retainedPickerBindingRestored; - saveRuntimeConfig(sourceConfig, runtimeConfig); + saveRuntimeConfig(sourceConfig, runtimeConfig, { surface: "internal", detail: "codex-auth: persist new codex account" }); } catch (error) { for (const key of Object.keys(runtimeConfig) as Array) { delete runtimeConfig[key]; @@ -1104,7 +1105,7 @@ function reconcileFreshPoolAccountPlans(runtimeConfig: OcxConfig, updates: Fresh } } return { changed, value: accepted }; - }); + }, { surface: "internal", detail: "wham: reconcile fresh pool plan observation" }); } catch (error) { // Plan persistence is derived metadata on a read route. Contention must fail closed without // turning account listing into a 500; a later refresh can retry against the latest files. @@ -1920,7 +1921,7 @@ export async function handleCodexAuthAPI( return jsonResponse({ error: "Invalid account id format" }, 400); } const pickerVisibilityChanged = deleteCodexAccount(runtimeConfig, id); - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "DELETE /api/codex-auth/accounts" }); reconcileLiveStateStores(); const catalogRefresh = await convergeAccountNamespaceCatalog( runtimeConfig, @@ -1944,7 +1945,7 @@ export async function handleCodexAuthAPI( if (!account) return jsonResponse({ error: "Account not found" }, 404); if (alias) account.alias = alias; else delete account.alias; - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/accounts/alias" }); return jsonResponse({ ok: true, id, alias: alias || null }); } @@ -1966,7 +1967,7 @@ export async function handleCodexAuthAPI( clearThreadAccountMapForAccount(id); selectFallbackAfterPause(runtimeConfig, id); } - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/accounts/pause" }); return jsonResponse({ ok: true, id, @@ -2014,7 +2015,7 @@ export async function handleCodexAuthAPI( // account switch — would outrank the order forever: it blocks preemption and caps // every eligibility list at its own tier until that account drains or is paused. clearCodexAccountPin(runtimeConfig); - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/accounts/priority" }); return jsonResponse({ ok: true, id, @@ -2027,7 +2028,7 @@ export async function handleCodexAuthAPI( const runtimeConfig = getRuntimeConfig(config); const result = await pauseExhaustedCodexAccounts( runtimeConfig, - () => saveRuntimeConfig(config, runtimeConfig), + () => saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/accounts/pause-exhausted" }), ); const { pausedAccountIds, checkedAccountCount, failedAccountCount } = result; if (checkedAccountCount === 0 && failedAccountCount > 0) { @@ -2095,7 +2096,7 @@ export async function handleCodexAuthAPI( if (body.accountId == null) clearCodexAccountPin(runtimeConfig); else setCodexAccountPin(runtimeConfig, targetAccountId); resetCodexRoutingForManualSelection(targetAccountId); - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/active" }); return jsonResponse({ ok: true, activeCodexAccountId: body.accountId, appliesImmediately: true }); } @@ -2125,7 +2126,7 @@ export async function handleCodexAuthAPI( } const runtimeConfig = getRuntimeConfig(config); runtimeConfig.autoSwitchThreshold = body.threshold; - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/auto-switch" }); return jsonResponse({ ok: true }); } @@ -2161,7 +2162,7 @@ export async function handleCodexAuthAPI( } if (nextStrategy !== undefined) runtimeConfig.accountPoolStrategy = nextStrategy; if (nextSticky !== undefined) runtimeConfig.accountPoolStickyLimit = nextSticky; - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: `${req.method} /api/codex-auth/pool-strategy` }); return jsonResponse({ ok: true, accountPoolStrategy: normalizeAccountPoolStrategy(runtimeConfig.accountPoolStrategy), @@ -2177,7 +2178,7 @@ export async function handleCodexAuthAPI( } const runtimeConfig = getRuntimeConfig(config); runtimeConfig.upstreamFailoverThreshold = body.threshold; - saveRuntimeConfig(config, runtimeConfig); + saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "PUT /api/codex-auth/failover" }); return jsonResponse({ ok: true }); } @@ -2583,7 +2584,7 @@ export async function handleCodexAuthAPI( isMain: false, }, accounts); latestConfig.codexAccounts = accounts; - saveRuntimeConfig(config, latestConfig); + saveRuntimeConfig(config, latestConfig, { surface: "internal", detail: "codex-auth: refresh account metadata after login/reauth" }); } else { const addedAccount = withCodexAccountLogLabel({ id: accountId, email, plan, isMain: false }, accounts); newAccountPersistence = persistNewCodexAccount( diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index df765a7853..a854041ceb 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -671,7 +671,7 @@ export async function convergeCodexCatalog( const mutable = snapshot.config as OcxConfig; mutable.modelDiscovery = state.discoveryConfig.modelDiscovery; mutable.disabledModels = state.discoveryConfig.disabledModels; - saveConfigPreservingClaudeCode(mutable); + saveConfigPreservingClaudeCode(mutable, { surface: "internal", detail: "catalog: persist converged discovery config" }); } return { changed: committed.kind === "committed" ? committed.changed : false, diff --git a/src/codex/desired-state.ts b/src/codex/desired-state.ts index d8ca52a84c..9abcb0efcd 100644 --- a/src/codex/desired-state.ts +++ b/src/codex/desired-state.ts @@ -21,6 +21,7 @@ * Design record: devlog/_fin/260803_codex_desktop_toggle/030_desired_state.md. */ import { deleteConfigTopLevelKey, loadConfig, mutatePersistedConfig } from "../config"; +import type { ConfigMutationSource } from "../config"; import type { OcxClientIntegrationsConfig, OcxConfig } from "../types"; import { runStartupReadinessSync, type ReadinessGate, type SyncOutcomeLike } from "../server/readiness"; @@ -116,6 +117,7 @@ export function codexIntegrationEnabledNow(): boolean { export function setIntegrationEnabled( client: DurableIntentClientId, enabled: boolean, + source: ConfigMutationSource = { surface: "internal", detail: "desired-state: setIntegrationEnabled" }, ): CodexDesiredStateResult { const outcome = mutatePersistedConfig(config => { const current = integrationEnabled(config, client); @@ -134,7 +136,7 @@ export function setIntegrationEnabled( if (Object.keys(integrations).length === 0) deleteConfigTopLevelKey(config, "clientIntegrations"); else config.clientIntegrations = integrations; return { changed: true, value: enabled }; - }); + }, source); if (outcome.status !== "unavailable") { return { ok: true, status: outcome.status, enabled }; @@ -156,12 +158,18 @@ export function setIntegrationEnabled( }; } -export function setCodexIntegrationEnabled(enabled: boolean): CodexDesiredStateResult { - return setIntegrationEnabled("codex", enabled); +export function setCodexIntegrationEnabled( + enabled: boolean, + source?: ConfigMutationSource, +): CodexDesiredStateResult { + return setIntegrationEnabled("codex", enabled, source); } -export function setGrokIntegrationEnabled(enabled: boolean): CodexDesiredStateResult { - return setIntegrationEnabled("grok", enabled); +export function setGrokIntegrationEnabled( + enabled: boolean, + source?: ConfigMutationSource, +): CodexDesiredStateResult { + return setIntegrationEnabled("grok", enabled, source); } /** Whether Claude Desktop's managed gateway profile is wanted. */ @@ -174,8 +182,11 @@ export function claudeDesktopIntegrationEnabledNow(): boolean { return claudeDesktopIntegrationEnabled(loadConfig()); } -export function setClaudeDesktopIntegrationEnabled(enabled: boolean): CodexDesiredStateResult { - return setIntegrationEnabled("claude-desktop", enabled); +export function setClaudeDesktopIntegrationEnabled( + enabled: boolean, + source?: ConfigMutationSource, +): CodexDesiredStateResult { + return setIntegrationEnabled("claude-desktop", enabled, source); } /** diff --git a/src/codex/plan-from-token.ts b/src/codex/plan-from-token.ts index be2585ec4e..73b2c1183e 100644 --- a/src/codex/plan-from-token.ts +++ b/src/codex/plan-from-token.ts @@ -84,7 +84,7 @@ function persistJwtPlanUpdates(runtimeConfig: OcxConfig, updates: FreshPoolPlanU } } return { changed, value: accepted }; - }); + }, { surface: "internal", detail: "wham: jwt plan updates" }); } catch (error) { if (error instanceof ConfigMutationLockError) return; throw error; diff --git a/src/codex/routing.ts b/src/codex/routing.ts index dbf9cab086..a4fbd016a0 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -1531,10 +1531,16 @@ function releaseCodexAccountPinFor(config: OcxConfig, accountId: string): boolea /** Persist operator (or quota-strategy) active selection to config + disk. */ function setActiveCodexAccount(config: OcxConfig, accountId: string): void { runtimeActiveCodexAccountId = undefined; + const activeAccountChanged = config.activeCodexAccountId !== accountId; const releasedPin = releaseCodexAccountPinFor(config, accountId); - if (config.activeCodexAccountId === accountId && !releasedPin) return; + if (!activeAccountChanged && !releasedPin) return; config.activeCodexAccountId = accountId; - saveConfigPreservingClaudeCode(config); + const detail = activeAccountChanged + ? releasedPin + ? "routing: active codex account selection and pin clear" + : "routing: active codex account selection" + : "routing: clear codex account pin"; + saveConfigPreservingClaudeCode(config, { surface: "internal", detail }); } /** Quota strategy persists; RR/fill-first keep a process-local cursor only. */ @@ -1626,7 +1632,7 @@ function pickPriorityPreemption( * on its own. Clearing the pin also removes the condition, so this writes at * most once per pin. */ -function releaseDrainedCodexAccountPin( +export function releaseDrainedCodexAccountPin( config: OcxConfig, selectionOptions?: Pick< CodexAccountUsabilityOptions, @@ -1639,7 +1645,7 @@ function releaseDrainedCodexAccountPin( const knownUnavailable = isAccountNeedsReauth(pinned) || isCodexAccountPaused(config, pinned); if (knownUnavailable) { clearCodexAccountPin(config); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "internal", detail: "routing: clear unavailable codex account pin" }); return; } // Temporary drain deliberately forbids every native-main read. A pin on main @@ -1650,7 +1656,7 @@ function releaseDrainedCodexAccountPin( || !hasCodexQuotaHeadroom(config, pinned, selectionOptions, now); if (!drained) return; clearCodexAccountPin(config); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "internal", detail: "routing: clear drained codex account pin" }); } function applyQuotaAutoSwitch( diff --git a/src/config-mutation-audit.ts b/src/config-mutation-audit.ts new file mode 100644 index 0000000000..3e4c870f03 --- /dev/null +++ b/src/config-mutation-audit.ts @@ -0,0 +1,622 @@ +/** + * Config mutation audit leaf: durable SQLite trail plus write-ahead recovery for + * persisted config mutations. + * + * This module intentionally does NOT import src/config.ts (or routing/server code): + * config.ts owns the save orchestration and passes the resolved config dir/path, the + * atomic-write function, and the open mutation transaction handle into this leaf, so + * the two boundaries stay acyclic and the pure diff/redaction logic is directly + * testable without loading the whole config stack. + */ +import { createHash } from "node:crypto"; +import { closeSync, existsSync, fsyncSync, openSync, readFileSync, readdirSync, unlinkSync } from "node:fs"; +import { join } from "node:path"; +import { Database } from "bun:sqlite"; +import { REDACTED_SECRET, redactSecretString, redactSecrets } from "./lib/redact"; + +export const CONFIG_MUTATION_DB_FILENAME = "config-mutation.sqlite"; +/** Per-mutation write-ahead marker prefix; each marker is `config-mutation-pending-.json`. */ +export const CONFIG_MUTATION_PENDING_AUDIT_FILENAME = "config-mutation-pending-"; +const CONFIG_MUTATION_PENDING_AUDIT_SUFFIX = ".json"; + +/** + * Who performed a persisted config mutation. `surface` separates the two human-facing + * entry points (management API vs CLI) from internal/automatic writers so an operator + * can tell a GUI edit from a background migration at a glance. + */ +export interface ConfigMutationSource { + readonly surface: "cli" | "api" | "internal"; + /** Human-readable route or command, e.g. "PUT /api/providers/blsc" or "ocx provider set". */ + readonly detail: string; +} + +export interface ConfigMutationAuditRow { + id: number; + mutationId: string; + createdAt: number; + surface: "cli" | "api" | "internal"; + detail: string; + fields: string[]; + before: Record; + after: Record; +} + +/** Bounded audit retention: newest N rows survive each insert; older rows are pruned. */ +export const CONFIG_AUDIT_MAX_ROWS = 5_000; +let configAuditMaxRows = CONFIG_AUDIT_MAX_ROWS; + +/** Test-only seam: shrink the audit retention bound without building a 5k-row fixture. */ +export function setConfigAuditMaxRowsForTests(value: number | null): void { + configAuditMaxRows = value ?? CONFIG_AUDIT_MAX_ROWS; +} +/** + * Test-only seam: run code after the read-path recovery snapshots the marker + * paths but before it deletes reconciled markers, so a concurrent-writer window + * can be exercised deterministically. + */ +let reconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests: (() => void) | null = null; +export function setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests(hook: (() => void) | null): void { + reconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests = hook; +} +/** A single changed-field path list is capped so a wholesale rewrite cannot bloat the row. */ +const CONFIG_AUDIT_MAX_FIELDS = 64; +/** Redacted before/after values are capped per entry; longer values are truncated. */ +const CONFIG_AUDIT_MAX_VALUE_CHARS = 4_096; +/** Route/command detail is capped so a long CLI line cannot bloat the row or marker. */ +const CONFIG_AUDIT_MAX_DETAIL_CHARS = 512; +/** A single redacted field-label segment is capped; longer keys are truncated. */ +const CONFIG_AUDIT_MAX_LABEL_CHARS = 256; +/** + * Durable write-ahead marker for one config write. Written (atomically) BEFORE the + * config.json rename and removed AFTER the audit row commits, so a process crash + * between the rename and the commit can be replayed instead of leaving a changed + * config with no audit record. `mutationId` also names the marker file, so recovery + * can only ever delete the exact marker it reconciled. + */ +export type PendingConfigMutationAudit = { + mutationId: string; + createdAt: number; + surface: ConfigMutationSource["surface"]; + detail: string; + fields: string[]; + before: Record; + after: Record; + /** SHA-256 of the exact config.json bytes the pending write produced. */ + afterSha256: string; +}; + +/** Marker path under the config dir for one mutation (read/write side). */ +export function configMutationPendingAuditPath(configDir: string, mutationId: string): string { + return join(configDir, `${CONFIG_MUTATION_PENDING_AUDIT_FILENAME}${mutationId}${CONFIG_MUTATION_PENDING_AUDIT_SUFFIX}`); +} + +/** Every pending marker currently on disk (deterministic filename order). */ +export function listPendingConfigMutationAuditPaths(configDir: string): string[] { + try { + return readdirSync(configDir) + .filter(name => name.startsWith(CONFIG_MUTATION_PENDING_AUDIT_FILENAME) && name.endsWith(CONFIG_MUTATION_PENDING_AUDIT_SUFFIX)) + .sort() + .map(name => join(configDir, name)); + } catch { + return []; + } +} + +/** Read-only DB path with no directory creation or ACL side effects. */ +export function configMutationDatabasePathForRead(configDir: string): string { + return join(configDir, CONFIG_MUTATION_DB_FILENAME); +} + +const CONFIG_MUTATION_AUDIT_TABLE_SQL = ` + CREATE TABLE IF NOT EXISTS config_mutation_audit ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + mutation_id TEXT NOT NULL UNIQUE, + created_at INTEGER NOT NULL, + surface TEXT NOT NULL, + detail TEXT NOT NULL, + fields TEXT NOT NULL, + before_json TEXT NOT NULL, + after_json TEXT NOT NULL + ) +`; + +export function ensureConfigMutationAuditTable(database: Database): void { + database.exec(CONFIG_MUTATION_AUDIT_TABLE_SQL); + // CREATE TABLE IF NOT EXISTS cannot add the constraint to an existing database, + // so enforce uniqueness idempotently for pre-existing audit stores as well. + database.exec( + "CREATE UNIQUE INDEX IF NOT EXISTS config_mutation_audit_mutation_id ON config_mutation_audit(mutation_id)", + ); +} + +function boundAuditDetail(detail: string): string { + return detail.length <= CONFIG_AUDIT_MAX_DETAIL_CHARS + ? detail + : `${detail.slice(0, Math.max(0, CONFIG_AUDIT_MAX_DETAIL_CHARS - "…[truncated]".length))}…[truncated]`; +} + +const CONFIG_AUDIT_TRUNCATION_SUFFIX = "…[truncated]"; + +function boundAuditLabelToLength(label: string, maxLength: number): string { + return label.length <= maxLength + ? label + : `${label.slice(0, Math.max(0, maxLength - CONFIG_AUDIT_TRUNCATION_SUFFIX.length))}${CONFIG_AUDIT_TRUNCATION_SUFFIX}`; +} + +function boundAuditLabel(label: string): string { + return boundAuditLabelToLength(label, CONFIG_AUDIT_MAX_LABEL_CHARS); +} + +/** Bound a duplicate label while reserving room for its occurrence suffix. */ +function boundAuditLabelWithOccurrence(label: string, seen: number): string { + const suffix = `#${seen}`; + return `${boundAuditLabelToLength(label, CONFIG_AUDIT_MAX_LABEL_CHARS - suffix.length)}${suffix}`; +} + +/** + * Insert one audit row deduped by the per-write mutation id, then prune retention. + * Used by the live write path and by crash recovery, so a replayed marker can never + * duplicate the row it already committed, while two distinct same-millisecond writes + * are never coalesced. + */ +export function insertConfigMutationAuditRow( + database: Database, + mutationId: string, + createdAt: number, + source: Pick, + fields: string[], + before: Record, + after: Record, +): void { + ensureConfigMutationAuditTable(database); + // Labels are already bounded and de-duplicated by buildConfigMutationSnapshot + // (and markers store that final form); inserting must not re-transform them or + // the persisted fields and the before/after keys can diverge. + const fieldsJson = JSON.stringify(fields); + const detail = boundAuditDetail(redactSecretString(source.detail)); + const beforeJson = JSON.stringify(before); + const afterJson = JSON.stringify(after); + const existing = database.prepare(` + SELECT 1 FROM config_mutation_audit WHERE mutation_id = ? LIMIT 1 + `).get(mutationId); + if (existing) return; + database.prepare(` + INSERT OR IGNORE INTO config_mutation_audit (mutation_id, created_at, surface, detail, fields, before_json, after_json) + VALUES (?, ?, ?, ?, ?, ?, ?) + `).run(mutationId, createdAt, source.surface, detail, fieldsJson, beforeJson, afterJson); + // Keep the newest CONFIG_AUDIT_MAX_ROWS rows: delete every row at or below the id of + // the (N+1)-th newest entry. COALESCE keeps a small table a no-op. + database.prepare(` + DELETE FROM config_mutation_audit + WHERE id <= COALESCE(( + SELECT id FROM config_mutation_audit ORDER BY id DESC LIMIT 1 OFFSET ? + ), 0) + `).run(configAuditMaxRows); +} + +/** + * Atomically persist the write-ahead marker for one mutation id, then fsync the + * directory as a best-effort ordering aid. This narrows the process-crash window; it + * is not a power-loss durability guarantee because the config temp file itself is not + * fsynced before its rename. + */ +export function writePendingConfigMutationAudit( + payload: PendingConfigMutationAudit, + configDir: string, + atomicWriteFile: (path: string, content: string) => void, +): void { + const path = configMutationPendingAuditPath(configDir, payload.mutationId); + atomicWriteFile(path, JSON.stringify({ ...payload, detail: boundAuditDetail(redactSecretString(payload.detail)) })); + try { + const dir = openSync(configDir, "r"); + try { fsyncSync(dir); } finally { closeSync(dir); } + } catch { /* best-effort */ } +} + +export function deletePendingConfigMutationAudit(configDir: string, mutationId: string): void { + deletePendingConfigMutationAuditAtPath(configMutationPendingAuditPath(configDir, mutationId)); +} + +/** Unlink one exact marker path; recovery uses this so a newer writer's marker is never removed. */ +export function deletePendingConfigMutationAuditAtPath(markerPath: string): void { + try { unlinkSync(markerPath); } catch (error) { + if (!isMissingPathError(error)) throw error; + } +} + +function readPendingConfigMutationAuditAtPath(path: string): PendingConfigMutationAudit | null { + let parsed: unknown; + try { + parsed = JSON.parse( + readFileSync(path, "utf8"), + ); + } catch (error) { + if (!isMissingPathError(error)) { + // A malformed marker must not block future writes; drop it and continue. + try { unlinkSync(path); } catch { /* best-effort */ } + } + return null; + } + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + // A JSON null/array root can never be a valid marker; drop it so recovery + // does not throw or reprocess it on every read. + try { unlinkSync(path); } catch { /* best-effort */ } + return null; + } + const marker = parsed as Partial; + const valid = + typeof marker.mutationId === "string" && marker.mutationId.length > 0 && + typeof marker.createdAt === "number" && Number.isSafeInteger(marker.createdAt) && + (marker.surface === "cli" || marker.surface === "api" || marker.surface === "internal") && + typeof marker.detail === "string" && + Array.isArray(marker.fields) && marker.fields.every(field => typeof field === "string") && + typeof marker.afterSha256 === "string" && marker.afterSha256.length > 0 && + !!marker.before && typeof marker.before === "object" && !Array.isArray(marker.before) && + !!marker.after && typeof marker.after === "object" && !Array.isArray(marker.after); + if (!valid) { + // A parseable-but-invalid marker can never be reconciled; drop it so + // recovery does not reprocess it on every read or write. Valid markers + // whose hashes mismatch stay intact: they may belong to an in-flight write. + try { unlinkSync(path); } catch { /* best-effort */ } + return null; + } + return marker as PendingConfigMutationAudit; +} + +export function readPendingConfigMutationAudit(configDir: string, mutationId: string): PendingConfigMutationAudit | null { + return readPendingConfigMutationAuditAtPath(configMutationPendingAuditPath(configDir, mutationId)); +} + +/** + * Reconcile one interrupted write inside an OPEN writable transaction: if the config + * bytes on disk match the pending marker, the rename landed before the crash, so the + * audit row is replayed (deduped by mutation id). Otherwise the rename never landed + * (or the marker belongs to a writer whose rename has not landed yet). + * + * Returns true only when the marker is fully resolved and safe to delete after the + * caller's transaction commits. A hash-mismatching marker may belong to an in-flight + * writer between the marker write and the config rename, so read-side recovery must + * retain it; write-side recovery (exclusively under `withConfigMutationLockSync`) + * decides stale markers and removes them after its commit. + */ +export function reconcilePendingConfigMutationAudit( + database: Database, + markerPath: string, + configPath: string, +): boolean { + const pending = readPendingConfigMutationAuditAtPath(markerPath); + if (!pending) return false; + let currentHash: string | null = null; + try { + currentHash = createHash("sha256").update(readFileSync(configPath)).digest("hex"); + } catch (error) { + if (!isMissingPathError(error)) throw error; + } + if (currentHash === pending.afterSha256) { + insertConfigMutationAuditRow( + database, + pending.mutationId, + pending.createdAt, + pending, + pending.fields, + pending.before, + pending.after, + ); + return true; + } + return false; +} + +/** + * Record the row described by one marker inside the CURRENT open transaction. Returns + * true when the caller should delete that exact marker after its transaction commits. + */ +export function recordPendingConfigMutationAuditNow( + database: Database, + configDir: string, + mutationId: string, +): boolean { + const pending = readPendingConfigMutationAudit(configDir, mutationId); + if (!pending) return false; + insertConfigMutationAuditRow( + database, + pending.mutationId, + pending.createdAt, + pending, + pending.fields, + pending.before, + pending.after, + ); + return true; +} + +/** Best-effort read-path recovery: replay orphaned markers when the DB already exists. */ +export function reconcilePendingConfigMutationAuditOnRead( + databasePath: string, + configDir: string, + configPath: string, +): void { + const markerPaths = listPendingConfigMutationAuditPaths(configDir); + if (markerPaths.length === 0) return; + const cleanupPaths: string[] = []; + try { + const writable = new Database(databasePath); + try { + for (const markerPath of markerPaths) { + if (reconcilePendingConfigMutationAudit(writable, markerPath, configPath)) { + cleanupPaths.push(markerPath); + } + } + } finally { + writable.close(); + } + // The implicit autocommit committed before close; only now is each marker's + // deletion safe. Every unlink targets the exact per-mutation path that was + // reconciled, so a marker created by a concurrent writer after this snapshot + // can never be removed by a stale recovery. + const beforeCleanup = reconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests; + reconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests = null; + beforeCleanup?.(); + for (const markerPath of cleanupPaths) { + deletePendingConfigMutationAuditAtPath(markerPath); + } + } catch { + // A concurrent writer may hold the SQLite lock; the next mutation reconciles. + } +} + +/** + * Read the bounded audit trail, newest first. Defaults to 100 rows; the cap is 1000. + * Missing database or table yields an empty trail, never an error. + */ +export function readConfigMutationAudit( + configDir: string, + configPath: string, + limit = 100, +): { rows: ConfigMutationAuditRow[]; maxRows: number } { + const safeLimit = Number.isSafeInteger(limit) && limit > 0 ? Math.min(limit, 1000) : 100; + let database: Database | undefined; + try { + // A management read must never create or harden the coordinator directory, so + // resolve the path without the write-side effects of configMutationDatabasePath(). + const path = configMutationDatabasePathForRead(configDir); + if (!existsSync(path)) return { rows: [], maxRows: configAuditMaxRows }; + reconcilePendingConfigMutationAuditOnRead(path, configDir, configPath); + database = new Database(path, { readonly: true }); + const rows = database.prepare(` + SELECT id, mutation_id AS mutationId, created_at AS createdAt, surface, detail, fields, + before_json AS beforeJson, after_json AS afterJson + FROM config_mutation_audit + ORDER BY id DESC + LIMIT ? + `).all(safeLimit).map((row: unknown) => { + const record = row as Record; + return { + id: Number(record.id), + mutationId: String(record.mutationId), + createdAt: Number(record.createdAt), + surface: String(record.surface) as ConfigMutationAuditRow["surface"], + detail: String(record.detail), + fields: JSON.parse(String(record.fields)) as string[], + before: JSON.parse(String(record.beforeJson)) as Record, + after: JSON.parse(String(record.afterJson)) as Record, + }; + }); + return { rows, maxRows: configAuditMaxRows }; + } catch { + return { rows: [], maxRows: configAuditMaxRows }; + } finally { + try { database?.close(); } catch { /* read path is best-effort */ } + } +} + +function isPlainConfigObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Collect changed paths as segment arrays, descending at most three object levels (e.g. providers..). */ +function collectConfigDiffPaths( + before: unknown, + after: unknown, + prefix: string[], + depth: number, + out: string[][], +): void { + if (out.length >= CONFIG_AUDIT_MAX_FIELDS) return; + const beforeObject = isPlainConfigObject(before); + const afterObject = isPlainConfigObject(after); + if (beforeObject && afterObject && depth < 3) { + const keys = new Set([ + ...Object.keys(before), + ...Object.keys(after), + ]); + for (const key of keys) { + collectConfigDiffPaths(before[key], after[key], [...prefix, key], depth + 1, out); + } + return; + } + if (!deepEqual(before, after)) out.push(prefix); +} + +function extractConfigValueAtPath(root: unknown, segments: readonly string[]): unknown { + let current = root; + for (const part of segments) { + if (!isPlainConfigObject(current)) return undefined; + current = current[part]; + if (current === undefined) return undefined; + } + return current; +} + +/** Redact the admission-secret `key` field of every apiKeys entry, including degraded rows. */ +function redactApiKeyEntries(value: unknown, inApiKeysSubtree = false): unknown { + if (Array.isArray(value)) { + return value.map(item => redactApiKeyEntries(item, inApiKeysSubtree)); + } + if (!isPlainConfigObject(value)) return value; + const out: Record = Object.create(null); + for (const [entryKey, entryValue] of Object.entries(value)) { + out[entryKey] = redactApiKeyEntries(entryValue, inApiKeysSubtree || entryKey === "apiKeys"); + } + // OcxApiKeyEntry.key is the data-plane admission secret. Leaf-name redaction + // cannot see it (the field is `key`, not `apiKey`), so the whole subtree is + // masked before the row is persisted or echoed by GET /api/config/mutations. + // The schema deliberately salvages degraded entries (missing id/name/createdAt + // metadata), so match by context plus a string key rather than the full + // happy-path shape; outside the apiKeys subtree require a plausible entry. + if ( + typeof out.key === "string" + && (inApiKeysSubtree || [out.id, out.name, out.createdAt].some(v => typeof v === "string")) + ) { + out.key = REDACTED_SECRET; + } + return out; +} + +type ProviderHeaderContext = "outside" | "provider" | "headers"; + +function providerHeaderContextForPath(segments: readonly string[]): ProviderHeaderContext { + if (segments[0] === "providers") { + if (segments.length >= 3 && segments[2] === "headers") return "headers"; + return "provider"; + } + return "outside"; +} + +function redactProviderHeaderValues(value: unknown, context: ProviderHeaderContext = "outside"): unknown { + if (Array.isArray(value)) return value.map(item => redactProviderHeaderValues(item, context)); + if (!isPlainConfigObject(value)) { + return context === "headers" && typeof value === "string" ? REDACTED_SECRET : value; + } + const out: Record = Object.create(null); + for (const [entryKey, entryValue] of Object.entries(value)) { + let next: ProviderHeaderContext = context; + if (context === "outside" && entryKey === "providers") next = "provider"; + else if (context === "provider" && entryKey === "headers") next = "headers"; + out[entryKey] = redactProviderHeaderValues(entryValue, next); + } + return out; +} + +/** Mask userinfo in any URL-shaped string (http://user:pass@host) while preserving the rest. */ +function redactUrlUserinfoString(value: string): string { + // Mask every scheme://authority@ occurrence wherever it appears in the string, + // preserving surrounding text. The authority ends at the first / or whitespace; + // userinfo itself may contain @ characters, so the delimiter is the FINAL @ + // inside that authority segment (the greedy class backtracks to the last @ + // before the first / or whitespace). + return value.replace( + /([a-zA-Z][a-zA-Z0-9+.-]*:[/][/])([^/\s]+)@/g, + (_whole, scheme) => `${scheme}${REDACTED_SECRET}@`, + ); +} + +/** Recursively mask URL userinfo in every string inside an audit snapshot. */ +function redactUrlUserinfo(value: unknown): unknown { + if (Array.isArray(value)) return value.map(redactUrlUserinfo); + if (!isPlainConfigObject(value)) { + return typeof value === "string" ? redactUrlUserinfoString(value) : value; + } + const out: Record = Object.create(null); + for (const [entryKey, entryValue] of Object.entries(value)) { + out[entryKey] = redactUrlUserinfo(entryValue); + } + return out; +} + +/** Redact secrets and bound the serialized size of one audit value. */ +function boundAuditValue(value: unknown, key: string, segments: readonly string[]): unknown { + // Wrap in an object so redactSecrets can see the field name: a bare string leaf + // like `sk-old` has no context of its own and would otherwise survive unmasked. + const wrapped = redactSecrets({ [key]: value }); + // Apply the apiKeys-entry mask to the WHOLE extracted subtree: a first-ever + // save snapshots the entire config under the root label, so the admission + // key must be redacted even when the outer path is not apiKeys.*. When the + // extracted subtree IS the apiKeys array, say so: a degraded entry with only + // a key (no id/name/createdAt) would otherwise fall through the heuristic + // unmasked because leaf-name redaction cannot see the admission key field. + const redacted = redactApiKeyEntries( + (wrapped as Record)[key], + key === "apiKeys", + ); + const redactedHeaders = redactProviderHeaderValues( + redacted, + providerHeaderContextForPath(segments), + ); + const redactedUserinfo = redactUrlUserinfo(redactedHeaders); + const text = JSON.stringify(redactedUserinfo); + if (text === undefined) return null; + return text.length <= CONFIG_AUDIT_MAX_VALUE_CHARS + ? redactedUserinfo + : `${text.slice(0, CONFIG_AUDIT_MAX_VALUE_CHARS)}…[truncated]`; +} + +/** + * Build the bounded, redacted before/after snapshot for one config write. Both inputs + * must be parsed config objects (raw JSON text must be JSON.parsed by the caller). + */ +export function buildConfigMutationSnapshot( + beforeRaw: unknown, + afterRaw: unknown, +): { fields: string[]; before: Record; after: Record } { + const segmentPaths: string[][] = []; + collectConfigDiffPaths(beforeRaw, afterRaw, [], 0, segmentPaths); + // Config keys are caller-controlled and can be token-shaped (see the provider-name + // redaction at the schema boundary). Redact every segment before it is persisted + // and echoed by GET /api/config/mutations; extraction keeps the raw segments. + // Segments and joined labels are bounded so passthrough root keys cannot bloat rows. + const fields = segmentPaths.map(segments => { + const joined = segments.map(part => redactSecretString(part)).join("."); + return joined === "" ? "" : boundAuditLabel(joined); + }); + // Redaction can collapse distinct paths (two token-shaped provider names both + // become providers.[REDACTED].), and a dotted key can collide with a + // dotted passthrough name. Give duplicates a deterministic, non-secret + // occurrence suffix so no before/after record overwrites another. + const labelCounts = new Map(); + for (const label of fields) labelCounts.set(label, (labelCounts.get(label) ?? 0) + 1); + const labelSeen = new Map(); + const uniqueFields = fields.map(label => { + const count = labelCounts.get(label) ?? 1; + if (count <= 1) return label; + const seen = (labelSeen.get(label) ?? 0) + 1; + labelSeen.set(label, seen); + // Bound the suffixed form too so the stored label never exceeds the cap, + // reserving room for the occurrence suffix before shortening. + return seen === 1 ? label : boundAuditLabelWithOccurrence(label, seen); + }); + const before: Record = Object.create(null); + const after: Record = Object.create(null); + segmentPaths.forEach((segments, index) => { + const label = uniqueFields[index]!; + const key = segments.at(-1) ?? label; + before[label] = boundAuditValue(extractConfigValueAtPath(beforeRaw, segments), key, segments); + after[label] = boundAuditValue(extractConfigValueAtPath(afterRaw, segments), key, segments); + }); + return { fields: uniqueFields, before, after }; +} + +function isMissingPathError(error: unknown): boolean { + if (error && typeof error === "object" && "code" in error) { + return (error as { code?: unknown }).code === "ENOENT"; + } + return false; +} + +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a === null || b === null || typeof a !== "object" || typeof b !== "object") return false; + if (Array.isArray(a) !== Array.isArray(b)) return false; + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((item, index) => deepEqual(item, b[index])); + } + const left = a as Record; + const right = b as Record; + // `undefined` values and absent keys are the same thing after a JSON round-trip. + const keys = new Set([...Object.keys(left), ...Object.keys(right)]); + for (const key of keys) { + if (left[key] === undefined && right[key] === undefined) continue; + if (!deepEqual(left[key], right[key])) return false; + } + return true; +} diff --git a/src/config.ts b/src/config.ts index fdcda9547c..ddb1251d2b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,6 +1,8 @@ -import { createHash } from "node:crypto"; -import { chmodSync, constants as fsConstants, copyFileSync, existsSync, linkSync, mkdirSync, readFileSync, truncateSync, unlinkSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { execFileSync } from "node:child_process"; +import { createHash, randomUUID } from "node:crypto"; +import { chmodSync, closeSync, constants as fsConstants, copyFileSync, existsSync, fsyncSync, linkSync, lstatSync, mkdirSync, openSync, readFileSync, realpathSync, truncateSync, unlinkSync, writeFileSync } from "node:fs"; +import { homedir } from "node:os"; +import { dirname, join, resolve } from "node:path"; import { Database } from "bun:sqlite"; import * as z from "zod/v4"; import { isValidProviderName, hasOwnProvider } from "./config/provider-name"; @@ -20,6 +22,20 @@ import { reasoningSummaryDeliveryRecordConfigError, upstreamHttpVersionConfigError, } from "./config/provider-validation"; +import { + buildConfigMutationSnapshot as buildAuditSnapshot, + CONFIG_MUTATION_DB_FILENAME, + deletePendingConfigMutationAudit, + deletePendingConfigMutationAuditAtPath, + ensureConfigMutationAuditTable, + listPendingConfigMutationAuditPaths, + readConfigMutationAudit as readAuditRows, + reconcilePendingConfigMutationAudit, + recordPendingConfigMutationAuditNow, + writePendingConfigMutationAudit, + type ConfigMutationAuditRow, + type ConfigMutationSource, +} from "./config-mutation-audit"; import { bumpConfigGenerationAtPath, bumpCurrentConfigGeneration, @@ -61,7 +77,11 @@ import { import { recordOwnedConfigPath } from "./lib/config-ownership"; import { assertNotRealHomeUnderTest } from "./lib/test-home-guard"; import { providerDestinationConfigError } from "./lib/destination-policy"; -import { redactSecretString } from "./lib/redact"; +import { REDACTED_SECRET, redactSecretString, redactSecrets } from "./lib/redact"; +import { + resolveTrustedWindowsPowerShellExe, + resolveTrustedWindowsSystemDirectory, +} from "./lib/windows-elevation"; import { openRouterRoutingConfigError } from "./providers/openrouter-routing"; import { MODEL_ALIAS_PATTERN } from "./providers/default-aliases"; import { MODEL_DISCOVERY_MAX_MODELS } from "./providers/model-discovery-limits"; @@ -1678,6 +1698,28 @@ function sanitizeRetryOn429ForLoad(parsed: unknown): void { } } +/** + * Management write-boundary validation for transientRetryOn5xx, mirroring + * retryOn429PolicyConfigError: the shared policy schema is strict (1-10 + * attempts), and a malformed write must be rejected before it can reach disk and + * then fail the next load. Never echoes values; secret-shaped unknown field names + * are redacted. + */ +export function transientRetryOn5xxPolicyConfigError(policy: unknown): string | null { + if (policy === undefined) return null; + const result = transientRetryOn5xxPolicySchema.safeParse(policy); + if (result.success) return null; + const first = result.error.issues[0]; + if (!first) return "transientRetryOn5xx is invalid"; + if (first.code === "unrecognized_keys") { + const names = first.keys.map(key => JSON.stringify(redactSecretString(key))).join(", "); + return "transientRetryOn5xx has unrecognized field" + (first.keys.length > 1 ? "s" : "") + ": " + names; + } + if (first.path.length === 0) return "transientRetryOn5xx is invalid (" + first.message + ")"; + const field = String(first.path[first.path.length - 1]); + return "transientRetryOn5xx." + field + " is invalid (" + first.message + ")"; +} + /** * Management write-boundary validation for `retryOn429` (fail closed). Unlike the * lenient load-time sanitizer, invalid values and unknown keys are rejected outright so @@ -2847,7 +2889,6 @@ export function readConfigAdmissionSnapshot(): ConfigAdmissionSnapshot { }; } -const CONFIG_MUTATION_DB_FILENAME = "config-mutation.sqlite"; const CONFIG_MUTATION_DB_SIDECARS = ["-journal", "-wal", "-shm"] as const; let warnedConfigMutationDirectoryAcl = false; @@ -2919,6 +2960,30 @@ export function prepareConfigMutationDatabasePathForWrite(): string { let configMutationLockDepth = 0; let configMutationDatabase: Database | null = null; +/** The mutation id whose marker is deleted after the surrounding transaction commits. */ +let pendingConfigMutationAuditCleanup: string | null = null; +/** Test-only seam: fail the config.json atomic write AFTER the pending marker is persisted. */ +let failConfigAtomicWriteForTests: (() => Error) | null = null; +/** Test-only seam: fail the config.json atomic write AFTER the rename lands, before the audit commit. */ +let failAfterConfigAtomicWriteForTests: (() => Error) | null = null; + +/** + * Test-only one-shot seam: make the next changed config.json persist throw after + * writing its pending audit marker but before the config rename lands. Mirrors a + * crash/power loss between the marker write and the atomic config write. + */ +export function setConfigAtomicWriteFailureForTests(factory: (() => Error) | null): void { + failConfigAtomicWriteForTests = factory; +} + +/** + * Test-only one-shot seam: make the next changed config.json persist throw AFTER + * the rename lands but before the audit row commits. Mirrors a crash between the + * rename and the commit, the exact state recovery replays. + */ +export function setConfigPostWriteFailureForTests(factory: (() => Error) | null): void { + failAfterConfigAtomicWriteForTests = factory; +} /** * Serialize synchronous config and Codex credential-generation commits across processes with an @@ -2946,6 +3011,33 @@ export function withConfigMutationLockSync(fn: () => T): T { database.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); transactionOpen = true; initializeConfigGeneration(database); + ensureConfigMutationAuditTable(database); + // Replay any interrupted write (config renamed but audit row not committed) + // in its OWN transaction before this mutation starts. A recovered marker's + // row commits here, and the marker is removed, so a later failure in the new + // mutation can neither roll the recovered row back nor let a new marker + // overwrite the one whose replay has not yet committed. + const pendingPaths = listPendingConfigMutationAuditPaths(getConfigDir()); + if (pendingPaths.length > 0) { + for (const markerPath of pendingPaths) { + reconcilePendingConfigMutationAudit(database, markerPath, getConfigPath()); + } + database.exec("COMMIT"); + transactionOpen = false; + // Every pending marker is decided here: valid ones were replayed or dropped, + // and a parseable-but-invalid one cannot be trusted, so it is removed too. + for (const markerPath of pendingPaths) { + try { + deletePendingConfigMutationAuditAtPath(markerPath); + } catch { + // Best-effort after the recovery COMMIT: the row is durable and a leftover + // marker is re-decided (replayed or dropped) on the next recovery, so a + // transient unlink failure must not fail an already-committed mutation. + } + } + database.exec("BEGIN IMMEDIATE"); + transactionOpen = true; + } } catch (cause) { if (transactionOpen) { try { database?.exec("ROLLBACK"); } catch { /* close below still releases the OS lock */ } @@ -2966,20 +3058,34 @@ export function withConfigMutationLockSync(fn: () => T): T { const value = fn(); database.exec("COMMIT"); transactionOpen = false; + if (pendingConfigMutationAuditCleanup) { + const mutationId = pendingConfigMutationAuditCleanup; + pendingConfigMutationAuditCleanup = null; + try { + deletePendingConfigMutationAudit(getConfigDir(), mutationId); + } catch { + // Best-effort after the COMMIT: the audit row is durable. A leftover marker + // is harmless and the next recovery reconciles it, so an unlink failure + // (e.g. a transient handle hold) must not surface as a failed write. + } + } return value; } catch (error) { if (transactionOpen) { try { database.exec("ROLLBACK"); } catch { /* close below still releases the OS lock */ } transactionOpen = false; } + pendingConfigMutationAuditCleanup = null; throw error; } finally { configMutationLockDepth = 0; configMutationDatabase = null; + pendingConfigMutationAuditCleanup = null; try { database.close(); } catch { /* the OS lock is released with the handle */ } } } + function bumpGenerationForCooperatingConfigWrite(): void { if (!configMutationDatabase) { throw new Error("A cooperating config write requires the config mutation transaction."); @@ -2999,6 +3105,19 @@ export function observeConfigGeneration(): ConfigGenerationObservation { return observeConfigGenerationAtPath(join(getConfigDir(), CONFIG_MUTATION_DB_FILENAME)); } +/** Read the bounded config mutation audit trail, newest first (default 100, cap 1000). */ +export function readConfigMutationAudit( + limit = 100, +): { rows: ConfigMutationAuditRow[]; maxRows: number } { + return readAuditRows(getConfigDir(), getConfigPath(), limit); +} + +export { + buildConfigMutationSnapshot, + setConfigAuditMaxRowsForTests, +} from "./config-mutation-audit"; +export type { ConfigMutationAuditRow, ConfigMutationSource }; + /** * Read the generation from the transaction that is open RIGHT NOW. * @@ -3071,11 +3190,14 @@ export const withExpectedConfigGenerationSync: WithExpectedConfigGenerationSync /** * Atomic config.json write WITHOUT the mutation lock; callers must hold - * `withConfigMutationLockSync`. Returns true when bytes changed. Refreshes the - * cost-overlay registry from the persisted config so runtime estimates follow - * every save path. + * `withConfigMutationLockSync`. Returns the exact persisted document when bytes + * changed and null when the save was byte-identical. Refreshes the cost-overlay + * registry from the persisted config so runtime estimates follow every save path. */ -function persistConfigUnlocked(config: OcxConfig): boolean { +function persistConfigUnlocked( + config: OcxConfig, + audit?: { before: unknown; source: ConfigMutationSource }, +): OcxConfig | null { const configPath = getConfigPath(); const rawBeforeWrite = readRawConfigJson(); const clientPersistenceError = failClosedClientPersistenceError(rawBeforeWrite, config); @@ -3102,25 +3224,62 @@ function persistConfigUnlocked(config: OcxConfig): boolean { // adopt the overlay without waiting for a changed save or restart. if (unchanged) { refreshUserCostOverlays(persisted); - return false; + return null; + } + const auditMutationId = audit ? randomUUID() : null; + if (audit) { + // Write-ahead marker: persisted BEFORE the rename so a crash between the rename + // and the audit-row commit is recovered (replayed or dropped) on the next access. + // Each marker is named by its mutation id, so recovery can only ever delete the + // exact marker it reconciled. + const snapshot = buildAuditSnapshot(audit.before, persisted); + writePendingConfigMutationAudit({ + mutationId: auditMutationId!, + createdAt: Date.now(), + surface: audit.source.surface, + detail: audit.source.detail, + fields: snapshot.fields, + before: snapshot.before, + after: snapshot.after, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + }, getConfigDir(), atomicWriteFile); + } + const failWrite = failConfigAtomicWriteForTests; + if (failWrite) { + failConfigAtomicWriteForTests = null; + throw failWrite(); } atomicWriteFile(configPath, bytes); + const failAfterWrite = failAfterConfigAtomicWriteForTests; + if (failAfterWrite) { + failAfterConfigAtomicWriteForTests = null; + throw failAfterWrite(); + } // For changed saves, refresh only AFTER the write succeeded so a failed // write cannot leave estimates reflecting configuration never persisted. refreshUserCostOverlays(persisted); - return true; + if (audit && configMutationDatabase) { + const recorded = recordPendingConfigMutationAuditNow(configMutationDatabase, getConfigDir(), auditMutationId!); + pendingConfigMutationAuditCleanup = recorded ? auditMutationId : null; + } + return persisted; } /** Persist `config` to config.json under the config-mutation lock. */ -export function saveConfig(config: OcxConfig): void { +export function saveConfig( + config: OcxConfig, + source: ConfigMutationSource = { surface: "internal", detail: "saveConfig" }, +): void { // Keep the real-home assertion ahead of even lock-directory preparation. assertNotRealHomeUnderTest(getConfigDir()); withConfigMutationLockSync(() => { + const beforeRaw = readRawConfigJson(); const withProvenance = projectCustomModelCatalogMigration( - readRawConfigJson(), + beforeRaw, projectConfigRebaseProvenance(config), ); - if (persistConfigUnlocked(withProvenance)) bumpGenerationForCooperatingConfigWrite(); + const persisted = persistConfigUnlocked(withProvenance, { before: beforeRaw, source }); + if (persisted) bumpGenerationForCooperatingConfigWrite(); adoptCustomModelCatalogMigration(config, withProvenance); if (withProvenance.configRebaseProvenance === undefined) delete config.configRebaseProvenance; else config.configRebaseProvenance = structuredClone(withProvenance.configRebaseProvenance); @@ -3159,6 +3318,7 @@ function unavailableConfigMutationReason(snapshot: ConfigFileSnapshot): "missing */ export function mutatePersistedConfig( mutate: (config: OcxConfig) => PersistedConfigMutation, + source: ConfigMutationSource = { surface: "internal", detail: "mutatePersistedConfig" }, ): PersistedConfigMutationOutcome { // Avoid creating/opening the coordinator database for a read-path update that already knows // there is no valid config. The same check runs again under the transaction for authority. @@ -3209,7 +3369,14 @@ export function mutatePersistedConfig( commitBase.diagnostics.config, projectConfigRebaseProvenance(confirmedConfig), ); - if (persistConfigUnlocked(projected)) bumpGenerationForCooperatingConfigWrite(); + const persisted = persistConfigUnlocked(projected, { + // The exact persisted document, matching saveConfig and + // saveConfigPreservingClaudeCode. The parsed config carries schema + // defaults and degraded fields that were never on disk. + before: readRawConfigJson(), + source, + }); + if (persisted) bumpGenerationForCooperatingConfigWrite(); return { status: "committed", value: confirmed.value }; } return { status: "unavailable", reason: "conflict" }; @@ -3569,7 +3736,10 @@ function readPersistedServerBinding( * Custom-model rows are merged by their stable `id`, preserving independent * edits and deletions across stale whole-config saves. */ -export function saveConfigPreservingClaudeCode(config: OcxConfig): void { +export function saveConfigPreservingClaudeCode( + config: OcxConfig, + source: ConfigMutationSource = { surface: "internal", detail: "saveConfigPreservingClaudeCode" }, +): void { withConfigMutationLockSync(() => { const bindingBaseline = persistedLiveServerBinding.get(config); // One authoritative pre-write read feeds both the live-config reconciliation and @@ -3636,10 +3806,12 @@ export function saveConfigPreservingClaudeCode(config: OcxConfig): void { const persistedConfig: OcxConfig = { ...projectedConfig, port: persistedBinding.port }; if (persistedBinding.hostname === undefined) delete persistedConfig.hostname; else persistedConfig.hostname = persistedBinding.hostname; - if (persistConfigUnlocked(persistedConfig)) bumpGenerationForCooperatingConfigWrite(); + const persisted = persistConfigUnlocked(persistedConfig, { before: onDisk, source }); + if (persisted) bumpGenerationForCooperatingConfigWrite(); persistedLiveServerBinding.set(config, persistedBinding); } else { - if (persistConfigUnlocked(projectedConfig)) bumpGenerationForCooperatingConfigWrite(); + const persisted = persistConfigUnlocked(projectedConfig, { before: onDisk, source }); + if (persisted) bumpGenerationForCooperatingConfigWrite(); } adoptCustomModelCatalogMigration(config, projectedConfig); if (claudeCodeBaseline.has(config)) { diff --git a/src/lib/redact.ts b/src/lib/redact.ts index ab82047aa3..62223ba7f2 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -5,7 +5,7 @@ export const REDACTED_SECRET = "[REDACTED]"; * credentials over an unsafe channel (e.g. plaintext non-loopback HTTP) rather than * re-deriving a narrower local list. */ -export const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set-cookie|set-cookie2|api[-_]?key|x-api-key|x-goog-api-key|x-amz-security-token|access[-_]?token|refresh[-_]?token|id[-_]?token|token|secret|client[-_]?secret|password|profile[-_]?arn|exa[-_]?api[-_]?key)$/i; +export const SENSITIVE_KEY_PATTERN = /^(?:authorization|proxy-authorization|cookie|set-cookie|set-cookie2|api[-_]?key|api[-_]?key[-_]?pool|x-api-key|x-goog-api-key|x-amz-security-token|access[-_]?token|refresh[-_]?token|id[-_]?token|token|secret|client[-_]?secret|oauth[-_]?client[-_]?secret|password|profile[-_]?arn|exa[-_]?api[-_]?key)$/i; /** * Colon-labelled credential headers echoed back inside an error body @@ -465,7 +465,9 @@ export function redactSecrets(value: unknown): unknown { if (value instanceof Date) return value; if (!isPlainObject(value)) return value; - const result: Record = {}; + // Null-prototype record: an own JSON key named "__proto__" must survive the + // copy instead of mutating the result's prototype. + const result: Record = Object.create(null); for (const [key, entryValue] of Object.entries(value)) { result[key] = isSensitiveKey(key) ? REDACTED_SECRET : redactSecrets(entryValue); } diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 1a8bd07157..e47dc6f0fa 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1365,7 +1365,7 @@ export function reconcileOAuthProviders(config: OcxConfig, persist = true): bool const next = projectOAuthProviderReconciliation(fresh); if (next.changed) adoptOAuthReconciliation(fresh, next); return { changed: next.changed, value: next }; - }); + }, { surface: "internal", detail: "oauth: reconcile provider presets" }); if (outcome.status === "unavailable") { console.warn( `[opencodex] OAuth provider reconciliation could not be persisted (${outcome.reason}); ` diff --git a/src/oauth/login-cli.ts b/src/oauth/login-cli.ts index 79a3aa6eca..817ee6a812 100644 --- a/src/oauth/login-cli.ts +++ b/src/oauth/login-cli.ts @@ -161,7 +161,7 @@ export async function commitKeyLoginProvider( const mergedProvider = mergeKeyLoginProviderRow(provider, config.providers[name]); initializeProviderModelSelection(name, mergedProvider, config.providers[name], config); config.providers[name] = mergedProvider; - saveConfig(config); + saveConfig(config, { surface: "cli", detail: "ocx key login" }); // Evaluate the reload BEFORE the optional call: `onLiveReload?.(await ...)` short-circuits // the whole argument list when no callback is supplied, so the reload would never fire for // callers that do not care about the outcome. diff --git a/src/providers/api-keys.ts b/src/providers/api-keys.ts index d138cdbf50..de8b8c0d3b 100644 --- a/src/providers/api-keys.ts +++ b/src/providers/api-keys.ts @@ -94,7 +94,7 @@ export function addProviderApiKey(config: OcxConfig, name: string, key: string, pool.push({ id, key: trimmed, ...(label?.trim() ? { label: label.trim() } : {}), addedAt: Date.now() }); } provider.apiKey = trimmed; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "api-keys: add provider key" }); return { id }; } @@ -105,7 +105,7 @@ export function setActiveProviderApiKey(config: OcxConfig, name: string, id: str const entry = ensurePool(provider).find(e => e.id === id); if (!entry) return false; provider.apiKey = entry.key; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "api-keys: set active provider key" }); return true; } @@ -117,7 +117,7 @@ export function setProviderApiKeyLabel(config: OcxConfig, name: string, id: stri if (!entry) return false; if (label) entry.label = label; else delete entry.label; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "api-keys: set provider key label" }); return true; } @@ -135,6 +135,6 @@ export function removeProviderApiKey(config: OcxConfig, name: string, id: string else delete provider.apiKey; } if (provider.apiKeyPool.length === 0) delete provider.apiKeyPool; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "api-keys: remove provider key" }); return true; } diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index 9c7a42e11b..1c052eb3cd 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -233,7 +233,7 @@ function rotateKeyAfterFailure( }; } return { changed: false, value: { exhaustedCount: pool.length, failedId: failedEntry?.id } }; - }); + }, { surface: "internal", detail: "key-failover: rotate active provider key" }); if (outcome.status === "unavailable" || outcome.value === null) return null; if (outcome.value.failedId) { // A 401 is a verdict about the credential itself, not a timing signal: the key is rejected diff --git a/src/providers/key-store.ts b/src/providers/key-store.ts index 12e4ce6cb7..20b33d8352 100644 --- a/src/providers/key-store.ts +++ b/src/providers/key-store.ts @@ -1,5 +1,5 @@ import { createRequire } from "node:module"; -import { resolveEnvValue, saveConfigPreservingClaudeCode } from "../config"; +import { resolveEnvValue, saveConfigPreservingClaudeCode, type ConfigMutationSource } from "../config"; import type { OcxConfig, OcxProviderConfig } from "../types"; import type { ProviderRegistryEntry } from "./registry"; @@ -129,7 +129,7 @@ function writeVerified(account: string, secret: string): void { * config with references. All keychain writes are verified before config changes; on any * failure the entries written so far are deleted and config is left untouched. */ -export function storeProviderKeyInKeychain(config: OcxConfig, name: string): { ok: true; moved: number } | { ok: false; error: string; status: number } { +export function storeProviderKeyInKeychain(config: OcxConfig, name: string, source: ConfigMutationSource = { surface: "internal", detail: "key-store: store provider keys in OS keychain" }): { ok: true; moved: number } | { ok: false; error: string; status: number } { const provider = config.providers[name]; if (!provider) return { ok: false, error: "unknown provider", status: 404 }; if (provider.authMode === "oauth" || provider.authMode === "forward") { @@ -174,12 +174,12 @@ export function storeProviderKeyInKeychain(config: OcxConfig, name: string): { o for (const apply of planned) apply(); resolvedCache.clear(); warnedAccounts.clear(); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, source); return { ok: true, moved: written.length }; } /** Reverse of `storeProviderKeyInKeychain`: read every reference back, write plaintext, delete items. */ -export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string): { ok: true; restored: number } | { ok: false; error: string; status: number } { +export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string, source: ConfigMutationSource = { surface: "internal", detail: "key-store: restore provider keys from OS keychain" }): { ok: true; restored: number } | { ok: false; error: string; status: number } { const provider = config.providers[name]; if (!provider) return { ok: false, error: "unknown provider", status: 404 }; const pool = provider.apiKeyPool ?? []; @@ -202,6 +202,6 @@ export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string): } resolvedCache.clear(); warnedAccounts.clear(); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, source); return { ok: true, restored: resolved.size }; } diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index ccc23c5ef5..d65e7b5c98 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -9,6 +9,7 @@ import { requestPacingConfigError, retryOn429PolicyConfigError, sanitizeModelCostsForDisplay, + transientRetryOn5xxPolicyConfigError, } from "../config"; import { apiKeyTransportConfigError, @@ -639,6 +640,10 @@ export function providerManagementConfigError(name: unknown, provider: unknown): // it before it reaches the management API response. return `provider ${JSON.stringify(redactSecretString(name))} ${retryOn429Error}`; } + const transientRetryOn5xxError = transientRetryOn5xxPolicyConfigError(raw.transientRetryOn5xx); + if (transientRetryOn5xxError) { + return `provider ${JSON.stringify(redactSecretString(name))} ${transientRetryOn5xxError}`; + } const requestPacingError = requestPacingConfigError(raw.requestPacing); if (requestPacingError) { return `provider ${JSON.stringify(redactSecretString(name))} ${requestPacingError}`; diff --git a/src/server/index.ts b/src/server/index.ts index 82f36d7b6a..7d82249c82 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -674,7 +674,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server["desktopProfile"], + source: ConfigMutationSource = { surface: "api", detail: "PUT /api/native-integrations/claude-desktop" }, ): { ok: true } | { ok: false; reason: "missing" | "invalid" | "conflict" } { const outcome = mutatePersistedConfig(persisted => { persisted.claudeCode = { ...(persisted.claudeCode ?? {}), desktopProfile }; return { changed: true, value: true }; - }); + }, source); // Only mirror into memory once the durable write actually landed; an // `unavailable` outcome must not leave the snapshot claiming a saved profile. if (outcome.status === "unavailable") return { ok: false, reason: outcome.reason }; @@ -218,7 +220,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise ); if (result.written && result.fingerprint) { current.claudeCode = { ...current.claudeCode, desktopProfile: { ...current.claudeCode.desktopProfile, appliedFingerprint: result.fingerprint, appliedAt: new Date().toISOString() } }; - saveConfigPreservingClaudeCode(current); + saveConfigPreservingClaudeCode(current, { surface: "internal", detail: "auto-apply desktop fingerprint" }); } } catch { /* best-effort */ } } @@ -360,13 +362,14 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise if (wantsMode) { if (mode === "default") deleteConfigTopLevelKey(config, "multiAgentMode"); else config.multiAgentMode = mode; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/v2 (mode)" }); warnings.push(`Multi-agent mode set to '${mode}'. Applies to new sessions.`); } if (wantsKeepNative) { if (body.keepNativeChatGptOnV1 === true) config.keepNativeChatGptOnV1 = true; else deleteConfigTopLevelKey(config, "keepNativeChatGptOnV1"); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/v2 (keep-native-v1)" }); + const effectiveMode = mode ?? config.multiAgentMode ?? "default"; warnings.push(body.keepNativeChatGptOnV1 === true ? (effectiveMode === "v2" ? "ChatGPT-native models stay on v1 while other models use v2. Applies to new sessions." @@ -583,7 +586,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise if (nextPrompt) config.injectionPrompt = nextPrompt; else deleteConfigTopLevelKey(config, "injectionPrompt"); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/injection-model" }); return jsonResponse({ ok: true, multiAgentGuidanceEnabled: multiAgentGuidanceEnabled(config), @@ -618,7 +621,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise } config[key] = value; } - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/effort-caps" }); return jsonResponse({ ok: true, effortCap: config.effortCap ?? null, subagentEffortCap: config.subagentEffortCap ?? null }); } @@ -665,7 +668,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise const chosen = Array.isArray(body.models) ? body.models.filter((m): m is string => typeof m === "string").slice(0, 5) : []; config.subagentModels = chosen; const { saveConfigPreservingClaudeCode: save } = await import("../../config"); - save(config); + save(config, { surface: "api", detail: "PUT /api/subagent-models" }); const catalogRefresh = await convergeCodexCatalog(); await syncClaudeAgentDefsBestEffort(); await autoApplyDesktopBestEffort(); @@ -734,7 +737,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise else deleteConfigTopLevelKey(config, "subagentModelFallback"); if (nextPollMs !== undefined) config.subagentModelFallbackPollMs = nextPollMs; else deleteConfigTopLevelKey(config, "subagentModelFallbackPollMs"); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/subagent-model-fallback" }); return jsonResponse({ ok: true, models: config.subagentModelFallback ?? [], @@ -776,7 +779,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise if (excluded.length > 2000) return jsonResponse({ error: "excluded list is too large" }, 400); if (excluded.length === 0) deleteConfigTopLevelKey(config, "grokExcludedModels"); else config.grokExcludedModels = excluded; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/grok/selection" }); return jsonResponse({ ok: true, excluded }); } @@ -833,7 +836,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise } const state = await buildClaudeDesktopState(config, parsed); config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: reconcileDesktopProfile(state.profile, state.models) }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PUT /api/claude-desktop (profile)" }); const saved = await buildClaudeDesktopState(config); const runtimePort = Number(url.port) || config.port; return jsonResponse({ ok: true, ...saved, port: runtimePort }); @@ -875,7 +878,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise } } const { setIntegrationEnabled, claudeDesktopIntegrationEnabled } = await import("../../codex/desired-state"); - const desired = setIntegrationEnabled("claude-desktop", true); + const desired = setIntegrationEnabled("claude-desktop", true, { surface: "api", detail: "POST /api/claude-desktop/apply" }); if (!desired.ok) return jsonResponse({ error: desired.message }, desired.retryable ? 409 : 500); // Disk now says ON; the reused server snapshot must agree, or the native // GET reports OFF and a later whole-snapshot save undoes this transition. @@ -886,7 +889,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise // its stale `clientIntegrations` back over that write and turn the enable // action into an immediate self-cancelling OFF — the guard below would then // refuse the apply it was asked to perform. Persist ONLY the profile field. - const profileSaved = persistDesktopProfileField(config, state.profile); + const profileSaved = persistDesktopProfileField(config, state.profile, { surface: "api", detail: "POST /api/claude-desktop/apply" }); if (!profileSaved.ok) { return jsonResponse({ error: `Claude Desktop profile could not be saved (${profileSaved.reason}); nothing was applied.`, @@ -938,7 +941,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise ...state.profile, appliedFingerprint: result.fingerprint, appliedAt: new Date().toISOString(), - }); + }, { surface: "api", detail: "POST /api/claude-desktop/apply (fingerprint)" }); if (!marked.ok) { return jsonResponse({ ok: true, @@ -1374,7 +1377,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise // auto would survive exactly one proxy lifetime with no way back. if (!next.authModeMigratedAt) next.authModeMigratedAt = new Date().toISOString(); const { saveConfigPreservingClaudeCode: save } = await import("../../config"); - save(config); + save(config, { surface: "api", detail: "PUT /api/claude-code" }); const warnings: string[] = []; // authMode changes must reconcile the injected system env too: switching back to // Subscription has to remove the opencodex-owned dummy ANTHROPIC_AUTH_TOKEN diff --git a/src/server/management/combo-routes.ts b/src/server/management/combo-routes.ts index 475e72db41..b97a8a8474 100644 --- a/src/server/management/combo-routes.ts +++ b/src/server/management/combo-routes.ts @@ -274,7 +274,7 @@ export async function handleComboRoutes(ctx: ManagementContext): Promise; try { @@ -560,7 +578,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise { const enabled = body.enabled; const { setCodexIntegrationEnabled } = await import("../../codex/desired-state"); - const persisted = setCodexIntegrationEnabled(enabled); + const persisted = setCodexIntegrationEnabled(enabled, { surface: "api", detail: "PUT /api/native-integrations/codex" }); /* * `missing` does not block the switch — see the Grok route for the reasoning. * A user with no config file yet still gets the artifact change; what they @@ -434,7 +434,7 @@ async function handleGrokToggle(ctx: ManagementContext): Promise { * on, where the other order leaves artifacts the next start undoes. */ const { setGrokIntegrationEnabled } = await import("../../codex/desired-state"); - const persisted = setGrokIntegrationEnabled(enabled); + const persisted = setGrokIntegrationEnabled(enabled, { surface: "api", detail: "PUT /api/native-integrations/grok" }); /* * `missing` does NOT block the toggle here. * @@ -616,7 +616,7 @@ async function handleClaudeDesktopToggle(ctx: ManagementContext): Promise 0) prov.oauthAccountFailover = next; else delete prov.oauthAccountFailover; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: `${req.method} /api/oauth/accounts/pool` }); return jsonResponse({ ok: true, ...genericPoolSettingsDto(provider, prov) }); } let enabled = config.anthropicAccountPool?.enabled === true; @@ -458,7 +458,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< ...(stickyLimit !== undefined ? { stickyLimit } : {}), ...(quotaWindow !== undefined ? { quotaWindow } : {}), }; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: `${req.method} /api/oauth/accounts/pool` }); reconcileLiveStateStores(); return jsonResponse({ ok: true, @@ -638,8 +638,8 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< if (body.action !== "store" && body.action !== "restore") return jsonResponse({ error: "action must be store or restore" }, 400); const { storeProviderKeyInKeychain, restoreProviderKeyFromKeychain, providerKeyStoreKind } = await import("../../providers/key-store"); const result = body.action === "store" - ? storeProviderKeyInKeychain(config, name) - : restoreProviderKeyFromKeychain(config, name); + ? storeProviderKeyInKeychain(config, name, { surface: "api", detail: "POST /api/providers/keychain (store)" }) + : restoreProviderKeyFromKeychain(config, name, { surface: "api", detail: "POST /api/providers/keychain (restore)" }); if (!result.ok) return jsonResponse({ error: result.error }, result.status); const { clearProviderQuotaCache } = await import("../../providers/quota"); clearProviderQuotaCache(); @@ -695,7 +695,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< // --------------------------------------------------------------------------- if (url.pathname === "/api/keys" && req.method === "GET") { if (removeExpiredApiKeyRotations(config)) { - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "GET /api/keys (expired rotation cleanup)" }); reconcileLiveStateStores(); } const keys = config.apiKeys ?? []; @@ -739,7 +739,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< if ("error" in result) { return jsonResponse({ error: result.error === "not-found" ? "key not found" : "rotation already pending" }, result.error === "not-found" ? 404 : 409, req, config); } - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "POST /api/keys/rotate" }); reconcileLiveStateStores(); return jsonResponse(result, 201, req, config); } @@ -752,10 +752,10 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< } const result = commitApiKeyRotation(config, body.id, body.rotationId); if ("error" in result) { - if (result.error === "expired") saveConfigPreservingClaudeCode(config); + if (result.error === "expired") saveConfigPreservingClaudeCode(config, { surface: "api", detail: "POST /api/keys/rotate/commit (expired rotation cleanup)" }); return jsonResponse({ error: result.error === "not-found" ? "key rotation not found" : `rotation ${result.error}` }, result.error === "not-found" ? 404 : 409, req, config); } - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "POST /api/keys/rotate/commit" }); reconcileLiveStateStores(); return jsonResponse({ ok: true }, 200, req, config); } @@ -769,7 +769,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< if (!abortApiKeyRotation(config, body.id, body.rotationId)) { return jsonResponse({ error: "key rotation not found or mismatched" }, 409, req, config); } - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "DELETE /api/keys/rotate" }); reconcileLiveStateStores(); return jsonResponse({ ok: true }, 200, req, config); } @@ -788,7 +788,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const key = "ocx_data_" + randomBytes(20).toString("hex"); const entry = { id: randomUUID(), name, key, createdAt: new Date().toISOString() }; config.apiKeys = [...(config.apiKeys ?? []), entry]; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "POST /api/keys" }); reconcileLiveStateStores(); return jsonResponse({ id: entry.id, name: entry.name, key: entry.key, createdAt: entry.createdAt }, 201, req, config); } @@ -802,7 +802,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const entry = (config.apiKeys ?? []).find(k => k.id === body.id); if (!entry) return jsonResponse({ error: "key not found" }, 404, req, config); entry.name = nameField.value; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "PATCH /api/keys" }); reconcileLiveStateStores(); // Never echo key material from a rename. return jsonResponse({ id: entry.id, name: entry.name, createdAt: entry.createdAt }, 200, req, config); @@ -816,7 +816,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< config.apiKeys = (config.apiKeys ?? []).filter(k => k.id !== body.id); // A stale id must not read as a successful revocation. if (config.apiKeys.length === before) return jsonResponse({ error: "key not found" }, 404, req, config); - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "DELETE /api/keys" }); reconcileLiveStateStores(); return jsonResponse({ success: true }, 200, req, config); } diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index e26420e003..e5cfadd31a 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -1009,7 +1009,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise 0) config.routingProfiles = nextProfiles; else deleteConfigTopLevelKey(config, "routingProfiles"); const saveConfigPreservingClaudeCodeSafe = deps.saveConfigPreservingClaudeCode ?? saveConfigPreservingClaudeCode; - saveConfigPreservingClaudeCodeSafe(config); + saveConfigPreservingClaudeCodeSafe(config, { surface: "api", detail: "DELETE /api/routing-profiles" }); reconcileLiveStateStores(); const catalogRefresh = await convergeCodexCatalog(); return jsonResponse({ success: true, id, catalogRefresh }, 200, req, config); diff --git a/src/server/subagent-models-startup.ts b/src/server/subagent-models-startup.ts index c3b1595048..2b99bf33d9 100644 --- a/src/server/subagent-models-startup.ts +++ b/src/server/subagent-models-startup.ts @@ -7,10 +7,13 @@ export function migrateStartupSubagentModels(config: OcxConfig): OcxConfig { const projection = { ...config }; if (!migrateSubagentModels(projection)) return config; try { - const outcome = mutatePersistedConfig(fresh => ({ - changed: migrateSubagentModels(fresh), - value: fresh, - })); + const outcome = mutatePersistedConfig( + fresh => ({ + changed: migrateSubagentModels(fresh), + value: fresh, + }), + { surface: "internal", detail: "startup: seed default subagent models" }, + ); if (outcome.status === "unavailable") { console.warn(`[subagent-models-migration] Persistence unavailable (${outcome.reason}); using the upgraded roster in memory only.`); } else { diff --git a/src/storage/policy.ts b/src/storage/policy.ts index d90a2473a5..f080e701d0 100644 --- a/src/storage/policy.ts +++ b/src/storage/policy.ts @@ -266,7 +266,7 @@ export function writeStorageCleanupPolicyToConfig(policy: StorageCleanupPolicy): const normalized = normalizeStorageCleanupPolicy(policy); const config = loadConfig(); config.storageCleanupPolicy = normalized; - saveConfigPreservingClaudeCode(config); + saveConfigPreservingClaudeCode(config, { surface: "api", detail: "storage-policy: write cleanup policy" }); livePolicySink?.(normalized); return normalized; } @@ -462,14 +462,17 @@ function commitPolicyRunMetadataToConfig( }; }; try { - const outcome = mutatePersistedConfig(config => { - const next = applyPolicyRunMetadata( - normalizeStorageCleanupPolicy(config.storageCleanupPolicy), - patch, - ); - config.storageCleanupPolicy = next; - return { changed: true, value: next }; - }); + const outcome = mutatePersistedConfig( + config => { + const next = applyPolicyRunMetadata( + normalizeStorageCleanupPolicy(config.storageCleanupPolicy), + patch, + ); + config.storageCleanupPolicy = next; + return { changed: true, value: next }; + }, + { surface: "internal", detail: "storage-policy: commit run metadata" }, + ); if (outcome.status === "unavailable") return unavailable(outcome.reason); livePolicySink?.(outcome.value); return { policy: outcome.value }; diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index bb8ec5630f..59ca2122e3 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -455,3 +455,37 @@ the residual directory for manual review; there is no recursive-delete fallback. ## Remote client key files Client connection metadata stores a stable `apiKeyId` and a non-secret rotation `pendingOperation`. The current data secret remains only in `service-api-token`; a bounded rotation temporarily keeps the old secret in owner-only `service-api-token.prev`. Commit or recovery clears the marker before orphan cleanup. `ocx disconnect` is local-only and leaves remote revocation to the hub's **Integrations → API Keys** page. Hub and local usage stores are not mirrored. +## Config mutation audit + +Every changed persisted config write is recorded in `config-mutation.sqlite` beside `config.json` +(`config_mutation_audit` table, newest-first, default read 100 rows / cap 1000, retention 5000 rows). +The row records the mutation surface (CLI / management API / internal), the route or command detail, +the changed field paths (redacted, capped at 64), and bounded redacted before/after snapshots +(4096 chars per entry). Raw credentials and request content are never stored; the pending +`config-mutation-pending-.json` markers (one per write) carry the same redacted snapshots plus +the SHA-256 of the exact bytes the write produced; each marker is named by a unique mutation id so +recovery can only ever delete the exact marker it reconciled. + +[Decision Log] +- 목적과 의도: Record who changed config.json, through which surface, and what the redacted before/after + looked like, without ever persisting admission secrets or request payloads. +- 기존 구현 및 제약 조건: config.json was byte-atomic with a generation counter, but there was no + durable attribution trail; the pre-existing SQLite coordinator and mutation lock were already used + by Codex credential-generation commits. +- 검토한 주요 대안: In-process ring buffer (lost on restart), plain append log (no ordering or + crash-recovery guarantee), or a write-ahead marker plus a SQLite audit table inside the existing + config mutation transaction. +- 선택한 방식: SQLite `config_mutation_audit` table sharing the config mutation lock, with an atomic + per-mutation `config-mutation-pending-.json` write-ahead markers written before the config + rename and removed after the audit row commits; an ordinary process crash between rename and + commit replays (deduped) or drops each marker on the next read or write. The marker directory + fsync is a best-effort ordering aid and is not a power-loss durability guarantee. +- 다른 대안 대신 이 방식을 선택한 이유: The marker makes the audit row survive the same process-crash + window the atomic rename protects. The config rename and the audit commit are coordinated under the + shared mutation lock as one logical mutation, not a single transaction: a crash between the two + separate operations is recovered by the write-ahead marker (replay or drop), and a write that never + reached the rename cannot leave an audit row behind. +- 장점, 단점 및 영향: Recovery is deterministic (replay or drop, never a phantom row), retention is + bounded, and the management read never creates or hardens the coordinator directory; the audit + subsystem lives in `src/config-mutation-audit.ts` and must not import back into config/routing/ + server code (enforced by a module-boundary regression). diff --git a/structure/05_gui-and-management-api.md b/structure/05_gui-and-management-api.md index 7a38460fc4..d035caed52 100644 --- a/structure/05_gui-and-management-api.md +++ b/structure/05_gui-and-management-api.md @@ -118,6 +118,7 @@ this document owns is which module holds which area and what invariant that area | --- | --- | | Config/settings | Read safe config/settings views; mutate supported settings only. Full `PUT /api/config` is disabled so masked secrets are not round-tripped. `PUT /api/settings` accepts `codexAutoStart`, `streamMode`, integer `appOwnedMemoryBudgetMb` (64..4096), strict boolean `codexAccountPickerEnabled`, and a validated per-account `codexQuotaAutoRefresh` toggle (each optional, at least one required). Picker enable initializes an empty UI-managed selector map, persists before one bounded catalog convergence, and reports only `catalogRefreshPending`; allocation/save failure restores every touched live field and skips convergence. Budget changes synchronously enforce the process-wide evictable retained-state cap; this is separate from RSS/native memory. `streamMode` persists the #314 stream-shape selection in config.json (Windows services need persisted input; macOS eager relay is explicit-only). | | Startup safety | `GET /api/startup-health` reports whether injected Codex routing is restart-safe, with secret-free service/shim diagnostics. `POST /api/startup-action` provides allowlisted one-click installation for the background service or launcher shim. On Windows a healthy script shim is CLI-only; Codex Desktop requires the background service for full protection. | +| Config mutation audit | `GET /api/config/mutations` returns the bounded persisted-config mutation trail, newest first: 100 rows by default, up to 1000 per request, with 5000-row retention. Each row records the surface (`cli`/`api`/`internal`), route or command detail, changed field paths, and redacted before/after values (truncated at 4096 chars per entry). Raw credentials and request content are never stored; field paths are redacted before persistence. | | Windows tray | `GET/POST /api/windows-tray` controls an owned, per-user HKCU login tray. The tray delegates fixed actions to the CLI and is never a proxy supervisor or restart-protection signal. | | Updates | `GET /api/update/check`, `POST /api/update/run`, and `GET /api/update/status` own dashboard self-update state. A launched worker PID is persisted in `update-job.json`; dead PIDs recover immediately, while legacy active records without a PID recover only after ten minutes. Live PIDs remain exclusive regardless of record age. `GET /api/update/badge` backs the sidebar badge: it reports that an update exists and links to the update surface rather than gating other actions. | | Providers | Create/update/delete ordinary provider configs and enrich registry metadata. The reserved `openai` card exposes Pool(default)/Direct account mode; `openai-apikey` remains the separate API route. | diff --git a/tests/cli/cli-provider.test.ts b/tests/cli/cli-provider.test.ts index b83bc8d514..1aa63e77c2 100644 --- a/tests/cli/cli-provider.test.ts +++ b/tests/cli/cli-provider.test.ts @@ -4,6 +4,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; +import { Database } from "bun:sqlite"; import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -121,6 +122,13 @@ describe("ocx provider", () => { expect(config.providers.deepseek).toBeDefined(); expect(config.providers.deepseek.adapter).toBe("openai-chat"); expect(config.providers.deepseek.apiKey).toBe("sk-test"); + const db = new Database(join(dir, "config-mutation.sqlite"), { readonly: true }); + try { + const row = db.query("SELECT detail FROM config_mutation_audit ORDER BY id DESC LIMIT 1").get() as { detail: string } | null; + expect(row?.detail).toBe("ocx provider add"); + } finally { + db.close(); + } } finally { removeTreeWithRetry(dir); } diff --git a/tests/config-mutation-audit-boundary.test.ts b/tests/config-mutation-audit-boundary.test.ts new file mode 100644 index 0000000000..2e6341ca68 --- /dev/null +++ b/tests/config-mutation-audit-boundary.test.ts @@ -0,0 +1,33 @@ +import { expect, test } from "bun:test"; + +function moduleSpecifiers(src: string): string[] { + // Strip comments first so a forbidden path inside a comment cannot trip the + // allowlist, and so comments cannot hide a real import. + const withoutComments = src.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/[^\n]*/g, ""); + const specs: string[] = []; + const spec = /(?:\b(?:from|import|export)\s*[({]*\s*)(["'])([^"']+)\1/g; + for (const match of withoutComments.matchAll(spec)) { + specs.push(match[2] ?? ""); + } + return specs; +} + +test("config-mutation-audit leaf has no imports back into config/routing/server", async () => { + const src = await Bun.file(new URL("../src/config-mutation-audit.ts", import.meta.url)).text(); + // Keyword-anchored extraction (with whitespace/newlines between the keyword and + // the specifier) covers named, multiline, side-effect, dynamic import(), and + // re-export forms, so a path cannot slip past a line-based regex. + const specs = moduleSpecifiers(src); + expect(specs.length).toBeGreaterThan(0); + for (const spec of specs) { + // The leaf must stay acyclic: only Node/Bun builtins and lib/ (e.g. + // ./lib/redact) are allowed; no parent-directory imports and no config, + // routing, router, provider, or server modules. + const allowed = spec.startsWith("node:") || spec.startsWith("bun:") || spec.startsWith("./lib/"); + expect(allowed, `forbidden import in leaf: ${spec}`).toBe(true); + } + expect(specs).toContain("./lib/redact"); + // The pure snapshot contract lives in the leaf so tests can target it directly. + expect(src).toContain("export function buildConfigMutationSnapshot"); + expect(src).toContain("export function readConfigMutationAudit"); +}); diff --git a/tests/config-mutation-audit.test.ts b/tests/config-mutation-audit.test.ts new file mode 100644 index 0000000000..c79a5a8998 --- /dev/null +++ b/tests/config-mutation-audit.test.ts @@ -0,0 +1,1310 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { Database } from "bun:sqlite"; +import { + loadConfig, + mutatePersistedConfig, + readConfigMutationAudit, + saveConfig, + saveConfigPreservingClaudeCode, + setConfigAtomicWriteFailureForTests, + setConfigPostWriteFailureForTests, +} from "../src/config"; +import { + buildConfigMutationSnapshot, + configMutationPendingAuditPath, + insertConfigMutationAuditRow, + listPendingConfigMutationAuditPaths, + setConfigAuditMaxRowsForTests, + setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests, +} from "../src/config-mutation-audit"; +import { addProviderApiKey } from "../src/providers/api-keys"; +import { rotateKeyOn429 } from "../src/providers/key-failover"; +import { writeStorageCleanupPolicyToConfig } from "../src/storage/policy"; +import { setIntegrationEnabled } from "../src/codex/desired-state"; +import { + clearAccountNeedsReauth, + clearAccountQuota, + handleCodexAuthAPI, + markAccountNeedsReauth, + updateAccountQuota, +} from "../src/codex/auth-api"; +import { + clearCodexUpstreamHealth, + clearThreadAccountMap, + releaseDrainedCodexAccountPin, + resolveCodexAccountForThread, +} from "../src/codex/routing"; +import { saveCodexAccountCredential } from "../src/codex/account-store"; +import { POOL_KEY_CODEX, clearPoolRotationState, seedPoolRotationAccount } from "../src/codex/pool-rotation"; +import { commitKeyLoginProvider } from "../src/oauth/login-cli"; +import { clearClientConnection, commitClientConnection } from "../src/client/state"; +import type { OcxClientConnectionConfig, OcxConfig, StorageCleanupPolicy } from "../src/types"; +import { handleManagementAPI } from "../src/server/management-api"; +import { + resetPreservedDiskOnlyProvidersForTests, + setPreservedDiskOnlyProviders, +} from "../src/usage/user-cost-overlays"; +import type { OcxProviderConfig } from "../src/types"; + +let testRoot = ""; +let previousHome: string | undefined; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + testRoot = mkdtempSync(join(import.meta.dir, ".tmp-config-audit-")); + process.env.OPENCODEX_HOME = testRoot; + setConfigAuditMaxRowsForTests(5); +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + setConfigAuditMaxRowsForTests(null); + setConfigAtomicWriteFailureForTests(null); + setConfigPostWriteFailureForTests(null); + setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests(null); + resetPreservedDiskOnlyProvidersForTests(); + rmSync(testRoot, { recursive: true, force: true }); +}); + +function configWithProvider(port = 10100): OcxConfig { + return { + port, + defaultProvider: "blsc", + providers: { + blsc: { + adapter: "openai-chat", + baseUrl: "https://llmapi.blsc.cn/v1", + authMode: "key", + apiKey: "sk-super-secret-value", + }, + }, + } as unknown as OcxConfig; +} + +function markerFile(mutationId: string): string { + return configMutationPendingAuditPath(testRoot, mutationId); +} + +describe("config mutation audit log", () => { + test("api-key pool writers record api surface and operation detail", () => { + saveConfig(configWithProvider()); + const live = loadConfig(); + addProviderApiKey(live, "blsc", "sk-second-key"); + rotateKeyOn429(live, "blsc", null); + const { rows } = readConfigMutationAudit(); + expect(rows).toHaveLength(3); + expect(rows[0].surface).toBe("internal"); + expect(rows[0].detail).toBe("key-failover: rotate active provider key"); + expect(rows[1].surface).toBe("api"); + expect(rows[1].detail).toBe("api-keys: add provider key"); + expect(JSON.stringify(rows[1].after)).not.toContain("sk-second-key"); + }); + + test("storage cleanup policy writes record api surface and operation detail", () => { + saveConfig(configWithProvider()); + const policy: StorageCleanupPolicy = { + enabled: true, + trigger: { archivedBytesOver: 1024 * 1024 }, + target: { reduceToBytes: 512 * 1024 }, + schedule: "manual", + mode: "quarantine", + }; + writeStorageCleanupPolicyToConfig(policy); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("storage-policy: write cleanup policy"); + expect(rows[0].fields).toContain("storageCleanupPolicy"); + }); + + test("cli integration mutations record the invoking command detail", () => { + saveConfig(configWithProvider()); + const live = loadConfig(); + setIntegrationEnabled("codex", false, { surface: "cli", detail: "ocx restore" }); + setIntegrationEnabled("codex", true, { surface: "cli", detail: "ocx restore back" }); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("cli"); + expect(rows[0].detail).toBe("ocx restore back"); + // Re-enable removes clientIntegrations when no disabled clients remain; the + // audit snapshot records the removed top-level key as null. + expect((rows[0].after as Record).clientIntegrations).toBeNull(); + expect(rows[1].surface).toBe("cli"); + expect(rows[1].detail).toBe("ocx restore"); + expect((rows[1].after as Record).clientIntegrations).toEqual({ codex: false }); + }); + + test("routing pin clears record unavailable vs drained provenance", () => { + saveConfig(configWithProvider()); + const live = loadConfig(); + live.activeCodexAccountPinned = "reauth-acc"; + saveConfigPreservingClaudeCode(live, { surface: "internal", detail: "test: set reauth pin" }); + markAccountNeedsReauth("reauth-acc"); + try { + releaseDrainedCodexAccountPin(live, {}); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("internal"); + expect(rows[0].detail).toBe("routing: clear unavailable codex account pin"); + expect((JSON.parse(readFileSync(join(testRoot, "config.json"), "utf8")) as Record).activeCodexAccountPinned) + .toBeUndefined(); + } finally { + clearAccountNeedsReauth("reauth-acc"); + } + const live2 = loadConfig(); + live2.activeCodexAccountPinned = "ghost-acc"; + saveConfigPreservingClaudeCode(live2, { surface: "internal", detail: "test: set pin" }); + releaseDrainedCodexAccountPin(live2, {}); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("internal"); + expect(rows[0].detail).toBe("routing: clear drained codex account pin"); + expect((JSON.parse(readFileSync(join(testRoot, "config.json"), "utf8")) as Record).activeCodexAccountPinned) + .toBeUndefined(); + }); + + test("pin-only active selection records the pin-clear audit detail", () => { + clearThreadAccountMap(); + clearCodexUpstreamHealth(); + clearPoolRotationState(); + clearAccountQuota(); + try { + saveConfig(configWithProvider()); + const live = loadConfig(); + live.codexAccounts = [ + { id: "a", email: "a@test", isMain: false }, + { id: "b", email: "b@test", isMain: false }, + { id: "c", email: "c@test", isMain: false }, + ]; + live.activeCodexAccountId = "a"; + live.accountPoolStrategy = "round-robin"; + // A stale pin on a healthy third account: moving an affined thread back onto + // the already-active account persists only the pin clear, so the audit row + // must say so instead of claiming a new active-account selection. + live.activeCodexAccountPinned = "c"; + saveConfigPreservingClaudeCode(live, { surface: "internal", detail: "test: seed stale pin" }); + saveCodexAccountCredential("a", { + accessToken: "access-a", + refreshToken: "refresh-a", + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: "acct-a", + }); + saveCodexAccountCredential("b", { + accessToken: "access-b", + refreshToken: "refresh-b", + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: "acct-b", + }); + saveCodexAccountCredential("c", { + accessToken: "access-c", + refreshToken: "refresh-c", + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: "acct-c", + }); + const config = loadConfig(); + seedPoolRotationAccount(POOL_KEY_CODEX, "b"); + // Bind the thread to b first so the quota re-evaluation below can move it + // back onto the already-active account a. + expect(resolveCodexAccountForThread("affined", config)).toBe("b"); + config.accountPoolStrategy = "quota"; + saveConfigPreservingClaudeCode(config, { surface: "internal", detail: "test: switch to quota" }); + updateAccountQuota("a", 10); + updateAccountQuota("b", 90); + updateAccountQuota("c", 10); + expect(resolveCodexAccountForThread("affined", config)).toBe("a"); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("internal"); + expect(rows[0].detail).toBe("routing: clear codex account pin"); + // The pin-only save must not touch any operator configuration field; the + // audit writer's own configRebaseProvenance bookkeeping is the only extra. + expect(rows[0].fields.filter(field => field !== "configRebaseProvenance")).toEqual(["activeCodexAccountPinned"]); + const before = rows[0].before as Record; + const after = rows[0].after as Record; + // Snapshots contain only changed fields: unchanged operator state is absent. + expect(before).not.toHaveProperty("activeCodexAccountId"); + expect(after).not.toHaveProperty("activeCodexAccountId"); + expect(before).not.toHaveProperty("accountPoolStrategy"); + expect(after).not.toHaveProperty("accountPoolStrategy"); + expect(before).not.toHaveProperty("codexAccounts"); + expect(after).not.toHaveProperty("codexAccounts"); + expect(before.activeCodexAccountPinned).toBe("c"); + expect(after.activeCodexAccountPinned).toBeNull(); + } finally { + clearThreadAccountMap(); + clearCodexUpstreamHealth(); + clearPoolRotationState(); + clearAccountQuota(); + } + }); + + test("saveConfig records source, changed fields, and redacts secrets", () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx test write" }); + const { rows } = readConfigMutationAudit(); + expect(rows).toHaveLength(1); + expect(rows[0].surface).toBe("cli"); + expect(rows[0].detail).toBe("ocx test write"); + expect(rows[0].fields).toEqual([""]); + expect(JSON.stringify(rows[0].after)).not.toContain("sk-super-secret-value"); + expect(JSON.stringify(rows[0].after)).toContain("[REDACTED]"); + expect(JSON.stringify(rows[0].before)).toBe(JSON.stringify({ "": null })); + }); + + test("a byte-identical save records nothing", () => { + saveConfig(configWithProvider()); + const before = readConfigMutationAudit().rows.length; + saveConfig(configWithProvider()); + expect(readConfigMutationAudit().rows.length).toBe(before); + }); + + test("mutatePersistedConfig records fine-grained fields with redaction", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers.blsc.apiKey = "sk-new-secret-value"; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers/blsc" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + expect(rows).toHaveLength(2); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/providers/blsc"); + expect(rows[0].fields).toContain("providers.blsc.apiKey"); + expect(JSON.stringify(rows[0].before)).not.toContain("sk-super-secret-value"); + expect(JSON.stringify(rows[0].after)).not.toContain("sk-new-secret-value"); + expect(JSON.stringify(rows[0].after)).toContain("[REDACTED]"); + }); + + test("saveConfigPreservingClaudeCode records the changed top-level field", () => { + saveConfig(configWithProvider()); + const live = loadConfig(); + live.streamMode = "eager-relay"; + saveConfigPreservingClaudeCode(live, { surface: "api", detail: "PUT /api/settings" }); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/settings"); + expect(rows[0].fields).toContain("streamMode"); + expect(rows[0].after.streamMode).toBe("eager-relay"); + }); + + test("retention keeps only the newest bounded rows", () => { + setConfigAuditMaxRowsForTests(3); + for (let i = 0; i < 5; i += 1) saveConfig(configWithProvider(10100 + i)); + const { rows, maxRows } = readConfigMutationAudit(); + expect(maxRows).toBe(3); + expect(rows).toHaveLength(3); + // Newest first: the last three ports survive. + expect(rows[0].fields).toContain("port"); + expect(rows.map(row => row.after.port)).toEqual([10104, 10103, 10102]); + expect(rows.map(row => row.before.port)).not.toContain(10100); + }); + + test("buildConfigMutationSnapshot is bounded and redacts secrets", () => { + const snapshot = buildConfigMutationSnapshot( + { providers: { a: { apiKey: "sk-old", baseUrl: "u" } }, port: 1 }, + { providers: { a: { apiKey: "sk-new", baseUrl: "u" } }, port: 2 }, + ); + expect(snapshot.fields.sort()).toEqual(["port", "providers.a.apiKey"]); + expect(JSON.stringify(snapshot.before)).not.toContain("sk-old"); + expect(JSON.stringify(snapshot.after)).not.toContain("sk-new"); + expect(JSON.stringify(snapshot.after)).toContain("[REDACTED]"); + }); + + test("provider header values are redacted regardless of the header name", () => { + const snapshot = buildConfigMutationSnapshot( + { providers: { p: { headers: { "X-Custom-Auth": "opaque-before" } } } }, + { providers: { p: { headers: { "X-Custom-Auth": "opaque-after" } } } }, + ); + expect(snapshot.fields).toContain("providers.p.headers"); + expect(JSON.stringify(snapshot.before)).not.toContain("opaque-before"); + expect(JSON.stringify(snapshot.after)).not.toContain("opaque-after"); + expect(JSON.stringify(snapshot.before)).toContain("[REDACTED]"); + expect(JSON.stringify(snapshot.after)).toContain("[REDACTED]"); + }); + + test("provider header values are redacted inside a whole-config snapshot", () => { + const snapshot = buildConfigMutationSnapshot( + undefined, + { providers: { p: { headers: { "X-Auth-Token": "opaque-value" } } }, port: 10100 }, + ); + expect(snapshot.fields).toEqual([""]); + expect(JSON.stringify(snapshot.after)).not.toContain("opaque-value"); + expect(JSON.stringify(snapshot.after)).toContain("[REDACTED]"); + }); + + test("a secret-shaped provider name is redacted in the changed-field paths", () => { + saveConfig(configWithProvider()); + const tokenName = "sk-" + "live-" + "abcdefghijklmnopqrstuvwxyz012345"; + const outcome = mutatePersistedConfig(persisted => { + persisted.providers[tokenName] = { + adapter: "openai-chat", + baseUrl: "https://example.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + expect(JSON.stringify(rows[0].fields)).not.toContain(tokenName); + }); + + test("dotted provider names keep their before/after values", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers["my.provider"] = { + adapter: "openai-chat", + baseUrl: "https://example.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + expect(rows[0].fields).toContain("providers.my.provider"); + expect(rows[0].after["providers.my.provider"]).toEqual({ + adapter: "openai-chat", + baseUrl: "https://example.invalid/v1", + }); + }); + + test("credential-shaped leaves are redacted by key matcher", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers.blsc = { + ...persisted.providers.blsc, + apiKeyPool: [{ key: "sk-pool-secret-value" }], + oauthClientSecret: "oauth-client-secret-value", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers/blsc" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain("sk-pool-secret-value"); + expect(serialized).not.toContain("oauth-client-secret-value"); + expect(serialized).toContain("[REDACTED]"); + }); + + test("apiKeys entry key is redacted even inside a whole-config snapshot", () => { + const rawAdmissionKey = "ocx_data_admission_secret_do_not_leak"; + saveConfig({ + ...configWithProvider(), + apiKeys: [{ + id: "admission-1", + name: "benchmark key", + key: rawAdmissionKey, + createdAt: "2026-08-23T00:00:00.000Z", + }], + }, { surface: "api", detail: "PUT /api/admission-keys" }); + const first = readConfigMutationAudit(); + expect(first.rows).toHaveLength(1); + expect(JSON.stringify(first.rows[0])).not.toContain(rawAdmissionKey); + expect(JSON.stringify(first.rows[0])).toContain("[REDACTED]"); + // A later mutation that changes only the key field must also be masked. + const outcome = mutatePersistedConfig(persisted => { + persisted.apiKeys = [{ + id: "admission-1", + name: "benchmark key", + key: "ocx_data_second_secret_do_not_leak", + createdAt: "2026-08-23T00:00:00.000Z", + }]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/admission-keys" }); + expect(outcome.status).toBe("committed"); + const second = readConfigMutationAudit(); + expect(JSON.stringify(second.rows[0])).not.toContain("ocx_data_second_secret_do_not_leak"); + expect(second.rows[0].fields).toContain("apiKeys"); + }); + + test("degraded apiKeys entries (missing metadata) are redacted in before and after", () => { + const rawAdmissionKey = "ocx_data_degraded_secret_do_not_leak"; + // A hand-edited / older row with only key+name: the schema salvages it, and the + // before snapshot is built from the RAW disk bytes, so the mask must not depend + // on the full happy-path entry shape. + const configPath = join(testRoot, "config.json"); + writeFileSync(configPath, JSON.stringify({ + ...configWithProvider(), + apiKeys: [{ key: rawAdmissionKey, name: "degraded" }], + }, null, 2) + "\n"); + const outcome = mutatePersistedConfig(persisted => { + persisted.apiKeys = [{ + id: "degraded-1", + name: "degraded", + key: rawAdmissionKey, + createdAt: "2026-08-23T00:00:00.000Z", + }]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/admission-keys" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain(rawAdmissionKey); + expect(serialized).toContain("[REDACTED]"); + }); + + test("a degraded apiKeys entry with only a key is redacted from rows and the endpoint", async () => { + const rawAdmissionKey = "ocx_data_minimal_degraded_secret_do_not_leak"; + // A hand-edited row with NO id/name/createdAt must still be masked: the raw + // before snapshot has no happy-path metadata for the heuristic to latch onto. + const configPath = join(testRoot, "config.json"); + writeFileSync(configPath, JSON.stringify({ + ...configWithProvider(), + apiKeys: [{ key: rawAdmissionKey }], + }, null, 2) + "\n"); + const outcome = mutatePersistedConfig(persisted => { + persisted.apiKeys = [{ key: "ocx_data_minimal_second_secret_do_not_leak" }]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/admission-keys" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain(rawAdmissionKey); + expect(serialized).not.toContain("ocx_data_minimal_second_secret_do_not_leak"); + expect(serialized).toContain("[REDACTED]"); + // The management endpoint must not echo the plaintext admission keys either. + const url = new URL("http://127.0.0.1:10100/api/config/mutations?limit=5"); + const response = await handleManagementAPI( + new Request(url, { headers: { Host: "127.0.0.1:10100" } }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response).not.toBeNull(); + const body = await response!.json() as { mutations: Array> }; + expect(JSON.stringify(body)).not.toContain(rawAdmissionKey); + expect(JSON.stringify(body)).not.toContain("ocx_data_minimal_second_secret_do_not_leak"); + }); + + test("the pending marker never contains the raw apiKeys admission secret", () => { + setConfigAtomicWriteFailureForTests(() => new Error("stop after marker")); + expect(() => saveConfig({ + ...configWithProvider(), + apiKeys: [{ + id: "marker-1", + name: "marker key", + key: "ocx_data_marker_secret_do_not_leak", + createdAt: "2026-08-23T00:00:00.000Z", + }], + }, { surface: "api", detail: "PUT /api/admission-keys" })).toThrow("stop after marker"); + const marker = readFileSync(listPendingConfigMutationAuditPaths(testRoot)[0]!, "utf8"); + expect(marker).not.toContain("ocx_data_marker_secret_do_not_leak"); + expect(marker).toContain("[REDACTED]"); + }); + + test("apiKeyPool rows are redacted by key name even without an sk- prefix", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers.blsc.apiKeyPool = [{ key: "plain-pool-secret-value" }]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers/blsc" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain("plain-pool-secret-value"); + expect(serialized).toContain("[REDACTED]"); + }); + + test("redacted field labels stay unique when distinct paths collapse", () => { + saveConfig(configWithProvider()); + const firstName = "sk-" + "live-" + "a".repeat(30); + const secondName = "sk-" + "live-" + "b".repeat(30); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers[firstName] = { + adapter: "openai-chat", + baseUrl: "https://a.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + persisted.providers[secondName] = { + adapter: "openai-chat", + baseUrl: "https://b.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const fields = rows[0].fields; + // Distinct paths that both redact to providers.[REDACTED]. keep unique labels. + expect(new Set(fields).size).toBe(fields.length); + const redactedFields = fields.filter(field => field.includes("[REDACTED]")); + expect(redactedFields.length).toBeGreaterThanOrEqual(2); + for (const field of redactedFields) { + expect(rows[0].after[field]).toBeDefined(); + } + }); + + test("persisted fields match the before/after keys after bounding and dedup", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + const longA = "a".repeat(300); + const longB = "b".repeat(300); + persisted.providers[longA] = { + adapter: "openai-chat", + baseUrl: "https://a.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + persisted.providers[longB] = { + adapter: "openai-chat", + baseUrl: "https://b.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const beforeKeys = Object.keys(rows[0].before).sort(); + const afterKeys = Object.keys(rows[0].after).sort(); + // The persisted fields array must reference exactly the labels stored in + // before/after; insertion must not re-bound them into a different label. + expect([...rows[0].fields].sort()).toEqual(beforeKeys); + expect([...rows[0].fields].sort()).toEqual(afterKeys); + for (const field of rows[0].fields) { + expect(rows[0].after[field]).toBeDefined(); + } + }); + + test("URL userinfo is redacted before a large value is truncated", () => { + saveConfig(configWithProvider()); + const userinfoUrl = "https://user:top-secret-userinfo@relay.test/v1/" + "x".repeat(10_000); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers.blsc.baseUrl = userinfoUrl; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers/blsc" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain("top-secret-userinfo"); + expect(serialized).toContain("[REDACTED]"); + expect(serialized).toContain("[truncated]"); + }); + + test("URL userinfo is redacted when embedded or repeated in a string", () => { + const note = "see https://user:secret-one@relay.test/a and https://second:pw@relay.test/b"; + const snapshot = buildConfigMutationSnapshot( + { note }, + { note: note + " updated" }, + ); + const serialized = JSON.stringify(snapshot.before); + expect(serialized).not.toContain("secret-one"); + expect(serialized).not.toContain("second:pw"); + expect(serialized).toContain("https://[REDACTED]@relay.test/a"); + expect(serialized).toContain("https://[REDACTED]@relay.test/b"); + }); + + test("__proto__ keys survive in audit snapshot fields and values", () => { + const before = { note: JSON.parse('{"__proto__":{"value":1}}') as Record }; + const after = { note: "replaced" }; + const snapshot = buildConfigMutationSnapshot(before, after); + expect(snapshot.fields).toContain("note"); + expect(JSON.stringify(snapshot.before)).toContain('"__proto__"'); + expect(JSON.stringify(snapshot.before)).toContain('"value":1'); + }); + + test("a disk-only provider is not reported as deleted", () => { + saveConfig(configWithProvider()); + + // Simulate an external editor adding a provider the in-memory config never saw. + const configPath = join(testRoot, "config.json"); + const onDisk = JSON.parse(readFileSync(configPath, "utf-8")) as Record; + const staging = { adapter: "openai-chat", baseUrl: "https://staging.invalid/v1" }; + onDisk.providers.staging = staging; + writeFileSync(configPath, JSON.stringify(onDisk, null, 2) + "\n"); + // Mirror the running server: the admission snapshot has already seen the disk-only row. + setPreservedDiskOnlyProviders({ staging } as Record); + + saveConfig(configWithProvider(10500), { surface: "cli", detail: "ocx port change" }); + + const { rows } = readConfigMutationAudit(); + expect(rows[0].fields).not.toContain("providers.staging"); + // The provider must still be on disk. + const after = JSON.parse(readFileSync(configPath, "utf-8")) as Record; + expect(after.providers.staging).toBeDefined(); + }); + + test("a crash after the config rename is replayed from the pending marker", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + // Simulate a crash between the config.json rename and the audit-row commit: + // disk already carries the new bytes, the audit row does not exist yet. + const next = configWithProvider(10500); + const bytes = JSON.stringify(next, null, 2) + "\n"; + writeFileSync(configPath, bytes); + writeFileSync(markerFile("crash-replay"), JSON.stringify({ + mutationId: "crash-replay", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/test-crash", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + + // The next write (even a byte-identical retry) reconciles the marker first. + saveConfig(configWithProvider(10500), { surface: "cli", detail: "ocx retry" }); + + const { rows } = readConfigMutationAudit(); + expect(rows.some(row => row.detail === "PUT /api/test-crash")).toBe(true); + // The byte-identical retry records nothing of its own. + expect(rows.some(row => row.detail === "ocx retry")).toBe(false); + expect(existsSync(markerFile("crash-replay"))).toBe(false); + }); + + test("a pending marker whose rename never landed is dropped without a phantom row", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(markerFile("never-landed"), JSON.stringify({ + mutationId: "never-landed", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/test-never-landed", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + + saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" }); + + const { rows } = readConfigMutationAudit(); + expect(rows.some(row => row.detail === "PUT /api/test-never-landed")).toBe(false); + expect(rows[0].detail).toBe("ocx next"); + expect(existsSync(markerFile("never-landed"))).toBe(false); + }); + + test("a malformed marker with non-string field labels is skipped without blocking later writes", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const markerPath = markerFile("bad-fields"); + writeFileSync(markerPath, JSON.stringify({ + mutationId: "bad-fields", + createdAt: Date.now(), + surface: "api", + detail: "PUT /api/test-bad-fields", + fields: ["port", null], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(JSON.stringify({ port: 10500 })).digest("hex"), + })); + // The malformed marker must not crash recovery or block the next write. + saveConfig(configWithProvider(10500), { surface: "cli", detail: "ocx next" }); + const { rows } = readConfigMutationAudit(); + expect(rows.some(row => row.detail === "PUT /api/test-bad-fields")).toBe(false); + expect(rows[0].detail).toBe("ocx next"); + }); + + test("the read path replays an orphaned pending marker without duplicating", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(configPath, bytes); + writeFileSync(markerFile("orphan-replay"), JSON.stringify({ + mutationId: "orphan-replay", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/test-crash", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + + const first = readConfigMutationAudit(); + const second = readConfigMutationAudit(); + expect(first.rows.filter(row => row.detail === "PUT /api/test-crash")).toHaveLength(1); + expect(second.rows.filter(row => row.detail === "PUT /api/test-crash")).toHaveLength(1); + expect(existsSync(markerFile("orphan-replay"))).toBe(false); + }); + + test("read-side recovery retains an in-flight marker whose rename has not landed", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + const mutationId = "in-flight-write"; + writeFileSync(markerFile(mutationId), JSON.stringify({ + mutationId, + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/in-flight", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + + // The marker is written BEFORE the config rename lands; a read in this window + // must not delete the marker or record a phantom row. + const first = readConfigMutationAudit(); + expect(first.rows.some(row => row.detail === "PUT /api/in-flight")).toBe(false); + expect(existsSync(markerFile(mutationId))).toBe(true); + + // The rename lands now; the next recovery replays exactly one audit row. + writeFileSync(configPath, bytes); + const second = readConfigMutationAudit(); + expect(second.rows.filter(row => row.detail === "PUT /api/in-flight")).toHaveLength(1); + expect(existsSync(markerFile(mutationId))).toBe(false); + }); + + test("a parseable-but-invalid pending marker is dropped by the read path", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const markerPath = markerFile("invalid-marker"); + // JSON.parse succeeds but validation fails (afterSha256 missing): the read path + // drops the unusable marker immediately so recovery does not reprocess it + // on every read or write. + writeFileSync(markerPath, JSON.stringify({ + mutationId: "invalid-marker", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/invalid-marker", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + })); + readConfigMutationAudit(); + expect(existsSync(markerPath)).toBe(false); + const { rows } = readConfigMutationAudit(); + expect(rows.some(row => row.detail === "PUT /api/invalid-marker")).toBe(false); + // The next successful save overwrites and drops the invalid marker without + // ever recording a phantom row. + saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" }); + expect(existsSync(markerPath)).toBe(false); + const after = readConfigMutationAudit(); + expect(after.rows.some(row => row.detail === "PUT /api/invalid-marker")).toBe(false); + expect(after.rows[0].detail).toBe("ocx next"); + }); + + test("a pending marker whose root is JSON null is removed by the read path", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const markerPath = markerFile("null-marker"); + writeFileSync(markerPath, "null"); + readConfigMutationAudit(); + expect(existsSync(markerPath)).toBe(false); + const { rows } = readConfigMutationAudit(); + expect(rows.some(row => row.detail === "PUT /api/null-marker")).toBe(false); + }); + + test("undefined-valued keys and absent keys compare equal after JSON semantics", () => { + const before = { providers: { p: { retryOn429: { attempts: undefined } } } }; + const after = { providers: { p: { retryOn429: {} } } }; + const snapshot = buildConfigMutationSnapshot(before, after); + expect(snapshot.fields).toEqual([]); + }); + + test("a rollback after reconciliation keeps the recovered audit row committed", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(configPath, bytes); + const markerPath = markerFile("rollback-replay"); + writeFileSync(markerPath, JSON.stringify({ + mutationId: "rollback-replay", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/test-crash", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + + // Reconciliation commits the recovered row in its own transaction, so a + // mutation that fails afterwards cannot roll the row back or let a new marker + // overwrite the marker whose replay already committed. + expect(() => mutatePersistedConfig(() => { + throw new Error("mutation failed after reconciliation"); + }, { surface: "api", detail: "PUT /api/fails" })).toThrow(); + let audit = readConfigMutationAudit(); + expect(audit.rows.some(row => row.detail === "PUT /api/test-crash")).toBe(true); + expect(existsSync(markerPath)).toBe(false); + + saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" }); + audit = readConfigMutationAudit(); + expect(audit.rows.some(row => row.detail === "PUT /api/test-crash")).toBe(true); + expect(audit.rows.filter(row => row.detail === "PUT /api/test-crash")).toHaveLength(1); + expect(audit.rows[0].detail).toBe("ocx next"); + }); + + test("a failed config write cannot let a new marker replace a recovered row", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + // Simulate a crash after an earlier rename: disk carries the new bytes and the + // audit row has not committed yet (recovered row C1). + const interruptedBytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(configPath, interruptedBytes); + writeFileSync(markerFile("recovered-row"), JSON.stringify({ + mutationId: "recovered-row", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/test-crash", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(interruptedBytes).digest("hex"), + })); + + // The next save writes a NEW marker (P2), then fails before the config rename + // lands. P2 must not be able to clobber the C1 row that already committed. + setConfigAtomicWriteFailureForTests(() => new Error("simulated config write failure")); + expect(() => saveConfig(configWithProvider(10600), { + surface: "api", + detail: "PUT /api/failed-write", + })).toThrow("simulated config write failure"); + // The failed write left its OWN per-mutation marker behind and the config + // rename never landed; the recovered marker was already removed by the lock + // recovery at the start of this mutation. + const failedMarkers = listPendingConfigMutationAuditPaths(testRoot); + expect(failedMarkers).toHaveLength(1); + expect(readFileSync(failedMarkers[0]!, "utf8")).toContain("PUT /api/failed-write"); + expect(existsSync(markerFile("recovered-row"))).toBe(false); + expect(readFileSync(configPath, "utf8")).toBe(interruptedBytes); + + const audit = readConfigMutationAudit(); + // The recovered C1 row survives exactly once; the failed write recorded nothing. + expect(audit.rows.filter(row => row.detail === "PUT /api/test-crash")).toHaveLength(1); + expect(audit.rows.some(row => row.detail === "PUT /api/failed-write")).toBe(false); + + // The next successful save reconciles P2 (rename never landed -> dropped) and + // C1 remains committed exactly once. + saveConfig(configWithProvider(10700), { surface: "cli", detail: "ocx next" }); + const after = readConfigMutationAudit(); + expect(after.rows.filter(row => row.detail === "PUT /api/test-crash")).toHaveLength(1); + expect(after.rows.some(row => row.detail === "PUT /api/failed-write")).toBe(false); + expect(after.rows[0].detail).toBe("ocx next"); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(0); + }); + + test("a crash between the config rename and the audit commit replays exactly one row", () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + const configPath = join(testRoot, "config.json"); + const before = readFileSync(configPath, "utf8"); + setConfigPostWriteFailureForTests(() => new Error("simulated post-rename failure")); + expect(() => saveConfig(configWithProvider(10500), { + surface: "api", + detail: "PUT /api/post-rename", + })).toThrow("simulated post-rename failure"); + // The rename landed: disk carries the new bytes even though the audit row + // never committed; the write-ahead marker is the only durable trace. + expect(readFileSync(configPath, "utf8")).not.toBe(before); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(1); + // Inspect the store directly: the read-path recovery must not be triggered + // before the next save so the replay actually exercises the marker. + const db = new Database(join(testRoot, "config-mutation.sqlite"), { readonly: true }); + try { + const count = db.query("SELECT COUNT(*) AS n FROM config_mutation_audit WHERE detail = ?").get("PUT /api/post-rename") as { n: number }; + expect(count.n).toBe(0); + } finally { + db.close(); + } + // The next save recovers the interrupted write: exactly one row for the + // post-rename mutation id, and the marker is removed. + saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" }); + const audit = readConfigMutationAudit(); + expect(audit.rows.filter(row => row.detail === "PUT /api/post-rename")).toHaveLength(1); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(0); + }); + + test("a marker that cannot be unlinked after a recovery commit does not fail the save", () => { + saveConfig(configWithProvider(10100)); + const configPath = join(testRoot, "config.json"); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(configPath, bytes); + // A directory at the marker path makes unlink fail (EISDIR/EPERM) like a + // transient handle hold. The recovery COMMIT already succeeded, so a leftover + // marker must never turn the next save into a reported failure. + const lockedMarker = markerFile("locked-marker"); + mkdirSync(lockedMarker, { recursive: true }); + writeFileSync(join(lockedMarker, "placeholder"), "x"); + expect(() => saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" })).not.toThrow(); + expect(readFileSync(configPath, "utf8")).toContain("\"port\": 10600"); + const audit = readConfigMutationAudit(); + expect(audit.rows[0].detail).toBe("ocx next"); + }); + + test("proxy URL userinfo is never stored in the pending marker", () => { + saveConfig(configWithProvider()); + setConfigAtomicWriteFailureForTests(() => new Error("stop after marker")); + expect(() => mutatePersistedConfig(persisted => { + persisted.proxy = "http://user:supersecretpw@127.0.0.1:8080"; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/proxy" })).toThrow("stop after marker"); + const marker = readFileSync(listPendingConfigMutationAuditPaths(testRoot)[0]!, "utf8"); + expect(marker).not.toContain("supersecretpw"); + expect(marker).toContain("[REDACTED]"); + }); + + test("proxy URL userinfo is redacted from audit rows", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.proxy = "http://user:supersecretpw@127.0.0.1:8080"; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/proxy" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain("supersecretpw"); + expect(serialized).toContain("[REDACTED]"); + }); + + test("userinfo containing multiple @ characters is fully redacted", () => { + saveConfig(configWithProvider()); + const outcome = mutatePersistedConfig(persisted => { + persisted.proxy = "http://user:p@ss-word@127.0.0.1:8080"; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/proxy" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const serialized = JSON.stringify(rows[0]); + expect(serialized).not.toContain("p@ss-word"); + expect(serialized).toContain("[REDACTED]@127.0.0.1"); + }); + + test("duplicate overlong labels keep a distinct occurrence suffix", () => { + saveConfig(configWithProvider()); + const shared = "a".repeat(300); + const outcome = mutatePersistedConfig(persisted => { + persisted.providers[`${shared}1`] = { + adapter: "openai-chat", + baseUrl: "https://a.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + persisted.providers[`${shared}2`] = { + adapter: "openai-chat", + baseUrl: "https://b.invalid/v1", + } as unknown as OcxConfig["providers"][string]; + return { changed: true, value: true }; + }, { surface: "api", detail: "PUT /api/providers" }); + expect(outcome.status).toBe("committed"); + const { rows } = readConfigMutationAudit(); + const fields = rows[0].fields; + expect(new Set(fields).size).toBe(fields.length); + const base = fields[0]!; + expect(base).toBeDefined(); + const suffixed = fields.find(field => field !== base)!; + expect(suffixed.endsWith("#2")).toBe(true); + for (const field of fields) { + expect(field.length).toBeLessThanOrEqual(256); + expect(rows[0].before[field]).toBeDefined(); + expect(rows[0].after[field]).toBeDefined(); + } + }); + + test("recovery deletes only the exact marker it reconciled, never a newer one", () => { + saveConfig(configWithProvider(10100)); + const configPath = join(testRoot, "config.json"); + const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + writeFileSync(configPath, bytes); + const oldMarker = markerFile("old-marker"); + writeFileSync(oldMarker, JSON.stringify({ + mutationId: "old-marker", + createdAt: 1234567890, + surface: "api", + detail: "PUT /api/old", + fields: ["port"], + before: { port: 10100 }, + after: { port: 10500 }, + afterSha256: createHash("sha256").update(bytes).digest("hex"), + })); + // A marker written after the recovery snapshot but before cleanup must be + // untouched: cleanup only deletes the exact paths it reconciled. + const newerMarker = markerFile("newer-marker"); + setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests(() => { + writeFileSync(newerMarker, JSON.stringify({ + mutationId: "newer-marker", + createdAt: 1234567891, + surface: "api", + detail: "PUT /api/newer", + fields: ["port"], + before: { port: 10500 }, + after: { port: 10600 }, + afterSha256: createHash("sha256").update(JSON.stringify(configWithProvider(10600), null, 2) + "\n").digest("hex"), + })); + }); + readConfigMutationAudit(); + expect(existsSync(oldMarker)).toBe(false); + expect(existsSync(newerMarker)).toBe(true); + }); + + test("insert dedupes by mutation id, not by identical same-millisecond content", () => { + const db = new Database(join(testRoot, "config-mutation.sqlite"), { create: true }); + try { + insertConfigMutationAuditRow(db, "m1", 1234567890, { surface: "cli", detail: "ocx same" }, ["port"], { port: 1 }, { port: 2 }); + insertConfigMutationAuditRow(db, "m2", 1234567890, { surface: "cli", detail: "ocx same" }, ["port"], { port: 1 }, { port: 2 }); + insertConfigMutationAuditRow(db, "m1", 1234567890, { surface: "cli", detail: "ocx same" }, ["port"], { port: 1 }, { port: 2 }); + const count = db.query("SELECT COUNT(*) AS n FROM config_mutation_audit").get() as { n: number }; + expect(count.n).toBe(2); + } finally { + db.close(); + } + }); + + test("audit detail and field labels are bounded", () => { + const db = new Database(join(testRoot, "config-mutation.sqlite"), { create: true }); + try { + const snapshot = buildConfigMutationSnapshot( + { providers: { ["x".repeat(300)]: { baseUrl: "https://a.invalid/v1" } } }, + { providers: { ["x".repeat(300)]: { baseUrl: "https://b.invalid/v1" } } }, + ); + insertConfigMutationAuditRow(db, "m-bound", 1, { surface: "cli", detail: "d".repeat(1000) }, snapshot.fields, snapshot.before, snapshot.after); + const row = db.query("SELECT detail, fields, before_json AS beforeJson FROM config_mutation_audit LIMIT 1").get() as { detail: string; fields: string; beforeJson: string }; + expect(row.detail.length).toBeLessThanOrEqual(512); + const storedFields = JSON.parse(row.fields) as string[]; + const storedBefore = JSON.parse(row.beforeJson) as Record; + // Insertion stores the final labels as-is; every label is bounded and + // still referenced by the before/after snapshot keys. + expect(storedFields).toEqual(snapshot.fields); + for (const field of storedFields) { + expect(field.length).toBeLessThanOrEqual(256); + expect(Object.keys(storedBefore)).toContain(field); + } + } finally { + db.close(); + } + }); +}); + +describe("config mutation audit management API", () => { + test("GET /api/config/mutations returns the bounded trail newest-first", async () => { + saveConfig(configWithProvider(10100), { surface: "cli", detail: "ocx first" }); + saveConfig(configWithProvider(10200), { surface: "api", detail: "PUT /api/test" }); + const url = new URL("http://127.0.0.1:10100/api/config/mutations?limit=1"); + const response = await handleManagementAPI( + new Request(url, { headers: { Host: "127.0.0.1:10100" } }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response).not.toBeNull(); + const body = await response!.json() as { mutations: Array<{ detail: string }>; retention: { maxRows: number } }; + expect(body.mutations).toHaveLength(1); + expect(body.mutations[0].detail).toBe("PUT /api/test"); + expect(body.retention.maxRows).toBe(5); + }); + + test("GET /api/config/mutations rejects anonymous and unauthorized principals", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/config/mutations"); + const request = () => new Request(url, { headers: { Host: "127.0.0.1:10100" } }); + const anonymous = await handleManagementAPI(request(), url, loadConfig()); + expect(anonymous?.status).toBe(401); + const capability = await handleManagementAPI(request(), url, loadConfig(), {}, "local-read-capability"); + expect(capability?.status).toBe(403); + const admin = await handleManagementAPI(request(), url, loadConfig(), {}, "admin-token"); + expect(admin?.status).toBe(200); + }); + + test("PUT /api/claude-code records api surface and route detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/claude-code"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ enabled: true }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/claude-code"); + expect(rows[0].fields).toContain("claudeCode"); + }); + + test("admission-key writes record api surface and route detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const keysUrl = new URL("http://127.0.0.1:10100/api/keys"); + const post = await handleManagementAPI( + new Request(keysUrl, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ name: "deploy" }), + }), + keysUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(post?.status).toBe(201); + const created = await post!.json() as { id: string }; + let rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/keys"); + expect(rows[0].fields).toContain("apiKeys"); + + const patch = await handleManagementAPI( + new Request(keysUrl, { + method: "PATCH", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: created.id, name: "deploy-renamed" }), + }), + keysUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(patch?.status).toBe(200); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PATCH /api/keys"); + + const del = await handleManagementAPI( + new Request(keysUrl, { + method: "DELETE", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: created.id }), + }), + keysUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(del?.status).toBe(200); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("DELETE /api/keys"); + }); + + test("PUT /api/oauth/accounts/pool records api surface and route detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/oauth/accounts/pool"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ provider: "anthropic", enabled: true }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/oauth/accounts/pool"); + expect(rows[0].fields).toContain("anthropicAccountPool"); + }); + + test("PATCH /api/oauth/accounts/pool records the actual request method in audit detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/oauth/accounts/pool"); + const response = await handleManagementAPI( + new Request(url, { + method: "PATCH", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ provider: "anthropic", strategy: "round-robin" }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PATCH /api/oauth/accounts/pool"); + expect(rows[0].fields).toContain("anthropicAccountPool"); + }); + + test("codex account management writes record api surface and route detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/codex-auth/auto-switch"); + const response = await handleCodexAuthAPI( + new Request(url, { + method: "PUT", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ threshold: 50 }), + }), + url, + loadConfig(), + ); + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/codex-auth/auto-switch"); + expect(rows[0].fields).toContain("autoSwitchThreshold"); + }); +}); + +describe("cli key login audit provenance", () => { + test("commitKeyLoginProvider records the cli surface and operation detail", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const config = loadConfig(); + await commitKeyLoginProvider(config, "blsc", { + adapter: "openai-chat", + baseUrl: "https://llmapi.blsc.cn/v1", + authMode: "key", + apiKey: "sk-rotated", + } as unknown as OcxProviderConfig); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("cli"); + expect(rows[0].detail).toBe("ocx key login"); + expect(rows[0].fields).toContain("providers.blsc.apiKey"); + }); +}); + +describe("config mutation audit attribution sweep", () => { + test("admission-key rotation lifecycle records route-specific api details", async () => { + const config = configWithProvider(); + config.apiKeys = [{ + id: "admission-1", + name: "admission", + key: "ocx_data_0123456789abcdef0123456789abcdef01234567", + createdAt: new Date(0).toISOString(), + }]; + saveConfig(config, { surface: "cli", detail: "ocx first" }); + const rotateUrl = new URL("http://127.0.0.1:10100/api/keys/rotate"); + const startResponse = await handleManagementAPI( + new Request(rotateUrl, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: "admission-1" }), + }), + rotateUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(startResponse?.status).toBe(201); + let rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/keys/rotate"); + const startBody = await startResponse!.json() as { rotationId: string }; + const commitUrl = new URL("http://127.0.0.1:10100/api/keys/rotate/commit"); + const commitResponse = await handleManagementAPI( + new Request(commitUrl, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: "admission-1", rotationId: startBody.rotationId }), + }), + commitUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(commitResponse?.status).toBe(200); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/keys/rotate/commit"); + expect(rows[0].fields).toContain("apiKeys"); + }); + + test("client connection commit and clear record cli operation details", () => { + const connection: OcxClientConnectionConfig = { + serverUrl: "http://127.0.0.1:1", + managementUrl: "http://127.0.0.1:1", + managementTransport: "direct", + selectedClients: ["codex"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", + apiKeyId: "client-key-1", + tokenFingerprint: "0123456789abcdef".repeat(4), + protocolVersion: 1, + connectedAt: new Date(0).toISOString(), + priorCatalog: "", + catalogSyncedAt: new Date(0).toISOString(), + }; + expect(commitClientConnection(connection)).toBe("committed"); + let rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("cli"); + expect(rows[0].detail).toBe("ocx connect: commit client connection"); + expect(clearClientConnection("client-key-1")).toBe("committed"); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("cli"); + expect(rows[0].detail).toBe("ocx disconnect: clear client connection"); + }); +}); diff --git a/tests/server/management-provider-validation.test.ts b/tests/server/management-provider-validation.test.ts index 35a7924ebe..7923ef6995 100644 --- a/tests/server/management-provider-validation.test.ts +++ b/tests/server/management-provider-validation.test.ts @@ -678,6 +678,81 @@ describe("provider management validation", () => { expect(secretNameError).toContain("[REDACTED]"); }); + test("provider management validates transientRetryOn5xx bounds and unknown keys", () => { + const base = { adapter: "openai-chat", baseUrl: "https://api.openai.com/v1" }; + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { enabled: true, attempts: 5 }, + })).toBeNull(); + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { attempts: 0 }, + })).toContain("transientRetryOn5xx.attempts is invalid"); + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { attempts: 11 }, + })).toContain("transientRetryOn5xx.attempts is invalid"); + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { enabled: "yes" }, + })).toContain("transientRetryOn5xx.enabled is invalid"); + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { attempt: 3 }, + })).toContain("transientRetryOn5xx has unrecognized field"); + expect(providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: "enabled", + })).toContain("transientRetryOn5xx is invalid"); + // A secret-shaped unknown field name must be redacted in the error, never echoed. + const secretError = providerManagementConfigError("custom", { + ...base, + transientRetryOn5xx: { "sk-super-secret-9876": true }, + })!; + expect(secretError).toContain("transientRetryOn5xx has unrecognized field"); + expect(secretError).not.toContain("sk-super-secret-9876"); + expect(secretError).toContain("[REDACTED]"); + // A secret-shaped PROVIDER name must not be echoed by this error path either. + const secretNameError = providerManagementConfigError("sk-super-secret-9876", { + ...base, + transientRetryOn5xx: { attempts: 0 }, + })!; + expect(secretNameError).toContain("transientRetryOn5xx.attempts is invalid"); + expect(secretNameError).not.toContain("sk-super-secret-9876"); + expect(secretNameError).toContain("[REDACTED]"); + }); + + test("provider POST rejects invalid transientRetryOn5xx attempts without persisting", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "relay", + provider: { + adapter: "openai-chat", + baseUrl: "https://relay.example/v1", + transientRetryOn5xx: { enabled: true, attempts: 11 }, + }, + }), + }); + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ + error: expect.stringContaining("transientRetryOn5xx.attempts is invalid"), + }); + // The invalid provider must not reach disk or the live config. + expect(loadConfig().providers.relay).toBeUndefined(); + } finally { + await server.stop(true); + } + }); + test("provider management redacts provider names from auto-compaction validation errors", () => { const secretName = "sk-super-secret-9876"; const error = providerManagementConfigError(secretName, { diff --git a/tests/server/server-management-auth.test.ts b/tests/server/server-management-auth.test.ts index aece9bc93a..25dac804e8 100644 --- a/tests/server/server-management-auth.test.ts +++ b/tests/server/server-management-auth.test.ts @@ -661,6 +661,20 @@ describe("management and data-plane credential separation", () => { errorSpy.mockRestore(); } }); + test("GET /api/config/mutations requires the management token", async () => { + saveConfig(remoteConfig()); + const server = startServer(0); + try { + const anonymous = await fetch(new URL("/api/config/mutations", server.url)); + expect(anonymous.status).toBe(401); + const authorized = await fetch(new URL("/api/config/mutations", server.url), { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect(authorized.status).toBe(200); + } finally { + await server.stop(true); + } + }); test("a management token that matches the data environment token closes only the management plane", async () => { process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "data-secret"; saveConfig(remoteConfig()); From 22745e129ff8bc5a459248686f9b266e8fbdf76f Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 3 Sep 2026 17:31:04 +0800 Subject: [PATCH 2/5] fix(config): audit default limit, account-delete and provider-batch sources --- src/codex/account-lifecycle.ts | 9 +++- src/codex/auth-api.ts | 5 +- src/server/management/config-routes.ts | 9 +++- src/server/management/provider-routes.ts | 2 +- tests/config-mutation-audit.test.ts | 54 ++++++++++++++++++- .../provider-config-batch-management.test.ts | 21 +++++++- 6 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 0098fb97c4..e55a195797 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -6,6 +6,7 @@ import { saveConfigPreservingClaudeCode, withConfigMutationLockSync, } from "../config"; +import type { ConfigMutationSource } from "../config-mutation-audit"; import { removeCodexAccountCredential } from "./account-store"; import { clearAccountNeedsReauth } from "./account-runtime-state"; import { getMainChatgptAccountId } from "./auth-collision"; @@ -127,7 +128,11 @@ function restorePersistedConfig(configPath: string, previousBytes: string): void * * Returns true when a picker-visible row disappeared and the catalog must converge. */ -export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): boolean { +export function deleteCodexAccount( + runtimeConfig: OcxConfig, + accountId: string, + source: ConfigMutationSource = { surface: "internal", detail: "account lifecycle: remove account" }, +): boolean { let cleanupFailed = false; const pickerVisibilityChanged = withConfigMutationLockSync(() => { const previousConfig = structuredClone(runtimeConfig); @@ -158,7 +163,7 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): try { // Persist first for durable configs. Destructive cleanup below must never run for a // deletion that failed to commit. Transient configs intentionally skip this write. - saveConfigPreservingClaudeCode(runtimeConfig, { surface: "internal", detail: "account lifecycle: remove account" }); + saveConfigPreservingClaudeCode(runtimeConfig, source); } catch (error) { restoreRuntimeConfig(runtimeConfig, previousConfig); try { diff --git a/src/codex/auth-api.ts b/src/codex/auth-api.ts index a17eba7fd6..4337eae7e9 100644 --- a/src/codex/auth-api.ts +++ b/src/codex/auth-api.ts @@ -1920,7 +1920,10 @@ export async function handleCodexAuthAPI( if (!isValidCodexAccountId(id) && !isLegacyPoolAccount) { return jsonResponse({ error: "Invalid account id format" }, 400); } - const pickerVisibilityChanged = deleteCodexAccount(runtimeConfig, id); + const pickerVisibilityChanged = deleteCodexAccount(runtimeConfig, id, { + surface: "api", + detail: "DELETE /api/codex-auth/accounts", + }); saveRuntimeConfig(config, runtimeConfig, { surface: "api", detail: "DELETE /api/codex-auth/accounts" }); reconcileLiveStateStores(); const catalogRefresh = await convergeAccountNamespaceCatalog( diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index e175a5bd80..967c903616 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -282,8 +282,13 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise 0 ? requested : 100; + const { rows, maxRows } = readConfigMutationAudit(effectiveLimit); return jsonResponse({ mutations: rows, retention: { maxRows } }); } diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index e5cfadd31a..4d8ec6629d 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -857,7 +857,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise { expect(admin?.status).toBe(200); }); + // The route's best-effort agent-def sync performs a real provider model discovery after + // the audited save. Under parallel workers that network probe can exceed the default + // 5s test budget even though the audit row is written before the sync starts. test("PUT /api/claude-code records api surface and route detail", async () => { saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); const url = new URL("http://127.0.0.1:10100/api/claude-code"); @@ -1103,7 +1106,7 @@ describe("config mutation audit management API", () => { expect(rows[0].surface).toBe("api"); expect(rows[0].detail).toBe("PUT /api/claude-code"); expect(rows[0].fields).toContain("claudeCode"); - }); + }, 20_000); test("admission-key writes record api surface and route detail", async () => { saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); @@ -1307,4 +1310,53 @@ describe("config mutation audit attribution sweep", () => { expect(rows[0].surface).toBe("cli"); expect(rows[0].detail).toBe("ocx disconnect: clear client connection"); }); + + test("GET /api/config/mutations defaults missing, blank, non-positive, or unparseable limits to 100", async () => { + saveConfig(configWithProvider(), { surface: "api", detail: "PUT /api/audit-seed" }); + const config = loadConfig(); + for (const suffix of ["", "?limit=", "?limit=0", "?limit=-1", "?limit=abc", "?limit=1.5"]) { + const url = new URL(`http://127.0.0.1:10100/api/config/mutations${suffix}`); + const response = await handleManagementAPI( + new Request(url, { headers: { Host: "127.0.0.1:10100" } }), + url, + config, + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const body = await response!.json() as { mutations: Array<{ detail?: string }> }; + expect(body.mutations.length).toBeGreaterThan(0); + expect(body.mutations[0]?.detail).toBe("PUT /api/audit-seed"); + } + const limited = new URL("http://127.0.0.1:10100/api/config/mutations?limit=1"); + const limitedResponse = await handleManagementAPI( + new Request(limited, { headers: { Host: "127.0.0.1:10100" } }), + limited, + config, + {}, + "admin-token", + ); + const limitedBody = await limitedResponse!.json() as { mutations: unknown[] }; + expect(limitedBody.mutations).toHaveLength(1); + }); + + test("DELETE /api/codex-auth/accounts records the API source in the audit", async () => { + const live = configWithProvider(); + live.codexAccounts = [{ id: "audit-delete", email: "audit-delete@example.test", isMain: false }]; + saveConfig(live, { surface: "internal", detail: "test: seed account" }); + saveCodexAccountCredential("audit-delete", { + accessToken: "access-delete-audit", + refreshToken: "refresh-delete-audit", + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: "acct-delete-audit", + }); + const config = loadConfig(); + const req = new Request("http://localhost/api/codex-auth/accounts?id=audit-delete", { method: "DELETE" }); + const resp = await handleCodexAuthAPI(req, new URL(req.url), config); + expect(resp?.status).toBe(200); + expect(config.codexAccounts).toEqual([]); + const { rows } = readConfigMutationAudit(); + expect(rows[0]).toMatchObject({ surface: "api", detail: "DELETE /api/codex-auth/accounts" }); + expect(JSON.stringify(rows[0].fields)).not.toContain("access-delete-audit"); + }); }); diff --git a/tests/providers/provider-config-batch-management.test.ts b/tests/providers/provider-config-batch-management.test.ts index c0646c98f9..4e4af564d8 100644 --- a/tests/providers/provider-config-batch-management.test.ts +++ b/tests/providers/provider-config-batch-management.test.ts @@ -3,7 +3,7 @@ import { mkdirSync, mkdtempSync, readFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import * as configModule from "../../src/config"; -import { getConfigPath, loadConfig, saveConfig } from "../../src/config"; +import { getConfigPath, loadConfig, readConfigMutationAudit, saveConfig } from "../../src/config"; import * as destinationPolicy from "../../src/lib/destination-policy"; import { safeConfigDTO } from "../../src/server/auth-cors"; import { handleManagementAPI } from "../../src/server/management-api"; @@ -360,4 +360,23 @@ describe("atomic provider editor batch", () => { error: "Full config PUT is disabled. Use /api/providers POST for provider changes.", }); }); + + test("PUT /api/providers records the API source in the config mutation audit", async () => { + const liveConfig = seededConfig(); + saveConfig(liveConfig); + const baseline = editorBaseline(liveConfig); + const next: EditorConfig = structuredClone(baseline); + next.providers.alpha!.defaultModel = "alpha-new-audit"; + + const destinationSpy = spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + let response: Response | null; + try { + response = await putBatch(liveConfig, { baseline, next }); + } finally { + destinationSpy.mockRestore(); + } + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0]).toMatchObject({ surface: "api", detail: "PUT /api/providers" }); + }); }); From 552c46b18c211ed1e75577a0274864e7a4580646 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 3 Sep 2026 19:52:10 +0800 Subject: [PATCH 3/5] test(audit): distinguish fallback limit from limit=1 with two mutations --- tests/config-mutation-audit.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/config-mutation-audit.test.ts b/tests/config-mutation-audit.test.ts index 1f46710a32..87eaf01fe6 100644 --- a/tests/config-mutation-audit.test.ts +++ b/tests/config-mutation-audit.test.ts @@ -1313,6 +1313,7 @@ describe("config mutation audit attribution sweep", () => { test("GET /api/config/mutations defaults missing, blank, non-positive, or unparseable limits to 100", async () => { saveConfig(configWithProvider(), { surface: "api", detail: "PUT /api/audit-seed" }); + saveConfig(configWithProvider(10200), { surface: "api", detail: "PUT /api/audit-seed-2" }); const config = loadConfig(); for (const suffix of ["", "?limit=", "?limit=0", "?limit=-1", "?limit=abc", "?limit=1.5"]) { const url = new URL(`http://127.0.0.1:10100/api/config/mutations${suffix}`); @@ -1325,8 +1326,10 @@ describe("config mutation audit attribution sweep", () => { ); expect(response?.status).toBe(200); const body = await response!.json() as { mutations: Array<{ detail?: string }> }; - expect(body.mutations.length).toBeGreaterThan(0); - expect(body.mutations[0]?.detail).toBe("PUT /api/audit-seed"); + // Two mutations make the fallback limit (100) distinguishable from limit=1: the + // fallback must return more than one row, while an explicit limit=1 returns one. + expect(body.mutations.length).toBeGreaterThan(1); + expect(body.mutations[0]?.detail).toBe("PUT /api/audit-seed-2"); } const limited = new URL("http://127.0.0.1:10100/api/config/mutations?limit=1"); const limitedResponse = await handleManagementAPI( From 4d1fbdf521862425ed77f9d9884387802431813c Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 3 Sep 2026 21:38:54 +0800 Subject: [PATCH 4/5] test(audit): cover remaining route branches and recovery unlink-failure seam --- src/config.ts | 17 +++ tests/config-mutation-audit.test.ts | 221 ++++++++++++++++++++++++++-- 2 files changed, 226 insertions(+), 12 deletions(-) diff --git a/src/config.ts b/src/config.ts index ddb1251d2b..217cd796e9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2966,6 +2966,8 @@ let pendingConfigMutationAuditCleanup: string | null = null; let failConfigAtomicWriteForTests: (() => Error) | null = null; /** Test-only seam: fail the config.json atomic write AFTER the rename lands, before the audit commit. */ let failAfterConfigAtomicWriteForTests: (() => Error) | null = null; +/** Test-only seam: make recovery marker deletion fail after the recovery transaction commits. */ +let failRecoveryMarkerUnlinkForTests: (() => Error) | null = null; /** * Test-only one-shot seam: make the next changed config.json persist throw after @@ -2985,6 +2987,16 @@ export function setConfigPostWriteFailureForTests(factory: (() => Error) | null) failAfterConfigAtomicWriteForTests = factory; } +/** + * Test-only one-shot seam: make the next recovery-marker deletion throw AFTER + * the recovery transaction commits. Mirrors a transient handle hold on the + * marker path; the committed audit row must survive and the leftover marker + * must be re-decided on the next recovery without failing the save. + */ +export function setConfigRecoveryMarkerUnlinkFailureForTests(factory: (() => Error) | null): void { + failRecoveryMarkerUnlinkForTests = factory; +} + /** * Serialize synchronous config and Codex credential-generation commits across processes with an * OS-backed SQLite write transaction. `busy_timeout=0` is deliberate: runtime request paths must @@ -3028,6 +3040,11 @@ export function withConfigMutationLockSync(fn: () => T): T { // and a parseable-but-invalid one cannot be trusted, so it is removed too. for (const markerPath of pendingPaths) { try { + const failure = failRecoveryMarkerUnlinkForTests; + if (failure) { + failRecoveryMarkerUnlinkForTests = null; + throw failure(); + } deletePendingConfigMutationAuditAtPath(markerPath); } catch { // Best-effort after the recovery COMMIT: the row is durable and a leftover diff --git a/tests/config-mutation-audit.test.ts b/tests/config-mutation-audit.test.ts index 87eaf01fe6..dbcb446670 100644 --- a/tests/config-mutation-audit.test.ts +++ b/tests/config-mutation-audit.test.ts @@ -10,6 +10,7 @@ import { saveConfig, saveConfigPreservingClaudeCode, setConfigAtomicWriteFailureForTests, + setConfigRecoveryMarkerUnlinkFailureForTests, setConfigPostWriteFailureForTests, } from "../src/config"; import { @@ -21,6 +22,7 @@ import { setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests, } from "../src/config-mutation-audit"; import { addProviderApiKey } from "../src/providers/api-keys"; +import { setProviderKeychainEntryFactoryForTests, type ProviderKeychainEntry } from "../src/providers/key-store"; import { rotateKeyOn429 } from "../src/providers/key-failover"; import { writeStorageCleanupPolicyToConfig } from "../src/storage/policy"; import { setIntegrationEnabled } from "../src/codex/desired-state"; @@ -64,7 +66,9 @@ afterEach(() => { else process.env.OPENCODEX_HOME = previousHome; setConfigAuditMaxRowsForTests(null); setConfigAtomicWriteFailureForTests(null); + setConfigRecoveryMarkerUnlinkFailureForTests(null); setConfigPostWriteFailureForTests(null); + setProviderKeychainEntryFactoryForTests(null); setReconcilePendingConfigMutationAuditOnReadBeforeCleanupForTests(null); resetPreservedDiskOnlyProvidersForTests(); rmSync(testRoot, { recursive: true, force: true }); @@ -898,18 +902,41 @@ describe("config mutation audit log", () => { test("a marker that cannot be unlinked after a recovery commit does not fail the save", () => { saveConfig(configWithProvider(10100)); const configPath = join(testRoot, "config.json"); - const bytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; - writeFileSync(configPath, bytes); - // A directory at the marker path makes unlink fail (EISDIR/EPERM) like a - // transient handle hold. The recovery COMMIT already succeeded, so a leftover - // marker must never turn the next save into a reported failure. - const lockedMarker = markerFile("locked-marker"); - mkdirSync(lockedMarker, { recursive: true }); - writeFileSync(join(lockedMarker, "placeholder"), "x"); - expect(() => saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next" })).not.toThrow(); - expect(readFileSync(configPath, "utf8")).toContain("\"port\": 10600"); - const audit = readConfigMutationAudit(); - expect(audit.rows[0].detail).toBe("ocx next"); + // Plant a VALID marker whose hash matches the on-disk bytes — the exact state a + // real writer leaves after a crash between the config rename and audit commit. + const interruptedBytes = JSON.stringify(configWithProvider(10500), null, 2) + "\n"; + setConfigPostWriteFailureForTests(() => new Error("simulated post-rename failure")); + expect(() => saveConfig(configWithProvider(10500), { + surface: "api", + detail: "PUT /api/post-rename", + })).toThrow("simulated post-rename failure"); + expect(readFileSync(configPath, "utf8")).toBe(interruptedBytes); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(1); + + // Recovery COMMITs the row first; the one-shot unlink seam then throws, so + // the leftover marker must not turn the next save into a reported failure. + setConfigRecoveryMarkerUnlinkFailureForTests(() => new Error("simulated marker unlink failure")); + expect(() => saveConfig(configWithProvider(10600), { surface: "cli", detail: "ocx next-1" })).not.toThrow(); + let audit = readConfigMutationAudit(); + expect(audit.rows.filter(row => row.detail === "PUT /api/post-rename")).toHaveLength(1); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(1); + + // Restore the interrupted bytes and let recovery replay the SAME marker: + // mutation-id dedupe must keep exactly one row while the unlink fails again. + writeFileSync(configPath, interruptedBytes); + setConfigRecoveryMarkerUnlinkFailureForTests(() => new Error("simulated marker unlink failure")); + expect(() => saveConfig(configWithProvider(10700), { surface: "cli", detail: "ocx next-2" })).not.toThrow(); + audit = readConfigMutationAudit(); + expect(audit.rows.filter(row => row.detail === "PUT /api/post-rename")).toHaveLength(1); + expect(audit.rows[0].detail).toBe("ocx next-2"); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(1); + + // Once the unlink succeeds the marker is removed and the row stays at one. + expect(() => saveConfig(configWithProvider(10800), { surface: "cli", detail: "ocx next-3" })).not.toThrow(); + audit = readConfigMutationAudit(); + expect(audit.rows.filter(row => row.detail === "PUT /api/post-rename")).toHaveLength(1); + expect(audit.rows[0].detail).toBe("ocx next-3"); + expect(listPendingConfigMutationAuditPaths(testRoot)).toHaveLength(0); }); test("proxy URL userinfo is never stored in the pending marker", () => { @@ -1204,6 +1231,41 @@ describe("config mutation audit management API", () => { expect(rows[0].fields).toContain("anthropicAccountPool"); }); + test("generic OAuth pool persistence records api surface and route detail", async () => { + saveConfig({ + port: 10100, + defaultProvider: "google-antigravity", + providers: { + "google-antigravity": { + adapter: "google", + baseUrl: "https://daily-cloudcode-pa.googleapis.com", + authMode: "oauth", + }, + }, + } as unknown as OcxConfig, { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/oauth/accounts/pool"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ + provider: "google-antigravity", + enabled: true, + strategy: "round-robin", + }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const { rows } = readConfigMutationAudit(); + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("PUT /api/oauth/accounts/pool"); + expect(rows[0].fields).toContain("providers.google-antigravity.oauthAccountFailover"); + }); + test("codex account management writes record api surface and route detail", async () => { saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); const url = new URL("http://127.0.0.1:10100/api/codex-auth/auto-switch"); @@ -1287,6 +1349,141 @@ describe("config mutation audit attribution sweep", () => { expect(rows[0].fields).toContain("apiKeys"); }); + test("keychain store and restore record route-specific api details", async () => { + saveConfig(configWithProvider(), { surface: "cli", detail: "ocx first" }); + const secrets = new Map(); + const factory = (service: string, account: string): ProviderKeychainEntry => { + const key = service + "\u0000" + account; + return { + getPassword: () => secrets.get(key) ?? null, + setPassword: value => { secrets.set(key, value); }, + deletePassword: () => secrets.delete(key), + }; + }; + setProviderKeychainEntryFactoryForTests(factory); + try { + const url = new URL("http://127.0.0.1:10100/api/providers/keychain"); + const store = await handleManagementAPI( + new Request(url, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ name: "blsc", action: "store" }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(store?.status).toBe(200); + let rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/providers/keychain (store)"); + expect(rows[0].fields).toContain("providers.blsc.apiKey"); + + const restore = await handleManagementAPI( + new Request(url, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ name: "blsc", action: "restore" }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(restore?.status).toBe(200); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/providers/keychain (restore)"); + expect(rows[0].fields).toContain("providers.blsc.apiKey"); + } finally { + setProviderKeychainEntryFactoryForTests(null); + } + }); + + test("expired rotation cleanup during GET and commit records route-specific details", async () => { + const past = new Date(0).toISOString(); + const key = "ocx_data_0123456789abcdef0123456789abcdef01234567"; + const seedExpired = () => { + const config = configWithProvider(); + config.apiKeys = [{ + id: "admission-expired", name: "admission", key, createdAt: past, + pendingRotation: { + id: "rotation-expired", key, createdAt: past, expiresAt: past, + }, + }]; + saveConfig(config, { surface: "cli", detail: "ocx first" }); + }; + + seedExpired(); + const keysUrl = new URL("http://127.0.0.1:10100/api/keys"); + const list = await handleManagementAPI( + new Request(keysUrl, { headers: { Host: "127.0.0.1:10100" } }), + keysUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(list?.status).toBe(200); + let rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("GET /api/keys (expired rotation cleanup)"); + expect(rows[0].fields).toContain("apiKeys"); + expect(loadConfig().apiKeys?.[0]?.pendingRotation).toBeUndefined(); + + seedExpired(); + const commitUrl = new URL("http://127.0.0.1:10100/api/keys/rotate/commit"); + const commit = await handleManagementAPI( + new Request(commitUrl, { + method: "POST", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: "admission-expired", rotationId: "rotation-expired" }), + }), + commitUrl, + loadConfig(), + {}, + "admin-token", + ); + expect(commit?.status).toBe(409); + rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("POST /api/keys/rotate/commit (expired rotation cleanup)"); + expect(rows[0].fields).toContain("apiKeys"); + }); + + test("rotation abort records DELETE /api/keys/rotate", async () => { + const now = Date.now(); + const createdAt = new Date(now).toISOString(); + const key = "ocx_data_0123456789abcdef0123456789abcdef01234567"; + const config = configWithProvider(); + config.apiKeys = [{ + id: "admission-abort", name: "admission", key, createdAt, + pendingRotation: { + id: "rotation-abort", key, createdAt, + expiresAt: new Date(now + 60_000).toISOString(), + }, + }]; + saveConfig(config, { surface: "cli", detail: "ocx first" }); + const url = new URL("http://127.0.0.1:10100/api/keys/rotate"); + const response = await handleManagementAPI( + new Request(url, { + method: "DELETE", + headers: { Host: "127.0.0.1:10100", "Content-Type": "application/json" }, + body: JSON.stringify({ id: "admission-abort", rotationId: "rotation-abort" }), + }), + url, + loadConfig(), + {}, + "admin-token", + ); + expect(response?.status).toBe(200); + const rows = readConfigMutationAudit().rows; + expect(rows[0].surface).toBe("api"); + expect(rows[0].detail).toBe("DELETE /api/keys/rotate"); + expect(rows[0].fields).toContain("apiKeys"); + expect(loadConfig().apiKeys?.[0]?.pendingRotation).toBeUndefined(); + }); + test("client connection commit and clear record cli operation details", () => { const connection: OcxClientConnectionConfig = { serverUrl: "http://127.0.0.1:1", From b0986b17520c9ca14e20dbd74e914bbc0bf970c9 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Sat, 5 Sep 2026 21:27:24 +0800 Subject: [PATCH 5/5] fix(config): drain nested marker cleanup, cache audit responses, preserve eject provenance --- src/cli/dispatch.ts | 8 +++++++- src/config.ts | 27 +++++++++++++++----------- src/server/management/config-routes.ts | 5 ++++- tests/cli/cli-provider.test.ts | 7 +++++++ tests/config-mutation-audit.test.ts | 17 ++++++++++++++++ 5 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index e5f0cdea73..96165c99e4 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -52,6 +52,11 @@ export interface CliDispatchDeps { type CommandRunner = (deps: CliDispatchDeps) => Promise; +/** Detail label for the shared restore/eject runner so audit provenance stays distinct. */ +export function restoreCommandDetail(command: string | undefined): string { + return command === "eject" ? "ocx eject" : "ocx restore"; +} + const commandRunners: Record = { init: async () => { const { runInit } = await import("./init"); @@ -111,7 +116,8 @@ const commandRunners: Record = { const target = collectOrcaCodexHomeDiagnostic(); return emitBack(true, `Plain \`codex\` now routes through opencodex in ${target.effectiveCodexHome} (undo with: ocx restore).`, 0); } - const desired = setIntegrationEnabled("codex", false, { surface: "cli", detail: "ocx restore" }); + const detail = restoreCommandDetail(deps.command); + const desired = setIntegrationEnabled("codex", false, { surface: "cli", detail }); if (!desired.ok) { if (restoreJson) { // Machine-readable contract: every restore --json outcome emits one diff --git a/src/config.ts b/src/config.ts index 217cd796e9..55b92a3a8f 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2960,8 +2960,8 @@ export function prepareConfigMutationDatabasePathForWrite(): string { let configMutationLockDepth = 0; let configMutationDatabase: Database | null = null; -/** The mutation id whose marker is deleted after the surrounding transaction commits. */ -let pendingConfigMutationAuditCleanup: string | null = null; +/** Mutation ids whose markers are deleted after the surrounding transaction commits. */ +let pendingConfigMutationAuditCleanup: Set | null = null; /** Test-only seam: fail the config.json atomic write AFTER the pending marker is persisted. */ let failConfigAtomicWriteForTests: (() => Error) | null = null; /** Test-only seam: fail the config.json atomic write AFTER the rename lands, before the audit commit. */ @@ -3075,15 +3075,17 @@ export function withConfigMutationLockSync(fn: () => T): T { const value = fn(); database.exec("COMMIT"); transactionOpen = false; - if (pendingConfigMutationAuditCleanup) { - const mutationId = pendingConfigMutationAuditCleanup; + if (pendingConfigMutationAuditCleanup && pendingConfigMutationAuditCleanup.size > 0) { + const mutationIds = [...pendingConfigMutationAuditCleanup]; pendingConfigMutationAuditCleanup = null; - try { - deletePendingConfigMutationAudit(getConfigDir(), mutationId); - } catch { - // Best-effort after the COMMIT: the audit row is durable. A leftover marker - // is harmless and the next recovery reconciles it, so an unlink failure - // (e.g. a transient handle hold) must not surface as a failed write. + for (const mutationId of mutationIds) { + try { + deletePendingConfigMutationAudit(getConfigDir(), mutationId); + } catch { + // Best-effort after the COMMIT: the audit row is durable. A leftover marker + // is harmless and the next recovery reconciles it, so an unlink failure + // (e.g. a transient handle hold) must not surface as a failed write. + } } } return value; @@ -3277,7 +3279,10 @@ function persistConfigUnlocked( refreshUserCostOverlays(persisted); if (audit && configMutationDatabase) { const recorded = recordPendingConfigMutationAuditNow(configMutationDatabase, getConfigDir(), auditMutationId!); - pendingConfigMutationAuditCleanup = recorded ? auditMutationId : null; + if (recorded && auditMutationId) { + if (!pendingConfigMutationAuditCleanup) pendingConfigMutationAuditCleanup = new Set(); + pendingConfigMutationAuditCleanup.add(auditMutationId); + } } return persisted; } diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 967c903616..98688f3444 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -289,7 +289,10 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise 0 ? requested : 100; const { rows, maxRows } = readConfigMutationAudit(effectiveLimit); - return jsonResponse({ mutations: rows, retention: { maxRows } }); + const response = jsonResponse({ mutations: rows, retention: { maxRows } }); + const headers = new Headers(response.headers); + headers.set("Cache-Control", "no-store"); + return new Response(response.body, { status: response.status, headers }); } if (url.pathname === "/api/settings" && req.method === "GET") { diff --git a/tests/cli/cli-provider.test.ts b/tests/cli/cli-provider.test.ts index 1aa63e77c2..1ca77667bf 100644 --- a/tests/cli/cli-provider.test.ts +++ b/tests/cli/cli-provider.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { Database } from "bun:sqlite"; +import { restoreCommandDetail } from "../../src/cli/dispatch"; import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -16,6 +17,12 @@ const isolatedCodexHome = mkdtempSync(join(tmpdir(), "ocx-prov-codex-home-")); // routinely blow the 5s default before --help returns; the spawn IS the assertion. setDefaultTimeout(SPAWN_BUDGET_MS); +test("restore runner provenance distinguishes eject from restore", () => { + expect(restoreCommandDetail("eject")).toBe("ocx eject"); + expect(restoreCommandDetail("restore")).toBe("ocx restore"); + expect(restoreCommandDetail(undefined)).toBe("ocx restore"); +}); + function runCli(args: string[], env: Record = {}) { return spawnSync(process.execPath, [cliPath, ...args], { cwd: repoRoot, diff --git a/tests/config-mutation-audit.test.ts b/tests/config-mutation-audit.test.ts index dbcb446670..d97b361d01 100644 --- a/tests/config-mutation-audit.test.ts +++ b/tests/config-mutation-audit.test.ts @@ -12,6 +12,7 @@ import { setConfigAtomicWriteFailureForTests, setConfigRecoveryMarkerUnlinkFailureForTests, setConfigPostWriteFailureForTests, + withConfigMutationLockSync, } from "../src/config"; import { buildConfigMutationSnapshot, @@ -108,6 +109,20 @@ describe("config mutation audit log", () => { expect(JSON.stringify(rows[1].after)).not.toContain("sk-second-key"); }); + test("nested changed saves remove every pending marker after commit", () => { + withConfigMutationLockSync(() => { + const first = configWithProvider(10100); + first.managementUsageMaxReadBytes = 111; + saveConfig(first, { surface: "internal", detail: "nested first" }); + const second = configWithProvider(10200); + second.managementUsageMaxReadBytes = 222; + saveConfig(second, { surface: "internal", detail: "nested second" }); + }); + const { rows } = readConfigMutationAudit(); + expect(rows.map(row => row.detail)).toEqual(["nested second", "nested first"]); + expect(listPendingConfigMutationAuditPaths(testRoot)).toEqual([]); + }); + test("storage cleanup policy writes record api surface and operation detail", () => { saveConfig(configWithProvider()); const policy: StorageCleanupPolicy = { @@ -1093,6 +1108,7 @@ describe("config mutation audit management API", () => { "admin-token", ); expect(response).not.toBeNull(); + expect(response!.headers.get("Cache-Control")).toBe("no-store"); const body = await response!.json() as { mutations: Array<{ detail: string }>; retention: { maxRows: number } }; expect(body.mutations).toHaveLength(1); expect(body.mutations[0].detail).toBe("PUT /api/test"); @@ -1109,6 +1125,7 @@ describe("config mutation audit management API", () => { expect(capability?.status).toBe(403); const admin = await handleManagementAPI(request(), url, loadConfig(), {}, "admin-token"); expect(admin?.status).toBe(200); + expect(admin?.headers.get("Cache-Control")).toBe("no-store"); }); // The route's best-effort agent-def sync performs a real provider model discovery after