From b10d5f7df7f7e0a87b7debe79199fc290ad9bee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Mon, 18 May 2026 23:13:15 +0800 Subject: [PATCH] Prevent secure-storage cache keys from overriding env-pinned OAuth files The sync token reader memoized raw path strings and reserved '__secure-storage__' as the secure-storage discriminator. That allowed a real OPENAI_CODEX_OAUTH_FILE value with the same relative path to bypass the file branch and return secure-storage data instead. Use structured cache keys so file-backed lookups always memoize under a prefixed path while secure storage keeps a dedicated discriminator. Add a regression that seeds different secure-storage and file tokens for '__secure-storage__' and proves both sync and async getters honor the env-pinned file. Constraint: OPENAI_CODEX_OAUTH_FILE must remain authoritative even for relative paths that match internal sentinel names Rejected: Keep the raw sentinel and special-case path equality | still leaves memoization coupled to a valid user-controlled filename Directive: Preserve the file-vs-secure-storage branch split whenever sync cache keys change so env-pinned file authority stays collision-proof Confidence: high Scope-risk: narrow Tested: bun test src/services/openaiAuth/storage.test.ts --test-name-pattern "prefers env-pinned file authority when OPENAI_CODEX_OAUTH_FILE matches the secure-storage sentinel" Tested: bun test src/services/openaiAuth/storage.test.ts src/server/__tests__/haha-openai-oauth-service.test.ts Tested: bun test src/services/openaiAuth/storage.test.ts src/server/__tests__/haha-openai-oauth-service.test.ts src/server/__tests__/haha-openai-oauth-api.test.ts Tested: git diff --check --- src/services/openaiAuth/storage.test.ts | 36 +++++++++++++++++++++++++ src/services/openaiAuth/storage.ts | 12 ++++++--- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/services/openaiAuth/storage.test.ts b/src/services/openaiAuth/storage.test.ts index a03ee2e3..1c732d57 100644 --- a/src/services/openaiAuth/storage.test.ts +++ b/src/services/openaiAuth/storage.test.ts @@ -20,6 +20,7 @@ describe('OpenAI OAuth desktop token file storage', () => { let originalConfigDir: string | undefined let originalHome: string | undefined let originalUserProfile: string | undefined + let originalCwd: string beforeEach(async () => { tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'openai-oauth-storage-')) @@ -28,6 +29,7 @@ describe('OpenAI OAuth desktop token file storage', () => { originalConfigDir = process.env.CLAUDE_CONFIG_DIR originalHome = process.env.HOME originalUserProfile = process.env.USERPROFILE + originalCwd = process.cwd() process.env.CLAUDE_CONFIG_DIR = tmpDir process.env.HOME = tmpDir process.env.USERPROFILE = tmpDir @@ -57,6 +59,7 @@ describe('OpenAI OAuth desktop token file storage', () => { } else { process.env.USERPROFILE = originalUserProfile } + process.chdir(originalCwd) clearOpenAIOAuthTokenCache() await fsp.rm(tmpDir, { recursive: true, force: true }) }) @@ -230,6 +233,39 @@ describe('OpenAI OAuth desktop token file storage', () => { expect(getOpenAIOAuthTokens()?.accessToken).toBe('access-b') }) + test('prefers env-pinned file authority when OPENAI_CODEX_OAUTH_FILE matches the secure-storage sentinel', async () => { + const sentinelPath = '__secure-storage__' + seedSecureStorage({ + accessToken: 'secure-access', + refreshToken: 'secure-refresh', + expiresAt: 4_100_000_000_000, + }) + + process.chdir(tmpDir) + process.env.OPENAI_CODEX_OAUTH_FILE = sentinelPath + await fsp.writeFile( + path.join(tmpDir, sentinelPath), + JSON.stringify({ + accessToken: 'file-access', + refreshToken: 'file-refresh', + expiresAt: 4_100_000_000_123, + }), + 'utf-8', + ) + clearOpenAIOAuthTokenCache() + + expect(getOpenAIOAuthTokens()).toMatchObject({ + accessToken: 'file-access', + refreshToken: 'file-refresh', + expiresAt: 4_100_000_000_123, + }) + await expect(getOpenAIOAuthTokensAsync()).resolves.toMatchObject({ + accessToken: 'file-access', + refreshToken: 'file-refresh', + expiresAt: 4_100_000_000_123, + }) + }) + test('cleans up tmp file if desktop token rename fails', async () => { const renameSyncSpy = spyOn(fs, 'renameSync').mockImplementation(() => { const error = new Error('rename failed') as NodeJS.ErrnoException diff --git a/src/services/openaiAuth/storage.ts b/src/services/openaiAuth/storage.ts index 0c4eff51..db2d9a3f 100644 --- a/src/services/openaiAuth/storage.ts +++ b/src/services/openaiAuth/storage.ts @@ -13,7 +13,8 @@ type SecureStorageShape = Record & { openaiCodexOauth?: OpenAIOAuthTokens } -const SECURE_STORAGE_CACHE_KEY = '__secure-storage__' +const SECURE_STORAGE_CACHE_KEY = 'secure-storage' +const FILE_CACHE_KEY_PREFIX = 'file:' function getDesktopTokenFilePath(): string | null { const filePath = process.env[OPENAI_CODEX_OAUTH_FILE_ENV_KEY]?.trim() @@ -129,8 +130,10 @@ export function saveOpenAIOAuthTokens(tokens: OpenAIOAuthTokens): { const getOpenAIOAuthTokensCached = memoize( (cacheKey: string): OpenAIOAuthTokens | null => { - if (cacheKey !== SECURE_STORAGE_CACHE_KEY) { - return readDesktopTokenFileSync(cacheKey) + if (cacheKey.startsWith(FILE_CACHE_KEY_PREFIX)) { + return readDesktopTokenFileSync( + cacheKey.slice(FILE_CACHE_KEY_PREFIX.length), + ) } try { @@ -145,8 +148,9 @@ const getOpenAIOAuthTokensCached = memoize( ) export function getOpenAIOAuthTokens(): OpenAIOAuthTokens | null { + const filePath = getDesktopTokenFilePath() return getOpenAIOAuthTokensCached( - getDesktopTokenFilePath() ?? SECURE_STORAGE_CACHE_KEY, + filePath ? `${FILE_CACHE_KEY_PREFIX}${filePath}` : SECURE_STORAGE_CACHE_KEY, ) }