diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 04ac676c9a..a7ea6eaa2d 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -437,6 +437,14 @@ function backupLegacyOnce(): void { try { copyFileSync(path, backup); try { chmodSync(backup, 0o600); } catch { /* best-effort */ } + try { + // Register only the copy we just created. An unowned home still needs downgrade recovery. + if (!recordOwnedConfigPath(getConfigDir(), backup)) { + console.warn("[oauth] Recovery backup created, but uninstall ownership registration failed."); + } + } catch { + console.warn("[oauth] Recovery backup created, but uninstall ownership registration failed."); + } } catch { /* best-effort */ } } diff --git a/structure/config.md b/structure/config.md index d00b36b06b..dcef861088 100644 --- a/structure/config.md +++ b/structure/config.md @@ -244,6 +244,17 @@ and removes only normalized manifest entries. Manifest-owned directory links are traversing their targets. Unknown files remain in place and make the command report a partial uninstall with their exact paths. +The newly created OAuth downgrade copy is registered after copying, so owned uninstall +includes it. Invalid-config recovery copies are deliberately NOT registered: their names carry +a timestamp, so one entry per invalid load would grow the uninstall manifest without bound, and +the manifest stops validating past its path ceiling. A manifest that stops validating makes +uninstall refuse outright, which would leave credentials on disk. Sweeping those copies by name +pattern at removal time is the shape that fits; it is not in this change. Registration is best-effort: an intentionally +unowned legacy home or a metadata-write failure must not suppress the recovery copy. Existing +OAuth downgrade copies are neither rewritten nor retroactively claimed. Both a `false` registration +result and a thrown registration error emit the same fixed warning without error details. Unregistered copies +remain subject to the existing partial/refused uninstall result. + Legacy nonempty config directories are deliberately not retroactively claimed. If either ownership file is missing, malformed, or bound to another root, uninstall refuses config deletion and reports the residual directory for manual review; there is no recursive-delete fallback. diff --git a/structure/overview.md b/structure/overview.md index 0151115adc..b81b030f0c 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -47,7 +47,8 @@ preserves saved user model selections and historical usage. See the bounded installed service resolve it the same way (`src/config.ts`). Ownership inside that root is tracked by the uninstall manifest in `src/lib/config-ownership.ts`, which starts from a declared path list and grows as opencodex claims further paths at runtime — so the manifest, not this table, is what -bounds uninstall. This table groups the state by purpose; it is not an exhaustive file list, and +bounds uninstall. Newly generated recovery backups follow the [backup ownership contract](config.md#restore) +without suppressing recovery when registration is unavailable. This table groups state by purpose; it is not an exhaustive file list, and derived files such as `auth.json.pre-multiauth` are covered by the group they belong to. `$CODEX_HOME` is a separate root with a separate owner, and opencodex writes there too: removing the diff --git a/structure/providers/xai-grok.md b/structure/providers/xai-grok.md index dc982c5639..b1cf71d509 100644 --- a/structure/providers/xai-grok.md +++ b/structure/providers/xai-grok.md @@ -38,6 +38,8 @@ The shared Responses path follows the [bounded multipart recovery contract](../s `auth.json` load-merge-persist (`src/oauth/store.ts`); generation-guarded persist (`expectedGeneration` → superseded adoption), conditional `needsReauth`, bounded jittered retry for transient token-endpoint failures. + Newly created legacy-store recovery copies follow the [backup ownership contract](../config.md#restore); + an ownership-registration failure (a `false` return or thrown error) warns without discarding downgrade recovery. - **Reactive 401 replay:** both the adapter recovery loop and native Responses passthrough branch force-refresh once (singleflight, generation-checked) and replay OAuth-backed xAI requests exactly once with a re-resolved transport; API-key/BYOK paths are excluded diff --git a/structure/runtime.md b/structure/runtime.md index bd9ebbd561..5b75cf67ba 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -1,5 +1,8 @@ # Runtime +OAuth recovery-copy registration warns on both refusal and exceptions while preserving the copy; +see the [backup ownership contract](config.md#restore). + Responses admission and finalization are composed through the [core module ownership](transports/responses.md#core-module-ownership). This surface retains its existing behavior. diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index ca80373a8e..9f8521ae79 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -1,5 +1,8 @@ # Transport Inventory +The shared OAuth store warns on recovery-copy ownership refusal or exceptions without losing +the copy; see [backup ownership](../config.md#restore). + The existing Responses transport is divided by responsibility in the [core module ownership](responses.md#core-module-ownership). This surface retains its existing behavior. diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 02d8f8024f..32fcf293f8 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -4,6 +4,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import * as atomicWrite from "../../src/config/atomic-write"; import * as oauthStore from "../../src/oauth/store"; +import * as configOwnership from "../../src/lib/config-ownership"; import { flushConfigDirHardeningForTests } from "../../src/config/paths"; import { resetHardenedStateForTests, @@ -36,6 +37,7 @@ import { } from "../../src/oauth/store"; import type { OAuthCredentials } from "../../src/oauth/types"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { recordOwnedConfigPath, removeOwnedConfigState } from "../../src/lib/config-ownership"; const TEST_DIR = join(import.meta.dir, ".tmp-oauth-store-multi-test"); let previousOpencodexHome: string | undefined; @@ -152,6 +154,65 @@ describe("multi-account auth store", () => { expect(existsSync(`${authPath}.pre-multiauth`)).toBe(true); }); + test("uninstall removes a legacy recovery backup from an owned home", async () => { + const dir = join(TEST_DIR, "owned"); + const path = join(dir, "auth.json"); + process.env.OPENCODEX_HOME = dir; + try { + expect(recordOwnedConfigPath(dir, path)).toBe(true); + const original = JSON.stringify({ xai: cred({ email: "old@example.test" }) }); + writeFileSync(path, original); + await saveCredential("xai", cred({ email: "old@example.test", access: "new-access" })); + expect(readFileSync(`${path}.pre-multiauth`, "utf8")).toBe(original); + await flushConfigDirHardeningForTests(); + expect(removeOwnedConfigState(dir).status).toBe("removed"); + expect(existsSync(`${path}.pre-multiauth`)).toBe(false); + } finally { + process.env.OPENCODEX_HOME = TEST_DIR; + } + }); + + test.each(["false", "throw"] as const)("recovery survives registration %s and warns without exposing credentials", async (failure) => { + const path = join(TEST_DIR, "auth.json"); + const backup = `${path}.pre-multiauth`; + const original = JSON.stringify({ xai: cred({ email: "old@example.test" }) }); + writeFileSync(path, original); + const register = configOwnership.recordOwnedConfigPath; + const registration = spyOn(configOwnership, "recordOwnedConfigPath").mockImplementation((dir, candidate) => { + if (candidate !== backup) return register(dir, candidate); + if (failure === "throw") throw new Error("private ownership failure fixture"); + return false; + }); + const warning = spyOn(console, "warn").mockImplementation(() => {}); + try { + await saveCredential("xai", cred({ email: "old@example.test", access: "new-access" })); + expect(registration).toHaveBeenCalledWith(TEST_DIR, backup); + expect(readFileSync(backup, "utf8")).toBe(original); + expect(getCredential("xai")?.access).toBe("new-access"); + expect(warning.mock.calls).toEqual([["[oauth] Recovery backup created, but uninstall ownership registration failed."]]); + } finally { + warning.mockRestore(); + registration.mockRestore(); + } + }); + + test("migration leaves a pre-existing unregistered backup unchanged and unclaimed", async () => { + const dir = join(TEST_DIR, "existing-backup"); + const path = join(dir, "auth.json"); + process.env.OPENCODEX_HOME = dir; + try { + expect(recordOwnedConfigPath(dir, path)).toBe(true); + writeFileSync(path, JSON.stringify({ xai: cred({ email: "old@example.test" }) })); + writeFileSync(`${path}.pre-multiauth`, "prior-recovery-fixture"); + await saveCredential("xai", cred({ email: "old@example.test" })); + await flushConfigDirHardeningForTests(); + expect(removeOwnedConfigState(dir).status).toBe("partial"); + expect(readFileSync(`${path}.pre-multiauth`, "utf8")).toBe("prior-recovery-fixture"); + } finally { + process.env.OPENCODEX_HOME = TEST_DIR; + } + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and