diff --git a/src/main/claude/claude-folder-trust-file-wsl-guest.test.ts b/src/main/claude/claude-folder-trust-file-wsl-guest.test.ts new file mode 100644 index 00000000000..e831b7c08ee --- /dev/null +++ b/src/main/claude/claude-folder-trust-file-wsl-guest.test.ts @@ -0,0 +1,133 @@ +import type * as NodeFs from 'node:fs' +import { + chmodSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + realpathSync, + rmSync, + statSync, + writeFileSync +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { WslResult, WslSpec } from '../wsl/wsl-runner' +import { grantClaudeFolderTrust } from './claude-folder-trust-file' + +const { DISTRO, GUEST_PREFIX, guest } = vi.hoisted(() => ({ + DISTRO: 'Ubuntu-24.04', + GUEST_PREFIX: '//wsl.localhost/Ubuntu-24.04', + guest: { root: '' } +})) + +function toGuestDisk(path: string): string { + return path.startsWith(GUEST_PREFIX) ? `${guest.root}${path.slice(GUEST_PREFIX.length)}` : path +} + +// Why: models Windows Node over \\wsl.localhost: a synthetic 0o666 mode, chmod that cannot reach +// the guest bits, and new files at the guest's default 0644 whatever mode was requested. +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + const onGuest = (path: unknown): path is string => + typeof path === 'string' && path.startsWith(GUEST_PREFIX) + const disk = (path: string): string => `${guest.root}${path.slice(GUEST_PREFIX.length)}` + const windowsView = (stats: NodeFs.Stats): NodeFs.Stats => + Object.assign(stats, { mode: (stats.mode & ~0o777) | 0o666 }) + return { + ...actual, + existsSync: (path: string) => actual.existsSync(onGuest(path) ? disk(path) : path), + lstatSync: (path: string) => + onGuest(path) ? windowsView(actual.lstatSync(disk(path))) : actual.lstatSync(path), + statSync: (path: string) => + onGuest(path) ? windowsView(actual.statSync(disk(path))) : actual.statSync(path), + realpathSync: (path: string) => + onGuest(path) + ? `${GUEST_PREFIX}${actual.realpathSync(disk(path)).slice(guest.root.length)}` + : actual.realpathSync(path), + readFileSync: (path: string, options: BufferEncoding) => + actual.readFileSync(onGuest(path) ? disk(path) : path, options), + writeFileSync: (path: string, data: string, options?: NodeFs.WriteFileOptions) => { + if (!onGuest(path)) { + return actual.writeFileSync(path, data, options) + } + const created = !actual.existsSync(disk(path)) + const flag = typeof options === 'object' && options ? options.flag : undefined + actual.writeFileSync(disk(path), data, { flag }) + if (created) { + actual.chmodSync(disk(path), 0o644) + } + }, + chmodSync: (path: string, mode: number) => + onGuest(path) ? undefined : actual.chmodSync(path, mode), + renameSync: (from: string, to: string) => + actual.renameSync(onGuest(from) ? disk(from) : from, onGuest(to) ? disk(to) : to), + rmSync: (path: string, options?: NodeFs.RmOptions) => + actual.rmSync(onGuest(path) ? disk(path) : path, options) + } +}) + +vi.mock('proper-lockfile', () => ({ + lock: vi.fn(async () => async () => {}) +})) + +const runWslProcess = vi.hoisted(() => vi.fn<(spec: WslSpec) => Promise>()) +vi.mock('../wsl/wsl-runner', () => ({ runWslProcess })) + +/** Runs `chmod --reference= -- ` as the guest would. */ +async function guestChmod(spec: WslSpec): Promise { + const [reference, separator, target] = spec.args ?? [] + expect(spec).toMatchObject({ distro: DISTRO, loginPath: 'none', program: 'chmod' }) + expect(separator).toBe('--') + const from = `${guest.root}${reference.replace(/^--reference=/, '')}` + chmodSync(`${guest.root}${target}`, statSync(from).mode & 0o7777) + return { environmentResolved: true, code: 0, stdout: '', stderr: '', timedOut: false } +} + +const originalPlatform = process.platform + +describe.skipIf(originalPlatform === 'win32')('grantClaudeFolderTrust on a WSL guest file', () => { + const guestFile = `${GUEST_PREFIX}/home/u/.claude.json` + + beforeEach(() => { + guest.root = realpathSync(mkdtempSync(join(tmpdir(), 'orca-claude-trust-guest-'))) + mkdirSync(join(guest.root, 'home/u'), { recursive: true }) + writeFileSync(toGuestDisk(guestFile), JSON.stringify({ theme: 'dark' })) + chmodSync(toGuestDisk(guestFile), 0o600) + runWslProcess.mockReset().mockImplementation(guestChmod) + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + rmSync(guest.root, { recursive: true, force: true }) + }) + + it("keeps the guest file's owner-only mode through the rewrite", async () => { + await expect( + grantClaudeFolderTrust({ configFile: guestFile, folderKeys: ['/home/u/wt'] }) + ).resolves.toBe('granted') + expect(statSync(toGuestDisk(guestFile)).mode & 0o777).toBe(0o600) + expect(JSON.parse(readFileSync(toGuestDisk(guestFile), 'utf-8'))).toEqual({ + theme: 'dark', + projects: { '/home/u/wt': { hasTrustDialogAccepted: true } } + }) + }) + + it('leaves the file untouched when the guest cannot copy its mode', async () => { + runWslProcess.mockResolvedValue({ + environmentResolved: true, + code: 1, + stdout: '', + stderr: "chmod: unrecognized option '--reference'", + timedOut: false + }) + await expect( + grantClaudeFolderTrust({ configFile: guestFile, folderKeys: ['/home/u/wt'] }) + ).rejects.toThrow(/guest mode/) + expect(readFileSync(toGuestDisk(guestFile), 'utf-8')).toBe(JSON.stringify({ theme: 'dark' })) + expect(statSync(toGuestDisk(guestFile)).mode & 0o777).toBe(0o600) + expect(readdirSync(join(guest.root, 'home/u'))).toEqual(['.claude.json']) + }) +}) diff --git a/src/main/claude/claude-folder-trust-file.test.ts b/src/main/claude/claude-folder-trust-file.test.ts index 1c876be1457..456e8938cb6 100644 --- a/src/main/claude/claude-folder-trust-file.test.ts +++ b/src/main/claude/claude-folder-trust-file.test.ts @@ -4,6 +4,7 @@ import { lstatSync, mkdirSync, mkdtempSync, + readdirSync, readFileSync, realpathSync, rmSync, @@ -178,6 +179,8 @@ describe('grantClaudeFolderTrust', () => { ) expect(readConfig(file)).toEqual({}) expect(existsSync(lockDir)).toBe(true) + // Why: the replacement file is created before the lock, so a skipped write must remove it. + expect(readdirSync(root).sort()).toEqual(['.claude.json', '.claude.json.lock']) }) it('updates a symlinked config through its target and keeps the link', async () => { diff --git a/src/main/claude/claude-folder-trust-file.ts b/src/main/claude/claude-folder-trust-file.ts index ca35351eeb0..5ec9bccebcc 100644 --- a/src/main/claude/claude-folder-trust-file.ts +++ b/src/main/claude/claude-folder-trust-file.ts @@ -15,6 +15,7 @@ import { lock } from 'proper-lockfile' import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' import { runKeyedSerializedOperation } from '../cli/keyed-promise-queue' import { parseWslUncPath } from '../../shared/wsl-paths' +import { runWslProcess } from '../wsl/wsl-runner' import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth/runtime-auth-types' export type ClaudeTrustPathStyle = 'posix' | 'win32' @@ -144,20 +145,52 @@ function readConfigObject(target: string): Record | null { } } -function writeConfigAtomically(target: string, config: Record): void { - const mode = statSync(target).mode & 0o777 - const tmpPath = `${target}.orca-trust-${randomUUID()}.tmp` +type ReplacementFile = { target: string; path: string } + +/** + * Creates an empty file beside `target` that already has `target`'s permission bits, so + * renaming it over `target` can never widen them. + */ +async function createReplacementFile(target: string): Promise { + const suffix = `.orca-trust-${randomUUID()}.tmp` + const path = `${target}${suffix}` + const guestFile = parseWslUncPath(target) try { - writeFileSync(tmpPath, `${JSON.stringify(config, null, 2)}\n`, { encoding: 'utf-8', mode }) - if (process.platform !== 'win32') { - // Why: umask may narrow the requested mode; the replacement must match the original exactly. - chmodSync(tmpPath, mode) + if (guestFile) { + // Why: Windows sees a synthetic 0o666 for a guest file and cannot set its bits, and the + // guest gives a file made through \\wsl.localhost its default 0644; only the guest can copy them. + writeFileSync(path, '', { flag: 'wx' }) + const copied = await runWslProcess({ + distro: guestFile.distro, + loginPath: 'none', + program: 'chmod', + args: [`--reference=${guestFile.linuxPath}`, '--', `${guestFile.linuxPath}${suffix}`] + }) + if (copied.code !== 0) { + throw new Error(`could not copy the guest mode of ${target}: ${copied.stderr.trim()}`) + } + } else { + const mode = statSync(target).mode & 0o777 + writeFileSync(path, '', { flag: 'wx', mode }) + if (process.platform !== 'win32') { + // Why: umask may narrow the requested mode; the replacement must match the original exactly. + chmodSync(path, mode) + } } - renameFileWithWindowsRetry(tmpPath, target) } catch (error) { - rmSync(tmpPath, { force: true }) + rmSync(path, { force: true }) throw error } + return { target, path } +} + +function replaceConfig(replacement: ReplacementFile, config: Record): void { + // Why r+: reopening without create/truncate keeps the mode the replacement was given. + writeFileSync(replacement.path, `${JSON.stringify(config, null, 2)}\n`, { + encoding: 'utf-8', + flag: 'r+' + }) + renameFileWithWindowsRetry(replacement.path, replacement.target) } /** @@ -188,37 +221,49 @@ async function grantClaudeFolderTrustNow(args: { return planned === 'refuse' ? 'unreadable' : 'unchanged' } - let release: () => Promise + // Why before the lock: a WSL guest's mode takes a guest process to copy, and Claude's + // lock should stay held only for the synchronous read → rename below. + const replacement = await createReplacementFile(probe.path) try { - release = await lock(args.configFile, { - // Why: Claude locks the literal `.lock`, not a realpath'd one. - lockfilePath: `${args.configFile}.lock`, - realpath: false, - stale: NEVER_STALE_MS, - retries: LOCK_RETRIES, - onCompromised: () => {} - }) - } catch { - return 'locked' - } - try { - // Why: read → rename stays synchronous so Orca's own synchronous auth writer to - // this file cannot interleave and lose an update. - const current = readConfigAt(resolveConfigTarget(args.configFile)) - if (typeof current === 'string') { - return current + let release: () => Promise + try { + release = await lock(args.configFile, { + // Why: Claude locks the literal `.lock`, not a realpath'd one. + lockfilePath: `${args.configFile}.lock`, + realpath: false, + stale: NEVER_STALE_MS, + retries: LOCK_RETRIES, + onCompromised: () => {} + }) + } catch { + return 'locked' } - const change = applyClaudeFolderTrust(current.config, args.folderKeys) - if (change.kind === 'refuse') { - return 'unreadable' + try { + // Why: read → rename stays synchronous so Orca's own synchronous auth writer to + // this file cannot interleave and lose an update. + const current = readConfigAt(resolveConfigTarget(args.configFile)) + if (typeof current === 'string') { + return current + } + // Why: a link retargeted since the replacement copied its mode means Claude asks. + if (current.path !== replacement.target) { + return 'unreadable' + } + const change = applyClaudeFolderTrust(current.config, args.folderKeys) + if (change.kind === 'refuse') { + return 'unreadable' + } + if (change.kind === 'unchanged') { + return 'unchanged' + } + replaceConfig(replacement, change.config) + return 'granted' + } finally { + await release().catch(() => {}) } - if (change.kind === 'unchanged') { - return 'unchanged' - } - writeConfigAtomically(current.path, change.config) - return 'granted' } finally { - await release().catch(() => {}) + // Why: a no-op after the rename; otherwise the unused replacement must not linger. + rmSync(replacement.path, { force: true }) } }