fix(files): classify what copy follows through a leaf symlink

preserveSymlink keeps the leaf on purpose so rename and delete act on the
directory entry, but copyFile reads and writes *through* it. A worktree symlink
`hook-link -> .git/hooks/pre-commit` classified as an ordinary path, and the
copy then read the real hook — exfiltrating hooks and .git/config into the
working tree.

Classify the canonicalized leaf for operands of link-following operations, i.e.
both ends of copy. rename and delete keep leaf-preserving classification so
removing a symlink itself still works. Fail closed when the leaf exists but
cannot be canonicalized; a not-yet-created path has no link to follow.

The destination half was already blocked by COPYFILE_EXCL, which is the only
thing that blocked it; classifying it too removes that dependency.
This commit is contained in:
Merge Sim
2026-08-31 18:32:32 -07:00
parent 7c096be48d
commit 9b8d5cafa5
3 changed files with 147 additions and 6 deletions
@@ -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', () => {
+6 -2
View File
@@ -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).
@@ -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<string> {
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<string> {
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)
}
}