diff --git a/docs/2_ARCHITECTURE.md b/docs/2_ARCHITECTURE.md index a369380..fc31e68 100644 --- a/docs/2_ARCHITECTURE.md +++ b/docs/2_ARCHITECTURE.md @@ -208,8 +208,10 @@ The only accepted agent output is `submit_investigation`: 1. Parse `InvestigationResult`. 2. Require evidence for observed claims. 3. Validate repository name and commit against the manifest. -4. Resolve each path inside the checkout and reject traversal, `.git`, missing - files, and invalid line ranges. +4. Resolve each path inside the checkout, following symlinks, and record the + target's path so the evidence link shows the lines that were checked. + Reject traversal, paths or symlink targets inside `.git`, missing files, + and invalid line ranges. 5. Return validation errors to the model for up to three submissions. The runner emits one terminal `result` or `error` event. The host downloads diff --git a/src/runner/submit.ts b/src/runner/submit.ts index 4f33bcc..f9dbf4e 100644 --- a/src/runner/submit.ts +++ b/src/runner/submit.ts @@ -92,16 +92,21 @@ function inspectFile(repoDir: string, path: string, repository: string): { path: const normalized = posix.normalize(path).replace(/^\.\//, ""); if (normalized === ".git" || normalized.startsWith(".git/")) return { problem: `"${path}" is inside .git` }; + let root: string; let real: string; try { - real = realpathSync(resolve(repoDir, normalized)); + root = realpathSync(repoDir); + real = realpathSync(resolve(root, normalized)); } catch { const hint = normalized.startsWith(`${repository}/`) ? ` (paths are relative to the repository root; try "${normalized.slice(repository.length + 1)}")` : ""; return { problem: `"${path}" does not exist in ${repository}${hint}` }; } - if (!real.startsWith(realpathSync(repoDir) + sep)) return { problem: `"${path}" resolves outside ${repository}` }; + if (!real.startsWith(root + sep)) return { problem: `"${path}" resolves outside ${repository}` }; + // Cite the symlink's target, not the alias: the evidence URL must show the lines counted here. + const target = relative(root, real).split(sep).join("/"); + if (target.split("/").includes(".git")) return { problem: `"${path}" resolves inside .git` }; if (!statSync(real).isFile()) return { problem: `"${path}" is not a regular file` }; - return { path: normalized, lines: countLines(readFileSync(real)) }; + return { path: target, lines: countLines(readFileSync(real)) }; } function countLines(content: Buffer): number { diff --git a/test/evidence.test.ts b/test/evidence.test.ts index f614b13..35d2705 100644 --- a/test/evidence.test.ts +++ b/test/evidence.test.ts @@ -1,7 +1,11 @@ -import { describe, expect, it } from "vitest"; +import { afterAll, describe, expect, it } from "vitest"; import { buildEvidenceUrl, type Evidence, type FinalAnswer } from "../src/investigation.ts"; import type { RepoManifest } from "../src/manifest.ts"; import { filterEvidence } from "../src/tasks.ts"; +import { rm } from "node:fs/promises"; +import { dirname } from "node:path"; +import { validateSubmission } from "../src/runner/submit.ts"; +import { makeCitationFixture, makeInput, REPO } from "./helpers.ts"; const sha = "0123456789abcdef0123456789abcdef01234567"; const manifest: RepoManifest = { @@ -77,3 +81,23 @@ describe("buildEvidenceUrl", () => { ); }); }); + +describe("validated citation links", () => { + const fx = makeCitationFixture(); + const input = makeInput(fx); + afterAll(() => rm(dirname(fx.reposRoot), { recursive: true, force: true })); + + it("links an accepted symlink citation to its target file", () => { + const verdict = validateSubmission(finalAnswer([{ + repository: REPO, commit: fx.commit.slice(0, 7), path: "source-alias.ts", start_line: 2, end_line: 3, + }]), { manifest: input.manifest, repos_root: fx.reposRoot }); + expect(verdict.ok).toBe(true); + if (!verdict.ok) throw new Error(verdict.problems.join("\n")); + const { result, dropped } = filterEvidence(verdict.result, input.manifest); + expect(dropped).toEqual([]); + expect(kept(result)).toEqual([{ + repository: REPO, commit: fx.commit, path: "src/source.ts", start_line: 2, end_line: 3, + url: `https://github.com/${REPO}/blob/${fx.commit}/src/source.ts#L2-L3`, + }]); + }); +}); diff --git a/test/helpers.ts b/test/helpers.ts index 1495175..3f29b6f 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -108,6 +108,29 @@ export function makeFixture(): Fixture { return { reposRoot, scratchDir, repoDir, commit: git("rev-parse", "HEAD") }; } +/** A committed corpus of symlinks to a source file, a directory, and repository metadata. */ +export function makeCitationFixture(): Fixture { + const fx = makeFixture(); + const g = (...args: string[]) => execFileSync("git", [...ident, "-C", fx.repoDir, ...args], { encoding: "utf8" }).trim(); + mkdirSync(join(fx.repoDir, "nested")); + writeFileSync(join(fx.repoDir, "src/source.ts"), "first line\nsecond line\nthird line\n"); + const aliases = { + "source-alias.ts": "src/source.ts", + "source-chain.ts": "source-alias.ts", + "source-dir": "src", + "nested/source-dir": "../src", + "metadata.txt": ".git/config", + "metadata-dir": ".git", + "dangling.ts": "missing.ts", + "cycle-a.ts": "cycle-b.ts", + "cycle-b.ts": "cycle-a.ts", + }; + for (const [path, target] of Object.entries(aliases)) symlinkSync(target, join(fx.repoDir, path)); + g("add", "-A"); + g("commit", "-q", "-m", "add citation aliases"); + return { ...fx, commit: g("rev-parse", "HEAD") }; +} + export function makeInput(fx: Fixture, overrides: { text?: string; timeout_ms?: number } = {}): RunnerInput { return { trace_id: randomUUID(), diff --git a/test/runner.test.ts b/test/runner.test.ts index 53ce680..5ac03b5 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -4,7 +4,7 @@ import { isBuiltin } from "node:module"; import { dirname, join } from "node:path"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { fauxAssistantMessage } from "@earendil-works/pi-ai"; -import { buildRunner, CLARIFY, call, finalAnswer, makeFixture, makeInput, parseEvents, REPO, RUNNER_DIR, runCli, runScripted, say, terminal, toolResults } from "./helpers.ts"; +import { buildRunner, CLARIFY, call, finalAnswer, makeCitationFixture, makeFixture, makeInput, parseEvents, REPO, RUNNER_DIR, runCli, runScripted, say, terminal, toolResults } from "./helpers.ts"; import { apiKeyFor, takeCredentials } from "../src/runner/main.ts"; import { validateSubmission } from "../src/runner/submit.ts"; import { rejectReason } from "../src/runner/tools.ts"; @@ -276,6 +276,35 @@ describe("citation validation", () => { }); }); +describe("symlink citations", () => { + const fx = makeCitationFixture(); + const checkouts = { manifest: makeInput(fx).manifest, repos_root: fx.reposRoot }; + const cite = (path: string) => finalAnswer([{ repository: REPO, commit: fx.commit.slice(0, 7), path, start_line: 2, end_line: 3 }]); + afterAll(() => rm(dirname(fx.reposRoot), { recursive: true, force: true })); + + it.each(["source-alias.ts", "source-chain.ts", "source-dir/source.ts", "nested/source-dir/source.ts", "./src//source.ts"])( + "records the target path for %s", (path) => { + const verdict = validateSubmission(cite(path), checkouts); + expect(verdict.ok && verdict.result.status === "final_answer" && verdict.result.evidence).toEqual([ + { repository: REPO, commit: fx.commit, path: "src/source.ts", start_line: 2, end_line: 3 }, + ]); + }, + ); + + it.each([ + ["metadata.txt", "resolves inside .git"], + ["metadata-dir/config", "resolves inside .git"], + ["escape.txt", "resolves outside"], + ["dangling.ts", "does not exist"], + ["cycle-a.ts", "does not exist"], + ["source-dir", "not a regular file"], + ])("rejects %s", (path, message) => { + const verdict = validateSubmission(cite(path), checkouts); + expect(verdict.ok).toBe(false); + expect(!verdict.ok && verdict.problems[0]).toContain(message); + }); +}); + const RUNNER_PACKAGES = new Set(["@earendil-works/pi-agent-core", "@earendil-works/pi-ai", "typebox", "zod"]); const SHARED_MODULES = new Set(["investigation.ts", "manifest.ts", "util.ts"]); const importsOf = (code: string) => [...code.matchAll(/from\s+["']([^"']+)["']/g)].map((m) => m[1]!);