diff --git a/server/src/__tests__/execution-workspace-reopen.test.ts b/server/src/__tests__/execution-workspace-reopen.test.ts index fe2be10407b..704f156bf38 100644 --- a/server/src/__tests__/execution-workspace-reopen.test.ts +++ b/server/src/__tests__/execution-workspace-reopen.test.ts @@ -1,7 +1,7 @@ import { EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; import { execFileSync } from "node:child_process"; import { mkdirSync, writeFileSync } from "node:fs"; -import { mkdtemp, rm, stat, readFile } from "node:fs/promises"; +import { mkdtemp, rm, stat, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { randomUUID } from "node:crypto"; @@ -417,7 +417,9 @@ describeEmbeddedPostgres("reopen archived isolated execution workspace", () => { } }); - it.each([false, true])("guards a private local managed reopen before Git and retains privacy after provisioning failure=%s", async provisionFails => { + it.each([false, true, "revoked_during_provision"] as const)("guards a private local managed reopen before Git and retains privacy after provisioning failure=%s", async scenario => { + const provisionFails = scenario === true; + const revokeDuringProvision = scenario === "revoked_during_provision"; const previousHome = process.env.PAPERCLIP_HOME; const previousPrivacy = process.env.PAPERCLIP_ISSUE_PRIVACY_MODE; const home = await makeExistingDir(); @@ -441,19 +443,43 @@ describeEmbeddedPostgres("reopen archived isolated execution workspace", () => { await db.insert(issues).values({ id: privateIssueId, companyId, title: "Private source", visibility: "private", status: "todo", responsibleUserId: "another-owner" }); await db.insert(executionWorkspaces).values({ companyId, projectId: null, sourceIssueId: privateIssueId, name: "Private owner", mode: "shared_workspace", strategyType: "project_primary", cwd: source, providerType: "local_fs", status: "active" }); const workspaceId = await seedClosedGitWorktreeWorkspace({ companyId, projectId, projectWorkspaceId, cwd: target, repoUrl, branchName: "reopen-feature" }); - if (provisionFails) await db.update(executionWorkspaces).set({ metadata: { [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: 1, config: { provisionCommand: "exit 42" } } }).where(eq(executionWorkspaces.id, workspaceId)); + const waitingProvision = `node -e 'const fs = require("node:fs"); fs.writeFileSync("dirty.txt", "DIRTY_REOPEN_BYTES"); fs.writeFileSync("provision-started", "ready"); const deadline = Date.now() + 15000; const timer = setInterval(() => { if (fs.existsSync("provision-continue")) { clearInterval(timer); process.exit(0); } if (Date.now() > deadline) process.exit(2); }, 20);'`; + if (provisionFails || revokeDuringProvision) await db.update(executionWorkspaces).set({ metadata: { [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: 1, config: { provisionCommand: provisionFails ? "exit 42" : waitingProvision } } }).where(eq(executionWorkspaces.id, workspaceId)); const issueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4901 }); + if (revokeDuringProvision) await db.update(issues).set({ status: "done" }).where(eq(issues.id, issueId)); const reopen = () => executionWorkspaceService(db).reopenClosedIsolatedExecutionWorkspaceForIssue({ workspaceId, issue: { id: issueId, companyId, projectId }, actor: { actorType: "user", agentId: null }, authorizationActor }); expect(await reopen()).toMatchObject({ ok: false, code: "rebuild_failed" }); await expect(stat(target)).rejects.toMatchObject({ code: "ENOENT" }); const managed = resolveManagedProjectWorkspaceDir({ companyId, projectId, repoName: "private-source" }); await expect(stat(managed)).rejects.toMatchObject({ code: "ENOENT" }); await db.insert(issueAccessGrants).values({ issueId: privateIssueId, subjectType: "user", subjectId: readerId, source: "explicit" }); - const results = provisionFails ? [await reopen()] : await Promise.all([reopen(), reopen()]); - expect(results.every(result => result.ok)).toBe(!provisionFails); - if (!provisionFails) expect(results.map(result => result.ok && result.reopened).sort()).toEqual([false, true]); + const attempt = revokeDuringProvision ? reopen() : null; + if (attempt) { + try { + await vi.waitFor(async () => expect(await readFile(join(target, "provision-started"), "utf8")).toBe("ready"), { timeout: 10_000 }); + await db.delete(issueAccessGrants).where(eq(issueAccessGrants.issueId, privateIssueId)); + } finally { + await writeFile(join(target, "provision-continue"), "resume"); + } + } + const results = attempt ? [await attempt] : provisionFails ? [await reopen()] : await Promise.all([reopen(), reopen()]); + expect(results.every(result => result.ok)).toBe(!provisionFails && !revokeDuringProvision); + if (!provisionFails && !revokeDuringProvision) expect(results.map(result => result.ok && result.reopened).sort()).toEqual([false, true]); expect(await readFile(join(target, "private.txt"), "utf8")).toBe("PRIVATE_REOPEN_BYTES"); - await expect(assertExecutionWorkspaceRootReadAccess(db, { companyId, cwd: target, actor: authorizationActor })).resolves.toBeUndefined(); + if (revokeDuringProvision) { + expect(results[0]).toMatchObject({ ok: false, code: "rebuild_failed" }); + const reopened = await readWorkspace(workspaceId); + expect(reopened?.status).toBe("active"); + expect(metadataHasReopenPendingConsumption(reopened?.metadata)).toBe(false); + expect(await readFile(join(target, "dirty.txt"), "utf8")).toBe("DIRTY_REOPEN_BYTES"); + const activity = await db.select().from(activityLog).where(eq(activityLog.entityId, workspaceId)); + expect(activity.map(row => row.action)).toContain("execution_workspace.reopen_unconsumed"); + const archive = await executionWorkspaceService(db).archiveWorkspaceUnderLifecycleLock({ id: workspaceId, patch: {}, closedAt: new Date() }); + expect(archive?.outcome).toBe("archived"); + expect(await readFile(join(target, "dirty.txt"), "utf8")).toBe("DIRTY_REOPEN_BYTES"); + } else { + await expect(assertExecutionWorkspaceRootReadAccess(db, { companyId, cwd: target, actor: authorizationActor })).resolves.toBeUndefined(); + } await db.delete(projectWorkspaces).where(eq(projectWorkspaces.id, projectWorkspaceId)); await db.delete(issueAccessGrants).where(eq(issueAccessGrants.issueId, privateIssueId)); await rm(source, { recursive: true, force: true }); diff --git a/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts b/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts index 74c1b602e79..64ef4636569 100644 --- a/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts +++ b/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { promisify } from "node:util"; +import { pathToFileURL } from "node:url"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { ensureManagedProjectWorkspace, prepareProjectRepositoryWorkspaces } from "../services/heartbeat.ts"; import { buildGitAuthInvocation, GIT_CREDENTIAL_TOKEN_ENV_KEY } from "../services/git-credentials.ts"; @@ -142,6 +143,77 @@ exit 128 } }); + it("clones a remote sibling with a missing configured cwd and rechecks destination access on reuse", async () => { + const source = await createLocalSourceRepo(); + const task = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-remote-sibling-")); + try { + const remote = "https://example.test/remote.git"; + // Exercise the real clone without contacting a network provider. + const resolveGitAuth = vi.fn(async () => ({ + configArgs: ["-c", `url.${pathToFileURL(source).href}.insteadOf=${remote}`], + env: {}, source: "company_secret" as const, secretName: "fixture", + })); + const prepareLocalSource = vi.fn(async (cwd: string) => { + await fs.realpath(cwd); + return async () => {}; + }); + let destinationReadable = true; + const beforePublish = vi.fn(async () => { + if (!destinationReadable) throw new Error("Retained private destination is not authorized"); + }); + const input = { + cwd: task, anchorRepoUrl: null, resolveGitAuth, prepareLocalSource, beforePublish, + workspaces: [{ id: "remote", cwd: path.join(task, "missing"), repoUrl: remote, repoRef: null }], + }; + const [repo] = await prepareProjectRepositoryWorkspaces(input); + expect(await fs.readFile(path.join(repo!.cwd, "README.md"), "utf8")).toBe("hello\n"); + expect(prepareLocalSource).not.toHaveBeenCalled(); + expect(resolveGitAuth).toHaveBeenCalledTimes(1); + await fs.writeFile(path.join(repo!.cwd, "README.md"), "retained task edits"); + beforePublish.mockClear(); + expect(await prepareProjectRepositoryWorkspaces(input)).toEqual([repo]); + expect(beforePublish).toHaveBeenCalledTimes(1); + expect(resolveGitAuth).toHaveBeenCalledTimes(1); + expect(await fs.readFile(path.join(repo!.cwd, "README.md"), "utf8")).toBe("retained task edits"); + destinationReadable = false; + await expect(prepareProjectRepositoryWorkspaces(input)).rejects.toThrow("Retained private destination is not authorized"); + expect(prepareLocalSource).not.toHaveBeenCalled(); + expect(await fs.readFile(path.join(repo!.cwd, "README.md"), "utf8")).toBe("retained task edits"); + } finally { + await Promise.all([source, task].map((cwd) => fs.rm(cwd, { recursive: true, force: true }))); + } + }); + + it.each(["file_url", "absolute_path"] as const)("authorizes the actual %s fallback when the configured cwd is missing", async (transport) => { + const source = await createLocalSourceRepo(); + const task = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-local-sibling-")); + try { + let sourceReadable = false; + const recheck = vi.fn(async () => {}); + const prepareLocalSource = vi.fn(async (cwd: string) => { + expect(cwd).toBe(source); + if (!sourceReadable) throw new Error("Private source is not authorized"); + return recheck; + }); + const input = { + cwd: task, anchorRepoUrl: null, prepareLocalSource, + workspaces: [{ id: "local", cwd: path.join(task, "missing"), + repoUrl: transport === "file_url" ? pathToFileURL(source).href : source, repoRef: null }], + }; + await expect(prepareProjectRepositoryWorkspaces(input)).rejects.toThrow("Private source is not authorized"); + expect(await fs.readdir(path.join(task, ".paperclip-repositories"))).toEqual([]); + sourceReadable = true; + await fs.writeFile(path.join(source, "README.md"), "uncommitted source edits"); + const [repo] = await prepareProjectRepositoryWorkspaces(input); + expect(await fs.readFile(path.join(repo!.cwd, "README.md"), "utf8")).toBe("hello\n"); + expect(recheck).toHaveBeenCalled(); + sourceReadable = false; + await expect(prepareProjectRepositoryWorkspaces(input)).rejects.toThrow("Private source is not authorized"); + } finally { + await Promise.all([source, task].map((cwd) => fs.rm(cwd, { recursive: true, force: true }))); + } + }); + it("fails task preparation when any attached repository cannot be cloned", async () => { const first = await createLocalSourceRepo(); try { diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 6652ebc4d66..76458e80e8c 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -3592,6 +3592,13 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic actor: input.authorizationActor, captured: capturedSource, issueId: issue.id, phase: "verified", }); } catch { + // The route cannot consume a denied reopen. Release only the fence + // published by this attempt; a newer reopen keeps its own generation. + if (result.ok && result.reopened) { + await executionWorkspaceService(db).clearReopenPendingConsumptionForUnconsumedReopen({ + workspaceId: snapshot.id, issue, actor, expectedGeneration: result.generation, + }); + } return { ok: false, code: "rebuild_failed", message: "Failed to rebuild the execution workspace" }; } } diff --git a/server/src/services/heartbeat/workspaces.ts b/server/src/services/heartbeat/workspaces.ts index 60ab4bc2a5a..93656e3c346 100644 --- a/server/src/services/heartbeat/workspaces.ts +++ b/server/src/services/heartbeat/workspaces.ts @@ -543,17 +543,17 @@ export async function prepareProjectRepositoryWorkspaces(input: { const existing = await fs.lstat(cwd).catch(() => null); if (existing && (!existing.isDirectory() || existing.isSymbolicLink())) throw new Error("Invalid project repository checkout path"); const configuredLocalSource = workspace.cwd && workspace.cwd !== REPO_ONLY_CWD_SENTINEL ? workspace.cwd : null; + const localSource = configuredLocalSource + && await fs.stat(configuredLocalSource).then((entry) => entry.isDirectory()).catch(() => false) + ? configuredLocalSource : null; // A URL-only clone can still read this host. Keep its committed-only clone // behavior, but authorize the physical file source before any Git read. const urlLocalSource = localRepositoryPath(repoUrl); // Run before the materializer's existing-checkout shortcut too. A sibling // added after the first admission still contributes private source bytes. - const sourceToAuthorize = configuredLocalSource ?? urlLocalSource; + const sourceToAuthorize = localSource ?? urlLocalSource; const recheckLocalSource = sourceToAuthorize ? await input.prepareLocalSource?.(sourceToAuthorize) : undefined; - const localSource = configuredLocalSource - && await fs.stat(configuredLocalSource).then((entry) => entry.isDirectory()).catch(() => false) - ? configuredLocalSource : null; const beforePublish = async () => { await recheckLocalSource?.(); await input.beforePublish?.();