mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(runner): guide missing deliverables to the task workspace (#18226)
Explain missing task-workspace deliverables and preserve safe remote file errors across output streams. Keep the existing file confinement and granted-location contracts. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4de82bc12e
commit
1e984cc9ca
5 files changed
+123
-10
No files matched your search
@@ -46,6 +46,12 @@ to an external chat provider. If registration fails, use the returned error to
|
||||
resolve the failure or explain the limitation; do not describe a workspace path
|
||||
as an uploaded file.
|
||||
|
||||
With workspace-relative file access, create or copy task deliverables into the
|
||||
directory returned by `get_workspace.cwd` before registration. `AGENT_HOME`
|
||||
holds personal instructions and memory. If registration reports a missing
|
||||
workspace file, copy the file to that task directory and retry with a relative
|
||||
`contentRef`.
|
||||
|
||||
## Legacy adapters
|
||||
|
||||
Use Bash to run the helper bundled with the Paperclip skill from the repo root; installed skill files may not retain executable permissions:
|
||||
|
||||
@@ -1678,3 +1678,27 @@ describe("PaperclipRunnerToolAuthority", () => {
|
||||
).rejects.toThrow("paperclip_runner_tool_binding_not_authorized");
|
||||
});
|
||||
});
|
||||
|
||||
describe("deliverable tool location guidance", () => {
|
||||
const binding = { companyId: "company", agentId: "agent", issueId: "issue", runId: "run", workspaceRoot: "/task-workspace", apiToolsEnabled: false };
|
||||
const db = new Proxy({} as ConstructorParameters<typeof PaperclipRunnerToolAuthority>[0], {
|
||||
get() { throw new Error("Tool definitions must not query the database"); },
|
||||
});
|
||||
it.each(["local", "remote"] as const)("directs %s workspace-only deliverables to the task cwd", (executionTargetKind) => {
|
||||
const authority = new PaperclipRunnerToolAuthority(db, { ...binding, executionTargetKind,
|
||||
...(executionTargetKind === "remote" ? { readRemoteWorkspaceFile: async () => Buffer.alloc(0) } : {}),
|
||||
});
|
||||
const tool = authority.definitions().find(tool => tool.name === "register_deliverable")!;
|
||||
const description = (tool.inputSchema as { properties: { contentRef: { description: string } } }).properties.contentRef.description;
|
||||
expect(description).toContain("get_workspace.cwd");
|
||||
expect(description).toContain("AGENT_HOME");
|
||||
expect(description).toContain("Create or copy");
|
||||
});
|
||||
it("preserves the granted-location reader contract", () => {
|
||||
const authority = new PaperclipRunnerToolAuthority(db, { ...binding, executionTargetKind: "remote", readRemoteTaskFile: async () => Buffer.alloc(0) });
|
||||
const tool = authority.definitions().find(tool => tool.name === "register_deliverable")!;
|
||||
const description = (tool.inputSchema as { properties: { contentRef: { description: string } } }).properties.contentRef.description;
|
||||
expect(description).toContain("absolute within a location granted to this task");
|
||||
expect(description).toContain("including its agent home");
|
||||
});
|
||||
});
|
||||
@@ -368,7 +368,7 @@ export class PaperclipRunnerToolAuthority {
|
||||
...(descriptor.inputSchema.properties?.contentRef ?? {}),
|
||||
description: this.binding.readRemoteTaskFile
|
||||
? "Path to the file on the execution host: workspace-relative or absolute within a location granted to this task, including its agent home."
|
||||
: "Workspace-relative source path. Absolute paths, URLs, traversal, and symlinks are rejected.",
|
||||
: "Workspace-relative source path under get_workspace.cwd. Create or copy the file there before registering; AGENT_HOME is for personal instructions and memory. Absolute paths, URLs, traversal, and symlinks are rejected.",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { execFile, spawnSync } from "node:child_process";
|
||||
import { link, mkdir, mkdtemp, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { copyFile, link, mkdir, mkdtemp, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -32,6 +32,43 @@ describe("remote deliverable admission", () => {
|
||||
.rejects.toThrow("paperclip_runner_file_handoff_remote_read_failed");
|
||||
});
|
||||
|
||||
it.each(["path_denied", "symlink_denied", "file_changed", "hash_mismatch"])("preserves the safe %s error from either output stream", async (suffix) => {
|
||||
const code = `paperclip_runner_file_handoff_${suffix}`;
|
||||
for (const stream of ["stdout", "stderr"]) {
|
||||
const execute = vi.fn().mockResolvedValue({ ...success, exitCode: 1, stdout: "", stderr: "", [stream]: ` ${code}\n` });
|
||||
await expect(readVerifiedRemoteWorkspaceFile({ ...request, runner: { execute } })).rejects.toThrow(new Error(code));
|
||||
}
|
||||
});
|
||||
|
||||
it("explains a missing task workspace file from either output stream without exposing paths", async () => {
|
||||
const code = "paperclip_runner_file_handoff_workspace_file_missing";
|
||||
for (const stream of ["stdout", "stderr"]) {
|
||||
const execute = vi.fn().mockResolvedValue({ ...success, exitCode: 1, stdout: "", stderr: "", [stream]: code });
|
||||
await expect(readVerifiedRemoteWorkspaceFile({ ...request, runner: { execute } })).rejects.toThrow(new Error(
|
||||
`${code}: Create or copy the file into get_workspace.cwd, then retry with a workspace-relative contentRef. AGENT_HOME is for personal instructions and memory.`,
|
||||
));
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
"/private/provider/details",
|
||||
"paperclip_runner_file_handoff_unknown_failure",
|
||||
"paperclip_runner_file_handoff_workspace_file_missing\n/private/provider/details",
|
||||
"warning: paperclip_runner_file_handoff_hash_mismatch",
|
||||
"paperclip_runner_file_handoff_hash_mismatch extra",
|
||||
"paperclip_runner_file_handoff_hash_mismatch\n/private/provider/details",
|
||||
"paperclip_runner_file_handoff_hash_mismatch\nAPI_KEY=private-value",
|
||||
'{"error":"paperclip_runner_file_handoff_hash_mismatch"}',
|
||||
"\u001b[31mpaperclip_runner_file_handoff_hash_mismatch\u001b[0m",
|
||||
"paperclip_runner_file_handoff_hash_mismatch\u0000",
|
||||
])("does not expose unrecognized remote output: %j", async (output) => {
|
||||
for (const stream of ["stdout", "stderr"]) {
|
||||
const execute = vi.fn().mockResolvedValue({ ...success, exitCode: 1, stdout: "", stderr: "", [stream]: output });
|
||||
await expect(readVerifiedRemoteWorkspaceFile({ ...request, runner: { execute } }))
|
||||
.rejects.toThrow(new Error("paperclip_runner_file_handoff_remote_read_failed"));
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ stdout: "" }, { stdout: success.stdout + "\n" }, { stdout: "?".repeat(success.stdout.length) },
|
||||
{ stdout: Buffer.alloc(body.length, 65).toString("base64") }, { timedOut: true }, { exitCode: 1, stderr: "/private/provider/details" },
|
||||
@@ -45,6 +82,7 @@ describe.skipIf(!hasLinuxNode)("remote deliverable real Linux descriptor reads",
|
||||
let root: string;
|
||||
let workspaceRoot: string;
|
||||
let preload: string | undefined;
|
||||
let mergedOutput: boolean;
|
||||
let runner: Pick<CommandManagedRuntimeRunner, "execute">;
|
||||
|
||||
beforeEach(async () => {
|
||||
@@ -53,6 +91,7 @@ describe.skipIf(!hasLinuxNode)("remote deliverable real Linux descriptor reads",
|
||||
await mkdir(workspaceRoot);
|
||||
await writeFile(join(workspaceRoot, "checklist.md"), body);
|
||||
preload = undefined;
|
||||
mergedOutput = false;
|
||||
runner = { execute: vi.fn(async (input) => {
|
||||
const nodeArgs = [...(preload ? ["--require", preload] : []), ...(input.args ?? [])];
|
||||
const command = process.platform === "linux" ? process.execPath : "docker";
|
||||
@@ -64,7 +103,8 @@ describe.skipIf(!hasLinuxNode)("remote deliverable real Linux descriptor reads",
|
||||
execFile(command, args, { encoding: "utf8", timeout: input.timeoutMs, maxBuffer: 16 * 1024 * 1024,
|
||||
env: { ...process.env, NODE_OPTIONS: "", NODE_PATH: "" } }, (error, stdout, stderr) => resolve({
|
||||
pid: null, startedAt: new Date().toISOString(), exitCode: error ? 1 : 0,
|
||||
signal: null, timedOut: Boolean(error?.killed), stdout, stderr,
|
||||
signal: null, timedOut: Boolean(error?.killed),
|
||||
stdout: mergedOutput ? stdout + stderr : stdout, stderr: mergedOutput ? "" : stderr,
|
||||
}));
|
||||
});
|
||||
}) };
|
||||
@@ -81,6 +121,42 @@ describe.skipIf(!hasLinuxNode)("remote deliverable real Linux descriptor reads",
|
||||
expect(await readFile(join(workspaceRoot, "checklist.md"))).toEqual(body);
|
||||
});
|
||||
|
||||
it("reads a 155-byte proof and preserves a real hash error through merged output", async () => {
|
||||
const proof = Buffer.from(JSON.stringify({
|
||||
nonce: "abcdefghijklmnopqrstuvwx", count: 7, total: 271391, sha256: "d".repeat(64),
|
||||
}, null, 2) + "\n");
|
||||
expect(proof.length).toBe(155);
|
||||
await writeFile(join(workspaceRoot, "connection-proof.json"), proof);
|
||||
mergedOutput = true;
|
||||
const input = { runner, workspaceRoot, contentRef: "connection-proof.json", byteSize: proof.length, sha256: digest(proof) };
|
||||
await expect(readVerifiedRemoteWorkspaceFile(input)).resolves.toEqual(proof);
|
||||
await expect(readVerifiedRemoteWorkspaceFile({ ...input, sha256: "0".repeat(64) }))
|
||||
.rejects.toThrow(new Error("paperclip_runner_file_handoff_hash_mismatch"));
|
||||
});
|
||||
|
||||
it("requires an agent-home deliverable to be copied into the task workspace before registration", async () => {
|
||||
const proof = Buffer.from(JSON.stringify({
|
||||
nonce: "abcdefghijklmnopqrstuvwx", count: 7, total: 271391, sha256: "d".repeat(64),
|
||||
}, null, 2) + "\n");
|
||||
expect(proof.length).toBe(155);
|
||||
const agentHome = join(workspaceRoot, ".paperclip-runtime", "agent-files", "agent", "origin");
|
||||
await mkdir(agentHome, { recursive: true });
|
||||
await writeFile(join(agentHome, "connection-proof.json"), proof);
|
||||
mergedOutput = true;
|
||||
const input = { runner, workspaceRoot, contentRef: "connection-proof.json", byteSize: proof.length, sha256: digest(proof) };
|
||||
await expect(readVerifiedRemoteWorkspaceFile(input)).rejects.toThrow("paperclip_runner_file_handoff_workspace_file_missing");
|
||||
await expect(readFile(join(workspaceRoot, "connection-proof.json"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
expect(await readFile(join(agentHome, "connection-proof.json"))).toEqual(proof);
|
||||
await copyFile(join(agentHome, "connection-proof.json"), join(workspaceRoot, "connection-proof.json"));
|
||||
await expect(readVerifiedRemoteWorkspaceFile(input)).resolves.toEqual(proof);
|
||||
expect(await readFile(join(agentHome, "connection-proof.json"))).toEqual(proof);
|
||||
});
|
||||
|
||||
it("does not classify an unavailable workspace root as a missing deliverable", async () => {
|
||||
await expect(readVerifiedRemoteWorkspaceFile({ ...request, workspaceRoot: join(root, "missing-workspace"), runner }))
|
||||
.rejects.toThrow(new Error("paperclip_runner_file_handoff_remote_read_failed"));
|
||||
});
|
||||
|
||||
it("passes metacharacters as data, never shell syntax", async () => {
|
||||
const contentRef = "draft ' $(touch SHOULD_NOT_EXIST).md";
|
||||
await writeFile(join(workspaceRoot, contentRef), body);
|
||||
|
||||
@@ -21,11 +21,15 @@ const within = (root, file) => {
|
||||
const relative = path.relative(root, file);
|
||||
return relative && relative !== '..' && !relative.startsWith('../') && !path.isAbsolute(relative);
|
||||
};
|
||||
async function noSymlinks(root, relative) {
|
||||
async function noSymlinks(root, relative, missingCode) {
|
||||
let current = root;
|
||||
for (const segment of relative.split('/')) {
|
||||
current = path.join(current, segment);
|
||||
if ((await fs.lstat(current)).isSymbolicLink()) fail('symlink_denied');
|
||||
const stat = await fs.lstat(current).catch(error => {
|
||||
if (missingCode && error.code === 'ENOENT') fail(missingCode);
|
||||
throw error;
|
||||
});
|
||||
if (stat.isSymbolicLink()) fail('symlink_denied');
|
||||
}
|
||||
}
|
||||
(async () => {
|
||||
@@ -35,7 +39,7 @@ async function noSymlinks(root, relative) {
|
||||
const relative = path.normalize(input.contentRef);
|
||||
const candidate = path.resolve(root, relative);
|
||||
if (!within(root, candidate)) fail('path_denied');
|
||||
await noSymlinks(root, relative);
|
||||
await noSymlinks(root, relative, 'workspace_file_missing');
|
||||
const canonical = await fs.realpath(candidate);
|
||||
if (!within(root, canonical)) fail('path_denied');
|
||||
const pathBefore = await fs.lstat(canonical, { bigint: true });
|
||||
@@ -65,7 +69,7 @@ async function noSymlinks(root, relative) {
|
||||
await new Promise((resolve, reject) => process.stdout.write(body.toString('base64'), error => error ? reject(error) : resolve()));
|
||||
} finally { await handle.close(); }
|
||||
})().catch(error => {
|
||||
const allowed = /^paperclip_runner_file_handoff_(path_denied|symlink_denied|file_changed|hash_mismatch)$/;
|
||||
const allowed = /^paperclip_runner_file_handoff_(path_denied|symlink_denied|file_changed|hash_mismatch|workspace_file_missing)$/;
|
||||
process.stderr.write(allowed.test(error.message) ? error.message : 'paperclip_runner_file_handoff_remote_read_failed');
|
||||
process.exitCode = 1;
|
||||
});
|
||||
@@ -103,9 +107,12 @@ export async function readVerifiedRemoteWorkspaceFile(input: {
|
||||
}).catch(() => { throw new Error(`${PREFIX}remote_read_failed`); });
|
||||
if (result.timedOut) throw new Error(`${PREFIX}remote_read_timeout`);
|
||||
if (result.exitCode !== 0) {
|
||||
const code = result.stderr.trim();
|
||||
throw new Error(/^paperclip_runner_file_handoff_(path_denied|symlink_denied|file_changed|hash_mismatch)$/u.test(code)
|
||||
? code : `${PREFIX}remote_read_failed`);
|
||||
const code = [result.stderr.trim(), result.stdout.trim()].find((output) =>
|
||||
/^paperclip_runner_file_handoff_(path_denied|symlink_denied|file_changed|hash_mismatch|workspace_file_missing)$/u.test(output));
|
||||
if (code === `${PREFIX}workspace_file_missing`) {
|
||||
throw new Error(`${code}: Create or copy the file into get_workspace.cwd, then retry with a workspace-relative contentRef. AGENT_HOME is for personal instructions and memory.`);
|
||||
}
|
||||
throw new Error(code ?? `${PREFIX}remote_read_failed`);
|
||||
}
|
||||
const encoded = result.stdout;
|
||||
if (encoded.length !== 4 * Math.ceil(byteSize / 3)) throw new Error(`${PREFIX}size_denied`);
|
||||
|
||||
Reference in new issue
Block a user