From 3276a19da04debc0b0d84f359b941c2f4f104264 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: Sat, 9 May 2026 23:32:39 +0800 Subject: [PATCH] Prevent managed settings races from dropping H5 or provider state Move cc-haha managed settings writes behind one shared transaction helper so H5 access mutations and provider settings sync serialize against the same file. Also treat persisted H5 state without a valid token hash as disabled so origin allowlists cannot remain active after partial corruption. Constraint: H5 and provider config both share ~/.claude/cc-haha/settings.json Rejected: Keep per-service locks | concurrent read-modify-write still drops unrelated fields Rejected: Only harden isOriginAllowed | corrupted enabled state would still surface as active in settings reads Confidence: high Scope-risk: narrow Directive: Any future writer of cc-haha/settings.json must go through the shared managed settings transaction path Tested: bun test src/server/__tests__/h5-access-service.test.ts src/server/__tests__/h5-access-api.test.ts src/server/__tests__/providers.test.ts src/server/__tests__/provider-presets.test.ts Not-tested: Full repo typecheck remains blocked by the existing bun-types/tsconfig environment issue in this worktree --- .../__tests__/h5-access-service.test.ts | 55 +++++++ src/server/services/h5AccessService.ts | 139 ++++++------------ src/server/services/managedSettingsService.ts | 83 +++++++++++ src/server/services/providerService.ts | 95 ++++++------ 4 files changed, 228 insertions(+), 144 deletions(-) create mode 100644 src/server/services/managedSettingsService.ts diff --git a/src/server/__tests__/h5-access-service.test.ts b/src/server/__tests__/h5-access-service.test.ts index c5867860..71adab5e 100644 --- a/src/server/__tests__/h5-access-service.test.ts +++ b/src/server/__tests__/h5-access-service.test.ts @@ -3,6 +3,7 @@ import * as fs from 'node:fs/promises' import * as os from 'node:os' import * as path from 'node:path' import { H5AccessService } from '../services/h5AccessService.js' +import { ProviderService } from '../services/providerService.js' let tmpDir: string let originalConfigDir: string | undefined @@ -154,4 +155,58 @@ describe('H5AccessService', () => { await expect(service.isOriginAllowed('https://other.example.com')).resolves.toBe(false) await expect(service.isOriginAllowed('notaurl')).resolves.toBe(false) }) + + test('malformed persisted enabled state without token hash is treated as disabled', async () => { + await fs.mkdir(path.dirname(getManagedSettingsPath()), { recursive: true }) + await fs.writeFile( + getManagedSettingsPath(), + JSON.stringify({ + h5Access: { + enabled: true, + allowedOrigins: ['https://example.com/path'], + publicBaseUrl: 'https://public.example.com', + }, + }), + 'utf-8', + ) + + const service = new H5AccessService() + + await expect(service.getSettings()).resolves.toEqual({ + enabled: false, + tokenPreview: null, + allowedOrigins: ['https://example.com'], + publicBaseUrl: 'https://public.example.com', + }) + await expect(service.validateToken('anything')).resolves.toBe(false) + await expect(service.isOriginAllowed('https://example.com')).resolves.toBe(false) + }) + + test('concurrent h5 enable and provider managed settings update preserve both fields', async () => { + const h5Service = new H5AccessService() + const providerService = new ProviderService() + + await Promise.all([ + h5Service.enable(), + providerService.updateManagedSettings({ + env: { + ANTHROPIC_MODEL: 'keep-me', + }, + }), + ]) + + const saved = JSON.parse(await fs.readFile(getManagedSettingsPath(), 'utf-8')) as { + env?: { + ANTHROPIC_MODEL?: string + } + h5Access?: { + enabled?: boolean + tokenHash?: string | null + } + } + + expect(saved.env?.ANTHROPIC_MODEL).toBe('keep-me') + expect(saved.h5Access?.enabled).toBe(true) + expect(saved.h5Access?.tokenHash).toEqual(expect.any(String)) + }) }) diff --git a/src/server/services/h5AccessService.ts b/src/server/services/h5AccessService.ts index 33bd892c..d93baca1 100644 --- a/src/server/services/h5AccessService.ts +++ b/src/server/services/h5AccessService.ts @@ -1,10 +1,6 @@ -import * as fs from 'node:fs/promises' -import * as os from 'node:os' -import * as path from 'node:path' import { createHash, randomBytes } from 'node:crypto' import { ApiError } from '../middleware/errorHandler.js' -import { normalizeJsonObject, readRecoverableJsonFile } from './recoverableJsonFile.js' -import { ensurePersistentStorageUpgraded } from './persistentStorageMigrations.js' +import { ManagedSettingsService } from './managedSettingsService.js' export type H5AccessSettings = { enabled: boolean @@ -30,6 +26,8 @@ const DEFAULT_STORED_SETTINGS: StoredH5AccessSettings = { publicBaseUrl: null, } +const TOKEN_HASH_RE = /^[a-f0-9]{64}$/ + function isRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value) } @@ -149,97 +147,40 @@ function normalizeStoredSettings(value: unknown): StoredH5AccessSettings { } } + const tokenHash = typeof value.tokenHash === 'string' && TOKEN_HASH_RE.test(value.tokenHash) + ? value.tokenHash + : null + return { - enabled: value.enabled === true, - tokenHash: typeof value.tokenHash === 'string' ? value.tokenHash : null, - tokenPreview: typeof value.tokenPreview === 'string' ? value.tokenPreview : null, + enabled: value.enabled === true && tokenHash !== null, + tokenHash, + tokenPreview: tokenHash && typeof value.tokenPreview === 'string' ? value.tokenPreview : null, allowedOrigins, publicBaseUrl, } } export class H5AccessService { - private static writeLocks = new Map>() - - private getConfigDir(): string { - return process.env.CLAUDE_CONFIG_DIR || path.join(os.homedir(), '.claude') - } - - private getSettingsPath(): string { - return path.join(this.getConfigDir(), 'cc-haha', 'settings.json') - } - - private async withWriteLock( - filePath: string, - task: () => Promise, - ): Promise { - const previousWrite = H5AccessService.writeLocks.get(filePath) ?? Promise.resolve() - const nextWrite = previousWrite.catch(() => {}).then(task) - const trackedWrite = nextWrite.then(() => {}, () => {}) - - H5AccessService.writeLocks.set(filePath, trackedWrite) - - try { - return await nextWrite - } finally { - if (H5AccessService.writeLocks.get(filePath) === trackedWrite) { - H5AccessService.writeLocks.delete(filePath) - } - } - } - - private async readManagedSettings(): Promise> { - await ensurePersistentStorageUpgraded() - return readRecoverableJsonFile({ - filePath: this.getSettingsPath(), - label: 'cc-haha managed settings', - defaultValue: {}, - normalize: normalizeJsonObject, - }) - } - - private async writeManagedSettings(settings: Record): Promise { - const filePath = this.getSettingsPath() - const dir = path.dirname(filePath) - const contents = JSON.stringify(settings, null, 2) + '\n' - const tmpFile = `${filePath}.tmp.${process.pid}.${Date.now()}.${randomBytes(6).toString('hex')}` - - await fs.mkdir(dir, { recursive: true }) - - try { - await fs.writeFile(tmpFile, contents, 'utf-8') - await fs.rename(tmpFile, filePath) - } catch (error) { - await fs.unlink(tmpFile).catch(() => {}) - throw ApiError.internal(`Failed to write settings.json: ${error}`) - } - } + private managedSettingsService = new ManagedSettingsService() private async readStoredSettings(): Promise<{ managedSettings: Record h5Access: StoredH5AccessSettings }> { - const managedSettings = await this.readManagedSettings() + const managedSettings = await this.managedSettingsService.readSettings() return { managedSettings, h5Access: normalizeStoredSettings(managedSettings.h5Access), } } - private async saveStoredSettings( - managedSettings: Record, - h5Access: StoredH5AccessSettings, - ): Promise { - await this.writeManagedSettings({ - ...managedSettings, - h5Access, - }) - } - private async setToken( managedSettings: Record, current: StoredH5AccessSettings, - ): Promise { + ): Promise<{ + settings: Record + result: H5AccessEnableResult + }> { const token = createToken() const nextSettings: StoredH5AccessSettings = { ...current, @@ -248,11 +189,15 @@ export class H5AccessService { tokenPreview: createTokenPreview(token), } - await this.saveStoredSettings(managedSettings, nextSettings) - return { - settings: toPublicSettings(nextSettings), - token, + settings: { + ...managedSettings, + h5Access: nextSettings, + }, + result: { + settings: toPublicSettings(nextSettings), + token, + }, } } @@ -262,15 +207,14 @@ export class H5AccessService { } async enable(): Promise { - return this.withWriteLock(this.getSettingsPath(), async () => { - const { managedSettings, h5Access } = await this.readStoredSettings() - return this.setToken(managedSettings, h5Access) + return this.managedSettingsService.updateSettings(async (current) => { + return this.setToken(current, normalizeStoredSettings(current.h5Access)) }) } async disable(): Promise { - return this.withWriteLock(this.getSettingsPath(), async () => { - const { managedSettings, h5Access } = await this.readStoredSettings() + return this.managedSettingsService.updateSettings(async (current) => { + const h5Access = normalizeStoredSettings(current.h5Access) const nextSettings: StoredH5AccessSettings = { ...h5Access, enabled: false, @@ -278,15 +222,19 @@ export class H5AccessService { tokenPreview: null, } - await this.saveStoredSettings(managedSettings, nextSettings) - return toPublicSettings(nextSettings) + return { + settings: { + ...current, + h5Access: nextSettings, + }, + result: toPublicSettings(nextSettings), + } }) } async regenerateToken(): Promise { - return this.withWriteLock(this.getSettingsPath(), async () => { - const { managedSettings, h5Access } = await this.readStoredSettings() - return this.setToken(managedSettings, h5Access) + return this.managedSettingsService.updateSettings(async (current) => { + return this.setToken(current, normalizeStoredSettings(current.h5Access)) }) } @@ -294,8 +242,8 @@ export class H5AccessService { allowedOrigins?: string[] publicBaseUrl?: string | null }): Promise { - return this.withWriteLock(this.getSettingsPath(), async () => { - const { managedSettings, h5Access } = await this.readStoredSettings() + return this.managedSettingsService.updateSettings(async (current) => { + const h5Access = normalizeStoredSettings(current.h5Access) const nextSettings: StoredH5AccessSettings = { ...h5Access, allowedOrigins: input.allowedOrigins === undefined @@ -306,8 +254,13 @@ export class H5AccessService { : normalizePublicBaseUrl(input.publicBaseUrl), } - await this.saveStoredSettings(managedSettings, nextSettings) - return toPublicSettings(nextSettings) + return { + settings: { + ...current, + h5Access: nextSettings, + }, + result: toPublicSettings(nextSettings), + } }) } diff --git a/src/server/services/managedSettingsService.ts b/src/server/services/managedSettingsService.ts new file mode 100644 index 00000000..cc267aa1 --- /dev/null +++ b/src/server/services/managedSettingsService.ts @@ -0,0 +1,83 @@ +import * as fs from 'node:fs/promises' +import * as os from 'node:os' +import * as path from 'node:path' +import { randomBytes } from 'node:crypto' +import { ApiError } from '../middleware/errorHandler.js' +import { normalizeJsonObject, readRecoverableJsonFile } from './recoverableJsonFile.js' +import { ensurePersistentStorageUpgraded } from './persistentStorageMigrations.js' + +export class ManagedSettingsService { + private static writeLocks = new Map>() + + private getConfigDir(): string { + return process.env.CLAUDE_CONFIG_DIR || path.join(os.homedir(), '.claude') + } + + private getSettingsPath(): string { + return path.join(this.getConfigDir(), 'cc-haha', 'settings.json') + } + + private async withWriteLock( + filePath: string, + task: () => Promise, + ): Promise { + const previousWrite = ManagedSettingsService.writeLocks.get(filePath) ?? Promise.resolve() + const nextWrite = previousWrite.catch(() => {}).then(task) + const trackedWrite = nextWrite.then(() => {}, () => {}) + + ManagedSettingsService.writeLocks.set(filePath, trackedWrite) + + try { + return await nextWrite + } finally { + if (ManagedSettingsService.writeLocks.get(filePath) === trackedWrite) { + ManagedSettingsService.writeLocks.delete(filePath) + } + } + } + + private async writeSettings(settings: Record): Promise { + const filePath = this.getSettingsPath() + const dir = path.dirname(filePath) + const contents = JSON.stringify(settings, null, 2) + '\n' + const tmpFile = `${filePath}.tmp.${process.pid}.${Date.now()}.${randomBytes(6).toString('hex')}` + + await fs.mkdir(dir, { recursive: true }) + + try { + await fs.writeFile(tmpFile, contents, 'utf-8') + await fs.rename(tmpFile, filePath) + } catch (error) { + await fs.unlink(tmpFile).catch(() => {}) + throw ApiError.internal(`Failed to write settings.json: ${error}`) + } + } + + async readSettings(): Promise> { + await ensurePersistentStorageUpgraded() + return readRecoverableJsonFile({ + filePath: this.getSettingsPath(), + label: 'cc-haha managed settings', + defaultValue: {}, + normalize: normalizeJsonObject, + }) + } + + async updateSettings( + updater: (current: Record) => Promise<{ + settings: Record + result: T + }> | { + settings: Record + result: T + }, + ): Promise { + const filePath = this.getSettingsPath() + return this.withWriteLock(filePath, async () => { + const current = await this.readSettings() + const { settings, result } = await updater(current) + await this.writeSettings(settings) + return result + }) + } +} diff --git a/src/server/services/providerService.ts b/src/server/services/providerService.ts index 82e1c4bc..2d4202f0 100644 --- a/src/server/services/providerService.ts +++ b/src/server/services/providerService.ts @@ -10,7 +10,8 @@ import * as fs from 'fs/promises' import * as path from 'path' import * as os from 'os' import { ApiError } from '../middleware/errorHandler.js' -import { normalizeJsonObject, readRecoverableJsonFile } from './recoverableJsonFile.js' +import { readRecoverableJsonFile } from './recoverableJsonFile.js' +import { ManagedSettingsService } from './managedSettingsService.js' import { anthropicToOpenaiChat } from '../proxy/transform/anthropicToOpenaiChat.js' import { anthropicToOpenaiResponses } from '../proxy/transform/anthropicToOpenaiResponses.js' import { openaiChatToAnthropic } from '../proxy/transform/openaiChatToAnthropic.js' @@ -171,6 +172,7 @@ function getManagedEnvKeys(): string[] { export class ProviderService { private static serverPort = 3456 + private managedSettingsService = new ManagedSettingsService() static setServerPort(port: number): void { ProviderService.serverPort = port @@ -191,10 +193,6 @@ export class ProviderService { return path.join(this.getCcHahaDir(), 'providers.json') } - private getSettingsPath(): string { - return path.join(this.getCcHahaDir(), 'settings.json') - } - private async readIndex(): Promise { await ensurePersistentStorageUpgraded() return readRecoverableJsonFile({ @@ -221,28 +219,7 @@ export class ProviderService { } private async readSettings(): Promise> { - await ensurePersistentStorageUpgraded() - return readRecoverableJsonFile({ - filePath: this.getSettingsPath(), - label: 'cc-haha managed settings', - defaultValue: {}, - normalize: normalizeJsonObject, - }) - } - - private async writeSettings(settings: Record): Promise { - const filePath = this.getSettingsPath() - const dir = path.dirname(filePath) - await fs.mkdir(dir, { recursive: true }) - - const tmpFile = `${filePath}.tmp.${Date.now()}` - try { - await fs.writeFile(tmpFile, JSON.stringify(settings, null, 2) + '\n', 'utf-8') - await fs.rename(tmpFile, filePath) - } catch (err) { - await fs.unlink(tmpFile).catch(() => {}) - throw ApiError.internal(`Failed to write settings.json: ${err}`) - } + return this.managedSettingsService.readSettings() } async getManagedSettings(): Promise> { @@ -250,8 +227,10 @@ export class ProviderService { } async updateManagedSettings(settings: Record): Promise { - const current = await this.readSettings() - await this.writeSettings(Object.assign({}, current, settings)) + await this.managedSettingsService.updateSettings((current) => ({ + settings: Object.assign({}, current, settings), + result: undefined, + })) } // --- CRUD --- @@ -415,36 +394,50 @@ export class ProviderService { } private async syncToSettings(provider: SavedProvider): Promise { - const settings = await this.readSettings() - const existingEnv = (settings.env as Record) || {} - const cleanedEnv = { ...existingEnv } + await this.managedSettingsService.updateSettings((settings) => { + const existingEnv = (settings.env as Record) || {} + const cleanedEnv = { ...existingEnv } - for (const key of getManagedEnvKeys()) { - delete cleanedEnv[key] - } + for (const key of getManagedEnvKeys()) { + delete cleanedEnv[key] + } - settings.env = { - ...cleanedEnv, - ...this.buildManagedEnv(provider), - } - - await this.writeSettings(settings) + return { + settings: { + ...settings, + env: { + ...cleanedEnv, + ...this.buildManagedEnv(provider), + }, + }, + result: undefined, + } + }) } private async clearProviderFromSettings(): Promise { - const settings = await this.readSettings() - const env = (settings.env as Record) || {} + await this.managedSettingsService.updateSettings((settings) => { + const env = { ...((settings.env as Record) || {}) } - for (const key of getManagedEnvKeys()) { - delete env[key] - } + for (const key of getManagedEnvKeys()) { + delete env[key] + } - settings.env = env - if (Object.keys(env).length === 0) { - delete settings.env - } + const nextSettings: Record = { + ...settings, + } - await this.writeSettings(settings) + if (Object.keys(env).length === 0) { + delete nextSettings.env + } else { + nextSettings.env = env + } + + return { + settings: nextSettings, + result: undefined, + } + }) } // --- Auth status ---