From d913c44e910eced88f4556919bde10a9fbf64dfc Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 15 Sep 2026 21:24:16 -0700 Subject: [PATCH] fix: keep legacy link cleanup inside the workspace --- src/main/git/worktree-copy-selection.test.ts | 10 +++++++ src/main/git/worktree-copy-selection.ts | 9 ++++++- .../git/worktree-symlink-detection.test.ts | 6 +++++ src/main/git/worktree-symlink-detection.ts | 26 +++++-------------- src/main/ipc/worktree-symlinks.test.ts | 10 +++++++ src/main/ipc/worktree-symlinks.ts | 1 + 6 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/main/git/worktree-copy-selection.test.ts b/src/main/git/worktree-copy-selection.test.ts index 126f7822811..42526a61b92 100644 --- a/src/main/git/worktree-copy-selection.test.ts +++ b/src/main/git/worktree-copy-selection.test.ts @@ -32,6 +32,16 @@ async function fixture() { } describe('repository and project copy selection', () => { + it('reports project paths beyond the manifest limit', async () => { + const { source } = await fixture() + await writeFile( + join(source, '.worktreeinclude'), + Array.from({ length: 1002 }, (_, i) => `absent-${i}`).join('\n') + ) + expect((await resolveWorktreeCopySelection(source, [])).notices).toContain( + '2 .worktreeinclude entries exceeded the 1,000-path limit and were skipped.' + ) + }) it('copies the union privately without CoW, collapses children, and preserves existing destinations', async () => { const { source, target } = await fixture() expect( diff --git a/src/main/git/worktree-copy-selection.ts b/src/main/git/worktree-copy-selection.ts index 2aa53fefb9f..bc1b16b52cf 100644 --- a/src/main/git/worktree-copy-selection.ts +++ b/src/main/git/worktree-copy-selection.ts @@ -54,7 +54,9 @@ export async function resolveWorktreeCopySelection( ) } const invalidProject = project.filter((path) => !isWorktreeCopyPath(path)) - project = project.filter(isWorktreeCopyPath).slice(0, 1000) + project = project.filter(isWorktreeCopyPath) + const excessProjectPaths = Math.max(0, project.length - 1000) + project = project.slice(0, 1000) const candidates = [...new Set([...personal, ...project])] const existing = await mapWithConcurrency(candidates, 8, async (path) => { try { @@ -74,6 +76,11 @@ export async function resolveWorktreeCopySelection( invalidProject.length > 0 ? [`${invalidProject.length} unsupported .worktreeinclude entries were skipped.`] : [] + if (excessProjectPaths) { + notices.push( + `${excessProjectPaths} .worktreeinclude entries exceeded the 1,000-path limit and were skipped.` + ) + } notices.push( ...missing.slice(0, 5).map((path) => { const name = path.length > 160 ? `${path.slice(0, 157)}…` : path diff --git a/src/main/git/worktree-symlink-detection.test.ts b/src/main/git/worktree-symlink-detection.test.ts index 4f96d9aa081..dfa50b5aeb2 100644 --- a/src/main/git/worktree-symlink-detection.test.ts +++ b/src/main/git/worktree-symlink-detection.test.ts @@ -5,6 +5,12 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { findExistingWorktreeSymlinkPaths, getSafeRelativePath } from './worktree-symlink-detection' describe('getSafeRelativePath', () => { + it.each(['.//cache', '.\\/cache', './\\cache', '/././/cache', './cache//./'])( + 'keeps %s relative after normalization', + (path) => { + expect(getSafeRelativePath(path)).toEqual({ safe: true, rel: 'cache' }) + } + ) // The only case in this file that binds the production change: every one of // these is admitted by at least one host's `path.isAbsolute` — the // drive-relative spellings are admitted by *every* host's, Windows included. diff --git a/src/main/git/worktree-symlink-detection.ts b/src/main/git/worktree-symlink-detection.ts index dc45df9f3a0..9794f924abb 100644 --- a/src/main/git/worktree-symlink-detection.ts +++ b/src/main/git/worktree-symlink-detection.ts @@ -9,33 +9,19 @@ import { resolveWorktreeHostPath } from '../../shared/git-metadata-path' export type SafeRelativePathResult = { safe: true; rel: string } | { safe: false } -// Why a regex rather than `path.isAbsolute`: the strip below already removed -// every leading `/` and `\`, so the only rooted spelling that can still reach -// the guard is a Windows drive designator — and `win32.isAbsolute` misses the -// drive-RELATIVE form (`C:foo`), which `win32.resolve` still resolves against -// that drive's current directory instead of the worktree root. +// Drive-relative paths (C:foo) must also stay outside repository-relative input. const WINDOWS_DRIVE_DESIGNATOR = /^[a-zA-Z]:/ export function getSafeRelativePath(rawPath: string): SafeRelativePathResult { - // Why: strip leading separators (both `/` and `\`) before the guard so - // Windows-style input like `\foo` is normalized the same way POSIX `/foo` - // is, and the traversal check below sees the already-relative form. - const rel = rawPath + const parts = rawPath .trim() .replace(/\\/g, '/') - .replace(/^\/+/, '') - .replace(/^\.\//, '') - .replace(/\/+$/, '') - // Why: split on both separators so a Windows-authored `..\escape` is - // rejected the same way POSIX `../escape` is; the split catches relative - // backslash traversal that `.split('/')` would otherwise miss. - // Why the drive check runs on every host: the same entry — per-user Shared - // Paths setting or repo `orca.yaml` — is evaluated on every host Orca runs - // on, so the verdict must not depend on which one is asking. - if (!rel || WINDOWS_DRIVE_DESIGNATOR.test(rel) || rel.split(/[\\/]/).includes('..')) { + .split('/') + .filter((part) => part && part !== '.') + if (!parts[0] || WINDOWS_DRIVE_DESIGNATOR.test(parts[0]) || parts.includes('..')) { return { safe: false } } - return { safe: true, rel } + return { safe: true, rel: parts.join('/') } } export type WorktreeSymlinkDetectionOptions = { diff --git a/src/main/ipc/worktree-symlinks.test.ts b/src/main/ipc/worktree-symlinks.test.ts index 19801e96636..0f1335a9bd5 100644 --- a/src/main/ipc/worktree-symlinks.test.ts +++ b/src/main/ipc/worktree-symlinks.test.ts @@ -938,6 +938,16 @@ describe('removeWorktreeSymlinks', () => { expect(existsSync(join(worktree, '.env'))).toBe(true) }) + it('never removes links outside the worktree through ambiguous paths or linked parents', async () => { + writeFileSync(join(primary, 'value'), 'keep') + const outside = join(primary, 'alias') + symlinkSync(join(primary, 'value'), outside) + symlinkSync(primary, join(worktree, 'parent'), 'junction') + await removeWorktreeLinkedPaths(worktree, [`.//${outside}`, 'parent/alias']) + expect(lstatSync(outside).isSymbolicLink()).toBe(true) + expect(readFileSync(outside, 'utf8')).toBe('keep') + }) + it('ignores missing entries', async () => { await removeWorktreeSymlinks(worktree, ['.env', 'node_modules']) expect(error).not.toHaveBeenCalled() diff --git a/src/main/ipc/worktree-symlinks.ts b/src/main/ipc/worktree-symlinks.ts index 79082ce9556..377020085d4 100644 --- a/src/main/ipc/worktree-symlinks.ts +++ b/src/main/ipc/worktree-symlinks.ts @@ -374,6 +374,7 @@ export async function removeWorktreeLinkedPaths( try { const s = await lstat(target) if (s.isSymbolicLink()) { + await assertWorktreeMaterializationTarget(worktreePath, target) await unlink(target) } } catch (error) {