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

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))
+  })
 })

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<string, unknown> {
   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<string, Promise<void>>()
-
-  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<T>(
-    filePath: string,
-    task: () => Promise<T>,
-  ): Promise<T> {
-    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<Record<string, unknown>> {
-    await ensurePersistentStorageUpgraded()
-    return readRecoverableJsonFile({
-      filePath: this.getSettingsPath(),
-      label: 'cc-haha managed settings',
-      defaultValue: {},
-      normalize: normalizeJsonObject,
-    })
-  }
-
-  private async writeManagedSettings(settings: Record<string, unknown>): Promise<void> {
-    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<string, unknown>
     h5Access: StoredH5AccessSettings
   }> {
-    const managedSettings = await this.readManagedSettings()
+    const managedSettings = await this.managedSettingsService.readSettings()
     return {
       managedSettings,
       h5Access: normalizeStoredSettings(managedSettings.h5Access),
     }
   }
 
-  private async saveStoredSettings(
-    managedSettings: Record<string, unknown>,
-    h5Access: StoredH5AccessSettings,
-  ): Promise<void> {
-    await this.writeManagedSettings({
-      ...managedSettings,
-      h5Access,
-    })
-  }
-
   private async setToken(
     managedSettings: Record<string, unknown>,
     current: StoredH5AccessSettings,
-  ): Promise<H5AccessEnableResult> {
+  ): Promise<{
+    settings: Record<string, unknown>
+    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<H5AccessEnableResult> {
-    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<H5AccessSettings> {
-    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<H5AccessEnableResult> {
-    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<H5AccessSettings> {
-    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),
+      }
     })
   }
 

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<string, Promise<void>>()
+
+  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<T>(
+    filePath: string,
+    task: () => Promise<T>,
+  ): Promise<T> {
+    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<string, unknown>): Promise<void> {
+    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<Record<string, unknown>> {
+    await ensurePersistentStorageUpgraded()
+    return readRecoverableJsonFile({
+      filePath: this.getSettingsPath(),
+      label: 'cc-haha managed settings',
+      defaultValue: {},
+      normalize: normalizeJsonObject,
+    })
+  }
+
+  async updateSettings<T>(
+    updater: (current: Record<string, unknown>) => Promise<{
+      settings: Record<string, unknown>
+      result: T
+    }> | {
+      settings: Record<string, unknown>
+      result: T
+    },
+  ): Promise<T> {
+    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
+    })
+  }
+}

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<ProvidersIndex> {
     await ensurePersistentStorageUpgraded()
     return readRecoverableJsonFile({
@@ -221,28 +219,7 @@ export class ProviderService {
   }
 
   private async readSettings(): Promise<Record<string, unknown>> {
-    await ensurePersistentStorageUpgraded()
-    return readRecoverableJsonFile({
-      filePath: this.getSettingsPath(),
-      label: 'cc-haha managed settings',
-      defaultValue: {},
-      normalize: normalizeJsonObject,
-    })
-  }
-
-  private async writeSettings(settings: Record<string, unknown>): Promise<void> {
-    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<Record<string, unknown>> {
@@ -250,8 +227,10 @@ export class ProviderService {
   }
 
   async updateManagedSettings(settings: Record<string, unknown>): Promise<void> {
-    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<void> {
-    const settings = await this.readSettings()
-    const existingEnv = (settings.env as Record<string, string>) || {}
-    const cleanedEnv = { ...existingEnv }
+    await this.managedSettingsService.updateSettings((settings) => {
+      const existingEnv = (settings.env as Record<string, string>) || {}
+      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<void> {
-    const settings = await this.readSettings()
-    const env = (settings.env as Record<string, string>) || {}
+    await this.managedSettingsService.updateSettings((settings) => {
+      const env = { ...((settings.env as Record<string, string>) || {}) }
 
-    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<string, unknown> = {
+        ...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 ---