diff --git a/src/main/ipc/worktrees-test-harness.ts b/src/main/ipc/worktrees-test-harness.ts index 8297c695374..6e1019d2150 100644 --- a/src/main/ipc/worktrees-test-harness.ts +++ b/src/main/ipc/worktrees-test-harness.ts @@ -11,7 +11,7 @@ import { resetSshProviderAuthorities } from '../ssh/ssh-provider-authority' import { createWorktreeRuntimeStub, type WorktreeRuntimeStub } from './worktrees-test-runtime-stub' import { handlers, mainWindow, store } from './worktrees-test-ipc-surface' import { configureMetadataPruningStoreMocks } from './worktrees-test-metadata-pruning-store' -import { resetWorktreeTestSshHostHome } from './worktrees-test-ssh-host-home' +import { resetWorktreeTestSshHostHome } from '../worktree-removal-test-ssh-host-home' import { ORIGINAL_PLATFORM, setPlatform, diff --git a/src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.ts index 678ea23eb1d..a532cd45f10 100644 --- a/src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.ts @@ -1,4 +1,6 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resetWorktreeTestSshHostHome } from '../../worktree-removal-test-ssh-host-home' + import { OrcaRuntimeService, SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV, @@ -30,6 +32,10 @@ import { syncSinglePty } from '../orca-runtime-test-fixtures.spec' +// Why: these fixtures register an SSH provider, which models a connected relay session — and a +// connected session has always read the host's `$HOME`. The removal guards refuse without it. +beforeEach(resetWorktreeTestSshHostHome) + describe('OrcaRuntimeService', () => { it('launches SSH setup terminals for runtime task-created worktrees', async () => { vi.mocked(listWorktrees).mockClear() diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts index 0c7f99da477..58c1998e2da 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts @@ -1,4 +1,6 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resetWorktreeTestSshHostHome } from '../../worktree-removal-test-ssh-host-home' + import { MOCK_GIT_WORKTREES, ORIGINAL_PLATFORM, @@ -45,6 +47,10 @@ import { } from '../orca-runtime-test-fixtures.spec' import { createWorktreeRemovalRuntime } from '../orca-runtime-test-scenario-builders.spec' +// Why: these fixtures register an SSH provider, which models a connected relay session — and a +// connected session has always read the host's `$HOME`. The removal guards refuse without it. +beforeEach(resetWorktreeTestSshHostHome) + describe('OrcaRuntimeService', () => { it('warns that a missing-repo removal only forgot the workspace', async () => { const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.ts index 941579fe645..a9edac0ede7 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.ts @@ -1,4 +1,6 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resetWorktreeTestSshHostHome } from '../../worktree-removal-test-ssh-host-home' + import { OrcaRuntimeService, assertWorktreeCleanForRemoval, @@ -24,7 +26,6 @@ import { writeFile } from '../orca-runtime-test-mocks.spec' import type { WorktreeMeta } from '../orca-runtime-test-mocks.spec' -import { setWorktreeRemovalSshHostHomeResolver } from '../../worktree-removal-execution-host-route' import { TEST_REPO_ID, TEST_REPO_PATH, @@ -37,6 +38,10 @@ import { } from '../orca-runtime-test-fixtures.spec' import { createWorktreeRemovalRuntime } from '../orca-runtime-test-scenario-builders.spec' +// Why: these fixtures register an SSH provider, which models a connected relay session — and a +// connected session has always read the host's `$HOME`. The removal guards refuse without it. +beforeEach(resetWorktreeTestSshHostHome) + describe('OrcaRuntimeService', () => { it('force-deletes a preserved branch on the qualified host when repo ids collide', async () => { const localRepo = store.getRepo(TEST_REPO_ID)! @@ -463,9 +468,6 @@ describe('OrcaRuntimeService', () => { } registerSshGitProvider(repo.connectionId, gitProvider as never) registerSshFilesystemProvider(repo.connectionId, fsProvider as never) - // Why: the orphan-directory gate is a recursive delete, so it refuses until the host names its - // own home. A connected relay session always has, which is what this fixture stands for. - setWorktreeRemovalSshHostHomeResolver(() => '/home/remote-user') const runtime = new OrcaRuntimeService(runtimeStore as never, undefined, { getSshProvider: () => ptyProvider as never }) @@ -475,7 +477,6 @@ describe('OrcaRuntimeService', () => { runtime.removeManagedWorktree(`id:${worktreeId}`, { force: true }) ).resolves.toEqual({}) } finally { - setWorktreeRemovalSshHostHomeResolver(() => null) unregisterSshGitProvider(repo.connectionId) unregisterSshFilesystemProvider(repo.connectionId) } diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-execution-host.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-execution-host.spec.ts index 4370d02e217..66f561527ce 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-execution-host.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-execution-host.spec.ts @@ -9,6 +9,8 @@ import { } from '../orca-runtime-test-mocks.spec' import type { WorktreeMeta } from '../orca-runtime-test-mocks.spec' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resetWorktreeTestSshHostHome } from '../../worktree-removal-test-ssh-host-home' + import { TEST_WORKTREE_ID, TEST_WORKTREE_PATH, @@ -18,6 +20,10 @@ import { import { createWorktreeRemovalRuntime } from '../orca-runtime-test-scenario-builders.spec' import type { ExecutionHostId } from '../../../shared/execution-host' +// Why: these fixtures register an SSH provider, which models a connected relay session — and a +// connected session has always read the host's `$HOME`. The removal guards refuse without it. +beforeEach(resetWorktreeTestSshHostHome) + const REMOTE_REPO_PATH = '/remote/repo' function missingPath(): never { diff --git a/src/main/worktree-removal-home-guard.ts b/src/main/worktree-removal-home-guard.ts index d6c36fefb84..004be97d6dc 100644 --- a/src/main/worktree-removal-home-guard.ts +++ b/src/main/worktree-removal-home-guard.ts @@ -40,13 +40,19 @@ export function executionHostRemovalHome( /** * Whether the host that executes the removal actually named its home directory. * - * `false` is `unverifiable`, not "no home here" (docs/reference/ssh-execution-boundary.md). The - * recursive-directory gates in `worktree-removal-safety.ts` require `true`, because there the home - * guard is the only evidence standing between an `rm -rf` and somebody's `$HOME` — a bare-repo - * dotfiles checkout puts a real `.git` file at the top of a home directory, which is exactly the - * orphan proof those gates accept. `git worktree remove` does not require it: the execution host's - * own Git registry already established that the path is a linked worktree of that repo, and a - * missing second opinion does not retract that. + * `false` is `unverifiable`, not "no home here" (docs/reference/ssh-execution-boundary.md). Every + * gate in `worktree-removal-safety.ts` that authorises a delete requires `true`, because nothing + * else in reach rules out a home directory: + * + * - The orphan gates accept a `.git` file at the top of a directory as proof, which is also what + * a bare-repo dotfiles `$HOME` looks like. + * - The registry does not help either. `git worktree add` takes a pre-existing empty directory, + * and that directory can afterwards be somebody's `$HOME` — a build account's home, a + * container's `HOME=/workspace`. Being a linked worktree of the repo proves provenance, not + * that the path is not a home, and `git worktree remove --force` deletes the checkout. + * + * With the host's answer both are caught by containment. Without it only the path shapes remain, + * and a home at a non-standard location has no shape to match. */ export function isRemovalHomeAuthorityResolved(home: WorktreeRemovalHomeAuthority): boolean { return home.kind === 'client' || !!home.homePath diff --git a/src/main/worktree-removal-safety.test.ts b/src/main/worktree-removal-safety.test.ts index 3a2da4c2481..9aa9775de44 100644 --- a/src/main/worktree-removal-safety.test.ts +++ b/src/main/worktree-removal-safety.test.ts @@ -4,6 +4,7 @@ import type { GitWorktreeInfo } from '../shared/worktree/types' import { canCleanupUnregisteredOrcaLeftoverDirectory, canSafelyRemoveOrphanedWorktreeDirectory, + findRegisteredDeletableWorktree, getRegisteredDeletableWorktree, isDangerousWorktreeRemovalPath } from './worktree-removal-safety' @@ -563,6 +564,42 @@ describe('isDangerousWorktreeRemovalPath on an execution host', () => { ) }) + it('refuses a registered worktree while the execution host home is unanswered', () => { + // `git worktree add` accepts a pre-existing empty directory, and that directory can afterwards + // be somebody's `$HOME` (a build account's home, a container's `HOME=/workspace`). So the + // host's own Git registry proves provenance, not "this is not a home" — and `git worktree + // remove --force` deletes the checkout. With the host's answer the path is caught by + // containment; without it there is nothing left to catch a non-standard home shape. + const registered = [makeGitWorktree('/opt/src/repo', true), makeGitWorktree('/srv/homes/alice')] + + expect(() => + findRegisteredDeletableWorktree( + '/opt/src/repo', + '/srv/homes/alice', + registered, + executionHostRemovalHome(null) + ) + ).toThrow('Refusing to delete protected worktree path: /srv/homes/alice') + expect(() => + findRegisteredDeletableWorktree( + '/opt/src/repo', + '/srv/homes/alice', + registered, + executionHostRemovalHome('/srv/homes/alice') + ) + ).toThrow('Refusing to delete protected worktree path: /srv/homes/alice') + // An answering host whose home is elsewhere still deletes it: the refusals above are the + // missing answer and the matching answer, not the path. + expect( + findRegisteredDeletableWorktree( + '/opt/src/repo', + '/srv/homes/alice', + registered, + executionHostRemovalHome('/srv/homes/bob') + ) + ).toEqual(registered[1]) + }) + it('refuses a home the host reported even when no path rule recognises it', () => { expect( isDangerousWorktreeRemovalPath( diff --git a/src/main/worktree-removal-safety.ts b/src/main/worktree-removal-safety.ts index c26d3fd0bf5..7f86c5f0e3a 100644 --- a/src/main/worktree-removal-safety.ts +++ b/src/main/worktree-removal-safety.ts @@ -103,7 +103,11 @@ export function findRegisteredDeletableWorktree( if (!worktree) { return null } - if (worktree.isMainWorktree || isDangerousWorktreeRemovalPath(worktree.path, repoPath, home)) { + if ( + !isRemovalHomeAuthorityResolved(home) || + worktree.isMainWorktree || + isDangerousWorktreeRemovalPath(worktree.path, repoPath, home) + ) { throw new Error(`Refusing to delete protected worktree path: ${worktree.path}`) } assertWorktreeDoesNotContainRegisteredWorktree(worktree.path, worktrees) diff --git a/src/main/ipc/worktrees-test-ssh-host-home.ts b/src/main/worktree-removal-test-ssh-host-home.ts similarity index 52% rename from src/main/ipc/worktrees-test-ssh-host-home.ts rename to src/main/worktree-removal-test-ssh-host-home.ts index a373afe0e51..696f7606520 100644 --- a/src/main/ipc/worktrees-test-ssh-host-home.ts +++ b/src/main/worktree-removal-test-ssh-host-home.ts @@ -1,15 +1,15 @@ -import { setWorktreeRemovalSshHostHomeResolver } from '../worktree-removal-execution-host-route' +import { setWorktreeRemovalSshHostHomeResolver } from './worktree-removal-execution-host-route' -/** The `$HOME` the worktree IPC suites' SSH hosts report. */ +/** The `$HOME` the worktree-removal suites' SSH hosts report. */ export const TEST_SSH_HOST_HOME = '/home/remote-user' /** - * Makes the harness's SSH hosts answer the removal guards' home question. + * Makes a suite's SSH hosts answer the removal guards' home question. * * Every suite that registers an SSH provider is modelling a connected relay session, and a - * connected session has always read the host's `$HOME`. Without it the guards refuse the recursive - * orphan delete — the right answer for a host that never answered, the wrong fixture for one that - * did. Deliberately not wired from `worktrees-test-module-mocks`: that module is imported from + * connected session has always read the host's `$HOME`. Without it the guards refuse the delete — + * the right answer for a host that never answered, the wrong fixture for one that did. + * Deliberately not wired from `ipc/worktrees-test-module-mocks`: that module is imported from * `vi.mock` factories, and reaching the production route module from there pulls in * `providers/ssh-git-dispatch` while it is being mocked, which deadlocks the module runner. */