From 67ca905df455fe2a8852ab6afbd756de85879ea6 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:36:22 -0700 Subject: [PATCH] fix(files): fsync read-only files on POSIX --- .../orca-profiles/profile-index-store.test.ts | 22 ++++++++ src/shared/secure-file-fsync-flags.test.ts | 51 ++++++++++++++++--- src/shared/secure-file.ts | 2 +- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/main/orca-profiles/profile-index-store.test.ts b/src/main/orca-profiles/profile-index-store.test.ts index a2d3446a99c..128dc56cea3 100644 --- a/src/main/orca-profiles/profile-index-store.test.ts +++ b/src/main/orca-profiles/profile-index-store.test.ts @@ -3,6 +3,7 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync import { join } from 'node:path' import { tmpdir } from 'node:os' import { + createDefaultLocalOrcaProfile, DEFAULT_LOCAL_ORCA_PROFILE_ID, DEFAULT_LOCAL_ORCA_PROFILE_NAME, ORCA_PROFILE_INDEX_SCHEMA_VERSION, @@ -154,6 +155,27 @@ describe('profile index store', () => { expect(() => setActiveOrcaProfile('missing-profile')).toThrow('unknown_orca_profile') }) + const posixIt = process.platform === 'win32' ? it.skip : it + posixIt('writes a fresh profile index when umask removes owner-write permission', async () => { + const store = await loadProfileIndexStore() + const indexPath = store.getOrcaProfileIndexPath() + const profile = createDefaultLocalOrcaProfile(1) + const index: OrcaProfileIndex = { + schemaVersion: ORCA_PROFILE_INDEX_SCHEMA_VERSION, + activeProfileId: profile.id, + profiles: [profile] + } + const originalUmask = process.umask(0o200) + + try { + expect(() => store.writeProfileIndex(indexPath, index)).not.toThrow() + } finally { + process.umask(originalUmask) + } + + expect(readJson(indexPath)).toEqual(index) + }) + it('recovers a corrupted profile index from the backup copy', async () => { const store = await loadProfileIndexStore() store.ensureActiveOrcaProfile() diff --git a/src/shared/secure-file-fsync-flags.test.ts b/src/shared/secure-file-fsync-flags.test.ts index 1c167bffdad..15505151aef 100644 --- a/src/shared/secure-file-fsync-flags.test.ts +++ b/src/shared/secure-file-fsync-flags.test.ts @@ -1,23 +1,33 @@ -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import type * as NodeFs from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' -const openedPaths = vi.hoisted(() => [] as { path: string; flags: string | number }[]) +const openedPaths = vi.hoisted( + () => [] as { path: string; flags: string | number; ownerWritable: boolean }[] +) vi.mock('node:fs', async () => { const actual = await vi.importActual('node:fs') return { ...actual, openSync: (path: NodeFs.PathLike, flags: string | number, mode?: NodeFs.Mode) => { - openedPaths.push({ path: String(path), flags }) + openedPaths.push({ + path: String(path), + flags, + ownerWritable: Boolean(actual.statSync(path).mode & 0o200) + }) return actual.openSync(path, flags, mode) } } }) -import { bestEffortFsyncDirectorySync, fsyncFileSync } from './secure-file' +import { + bestEffortFsyncDirectorySync, + fsyncFileSync, + writeDurableSecureJsonFile +} from './secure-file' const createdPaths: string[] = [] @@ -29,7 +39,8 @@ afterEach(() => { }) describe('secure file fsync flags', () => { - it('opens files read/write before fsync', () => { + const windowsIt = process.platform === 'win32' ? it : it.skip + windowsIt('opens files read/write before fsync on Windows', () => { const directory = mkdtempSync(join(tmpdir(), 'orca-file-fsync-')) createdPaths.push(directory) const path = join(directory, 'record.json') @@ -37,16 +48,42 @@ describe('secure file fsync flags', () => { fsyncFileSync(path) - expect(openedPaths).toEqual([{ path, flags: 'r+' }]) + expect(openedPaths).toMatchObject([{ path, flags: 'r+' }]) }) const posixIt = process.platform === 'win32' ? it.skip : it + posixIt('fsyncs files without owner-write permission', () => { + const directory = mkdtempSync(join(tmpdir(), 'orca-file-fsync-read-only-')) + createdPaths.push(directory) + const path = join(directory, 'record.json') + writeFileSync(path, '{}') + chmodSync(path, 0o400) + + expect(() => fsyncFileSync(path)).not.toThrow() + expect(openedPaths).toEqual([{ path, flags: 'r', ownerWritable: false }]) + }) + + posixIt('durably writes when umask removes owner-write from the temporary file', () => { + const directory = mkdtempSync(join(tmpdir(), 'orca-durable-file-fsync-read-only-')) + createdPaths.push(directory) + const path = join(directory, 'record.json') + const originalUmask = process.umask(0o200) + + try { + expect(() => writeDurableSecureJsonFile(path, { ok: true })).not.toThrow() + } finally { + process.umask(originalUmask) + } + + expect(openedPaths[0]).toMatchObject({ flags: 'r', ownerWritable: false }) + }) + posixIt('opens directories read-only before fsync', () => { const directory = mkdtempSync(join(tmpdir(), 'orca-directory-fsync-')) createdPaths.push(directory) bestEffortFsyncDirectorySync(directory) - expect(openedPaths).toEqual([{ path: directory, flags: 'r' }]) + expect(openedPaths).toMatchObject([{ path: directory, flags: 'r' }]) }) }) diff --git a/src/shared/secure-file.ts b/src/shared/secure-file.ts index 7b2f646c540..639630c7c65 100644 --- a/src/shared/secure-file.ts +++ b/src/shared/secure-file.ts @@ -144,7 +144,7 @@ function fsyncPathSync(path: string, flags: 'r' | 'r+'): void { export function fsyncFileSync(path: string): void { // FlushFileBuffers requires a write-capable handle on Windows. - fsyncPathSync(path, 'r+') + fsyncPathSync(path, process.platform === 'win32' ? 'r+' : 'r') } export function bestEffortFsyncDirectorySync(directory: string): void {