mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 12:59:13 +01:00
fix(workspaces): preserve remote fallback and release denied reopens
Authorize the local directory or transport that Git will actually read. Keep remote sibling fallback when its configured directory is missing, while preserving inherited destination permissions on checkout reuse. Clear only this reopen's pending generation after post-provision source revocation so a denied, dirty worktree can still be archived safely. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
089d2048ae
commit
a2d6555d81
4 files changed
+116
-11
No files matched your search
@@ -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 });
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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?.();
|
||||
|
||||
Reference in new issue
Block a user