mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
Fix fake sandbox cleanup of sealed runtime directories
This commit is contained in:
1 parent
ce332ecec0
commit
bf078da227
3 files changed
+174
-3
No files matched your search
@@ -0,0 +1,127 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { access, chmod, mkdir, mkdtemp, readFile, rename, rm, stat, symlink, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createEnvironmentTestHarness } from "@paperclipai/plugin-sdk/testing";
|
||||
import manifest from "./manifest.js";
|
||||
import plugin from "./plugin.js";
|
||||
|
||||
vi.mock("node:fs/promises", async (importOriginal) => {
|
||||
const original = await importOriginal<typeof import("node:fs/promises")>();
|
||||
return { ...original, rm: vi.fn(original.rm) };
|
||||
});
|
||||
|
||||
async function createLease() {
|
||||
const definition = plugin.definition;
|
||||
const harness = createEnvironmentTestHarness({
|
||||
manifest,
|
||||
environmentDriver: {
|
||||
driverKey: "fake-plugin",
|
||||
onAcquireLease: definition.onEnvironmentAcquireLease,
|
||||
onReleaseLease: definition.onEnvironmentReleaseLease,
|
||||
onInspectLease: definition.onEnvironmentInspectLease,
|
||||
onDestroyLease: definition.onEnvironmentDestroyLease,
|
||||
},
|
||||
});
|
||||
const base = {
|
||||
driverKey: "fake-plugin",
|
||||
companyId: "company-cleanup",
|
||||
environmentId: randomUUID(),
|
||||
config: { image: "fake:cleanup", reuseLease: false },
|
||||
};
|
||||
const lease = await harness.acquireLease({ ...base, runId: randomUUID() });
|
||||
const root = lease.metadata?.fakeRootDir;
|
||||
const providerLeaseId = lease.providerLeaseId;
|
||||
if (typeof root !== "string") throw new Error("Fake lease did not identify its owned root");
|
||||
if (typeof providerLeaseId !== "string") throw new Error("Fake lease did not identify its provider lease");
|
||||
return { harness, root, params: { ...base, providerLeaseId } };
|
||||
}
|
||||
|
||||
describe("fake sandbox lease cleanup", () => {
|
||||
it.each(["destroy", "release"] as const)("%s removes sealed native runtime context", async (operation) => {
|
||||
const { harness, root, params } = await createLease();
|
||||
const instructions = path.join(root, "workspace/.paperclip-runtime/paperclip-runner/sessions/session/filesystem/context/instructions");
|
||||
const nested = path.join(instructions, "nested");
|
||||
await mkdir(nested, { recursive: true });
|
||||
await writeFile(path.join(nested, "AGENTS.md"), "Assigned immutable instructions", { mode: 0o444 });
|
||||
await chmod(nested, 0o555);
|
||||
await chmod(instructions, 0o555);
|
||||
try {
|
||||
const result = operation === "destroy"
|
||||
? await harness.destroyLease(params)
|
||||
: await harness.releaseLease(params);
|
||||
expect(result).toBeUndefined();
|
||||
await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
|
||||
} finally {
|
||||
await chmod(instructions, 0o700).catch(() => undefined);
|
||||
await chmod(nested, 0o700).catch(() => undefined);
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not change a symlink target outside the lease tree", async () => {
|
||||
const { harness, root, params } = await createLease();
|
||||
const outside = await mkdtemp(path.join(os.tmpdir(), "paperclip-fake-cleanup-outside-"));
|
||||
const target = path.join(outside, "sealed");
|
||||
const file = path.join(target, "instructions.md");
|
||||
await mkdir(target);
|
||||
await writeFile(file, "Unrelated instructions", { mode: 0o444 });
|
||||
await chmod(target, 0o500);
|
||||
await symlink(target, path.join(root, "workspace/outside-context"), "dir");
|
||||
const mode = (await stat(target)).mode;
|
||||
try {
|
||||
await harness.destroyLease(params);
|
||||
await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect((await stat(target)).mode).toBe(mode);
|
||||
expect(await readFile(file, "utf8")).toBe("Unrelated instructions");
|
||||
expect((await stat(file)).mode & 0o777).toBe(0o444);
|
||||
} finally {
|
||||
await chmod(target, 0o700);
|
||||
await rm(outside, { recursive: true, force: true });
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("retains a failed cleanup for inspection and retry", async () => {
|
||||
const { harness, root, params } = await createLease();
|
||||
const failure = Object.assign(new Error("Injected removal failure"), { code: "EACCES" });
|
||||
vi.mocked(rm).mockRejectedValueOnce(failure);
|
||||
try {
|
||||
await expect(harness.destroyLease(params)).rejects.toBe(failure);
|
||||
expect(await harness.inspectLease(params)).toMatchObject({ state: "active" });
|
||||
expect((await stat(root)).isDirectory()).toBe(true);
|
||||
|
||||
await expect(harness.destroyLease(params)).resolves.toBeUndefined();
|
||||
await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a replaced root without changing it or forgetting the lease", async () => {
|
||||
const { harness, root, params } = await createLease();
|
||||
const retained = `${root}-retained`;
|
||||
await rename(root, retained);
|
||||
await mkdir(root);
|
||||
const file = path.join(root, "unrelated.txt");
|
||||
await writeFile(file, "Unrelated replacement");
|
||||
await chmod(root, 0o500);
|
||||
const mode = (await stat(root)).mode;
|
||||
try {
|
||||
await expect(harness.destroyLease(params)).rejects.toThrow("replaced fake sandbox lease root");
|
||||
expect(await harness.inspectLease(params)).toMatchObject({ state: "active" });
|
||||
expect((await stat(root)).mode).toBe(mode);
|
||||
expect(await readFile(file, "utf8")).toBe("Unrelated replacement");
|
||||
} finally {
|
||||
await chmod(root, 0o700);
|
||||
await rm(root, { recursive: true, force: true });
|
||||
await rename(retained, root);
|
||||
await harness.destroyLease(params);
|
||||
}
|
||||
await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,40 @@
|
||||
import { constants } from "node:fs";
|
||||
import { open, readdir, rm } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
export interface LeaseRootIdentity {
|
||||
dev: number;
|
||||
ino: number;
|
||||
}
|
||||
|
||||
async function makeDirectoriesWritable(directory: string, expected?: LeaseRootIdentity): Promise<boolean> {
|
||||
const handle = await open(
|
||||
directory,
|
||||
constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW,
|
||||
).catch((error: NodeJS.ErrnoException) => {
|
||||
if (error.code === "ENOENT") return null;
|
||||
throw error;
|
||||
});
|
||||
if (!handle) return false;
|
||||
try {
|
||||
const identity = await handle.stat();
|
||||
if (!identity.isDirectory() || (expected && (identity.dev !== expected.dev || identity.ino !== expected.ino))) {
|
||||
throw new Error("Refusing to remove a replaced fake sandbox lease root");
|
||||
}
|
||||
// Native context directories are sealed. Restore deletion permissions on
|
||||
// directories only; opening without following links protects outside targets.
|
||||
await handle.chmod(0o700);
|
||||
for (const entry of await readdir(directory, { withFileTypes: true })) {
|
||||
if (entry.isDirectory()) await makeDirectoriesWritable(path.join(directory, entry.name));
|
||||
}
|
||||
return true;
|
||||
} finally {
|
||||
await handle.close();
|
||||
}
|
||||
}
|
||||
|
||||
export async function removeOwnedLeaseDirectory(rootDir: string, identity: LeaseRootIdentity): Promise<void> {
|
||||
if (await makeDirectoriesWritable(rootDir, identity)) {
|
||||
await rm(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
@@ -1,9 +1,10 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdir, mkdtemp, rm } from "node:fs/promises";
|
||||
import { lstat, mkdir, mkdtemp } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { spawn } from "node:child_process";
|
||||
import { definePlugin } from "@paperclipai/plugin-sdk";
|
||||
import { removeOwnedLeaseDirectory, type LeaseRootIdentity } from "./lease-cleanup.js";
|
||||
import type {
|
||||
PluginEnvironmentAcquireLeaseParams,
|
||||
PluginEnvironmentCancelInteractiveSetupParams,
|
||||
@@ -40,6 +41,7 @@ interface FakeDriverConfig {
|
||||
interface FakeLeaseState {
|
||||
providerLeaseId: string;
|
||||
rootDir: string;
|
||||
rootIdentity: LeaseRootIdentity;
|
||||
remoteCwd: string;
|
||||
image: string;
|
||||
reuseLease: boolean;
|
||||
@@ -87,11 +89,13 @@ async function createLeaseState(input: {
|
||||
reuseLease: boolean;
|
||||
}): Promise<FakeLeaseState> {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-fake-sandbox-"));
|
||||
const { dev, ino } = await lstat(rootDir);
|
||||
const remoteCwd = path.join(rootDir, "workspace");
|
||||
await mkdir(remoteCwd, { recursive: true });
|
||||
const state = {
|
||||
providerLeaseId: input.providerLeaseId,
|
||||
rootDir,
|
||||
rootIdentity: { dev, ino },
|
||||
remoteCwd,
|
||||
image: input.image,
|
||||
reuseLease: input.reuseLease,
|
||||
@@ -183,9 +187,9 @@ function cancelStatusFromReason(
|
||||
async function removeLease(providerLeaseId: string | null | undefined): Promise<void> {
|
||||
if (!providerLeaseId) return;
|
||||
const state = leases.get(providerLeaseId);
|
||||
leases.delete(providerLeaseId);
|
||||
if (state) {
|
||||
await rm(state.rootDir, { recursive: true, force: true });
|
||||
await removeOwnedLeaseDirectory(state.rootDir, state.rootIdentity);
|
||||
leases.delete(providerLeaseId);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user