mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-18 05:23:39 +02:00
fix: retain task cwd overrides and protect ignored host files
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3cfe5d58df
commit
5d6d6eabdf
5 files changed
+140
-6
No files matched your search
@@ -7,6 +7,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { captureDirectorySnapshot, disposeDirectorySnapshot, mergeDirectoryWithBaseline } from "./workspace-restore-merge.js";
|
||||
import { writeCheckpointBaseline, workspaceCheckpointScript, readCheckpointSnapshot, validateCheckpointPayload } from "./workspace-checkpoint.js";
|
||||
import { publishWorkspaceSeedGeneration, readWorkspaceSeedGeneration, workspaceSeedGeneration } from "./workspace-seed-cache.js";
|
||||
import { WorkspaceManifestWriter } from "./workspace-manifest.js";
|
||||
const execute = promisify(execFile);
|
||||
const roots: string[] = [];
|
||||
afterEach(async () => { vi.unstubAllEnvs(); await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); });
|
||||
@@ -99,6 +100,65 @@ describe("workspace sparse checkpoint", () => {
|
||||
await expect(validateCheckpointPayload({ baseline: f.baseline, snapshot: result.snapshot, payload: result.payload })).rejects.toThrow("payload mismatch");
|
||||
await disposeDirectorySnapshot(result.snapshot); await disposeDirectorySnapshot(f.baseline);
|
||||
});
|
||||
it.each(["array", "manifest"] as const)("rejects ignored entries and ancestor replacements before merge with %s ignore paths", async storage => {
|
||||
for (const attack of ["matching_file", "matching_descendant", "payload_only", "ancestor_file", "ancestor_link"] as const) {
|
||||
const f = await fixture(); await disposeDirectorySnapshot(f.baseline);
|
||||
await fs.writeFile(path.join(f.root, ".env"), "host secret");
|
||||
await fs.mkdir(path.join(f.root, "private")); await fs.writeFile(path.join(f.root, "private", "token"), "private token");
|
||||
await fs.mkdir(path.join(f.root, "config")); await fs.writeFile(path.join(f.root, "config", ".env"), "nested secret");
|
||||
const paths = [".env", "private", "config/.env"];
|
||||
const writer = new WorkspaceManifestWriter(path.join(f.temp, "ignored.sqlite"));
|
||||
for (const relative of paths) writer.add("ignored", relative);
|
||||
const ignoredPaths = storage === "manifest" ? writer.paths("ignored") : paths;
|
||||
writer.close();
|
||||
const baseline = await captureDirectorySnapshot(f.root, { exclude: ["cache"], ignoredPaths, diskBacked: true });
|
||||
const payload = path.join(f.temp, "forged-payload"); await fs.mkdir(payload);
|
||||
await fs.writeFile(path.join(payload, "unchanged"), "otherwise valid edit");
|
||||
if (attack === "matching_file" || attack === "payload_only") await fs.writeFile(path.join(payload, ".env"), "injected secret");
|
||||
if (attack === "matching_descendant") {
|
||||
await fs.mkdir(path.join(payload, "private")); await fs.writeFile(path.join(payload, "private", "token"), "injected token");
|
||||
}
|
||||
if (attack === "ancestor_file") await fs.writeFile(path.join(payload, "config"), "replace directory");
|
||||
if (attack === "ancestor_link") await fs.symlink("unchanged", path.join(payload, "config"));
|
||||
const captured = await captureDirectorySnapshot(payload);
|
||||
const entries = new Map(baseline.entries);
|
||||
entries.delete("deleted"); // An otherwise valid deletion must not apply on rejection.
|
||||
for (const [relative, entry] of captured.entries) if (!(attack === "payload_only" && relative === ".env")) entries.set(relative, entry);
|
||||
if (attack === "matching_descendant") entries.delete("private"); // Reject the descendant itself, too.
|
||||
const manifestPath = path.join(f.temp, "forged.sqlite");
|
||||
await writeCheckpointBaseline({ ...baseline, entries }, manifestPath);
|
||||
const snapshot = readCheckpointSnapshot(manifestPath, baseline);
|
||||
try {
|
||||
await expect((async () => {
|
||||
await validateCheckpointPayload({ baseline, snapshot, payload });
|
||||
await mergeDirectoryWithBaseline({ baseline, sourceDir: payload, targetDir: f.root, snapshots: { source: snapshot } });
|
||||
})()).rejects.toThrow(attack.startsWith("ancestor") ? "ignored path ancestor" : "Excluded workspace checkpoint entry");
|
||||
expect(await fs.readFile(path.join(f.root, "unchanged"), "utf8")).toBe("retained");
|
||||
expect(await fs.readFile(path.join(f.root, "deleted"), "utf8")).toBe("delete me");
|
||||
expect(await fs.readFile(path.join(f.root, ".env"), "utf8")).toBe("host secret");
|
||||
expect(await fs.readFile(path.join(f.root, "private", "token"), "utf8")).toBe("private token");
|
||||
expect(await fs.readFile(path.join(f.root, "config", ".env"), "utf8")).toBe("nested secret");
|
||||
} finally { await disposeDirectorySnapshot(snapshot); await disposeDirectorySnapshot(baseline); }
|
||||
}
|
||||
});
|
||||
it("allows directory tombstones while retaining ignored host children", async () => {
|
||||
const f = await fixture(); await disposeDirectorySnapshot(f.baseline);
|
||||
await fs.mkdir(path.join(f.root, "config"));
|
||||
await fs.writeFile(path.join(f.root, "config", ".env"), "host only");
|
||||
await fs.writeFile(path.join(f.root, "config", "tracked"), "delete tracked child");
|
||||
const baseline = await captureDirectorySnapshot(f.root, { exclude: ["cache"], ignoredPaths: ["config/.env"], diskBacked: true });
|
||||
const payload = path.join(f.temp, "payload"); await fs.mkdir(payload);
|
||||
await fs.writeFile(path.join(payload, "unchanged"), "valid edit");
|
||||
const captured = await captureDirectorySnapshot(payload);
|
||||
const snapshot = { ...baseline, entries: new Map(captured.entries) };
|
||||
await validateCheckpointPayload({ baseline, snapshot, payload });
|
||||
await mergeDirectoryWithBaseline({ baseline, sourceDir: payload, targetDir: f.root, snapshots: { source: snapshot } });
|
||||
expect(await fs.readFile(path.join(f.root, "config", ".env"), "utf8")).toBe("host only");
|
||||
expect(await fs.readFile(path.join(f.root, "unchanged"), "utf8")).toBe("valid edit");
|
||||
await expect(fs.stat(path.join(f.root, "config", "tracked"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
await expect(fs.stat(path.join(f.root, "deleted"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
await disposeDirectorySnapshot(baseline);
|
||||
});
|
||||
it("reuses durable seed generations and rejects corruption after run cleanup", async () => {
|
||||
const f = await fixture(); const cache = path.join(f.temp, "cache-seeds");
|
||||
const archive = path.join(f.temp, "seed.tar"); await fs.writeFile(archive, "archive");
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { DatabaseSync } from "node:sqlite";
|
||||
import { WorkspaceManifestWriter, WorkspaceManifestMap, workspacePaths } from "./workspace-manifest.js";
|
||||
import { WorkspaceManifestWriter, WorkspaceManifestMap, workspacePaths, workspacePathMatcher } from "./workspace-manifest.js";
|
||||
import { shouldExcludePath, excludePatternMatches, isRelativePathOrDescendant } from "./exclude-patterns.js";
|
||||
import { openDirectorySnapshot, type DirectorySnapshot } from "./workspace-restore-merge.js";
|
||||
|
||||
@@ -125,12 +125,19 @@ export function readCheckpointSnapshot(filePath: string, baseline: DirectorySnap
|
||||
* it. Verify changed entries before the existing merge engine sees the source. */
|
||||
export async function validateCheckpointPayload(input: { baseline: DirectorySnapshot; snapshot: DirectorySnapshot; payload: string }): Promise<WorkspaceCheckpointMetrics> {
|
||||
const { captureDirectorySnapshot, disposeDirectorySnapshot } = await import("./workspace-restore-merge.js");
|
||||
const actual = await captureDirectorySnapshot(input.payload, { diskBacked: true });
|
||||
const ignored = workspacePathMatcher(input.baseline.ignoredPaths);
|
||||
let actual: DirectorySnapshot | null = null;
|
||||
const equal = (left: unknown, right: unknown) => JSON.stringify(left) === JSON.stringify(right);
|
||||
const assertIncluded = (relative: string) => {
|
||||
if (shouldExcludePath(relative, input.baseline.exclude) || ignored.matches(relative)) {
|
||||
throw new Error("Excluded workspace checkpoint entry");
|
||||
}
|
||||
};
|
||||
let changedFiles = 0, payloadBytes = 0;
|
||||
try {
|
||||
actual = await captureDirectorySnapshot(input.payload, { diskBacked: true });
|
||||
for (const [relative, entry] of input.snapshot.entries) {
|
||||
if (shouldExcludePath(relative, input.baseline.exclude)) throw new Error("Excluded workspace checkpoint entry");
|
||||
assertIncluded(relative);
|
||||
if (entry.kind === "symlink") {
|
||||
const target = path.resolve(input.payload, path.dirname(relative), entry.target);
|
||||
if (path.isAbsolute(entry.target) || (target !== path.resolve(input.payload) && !target.startsWith(`${path.resolve(input.payload)}/`))) {
|
||||
@@ -140,11 +147,24 @@ export async function validateCheckpointPayload(input: { baseline: DirectorySnap
|
||||
if (!equal(entry, input.baseline.entries.get(relative)) && !equal(entry, actual.entries.get(relative))) throw new Error("Workspace checkpoint payload mismatch");
|
||||
}
|
||||
for (const [relative, entry] of actual.entries) {
|
||||
assertIncluded(relative);
|
||||
if (!equal(entry, input.snapshot.entries.get(relative))) throw new Error("Unexpected workspace checkpoint payload");
|
||||
if (entry.kind === "file") { changedFiles++; payloadBytes += (await fs.stat(path.join(input.payload, relative))).size; }
|
||||
}
|
||||
// Replacing an ancestor directory with a file/link recursively removes its
|
||||
// host-only ignored children. Directory entries and ordinary tombstones
|
||||
// remain valid: the merge removes empty directories without deleting those
|
||||
// retained children. Stream the trusted ignore manifest, never buffer it.
|
||||
if (input.baseline.ignoredPaths) for (const relative of workspacePaths(input.baseline.ignoredPaths)) {
|
||||
for (let slash = relative.lastIndexOf("/"); slash >= 0; slash = relative.lastIndexOf("/", slash - 1)) {
|
||||
const ancestor = relative.slice(0, slash);
|
||||
for (const entry of [input.snapshot.entries.get(ancestor), actual.entries.get(ancestor)]) {
|
||||
if (entry && entry.kind !== "dir") throw new Error("Workspace checkpoint replaces an ignored path ancestor");
|
||||
}
|
||||
}
|
||||
}
|
||||
return { mode: "sparse", scannedEntries: input.snapshot.entries.size, changedFiles, payloadBytes };
|
||||
} finally { await disposeDirectorySnapshot(actual); }
|
||||
} finally { ignored.close(); await disposeDirectorySnapshot(actual); }
|
||||
}
|
||||
|
||||
export interface WorkspaceCheckpointMetrics {
|
||||
|
||||
@@ -197,6 +197,56 @@ suite("task project repository provisioning", () => {
|
||||
}
|
||||
}, 25_000);
|
||||
|
||||
it.each([
|
||||
{ name: "task override without agent cwd", agentCwd: false, taskDirectory: false },
|
||||
{ name: "task override replacing agent cwd", agentCwd: true, taskDirectory: false },
|
||||
{ name: "explicit task directory ahead of cwd overrides", agentCwd: true, taskDirectory: true },
|
||||
])("resolves $name before binding and retains that root on continuation", async ({ agentCwd, taskDirectory }) => {
|
||||
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID();
|
||||
const overrideCwd = path.join(root, companyId, "task-override");
|
||||
const agentDefaultCwd = path.join(root, companyId, "agent-default");
|
||||
const laterCwd = path.join(root, companyId, "later-override");
|
||||
for (const cwd of [overrideCwd, agentDefaultCwd, laterCwd]) {
|
||||
await mkdir(cwd, { recursive: true });
|
||||
await writeFile(path.join(cwd, "input.txt"), cwd);
|
||||
}
|
||||
await db.insert(companies).values({ id: companyId, name: "Task cwd", issuePrefix: `W${companyId.slice(0, 6)}`, defaultResponsibleUserId: "responsible-user" });
|
||||
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: "responsible-user", status: "active", membershipRole: "owner" });
|
||||
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", status: "idle", adapterType: "codex_local", adapterConfig: agentCwd ? { cwd: agentDefaultCwd } : {}, permissions: {} });
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Use task files", status: "todo", assigneeAgentId: agentId,
|
||||
assigneeAdapterOverrides: { adapterConfig: { cwd: overrideCwd } },
|
||||
workspaceSelection: taskDirectory ? { version: 1, source: "explicit", selection: { kind: "task_directory" } } : null });
|
||||
const expectedCwd = taskDirectory
|
||||
? path.join(root, "home", "instances", "default", "isolated-workspaces", companyId, issueId)
|
||||
: overrideCwd;
|
||||
let bindingId: string | null = null;
|
||||
for (let admission = 0; admission < 2; admission++) {
|
||||
const defaultExecute = execute.getMockImplementation()!;
|
||||
execute.mockImplementationOnce(async (input) => {
|
||||
const cwd = input.context.paperclipWorkspace.cwd;
|
||||
expect(await realpath(cwd)).toBe(await realpath(expectedCwd));
|
||||
if (!taskDirectory) expect(await readFile(path.join(cwd, "input.txt"), "utf8")).toBe(overrideCwd);
|
||||
if (admission === 1) expect(await readFile(path.join(cwd, "output.txt"), "utf8")).toBe("first admission");
|
||||
await writeFile(path.join(cwd, "output.txt"), admission === 0 ? "first admission" : "continued");
|
||||
return defaultExecute(input);
|
||||
});
|
||||
const run = await heartbeat.wakeup(agentId, { source: "on_demand", triggerDetail: "manual", contextSnapshot: { issueId } });
|
||||
await vi.waitFor(async () => { expect(await heartbeat.getRun(run!.id)).toMatchObject({ status: "succeeded", error: null }); }, { timeout: 15_000 });
|
||||
await drainHeartbeatRunsToQuiescence(db, heartbeat);
|
||||
expect(execute.mock.calls.filter(([input]) => input.runId === run!.id)).toHaveLength(1);
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, issueId));
|
||||
const [workspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, task.executionWorkspaceId!));
|
||||
expect(workspace.projectId).toBeNull();
|
||||
expect(await realpath(workspace.cwd!)).toBe(await realpath(expectedCwd));
|
||||
if (bindingId) expect(workspace.id).toBe(bindingId);
|
||||
bindingId = workspace.id;
|
||||
await db.update(issues).set({ status: "todo", assigneeAdapterOverrides: { adapterConfig: { cwd: laterCwd } } }).where(eq(issues.id, issueId));
|
||||
}
|
||||
expect(await readFile(path.join(expectedCwd, "output.txt"), "utf8")).toBe("continued");
|
||||
await expect(readFile(path.join(laterCwd, "output.txt"), "utf8")).rejects.toThrow();
|
||||
await expect(readFile(path.join(agentDefaultCwd, "output.txt"), "utf8")).rejects.toThrow();
|
||||
}, 40_000);
|
||||
|
||||
it("keeps a task directory source projectless across chat admissions with an organizational project", async () => {
|
||||
const companyId = randomUUID(), projectId = randomUUID(), agentId = randomUUID(), issueId = randomUUID();
|
||||
await db.insert(companies).values({ id: companyId, name: "Chat files", issuePrefix: `C${companyId.slice(0, 6)}`, defaultResponsibleUserId: "responsible-user" });
|
||||
|
||||
@@ -279,6 +279,7 @@ export async function prepareHeartbeatWorkspace(db: Db, input: HeartbeatWorkspac
|
||||
{
|
||||
useProjectWorkspace:
|
||||
requestedExecutionWorkspaceMode !== "agent_default",
|
||||
configuredCwd: readNonEmptyString(mergedConfig.cwd),
|
||||
anchorWorkspace: requestedShouldReuseExisting && reusableExistingExecutionWorkspace?.strategyType === "git_worktree"
|
||||
? await resolveReusedGitWorkspaceAnchor({
|
||||
agent,
|
||||
|
||||
@@ -3321,7 +3321,7 @@ export function createHeartbeatWorkspaceResolver(db: Db) {
|
||||
agent: typeof agents.$inferSelect,
|
||||
context: Record<string, unknown>,
|
||||
previousSessionParams: Record<string, unknown> | null,
|
||||
opts?: { useProjectWorkspace?: boolean | null },
|
||||
opts?: { useProjectWorkspace?: boolean | null; configuredCwd?: string | null },
|
||||
): Promise<ResolvedAnchorWorkspaceForRun> {
|
||||
const issueId =
|
||||
readNonEmptyString(context.issueId) ?? readNonEmptyString(context.taskId);
|
||||
@@ -3515,7 +3515,9 @@ export function createHeartbeatWorkspaceResolver(db: Db) {
|
||||
}
|
||||
}
|
||||
|
||||
const configuredCwd = !forceTaskDirectory ? readNonEmptyString(parseObject(agent.adapterConfig).cwd) : null;
|
||||
const configuredCwd = forceTaskDirectory ? null
|
||||
: opts?.configuredCwd !== undefined ? opts.configuredCwd
|
||||
: readNonEmptyString(parseObject(agent.adapterConfig).cwd);
|
||||
const cwd = configuredCwd ? path.resolve(configuredCwd)
|
||||
: issueId ? await materializeIsolatedTaskDirectory({ companyId: agent.companyId, issueId })
|
||||
: resolveDefaultAgentWorkspaceDir(agent.id);
|
||||
@@ -3563,6 +3565,7 @@ export function createHeartbeatWorkspaceResolver(db: Db) {
|
||||
previousSessionParams: Record<string, unknown> | null,
|
||||
opts?: {
|
||||
useProjectWorkspace?: boolean | null;
|
||||
configuredCwd?: string | null;
|
||||
executionEnvironmentDriver?: string | null;
|
||||
anchorWorkspace?: ResolvedAnchorWorkspaceForRun;
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user