From 6dab927e1b537c34d330874aa710aea2f1a5a111 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:26:00 +0200 Subject: [PATCH 1/3] fix(oauth): harden generated account ids against collisions --- src/oauth/store.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 287aa38df1..4613f9887b 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -198,7 +198,7 @@ interface LockSnapshot { bytes: string; dev: number; ino: number; mtimeMs: numbe export interface OAuthFileLockOptions { path: string; waitTimeoutMs?: number; staleAfterMs?: number; pollMinMs?: number; pollMaxMs?: number; sleep?: (ms: number) => Promise; now?: () => number; random?: () => number; beforeStaleUnlink?: () => void; beforeReleaseUnlink?: () => void; beforeFailedCreateUnlink?: () => void; writeMetadata?: (fd: number, bytes: string) => void } export interface OAuthFileLockGuard { readonly ownerId: string; release(): void } function errorCode(error: unknown): string | undefined { return error && typeof error === "object" && "code" in error ? String((error as { code?: unknown }).code) : undefined; } -function snapshot(path: string): LockSnapshot { const bytes = readFileSync(path, "utf8"); const s = statSync(path); return { bytes, dev:s.dev, ino:s.ino, mtimeMs:s.mtimeMs, size:s.size }; } +function snapshot(path: string): LockSnapshot { const bytes = readFileSync(path, "utf8"); const s = statSync(path); return { bytes, dev:s.dev, ino:s.ino,mtimeMs:s.mtimeMs,size:s.size }; } function sameSnapshot(a: LockSnapshot,b: LockSnapshot): boolean { return a.bytes===b.bytes&&a.dev===b.dev&&a.ino===b.ino&&a.mtimeMs===b.mtimeMs&&a.size===b.size; } function sameFd(a: LockSnapshot,b: ReturnType): boolean { return a.dev===b.dev&&a.ino===b.ino&&a.mtimeMs===b.mtimeMs&&a.size===b.size; } export function createOAuthFileLock(options: OAuthFileLockOptions): { acquire(): Promise } { @@ -286,15 +286,18 @@ function normalizeCredential(cred: unknown): OAuthCredentials | null { } /** - * Stable short account id. MUST be deterministic for a given credential: legacy - * single-credential stores are re-normalized on EVERY load without being persisted, + * Stable collision-resistant account id. MUST be deterministic for a given credential: + * legacy single-credential stores are re-normalized on EVERY load without being persisted, * so a time-salted id would differ between two loads (getAccountSet vs * getAccountCredential), surfacing as a spurious OAuthLoginRequiredError and making * refresh persists silently miss the account (rotated refresh token lost). + * + * Keep 128 bits of SHA-256 rather than the historical 32-bit prefix. Existing persisted + * account ids are read as-is; only newly-derived ids and legacy normalization use this width. */ function newAccountId(cred: OAuthCredentials): string { const identity = cred.accountId ?? cred.email ?? cred.refresh; - return createHash("sha256").update(identity).digest("hex").slice(0, 8); + return createHash("sha256").update(identity).digest("hex").slice(0, 32); } function normalizeAccount(value: unknown): ProviderAccount | null { From c9ad90a15125d762c663f318cb8f6a2a1b5e6f64 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:26:20 +0200 Subject: [PATCH 2/3] test(oauth): cover account id collision hardening --- tests/oauth-account-id-collision.test.ts | 100 +++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/oauth-account-id-collision.test.ts diff --git a/tests/oauth-account-id-collision.test.ts b/tests/oauth-account-id-collision.test.ts new file mode 100644 index 0000000000..3ff6e31cc4 --- /dev/null +++ b/tests/oauth-account-id-collision.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { + resetHardenedStateForTests, + setIcaclsRunnerForTests, +} from "../src/lib/windows-secret-acl"; +import { + getAccountSet, + getCredential, + saveCredential, +} from "../src/oauth/store"; + +const TEST_DIR = join(import.meta.dir, ".tmp-oauth-account-id-collision-test"); +let previousOpencodexHome: string | undefined; + +const COLLIDING_ACCOUNT_A = "account-collision-16138"; +const COLLIDING_ACCOUNT_B = "account-collision-28806"; + +describe("OAuth account id collision hardening", () => { + beforeEach(() => { + previousOpencodexHome = process.env.OPENCODEX_HOME; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + resetHardenedStateForTests(); + setIcaclsRunnerForTests(() => ({ + success: true, + exitCode: 0, + timedOut: false, + stdout: "", + })); + }); + + afterEach(() => { + setIcaclsRunnerForTests(null); + resetHardenedStateForTests(); + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + }); + + test("distinct identities that collide on the historical 32-bit prefix get distinct slots", async () => { + // sha256(account-collision-16138) and sha256(account-collision-28806) both + // start with da1e26d2. The historical 8-hex id therefore aliased these + // two accounts and made active-account lookup return the wrong credential. + await saveCredential("anthropic", { + access: "access-a", + refresh: "refresh-a", + expires: Date.now() + 3600_000, + accountId: COLLIDING_ACCOUNT_A, + }); + await saveCredential("anthropic", { + access: "access-b", + refresh: "refresh-b", + expires: Date.now() + 3600_000, + accountId: COLLIDING_ACCOUNT_B, + }); + + const set = getAccountSet("anthropic"); + expect(set).not.toBeNull(); + expect(set!.accounts).toHaveLength(2); + expect(new Set(set!.accounts.map(account => account.id)).size).toBe(2); + expect(set!.accounts.every(account => account.id.length === 32)).toBe(true); + expect(getCredential("anthropic")?.accountId).toBe(COLLIDING_ACCOUNT_B); + expect(getCredential("anthropic")?.access).toBe("access-b"); + }); + + test("existing persisted 32-bit account ids remain valid and are not rewritten", async () => { + const authPath = join(TEST_DIR, "auth.json"); + writeFileSync(authPath, JSON.stringify({ + anthropic: { + activeAccountId: "deadbeef", + accounts: [{ + id: "deadbeef", + credential: { + access: "old-access", + refresh: "old-refresh", + expires: Date.now() + 3600_000, + accountId: "existing-account", + }, + }], + }, + })); + + expect(getAccountSet("anthropic")?.activeAccountId).toBe("deadbeef"); + + await saveCredential("anthropic", { + access: "rotated-access", + refresh: "rotated-refresh", + expires: Date.now() + 7200_000, + accountId: "existing-account", + }); + + const set = getAccountSet("anthropic"); + expect(set?.activeAccountId).toBe("deadbeef"); + expect(set?.accounts[0]?.id).toBe("deadbeef"); + expect(getCredential("anthropic")?.access).toBe("rotated-access"); + }); +}); From 76ad058cf9b409a6ff9bfe5ad778175f477d32d6 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:28:25 +0200 Subject: [PATCH 3/3] chore(oauth): keep collision hardening diff focused --- src/oauth/store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 4613f9887b..478f9c73fd 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -198,7 +198,7 @@ interface LockSnapshot { bytes: string; dev: number; ino: number; mtimeMs: numbe export interface OAuthFileLockOptions { path: string; waitTimeoutMs?: number; staleAfterMs?: number; pollMinMs?: number; pollMaxMs?: number; sleep?: (ms: number) => Promise; now?: () => number; random?: () => number; beforeStaleUnlink?: () => void; beforeReleaseUnlink?: () => void; beforeFailedCreateUnlink?: () => void; writeMetadata?: (fd: number, bytes: string) => void } export interface OAuthFileLockGuard { readonly ownerId: string; release(): void } function errorCode(error: unknown): string | undefined { return error && typeof error === "object" && "code" in error ? String((error as { code?: unknown }).code) : undefined; } -function snapshot(path: string): LockSnapshot { const bytes = readFileSync(path, "utf8"); const s = statSync(path); return { bytes, dev:s.dev, ino:s.ino,mtimeMs:s.mtimeMs,size:s.size }; } +function snapshot(path: string): LockSnapshot { const bytes = readFileSync(path, "utf8"); const s = statSync(path); return { bytes, dev:s.dev, ino:s.ino, mtimeMs:s.mtimeMs, size:s.size }; } function sameSnapshot(a: LockSnapshot,b: LockSnapshot): boolean { return a.bytes===b.bytes&&a.dev===b.dev&&a.ino===b.ino&&a.mtimeMs===b.mtimeMs&&a.size===b.size; } function sameFd(a: LockSnapshot,b: ReturnType): boolean { return a.dev===b.dev&&a.ino===b.ino&&a.mtimeMs===b.mtimeMs&&a.size===b.size; } export function createOAuthFileLock(options: OAuthFileLockOptions): { acquire(): Promise } {