mirror of
https://github.com/stablyai/orca.git
synced 2026-10-06 08:02:28 +00:00
fix(worktree): gate the registered removal on the host home answer too
I argued `git worktree remove --force` did not need the host's home answer, because the host's own Git registry had already established that the path is a linked worktree of that repo. That is true and it is not enough: `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`. Being a linked worktree proves provenance, not that the path is not a home, and the remove deletes the checkout either way. With the host's answer that case is already caught by containment. Without it only the path shapes remain, and a home at a non-standard location (`/var/home/<u>`, `/export/home/<u>`, `D:\\Profiles\\<u>`) has no shape to match. So `findRegisteredDeletableWorktree` now requires the answer as well, and every gate that authorises a delete is on the same rule. The fixture that models a connected relay session moves out of `ipc/` and is shared: four runtime specs register an SSH provider without one, and a live provider implies a reported home in production.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
+7
-1
@@ -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, {
|
||||
|
||||
+7
-6
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
|
||||
+6
-6
@@ -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.
|
||||
*/
|
||||
Reference in New Issue
Block a user