feat(runner): verify workspace file references (#12368)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Runner replies can refer to files produced inside an assigned workspace > - The task page needs stable file metadata without trusting arbitrary Markdown paths > - File verification must stay within the canonical workspace even through symlinks > - Reads and previews also need strict count and byte bounds > - This pull request adds that provider-neutral file-reference boundary > - A later pull request will connect it to the Codex session driver ## Linked Issues or Issue Description **Subsystem affected** `packages/paperclip-runner` workspace file-reference discovery. **Problem or motivation** Assistant-authored Markdown can contain external, absolute, escaping, or symlinked paths. Reading those paths directly could disclose files outside the assigned workspace or retain unbounded content. **Proposed solution** Parse a bounded set of local Markdown references, normalize them relative to the workspace, verify canonical paths before reading, reject symlink escapes, and retain bounded previews plus content digests. **Alternatives considered** Leaving path handling inside a provider driver would duplicate a security-sensitive boundary and make it harder to test independently. **Roadmap alignment** This supports the Codex-first experimental runner and future provider-neutral task projection. It does not enable the runner adapter. ## What Changed - Added stable workspace file-reference records. - Added local Markdown link extraction and path normalization. - Added canonical-path and symlink-escape checks. - Added bounded file reads, previews, and SHA-256 digests. - Added focused path, preview, and symlink tests. ## Verification - `pnpm --filter @paperclipai/paperclip-runner test:typescript` - `pnpm -r typecheck` - `pnpm build` - The focused workspace-reference test has 3 passing cases. ## Risks The main risk is reading outside the assigned workspace or retaining excessive data. Tests cover absolute and external paths, symlink escapes, preview bounds, and deterministic metadata. ## Model Used OpenAI Codex with GPT-5.6 and repository tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
ac7f6ec1a3
commit
6f6415d07e
|
|
@ -0,0 +1,148 @@
|
|||
import { link, mkdir, mkdtemp, realpath, rm, symlink, unlink, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
discoverPaperclipWorkspaceFileReferences,
|
||||
paperclipWorkspaceFileReferencesFromText,
|
||||
} from "./workspace-file-reference.js";
|
||||
|
||||
describe("workspace file references", () => {
|
||||
it("normalizes Markdown links without retaining unproven file bytes", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-file-reference-"));
|
||||
try {
|
||||
await writeFile(join(root, "guide.md"), "# Guide\n\nSafe content.\n");
|
||||
const canonicalRoot = await realpath(root);
|
||||
const references = await discoverPaperclipWorkspaceFileReferences(
|
||||
root,
|
||||
`Read [the guide](${join(canonicalRoot, "guide.md")}:2).`,
|
||||
"turn-1",
|
||||
);
|
||||
expect(references).toHaveLength(1);
|
||||
expect(references[0]).toMatchObject({
|
||||
schema: "paperclip.workspace.file_reference.v1",
|
||||
source: "runner_verified",
|
||||
path: "guide.md",
|
||||
displayName: "the guide",
|
||||
presentation: "document",
|
||||
line: 2,
|
||||
preview: null,
|
||||
contentDigest: null,
|
||||
});
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects external URLs and paths outside the authorized workspace", () => {
|
||||
const references = paperclipWorkspaceFileReferencesFromText(
|
||||
"/workspace",
|
||||
"[external](https://example.com/file.md) [outside](/etc/passwd) [safe](docs/safe.md)",
|
||||
"turn-1",
|
||||
);
|
||||
expect(references.map((reference) => reference.path)).toEqual(["docs/safe.md"]);
|
||||
});
|
||||
|
||||
it("does not verify a symlink that resolves outside the workspace", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-file-reference-root-"));
|
||||
const outside = await mkdtemp(join(tmpdir(), "paperclip-file-reference-outside-"));
|
||||
try {
|
||||
const protectedPath = join(outside, "protected.md");
|
||||
await writeFile(protectedPath, "must not be disclosed");
|
||||
await symlink(protectedPath, join(root, "linked.md"));
|
||||
|
||||
await expect(discoverPaperclipWorkspaceFileReferences(
|
||||
root,
|
||||
"[linked](linked.md)",
|
||||
"turn-1",
|
||||
)).resolves.toEqual([]);
|
||||
} finally {
|
||||
await Promise.all([
|
||||
rm(root, { recursive: true, force: true }),
|
||||
rm(outside, { recursive: true, force: true }),
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it("anchors parsing and verification to a canonical workspace root", async () => {
|
||||
const parent = await mkdtemp(join(tmpdir(), "paperclip-file-reference-parent-"));
|
||||
const root = join(parent, "workspace");
|
||||
const alias = join(parent, "workspace-alias");
|
||||
try {
|
||||
await mkdir(root);
|
||||
await writeFile(join(root, "guide.md"), "# Guide\n");
|
||||
await symlink(root, alias);
|
||||
const canonicalRoot = await realpath(root);
|
||||
|
||||
await expect(discoverPaperclipWorkspaceFileReferences(
|
||||
alias,
|
||||
`[guide](${join(canonicalRoot, "guide.md")})`,
|
||||
"turn-1",
|
||||
)).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
path: "guide.md",
|
||||
source: "runner_verified",
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await rm(parent, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not expose bytes through an in-workspace hard link", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-file-reference-root-"));
|
||||
const outside = await mkdtemp(join(tmpdir(), "paperclip-file-reference-outside-"));
|
||||
try {
|
||||
const protectedPath = join(outside, "protected.md");
|
||||
await writeFile(protectedPath, "must not be disclosed");
|
||||
await link(protectedPath, join(root, "linked.md"));
|
||||
|
||||
await expect(discoverPaperclipWorkspaceFileReferences(
|
||||
root,
|
||||
"[linked](linked.md)",
|
||||
"turn-1",
|
||||
)).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
path: "linked.md",
|
||||
preview: null,
|
||||
contentDigest: null,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await Promise.all([
|
||||
rm(root, { recursive: true, force: true }),
|
||||
rm(outside, { recursive: true, force: true }),
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not expose bytes after an outside hard link is removed", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-file-reference-root-"));
|
||||
const outside = await mkdtemp(join(tmpdir(), "paperclip-file-reference-outside-"));
|
||||
try {
|
||||
const protectedPath = join(outside, "protected.md");
|
||||
await writeFile(protectedPath, "must not be disclosed");
|
||||
await link(protectedPath, join(root, "linked.md"));
|
||||
await unlink(protectedPath);
|
||||
|
||||
await expect(discoverPaperclipWorkspaceFileReferences(
|
||||
root,
|
||||
"[linked](linked.md)",
|
||||
"turn-1",
|
||||
)).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
path: "linked.md",
|
||||
preview: null,
|
||||
contentDigest: null,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await Promise.all([
|
||||
rm(root, { recursive: true, force: true }),
|
||||
rm(outside, { recursive: true, force: true }),
|
||||
]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,141 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import { realpath } from "node:fs/promises";
|
||||
import { basename, extname, isAbsolute, relative, resolve, sep } from "node:path";
|
||||
|
||||
export const PAPERCLIP_WORKSPACE_FILE_REFERENCE_SCHEMA = "paperclip.workspace.file_reference.v1" as const;
|
||||
|
||||
export interface PaperclipWorkspaceFileReference {
|
||||
schema: typeof PAPERCLIP_WORKSPACE_FILE_REFERENCE_SCHEMA;
|
||||
referenceId: string;
|
||||
source: "harness_reported" | "runner_verified";
|
||||
path: string;
|
||||
displayName: string;
|
||||
mediaType: string | null;
|
||||
presentation: "document" | "code" | "image" | "generic";
|
||||
line: number | null;
|
||||
preview: string | null;
|
||||
previewTruncated: boolean;
|
||||
contentDigest: string | null;
|
||||
}
|
||||
|
||||
const MAX_REFERENCES = 32;
|
||||
const MAX_LINE_NUMBER = 1_000_000;
|
||||
|
||||
function media(path: string): Pick<PaperclipWorkspaceFileReference, "mediaType" | "presentation"> {
|
||||
const extension = extname(path).toLowerCase();
|
||||
if ([".md", ".mdx", ".txt", ".rst"].includes(extension)) {
|
||||
return { mediaType: extension === ".md" || extension === ".mdx" ? "text/markdown" : "text/plain", presentation: "document" };
|
||||
}
|
||||
if ([".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg"].includes(extension)) {
|
||||
const imageType = extension === ".jpg" ? "jpeg" : extension.slice(1);
|
||||
return { mediaType: `image/${imageType}`, presentation: "image" };
|
||||
}
|
||||
if ([".ts", ".tsx", ".js", ".jsx", ".json", ".rs", ".py", ".go", ".java", ".css", ".html", ".yaml", ".yml", ".toml", ".sh"].includes(extension)) {
|
||||
return { mediaType: "text/plain", presentation: "code" };
|
||||
}
|
||||
return { mediaType: null, presentation: "generic" };
|
||||
}
|
||||
|
||||
function safeLineNumber(value: string): number | null {
|
||||
const line = Number(value);
|
||||
return Number.isSafeInteger(line) && line > 0 && line <= MAX_LINE_NUMBER
|
||||
? line
|
||||
: null;
|
||||
}
|
||||
|
||||
function localTarget(rawTarget: string): { target: string; line: number | null } | null {
|
||||
let target = rawTarget.trim();
|
||||
if (target.startsWith("<") && target.endsWith(">")) target = target.slice(1, -1);
|
||||
if (/^[a-z][a-z0-9+.-]*:/i.test(target) && !target.startsWith("file:")) return null;
|
||||
if (target.startsWith("file://")) {
|
||||
try { target = decodeURIComponent(new URL(target).pathname); } catch { return null; }
|
||||
} else {
|
||||
try { target = decodeURIComponent(target); } catch { return null; }
|
||||
}
|
||||
let line: number | null = null;
|
||||
const hashLine = target.match(/#L(\d+)(?:C\d+)?$/i);
|
||||
if (hashLine) {
|
||||
line = safeLineNumber(hashLine[1]);
|
||||
target = target.slice(0, hashLine.index);
|
||||
} else {
|
||||
const suffixLine = target.match(/:(\d+)$/);
|
||||
if (suffixLine) {
|
||||
line = safeLineNumber(suffixLine[1]);
|
||||
target = target.slice(0, suffixLine.index);
|
||||
}
|
||||
}
|
||||
return target.length === 0 ? null : { target, line };
|
||||
}
|
||||
|
||||
function workspaceRelativePath(root: string, target: string): string | null {
|
||||
const candidate = relative(root, target);
|
||||
if (
|
||||
!candidate ||
|
||||
candidate === ".." ||
|
||||
candidate.startsWith(`..${sep}`) ||
|
||||
isAbsolute(candidate)
|
||||
) return null;
|
||||
return candidate.split(sep).join("/");
|
||||
}
|
||||
|
||||
export function paperclipWorkspaceFileReferencesFromText(
|
||||
workspace: string,
|
||||
assistantText: string,
|
||||
turnId: string,
|
||||
source: PaperclipWorkspaceFileReference["source"] = "harness_reported",
|
||||
): PaperclipWorkspaceFileReference[] {
|
||||
const root = resolve(workspace);
|
||||
const references: PaperclipWorkspaceFileReference[] = [];
|
||||
const seen = new Set<string>();
|
||||
const links = assistantText.matchAll(/\[([^\]]+)]\((<[^>]+>|[^)\s]+)(?:\s+["'][^"']*["'])?\)/g);
|
||||
for (const match of links) {
|
||||
if (references.length >= MAX_REFERENCES) break;
|
||||
const parsed = localTarget(match[2] ?? "");
|
||||
if (parsed === null) continue;
|
||||
const relativePath = workspaceRelativePath(root, resolve(root, parsed.target));
|
||||
if (relativePath === null) continue;
|
||||
const key = `${relativePath}:${parsed.line ?? ""}`;
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
const identity = createHash("sha256").update(`${turnId}\0${key}`).digest("hex").slice(0, 24);
|
||||
references.push({
|
||||
schema: PAPERCLIP_WORKSPACE_FILE_REFERENCE_SCHEMA,
|
||||
referenceId: `${turnId}:file:${identity}`,
|
||||
source,
|
||||
path: relativePath,
|
||||
displayName: (match[1]?.trim() || basename(relativePath)).slice(0, 255),
|
||||
...media(relativePath),
|
||||
line: parsed.line,
|
||||
preview: null,
|
||||
previewTruncated: false,
|
||||
contentDigest: null,
|
||||
});
|
||||
}
|
||||
return references;
|
||||
}
|
||||
|
||||
export async function discoverPaperclipWorkspaceFileReferences(
|
||||
workspace: string,
|
||||
assistantText: string,
|
||||
turnId: string,
|
||||
): Promise<PaperclipWorkspaceFileReference[]> {
|
||||
let canonicalRoot: string;
|
||||
try { canonicalRoot = await realpath(workspace); } catch { return []; }
|
||||
// Resolve references from the same canonical root used for the containment
|
||||
// check. Do not consult a caller-controlled workspace symlink again after
|
||||
// canonicalizing it above.
|
||||
const references = paperclipWorkspaceFileReferencesFromText(canonicalRoot, assistantText, turnId, "runner_verified");
|
||||
const verified: PaperclipWorkspaceFileReference[] = [];
|
||||
for (const reference of references) {
|
||||
let canonicalPath: string;
|
||||
try { canonicalPath = await realpath(resolve(canonicalRoot, reference.path)); } catch { continue; }
|
||||
if (workspaceRelativePath(canonicalRoot, canonicalPath) === null) continue;
|
||||
// POSIX exposes an inode's current links, not where its bytes originated.
|
||||
// Once an outside hard link is removed, an in-workspace link is
|
||||
// indistinguishable from a file created inside the workspace. Keep the
|
||||
// canonical, bounded reference metadata but do not retain bytes until a
|
||||
// trusted workspace snapshot/manifest can prove file provenance.
|
||||
verified.push(reference);
|
||||
}
|
||||
return verified;
|
||||
}
|
||||
Loading…
Reference in New Issue