diff --git a/cli/src/__tests__/config-store.test.ts b/cli/src/__tests__/config-store.test.ts index cc19deda1b3..21abb4bf60e 100644 --- a/cli/src/__tests__/config-store.test.ts +++ b/cli/src/__tests__/config-store.test.ts @@ -114,15 +114,25 @@ describe("config store", () => { fs.writeFileSync(configPath, invalidBytes); fs.writeFileSync(`${configPath}.invalid-1`, "existing backup"); + const open = vi.spyOn(fs, "openSync"); + const sync = vi.spyOn(fs, "fsyncSync"); const backupPath = backupInvalidConfig(configPath); expect(backupPath).toBe(`${configPath}.invalid-2`); expect(fs.readFileSync(backupPath)).toEqual(invalidBytes); + expect(open).toHaveBeenCalledWith(backupPath, "r"); + expect(open).toHaveBeenCalledWith(path.dirname(configPath), "r"); + expect(sync).toHaveBeenCalled(); expect(() => writeConfig(defaultConfig(), configPath)).toThrow(/Refusing to overwrite invalid config/); expect(fs.readFileSync(configPath)).toEqual(invalidBytes); + open.mockClear(); + sync.mockClear(); const rename = vi.spyOn(fs, "renameSync"); expect(writeConfig(defaultConfig(), configPath, { invalidBackupPath: backupPath })).toBe(true); expect(rename).toHaveBeenCalledWith(expect.stringMatching(/config\.json\.tmp-\d+-\d+$/), configPath); + expect(open).toHaveBeenCalledWith(path.dirname(configPath), "r"); + expect(open.mock.invocationCallOrder.at(-1)!).toBeGreaterThan(rename.mock.invocationCallOrder.at(-1)!); + expect(sync.mock.invocationCallOrder.at(-1)!).toBeGreaterThan(open.mock.invocationCallOrder.at(-1)!); expect(readConfig(configPath)).not.toBeNull(); expect(fs.readdirSync(path.dirname(configPath)).some((entry) => entry.includes(".tmp-"))).toBe(false); }); diff --git a/cli/src/config/store.ts b/cli/src/config/store.ts index e9893036457..b1ab0229d17 100644 --- a/cli/src/config/store.ts +++ b/cli/src/config/store.ts @@ -110,6 +110,34 @@ function effectiveConfig(config: PaperclipConfig): Record { }; } +function syncDirectory(directoryPath: string): void { + let directoryDescriptor: number | null = null; + try { + directoryDescriptor = fs.openSync(directoryPath, "r"); + fs.fsyncSync(directoryDescriptor); + } catch (error) { + const code = error instanceof Error && "code" in error ? error.code : null; + if (process.platform !== "win32" || !["EACCES", "EINVAL", "EISDIR", "ENOTSUP", "EPERM"].includes(String(code))) { + throw error; + } + } finally { + if (directoryDescriptor !== null) fs.closeSync(directoryDescriptor); + } +} + +function durableCopyFile(sourcePath: string, destinationPath: string, flags = 0): void { + fs.copyFileSync(sourcePath, destinationPath, flags); + fs.chmodSync(destinationPath, 0o600); + + const backupDescriptor = fs.openSync(destinationPath, "r"); + try { + fs.fsyncSync(backupDescriptor); + } finally { + fs.closeSync(backupDescriptor); + } + syncDirectory(path.dirname(destinationPath)); +} + function atomicWriteFile(filePath: string, contents: string): void { let attempt = 0; @@ -124,6 +152,7 @@ function atomicWriteFile(filePath: string, contents: string): void { fs.closeSync(fileDescriptor); fileDescriptor = null; fs.renameSync(temporaryPath, filePath); + syncDirectory(path.dirname(filePath)); return; } catch (error) { if (fileDescriptor !== null) fs.closeSync(fileDescriptor); @@ -144,8 +173,7 @@ export function backupInvalidConfig(configPath?: string): string { for (let suffix = 1; ; suffix += 1) { const backupPath = `${filePath}.invalid-${suffix}`; try { - fs.copyFileSync(filePath, backupPath, fs.constants.COPYFILE_EXCL); - fs.chmodSync(backupPath, 0o600); + durableCopyFile(filePath, backupPath, fs.constants.COPYFILE_EXCL); return backupPath; } catch (error) { const code = error instanceof Error && "code" in error ? error.code : null; @@ -193,8 +221,7 @@ export function writeConfig( // Backup existing config before overwriting if (fs.existsSync(filePath)) { const backupPath = filePath + ".backup"; - fs.copyFileSync(filePath, backupPath); - fs.chmodSync(backupPath, 0o600); + durableCopyFile(filePath, backupPath); } atomicWriteFile(filePath, JSON.stringify(nextConfig, null, 2) + "\n"); diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index 6f84dacc9ec..36a859d12b2 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -1,3 +1,4 @@ +import fsSync from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -1029,8 +1030,13 @@ describe("worktree config repair", () => { delete process.env.PORT; delete process.env.DATABASE_URL; + const open = vi.spyOn(fsSync, "openSync"); + const sync = vi.spyOn(fsSync, "fsyncSync"); maybePersistWorktreeRuntimePorts({ serverPort: 3103, databasePort: 54335 }); + expect(open).toHaveBeenCalledWith(paperclipDir, "r"); + expect(sync).toHaveBeenCalled(); + const writtenConfig = JSON.parse(await fs.readFile(configPath, "utf8")); expect(writtenConfig).toMatchObject({ topLevelExtension: { enabled: true }, diff --git a/server/src/worktree-config.ts b/server/src/worktree-config.ts index df8073c8064..de999134bdd 100644 --- a/server/src/worktree-config.ts +++ b/server/src/worktree-config.ts @@ -324,6 +324,21 @@ function atomicWriteFile(filePath: string, contents: string): void { fs.closeSync(fileDescriptor); fileDescriptor = null; fs.renameSync(temporaryPath, filePath); + let directoryDescriptor: number | null = null; + try { + directoryDescriptor = fs.openSync(path.dirname(filePath), "r"); + fs.fsyncSync(directoryDescriptor); + } catch (error) { + const code = error instanceof Error && "code" in error ? error.code : null; + if ( + process.platform !== "win32" || + !["EACCES", "EINVAL", "EISDIR", "ENOTSUP", "EPERM"].includes(String(code)) + ) { + throw error; + } + } finally { + if (directoryDescriptor !== null) fs.closeSync(directoryDescriptor); + } return; } catch (error) { if (fileDescriptor !== null) fs.closeSync(fileDescriptor);