mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
fix(desktop): harden portable storage writes (#949)
Guard best-effort Electron window-state persistence so an unwritable data directory cannot crash the main process. Validate portable data directories before app-mode persistence and surface failures back to settings so restart is not attempted after rollback. Tested: cd desktop && bun test electron/services/windows.test.ts electron/services/appMode.test.ts Tested: cd desktop && bun run test src/stores/settingsStore.test.ts -t "settingsStore app mode" --run Tested: cd desktop && bun run check:electron Tested: bun run check:desktop Not-tested: bun run check:native; bun run verify; coverage gates not run for scoped local handoff. Confidence: high Scope-risk: narrow
This commit is contained in:
parent
6e6c87aa16
commit
718e80f8c5
@ -117,6 +117,18 @@ describe('Electron app mode service', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('does not point existing config at a portable dir that cannot persist app-mode.json', () => {
|
||||
const fakeApp = app()
|
||||
const active = tempDir()
|
||||
const selected = path.join(tempDir(), 'portable')
|
||||
fs.mkdirSync(path.join(selected, 'app-mode.json'), { recursive: true })
|
||||
|
||||
expect(() => setAppMode(fakeApp, { mode: 'portable', portableDir: selected }, { CLAUDE_CONFIG_DIR: active }))
|
||||
.toThrow()
|
||||
expect(fs.existsSync(path.join(active, 'app-mode.json'))).toBe(false)
|
||||
expect(fs.existsSync(path.join(fakeApp.getPath('userData'), 'app-mode.json'))).toBe(false)
|
||||
})
|
||||
|
||||
it('reports whether the default portable dir already has data', () => {
|
||||
const fakeApp = app()
|
||||
expect(detectPortableDir(fakeApp)).toEqual({
|
||||
|
||||
@ -60,6 +60,20 @@ export function writeAppModeConfig(configDir: string, config: PersistedAppModeCo
|
||||
fs.writeFileSync(path.join(configDir, APP_MODE_FILE), JSON.stringify(config, null, 2))
|
||||
}
|
||||
|
||||
function assertWritableDataDir(configDir: string): void {
|
||||
try {
|
||||
fs.mkdirSync(configDir, { recursive: true })
|
||||
const probeDir = fs.mkdtempSync(path.join(configDir, '.cc-haha-write-test-'))
|
||||
try {
|
||||
fs.writeFileSync(path.join(probeDir, 'probe'), '')
|
||||
} finally {
|
||||
fs.rmSync(probeDir, { recursive: true, force: true })
|
||||
}
|
||||
} catch {
|
||||
throw new Error(`Data storage directory is not writable: ${configDir}`)
|
||||
}
|
||||
}
|
||||
|
||||
export function determineStartupPortableDir(
|
||||
app: AppModeAppLike,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
@ -129,7 +143,7 @@ export function setAppMode(
|
||||
if (fs.existsSync(selectedDir) && !fs.statSync(selectedDir).isDirectory()) {
|
||||
throw new Error(`portable config path is not a directory: ${selectedDir}`)
|
||||
}
|
||||
fs.mkdirSync(selectedDir, { recursive: true })
|
||||
assertWritableDataDir(selectedDir)
|
||||
targetPortableDir = selectedDir
|
||||
config = {
|
||||
mode: 'portable',
|
||||
@ -137,14 +151,15 @@ export function setAppMode(
|
||||
}
|
||||
}
|
||||
|
||||
writeAppModeConfig(activeConfigDir, config)
|
||||
if (targetPortableDir && targetPortableDir !== activeConfigDir) {
|
||||
writeAppModeConfig(targetPortableDir, config)
|
||||
}
|
||||
|
||||
const systemConfigDir = app.getPath('userData')
|
||||
if (systemConfigDir !== activeConfigDir) {
|
||||
writeAppModeConfig(systemConfigDir, config)
|
||||
const configDirs = [
|
||||
targetPortableDir,
|
||||
activeConfigDir,
|
||||
systemConfigDir,
|
||||
].filter((dir): dir is string => Boolean(dir))
|
||||
|
||||
for (const configDir of [...new Set(configDirs)]) {
|
||||
writeAppModeConfig(configDir, config)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
@ -42,6 +42,25 @@ describe('Electron window service', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('does not crash when window state cannot be written', () => {
|
||||
const tmp = mkdtempSync(path.join(tmpdir(), 'electron-window-state-unwritable-'))
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined)
|
||||
try {
|
||||
const app = fakeApp(path.join(tmp, 'user-data'))
|
||||
const state = { x: 10, y: 20, width: 1280, height: 820, maximized: false }
|
||||
mkdirSync(path.join(tmp, 'window-state.json'))
|
||||
|
||||
expect(() => writeWindowState(app as never, state, { CLAUDE_CONFIG_DIR: tmp })).not.toThrow()
|
||||
expect(consoleError).toHaveBeenCalledWith(
|
||||
expect.stringContaining('[desktop] failed to write Electron window state'),
|
||||
expect.any(Error),
|
||||
)
|
||||
} finally {
|
||||
consoleError.mockRestore()
|
||||
rmSync(tmp, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('rejects undersized or off-screen state before restore', () => {
|
||||
expect(isPersistableWindowState({ x: 0, y: 0, width: 100, height: 100, maximized: false })).toBe(false)
|
||||
expect(hasMeaningfulIntersection(
|
||||
|
||||
@ -8,6 +8,7 @@ export const DEFAULT_WINDOW_HEIGHT = 820
|
||||
export const MIN_WINDOW_WIDTH = 960
|
||||
export const MIN_WINDOW_HEIGHT = 640
|
||||
const MIN_VISIBLE_PIXELS = 80
|
||||
const failedWindowStateWritePaths = new Set<string>()
|
||||
|
||||
export type StoredWindowState = {
|
||||
x: number
|
||||
@ -115,8 +116,16 @@ export function writeWindowState(
|
||||
) {
|
||||
if (!isPersistableWindowState(state)) return
|
||||
const statePath = windowStatePath(app, env)
|
||||
mkdirSync(path.dirname(statePath), { recursive: true })
|
||||
writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`)
|
||||
try {
|
||||
mkdirSync(path.dirname(statePath), { recursive: true })
|
||||
writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`)
|
||||
failedWindowStateWritePaths.delete(statePath)
|
||||
} catch (error) {
|
||||
if (!failedWindowStateWritePaths.has(statePath)) {
|
||||
failedWindowStateWritePaths.add(statePath)
|
||||
console.error(`[desktop] failed to write Electron window state ${statePath}:`, error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function captureWindowState(window: BrowserWindow): StoredWindowState | null {
|
||||
|
||||
@ -536,6 +536,30 @@ describe('settingsStore app mode', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('rolls back and surfaces app mode persistence failures', async () => {
|
||||
const error = new Error('Data storage directory is not writable')
|
||||
const setAppMode = vi.fn().mockRejectedValue(error)
|
||||
installElectronAppModeHost({ set: setAppMode })
|
||||
|
||||
const { useSettingsStore } = await import('./settingsStore')
|
||||
const prevAppMode = {
|
||||
mode: 'default' as const,
|
||||
portableDir: null,
|
||||
defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
activeConfigDir: null,
|
||||
configDirSource: 'system' as const,
|
||||
}
|
||||
useSettingsStore.setState({
|
||||
appMode: prevAppMode,
|
||||
appModeRequiresRestart: false,
|
||||
})
|
||||
|
||||
await expect(useSettingsStore.getState().setAppMode('portable', 'D:\\blocked-data'))
|
||||
.rejects.toThrow('Data storage directory is not writable')
|
||||
expect(useSettingsStore.getState().appMode).toEqual(prevAppMode)
|
||||
expect(useSettingsStore.getState().appModeRequiresRestart).toBe(false)
|
||||
})
|
||||
|
||||
it('switches app mode back to the system data source', async () => {
|
||||
const setAppMode = vi.fn().mockResolvedValue(undefined)
|
||||
installElectronAppModeHost({ set: setAppMode })
|
||||
|
||||
@ -593,8 +593,9 @@ export const useSettingsStore = create<SettingsStore>((set, get) => ({
|
||||
mode,
|
||||
portableDir: newMode.portableDir || null,
|
||||
})
|
||||
} catch {
|
||||
} catch (error) {
|
||||
set({ appMode: prev, appModeRequiresRestart: false })
|
||||
throw error
|
||||
}
|
||||
},
|
||||
}))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user