diff --git a/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts b/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts index e79257225aa..93780462682 100644 --- a/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts +++ b/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts @@ -601,6 +601,112 @@ describe('resolved-path classification', () => { }) }) +// Why: `preserveSymlink` keeps the leaf on purpose so rename/delete act on the link itself, but +// copyFile reads and writes THROUGH the leaf, so for copy the link's target is the real object. +describe.skipIf(process.platform === 'win32')( + 'files.copy refuses a .git aliased through a leaf symlink', + () => { + beforeEach(async () => { + await buildRepo() + await mkdir(join(repoPath, '.git', 'hooks'), { recursive: true }) + await writeFile( + join(repoPath, '.git', 'hooks', 'pre-commit'), + '#!/bin/sh\nreal hook\n', + 'utf-8' + ) + await symlink( + join(repoPath, '.git', 'hooks', 'pre-commit'), + join(repoPath, 'hook-link'), + 'file' + ) + await symlink(join(repoPath, '.git', 'config'), join(repoPath, 'config-link'), 'file') + }) + + afterEach(async () => { + await rm(repoPath, { recursive: true, force: true }) + }) + + it('refuses a leaf symlink to a hook as the source', async () => { + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'hook-link', + destinationRelativePath: 'stolen' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, 'stolen'))).toBe(false) + }) + + it('refuses a leaf symlink to .git/config as the source', async () => { + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'config-link', + destinationRelativePath: 'stolen-config' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, 'stolen-config'))).toBe(false) + }) + + // Why also the destination: COPYFILE_EXCL happens to block this today, and it is the only thing + // that does. Classifying it too keeps the guard from depending on that flag staying put. + it('refuses a leaf symlink into .git as the destination', async () => { + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'tracked.txt', + destinationRelativePath: 'hook-link' + }) + + expectRefused(response) + expect(await readFile(join(repoPath, '.git', 'hooks', 'pre-commit'), 'utf-8')).toBe( + '#!/bin/sh\nreal hook\n' + ) + }) + + // Why: a symlink loop makes realpath fail with ELOOP, not ENOENT — the leaf exists but what it + // points at is unknowable, so the copy is refused rather than attempted. + it('fails closed when the leaf cannot be canonicalized', async () => { + await symlink(join(repoPath, 'loop-b'), join(repoPath, 'loop-a'), 'file') + await symlink(join(repoPath, 'loop-a'), join(repoPath, 'loop-b'), 'file') + + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'loop-a', + destinationRelativePath: 'looped-copy' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, 'looped-copy'))).toBe(false) + }) + + it('still copies through a leaf symlink that stays in the working tree', async () => { + await symlink(join(repoPath, 'tracked.txt'), join(repoPath, 'plain-link'), 'file') + + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'plain-link', + destinationRelativePath: 'copied.txt' + }) + + expect(response.ok).toBe(true) + expect(await readFile(join(repoPath, 'copied.txt'), 'utf-8')).toBe('working tree content\n') + }) + + // Why: rename and delete act on the directory entry, never on what the link points at. + it('still renames and deletes the link itself, leaving the hook intact', async () => { + const renamed = await dispatchFileMethod('files.rename', { + oldRelativePath: 'hook-link', + newRelativePath: 'hook-link-moved' + }) + const deleted = await dispatchFileMethod('files.delete', { + relativePath: 'hook-link-moved', + recursive: false + }) + + expect(renamed.ok).toBe(true) + expect(deleted.ok).toBe(true) + expect(await readFile(join(repoPath, '.git', 'hooks', 'pre-commit'), 'utf-8')).toBe( + '#!/bin/sh\nreal hook\n' + ) + }) + } +) + // Why: the SSH branch returns before the local gate, so the canonical-path check never runs there. // The relative-path guard at the RPC boundary is the only thing covering it. describe('files.* RPCs refuse repository admin paths on the SSH branch', () => { diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index 612c1aef584..b905749f861 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -2082,11 +2082,15 @@ export class RuntimeFileCommands { } const store = this.host.requireStore() + // Why followsLeafSymlink: copyFile reads and writes *through* a leaf symlink, so the link's + // target is the object it touches — unlike rename/delete, which act on the entry itself. const sourcePath = await resolveAuthorizedMutablePath(sourceTarget.path, store, { - preserveSymlink: true + preserveSymlink: true, + followsLeafSymlink: true }) const destinationPath = await resolveAuthorizedMutablePath(destinationTarget.path, store, { - preserveSymlink: true + preserveSymlink: true, + followsLeafSymlink: true }) await mkdir(dirname(destinationPath), { recursive: true }) // Why: COPYFILE_EXCL preserves the no-clobber invariant of the local shell copy IPC (caller already deconflicts names). diff --git a/src/main/runtime/repository-admin-path-authorization.ts b/src/main/runtime/repository-admin-path-authorization.ts index 3f7eb8c0e98..089950a0952 100644 --- a/src/main/runtime/repository-admin-path-authorization.ts +++ b/src/main/runtime/repository-admin-path-authorization.ts @@ -1,10 +1,20 @@ +import { realpath } from 'node:fs/promises' import type { Store } from '../persistence' import { resolveAuthorizedPath, type ResolveAuthorizedPathOptions } from '../ipc/filesystem-auth' +import { isENOENT } from '../ipc/filesystem-path-containment' import { isRepositoryAdminPath, REPOSITORY_ADMIN_PATH_DENIED_MESSAGE } from '../../shared/repository-admin-path' +export type ResolveAuthorizedMutablePathOptions = ResolveAuthorizedPathOptions & { + /** + * The syscall reads or writes *through* a leaf symlink (copy does; rename and delete act on the + * directory entry instead). Set it so the link's target is classified as well. + */ + followsLeafSymlink?: boolean +} + /** * Authorizes a file-explorer mutation, then refuses repository admin state on the path the * filesystem will actually touch. @@ -19,11 +29,32 @@ import { export async function resolveAuthorizedMutablePath( targetPath: string, store: Store, - options: ResolveAuthorizedPathOptions = {} + options: ResolveAuthorizedMutablePathOptions = {} ): Promise { - const resolvedPath = await resolveAuthorizedPath(targetPath, store, options) - if (isRepositoryAdminPath(resolvedPath, process.platform === 'win32' ? 'win32' : 'posix')) { - throw new Error(REPOSITORY_ADMIN_PATH_DENIED_MESSAGE) + const { followsLeafSymlink, ...authorizationOptions } = options + const resolvedPath = await resolveAuthorizedPath(targetPath, store, authorizationOptions) + assertMutablePath(resolvedPath) + if (followsLeafSymlink) { + assertMutablePath(await canonicalLeaf(resolvedPath)) } return resolvedPath } + +function assertMutablePath(path: string): void { + if (isRepositoryAdminPath(path, process.platform === 'win32' ? 'win32' : 'posix')) { + throw new Error(REPOSITORY_ADMIN_PATH_DENIED_MESSAGE) + } +} + +async function canonicalLeaf(path: string): Promise { + try { + return await realpath(path) + } catch (error) { + // A path that does not exist yet has no link to follow; the caller creates a real file there. + if (isENOENT(error)) { + return path + } + // Fail closed: the leaf exists but cannot be canonicalized, so what it points at is unknown. + throw new Error(REPOSITORY_ADMIN_PATH_DENIED_MESSAGE) + } +}