From 0a99165e2bf67dd9fd03f09c0689bc4919eb327f Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 3 Nov 2026 13:39:03 -0600 Subject: [PATCH] fix(workspaces): fence physical aliases and preserve capture scope Co-Authored-By: Paperclip --- .../remote-workspace-migration-tree.ts | 72 +++++++++++++------ .../remote-workspace-migration.test.ts | 25 +++++++ .../services/remote-workspace-migration.ts | 24 +++++-- 3 files changed, 94 insertions(+), 27 deletions(-) diff --git a/server/src/services/remote-workspace-migration-tree.ts b/server/src/services/remote-workspace-migration-tree.ts index a42c019c4b6..3c7deed9113 100644 --- a/server/src/services/remote-workspace-migration-tree.ts +++ b/server/src/services/remote-workspace-migration-tree.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { and, eq, inArray, ne, sql } from "drizzle-orm"; import { executionWorkspaces, executionWorkspaceSnapshots, heartbeatRuns, workspaceOperations, type Db } from "@paperclipai/db"; import type { StorageService } from "../storage/types.js"; @@ -14,6 +15,29 @@ export type ImportedWorkspaceChild = { const overlaps = (a: string, b: string) => a === b || a.startsWith(`${b}/`) || b.startsWith(`${a}/`); const rootOf = (workspace: Pick) => workspace.cwd ?? workspace.providerRef; +// Missing historical paths still resolve through their nearest surviving +// parent, so a symlinked ancestor cannot hide an alias from the writer fence. +async function physicalPath(value: string): Promise { + let current = path.resolve(value); const missing: string[] = []; + for (;;) { + try { return path.join(await fs.realpath(current), ...missing.reverse()); } + catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT" || current === path.dirname(current)) throw error; + missing.push(path.basename(current)); current = path.dirname(current); + } + } +} + +export function validateImportedWorkspaceChildren(raw: unknown): ImportedWorkspaceChild[] { + if (!Array.isArray(raw) || raw.some(child => !child || typeof child !== "object" + || typeof child.relativePath !== "string" || !child.relativePath.startsWith(".paperclip/worktrees/") + || child.relativePath.split("/").some((part: string) => !part || part === "." || part === "..") + || !/^[a-f0-9-]{36}$/.test(child.workspaceId) || !/^[a-f0-9-]{36}$/.test(child.snapshotId) + || !/^[a-f0-9-]{36}$/.test(child.operationId) || !/^[a-f0-9]{64}$/.test(child.manifestDigest))) + throw new Error("nested_workspace_coverage_invalid"); + return raw; +} + export async function offlineWorkspaceMigrationBlockers(db: ReadDb) { if (process.env.PAPERCLIP_EPHEMERAL_MIGRATION !== "true") return ["offline_maintenance_required"]; const [active] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns) @@ -32,15 +56,16 @@ export async function workspaceMigrationLayoutBlockers(db: ReadDb, workspace: Wo eq(executionWorkspaces.companyId, workspace.companyId), ne(executionWorkspaces.id, workspace.id), inArray(executionWorkspaces.providerType, ["local_fs", "git_worktree"]), )); - if (local.some(row => rootOf(row) && overlaps(path.resolve(cwd), path.resolve(rootOf(row)!)))) { + const root = await physicalPath(cwd); + for (const row of local) if (rootOf(row) && overlaps(root, await physicalPath(rootOf(row)!))) return ["overlapping_workspace_requires_maintenance"]; - } const migrated = await db.select({ cwd: workspaceOperations.cwd }).from(workspaceOperations).where(and( eq(workspaceOperations.companyId, workspace.companyId), eq(workspaceOperations.phase, "remote_workspace_migration"), eq(workspaceOperations.status, "succeeded"), ne(workspaceOperations.executionWorkspaceId, workspace.id), )); - return migrated.some(row => row.cwd && path.resolve(row.cwd).startsWith(`${path.resolve(cwd)}/.paperclip/worktrees/`)) - ? ["overlapping_workspace_requires_maintenance"] : []; + for (const row of migrated) if (row.cwd && (await physicalPath(row.cwd)).startsWith(`${root}/.paperclip/worktrees/`)) + return ["overlapping_workspace_requires_maintenance"]; + return []; } /** Runtime-created nested task folders can be omitted only after an exact @@ -48,9 +73,12 @@ export async function workspaceMigrationLayoutBlockers(db: ReadDb, workspace: Wo * the parent archive. The snapshot owner retains the resulting dependencies. */ export async function prepareNestedWorkspaceImports(input: { db: Db; storage: StorageService; workspace: Workspace; scratch: string; signal?: AbortSignal; + recordedChildren?: ImportedWorkspaceChild[]; }): Promise { - const root = rootOf(input.workspace); - if (!root) return []; + const sourceRoot = rootOf(input.workspace); + if (!sourceRoot) return []; + if (input.recordedChildren && !input.recordedChildren.length) return []; + const root = await physicalPath(sourceRoot); const rows = await input.db.select({ operation: workspaceOperations, snapshot: executionWorkspaceSnapshots }) .from(workspaceOperations).innerJoin(executionWorkspaceSnapshots, and( eq(executionWorkspaceSnapshots.companyId, workspaceOperations.companyId), @@ -61,21 +89,27 @@ export async function prepareNestedWorkspaceImports(input: { eq(workspaceOperations.status, "succeeded"), inArray(executionWorkspaceSnapshots.status, ["committed", "superseded"]), sql`${workspaceOperations.metadata}->>'snapshotId' = ${executionWorkspaceSnapshots.id}::text`, )); - const candidates = rows.filter(row => row.operation.cwd && path.resolve(row.operation.cwd).startsWith(`${path.resolve(root)}/.paperclip/worktrees/`)) - .sort((a, b) => a.operation.cwd!.length - b.operation.cwd!.length || a.operation.id.localeCompare(b.operation.id)); + const located = []; + for (const row of rows) { + if (!row.operation.cwd || input.recordedChildren && !input.recordedChildren.some(child => child.operationId === row.operation.id)) continue; + located.push({ ...row, sourcePath: await physicalPath(row.operation.cwd) }); + } + const candidates = located.filter(row => row.sourcePath.startsWith(`${root}/.paperclip/worktrees/`)) + .sort((a, b) => a.sourcePath.length - b.sourcePath.length || a.operation.id.localeCompare(b.operation.id)); const covered: ImportedWorkspaceChild[] = []; - for (const { operation, snapshot } of candidates) { + for (const { operation, snapshot, sourcePath } of candidates) { input.signal?.throwIfAborted(); - const relativePath = path.relative(root, operation.cwd!).split(path.sep).join("/"); + const relativePath = path.relative(root, sourcePath).split(path.sep).join("/"); if (covered.some(item => relativePath === item.relativePath || relativePath.startsWith(`${item.relativePath}/`))) continue; - const entry = await fs.lstat(operation.cwd!).catch((error: NodeJS.ErrnoException) => { if (error.code === "ENOENT") return null; throw error; }); - if (!entry) continue; + const entry = await fs.lstat(sourcePath).catch((error: NodeJS.ErrnoException) => { if (error.code === "ENOENT") return null; throw error; }); + if (!entry) { if (input.recordedChildren) throw new Error("nested_workspace_source_changed"); continue; } if (!entry.isDirectory() || entry.isSymbolicLink()) throw new Error("nested_workspace_source_changed"); - await verifyCoveredWorkspace({ ...input, sourcePath: operation.cwd!, snapshot, operationId: operation.id, + await verifyCoveredWorkspace({ ...input, sourcePath, snapshot, operationId: operation.id, expectedDigest: operation.metadata?.manifestDigest, depth: 0 }); covered.push({ workspaceId: snapshot.executionWorkspaceId, snapshotId: snapshot.id, operationId: operation.id, relativePath, manifestDigest: snapshot.manifestDigest! }); } + if (input.recordedChildren && !isDeepStrictEqual(covered, input.recordedChildren)) throw new Error("nested_workspace_scope_changed"); return covered; } @@ -90,15 +124,7 @@ async function verifyCoveredWorkspace(input: { workspaceId: snapshot.executionWorkspaceId, snapshotId: snapshot.id }); if (!reference || reference.manifestDigest !== input.expectedDigest || snapshot.sourceOperationId !== input.operationId) throw new Error("nested_workspace_snapshot_changed"); - const raw = snapshot.metadata?.importedWorkspaceChildren; - if (raw !== undefined && (!Array.isArray(raw) || raw.some(child => !child || typeof child !== "object" - || typeof child.relativePath !== "string" || !child.relativePath.startsWith(".paperclip/worktrees/") - || child.relativePath.split("/").some((part: string) => !part || part === "." || part === "..") - || !/^[a-f0-9-]{36}$/.test(child.workspaceId) || !/^[a-f0-9-]{36}$/.test(child.snapshotId) - || !/^[a-f0-9-]{36}$/.test(child.operationId) || !/^[a-f0-9]{64}$/.test(child.manifestDigest)))) { - throw new Error("nested_workspace_coverage_invalid"); - } - const children = (raw ?? []) as ImportedWorkspaceChild[]; + const children = validateImportedWorkspaceChildren(snapshot.metadata?.importedWorkspaceChildren ?? []); const area = await fs.mkdtemp(path.join(input.scratch, "covered-")); for (const child of children) { const [saved] = await input.db.select({ operation: workspaceOperations, snapshot: executionWorkspaceSnapshots }) @@ -109,7 +135,7 @@ async function verifyCoveredWorkspace(input: { eq(workspaceOperations.status, "succeeded"), eq(workspaceOperations.phase, "remote_workspace_migration"), inArray(executionWorkspaceSnapshots.status, ["committed", "superseded"]))); const sourcePath = path.join(input.sourcePath, child.relativePath); - if (!saved || saved.operation.cwd !== sourcePath) throw new Error("nested_workspace_coverage_invalid"); + if (!saved?.operation.cwd || await physicalPath(saved.operation.cwd) !== sourcePath) throw new Error("nested_workspace_coverage_invalid"); await verifyCoveredWorkspace({ ...input, sourcePath, snapshot: saved.snapshot, operationId: child.operationId, expectedDigest: child.manifestDigest, scratch: area, depth: input.depth + 1 }); } diff --git a/server/src/services/remote-workspace-migration.test.ts b/server/src/services/remote-workspace-migration.test.ts index a4887fbe94b..557687534a6 100644 --- a/server/src/services/remote-workspace-migration.test.ts +++ b/server/src/services/remote-workspace-migration.test.ts @@ -115,6 +115,31 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { .rejects.toThrow("active_execution_requires_drain"); expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); }); + it("detects overlapping folders reached through symlinked parent directories", async () => { + const f = await seed(), alias = path.join(root, "alias"); await fs.symlink(root, alias); + await db.insert(executionWorkspaces).values({ companyId: f.companyId, name: "Physical alias", mode: "isolated_workspace", + strategyType: "project_primary", providerType: "local_fs", cwd: path.join(alias, "original") }); + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("overlapping_workspace_requires_maintenance"); + }); + it.each(["current", "legacy"])("keeps the parent capture scope when children migrate between retries (%s)", async journal => { + const { parent, child } = await nestedFixture(); vi.stubEnv("PAPERCLIP_EPHEMERAL_MIGRATION", "true"); + const put = storage.putFile.bind(storage); let fail = true; + storage.putFile = async input => { const saved = await put(input); if (fail) { fail = false; throw new Error("upload reply lost"); } return saved; }; + await expect(migrateRemoteWorkspace({ db, storage, ...parent, apply: true, writersStopped: true })).rejects.toThrow("upload reply lost"); + if (journal === "legacy") { + const [operation] = await db.select().from(workspaceOperations).where(eq(workspaceOperations.executionWorkspaceId, parent.workspaceId)); + const metadata = { ...operation!.metadata }; delete metadata.nestedWorkspaceScope; + await db.update(workspaceOperations).set({ metadata }).where(eq(workspaceOperations.id, operation!.id)); + } + await migrateRemoteWorkspace({ db, storage, ...child, apply: true, writersStopped: true }); + await migrateRemoteWorkspace({ db, storage, ...parent, apply: true, writersStopped: true }); + const authority = readRemoteWorkspaceAuthority((await workspace(parent.workspaceId)).metadata)!; + const reference = await getWorkspaceSnapshot(db, { ...parent, snapshotId: authority.snapshotId! }); + const archivePath = path.join(root, "retried-parent.pcws"), targetDir = path.join(root, "retried-parent"); + await loadWorkspaceSnapshotArchive({ storage, reference: reference!, targetPath: archivePath }); + await restoreRemoteWorkspaceArchive({ archivePath, targetDir, expectedManifestDigest: reference!.manifestDigest }); + expect(await fs.readFile(path.join(targetDir, ".paperclip/worktrees/child/child.txt"), "utf8")).toBe("saved child edits\n"); + }); it("blocks distinct legacy records that alias the same folder", async () => { const f = await seed(), id = randomUUID(); await db.insert(executionWorkspaces).values({ id, companyId: f.companyId, name: "Historical alias", mode: "isolated_workspace", strategyType: "project_primary", providerType: "local_fs", cwd: f.cwd }); diff --git a/server/src/services/remote-workspace-migration.ts b/server/src/services/remote-workspace-migration.ts index b0ccbd46f05..91cdc407c22 100644 --- a/server/src/services/remote-workspace-migration.ts +++ b/server/src/services/remote-workspace-migration.ts @@ -17,7 +17,7 @@ import { notifyRemoteWorkspaceSnapshots } from "./remote-workspace-snapshot-rete import { materializeTaskDirectorySnapshot, removeTaskDirectoryTree } from "./isolated-task-directory-snapshot.js"; import { loadTaskDirectoryRestorePoint, mayBeIsolatedTaskDirectoryWorkspace, readTaskDirectoryRetention, validateTaskDirectoryRestorePointRef } from "./isolated-task-directory-retention.js"; import { inspectArchivedGitMigrationSource, materializeArchivedGitMigrationSource, type ArchivedGitMigrationSource } from "./remote-workspace-migration-git.js"; -import { prepareNestedWorkspaceImports, workspaceMigrationLayoutBlockers } from "./remote-workspace-migration-tree.js"; +import { prepareNestedWorkspaceImports, validateImportedWorkspaceChildren, workspaceMigrationLayoutBlockers } from "./remote-workspace-migration-tree.js"; const SCHEMA = "paperclip.remote-workspace-migration.v1"; type Workspace = typeof executionWorkspaces.$inferSelect; @@ -186,6 +186,7 @@ export async function migrateRemoteWorkspace(input: { const [operation] = await tx.insert(workspaceOperations).values({ companyId: workspace.companyId, executionWorkspaceId: workspace.id, issueId: workspace.sourceIssueId, cwd: workspace.cwd ?? workspace.providerRef, phase: "remote_workspace_migration", status: "running", metadata: { schema: SCHEMA, sourceFingerprint, source, sourceIssueBinding: current.issueBinding, + nestedWorkspaceScope: { version: 1, state: "pending" }, ...(current.archivedGitSource ? { archivedGitSource: gitSourceProof(current.archivedGitSource) } : {}), ...(current.retainedDirectory ? { retainedDirectory: { restorePointId: current.retainedDirectory.restorePoint.id, generation: current.retainedDirectory.generation, digest: current.retainedDirectory.restorePoint.digest } } : {}) } }).returning(); @@ -196,7 +197,7 @@ export async function migrateRemoteWorkspace(input: { return { workspace, authority: null, operation: operation!, source, sourceFingerprint, issueBinding: current.issueBinding, retainedDirectory: current.retainedDirectory, archivedGitSource: current.archivedGitSource }; }); if (reserved.authority) return { ...await inventoryRemoteWorkspaceMigration(input.db, input), applied: true, replayed: true }; - const operation = reserved.operation!; + let operation = reserved.operation!; const snapshot = await beginWorkspaceSnapshot(input.db, { companyId: input.companyId, workspaceId: input.workspaceId, operationId: operation.id, generation: 1, epoch: 1 }); const scratch = await fs.mkdtemp(path.join(os.tmpdir(), "pc-workspace-migration-")); @@ -208,8 +209,23 @@ export async function migrateRemoteWorkspace(input: { target: path.join(scratch, "retained-source"), expectedDigest: retained.digest, }) : reserved.archivedGitSource ? await materializeArchivedGitMigrationSource({ db: input.db, storage: input.storage, workspace: reserved.workspace, source: reserved.archivedGitSource, scratch }) : reserved.workspace.cwd ?? reserved.workspace.providerRef!; + const scope = object(operation.metadata?.nestedWorkspaceScope); + if (operation.metadata?.nestedWorkspaceScope !== undefined && (scope.version !== 1 || !["pending", "fixed"].includes(String(scope.state)))) + throw new Error("nested_workspace_scope_invalid"); + // Journals predating scoped captures included every child. Preserve that + // archive identity even if a later batch has already imported the leaves. + const recordedChildren = scope.state === "fixed" ? validateImportedWorkspaceChildren(scope.children) + : scope.state === "pending" ? undefined : []; const nested = input.writersStopped && !retained && !reserved.archivedGitSource - ? await prepareNestedWorkspaceImports({ ...input, workspace: reserved.workspace, scratch }) : []; + ? await prepareNestedWorkspaceImports({ ...input, workspace: reserved.workspace, scratch, recordedChildren }) : []; + if (scope.state === "pending") { + const [saved] = await input.db.update(workspaceOperations).set({ metadata: { ...operation.metadata, + nestedWorkspaceScope: { version: 1, state: "fixed", children: nested } }, updatedAt: new Date() }) + .where(and(eq(workspaceOperations.companyId, input.companyId), eq(workspaceOperations.id, operation.id), + eq(workspaceOperations.status, "running"))).returning(); + if (!saved) throw new Error("remote_workspace_migration_operation_changed"); + operation = saved; + } const exclude = nested.map(child => child.relativePath); const archivePath = path.join(scratch, "workspace.pcws"); const captured = await captureRemoteWorkspaceDirectory({ cwd: sourceRoot, archivePath, exclude, signal: input.signal }); @@ -223,7 +239,7 @@ export async function migrateRemoteWorkspace(input: { if (current.manifestDigest !== captured.manifestDigest) throw new Error("remote_workspace_migration_source_changed"); if (nested.length) { const verifyScratch = path.join(scratch, "verify-nested"); await fs.mkdir(verifyScratch); - const verified = await prepareNestedWorkspaceImports({ ...input, workspace: reserved.workspace, scratch: verifyScratch }); + const verified = await prepareNestedWorkspaceImports({ ...input, workspace: reserved.workspace, scratch: verifyScratch, recordedChildren: nested }); if (canonical(verified) !== canonical(nested)) throw new Error("nested_workspace_source_changed"); } input.signal?.throwIfAborted();