diff --git a/AGENTS.md b/AGENTS.md index b2981888..a97313a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,7 @@ oc-codex-multi-auth doctor - Per-project accounts: `~/.opencode/projects//oc-codex-multi-auth-accounts.json`. - Global accounts: `~/.opencode/oc-codex-multi-auth-accounts.json`. - Flagged accounts: `oc-codex-multi-auth-flagged-accounts.json`, written beside the active accounts file (per project when `perProjectAccounts` is on). +- Credential snapshots: `backups/codex-credential-snapshot-*.json`, written beside the active accounts file. Holds the previous store content, captured before a significant write; retention prunes strictly by that prefix so it never deletes another backup kind. - Quota notification state: `oc-codex-multi-auth-quota-notifications.json`, written beside the active accounts file (per project when `perProjectAccounts` is on). - Request logs: `~/.opencode/logs/codex-plugin/` when logging is enabled. - Model catalog: 13 modern bases / 59 variants; legacy 59 explicit. diff --git a/README.md b/README.md index 11b4e230..b011ff8f 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,8 @@ Selected runtime/environment overrides: | `CODEX_TUI_MASK_EMAIL=0/1` | Mask account emails across account-display surfaces (list/status/limits/health/dashboard/menus + TUI quota status) | | `CODEX_TUI_MASK_EMAIL_DETAILS=0/1` | Also hide account email in quota details when prompt masking is enabled | | `CODEX_AUTH_PER_PROJECT_ACCOUNTS=0/1` | Disable/enable per-project account pools | +| `CODEX_AUTH_CREDENTIAL_SNAPSHOTS=0/1` | Disable/enable pre-write snapshots of the credential store (default on) | +| `CODEX_AUTH_CREDENTIAL_SNAPSHOTS_MAX_COUNT=` | How many credential snapshots to keep (`0` keeps all of them) | | `CODEX_AUTH_AUTO_UPDATE=0/1` | Disable/enable daily npm update check and cache refresh | | `CODEX_AUTH_ROTATION_STRATEGY=hybrid\|sticky\|round-robin` | Account selection strategy | | `CODEX_AUTH_UNSUPPORTED_MODEL_POLICY=strict\|fallback` | Control unsupported-model retry behavior | @@ -456,6 +458,8 @@ By default, account pools are stored locally as V3 JSON files. File permissions Use JSON storage when you want predictable, inspectable local files and easy backup/export behavior. +Before the store is changed in a way that matters, the plugin copies the previous version of the file into `backups/` as `codex-credential-snapshot-*.json`, mode `0600` in a `0700` directory. The snapshot holds the state being replaced, not the state replacing it, which is what makes it useful if the file is ever overwritten wholesale. Token refreshes count as significant, which bounds how stale a restore can be. Refresh tokens are single-use, so a snapshot taken just before a refresh holds the consumed token for the one account that refresh rotated - that account needs a fresh `opencode auth login` - while every other account in the pool comes back with the token that was live at that moment. A snapshot old enough to predate many refreshes restores a pool where most or all accounts can no longer authenticate, which is the failure this bounding exists to avoid. Rotation bookkeeping - `lastUsed`, rate-limit and cooldown state, quota stamps, and the rotation cursor - never triggers one on its own, so the kept snapshots are not churned away by ordinary traffic. The plugin keeps the 10 most recent and prunes strictly by that filename prefix, so nothing else in `backups/` is touched. Set `credentialSnapshots: false` to turn it off, or `credentialSnapshotsMaxCount` to keep a different number (`0` keeps all of them). +
diff --git a/docs/configuration.md b/docs/configuration.md index f438246c..6bede8e3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -224,6 +224,8 @@ a restart to change their configuration. "server": 2 }, "perProjectAccounts": true, + "credentialSnapshots": true, + "credentialSnapshotsMaxCount": 10, "autoUpdate": true, "toastDurationMs": 5000, "accountToasts": true, @@ -281,6 +283,8 @@ The sample above intentionally sets `"retryAllAccountsMaxRetries": 3` as a bound | `retryProfile` | `balanced` | retry budget profile for request classes (`conservative`, `balanced`, `aggressive`) | | `retryBudgetOverrides` | `{}` | optional per-class budget overrides (`authRefresh`, `network`, `server`, `rateLimitShort`, `rateLimitGlobal`, `emptyResponse`) | | `perProjectAccounts` | `true` | each project gets its own account storage | +| `credentialSnapshots` | `true` | before a significant change to the account store, copy the previous on-disk version into `backups/` so a clobbered store can be restored to a recent state. Refresh tokens are single-use: a snapshot taken just before a refresh holds the consumed token for the one account that refresh rotated, and the live token for every other account, so restoring costs at most a re-login for that one account rather than the whole pool. Snapshots are taken for account additions and removals, token refreshes, identity changes, label/tag/note/enabled changes, plan changes, schema-version changes, and deletion of the store. Rotation bookkeeping never triggers one on its own: `lastUsed`, `lastSwitchReason`, rate-limit and cooldown state, quota-exhaustion stamps, and the `activeIndex` / `activeIndexByFamily` rotation cursor. A snapshot failure is logged and never fails the write it precedes. Snapshots cover the default JSON backend only, not `CODEX_KEYCHAIN=1` | +| `credentialSnapshotsMaxCount` | `10` | how many credential snapshots to keep. Pruning deletes strictly by the snapshot filename prefix, so other files in `backups/` are never touched. `0` means keep every snapshot; use `credentialSnapshots: false` to turn the feature off | | `autoUpdate` | `true` | check npm daily and clear the OpenCode-managed plugin cache on exit when a newer version is available; restart OpenCode to install it | | `toastDurationMs` | `5000` | how long toast notifications stay visible (ms) | | `accountToasts` | `true` | show the transient `Using (N/N)` account-selection toast; set `false` to hide only this informational toast (rate-limit/auth/recovery warnings and errors still show) | @@ -469,6 +473,8 @@ override any config with env vars (boolean values are truthy only for `"1"`): | `CODEX_AUTH_BEGINNER_SAFE_MODE=1` | enable beginner-safe retry behavior | | `CODEX_AUTH_RETRY_PROFILE=aggressive` | override retry profile (`conservative`, `balanced`, `aggressive`) | | `CODEX_AUTH_PER_PROJECT_ACCOUNTS=0` | disable per-project accounts | +| `CODEX_AUTH_CREDENTIAL_SNAPSHOTS=0` | disable pre-write credential-store snapshots (enabled by default) | +| `CODEX_AUTH_CREDENTIAL_SNAPSHOTS_MAX_COUNT=25` | how many credential snapshots to keep (`0` keeps all of them) | | `CODEX_AUTH_PARALLEL_PROBING=1` | enable concurrent account health probes | | `CODEX_AUTH_PARALLEL_PROBING_MAX_CONCURRENCY=3` | max concurrent probes (1–5) | | `CODEX_AUTH_EMPTY_RESPONSE_MAX_RETRIES=3` | override empty-response retry count | @@ -616,6 +622,7 @@ opencode run "task" --model=openai/gpt-5.6-sol-high | `~/.opencode/oc-codex-multi-auth-accounts.json` | global V3 account pool | | `~/.opencode/projects//oc-codex-multi-auth-accounts.json` | per-project account pool | | `~/.opencode/projects//oc-codex-multi-auth-flagged-accounts.json` | flagged/deactivated account metadata, written beside the active accounts file. With the default `perProjectAccounts` this is the per-project path; with project storage off it is `~/.opencode/oc-codex-multi-auth-flagged-accounts.json` | +| `~/.opencode/backups/codex-credential-snapshot-*.json` | pre-write credential-store snapshots, written beside the accounts file they belong to (so the per-project `backups/` directory when `perProjectAccounts` is on). Mode `0600` in a `0700` directory, because they hold live refresh tokens | | `~/.opencode/logs/codex-plugin/` | request/debug logs when enabled | | `~/.opencode/cache/` | instruction/catalog and auto-update caches | | `~/.local/state/opencode/oc-codex-multi-auth-tui-quota.json` | TUI quota snapshot cache shared by the provider and TUI plugins; `$OPENCODE_STATE_DIR` overrides the directory when set | diff --git a/docs/development/CONFIG_FIELDS.md b/docs/development/CONFIG_FIELDS.md index 5bbe403b..14e52733 100644 --- a/docs/development/CONFIG_FIELDS.md +++ b/docs/development/CONFIG_FIELDS.md @@ -248,6 +248,8 @@ Defaults come from `lib/config.ts` / `lib/schemas.ts`. Environment overrides win | `toastDurationMs` | `5000` | `CODEX_AUTH_TOAST_DURATION_MS` | Toast visibility duration | | `accountToasts` | `true` | `CODEX_AUTH_ACCOUNT_TOASTS` | Gates only the informational "Using \ (N/N)" selection toast; warning/error toasts are unaffected | | `perProjectAccounts` | `true` | `CODEX_AUTH_PER_PROJECT_ACCOUNTS` | Project-scoped account pools | +| `credentialSnapshots` | `true` | `CODEX_AUTH_CREDENTIAL_SNAPSHOTS` | Copy the previous account store into `backups/` before a significant change | +| `credentialSnapshotsMaxCount` | `10` | `CODEX_AUTH_CREDENTIAL_SNAPSHOTS_MAX_COUNT` | Snapshots kept; `0` keeps all of them, and disabling is `credentialSnapshots`' job | | `sessionRecovery` | `true` | `CODEX_AUTH_SESSION_RECOVERY` | Auto-recover common API errors | | `autoResume` | `true` | `CODEX_AUTH_AUTO_RESUME` | Auto-resume after thinking-block recovery | | `autoUpdate` | `true` | `CODEX_AUTH_AUTO_UPDATE` | Daily npm update check + cache refresh | @@ -277,6 +279,7 @@ Defaults come from `lib/config.ts` / `lib/schemas.ts`. Environment overrides win | `streamStallTimeoutMs` | at least 1000 | 1000 | | `quotaNotifications.intervalMs` | at least 30000 | clamped up to 30000 | | `retryBudgetOverrides.*` | integer, at least 0 | (file only) | +| `credentialSnapshotsMaxCount` | integer, at least 0 | 0, no ceiling | So `parallelProbingMaxConcurrency: 9` in the file falls back to the default `2`, while `CODEX_AUTH_PARALLEL_PROBING_MAX_CONCURRENCY=9` is accepted with no ceiling. diff --git a/index.ts b/index.ts index 1cb71259..aa6ea961 100644 --- a/index.ts +++ b/index.ts @@ -394,6 +394,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { let startupPreflightShown = false; let beginnerSafeModeEnabled = false; const MIN_BACKOFF_MS = 100; + // An all-accounts rate-limit wait can run for days, and the local accounts + // file is its only wake-up. A quota reset granted server-side leaves that file + // untouched, so such a wait would be slept straight through. Long waits + // therefore re-probe upstream: first after a minute, doubling to a quarter + // hour, so a multi-day sleep costs a handful of usage requests rather than one + // per countdown tick. + const UPSTREAM_REPROBE_MIN_WAIT_MS = 60_000; + const UPSTREAM_REPROBE_FIRST_DELAY_MS = 60_000; + const UPSTREAM_REPROBE_MAX_DELAY_MS = 15 * 60_000; const runtimeMetrics: RuntimeMetrics = { startedAt: Date.now(), @@ -1675,6 +1684,50 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + /** + * `loadAccounts()` reports a read or parse failure the same way it reports + * an absent file - by returning null - and a null load builds a manager + * holding zero accounts. Installing that over a working pool makes this + * process answer "No Codex accounts configured" while the accounts file on + * disk is intact, which cross-process lock contention makes reachable. + * + * Emptying the pool for real always goes through an explicit action + * (`codex-remove`, logout, a storage-mode switch); each installs its own + * manager rather than arriving here, so refusing the shrink costs a genuine + * deletion nothing. + */ + const isUntrustworthyEmptyReload = ( + incumbent: AccountManager | null, + reloaded: AccountManager, + ): boolean => + incumbent !== null && + incumbent !== reloaded && + reloaded.getAccountCount() === 0 && + incumbent.getAccountCount() > 0; + + const EMPTY_RELOAD_RETRY_DELAY_MS = 2000; + const EMPTY_RELOAD_MAX_RETRIES = 3; + let emptyReloadRetries = 0; + let emptyReloadRetryTimer: ReturnType | undefined; + const cancelEmptyReloadRetry = (): void => { + clearTimeout(emptyReloadRetryTimer); + emptyReloadRetryTimer = undefined; + emptyReloadRetries = 0; + }; + const scheduleEmptyReloadRetry = (retry: () => Promise): void => { + if (emptyReloadRetries >= EMPTY_RELOAD_MAX_RETRIES) { + emptyReloadRetries = 0; + return; + } + emptyReloadRetries += 1; + clearTimeout(emptyReloadRetryTimer); + emptyReloadRetryTimer = setTimeout(() => { + emptyReloadRetryTimer = undefined; + void retry(); + }, EMPTY_RELOAD_RETRY_DELAY_MS); + emptyReloadRetryTimer.unref(); + }; + const reloadCachedAccountManager = async (): Promise => { if (!cachedAccountManager) return; const previous = cachedAccountManager; @@ -1693,6 +1746,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } try { const reloadedManager = await AccountManager.loadFromDisk(); + if (isUntrustworthyEmptyReload(previous, reloadedManager)) { + reloadedManager.disposeShutdownHandler(); + logWarn( + `[${PLUGIN_NAME}] Account reload returned no accounts while ${previous.getAccountCount()} are held; keeping the loaded pool and retrying`, + ); + scheduleEmptyReloadRetry(reloadCachedAccountManager); + return; + } + cancelEmptyReloadRetry(); cachedAccountManager = reloadedManager; accountManagerPromise = Promise.resolve(reloadedManager); // Dispose only after the replacement is installed so we never leak @@ -1725,21 +1787,41 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountsWatcherDisposed = true; unsubscribeAccountsPath?.(); stopAccountsWatcher(); + cancelEmptyReloadRetry(); unregisterCleanup(disposeAccountsWatcher); }; - const readAccountsDigest = async (path: string): Promise => { + const readAccountsFileState = async ( + path: string, + ): Promise<{ digest: string; accountCount: number } | undefined> => { try { const content = await readFile(path, "utf8"); - if (!AnyAccountStorageSchema.safeParse(JSON.parse(content)).success) return; - return createHash("sha256").update(content).digest("hex"); + const data = JSON.parse(content) as unknown; + if (!AnyAccountStorageSchema.safeParse(data).success) return; + // Counted off the raw document rather than the parsed union so the + // count is the same for every storage version. + const accounts = (data as { accounts?: unknown }).accounts; + return { + digest: createHash("sha256").update(content).digest("hex"), + accountCount: Array.isArray(accounts) ? accounts.length : 0, + }; } catch { return; } }; const reloadForExternalAccountsChange = async (path: string, generation: number, attempt = 0, retired?: AccountManager): Promise => { - const digest = await readAccountsDigest(path); - if (generation !== accountsWatchGeneration || !digest || path !== getStoragePath()) return; + const observed = await readAccountsFileState(path); + if (generation !== accountsWatchGeneration || !observed || path !== getStoragePath()) return; + const digest = observed.digest; if (digest === consumeLastWrittenAccountsDigest(path)) return; + const retryLater = (): void => { + if (attempt < 2 && generation === accountsWatchGeneration && !accountsWatcherDisposed) { + accountsReloadTimer = setTimeout(() => { + accountsReloadTimer = undefined; + void reloadForExternalAccountsChange(path, generation, attempt + 1, retired); + }, 1500); + accountsReloadTimer.unref(); + } + }; const previous = cachedAccountManager; try { // A null cache means an invalidation retired the incumbent; the @@ -1760,6 +1842,21 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { reloaded.disposeShutdownHandler(); return; } + // The file this reload observed carried accounts but the load + // produced none, so `loadAccounts()` failed to read it rather than + // the accounts having gone away - a failure it reports as an empty + // result, never as a throw, so the catch below cannot see it. + // Adopting it would answer "No Codex accounts configured" against an + // intact file; the retired incumbent still serves its accounts until + // a retry lands a real one. + if (observed.accountCount > 0 && reloaded.getAccountCount() === 0) { + reloaded.disposeShutdownHandler(); + logWarn( + `[${PLUGIN_NAME}] Externally changed accounts file holds ${observed.accountCount} account(s) but loaded as empty; keeping the current pool and retrying`, + ); + retryLater(); + return; + } const outgoing = cachedAccountManager; if (outgoing && outgoing !== retired) { // Another actor replaced the cached manager while this reload @@ -1775,13 +1872,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { observedAccountsDigest = digest; } catch { logWarn("Could not reload externally updated account storage"); - if (attempt < 2 && generation === accountsWatchGeneration && !accountsWatcherDisposed) { - accountsReloadTimer = setTimeout(() => { - accountsReloadTimer = undefined; - void reloadForExternalAccountsChange(path, generation, attempt + 1, retired); - }, 1500); - accountsReloadTimer.unref(); - } + retryLater(); return; } logDebug("Reloaded cached account manager after external accounts file change"); @@ -1795,8 +1886,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const path = watchedAccountsPath; if (!path) return; const generation = accountsWatchGeneration; - const digest = await readAccountsDigest(path); - if (generation !== accountsWatchGeneration || !digest || digest === observedAccountsDigest) return; + const observed = await readAccountsFileState(path); + if (generation !== accountsWatchGeneration || !observed || observed.digest === observedAccountsDigest) return; + const digest = observed.digest; observedAccountsDigest = digest; clearTimeout(accountsReloadTimer); accountsReloadTimer = undefined; @@ -1821,9 +1913,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }); watchedAccountsPath = path; const generation = accountsWatchGeneration; - const initialDigest = await readAccountsDigest(path); + const initial = await readAccountsFileState(path); if (generation !== accountsWatchGeneration) return; - observedAccountsDigest = initialDigest; + observedAccountsDigest = initial?.digest; // Stat polling follows the path across the storage writer's temp-file rename. watchFile(path, { interval: 1500, persistent: false }, onAccountsStatChanged); unregisterCleanup(disposeAccountsWatcher); @@ -2396,9 +2488,20 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const consumeRetryBudget = ( bucket: RetryBudgetClass, reason: string, + waitMs?: number, ): boolean => { - if (retryBudget.consume(bucket)) { - runtimeMetrics.retryBudgetUsage[bucket] += 1; + // Pass the wait so the charge scales with how long the retry + // blocks. Metrics follow the tracker's own counter rather than + // assuming one unit, or a free sub-second wait would report + // budget it never spent. + const usedBefore = retryBudget.getUsage()[bucket]; + const granted = + waitMs === undefined + ? retryBudget.consume(bucket) + : retryBudget.consumeWait(bucket, waitMs); + if (granted) { + runtimeMetrics.retryBudgetUsage[bucket] += + retryBudget.getUsage()[bucket] - usedBefore; return true; } runtimeMetrics.retryBudgetExhaustions += 1; @@ -2450,16 +2553,35 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { totalMs: number, message: string, intervalMs: number = 5000, + probeUpstream?: () => Promise, ): Promise => { const startTime = Date.now(); const endTime = startTime + totalMs; - + let probeDelayMs = UPSTREAM_REPROBE_FIRST_DELAY_MS; + let nextProbeAt = + probeUpstream && totalMs >= UPSTREAM_REPROBE_MIN_WAIT_MS + ? startTime + probeDelayMs + : Number.POSITIVE_INFINITY; + while (Date.now() < endTime) { if (cachedAccountManager !== accountManager) return; if (abortSignal?.aborted) { throw abortError(); } - + + if (probeUpstream && Date.now() >= nextProbeAt) { + if (await probeUpstream()) return; + if (cachedAccountManager !== accountManager) return; + if (abortSignal?.aborted) { + throw abortError(); + } + probeDelayMs = Math.min(probeDelayMs * 2, UPSTREAM_REPROBE_MAX_DELAY_MS); + // Measured from the end of the probe, so a slow usage + // request cannot schedule the next one in the past and + // collapse the countdown sleep below to zero. + nextProbeAt = Date.now() + probeDelayMs; + } + const remaining = Math.max(0, endTime - Date.now()); const waitLabel = formatWaitTime(remaining); await showToast( @@ -2467,8 +2589,8 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { "warning", { duration: Math.min(intervalMs + 1000, toastDurationMs) }, ); - - const sleepTime = Math.min(intervalMs, remaining); + + const sleepTime = Math.min(intervalMs, remaining, nextProbeAt - Date.now()); if (sleepTime > 0) { await sleep(sleepTime); } else { @@ -2477,6 +2599,32 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + /** + * True when an all-accounts wait can stop early. + * + * `runNow` refreshes `/wham/usage` for every account and persists + * whatever it finds, so a reset that never touched local disk + * becomes visible here. Persisting a recovery also drops the cached + * manager, which is what makes the enclosing retry loop re-resolve + * one that no longer reports a block. + */ + const probeUpstreamBlockLifted = async (): Promise => { + try { + await quotaMonitor.runNow(); + } catch (error) { + logDebug( + `[${PLUGIN_NAME}] Upstream quota re-probe failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return false; + } + if (cachedAccountManager !== accountManager) return true; + const manager = accountManager; + if (!manager) return false; + return manager.getMinWaitTimeForFamily(modelFamily, model) === 0; + }; + let allRateLimitedRetries = 0; let emptyResponseRetries = 0; const attemptedUnsupportedFallbackModels = new Set(); @@ -3795,10 +3943,16 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { consumeRetryBudget( "rateLimitGlobal", `All accounts rate-limited wait ${waitMs}ms`, + waitMs, ) ) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; - await sleepWithCountdown(addJitter(waitMs, 0.2), countdownMessage); + await sleepWithCountdown( + addJitter(waitMs, 0.2), + countdownMessage, + undefined, + probeUpstreamBlockLifted, + ); allRateLimitedRetries++; continue; } diff --git a/lib/AGENTS.md b/lib/AGENTS.md index 1729e4b6..8e9c0aa5 100644 --- a/lib/AGENTS.md +++ b/lib/AGENTS.md @@ -78,6 +78,8 @@ lib/ | Storage keychain | `storage/keychain.ts` | optional native keychain backend | | Storage migrations | `storage/migrations.ts` | V1 → V3 upgrade; V2 files throw a StorageError with code UNKNOWN_V2_FORMAT | | Backups/import/export | `storage/backup.ts`, `storage/export-import.ts` | timestamped backups and dry-run import preview | +| Credential snapshots | `storage/credential-snapshots.ts` | pre-write copy of the previous account store, denylist significance check, prefix-scoped retention | +| Test-home write guard | `storage/test-home-guard.ts` | refuses storage writes inside the real home during a vitest run | | Tool registry | `tools/index.ts` | `ToolContext`, `createToolRegistry` | | TUI quota status | `tui-status.ts`, `tui-quota-cache.ts`, `codex-usage.ts` | prompt quota display and usage cache | | Error types | `errors.ts`, `error-sentinels.ts` | StorageError and structured sentinel errors | diff --git a/lib/config.ts b/lib/config.ts index 11cc1bb7..70f04724 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -91,6 +91,8 @@ const DEFAULT_CONFIG: PluginConfig = { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, @@ -1013,6 +1015,34 @@ export function getPerProjectAccounts(pluginConfig: PluginConfig): boolean { ); } +/** + * Whether the credential store is snapshotted before a significant write. + * + * On by default: the snapshots are the only recourse if the accounts file is + * ever replaced wholesale, and they are worth little unless they are recent + * enough to hold refresh tokens that still work. + */ +export function getCredentialSnapshots(pluginConfig: PluginConfig): boolean { + return resolveBooleanSetting( + "CODEX_AUTH_CREDENTIAL_SNAPSHOTS", + pluginConfig.credentialSnapshots, + true, + ); +} + +/** + * How many credential snapshots to keep. `0` keeps every snapshot; turning the + * feature off is {@link getCredentialSnapshots}' job, not a magic zero. + */ +export function getCredentialSnapshotsMaxCount(pluginConfig: PluginConfig): number { + return resolveNumberSetting( + "CODEX_AUTH_CREDENTIAL_SNAPSHOTS_MAX_COUNT", + pluginConfig.credentialSnapshotsMaxCount, + 10, + { min: 0 }, + ); +} + export function getParallelProbing(pluginConfig: PluginConfig): boolean { return resolveBooleanSetting( "CODEX_AUTH_PARALLEL_PROBING", diff --git a/lib/request/retry-budget.ts b/lib/request/retry-budget.ts index d1ea2beb..a86d01bf 100644 --- a/lib/request/retry-budget.ts +++ b/lib/request/retry-budget.ts @@ -43,6 +43,18 @@ const PROFILE_LIMITS: Record = { }, }; +/** + * How much blocking one budget unit buys when a retry is charged through + * {@link RetryBudgetTracker.consumeWait}. + * + * The budgets are small (1/3/10) and were being charged one unit per wait + * regardless of length, so three consecutive sub-second waits exhausted the + * default and hard-failed a request that one more second would have served. + * Waiting is only expensive in proportion to the time it costs the caller, so + * that is what a unit now measures. + */ +export const RETRY_WAIT_BUDGET_UNIT_MS = 5_000; + const RETRY_BUDGET_CLASSES: RetryBudgetClass[] = [ "authRefresh", "network", @@ -87,12 +99,42 @@ function createUsedCounters(): RetryBudgetLimits { export class RetryBudgetTracker { private readonly used: RetryBudgetLimits = createUsedCounters(); + private readonly waitCarryMs: RetryBudgetLimits = createUsedCounters(); private readonly limits: RetryBudgetLimits; constructor(limits: RetryBudgetLimits) { this.limits = { ...limits }; } + /** + * Charge a retry that blocks for `waitMs` against a bucket, in proportion to + * how long it blocks. + * + * A wait of {@link RETRY_WAIT_BUDGET_UNIT_MS} or longer costs a full unit, + * so a multi-hour block stays governed exactly as before. Shorter waits + * accumulate on a per-bucket carry and only cost a unit once they have added + * up to one, so a burst of sub-second waits is effectively free. + * + * An exhausted bucket refuses even a free wait: the carry bounds how long + * short waits can loop, and without that check they would loop forever once + * the budget ran out. + */ + consumeWait(bucket: RetryBudgetClass, waitMs: number): boolean { + if (this.getRemaining(bucket) <= 0) return false; + + const wait = Number.isFinite(waitMs) && waitMs > 0 ? waitMs : 0; + if (wait >= RETRY_WAIT_BUDGET_UNIT_MS) return this.consume(bucket); + + const carried = this.waitCarryMs[bucket] + wait; + if (carried < RETRY_WAIT_BUDGET_UNIT_MS) { + this.waitCarryMs[bucket] = carried; + return true; + } + + this.waitCarryMs[bucket] = carried - RETRY_WAIT_BUDGET_UNIT_MS; + return this.consume(bucket); + } + consume(bucket: RetryBudgetClass): boolean { const limit = this.limits[bucket]; if (!Number.isFinite(limit)) { diff --git a/lib/schemas.ts b/lib/schemas.ts index f66ff438..36c99612 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -55,6 +55,8 @@ export const PluginConfigSchema = z.object({ toastDurationMs: z.number().min(1000).optional(), accountToasts: z.boolean().optional(), perProjectAccounts: z.boolean().optional(), + credentialSnapshots: z.boolean().optional(), + credentialSnapshotsMaxCount: z.number().int().min(0).optional(), sessionRecovery: z.boolean().optional(), autoResume: z.boolean().optional(), autoUpdate: z.boolean().optional(), diff --git a/lib/storage/backup.ts b/lib/storage/backup.ts index a8e38939..1ebfeab8 100644 --- a/lib/storage/backup.ts +++ b/lib/storage/backup.ts @@ -6,6 +6,13 @@ * dependency on the import pipeline. `writePreImportBackupFile` is the * bounded-time writer used inside `importAccounts` to snapshot the existing * accounts file before apply. + * + * Every file written here holds live refresh tokens, so the writer is the one + * place that owns the 0600 mode, the bounded write time, and the temp+rename + * swap. `writeBackupFileContent` exists so a caller that already has the exact + * bytes it wants preserved — the pre-write credential snapshotter, which + * copies the previous file verbatim rather than re-serializing it — does not + * hand-roll a second writer with weaker guarantees. */ import { promises as fs } from "node:fs"; @@ -39,22 +46,56 @@ function sanitizeBackupPrefix(prefix: string): string { return safe.length > 0 ? safe : "codex-backup"; } -export function createTimestampedBackupPath(prefix = "codex-backup"): string { - const storagePath = getStoragePath(); - const backupDir = join(dirname(storagePath), "backups"); +/** + * The `backups/` directory that belongs to one accounts file. + * + * Every backup kind shares it: pre-import backups, keychain migration + * artefacts, pre-global-migration directories, and credential snapshots. A + * caller that prunes must therefore scope its deletes by its own filename + * prefix rather than by this directory. + */ +export function getBackupDirectory(storagePath: string): string { + return join(dirname(storagePath), "backups"); +} + +/** + * Timestamped backup path beside an explicitly named accounts file. + * + * Callers that write a backup for a path other than the currently active one — + * the global-storage migration writes to the global file while a project path + * is active — must use this rather than {@link createTimestampedBackupPath}, + * so the backup lands next to the file it describes. + */ +export function createTimestampedBackupPathFor( + storagePath: string, + prefix = "codex-backup", +): string { const safePrefix = sanitizeBackupPrefix(prefix); const nonce = randomBytes(3).toString("hex"); - return join(backupDir, `${safePrefix}-${formatBackupTimestamp()}-${nonce}.json`); + return join( + getBackupDirectory(storagePath), + `${safePrefix}-${formatBackupTimestamp()}-${nonce}.json`, + ); } -export async function writePreImportBackupFile(backupPath: string, snapshot: AccountStorageV3): Promise { +export function createTimestampedBackupPath(prefix = "codex-backup"): string { + return createTimestampedBackupPathFor(getStoragePath(), prefix); +} + +/** + * Write one backup file atomically, with a bounded write time and mode 0600. + * + * `content` is written verbatim. Temp+rename keeps a half-written backup from + * ever being visible under its final name, and the timeout keeps a stuck disk + * from blocking the operation the backup precedes. + */ +export async function writeBackupFileContent(backupPath: string, content: string): Promise { const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; const tempPath = `${backupPath}.${uniqueSuffix}.tmp`; try { await fs.mkdir(dirname(backupPath), { recursive: true }); - const backupContent = JSON.stringify(snapshot, null, 2); - await writeFileWithTimeout(tempPath, backupContent, PRE_IMPORT_BACKUP_WRITE_TIMEOUT_MS); + await writeFileWithTimeout(tempPath, content, PRE_IMPORT_BACKUP_WRITE_TIMEOUT_MS); await renameWithWindowsRetry(tempPath, backupPath); } catch (error) { try { @@ -65,3 +106,7 @@ export async function writePreImportBackupFile(backupPath: string, snapshot: Acc throw error; } } + +export async function writePreImportBackupFile(backupPath: string, snapshot: AccountStorageV3): Promise { + await writeBackupFileContent(backupPath, JSON.stringify(snapshot, null, 2)); +} diff --git a/lib/storage/credential-snapshots.ts b/lib/storage/credential-snapshots.ts new file mode 100644 index 00000000..c1233265 --- /dev/null +++ b/lib/storage/credential-snapshots.ts @@ -0,0 +1,325 @@ +/** + * Pre-write snapshots of the credential store. + * + * The store is a single JSON file holding every account's live refresh token. + * Anything that replaces it wholesale — a bad merge, a test run that escaped + * its sandbox, a partial restore — takes the tokens with it, and a backup old + * enough to predate the last few refreshes restores accounts whose refresh + * tokens have since been rotated and are therefore dead. This module keeps a + * bounded ring of recent snapshots so there is always a *live-token* copy to + * restore from. + * + * Two decisions carry the design: + * + * 1. The snapshot captures the document already on disk, taken before the + * new one replaces it. Snapshotting the incoming document would be + * useless for the case this exists for: a clobber would simply be + * snapshotted as a clobber. What is worth keeping is the last good state. + * + * 2. Significance is a denylist, not an allowlist. Everything counts as a + * significant change unless it is explicitly ignored below. A field added + * to the schema later therefore cannot silently switch snapshots off; the + * worst it can do is cost one extra snapshot, which is recoverable, where + * a missing snapshot is not. + */ + +import { promises as fs } from "node:fs"; +import { join } from "node:path"; +import { loadPluginConfig, getCredentialSnapshots, getCredentialSnapshotsMaxCount } from "../config.js"; +import { createLogger } from "../logger.js"; +import { + createTimestampedBackupPathFor, + getBackupDirectory, + writeBackupFileContent, +} from "./backup.js"; +import { StorageError } from "./errors.js"; +import { isKeychainOptInEnabled } from "./keychain.js"; +import { + assertTestRunNeverTouchesRealHome, + TEST_HOME_ESCAPE_CODE, +} from "./test-home-guard.js"; +import type { AccountStorageV3 } from "./migrations.js"; + +const log = createLogger("credential-snapshots"); + +/** + * Filename prefix owned exclusively by this module. + * + * `backups/` is shared with `codex-pre-import-backup-*`, `codex-backup-*`, + * `*.migrated-to-keychain.*` and `pre-global-migration-*`. Retention deletes + * strictly by this prefix, because deleting one of those would be a + * data-loss bug inside a feature whose only purpose is preventing data loss. + */ +export const CREDENTIAL_SNAPSHOT_PREFIX = "codex-credential-snapshot"; + +/** + * Document-level fields that never, on their own, justify a snapshot. + * + * The rotation cursor moves on essentially every request under the default + * hybrid strategy. Snapshotting on it would churn the whole ring away within + * minutes and leave nothing but cursor movements to restore from. + */ +const IGNORED_ROOT_FIELDS: ReadonlySet = new Set([ + "activeIndex", + "activeIndexByFamily", +]); + +/** + * Per-account fields that never, on their own, justify a snapshot. + * + * All of it is scheduling churn: it changes constantly, it is re-derived from + * upstream on the next request, and none of it is recoverable state. Note what + * is deliberately absent — `refreshToken`, `accessToken`, `expiresAt`, + * `tokenRotatedAt` — so every token refresh produces a snapshot. That is the + * case that matters most: a snapshot whose tokens are stale restores accounts + * that cannot authenticate. + */ +const IGNORED_ACCOUNT_FIELDS: ReadonlySet = new Set([ + "lastUsed", + "lastSwitchReason", + "rateLimitResetTimes", + "rateLimitResetTime", + "coolingDownUntil", + "cooldownReason", + "quotaExhaustedUntil", + "quotaExhaustedStampAt", + "quotaExhaustedClearedAt", +]); + +export function isCredentialSnapshotFileName(name: string): boolean { + return name.startsWith(`${CREDENTIAL_SNAPSHOT_PREFIX}-`) && name.endsWith(".json"); +} + +/** + * Sort keys and drop `undefined`-valued ones so two documents with identical + * content compare equal as strings. + * + * This is load-bearing rather than tidiness: the previous document comes from + * `JSON.parse`, so its key order is the file's, while the incoming one is + * built in code. Comparing their raw serializations would report a difference + * on every single write. Dropping `undefined` matches `JSON.stringify`, which + * omits those keys when the document is written, so an in-memory + * `{ email: undefined }` and an on-disk absent `email` are the same state. + */ +function canonicalize(value: unknown): unknown { + if (Array.isArray(value)) return value.map(canonicalize); + if (value === null || typeof value !== "object") return value; + + const source = value as Record; + const result: Record = {}; + for (const key of Object.keys(source).sort()) { + const entry = source[key]; + if (entry === undefined) continue; + result[key] = canonicalize(entry); + } + return result; +} + +function projectAccount(account: unknown): unknown { + if (account === null || typeof account !== "object" || Array.isArray(account)) { + return account; + } + const source = account as Record; + const result: Record = {}; + for (const key of Object.keys(source)) { + if (IGNORED_ACCOUNT_FIELDS.has(key)) continue; + result[key] = source[key]; + } + return result; +} + +/** + * The document reduced to the parts a snapshot exists to preserve. + * + * Accounts are compared position-wise, so a reorder reads as significant. That + * is the intended bias: an extra snapshot costs one file, a missed one costs + * the credentials. + */ +function significantProjection(document: unknown): string { + if (document === null || typeof document !== "object" || Array.isArray(document)) { + return JSON.stringify(canonicalize(document)) ?? "null"; + } + + const source = document as Record; + const projected: Record = {}; + for (const key of Object.keys(source)) { + if (IGNORED_ROOT_FIELDS.has(key) || key === "accounts") continue; + projected[key] = source[key]; + } + + const accounts = source.accounts; + projected.accounts = Array.isArray(accounts) ? accounts.map(projectAccount) : accounts; + + return JSON.stringify(canonicalize(projected)) ?? "null"; +} + +export function isSignificantStorageChange( + previousContent: string, + next: AccountStorageV3, +): boolean { + let previous: unknown; + try { + previous = JSON.parse(previousContent) as unknown; + } catch { + // A file that no longer parses is precisely the state worth preserving: + // the write about to happen replaces it, and whatever it held is then gone + // for good. Treat it as changed so it is captured before that happens. + return true; + } + return significantProjection(previous) !== significantProjection(next); +} + +async function restrictDirectoryMode(directory: string): Promise { + if (process.platform === "win32") return; + try { + await fs.chmod(directory, 0o700); + } catch (error) { + log.warn("Failed to restrict credential snapshot directory to 0700", { + path: directory, + error: String(error), + }); + } +} + +/** + * Delete all but the newest `maxCount` snapshots. + * + * `maxCount <= 0` keeps every snapshot; turning the feature off is the + * boolean setting's job, not a magic zero. + */ +export async function pruneCredentialSnapshots( + backupDirectory: string, + maxCount: number, +): Promise { + if (!Number.isFinite(maxCount) || maxCount <= 0) return; + + let entries: string[]; + try { + entries = await fs.readdir(backupDirectory); + } catch { + return; + } + + const candidates = entries.filter(isCredentialSnapshotFileName); + if (candidates.length <= maxCount) return; + + const dated = await Promise.all( + candidates.map(async (name) => { + const full = join(backupDirectory, name); + let mtimeMs = Number.NEGATIVE_INFINITY; + let isFile = false; + try { + const stats = await fs.stat(full); + mtimeMs = stats.mtimeMs; + isFile = stats.isFile(); + } catch { + // Vanished between readdir and stat: leave it out rather than racing + // another process for the unlink. + } + return { full, name, mtimeMs, isFile }; + }), + ); + + // Newest first. The embedded timestamp breaks mtime ties, which happens when + // several snapshots land inside one filesystem timestamp granule. + const ordered = dated + .filter((entry) => entry.isFile) + .sort((a, b) => { + if (b.mtimeMs !== a.mtimeMs) return b.mtimeMs - a.mtimeMs; + return a.name < b.name ? 1 : a.name > b.name ? -1 : 0; + }); + + for (const stale of ordered.slice(maxCount)) { + try { + await fs.unlink(stale.full); + } catch (error) { + log.warn("Failed to prune credential snapshot", { + path: stale.full, + error: String(error), + }); + } + } +} + +/** + * Preserve the document currently at `storagePath` before it is replaced. + * + * `next` is the document about to be written, or `null` when the store is + * about to be deleted outright — deletion is unconditionally significant. + * + * Callers must already hold the storage lock, so the snapshot is consistent + * with the write it precedes. + */ +export async function snapshotCredentialStoreBeforeWrite( + storagePath: string, + next: AccountStorageV3 | null, +): Promise { + const config = loadPluginConfig(); + if (!getCredentialSnapshots(config)) return; + + // Scoped to the JSON backend, enforced here rather than at each call site. + // Under the keychain opt-in the authoritative pool lives in the OS keychain, + // and whatever JSON remains at `storagePath` is a pre-migration or + // write-fallback artefact. Copying it into `backups/` would put the whole + // token set in a plaintext file that a user who opted into the keychain + // asked us not to create, and it would archive a document that is already + // stale. Every write path - ordinary save, the JSON fallback after a failed + // keychain write, and `clearAccounts` - goes through here, so one check + // covers all of them. + if (isKeychainOptInEnabled()) return; + + const backupDirectory = getBackupDirectory(storagePath); + assertTestRunNeverTouchesRealHome(backupDirectory); + + let previousContent: string; + try { + previousContent = await fs.readFile(storagePath, "utf-8"); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn("Skipping credential snapshot: existing store is unreadable", { + path: storagePath, + error: String(error), + }); + } + // No file yet is the ordinary first-write case: there is no previous state + // to preserve, which is not an error. + return; + } + + if (next !== null && !isSignificantStorageChange(previousContent, next)) return; + + const snapshotPath = createTimestampedBackupPathFor(storagePath, CREDENTIAL_SNAPSHOT_PREFIX); + await fs.mkdir(backupDirectory, { recursive: true, mode: 0o700 }); + await restrictDirectoryMode(backupDirectory); + await writeBackupFileContent(snapshotPath, previousContent); + log.info("Captured credential store snapshot", { path: snapshotPath }); + + await pruneCredentialSnapshots(backupDirectory, getCredentialSnapshotsMaxCount(config)); +} + +/** + * {@link snapshotCredentialStoreBeforeWrite}, with every failure downgraded to + * a warning. + * + * A snapshot is a safety net, never a precondition. Letting a transient disk + * error fail the write it precedes would break a token refresh, and therefore + * the user's live sessions, to protect a copy of the file — strictly worse + * than having no snapshot. The one exception is the test-home guard: that + * exists to stop a test run writing over real credentials, so swallowing it + * would disarm it. + */ +export async function trySnapshotCredentialStoreBeforeWrite( + storagePath: string, + next: AccountStorageV3 | null, +): Promise { + try { + await snapshotCredentialStoreBeforeWrite(storagePath, next); + } catch (error) { + if (error instanceof StorageError && error.code === TEST_HOME_ESCAPE_CODE) throw error; + log.warn("Credential snapshot failed; continuing with the write", { + path: storagePath, + error: String(error), + }); + } +} diff --git a/lib/storage/load-save.ts b/lib/storage/load-save.ts index fddcbe71..d18271f9 100644 --- a/lib/storage/load-save.ts +++ b/lib/storage/load-save.ts @@ -26,6 +26,11 @@ import { renameWithWindowsRetry } from "./atomic-write.js"; import { formatStorageErrorHint, StorageError } from "./errors.js"; import { normalizeAccountStorage } from "./normalize.js"; import { getConfigDir } from "./paths.js"; +import { + assertTestRunNeverTouchesRealHome, + TEST_HOME_ESCAPE_CODE, +} from "./test-home-guard.js"; +import { trySnapshotCredentialStoreBeforeWrite } from "./credential-snapshots.js"; import { getCurrentLegacyProjectStoragePath, getCurrentProjectRoot, @@ -142,6 +147,10 @@ async function checkWorktreeLockForCurrentStorage( }); return; } + // Before the probe, not inside the try: `acquireOrDetectLock` writes a lock + // sidecar next to the accounts file, so a leaked HOME would touch the real + // store here even on a pure read, and this function's catch would hide it. + assertTestRunNeverTouchesRealHome(path); try { const result = await acquireOrDetectLock(path); if (!result.acquired && result.foreign) { @@ -537,6 +546,7 @@ async function loadAccountsInternal( * Callers must already be inside withStorageLock when using this helper directly. */ async function writeAccountsToPathUnlocked(path: string, storage: AccountStorageV3): Promise { + assertTestRunNeverTouchesRealHome(path); const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; const tempPath = `${path}.${uniqueSuffix}.tmp`; @@ -547,6 +557,12 @@ async function writeAccountsToPathUnlocked(path: string, storage: AccountStorage // Normalize before persisting so every write path enforces dedup semantics // (exact identity dedupe plus legacy email dedupe for identity-less records). const normalizedStorage = normalizeAccountStorage(storage) ?? storage; + // Preserve what is on disk now, before it is replaced. Compared against + // the normalized payload rather than the caller's, so a difference + // normalization erases never costs a snapshot. We are already inside + // `withStorageLock`, so the captured state is exactly the state this write + // supersedes. + await trySnapshotCredentialStoreBeforeWrite(path, normalizedStorage); const content = JSON.stringify(normalizedStorage, null, 2); await fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }); @@ -672,6 +688,13 @@ async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { await checkWorktreeLockForCurrentStorage("save"); if (isKeychainOptInEnabled()) { + // Credential snapshots are scoped to the JSON backend and do not cover + // keychain mode. That is enforced inside the snapshotter itself rather + // than by the absence of a call here, so neither the JSON fallback below + // nor `clearAccounts` can reintroduce a plaintext copy of the token set - + // see `snapshotCredentialStoreBeforeWrite`. Keychain users' recovery path + // stays `codex-export` plus the keychain's own backing store. + // // Normalize before serializing so the keychain receives the same shape // the JSON backend would have written. Using the same JSON format keeps // migration and rollback symmetric: a rolled-back JSON file is valid @@ -753,14 +776,34 @@ export async function saveAccounts(storage: AccountStorageV3): Promise { * keychain delete and log at `error`. Both copies remain in sync so the * caller can retry safely. The operation is still best-effort (never * throws) to preserve the existing contract above the storage layer. + * + * @throws StorageError (code `TEST_HOME_ESCAPE`) - the single exception to + * best-effort, and inert outside vitest. The guard refuses the deletion, so + * absorbing it would return success for a clear that never happened. */ export async function clearAccounts(): Promise { return withStorageLock(async () => { let jsonCleared = true; try { const path = getStoragePath(); + assertTestRunNeverTouchesRealHome(path); + // Deleting the store outright needs no significance test - `null` says + // there is no successor document to compare against. The snapshotter + // still applies its own config and keychain gates. + await trySnapshotCredentialStoreBeforeWrite(path, null); await fs.unlink(path); } catch (error) { + // The test-home guard is not a storage failure to absorb. It fires only + // under vitest, and it exists to fail a run that escaped its sandbox; it + // throws before the unlink, so swallowing it here would report a + // successful clear for a deletion that deliberately did not happen - + // fail-closed downgraded to fail-open on the one path that destroys the + // store. The same re-throw covers the snapshotter, which surfaces this + // code through `trySnapshotCredentialStoreBeforeWrite` for the same + // reason. + if (error instanceof StorageError && error.code === TEST_HOME_ESCAPE_CODE) { + throw error; + } const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { jsonCleared = false; diff --git a/lib/storage/paths.ts b/lib/storage/paths.ts index 1dc45bc6..ad2e11ce 100644 --- a/lib/storage/paths.ts +++ b/lib/storage/paths.ts @@ -81,7 +81,7 @@ function normalizePathForComparison(filePath: string): string { return process.platform === "win32" ? resolvedPath.toLowerCase() : resolvedPath; } -function isWithinDirectory(baseDir: string, targetPath: string): boolean { +export function isWithinDirectory(baseDir: string, targetPath: string): boolean { const normalizedBase = normalizePathForComparison(baseDir); const normalizedTarget = normalizePathForComparison(targetPath); const rel = relative(normalizedBase, normalizedTarget); diff --git a/lib/storage/test-home-guard.ts b/lib/storage/test-home-guard.ts new file mode 100644 index 00000000..b6c222d1 --- /dev/null +++ b/lib/storage/test-home-guard.ts @@ -0,0 +1,50 @@ +/** + * The test-run write guard, shared by every storage writer. + * + * Extracted from `lib/storage/load-save.ts` so the credential-snapshot writer + * can apply the same check. Importing it from `load-save.ts` directly would be + * a cycle: `load-save.ts` is what triggers snapshots in the first place. + */ + +import os from "node:os"; +import { StorageError } from "./errors.js"; +import { isWithinDirectory } from "./paths.js"; + +/** + * StorageError code for a write refused inside the developer's real home. + * + * Shared rather than spelled out at each site: several callers deliberately + * absorb storage failures and have to re-throw this one, and a typo in any of + * those copies would silently turn the guard back into a swallowed warning. + */ +export const TEST_HOME_ESCAPE_CODE = "TEST_HOME_ESCAPE"; + +/** + * Refuse to mutate account storage inside the developer's real home while the + * test suite is running. + * + * `vitest.config.ts` redirects HOME to a sandbox before any module loads, but a + * test that restores the captured real HOME, or a future regression in that + * config, would otherwise write fixtures straight over live ChatGPT + * credentials. `os.userInfo()` reads the passwd entry instead of `$HOME`, so it + * still names the real home after the redirect and gives the check something + * the sandbox cannot spoof. Inert outside vitest. + */ +export function assertTestRunNeverTouchesRealHome(path: string): void { + if (!process.env.VITEST) return; + + let realHome: string; + try { + realHome = os.userInfo().homedir; + } catch { + return; + } + if (!realHome || !isWithinDirectory(realHome, path)) return; + + throw new StorageError( + `Refusing to write account storage inside the real home directory during a test run: ${path}`, + TEST_HOME_ESCAPE_CODE, + path, + "A test resolved account storage against the developer's real home. Point HOME at a temp directory for the whole vitest process (see vitest.config.ts) instead of overriding it per test.", + ); +} diff --git a/test/accounts-live-reload.test.ts b/test/accounts-live-reload.test.ts index 1998ded9..3612f87a 100644 --- a/test/accounts-live-reload.test.ts +++ b/test/accounts-live-reload.test.ts @@ -9,6 +9,7 @@ const captured = vi.hoisted((): { context?: ToolContext; listener?: () => void; onWatch?: () => void; + onQuotaProbe?: () => Promise; reads: Promise[]; maxRetries?: number; } => ({ reads: [] })); @@ -32,7 +33,11 @@ vi.mock("../lib/tools/index.js", () => ({ createToolRegistry: (context: ToolContext) => { captured.context = context; return {}; }, })); vi.mock("../lib/quota-notifications.js", () => ({ - createQuotaMonitor: () => ({ start() {}, dispose() {} }), + createQuotaMonitor: () => ({ + start() {}, + dispose() {}, + runNow: async () => { await captured.onQuotaProbe?.(); }, + }), })); vi.mock("../lib/auto-update-checker.js", () => ({ checkAndNotify: vi.fn(async () => {}) })); vi.mock("../lib/config.js", async (original) => ({ @@ -91,6 +96,7 @@ describe("accounts live reload", () => { await fs.writeFile(path, JSON.stringify(storage(true))); captured.listener = undefined; captured.onWatch = undefined; + captured.onQuotaProbe = undefined; captured.reads.length = 0; captured.maxRetries = undefined; plugin = await Reflect.apply(OpenAIOAuthPlugin, undefined, [{ @@ -285,6 +291,106 @@ describe("accounts live reload", () => { await vi.advanceTimersByTimeAsync(5000); expect((await response).status).toBe(200); }); + it("wakes a long wait when an upstream re-probe finds the block lifted", async () => { + const manager = captured.context?.cachedAccountManagerRef.current; + if (!manager) throw new Error("Missing manager"); + const account = manager.getCurrentAccount(); + if (!account) throw new Error("Missing account"); + manager.markQuotaExhausted(account, Date.now() + 86_400_000, "gpt-5.1"); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response("data: [DONE]\n\n", { + status: 200, headers: { "content-type": "text/event-stream" }, + })); + let enteredWait: () => void = () => {}; + const waiting = new Promise((resolve) => { enteredWait = resolve; }); + const minWait = manager.getMinWaitTimeForFamily.bind(manager); + vi.spyOn(manager, "getMinWaitTimeForFamily").mockImplementation((...args) => { + enteredWait(); + return minWait(...args); + }); + let probes = 0; + captured.onQuotaProbe = async () => { + probes += 1; + // What a server-side grant looks like: usage reports the quota back, + // the recovery is persisted, and the cached manager is dropped. The + // accounts file is never written by another process, so the watcher + // has nothing to fire on - this is the wake-up it cannot provide. + captured.context?.invalidateAccountManagerCache(); + }; + const response = request("https://api.openai.com/v1/responses", { + method: "POST", body: JSON.stringify({ model: "gpt-5.1", stream: true, input: [] }), + }); + await waiting; + for (let elapsed = 0; elapsed < 120_000 && probes === 0; elapsed += 5000) { + await vi.advanceTimersByTimeAsync(5000); + } + expect(probes).toBe(1); + await vi.advanceTimersByTimeAsync(5000); + expect((await response).status).toBe(200); + }); + it("wakes a waiting request when a login adds an account mid-sleep", async () => { + const manager = captured.context?.cachedAccountManagerRef.current; + if (!manager) throw new Error("Missing manager"); + const account = manager.getCurrentAccount(); + if (!account) throw new Error("Missing account"); + const blockedUntil = Date.now() + 86_400_000; + manager.markQuotaExhausted(account, blockedUntil, "gpt-5.1"); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response("data: [DONE]\n\n", { + status: 200, headers: { "content-type": "text/event-stream" }, + })); + let enteredWait: () => void = () => {}; + const waiting = new Promise((resolve) => { enteredWait = resolve; }); + const minWait = manager.getMinWaitTimeForFamily.bind(manager); + vi.spyOn(manager, "getMinWaitTimeForFamily").mockImplementation((...args) => { + enteredWait(); + return minWait(...args); + }); + const response = request("https://api.openai.com/v1/responses", { + method: "POST", body: JSON.stringify({ model: "gpt-5.1", stream: true, input: [] }), + }); + await waiting; + const reloaded = nextReload(); + // The incumbent account stays blocked on disk, so the only thing that can + // end this wait is the account the login added. + await fs.writeFile(path, JSON.stringify({ ...storage(true), accounts: [ + { ...storage(true).accounts[0], quotaExhaustedUntil: blockedUntil }, + { accountId: "fresh-login", refreshToken: "fresh-refresh", accessToken: "fresh-access", + expiresAt: Date.now() + 86_400_000, enabled: true, addedAt: 2, lastUsed: 2 }, + ] })); + await tick(); + await settle(); + await reloaded; + await vi.advanceTimersByTimeAsync(5000); + expect((await response).status).toBe(200); + }); + it("keeps the loaded pool when an external change loads as empty", async () => { + const previous = captured.context?.cachedAccountManagerRef.current; + if (!previous) throw new Error("Missing manager"); + expect(previous.getAccountCount()).toBe(1); + const empty = new AccountManager(undefined, { ...storage(true), accounts: [] }); + const load = vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(empty); + await fs.writeFile(path, JSON.stringify(storage(false))); + await tick(); + await settle(); + expect(load).toHaveBeenCalledTimes(1); + expect(captured.context?.cachedAccountManagerRef.current).toBe(previous); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountCount()).toBe(1); + const reloaded = nextReload(); + await vi.advanceTimersByTimeAsync(1500); + await drainReads(); + await reloaded; + expect(load).toHaveBeenCalledTimes(2); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountsSnapshot()[0]?.enabled).toBe(false); + }); + it("adopts an external change that genuinely removes the last account", async () => { + const previous = captured.context?.cachedAccountManagerRef.current; + const reloaded = nextReload(); + await fs.writeFile(path, JSON.stringify({ ...storage(true), accounts: [] })); + await tick(); + await settle(); + await reloaded; + expect(captured.context?.cachedAccountManagerRef.current).not.toBe(previous); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountCount()).toBe(0); + }); it("keeps externally cleared blocks cleared despite queued and late saves from the old manager", async () => { const previous = captured.context?.cachedAccountManagerRef.current; if (!previous) throw new Error("Missing manager"); diff --git a/test/login-runner.test.ts b/test/login-runner.test.ts index b305d025..b57e12e0 100644 --- a/test/login-runner.test.ts +++ b/test/login-runner.test.ts @@ -218,7 +218,13 @@ describe("login-runner persistAccountPool", () => { resolveFirstRename?.(); await Promise.all([firstPersist, secondPersist]); - expect(renameSpy).toHaveBeenCalledTimes(2); + // Count only the renames that publish the accounts file. The + // pre-write credential snapshotter swaps its own file through the + // same `fs.rename`, so a raw call count also counts snapshots. + const accountFileRenames = renameSpy.mock.calls.filter( + ([, destinationPath]) => destinationPath === storagePath, + ); + expect(accountFileRenames).toHaveLength(2); const loaded = await loadAccounts(); expect(loaded?.accounts).toHaveLength(2); expect( diff --git a/test/paths.test.ts b/test/paths.test.ts index ca215ec8..c3a88ceb 100644 --- a/test/paths.test.ts +++ b/test/paths.test.ts @@ -2,10 +2,29 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { homedir, tmpdir } from "node:os"; import path from "node:path"; +// `resolvePath` guards against a path that merely shares a string prefix with +// an allowed root, so the lookalike cases below build a sibling of one root and +// require it to be outside all three. Against the real layout that only holds +// by accident: the suite runs with HOME redirected under the temp dir, which +// makes every sibling of home a child of tmpdir() and turns the assertion into +// a no-op. Fixed roots keep these cases meaningful wherever HOME points. +// +// This mock and the isolated HOME in `vitest.config.ts` are load-bearing +// together. Removing the mock as redundant silently turns both lookalike +// assertions into no-ops rather than failing them. +const FAKE_HOME = path.resolve("/fake-storage-paths-home"); +const FAKE_TMPDIR = path.resolve("/fake-storage-paths-tmp"); + vi.mock("node:fs", () => ({ existsSync: vi.fn(), })); +vi.mock("node:os", async (importOriginal) => ({ + ...(await importOriginal()), + homedir: () => FAKE_HOME, + tmpdir: () => FAKE_TMPDIR, +})); + import { existsSync } from "node:fs"; import { getConfigDir, diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index a74b02e5..fe660950 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -141,6 +141,8 @@ describe('Plugin Configuration', () => { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, @@ -190,6 +192,8 @@ describe('Plugin Configuration', () => { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, @@ -236,6 +240,8 @@ describe('Plugin Configuration', () => { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, @@ -293,6 +299,8 @@ describe('Plugin Configuration', () => { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, @@ -344,6 +352,8 @@ describe('Plugin Configuration', () => { toastDurationMs: 5_000, accountToasts: true, perProjectAccounts: true, + credentialSnapshots: true, + credentialSnapshotsMaxCount: 10, sessionRecovery: true, autoResume: true, autoUpdate: true, diff --git a/test/retry-budget.test.ts b/test/retry-budget.test.ts index 8f3e62fe..8d67dba7 100644 --- a/test/retry-budget.test.ts +++ b/test/retry-budget.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest"; import { RetryBudgetTracker, resolveRetryBudgetLimits, + RETRY_WAIT_BUDGET_UNIT_MS, type RetryBudgetLimits, } from "../lib/request/retry-budget.js"; @@ -72,6 +73,84 @@ describe("retry-budget", () => { expect(tracker.getUsage().authRefresh).toBe(1); }); + describe("consumeWait", () => { + const balanced = () => new RetryBudgetTracker(resolveRetryBudgetLimits("balanced")); + + it("does not charge the budget for a burst of sub-second waits", () => { + const tracker = balanced(); + + // The production regression: three consecutive 400ms waits spent the + // whole default budget and hard-failed the request. + for (let i = 0; i < 3; i++) { + expect(tracker.consumeWait("rateLimitGlobal", 400)).toBe(true); + } + expect(tracker.getUsage().rateLimitGlobal).toBe(0); + expect(tracker.getRemaining("rateLimitGlobal")).toBe(3); + + for (let i = 0; i < 27; i++) { + expect(tracker.consumeWait("rateLimitGlobal", 400)).toBe(true); + } + }); + + it("charges a full unit for a wait at or above the unit length", () => { + const tracker = balanced(); + + expect(tracker.consumeWait("rateLimitGlobal", RETRY_WAIT_BUDGET_UNIT_MS)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(true); + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(3); + + // A long wait stays governed: the fourth exceeds the balanced budget. + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(false); + }); + + it("accumulates short waits into whole units", () => { + const tracker = balanced(); + const waitMs = RETRY_WAIT_BUDGET_UNIT_MS / 10; + + for (let i = 0; i < 10; i++) { + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + } + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + }); + + it("refuses free short waits once the bucket is exhausted", () => { + const tracker = balanced(); + + for (let i = 0; i < 3; i++) { + expect(tracker.consumeWait("rateLimitGlobal", RETRY_WAIT_BUDGET_UNIT_MS)).toBe(true); + } + + // Without this the carry would grant sub-unit waits forever and the + // retry loop could never terminate. + expect(tracker.consumeWait("rateLimitGlobal", 1)).toBe(false); + expect(tracker.consumeWait("rateLimitGlobal", 0)).toBe(false); + }); + + it("keeps per-bucket carries independent", () => { + const tracker = balanced(); + const waitMs = RETRY_WAIT_BUDGET_UNIT_MS / 2; + + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + expect(tracker.consumeWait("rateLimitShort", waitMs)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(0); + expect(tracker.getUsage().rateLimitShort).toBe(0); + + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + expect(tracker.getUsage().rateLimitShort).toBe(0); + }); + + it("treats a zero-limit bucket as immediately exhausted", () => { + const tracker = new RetryBudgetTracker( + resolveRetryBudgetLimits("balanced", { rateLimitGlobal: 0 }), + ); + expect(tracker.consumeWait("rateLimitGlobal", 1)).toBe(false); + }); + }); + it("clones constructor limits to avoid external mutation", () => { const limits: RetryBudgetLimits = { authRefresh: 1, diff --git a/test/storage-credential-snapshots.test.ts b/test/storage-credential-snapshots.test.ts new file mode 100644 index 00000000..21f984b1 --- /dev/null +++ b/test/storage-credential-snapshots.test.ts @@ -0,0 +1,529 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { promises as fs } from "node:fs"; +import { join, resolve } from "node:path"; +import { mkdtemp } from "node:fs/promises"; +import { tmpdir, userInfo } from "node:os"; +import { + _resetBackendForTests, + _setBackendForTests, + type KeychainBackend, +} from "../lib/storage/keychain.js"; +import { TEST_HOME_ESCAPE_CODE } from "../lib/storage/test-home-guard.js"; +import { + clearAccounts, + saveAccounts, + setStoragePathDirect, + type AccountStorageV3, +} from "../lib/storage.js"; +import { + CREDENTIAL_SNAPSHOT_PREFIX, + isCredentialSnapshotFileName, + isSignificantStorageChange, + pruneCredentialSnapshots, +} from "../lib/storage/credential-snapshots.js"; +import { MODEL_FAMILIES } from "../lib/prompts/codex.js"; + +type StoredAccount = AccountStorageV3["accounts"][number]; + +let testDir: string; +let storagePath: string; +let backupsDir: string; + +function makeAccount(overrides: Partial = {}): StoredAccount { + return { + accountId: "acct-1", + email: "one@example.com", + refreshToken: "rt-1", + accessToken: "at-1", + expiresAt: 1_900_000_000_000, + addedAt: 1_000, + lastUsed: 2_000, + ...overrides, + }; +} + +function makeStorage(): AccountStorageV3 { + return { + version: 3, + accounts: [ + makeAccount(), + makeAccount({ + accountId: "acct-2", + email: "two@example.com", + refreshToken: "rt-2", + accessToken: "at-2", + }), + ], + activeIndex: 0, + }; +} + +function withAccount( + storage: AccountStorageV3, + index: number, + mutate: (account: StoredAccount) => StoredAccount, +): AccountStorageV3 { + return { + ...storage, + accounts: storage.accounts.map((account, i) => (i === index ? mutate({ ...account }) : account)), + }; +} + +async function listSnapshotNames(): Promise { + try { + return (await fs.readdir(backupsDir)).filter(isCredentialSnapshotFileName).sort(); + } catch { + return []; + } +} + +async function readSnapshotContents(): Promise { + const names = await listSnapshotNames(); + return Promise.all(names.map((name) => fs.readFile(join(backupsDir, name), "utf-8"))); +} + +async function readLiveStore(): Promise { + return JSON.parse(await fs.readFile(storagePath, "utf-8")) as AccountStorageV3; +} + +async function exists(path: string): Promise { + try { + await fs.stat(path); + return true; + } catch { + return false; + } +} + +beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), "oc-codex-credential-snapshots-")); + storagePath = join(testDir, "oc-codex-multi-auth-accounts.json"); + backupsDir = join(testDir, "backups"); + setStoragePathDirect(storagePath); +}); + +afterEach(async () => { + setStoragePathDirect(null); + vi.unstubAllEnvs(); + await fs.rm(testDir, { recursive: true, force: true }); +}); + +describe("credential snapshots: significance", () => { + it("writes no snapshot on the first save because there is no previous state", async () => { + await saveAccounts(makeStorage()); + + expect(await listSnapshotNames()).toEqual([]); + expect(await exists(backupsDir)).toBe(false); + }); + + const ignoredChanges: Array<[string, (storage: AccountStorageV3) => AccountStorageV3]> = [ + [ + "lastUsed", + (storage) => withAccount(storage, 0, (a) => ({ ...a, lastUsed: a.lastUsed + 90_000 })), + ], + [ + "lastSwitchReason", + (storage) => withAccount(storage, 0, (a) => ({ ...a, lastSwitchReason: "rotation" })), + ], + [ + "rateLimitResetTimes", + (storage) => + withAccount(storage, 0, (a) => ({ + ...a, + rateLimitResetTimes: { [MODEL_FAMILIES[0]]: Date.now() + 60_000 }, + })), + ], + [ + "coolingDownUntil and cooldownReason", + (storage) => + withAccount(storage, 0, (a) => ({ + ...a, + coolingDownUntil: Date.now() + 60_000, + cooldownReason: "network-error", + })), + ], + [ + "quota exhaustion stamps", + (storage) => + withAccount(storage, 0, (a) => ({ + ...a, + quotaExhaustedUntil: Date.now() + 60_000, + quotaExhaustedStampAt: Date.now(), + })), + ], + ["activeIndex", (storage) => ({ ...storage, activeIndex: 1 })], + [ + "activeIndexByFamily", + (storage) => ({ ...storage, activeIndexByFamily: { [MODEL_FAMILIES[0]]: 1 } }), + ], + ]; + + it.each(ignoredChanges)("takes no snapshot for a %s-only change", async (_label, mutate) => { + const base = makeStorage(); + await saveAccounts(base); + + await saveAccounts(mutate(base)); + + expect(await listSnapshotNames()).toEqual([]); + }); + + const significantChanges: Array<[string, (storage: AccountStorageV3) => AccountStorageV3]> = [ + [ + "account added", + (storage) => ({ + ...storage, + accounts: [ + ...storage.accounts, + makeAccount({ accountId: "acct-3", email: "three@example.com", refreshToken: "rt-3" }), + ], + }), + ], + ["account removed", (storage) => ({ ...storage, accounts: storage.accounts.slice(0, 1) })], + [ + "refresh token rotated", + (storage) => + withAccount(storage, 0, (a) => ({ + ...a, + refreshToken: "rt-rotated", + accessToken: "at-rotated", + expiresAt: (a.expiresAt ?? 0) + 3_600_000, + tokenRotatedAt: 1_800_000_000_000, + })), + ], + [ + "access token replaced", + (storage) => withAccount(storage, 0, (a) => ({ ...a, accessToken: "at-fresh" })), + ], + [ + "expiry moved", + (storage) => + withAccount(storage, 0, (a) => ({ ...a, expiresAt: (a.expiresAt ?? 0) + 600_000 })), + ], + ["label set", (storage) => withAccount(storage, 0, (a) => ({ ...a, accountLabel: "Work" }))], + ["tags set", (storage) => withAccount(storage, 0, (a) => ({ ...a, accountTags: ["work"] }))], + ["note set", (storage) => withAccount(storage, 0, (a) => ({ ...a, accountNote: "primary" }))], + ["disabled", (storage) => withAccount(storage, 0, (a) => ({ ...a, enabled: false }))], + [ + "email changed", + (storage) => withAccount(storage, 0, (a) => ({ ...a, email: "renamed@example.com" })), + ], + [ + "accountUserId set", + (storage) => withAccount(storage, 0, (a) => ({ ...a, accountUserId: "member-9" })), + ], + [ + "accountIdSource changed", + (storage) => withAccount(storage, 0, (a) => ({ ...a, accountIdSource: "manual" })), + ], + ["plan type changed", (storage) => withAccount(storage, 0, (a) => ({ ...a, planType: "pro" }))], + [ + "oauth scope changed", + (storage) => withAccount(storage, 0, (a) => ({ ...a, oauthScope: "openid profile" })), + ], + ]; + + it.each(significantChanges)("snapshots a %s", async (_label, mutate) => { + const base = makeStorage(); + await saveAccounts(base); + const before = await fs.readFile(storagePath, "utf-8"); + + await saveAccounts(mutate(base)); + + expect(await readSnapshotContents()).toEqual([before]); + }); + + it("preserves the previous document rather than the incoming one", async () => { + const base = makeStorage(); + await saveAccounts(base); + + await saveAccounts( + withAccount(base, 0, (a) => ({ ...a, refreshToken: "rt-clobbered-by-a-bad-write" })), + ); + + const [snapshot] = await readSnapshotContents(); + const snapshotDoc = JSON.parse(snapshot) as AccountStorageV3; + expect(snapshotDoc.accounts[0].refreshToken).toBe("rt-1"); + expect((await readLiveStore()).accounts[0].refreshToken).toBe("rt-clobbered-by-a-bad-write"); + }); + + it("snapshots a store that no longer parses", async () => { + await fs.writeFile(storagePath, "{ this is not json", "utf-8"); + + await saveAccounts(makeStorage()); + + expect(await readSnapshotContents()).toEqual(["{ this is not json"]); + }); + + it("treats a storage schema version change as significant", () => { + const previous = JSON.stringify({ + version: 1, + accounts: [{ refreshToken: "rt-1", addedAt: 1, lastUsed: 2 }], + activeIndex: 0, + }); + + expect( + isSignificantStorageChange(previous, { + version: 3, + accounts: [{ refreshToken: "rt-1", addedAt: 1, lastUsed: 2 }], + activeIndex: 0, + }), + ).toBe(true); + }); + + it("ignores key order and undefined-valued keys in the incoming document", () => { + const previous = JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [{ addedAt: 1, lastUsed: 2, refreshToken: "rt-1" }], + }); + + expect( + isSignificantStorageChange(previous, { + version: 3, + accounts: [{ refreshToken: "rt-1", addedAt: 1, lastUsed: 2, email: undefined }], + activeIndex: 0, + }), + ).toBe(false); + }); +}); + +describe("credential snapshots: clearAccounts", () => { + it("snapshots the store before deleting it", async () => { + await saveAccounts(makeStorage()); + const before = await fs.readFile(storagePath, "utf-8"); + + await clearAccounts(); + + expect(await exists(storagePath)).toBe(false); + expect(await readSnapshotContents()).toEqual([before]); + }); + + it("writes nothing when there is no store to delete", async () => { + await clearAccounts(); + + expect(await listSnapshotNames()).toEqual([]); + }); +}); + +describe("credential snapshots: retention", () => { + async function seedForeignBackups(): Promise { + await fs.mkdir(backupsDir, { recursive: true }); + const foreignFiles = [ + join(backupsDir, "codex-pre-import-backup-20250101-000000000-aaaaaa.json"), + join(backupsDir, "codex-backup-20250101-000000000-bbbbbb.json"), + join(backupsDir, "oc-codex-multi-auth-accounts.json.migrated-to-keychain.2025-01-01T00-00-00-000Z"), + ]; + for (const file of foreignFiles) { + await fs.writeFile(file, "foreign", "utf-8"); + } + const foreignDir = join(backupsDir, "pre-global-migration-20250101-000000000"); + await fs.mkdir(foreignDir, { recursive: true }); + return [...foreignFiles, foreignDir]; + } + + it("keeps only the configured number of snapshots across repeated saves", async () => { + vi.stubEnv("CODEX_AUTH_CREDENTIAL_SNAPSHOTS_MAX_COUNT", "3"); + const foreign = await seedForeignBackups(); + + const base = makeStorage(); + await saveAccounts(base); + for (let generation = 1; generation <= 5; generation += 1) { + await saveAccounts( + withAccount(base, 0, (a) => ({ ...a, accountLabel: `generation-${generation}` })), + ); + } + + expect(await listSnapshotNames()).toHaveLength(3); + for (const path of foreign) { + expect(await exists(path), `${path} must survive pruning`).toBe(true); + } + }); + + it("deletes the oldest snapshots and never a foreign file", async () => { + const foreign = await seedForeignBackups(); + const snapshots = ["oldest", "older", "old", "newer", "newest"]; + for (const [index, marker] of snapshots.entries()) { + const path = join( + backupsDir, + `${CREDENTIAL_SNAPSHOT_PREFIX}-2025010${index + 1}-000000000-abcdef.json`, + ); + await fs.writeFile(path, marker, "utf-8"); + const stamp = new Date(1_700_000_000_000 + index * 60_000); + await fs.utimes(path, stamp, stamp); + } + + await pruneCredentialSnapshots(backupsDir, 2); + + expect((await readSnapshotContents()).sort()).toEqual(["newer", "newest"].sort()); + for (const path of foreign) { + expect(await exists(path), `${path} must survive pruning`).toBe(true); + } + }); + + it("keeps every snapshot when the max count is zero", async () => { + await fs.mkdir(backupsDir, { recursive: true }); + for (let index = 0; index < 4; index += 1) { + await fs.writeFile( + join(backupsDir, `${CREDENTIAL_SNAPSHOT_PREFIX}-2025010${index + 1}-000000000-abcdef.json`), + `snapshot-${index}`, + "utf-8", + ); + } + + await pruneCredentialSnapshots(backupsDir, 0); + + expect(await listSnapshotNames()).toHaveLength(4); + }); + + it("recognizes only its own filenames", () => { + expect(isCredentialSnapshotFileName(`${CREDENTIAL_SNAPSHOT_PREFIX}-20250101-000000000-ab.json`)) + .toBe(true); + expect(isCredentialSnapshotFileName("codex-pre-import-backup-20250101-000000000-ab.json")) + .toBe(false); + expect(isCredentialSnapshotFileName("codex-backup-20250101-000000000-ab.json")).toBe(false); + expect(isCredentialSnapshotFileName(`${CREDENTIAL_SNAPSHOT_PREFIX}.json`)).toBe(false); + }); +}); + +describe("credential snapshots: safety", () => { + it.skipIf(process.platform === "win32")( + "writes snapshots as 0600 inside a 0700 directory", + async () => { + const base = makeStorage(); + await saveAccounts(base); + await saveAccounts(withAccount(base, 0, (a) => ({ ...a, refreshToken: "rt-rotated" }))); + + const [name] = await listSnapshotNames(); + const fileStats = await fs.stat(join(backupsDir, name)); + const dirStats = await fs.stat(backupsDir); + expect(fileStats.mode & 0o777).toBe(0o600); + expect(dirStats.mode & 0o777).toBe(0o700); + }, + ); + + it("does not fail the save when the snapshot cannot be written", async () => { + const base = makeStorage(); + await saveAccounts(base); + await fs.writeFile(backupsDir, "not a directory", "utf-8"); + + await expect( + saveAccounts(withAccount(base, 0, (a) => ({ ...a, refreshToken: "rt-rotated" }))), + ).resolves.toBeUndefined(); + + expect((await readLiveStore()).accounts[0].refreshToken).toBe("rt-rotated"); + expect(await fs.readFile(backupsDir, "utf-8")).toBe("not a directory"); + }); + + it("writes nothing at all when the feature is disabled", async () => { + vi.stubEnv("CODEX_AUTH_CREDENTIAL_SNAPSHOTS", "0"); + + const base = makeStorage(); + await saveAccounts(base); + await saveAccounts(withAccount(base, 0, (a) => ({ ...a, refreshToken: "rt-rotated" }))); + await clearAccounts(); + + expect(await exists(backupsDir)).toBe(false); + }); + + it("propagates a clear refused by the test-home guard instead of reporting success", async () => { + // A path under the real home that does not exist and is not a storage + // location, so a regression in the guard still cannot unlink a real + // account store. The guard runs before any filesystem call, so nothing + // here is created either. + const escaped = resolve( + userInfo().homedir, + ".oc-codex-credential-snapshot-guard-probe", + "oc-codex-multi-auth-accounts.json", + ); + setStoragePathDirect(escaped); + + await expect(clearAccounts()).rejects.toMatchObject({ + code: TEST_HOME_ESCAPE_CODE, + }); + expect(await exists(escaped)).toBe(false); + }); +}); + +describe("credential snapshots: keychain opt-in", () => { + function createMockKeychain(): KeychainBackend & { failWrites: boolean } { + const store = new Map(); + const backend = { + failWrites: false, + async get(service: string, account: string) { + return store.get(`${service}::${account}`) ?? null; + }, + async set(service: string, account: string, secret: string) { + if (backend.failWrites) throw new Error("simulated keychain failure"); + store.set(`${service}::${account}`, secret); + }, + async delete(service: string, account: string) { + return store.delete(`${service}::${account}`); + }, + async isAvailable() { + return true; + }, + }; + return backend; + } + + afterEach(() => { + _resetBackendForTests(); + }); + + it("writes no snapshot when clearing a keychain-backed store", async () => { + await saveAccounts(makeStorage()); + const before = await fs.readFile(storagePath, "utf-8"); + vi.stubEnv("CODEX_KEYCHAIN", "1"); + _setBackendForTests(createMockKeychain()); + + await clearAccounts(); + + expect(await listSnapshotNames()).toEqual([]); + expect(await exists(storagePath)).toBe(false); + // The store really did hold a plaintext pool, so a snapshot here would + // have copied live tokens into backups/ rather than been a no-op. + expect(before).toContain("rt-1"); + }); + + it("writes no snapshot when a failed keychain write falls back to JSON", async () => { + const base = makeStorage(); + await saveAccounts(base); + vi.stubEnv("CODEX_KEYCHAIN", "1"); + const backend = createMockKeychain(); + backend.failWrites = true; + _setBackendForTests(backend); + + await saveAccounts(withAccount(base, 0, (a) => ({ ...a, refreshToken: "rt-rotated" }))); + + // The fallback wrote the pool to JSON, which is the path that snapshots. + expect((await readLiveStore()).accounts[0].refreshToken).toBe("rt-rotated"); + expect(await listSnapshotNames()).toEqual([]); + }); +}); + +describe("credential snapshots: token rotation semantics", () => { + it("keeps the superseded token for the refreshed account and live tokens for the rest", async () => { + const base = makeStorage(); + await saveAccounts(base); + + await saveAccounts( + withAccount(base, 0, (a) => ({ + ...a, + refreshToken: "rt-1-rotated", + accessToken: "at-1-rotated", + tokenRotatedAt: 1_800_000_000_000, + })), + ); + + const [snapshot] = await readSnapshotContents(); + const snapshotDoc = JSON.parse(snapshot) as AccountStorageV3; + // A refresh consumes one account's token, so that one account's + // snapshotted token is the superseded one... + expect(snapshotDoc.accounts[0].refreshToken).toBe("rt-1"); + // ...while every other account in the pool is snapshotted with the + // token that is still live on disk. That bounds the staleness of a + // snapshot to the accounts a single write actually rotated. + expect(snapshotDoc.accounts[1].refreshToken).toBe("rt-2"); + expect((await readLiveStore()).accounts[1].refreshToken).toBe("rt-2"); + }); +}); diff --git a/test/test-home-isolation.test.ts b/test/test-home-isolation.test.ts new file mode 100644 index 00000000..ec58263b --- /dev/null +++ b/test/test-home-isolation.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect } from "vitest"; +import { homedir, userInfo } from "node:os"; +import { isAbsolute, relative, resolve } from "node:path"; +import { LOG_DIR } from "../lib/logger.js"; +import { ACCOUNTS_FILE_NAME } from "../lib/constants.js"; +import { saveAccounts } from "../lib/storage/load-save.js"; +import { getConfigDir } from "../lib/storage/paths.js"; +import { + getStoragePath, + setStoragePath, + setStoragePathDirect, +} from "../lib/storage/state.js"; + +function realUserHome(): string { + return userInfo().homedir; +} + +function isUnder(baseDir: string, targetPath: string): boolean { + const rel = relative(resolve(baseDir), resolve(targetPath)); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +describe("test home isolation", () => { + it("redirects homedir() away from the real user home", () => { + expect(process.env.OC_CODEX_TEST_HOME).toBeTruthy(); + expect(homedir()).toBe(process.env.OC_CODEX_TEST_HOME); + expect(isUnder(realUserHome(), homedir())).toBe(false); + }); + + it("keeps every resolved account storage path out of the real user home", () => { + const real = realUserHome(); + + setStoragePath(null); + expect(isUnder(real, getStoragePath())).toBe(false); + + setStoragePath(process.cwd()); + expect(isUnder(real, getStoragePath())).toBe(false); + + setStoragePath(null); + expect(isUnder(real, getConfigDir())).toBe(false); + }); + + // LOG_DIR is captured at module scope from homedir(), so it only lands in + // the sandbox if the override beat the import. That is the property a + // setupFiles entry cannot provide and this whole mechanism exists for. + it("beats module-scope homedir() capture", () => { + expect(isUnder(realUserHome(), LOG_DIR)).toBe(false); + expect(isUnder(process.env.OC_CODEX_TEST_HOME as string, LOG_DIR)).toBe(true); + }); + + it("refuses to write account storage that escapes into the real home", async () => { + const escaped = resolve(realUserHome(), ".opencode", ACCOUNTS_FILE_NAME); + setStoragePathDirect(escaped); + try { + await expect( + saveAccounts({ version: 3, accounts: [], activeIndex: 0 }), + ).rejects.toMatchObject({ code: "TEST_HOME_ESCAPE" }); + } finally { + setStoragePathDirect(null); + } + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 5499527f..41c0fe84 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,9 +1,39 @@ import { defineConfig } from 'vitest/config'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** + * Per-run throwaway home. The suite writes real account storage, so without + * this a `npm test` resolves `~/.opencode/oc-codex-multi-auth-accounts.json` + * against the developer's actual home and overwrites live ChatGPT credentials + * with fixtures. + * + * This must be `test.env`, not a `setupFiles` entry: vitest applies `test.env` + * before the worker imports any test module, and `lib/config.ts`, + * `lib/accounts/recovery.ts` and `lib/logger.ts` capture `homedir()` at module + * scope, so anything later than import time is too late for them. + */ +const isolatedHome = + process.env.OC_CODEX_TEST_HOME ?? + mkdtempSync(join(tmpdir(), 'oc-codex-multi-auth-test-home-')); +process.env.OC_CODEX_TEST_HOME = isolatedHome; export default defineConfig({ test: { globals: true, environment: 'node', + env: { + HOME: isolatedHome, + USERPROFILE: isolatedHome, + OC_CODEX_TEST_HOME: isolatedHome, + }, + // Four suites import the real `index.ts`, and the first one scheduled pays + // the transform of a 4900-line entry plus its dependency graph: measured at + // 3.2s-6.7s on an idle machine, against a 5s default. Whichever suite loses + // that race times out under full-suite CPU contention, which is flakiness in + // the harness rather than in any assertion (a warm re-import costs ~400ms). + testTimeout: 15_000, include: ['test/**/*.test.ts'], exclude: [ 'node_modules/**',