diff --git a/lib/storage.ts b/lib/storage.ts index 0509925d..983952f0 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -381,6 +381,20 @@ function getAccountsBackupRecoveryCandidates(path: string): string[] { return candidates; } +function normalizeStorageComparisonPath(path: string): string { + const resolved = resolvePath(path); + if (process.platform !== "win32") { + return resolved; + } + return resolved.replaceAll("\\", "/").toLowerCase(); +} + +function areEquivalentStoragePaths(left: string, right: string): boolean { + return ( + normalizeStorageComparisonPath(left) === normalizeStorageComparisonPath(right) + ); +} + async function getAccountsBackupRecoveryCandidatesWithDiscovery( path: string, ): Promise { @@ -813,8 +827,13 @@ function latestValidSnapshot( snapshots: BackupSnapshotMetadata[], ): BackupSnapshotMetadata | undefined { return snapshots - .filter((snapshot) => snapshot.valid) - .sort((left, right) => (right.mtimeMs ?? 0) - (left.mtimeMs ?? 0))[0]; + .map((snapshot, index) => ({ snapshot, index })) + .filter(({ snapshot }) => snapshot.valid) + .sort( + (left, right) => + (right.snapshot.mtimeMs ?? 0) - (left.snapshot.mtimeMs ?? 0) || + left.index - right.index, + )[0]?.snapshot; } function buildMetadataSection( @@ -1761,8 +1780,9 @@ async function loadAccountsFromJournal( async function loadAccountsInternal( persistMigration: ((storage: AccountStorageV3) => Promise) | null, + storagePath = getStoragePath(), ): Promise { - const path = getStoragePath(); + const path = storagePath; const resetMarkerPath = getIntentionalResetMarkerPath(path); await cleanupStaleRotatingBackupArtifacts(path); const migratedLegacyStorage = persistMigration @@ -1926,8 +1946,11 @@ async function loadAccountsInternal( } } -async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { - const path = getStoragePath(); +async function saveAccountsUnlocked( + storage: AccountStorageV3, + storagePath = getStoragePath(), +): Promise { + const path = storagePath; const resetMarkerPath = getIntentionalResetMarkerPath(path); const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; const tempPath = `${path}.${uniqueSuffix}.tmp`; @@ -2078,18 +2101,25 @@ export async function withAccountStorageTransaction( return withStorageLock(async () => { const storagePath = getStoragePath(); const state = { - snapshot: await loadAccountsInternal(saveAccountsUnlocked), - storagePath, + snapshot: await loadAccountsInternal( + (storage) => saveAccountsUnlocked(storage, storagePath), + storagePath, + ), active: true, + storagePath, }; const current = state.snapshot; const persist = async (storage: AccountStorageV3): Promise => { - await saveAccountsUnlocked(storage); + await saveAccountsUnlocked(storage, storagePath); state.snapshot = storage; }; - return transactionSnapshotContext.run(state, () => - handler(current, persist), - ); + return transactionSnapshotContext.run(state, async () => { + try { + return await handler(current, persist); + } finally { + state.active = false; + } + }); }); } @@ -2105,9 +2135,12 @@ export async function withAccountAndFlaggedStorageTransaction( return withStorageLock(async () => { const storagePath = getStoragePath(); const state = { - snapshot: await loadAccountsInternal(saveAccountsUnlocked), - storagePath, + snapshot: await loadAccountsInternal( + (storage) => saveAccountsUnlocked(storage, storagePath), + storagePath, + ), active: true, + storagePath, }; const current = state.snapshot; const persist = async ( @@ -2116,13 +2149,13 @@ export async function withAccountAndFlaggedStorageTransaction( ): Promise => { const previousAccounts = cloneAccountStorageForPersistence(state.snapshot); const nextAccounts = cloneAccountStorageForPersistence(accountStorage); - await saveAccountsUnlocked(nextAccounts); + await saveAccountsUnlocked(nextAccounts, storagePath); try { await saveFlaggedAccountsUnlocked(flaggedStorage); state.snapshot = nextAccounts; } catch (error) { try { - await saveAccountsUnlocked(previousAccounts); + await saveAccountsUnlocked(previousAccounts, storagePath); state.snapshot = previousAccounts; } catch (rollbackError) { const combinedError = new AggregateError( @@ -2141,9 +2174,13 @@ export async function withAccountAndFlaggedStorageTransaction( throw error; } }; - return transactionSnapshotContext.run(state, () => - handler(current, persist), - ); + return transactionSnapshotContext.run(state, async () => { + try { + return await handler(current, persist); + } finally { + state.active = false; + } + }); }); } @@ -2477,22 +2514,27 @@ export async function exportAccounts( beforeCommit?: (resolvedPath: string) => Promise | void, ): Promise { const resolvedPath = resolvePath(filePath); - const currentStoragePath = getStoragePath(); + const activeStoragePath = getStoragePath(); if (!force && existsSync(resolvedPath)) { throw new Error(`File already exists: ${resolvedPath}`); } const transactionState = transactionSnapshotContext.getStore(); - const storage = + if ( transactionState?.active && - transactionState.storagePath === currentStoragePath - ? transactionState.snapshot - : transactionState?.active - ? await loadAccountsInternal(saveAccountsUnlocked) - : await withAccountStorageTransaction((current) => - Promise.resolve(current), - ); + !areEquivalentStoragePaths(transactionState.storagePath, activeStoragePath) + ) { + throw new Error( + `Export blocked by storage path mismatch: transaction path is ` + + `${transactionState.storagePath}, active path is ${activeStoragePath}`, + ); + } + const storage = transactionState?.active + ? transactionState.snapshot + : await withAccountStorageTransaction((current) => + Promise.resolve(current), + ); if (!storage || storage.accounts.length === 0) { throw new Error("No accounts to export"); } diff --git a/test/storage.test.ts b/test/storage.test.ts index ccca65c0..44c5d563 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -13,6 +13,7 @@ import { findMatchingAccountIndex, formatStorageErrorHint, getFlaggedAccountsPath, + getBackupMetadata, getStoragePath, importAccounts, loadAccounts, @@ -28,6 +29,10 @@ import { withAccountStorageTransaction, } from "../lib/storage.js"; +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + // Mocking the behavior we're about to implement for TDD // Since the functions aren't in lib/storage.ts yet, we'll need to mock them or // accept that this test won't even compile/run until we add them. @@ -956,6 +961,389 @@ describe("storage", () => { ); }); + it("fails fast when export is called from an active transaction after the storage path changes", async () => { + const secondaryStoragePath = join(testWorkDir, "accounts-secondary.json"); + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-primary", + refreshToken: "refresh-primary", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + setStoragePathDirect(secondaryStoragePath); + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-secondary", + refreshToken: "refresh-secondary", + addedAt: 3, + lastUsed: 4, + }, + ], + }); + setStoragePathDirect(testStoragePath); + + await expect( + withAccountStorageTransaction(async () => { + setStoragePathDirect(secondaryStoragePath); + await exportAccounts(exportPath); + }), + ).rejects.toThrow( + new RegExp( + `storage path mismatch: transaction path is ${escapeRegExp( + testStoragePath, + )}, active path is ${escapeRegExp(secondaryStoragePath)}`, + ), + ); + expect(existsSync(exportPath)).toBe(false); + }); + + it("allows equivalent Windows-style storage paths during export from an active transaction", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { + value: "win32", + configurable: true, + }); + + try { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-primary", + refreshToken: "refresh-primary", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + + await expect( + withAccountStorageTransaction(async () => { + setStoragePathDirect(testStoragePath.toUpperCase()); + await exportAccounts(exportPath); + }), + ).resolves.toBeUndefined(); + + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts[0].accountId).toBe("acct-primary"); + } finally { + Object.defineProperty(process, "platform", { + value: originalPlatform, + configurable: true, + }); + } + }); + + it("reloads fresh storage after a transaction handler throws", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-before-throw", + refreshToken: "refresh-before-throw", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + + await expect( + withAccountStorageTransaction(async (current, persist) => { + await persist({ + ...(current ?? { version: 3, activeIndex: 0, accounts: [] }), + accounts: [ + { + accountId: "acct-thrown", + refreshToken: "refresh-thrown", + addedAt: 3, + lastUsed: 4, + }, + ], + }); + throw new Error("boom"); + }), + ).rejects.toThrow("boom"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-after-throw", + refreshToken: "refresh-after-throw", + addedAt: 5, + lastUsed: 6, + }, + ], + }); + + await exportAccounts(exportPath); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts[0].accountId).toBe("acct-after-throw"); + }); + + it("persists transaction updates to the original storage path after path drift", async () => { + const secondaryExportPath = join(testWorkDir, "secondary-export.json"); + const secondaryStoragePath = join(testWorkDir, "secondary-storage.json"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-primary-before-drift", + refreshToken: "refresh-primary-before-drift", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + setStoragePathDirect(secondaryStoragePath); + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-secondary-before-drift", + refreshToken: "refresh-secondary-before-drift", + addedAt: 3, + lastUsed: 4, + }, + ], + }); + setStoragePathDirect(testStoragePath); + + await withAccountStorageTransaction(async (current, persist) => { + setStoragePathDirect(secondaryStoragePath); + await persist({ + ...(current ?? { version: 3, activeIndex: 0, accounts: [] }), + accounts: [ + { + accountId: "acct-primary-after-drift", + refreshToken: "refresh-primary-after-drift", + addedAt: 5, + lastUsed: 6, + }, + ], + }); + }); + + setStoragePathDirect(testStoragePath); + await exportAccounts(exportPath); + const primaryExport = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(primaryExport.accounts[0].accountId).toBe("acct-primary-after-drift"); + + setStoragePathDirect(secondaryStoragePath); + await exportAccounts(secondaryExportPath); + const secondaryExport = JSON.parse( + await fs.readFile(secondaryExportPath, "utf-8"), + ); + expect(secondaryExport.accounts[0].accountId).toBe( + "acct-secondary-before-drift", + ); + }); + + it("reloads fresh storage after a transaction handler returns successfully", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-before-success", + refreshToken: "refresh-before-success", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + + await withAccountStorageTransaction(async (current, persist) => { + await persist({ + ...(current ?? { version: 3, activeIndex: 0, accounts: [] }), + accounts: [ + { + accountId: "acct-success", + refreshToken: "refresh-success", + addedAt: 3, + lastUsed: 4, + }, + ], + }); + }); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-after-success", + refreshToken: "refresh-after-success", + addedAt: 5, + lastUsed: 6, + }, + ], + }); + + await exportAccounts(exportPath); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts[0].accountId).toBe("acct-after-success"); + }); + + it("reloads fresh storage after a combined transaction handler throws", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-before-combined-throw", + refreshToken: "refresh-before-combined-throw", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + await saveFlaggedAccounts({ version: 1, accounts: [] }); + + await expect( + withAccountAndFlaggedStorageTransaction(async (current, persist) => { + await persist( + { + ...(current ?? { version: 3, activeIndex: 0, accounts: [] }), + accounts: [ + { + accountId: "acct-combined-thrown", + refreshToken: "refresh-combined-thrown", + addedAt: 3, + lastUsed: 4, + }, + ], + }, + { version: 1, accounts: [] }, + ); + throw new Error("combined boom"); + }), + ).rejects.toThrow("combined boom"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-after-combined-throw", + refreshToken: "refresh-after-combined-throw", + addedAt: 5, + lastUsed: 6, + }, + ], + }); + + await exportAccounts(exportPath); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts[0].accountId).toBe("acct-after-combined-throw"); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + accountId: "acct-flagged-after-combined-throw", + email: "flagged-after-combined-throw@example.com", + refreshToken: "refresh-flagged-after-combined-throw", + addedAt: 7, + lastUsed: 8, + flaggedAt: 9, + }, + ], + }); + const loadedFlagged = await loadFlaggedAccounts(); + expect(loadedFlagged.accounts[0]).toEqual( + expect.objectContaining({ + accountId: "acct-flagged-after-combined-throw", + refreshToken: "refresh-flagged-after-combined-throw", + }), + ); + }); + + it("reloads fresh storage after a combined transaction handler returns successfully", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-before-combined-success", + refreshToken: "refresh-before-combined-success", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + await saveFlaggedAccounts({ version: 1, accounts: [] }); + + await withAccountAndFlaggedStorageTransaction(async (current, persist) => { + await persist( + { + ...(current ?? { version: 3, activeIndex: 0, accounts: [] }), + accounts: [ + { + accountId: "acct-combined-success", + refreshToken: "refresh-combined-success", + addedAt: 3, + lastUsed: 4, + }, + ], + }, + { version: 1, accounts: [] }, + ); + }); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-after-combined-success", + refreshToken: "refresh-after-combined-success", + addedAt: 5, + lastUsed: 6, + }, + ], + }); + + await exportAccounts(exportPath); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts[0].accountId).toBe( + "acct-after-combined-success", + ); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + accountId: "acct-flagged-after-combined-success", + email: "flagged-after-combined-success@example.com", + refreshToken: "refresh-flagged-after-combined-success", + addedAt: 7, + lastUsed: 8, + flaggedAt: 9, + }, + ], + }); + const loadedFlagged = await loadFlaggedAccounts(); + expect(loadedFlagged.accounts[0]).toEqual( + expect.objectContaining({ + accountId: "acct-flagged-after-combined-success", + refreshToken: "refresh-flagged-after-combined-success", + }), + ); + }); + it("should fail import when file does not exist", async () => { const { importAccounts } = await import("../lib/storage.js"); const nonexistentPath = join(testWorkDir, "nonexistent-file.json"); @@ -3069,6 +3457,40 @@ describe("storage", () => { expect(historicalBackup.accounts?.[0]?.refreshToken).toBe("token-2"); expect(oldestBackup.accounts?.[0]?.refreshToken).toBe("token-1"); }); + + it("prefers earlier snapshot priority when valid backups share the same mtime", async () => { + const storagePath = getStoragePath(); + const snapshotTime = new Date("2026-03-20T00:00:00.000Z"); + const writeValidSnapshot = async (path: string, refreshToken: string) => { + await fs.writeFile( + path, + JSON.stringify( + { + version: 3, + activeIndex: 0, + accounts: [ + { + refreshToken, + addedAt: 1, + lastUsed: 1, + }, + ], + }, + null, + 2, + ), + "utf-8", + ); + await fs.utimes(path, snapshotTime, snapshotTime); + }; + + await writeValidSnapshot(storagePath, "primary-token"); + await writeValidSnapshot(`${storagePath}.bak`, "backup-token"); + await writeValidSnapshot(`${storagePath}.bak.1`, "backup-history-token"); + + const metadata = await getBackupMetadata(); + expect(metadata.accounts.latestValidPath).toBe(storagePath); + }); }); describe("clearAccounts edge cases", () => {