mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
Prevent env-pinned OpenAI OAuth reads from resurrecting stale secure tokens
When OPENAI_CODEX_OAUTH_FILE is set, the override file must stay the only source of truth. Reads now short-circuit to that file path, sync caching keys off the resolved override path, and failed atomic writes remove their tmp files before returning control. Constraint: OPENAI_CODEX_OAUTH_FILE must override shared secure storage even when the file is missing or corrupt Rejected: Keep zero-arg memoization and rely on manual cache clears | callers can switch override paths without touching the cache Confidence: high Scope-risk: narrow Directive: Do not reintroduce secure-storage fallback while the override env var is set 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: bun run check:server Tested: git diff --check
This commit is contained in:
parent
0927c80cdc
commit
b61df181d3
@ -1,4 +1,4 @@
|
||||
import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
|
||||
import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'
|
||||
import * as fs from 'fs'
|
||||
import * as fsp from 'fs/promises'
|
||||
import * as os from 'os'
|
||||
@ -10,30 +10,67 @@ import {
|
||||
getOpenAIOAuthTokensAsync,
|
||||
saveOpenAIOAuthTokens,
|
||||
} from './storage.js'
|
||||
import { plainTextStorage } from '../../utils/secureStorage/plainTextStorage.js'
|
||||
import type { OpenAIOAuthTokens } from './types.js'
|
||||
|
||||
describe('OpenAI OAuth desktop token file storage', () => {
|
||||
let tmpDir: string
|
||||
let tokenPath: string
|
||||
let originalTokenFile: string | undefined
|
||||
let originalConfigDir: string | undefined
|
||||
let originalHome: string | undefined
|
||||
let originalUserProfile: string | undefined
|
||||
|
||||
beforeEach(async () => {
|
||||
tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'openai-oauth-storage-'))
|
||||
tokenPath = path.join(tmpDir, 'openai-oauth.json')
|
||||
originalTokenFile = process.env.OPENAI_CODEX_OAUTH_FILE
|
||||
originalConfigDir = process.env.CLAUDE_CONFIG_DIR
|
||||
originalHome = process.env.HOME
|
||||
originalUserProfile = process.env.USERPROFILE
|
||||
process.env.CLAUDE_CONFIG_DIR = tmpDir
|
||||
process.env.HOME = tmpDir
|
||||
process.env.USERPROFILE = tmpDir
|
||||
process.env.OPENAI_CODEX_OAUTH_FILE = tokenPath
|
||||
clearOpenAIOAuthTokenCache()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
plainTextStorage.delete()
|
||||
if (originalTokenFile === undefined) {
|
||||
delete process.env.OPENAI_CODEX_OAUTH_FILE
|
||||
} else {
|
||||
process.env.OPENAI_CODEX_OAUTH_FILE = originalTokenFile
|
||||
}
|
||||
if (originalConfigDir === undefined) {
|
||||
delete process.env.CLAUDE_CONFIG_DIR
|
||||
} else {
|
||||
process.env.CLAUDE_CONFIG_DIR = originalConfigDir
|
||||
}
|
||||
if (originalHome === undefined) {
|
||||
delete process.env.HOME
|
||||
} else {
|
||||
process.env.HOME = originalHome
|
||||
}
|
||||
if (originalUserProfile === undefined) {
|
||||
delete process.env.USERPROFILE
|
||||
} else {
|
||||
process.env.USERPROFILE = originalUserProfile
|
||||
}
|
||||
clearOpenAIOAuthTokenCache()
|
||||
await fsp.rm(tmpDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function seedSecureStorage(tokens: OpenAIOAuthTokens): void {
|
||||
delete process.env.OPENAI_CODEX_OAUTH_FILE
|
||||
clearOpenAIOAuthTokenCache()
|
||||
expect(
|
||||
plainTextStorage.update({ openaiCodexOauth: tokens }).success,
|
||||
).toBe(true)
|
||||
process.env.OPENAI_CODEX_OAUTH_FILE = tokenPath
|
||||
clearOpenAIOAuthTokenCache()
|
||||
}
|
||||
|
||||
test('reads desktop token file synchronously', async () => {
|
||||
await fsp.writeFile(
|
||||
tokenPath,
|
||||
@ -119,4 +156,99 @@ describe('OpenAI OAuth desktop token file storage', () => {
|
||||
expect(deleteOpenAIOAuthTokens()).toBe(true)
|
||||
expect(fs.existsSync(tokenPath)).toBe(false)
|
||||
})
|
||||
|
||||
test('returns null from both getters when env override is set but file is missing', async () => {
|
||||
seedSecureStorage({
|
||||
accessToken: 'secure-access',
|
||||
refreshToken: 'secure-refresh',
|
||||
expiresAt: 4_100_000_000_000,
|
||||
})
|
||||
|
||||
expect(getOpenAIOAuthTokens()).toBeNull()
|
||||
await expect(getOpenAIOAuthTokensAsync()).resolves.toBeNull()
|
||||
})
|
||||
|
||||
test('returns null from both getters when env override file contains corrupt json', async () => {
|
||||
seedSecureStorage({
|
||||
accessToken: 'secure-access',
|
||||
refreshToken: 'secure-refresh',
|
||||
expiresAt: 4_100_000_000_000,
|
||||
})
|
||||
await fsp.writeFile(tokenPath, '{ definitely-not-json', 'utf-8')
|
||||
|
||||
expect(getOpenAIOAuthTokens()).toBeNull()
|
||||
await expect(getOpenAIOAuthTokensAsync()).resolves.toBeNull()
|
||||
})
|
||||
|
||||
test('does not fall back to secure storage after deleting env override file', async () => {
|
||||
seedSecureStorage({
|
||||
accessToken: 'secure-access',
|
||||
refreshToken: 'secure-refresh',
|
||||
expiresAt: 4_100_000_000_000,
|
||||
})
|
||||
await fsp.writeFile(
|
||||
tokenPath,
|
||||
JSON.stringify({
|
||||
accessToken: 'desktop-access',
|
||||
refreshToken: 'desktop-refresh',
|
||||
expiresAt: 4_100_000_000_000,
|
||||
}),
|
||||
'utf-8',
|
||||
)
|
||||
|
||||
expect(deleteOpenAIOAuthTokens()).toBe(true)
|
||||
expect(getOpenAIOAuthTokens()).toBeNull()
|
||||
await expect(getOpenAIOAuthTokensAsync()).resolves.toBeNull()
|
||||
})
|
||||
|
||||
test('reloads sync tokens when OPENAI_CODEX_OAUTH_FILE changes without clearing cache', async () => {
|
||||
const tokenPathA = path.join(tmpDir, 'openai-oauth-a.json')
|
||||
const tokenPathB = path.join(tmpDir, 'openai-oauth-b.json')
|
||||
await fsp.writeFile(
|
||||
tokenPathA,
|
||||
JSON.stringify({
|
||||
accessToken: 'access-a',
|
||||
refreshToken: 'refresh-a',
|
||||
expiresAt: 4_100_000_000_001,
|
||||
}),
|
||||
'utf-8',
|
||||
)
|
||||
await fsp.writeFile(
|
||||
tokenPathB,
|
||||
JSON.stringify({
|
||||
accessToken: 'access-b',
|
||||
refreshToken: 'refresh-b',
|
||||
expiresAt: 4_100_000_000_002,
|
||||
}),
|
||||
'utf-8',
|
||||
)
|
||||
|
||||
process.env.OPENAI_CODEX_OAUTH_FILE = tokenPathA
|
||||
expect(getOpenAIOAuthTokens()?.accessToken).toBe('access-a')
|
||||
|
||||
process.env.OPENAI_CODEX_OAUTH_FILE = tokenPathB
|
||||
expect(getOpenAIOAuthTokens()?.accessToken).toBe('access-b')
|
||||
})
|
||||
|
||||
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
|
||||
error.code = 'EXDEV'
|
||||
throw error
|
||||
})
|
||||
|
||||
const result = saveOpenAIOAuthTokens({
|
||||
accessToken: 'fresh-access',
|
||||
refreshToken: 'fresh-refresh',
|
||||
expiresAt: 4_100_000_000_000,
|
||||
})
|
||||
|
||||
renameSyncSpy.mockRestore()
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
const tmpFiles = (await fsp.readdir(tmpDir)).filter((name) =>
|
||||
name.startsWith('openai-oauth.json.tmp.'),
|
||||
)
|
||||
expect(tmpFiles).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
@ -13,6 +13,8 @@ type SecureStorageShape = Record<string, unknown> & {
|
||||
openaiCodexOauth?: OpenAIOAuthTokens
|
||||
}
|
||||
|
||||
const SECURE_STORAGE_CACHE_KEY = '__secure-storage__'
|
||||
|
||||
function getDesktopTokenFilePath(): string | null {
|
||||
const filePath = process.env[OPENAI_CODEX_OAUTH_FILE_ENV_KEY]?.trim()
|
||||
return filePath ? filePath : null
|
||||
@ -43,8 +45,7 @@ function normalizeTokenFile(value: unknown): OpenAIOAuthTokens | null {
|
||||
}
|
||||
}
|
||||
|
||||
function readDesktopTokenFileSync(): OpenAIOAuthTokens | null {
|
||||
const filePath = getDesktopTokenFilePath()
|
||||
function readDesktopTokenFileSync(filePath = getDesktopTokenFilePath()): OpenAIOAuthTokens | null {
|
||||
if (!filePath) return null
|
||||
|
||||
try {
|
||||
@ -57,8 +58,9 @@ function readDesktopTokenFileSync(): OpenAIOAuthTokens | null {
|
||||
}
|
||||
}
|
||||
|
||||
async function readDesktopTokenFileAsync(): Promise<OpenAIOAuthTokens | null> {
|
||||
const filePath = getDesktopTokenFilePath()
|
||||
async function readDesktopTokenFileAsync(
|
||||
filePath = getDesktopTokenFilePath(),
|
||||
): Promise<OpenAIOAuthTokens | null> {
|
||||
if (!filePath) return null
|
||||
|
||||
try {
|
||||
@ -78,11 +80,26 @@ function writeDesktopTokenFileSync(tokens: OpenAIOAuthTokens): boolean {
|
||||
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true })
|
||||
const tmpFile = `${filePath}.tmp.${process.pid}.${Date.now()}`
|
||||
fs.writeFileSync(tmpFile, JSON.stringify(tokens, null, 2) + '\n', {
|
||||
mode: 0o600,
|
||||
})
|
||||
fs.renameSync(tmpFile, filePath)
|
||||
return true
|
||||
let renamed = false
|
||||
|
||||
try {
|
||||
fs.writeFileSync(tmpFile, JSON.stringify(tokens, null, 2) + '\n', {
|
||||
mode: 0o600,
|
||||
})
|
||||
fs.renameSync(tmpFile, filePath)
|
||||
renamed = true
|
||||
return true
|
||||
} finally {
|
||||
if (!renamed) {
|
||||
try {
|
||||
fs.rmSync(tmpFile, { force: true })
|
||||
} catch (cleanupError) {
|
||||
if ((cleanupError as NodeJS.ErrnoException).code !== 'ENOENT') {
|
||||
logError(cleanupError)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function saveOpenAIOAuthTokens(tokens: OpenAIOAuthTokens): {
|
||||
@ -110,23 +127,34 @@ export function saveOpenAIOAuthTokens(tokens: OpenAIOAuthTokens): {
|
||||
}
|
||||
}
|
||||
|
||||
export const getOpenAIOAuthTokens = memoize((): OpenAIOAuthTokens | null => {
|
||||
const desktopTokens = readDesktopTokenFileSync()
|
||||
if (desktopTokens) return desktopTokens
|
||||
const getOpenAIOAuthTokensCached = memoize(
|
||||
(cacheKey: string): OpenAIOAuthTokens | null => {
|
||||
if (cacheKey !== SECURE_STORAGE_CACHE_KEY) {
|
||||
return readDesktopTokenFileSync(cacheKey)
|
||||
}
|
||||
|
||||
try {
|
||||
const storage = getSecureStorage()
|
||||
const data = storage.read() as SecureStorageShape | null
|
||||
return data?.openaiCodexOauth ?? null
|
||||
} catch (error) {
|
||||
logError(error)
|
||||
return null
|
||||
}
|
||||
})
|
||||
try {
|
||||
const storage = getSecureStorage()
|
||||
const data = storage.read() as SecureStorageShape | null
|
||||
return data?.openaiCodexOauth ?? null
|
||||
} catch (error) {
|
||||
logError(error)
|
||||
return null
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
export function getOpenAIOAuthTokens(): OpenAIOAuthTokens | null {
|
||||
return getOpenAIOAuthTokensCached(
|
||||
getDesktopTokenFilePath() ?? SECURE_STORAGE_CACHE_KEY,
|
||||
)
|
||||
}
|
||||
|
||||
export async function getOpenAIOAuthTokensAsync(): Promise<OpenAIOAuthTokens | null> {
|
||||
const desktopTokens = await readDesktopTokenFileAsync()
|
||||
if (desktopTokens) return desktopTokens
|
||||
const filePath = getDesktopTokenFilePath()
|
||||
if (filePath) {
|
||||
return readDesktopTokenFileAsync(filePath)
|
||||
}
|
||||
|
||||
try {
|
||||
const storage = getSecureStorage()
|
||||
@ -139,7 +167,7 @@ export async function getOpenAIOAuthTokensAsync(): Promise<OpenAIOAuthTokens | n
|
||||
}
|
||||
|
||||
export function clearOpenAIOAuthTokenCache(): void {
|
||||
getOpenAIOAuthTokens.cache?.clear?.()
|
||||
getOpenAIOAuthTokensCached.cache?.clear?.()
|
||||
}
|
||||
|
||||
export function deleteOpenAIOAuthTokens(): boolean {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user