From 81dd2fe59005db5a21c7227971abca98532eff3f Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:46:35 -0700 Subject: [PATCH] fix(worktrees): an unknown host home refuses the recursive delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveWorktreeRemovalHome` answers `{ kind: 'executionHost', homePath: null }` whenever the relay session has left `activeSessions` or never resolved its host env — an ordinary disconnect, and the default resolver before the registration side effect runs at all. `resolveGuardHomePath` then correctly declines to substitute the client's home, but `isHomeDirectoryRemovalPath` fell through to path SHAPES only, and shapes do not know `/srv/homes/alice`, `/export/home/alice` or `D:\Profiles\bob`. Injected: the host-home fixture with the resolver answering null. Measured: `removeRuntimeUnregisteredWorktree` called `deletePath('/srv/homes/alice', true)` — a recursive delete of the host's own home, the exact path the two shipped tests pin as refused when the resolver does answer. "Could not ask the host where its home is" is unverifiable, so the two recursive-delete gates now fail closed on it. `isDangerousWorktreeRemovalPath` deliberately does not consult it: it also fences the registered `git worktree remove` path, which must stay usable mid-reconnect. The stated cost is pinned too: an ordinary orphan is also declined while the home is unknown. Declining is recoverable — the row survives and the next connected removal proceeds — and a recursive delete of the wrong directory is not. --- ...worktrees-orphan-directory-cleanup.test.ts | 10 ++++++- ...istered-worktree-removal-host-home.test.ts | 28 +++++++++++++++++++ src/main/worktree-removal-safety.ts | 22 +++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/main/ipc/worktrees-orphan-directory-cleanup.test.ts b/src/main/ipc/worktrees-orphan-directory-cleanup.test.ts index 954a994940d..8b8a89ca450 100644 --- a/src/main/ipc/worktrees-orphan-directory-cleanup.test.ts +++ b/src/main/ipc/worktrees-orphan-directory-cleanup.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { lstat, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -15,6 +15,7 @@ import { import { handlers, mainWindow, setupWorktreeHandlers, store } from './worktrees-test-harness' import { makeWorktreeMeta, mockKnownFeatureWorktree } from './worktrees-test-fixtures' import type { WorktreeRuntimeStub } from './worktrees-test-runtime-stub' +import { setWorktreeRemovalSshHostHomeResolver } from '../worktree-removal-execution-host-route' vi.mock('electron', async () => (await import('./worktrees-test-module-mocks')).electronModuleMock() @@ -105,6 +106,10 @@ describe('registerWorktreeHandlers', () => { runtimeStub = setupWorktreeHandlers() }) + afterEach(() => { + setWorktreeRemovalSshHostHomeResolver(() => null) + }) + it('reports already-missing unregistered delete paths before teardown, hooks, or git removal', async () => { mockKnownFeatureWorktree('/workspace/real-feature') getEffectiveHooksMock.mockReturnValue({ @@ -421,7 +426,10 @@ describe('registerWorktreeHandlers', () => { } }) + // The recursive-delete gate now requires the execution host to have reported its `$HOME`, so + // this test has to establish it before it can reach the symlink check it is actually about. it('refuses SSH orphan cleanup when remote .git is a symlink', async () => { + setWorktreeRemovalSshHostHomeResolver(() => '/remote/home/alice') const repo = { id: 'repo-ssh-symlink-git', path: '/remote/repo', diff --git a/src/main/runtime/runtime-unregistered-worktree-removal-host-home.test.ts b/src/main/runtime/runtime-unregistered-worktree-removal-host-home.test.ts index 259209b9b04..3248680f023 100644 --- a/src/main/runtime/runtime-unregistered-worktree-removal-host-home.test.ts +++ b/src/main/runtime/runtime-unregistered-worktree-removal-host-home.test.ts @@ -77,6 +77,34 @@ describe('removeRuntimeUnregisteredWorktree against an SSH host home', () => { expect(fsProvider.deletePath).not.toHaveBeenCalled() }) + // The resolver answers null whenever the relay session left `activeSessions` or never resolved + // its host env — an ordinary disconnect. That skips the containment check entirely and leaves + // only path SHAPES, which do not know `/srv/homes/alice`. "Could not ask the host where its home + // is" is unverifiable, so the recursive delete has to fail closed. + it('refuses the recursive delete when the host never reported its home', async () => { + setWorktreeRemovalSshHostHomeResolver(() => null) + const fsProvider = provenOrphanFilesystem(HOST_HOME) + + await expect( + removeRuntimeUnregisteredWorktree(removalArgs(HOST_HOME, fsProvider)) + ).rejects.toThrow(`Refusing to delete unregistered worktree path: ${HOST_HOME}`) + expect(fsProvider.deletePath).not.toHaveBeenCalled() + }) + + // The stated cost of failing closed: an ordinary orphan is also declined until the host answers. + // Declining is recoverable — the row survives and the next connected removal proceeds — while a + // recursive delete of the wrong directory is not. + it('declines an ordinary orphan too while the home is unknown', async () => { + setWorktreeRemovalSshHostHomeResolver(() => null) + const worktreePath = `${HOST_HOME}/workspaces/leftover` + const fsProvider = provenOrphanFilesystem(worktreePath) + + await expect( + removeRuntimeUnregisteredWorktree(removalArgs(worktreePath, fsProvider)) + ).rejects.toThrow(`Refusing to delete unregistered worktree path: ${worktreePath}`) + expect(fsProvider.deletePath).not.toHaveBeenCalled() + }) + it('still deletes a proven orphan under that host home', async () => { setWorktreeRemovalSshHostHomeResolver(() => HOST_HOME) const worktreePath = `${HOST_HOME}/workspaces/leftover` diff --git a/src/main/worktree-removal-safety.ts b/src/main/worktree-removal-safety.ts index 129a44cfbef..8254c731661 100644 --- a/src/main/worktree-removal-safety.ts +++ b/src/main/worktree-removal-safety.ts @@ -128,6 +128,22 @@ export function assertWorktreeDoesNotContainRegisteredWorktree( } } +/** + * Whether the home guard was able to ask the machine that executes the removal. + * + * An execution host with no reported `$HOME` is `unknown`, not safe: the containment check is + * skipped entirely and only path SHAPES remain, and shapes do not know `/srv/homes/alice`, + * `/export/home/alice` or `D:\Profiles\bob`. The resolver answers `null` whenever the relay + * session is gone from `activeSessions` or never resolved its host env, which is an ordinary + * disconnect — and loss of contact is not permission to recursively delete + * (docs/reference/ssh-execution-boundary.md). Only the recursive-delete gates consult this; + * `isDangerousWorktreeRemovalPath` deliberately does not, because it also fences the registered + * `git worktree remove` path, which must stay usable while a session is mid-reconnect. + */ +function homeAuthorityAnswered(home: WorktreeRemovalHomeAuthority): boolean { + return home.kind !== 'executionHost' || Boolean(home.homePath) +} + export async function canSafelyRemoveOrphanedWorktreeDirectory( worktreePath: string, repoPath: string, @@ -135,6 +151,9 @@ export async function canSafelyRemoveOrphanedWorktreeDirectory( statPath: StatPath = lstat, readPath: ReadPath = (path) => readFile(path, 'utf8') ): Promise { + if (!homeAuthorityAnswered(home)) { + return false + } if (isDangerousWorktreeRemovalPath(worktreePath, repoPath, home)) { return false } @@ -185,6 +204,9 @@ export async function canCleanupUnregisteredOrcaLeftoverDirectory(args: { if (!hasCurrentOrcaCreationProvenance(args.meta) && !hasLegacyOrcaCreationEvidence(args.meta)) { return false } + if (!homeAuthorityAnswered(args.home)) { + return false + } if ( isDangerousWorktreeRemovalPath(args.worktreePath, args.repo.path, args.home) ||