From 98fa5a0eddb0213bbba7d3cd4c032bf61cd78ed5 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:15:09 +0800 Subject: [PATCH] Harden OpenAI OAuth token file writes The desktop OpenAI OAuth token store writes plaintext refresh credentials through a temporary file before rename. A failed rename previously left that temporary file behind in the server-side store, so the writer now mirrors the CLI storage cleanup path and removes failed temp files before surfacing the original write failure. Constraint: OpenAI OAuth token files may contain refresh and id tokens. Rejected: Rely on logout cleanup | failed writes can leave orphan temp files before logout runs. Confidence: high Scope-risk: narrow 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 --- .../haha-openai-oauth-service.test.ts | 32 ++++++++++++++++++- src/server/services/hahaOpenAIOAuthService.ts | 14 ++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/server/__tests__/haha-openai-oauth-service.test.ts b/src/server/__tests__/haha-openai-oauth-service.test.ts index 5e129072..8bd331b7 100644 --- a/src/server/__tests__/haha-openai-oauth-service.test.ts +++ b/src/server/__tests__/haha-openai-oauth-service.test.ts @@ -2,7 +2,7 @@ * Unit tests for HahaOpenAIOAuthService — haha 自管 OpenAI OAuth 的核心 service 层。 */ -import { describe, test, expect, beforeEach, afterEach } from 'bun:test' +import { describe, test, expect, beforeEach, afterEach, spyOn } from 'bun:test' import * as fs from 'fs/promises' import * as path from 'path' import * as os from 'os' @@ -75,6 +75,36 @@ describe('HahaOpenAIOAuthService — file storage', () => { await service.deleteTokens() expect(await service.loadTokens()).toBeNull() }) + + test('saveTokens cleans up tmp file when rename fails', async () => { + const renameSpy = spyOn(fs, 'rename').mockImplementation(async () => { + const error = new Error('rename failed') as NodeJS.ErrnoException + error.code = 'EXDEV' + throw error + }) + + try { + await expect( + service.saveTokens({ + accessToken: 'sensitive-access', + refreshToken: 'sensitive-refresh', + expiresAt: Date.now() + 3600_000, + idToken: 'sensitive-id-token', + email: 'test@example.com', + accountId: 'acct_123', + }), + ).rejects.toThrow('rename failed') + } finally { + renameSpy.mockRestore() + } + + const oauthPath = getHahaOpenAIOAuthFilePath() + const files = await fs.readdir(path.dirname(oauthPath)) + expect( + files.filter((name) => name.startsWith('openai-oauth.json.tmp.')), + ).toEqual([]) + expect(await service.loadTokens()).toBeNull() + }) }) describe('HahaOpenAIOAuthService — session management', () => { diff --git a/src/server/services/hahaOpenAIOAuthService.ts b/src/server/services/hahaOpenAIOAuthService.ts index b9dde1af..74738b9c 100644 --- a/src/server/services/hahaOpenAIOAuthService.ts +++ b/src/server/services/hahaOpenAIOAuthService.ts @@ -84,9 +84,17 @@ export class HahaOpenAIOAuthService { async saveTokens(tokens: StoredOpenAIOAuthTokens): Promise { const filePath = this.getOAuthFilePath() await fs.mkdir(path.dirname(filePath), { recursive: true }) - const tmp = `${filePath}.tmp.${process.pid}` - await fs.writeFile(tmp, JSON.stringify(tokens, null, 2), { mode: 0o600 }) - await fs.rename(tmp, filePath) + const tmp = `${filePath}.tmp.${process.pid}.${Date.now()}` + let renamed = false + try { + await fs.writeFile(tmp, JSON.stringify(tokens, null, 2), { mode: 0o600 }) + await fs.rename(tmp, filePath) + renamed = true + } finally { + if (!renamed) { + await fs.rm(tmp, { force: true }).catch(() => {}) + } + } } async deleteTokens(): Promise {