mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 04:25:52 +01:00
fix(computers): delete large agent files without downloading
Hash remote files through confined descriptors in bounded chunks before editor mutations. Preserve the remote compare-and-swap removal check and read-size cap; cover a real 17 MiB file, stale deletion, and path confinement. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
b43a695170
commit
7315159c3d
6 files changed
+73
-4
No files matched your search
@@ -129,7 +129,12 @@ describe("persistent agent directories", () => {
|
||||
remoteFiles.set(relative, bytes); return { sha256: fileHash(bytes) };
|
||||
});
|
||||
const seedBytes = vi.fn();
|
||||
const remote = { root: remoteRoot, list: async () => [], listPage: async () => ({ entries: [], truncated: false }), readBytes, writeBytes, seedBytes };
|
||||
const hash = vi.fn(async (relative: string) => {
|
||||
const bytes = remoteFiles.get(relative);
|
||||
if (!bytes) throw { code: "not_found" };
|
||||
return { sha256: fileHash(bytes), size: bytes.length };
|
||||
});
|
||||
const remote = { root: remoteRoot, list: async () => [], listPage: async () => ({ entries: [], truncated: false }), readBytes, hash, writeBytes, seedBytes };
|
||||
const lookup = vi.spyOn(persistentFiles, "persistentAgentFiles").mockResolvedValue(remote as never);
|
||||
const shell = vi.spyOn(executionTargetTools, "runAdapterExecutionTargetShellCommand");
|
||||
try {
|
||||
@@ -164,6 +169,29 @@ describe("persistent agent directories", () => {
|
||||
} finally { lookup.mockRestore(); shell.mockRestore(); }
|
||||
});
|
||||
|
||||
it("deletes oversized remote files by hash without downloading and rejects stale deletion", async () => {
|
||||
let bytes: Buffer | null = Buffer.alloc(17 * 1024 * 1024, 7);
|
||||
const correctHash = fileHash(bytes);
|
||||
const hash = vi.fn(async () => bytes ? { sha256: fileHash(bytes), size: bytes.length } : Promise.reject({ code: "not_found" }));
|
||||
const readBytes = vi.fn(async () => { throw new Error("Remote read exceeds 16 MiB"); });
|
||||
const remove = vi.fn(async (_relative: string, expected: string) => {
|
||||
if (!bytes || fileHash(bytes) !== expected) throw { code: "conflict" };
|
||||
bytes = null;
|
||||
});
|
||||
const remote = { root: "/home/user/paperclip/test/agents/target", listPage: async () => ({ entries: [], truncated: false }), hash, readBytes, remove };
|
||||
const lookup = vi.spyOn(persistentFiles, "persistentAgentFiles").mockResolvedValue(remote as never);
|
||||
try {
|
||||
const store = agentFileStore(db);
|
||||
await expect(store.write({ ...target(), path: "large.bin", bytes: null, baseHash: "stale" }, board())).rejects.toMatchObject({ status: 409 });
|
||||
expect(remove).not.toHaveBeenCalled();
|
||||
expect(bytes?.length).toBe(17 * 1024 * 1024);
|
||||
await expect(store.write({ ...target(), path: "large.bin", bytes: null, baseHash: correctHash }, board())).resolves.toEqual({ contentHash: null, changed: true });
|
||||
expect(remove).toHaveBeenCalledWith("large.bin", correctHash);
|
||||
expect(bytes).toBeNull();
|
||||
expect(readBytes).not.toHaveBeenCalled();
|
||||
} finally { lookup.mockRestore(); }
|
||||
});
|
||||
|
||||
describe("warm directory ownership", () => {
|
||||
async function nextRun() {
|
||||
const runId = randomUUID();
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
writeFileSync,
|
||||
mkdirSync,
|
||||
} from "node:fs";
|
||||
import { createHash } from "node:crypto";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { spawnSync } from "node:child_process";
|
||||
@@ -168,6 +169,25 @@ describe("confined computer files", () => {
|
||||
error: "invalid",
|
||||
});
|
||||
});
|
||||
it("hashes and deletes files over the read limit while fencing changed bytes", () => {
|
||||
const f = fixture();
|
||||
f.call({ action: "seed", files: {} });
|
||||
const bytes = Buffer.alloc(17 * 1024 * 1024, 7);
|
||||
writeFileSync(join(f.root, "large.bin"), bytes);
|
||||
const expected = createHash("sha256").update(bytes).digest("hex");
|
||||
expect(f.call({ action: "read", path: "large.bin" })).toEqual({ error: "invalid" });
|
||||
expect(f.call({ action: "hash", path: "large.bin" })).toEqual({ sha256: expected, size: bytes.length });
|
||||
bytes[0] = 8;
|
||||
writeFileSync(join(f.root, "large.bin"), bytes);
|
||||
expect(f.call({ action: "remove", path: "large.bin", expectedSha256: expected })).toEqual({ error: "conflict" });
|
||||
const current = f.call({ action: "hash", path: "large.bin" });
|
||||
expect(current.sha256).not.toBe(expected);
|
||||
expect(f.call({ action: "remove", path: "large.bin", expectedSha256: current.sha256 })).toEqual({});
|
||||
expect(f.call({ action: "hash", path: "large.bin" })).toEqual({ error: "not_found" });
|
||||
symlinkSync(join(f.temp, "outside"), join(f.root, "escape"));
|
||||
expect(f.call({ action: "hash", path: "escape" })).toEqual({ error: "invalid" });
|
||||
expect(f.call({ action: "hash", path: "../outside" })).toEqual({ error: "invalid" });
|
||||
});
|
||||
it("rejects stale edits and uses no-clobber move semantics", () => {
|
||||
const f = fixture();
|
||||
f.call({ action: "seed", files: {} });
|
||||
|
||||
@@ -137,6 +137,18 @@ try:
|
||||
finally:os.close(parentfd)
|
||||
if not stat.S_ISDIR(info.st_mode) and not stat.S_ISREG(info.st_mode):fail('invalid')
|
||||
print(json.dumps({'name':name,'kind':'directory' if stat.S_ISDIR(info.st_mode) else 'file','size':info.st_size,'mtimeMs':info.st_mtime*1000}))
|
||||
elif act=='hash':
|
||||
parentfd,name=parent(path)
|
||||
try:fd=os.open(name,os.O_RDONLY|os.O_NOFOLLOW,dir_fd=parentfd)
|
||||
finally:os.close(parentfd)
|
||||
with os.fdopen(fd,'rb') as f:
|
||||
if not stat.S_ISREG(os.fstat(f.fileno()).st_mode):fail('invalid')
|
||||
h=hashlib.sha256();size=0
|
||||
while True:
|
||||
block=f.read(65536)
|
||||
if not block:break
|
||||
h.update(block);size+=len(block)
|
||||
print(json.dumps({'sha256':h.hexdigest(),'size':size}))
|
||||
elif act=='read':
|
||||
parentfd,name=parent(path)
|
||||
try:fd=os.open(name,os.O_RDONLY|os.O_NOFOLLOW,dir_fd=parentfd)
|
||||
|
||||
@@ -702,6 +702,9 @@ finally:
|
||||
async stat(path: string): Promise<{ name: string; kind: "file" | "directory"; size: number; mtimeMs: number }> {
|
||||
return remote({ action: "stat", path });
|
||||
},
|
||||
async hash(path: string): Promise<{ sha256: string; size: number }> {
|
||||
return remote({ action: "hash", path });
|
||||
},
|
||||
async remove(path: string, expectedSha256: string) {
|
||||
await remote({ action: "remove", path, expectedSha256 });
|
||||
},
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { persistentAgentFiles, readPersistentAgentFile, seedPersistentAgentHome } from "./persistent-agent-files.js";
|
||||
import { persistentAgentFiles, hashPersistentAgentFile, readPersistentAgentFile, seedPersistentAgentHome } from "./persistent-agent-files.js";
|
||||
import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target";
|
||||
import { createHash } from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
@@ -232,8 +232,8 @@ export function agentFileStore(db: Db) {
|
||||
locked(input.companyId, input.agentId, actor, true, async (tx, agent, root, bound, remote) => {
|
||||
const relative = agentFilePath(input.path);
|
||||
if (input.bytes) assertFileSize(input.bytes.length, relative);
|
||||
const remotePrevious = remote ? await readPersistentAgentFile(remote, relative) : null;
|
||||
const previous = remote ? (remotePrevious ? { hash: remotePrevious.sha256, size: remotePrevious.bytes.length } : null) : await inspectAgentFile(root, relative);
|
||||
const remotePrevious = remote ? await hashPersistentAgentFile(remote, relative) : null;
|
||||
const previous = remote ? (remotePrevious ? { hash: remotePrevious.sha256, size: remotePrevious.size } : null) : await inspectAgentFile(root, relative);
|
||||
const currentHash = previous?.hash ?? null;
|
||||
const incomingHash = input.bytes === null ? null : fileHash(input.bytes);
|
||||
if (currentHash === incomingHash) return { contentHash: currentHash, changed: false };
|
||||
|
||||
@@ -38,6 +38,12 @@ export async function readPersistentAgentFile(files: NonNullable<Awaited<ReturnT
|
||||
catch (error) { if (isMissingRemoteFile(error)) return null; throw error; }
|
||||
}
|
||||
|
||||
/** Inspect remote bytes without downloading them or applying the editor read cap. */
|
||||
export async function hashPersistentAgentFile(files: NonNullable<Awaited<ReturnType<typeof persistentAgentFiles>>>, relative: string) {
|
||||
try { return await files.hash(instructionPath(relative)); }
|
||||
catch (error) { if (isMissingRemoteFile(error)) return null; throw error; }
|
||||
}
|
||||
|
||||
/** First attachment only. Existing directories are adopted, including empty ones.
|
||||
* Later turns never read/upload the controller copy of personal files. */
|
||||
export async function seedPersistentAgentHome(files: NonNullable<Awaited<ReturnType<typeof persistentAgentFiles>>>, localRoot: string) {
|
||||
|
||||
Reference in new issue
Block a user