From 10fedc7a7d746c264ddbabe576160d839fb53346 Mon Sep 17 00:00:00 2001 From: Dylan Boudro <121908331+starmorph@users.noreply.github.com> Date: Tue, 27 Oct 2026 10:10:22 -0700 Subject: [PATCH 1/2] Canonicalize citations to tracked source blobs --- docs/2_ARCHITECTURE.md | 11 +++++- docs/3_TAKEAWAYS.md | 3 ++ prompt.md | 2 + src/runner/submit.ts | 66 +++++++++++++++++++++++++++----- test/evidence.test.ts | 26 ++++++++++++- test/helpers.ts | 31 +++++++++++++++ test/runner.test.ts | 86 +++++++++++++++++++++++++++++++++++++++++- 7 files changed, 211 insertions(+), 14 deletions(-) diff --git a/docs/2_ARCHITECTURE.md b/docs/2_ARCHITECTURE.md index f3d1ec2..6025286 100644 --- a/docs/2_ARCHITECTURE.md +++ b/docs/2_ARCHITECTURE.md @@ -206,10 +206,17 @@ 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 contained source symlinks to their canonical target paths. Reject + traversal, metadata paths (including aliases into `.git`), unavailable or + non-regular targets, and files absent from the manifest commit. Validate + line ranges against the tracked target blob at that commit, so the + returned path and evidence URL identify the content that was checked. 5. Return validation errors to the model for up to three submissions. +Git object reads are local, bounded to 5 seconds and 16 MiB per read, and do +not apply checkout filters or fetch missing objects. Refresh snapshots after +deploying citation-validation changes so new Sandboxes receive the new runner. + The runner emits one terminal `result` or `error` event. The host downloads the event file even if the live exec stream dropped, classifies the run, and checks evidence against the manifest again. Trusted host code pins full diff --git a/docs/3_TAKEAWAYS.md b/docs/3_TAKEAWAYS.md index 3f42213..0d616e3 100644 --- a/docs/3_TAKEAWAYS.md +++ b/docs/3_TAKEAWAYS.md @@ -146,6 +146,9 @@ Structured output is only a shape until it is checked. The submission tool validates every citation against the manifest and actual checkout: repository, commit, path containment, file type, and line range. +Contained source aliases are recorded with their canonical tracked target +paths, and line ranges are checked against those blobs at the manifest commit. +Aliases into `.git` and targets absent from that commit are rejected. Rejected submissions return actionable errors so the model can repair them. The trusted host checks the manifest again and constructs evidence links itself. diff --git a/prompt.md b/prompt.md index b7f2021..0393ae4 100644 --- a/prompt.md +++ b/prompt.md @@ -15,3 +15,5 @@ Rank sources from strongest to weakest: 5. Examples, comments, and TODOs (intent, not behavior). When a client and a server both appear in the manifest, the client's code is authoritative for what the client sends and does, not for how the server responds. + +For citations, use regular source files tracked at the manifest commit. If you inspect a contained source symlink, cite its tracked target; the submission tool records the canonical target path. Repository metadata, escaping aliases, and untracked targets cannot support a citation. diff --git a/src/runner/submit.ts b/src/runner/submit.ts index 4f33bcc..7e7ed79 100644 --- a/src/runner/submit.ts +++ b/src/runner/submit.ts @@ -1,4 +1,5 @@ -import { readFileSync, realpathSync, statSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { realpathSync, statSync } from "node:fs"; import { isAbsolute, join, posix, relative, resolve, sep } from "node:path"; import { type Evidence, InvestigationResult } from "../investigation.ts"; import type { RunnerInput } from "./protocol.ts"; @@ -73,7 +74,7 @@ function checkEvidence(e: Evidence, label: string, { manifest, repos_root }: Che if (e.start_line > e.end_line) { problems.push(`${label}: start_line ${e.start_line} is after end_line ${e.end_line}`); } - const file = inspectFile(join(repos_root, repo.repository), e.path, repo.repository); + const file = inspectFile(join(repos_root, repo.repository), e.path, repo.repository, repo.commit); if ("problem" in file) { problems.push(`${label}.path: ${file.problem}`); } else if (e.end_line > file.lines) { @@ -83,25 +84,71 @@ function checkEvidence(e: Evidence, label: string, { manifest, repos_root }: Che return { repository: e.repository, commit: repo.commit, path, start_line: e.start_line, end_line: e.end_line }; } -function inspectFile(repoDir: string, path: string, repository: string): { path: string; lines: number } | { problem: string } { +type FileInspection = { path: string; lines: number } | { problem: string }; +const MAX_CITATION_BYTES = 16 * 1024 * 1024; +const GIT_READ_TIMEOUT_MS = 5_000; + +function inspectFile(repoDir: string, path: string, repository: string, commit: string): FileInspection { if (isAbsolute(path)) { const hint = path.startsWith(repoDir + sep) ? ` (did you mean "${relative(repoDir, path)}"?)` : ""; return { problem: `"${path}" must be relative to the repository root${hint}` }; } if (path.split(/[\\/]/).includes("..")) return { problem: `"${path}" must not contain ".."` }; const normalized = posix.normalize(path).replace(/^\.\//, ""); - if (normalized === ".git" || normalized.startsWith(".git/")) return { problem: `"${path}" is inside .git` }; + if (normalized.split("/").includes(".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 (!statSync(real).isFile()) return { problem: `"${path}" is not a regular file` }; - return { path: normalized, lines: countLines(readFileSync(real)) }; + if (real !== root && !real.startsWith(root + sep)) return { problem: `"${path}" resolves outside ${repository}` }; + const canonical = relative(root, real).split(sep).join("/"); + if (canonical.split("/").includes(".git")) return { problem: `"${path}" resolves inside .git` }; + try { + if (!statSync(real).isFile()) return { problem: `"${path}" is not a regular file` }; + } catch { + return { problem: `"${path}" cannot be inspected in ${repository}` }; + } + + try { + const entry = readGit(repoDir, ["ls-tree", "-l", "-z", "--full-tree", commit, "--", canonical]).toString("utf8"); + const tracked = /^(100644|100755) blob ([0-9a-f]{40}) +(\d+)\t([\s\S]+)\0$/.exec(entry); + if (!tracked || tracked[4] !== canonical) { + return { problem: `"${path}" resolves to "${canonical}", which is not a tracked regular file at manifest commit ${commit}` }; + } + if (Number(tracked[3]) > MAX_CITATION_BYTES) { + return { problem: `"${path}" exceeds the ${MAX_CITATION_BYTES / 1024 / 1024} MiB citation-validation limit` }; + } + // Count the exact blob the URL will cite, rather than a symlink blob or a later working-tree version. + const content = readGit(repoDir, ["cat-file", "blob", tracked[2]!]); + return { path: canonical, lines: countLines(content) }; + } catch { + return { problem: `"${path}" cannot be read at manifest commit ${commit} (Git reads must finish within ${GIT_READ_TIMEOUT_MS} ms and ${MAX_CITATION_BYTES / 1024 / 1024} MiB)` }; + } +} + +/** Read local objects without shell interpolation, checkout filters, ambient auth, or lazy network fetches. */ +function readGit(repoDir: string, args: string[]): Buffer { + return execFileSync("git", [ + "--no-pager", "--literal-pathspecs", "--no-optional-locks", + "-c", "safe.directory=*", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=/dev/null", "-c", "credential.helper=", + "-C", repoDir, ...args, + ], { + env: { + PATH: process.env.PATH || "/usr/local/bin:/usr/bin:/bin", + GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", + GIT_NO_REPLACE_OBJECTS: "1", GIT_NO_LAZY_FETCH: "1", GIT_ALLOW_PROTOCOL: "", + }, + timeout: GIT_READ_TIMEOUT_MS, + killSignal: "SIGKILL", + maxBuffer: MAX_CITATION_BYTES, + stdio: ["ignore", "pipe", "pipe"], + }); } function countLines(content: Buffer): number { @@ -126,7 +173,8 @@ export function createSubmitTool(checkouts: Checkouts, handlers: SubmitHandlers) description: "Submit the result of the investigation. Call exactly once, as your final action, on its own. Every citation is " + "checked against the manifest and the checkout: the repository must be in the manifest, commit must be its manifest " + - "commit, path must be a file relative to the repository root, and the line range must exist in that file. " + + "commit, path must resolve to a tracked regular source file inside the repository, and the line range must exist " + + "in its manifest-commit blob. Contained source symlinks are recorded with the target path; .git targets are rejected. " + `A rejected submission records nothing; fix every listed problem and call again (${MAX_SUBMIT_ATTEMPTS} attempts in total).`, parameters: SubmitParams, executionMode: "sequential", diff --git a/test/evidence.test.ts b/test/evidence.test.ts index f614b13..ef97ee8 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 manifest-commit source blob", () => { + 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..472e4c5 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -108,6 +108,37 @@ export function makeFixture(): Fixture { return { reposRoot, scratchDir, repoDir, commit: git("rev-parse", "HEAD") }; } +/** An isolated committed source/alias corpus; its manifest commit intentionally precedes future.ts. */ +export function makeCitationFixture(): Fixture { + const fx = makeFixture(); + const g = (...args: string[]) => execFileSync("git", [...ident, "--literal-pathspecs", "-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"); + writeFileSync(join(fx.repoDir, "src/[literal].ts"), "literal first\nliteral second\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", + "generated-alias.ts": "generated.ts", + "future-alias.ts": "future.ts", + }; + for (const [path, target] of Object.entries(aliases)) symlinkSync(target, join(fx.repoDir, path)); + g("add", "--", "src/source.ts", "src/[literal].ts", "escape.txt", ...Object.keys(aliases)); + g("commit", "-q", "-m", "add citation aliases"); + const commit = g("rev-parse", "HEAD"); + writeFileSync(join(fx.repoDir, "future.ts"), "added after the manifest commit\n"); + g("add", "--", "future.ts"); + g("commit", "-q", "-m", "add future source"); + writeFileSync(join(fx.repoDir, "generated.ts"), "untracked generated source\n"); + return { ...fx, commit }; +} + 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..6df6e04 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1,10 +1,10 @@ -import { existsSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { rm } from "node:fs/promises"; 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,88 @@ describe("citation validation", () => { }); }); +describe("committed citation aliases", () => { + const fx = makeCitationFixture(); + const checkouts = { manifest: makeInput(fx).manifest, repos_root: fx.reposRoot }; + const cite = (path: string, end_line = 3) => finalAnswer([{ repository: REPO, commit: fx.commit.slice(0, 7), path, start_line: 2, end_line }]); + 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"])( + "cites the tracked source target of %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("treats tracked filenames as literal paths rather than Git pathspec patterns", () => { + const verdict = validateSubmission(cite("src/[literal].ts", 2), checkouts); + expect(verdict.ok && verdict.result.status === "final_answer" && verdict.result.evidence[0]?.path).toBe("src/[literal].ts"); + }); + + it.each(["metadata.txt", "metadata-dir/config"])("rejects the committed metadata alias %s", (path) => { + const verdict = validateSubmission(cite(path), checkouts); + expect(verdict.ok).toBe(false); + expect(!verdict.ok && verdict.problems[0]).toContain("resolves inside .git"); + }); + + it.each([ + ["escape.txt", "resolves outside"], + ["dangling.ts", "does not exist"], + ["cycle-a.ts", "does not exist"], + ["source-dir", "not a regular file"], + ["generated-alias.ts", "not a tracked regular file at manifest commit"], + ["generated.ts", "not a tracked regular file at manifest commit"], + ["future-alias.ts", "not a tracked regular file at manifest commit"], + ["future.ts", "not a tracked regular file at manifest commit"], + ])("rejects the unavailable source target %s", (path, message) => { + const verdict = validateSubmission(cite(path), checkouts); + expect(verdict.ok).toBe(false); + expect(!verdict.ok && verdict.problems[0]).toContain(message); + }); + + it("checks line bounds against the manifest blob despite working-tree changes", async () => { + const changed = makeCitationFixture(); + try { + writeFileSync(join(changed.repoDir, "src/source.ts"), "first\nsecond\nthird\nfourth\nfifth\n"); + const verdict = validateSubmission(finalAnswer([{ + repository: REPO, commit: changed.commit, path: "source-alias.ts", start_line: 4, end_line: 4, + }]), { manifest: makeInput(changed).manifest, repos_root: changed.reposRoot }); + expect(verdict.ok).toBe(false); + expect(!verdict.ok && verdict.problems[0]).toContain("src/source.ts (3 lines)"); + } finally { + await rm(dirname(changed.reposRoot), { recursive: true, force: true }); + } + }); + + it("requires a regular blob at the manifest commit even if a symlink becomes a working-tree file", async () => { + const changed = makeCitationFixture(); + try { + unlinkSync(join(changed.repoDir, "source-alias.ts")); + writeFileSync(join(changed.repoDir, "source-alias.ts"), "first\nsecond\nthird\n"); + const verdict = validateSubmission(finalAnswer([{ + repository: REPO, commit: changed.commit, path: "source-alias.ts", start_line: 2, end_line: 3, + }]), { manifest: makeInput(changed).manifest, repos_root: changed.reposRoot }); + expect(verdict.ok).toBe(false); + expect(!verdict.ok && verdict.problems[0]).toContain("not a tracked regular file at manifest commit"); + } finally { + await rm(dirname(changed.reposRoot), { recursive: true, force: true }); + } + }); + + it("reads the manifest repository independently of ambient Git redirection", () => { + const previous = process.env.GIT_DIR; + process.env.GIT_DIR = join(fx.scratchDir, "not-a-git-repository"); + try { + expect(validateSubmission(cite("source-alias.ts"), checkouts).ok).toBe(true); + } finally { + if (previous === undefined) delete process.env.GIT_DIR; + else process.env.GIT_DIR = previous; + } + }); +}); + 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]!); From e664b5e7c8d5377643804ac0461217ef2579b7e1 Mon Sep 17 00:00:00 2001 From: Jacob Prall Date: Wed, 28 Oct 2026 12:59:33 -0700 Subject: [PATCH 2/2] Canonicalize symlink citations without reading Git objects Snapshot checkouts are clean, read-only, and detached at the manifest commit, so the file on disk is the cited blob. Resolving the symlink, recording the target path, and re-checking .git on that path closes both alias cases from the issue without spawning git from the key-holding runner. Co-authored-by: Cursor --- docs/2_ARCHITECTURE.md | 13 +++----- docs/3_TAKEAWAYS.md | 3 -- prompt.md | 2 -- src/runner/submit.ts | 65 +++++++--------------------------------- test/evidence.test.ts | 2 +- test/helpers.ts | 16 +++------- test/runner.test.ts | 67 +++++------------------------------------- 7 files changed, 27 insertions(+), 141 deletions(-) diff --git a/docs/2_ARCHITECTURE.md b/docs/2_ARCHITECTURE.md index 76bd1de..fc31e68 100644 --- a/docs/2_ARCHITECTURE.md +++ b/docs/2_ARCHITECTURE.md @@ -208,17 +208,12 @@ 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 contained source symlinks to their canonical target paths. Reject - traversal, metadata paths (including aliases into `.git`), unavailable or - non-regular targets, and files absent from the manifest commit. Validate - line ranges against the tracked target blob at that commit, so the - returned path and evidence URL identify the content that was checked. +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. -Git object reads are local, bounded to 5 seconds and 16 MiB per read, and do -not apply checkout filters or fetch missing objects. Refresh snapshots after -deploying citation-validation changes so new Sandboxes receive the new runner. - The runner emits one terminal `result` or `error` event. The host downloads the event file even if the live exec stream dropped, classifies the run, and checks evidence against the manifest again. Trusted host code pins full diff --git a/docs/3_TAKEAWAYS.md b/docs/3_TAKEAWAYS.md index 9e05d6d..b831dbe 100644 --- a/docs/3_TAKEAWAYS.md +++ b/docs/3_TAKEAWAYS.md @@ -152,9 +152,6 @@ Structured output is only a shape until it is checked. The submission tool validates every citation against the manifest and actual checkout: repository, commit, path containment, file type, and line range. -Contained source aliases are recorded with their canonical tracked target -paths, and line ranges are checked against those blobs at the manifest commit. -Aliases into `.git` and targets absent from that commit are rejected. Rejected submissions return actionable errors so the model can repair them. The trusted host checks the manifest again and constructs evidence links itself. diff --git a/prompt.md b/prompt.md index 0393ae4..b7f2021 100644 --- a/prompt.md +++ b/prompt.md @@ -15,5 +15,3 @@ Rank sources from strongest to weakest: 5. Examples, comments, and TODOs (intent, not behavior). When a client and a server both appear in the manifest, the client's code is authoritative for what the client sends and does, not for how the server responds. - -For citations, use regular source files tracked at the manifest commit. If you inspect a contained source symlink, cite its tracked target; the submission tool records the canonical target path. Repository metadata, escaping aliases, and untracked targets cannot support a citation. diff --git a/src/runner/submit.ts b/src/runner/submit.ts index 7e7ed79..f9dbf4e 100644 --- a/src/runner/submit.ts +++ b/src/runner/submit.ts @@ -1,5 +1,4 @@ -import { execFileSync } from "node:child_process"; -import { realpathSync, statSync } from "node:fs"; +import { readFileSync, realpathSync, statSync } from "node:fs"; import { isAbsolute, join, posix, relative, resolve, sep } from "node:path"; import { type Evidence, InvestigationResult } from "../investigation.ts"; import type { RunnerInput } from "./protocol.ts"; @@ -74,7 +73,7 @@ function checkEvidence(e: Evidence, label: string, { manifest, repos_root }: Che if (e.start_line > e.end_line) { problems.push(`${label}: start_line ${e.start_line} is after end_line ${e.end_line}`); } - const file = inspectFile(join(repos_root, repo.repository), e.path, repo.repository, repo.commit); + const file = inspectFile(join(repos_root, repo.repository), e.path, repo.repository); if ("problem" in file) { problems.push(`${label}.path: ${file.problem}`); } else if (e.end_line > file.lines) { @@ -84,18 +83,14 @@ function checkEvidence(e: Evidence, label: string, { manifest, repos_root }: Che return { repository: e.repository, commit: repo.commit, path, start_line: e.start_line, end_line: e.end_line }; } -type FileInspection = { path: string; lines: number } | { problem: string }; -const MAX_CITATION_BYTES = 16 * 1024 * 1024; -const GIT_READ_TIMEOUT_MS = 5_000; - -function inspectFile(repoDir: string, path: string, repository: string, commit: string): FileInspection { +function inspectFile(repoDir: string, path: string, repository: string): { path: string; lines: number } | { problem: string } { if (isAbsolute(path)) { const hint = path.startsWith(repoDir + sep) ? ` (did you mean "${relative(repoDir, path)}"?)` : ""; return { problem: `"${path}" must be relative to the repository root${hint}` }; } if (path.split(/[\\/]/).includes("..")) return { problem: `"${path}" must not contain ".."` }; const normalized = posix.normalize(path).replace(/^\.\//, ""); - if (normalized.split("/").includes(".git")) return { problem: `"${path}" is inside .git` }; + if (normalized === ".git" || normalized.startsWith(".git/")) return { problem: `"${path}" is inside .git` }; let root: string; let real: string; @@ -106,49 +101,12 @@ function inspectFile(repoDir: string, path: string, repository: string, commit: 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 !== root && !real.startsWith(root + sep)) return { problem: `"${path}" resolves outside ${repository}` }; - const canonical = relative(root, real).split(sep).join("/"); - if (canonical.split("/").includes(".git")) return { problem: `"${path}" resolves inside .git` }; - try { - if (!statSync(real).isFile()) return { problem: `"${path}" is not a regular file` }; - } catch { - return { problem: `"${path}" cannot be inspected in ${repository}` }; - } - - try { - const entry = readGit(repoDir, ["ls-tree", "-l", "-z", "--full-tree", commit, "--", canonical]).toString("utf8"); - const tracked = /^(100644|100755) blob ([0-9a-f]{40}) +(\d+)\t([\s\S]+)\0$/.exec(entry); - if (!tracked || tracked[4] !== canonical) { - return { problem: `"${path}" resolves to "${canonical}", which is not a tracked regular file at manifest commit ${commit}` }; - } - if (Number(tracked[3]) > MAX_CITATION_BYTES) { - return { problem: `"${path}" exceeds the ${MAX_CITATION_BYTES / 1024 / 1024} MiB citation-validation limit` }; - } - // Count the exact blob the URL will cite, rather than a symlink blob or a later working-tree version. - const content = readGit(repoDir, ["cat-file", "blob", tracked[2]!]); - return { path: canonical, lines: countLines(content) }; - } catch { - return { problem: `"${path}" cannot be read at manifest commit ${commit} (Git reads must finish within ${GIT_READ_TIMEOUT_MS} ms and ${MAX_CITATION_BYTES / 1024 / 1024} MiB)` }; - } -} - -/** Read local objects without shell interpolation, checkout filters, ambient auth, or lazy network fetches. */ -function readGit(repoDir: string, args: string[]): Buffer { - return execFileSync("git", [ - "--no-pager", "--literal-pathspecs", "--no-optional-locks", - "-c", "safe.directory=*", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=/dev/null", "-c", "credential.helper=", - "-C", repoDir, ...args, - ], { - env: { - PATH: process.env.PATH || "/usr/local/bin:/usr/bin:/bin", - GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", - GIT_NO_REPLACE_OBJECTS: "1", GIT_NO_LAZY_FETCH: "1", GIT_ALLOW_PROTOCOL: "", - }, - timeout: GIT_READ_TIMEOUT_MS, - killSignal: "SIGKILL", - maxBuffer: MAX_CITATION_BYTES, - stdio: ["ignore", "pipe", "pipe"], - }); + 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: target, lines: countLines(readFileSync(real)) }; } function countLines(content: Buffer): number { @@ -173,8 +131,7 @@ export function createSubmitTool(checkouts: Checkouts, handlers: SubmitHandlers) description: "Submit the result of the investigation. Call exactly once, as your final action, on its own. Every citation is " + "checked against the manifest and the checkout: the repository must be in the manifest, commit must be its manifest " + - "commit, path must resolve to a tracked regular source file inside the repository, and the line range must exist " + - "in its manifest-commit blob. Contained source symlinks are recorded with the target path; .git targets are rejected. " + + "commit, path must be a file relative to the repository root, and the line range must exist in that file. " + `A rejected submission records nothing; fix every listed problem and call again (${MAX_SUBMIT_ATTEMPTS} attempts in total).`, parameters: SubmitParams, executionMode: "sequential", diff --git a/test/evidence.test.ts b/test/evidence.test.ts index ef97ee8..35d2705 100644 --- a/test/evidence.test.ts +++ b/test/evidence.test.ts @@ -87,7 +87,7 @@ describe("validated citation links", () => { const input = makeInput(fx); afterAll(() => rm(dirname(fx.reposRoot), { recursive: true, force: true })); - it("links an accepted symlink citation to its manifest-commit source blob", () => { + 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 }); diff --git a/test/helpers.ts b/test/helpers.ts index 472e4c5..3f29b6f 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -108,13 +108,12 @@ export function makeFixture(): Fixture { return { reposRoot, scratchDir, repoDir, commit: git("rev-parse", "HEAD") }; } -/** An isolated committed source/alias corpus; its manifest commit intentionally precedes future.ts. */ +/** 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, "--literal-pathspecs", "-C", fx.repoDir, ...args], { encoding: "utf8" }).trim(); + 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"); - writeFileSync(join(fx.repoDir, "src/[literal].ts"), "literal first\nliteral second\n"); const aliases = { "source-alias.ts": "src/source.ts", "source-chain.ts": "source-alias.ts", @@ -125,18 +124,11 @@ export function makeCitationFixture(): Fixture { "dangling.ts": "missing.ts", "cycle-a.ts": "cycle-b.ts", "cycle-b.ts": "cycle-a.ts", - "generated-alias.ts": "generated.ts", - "future-alias.ts": "future.ts", }; for (const [path, target] of Object.entries(aliases)) symlinkSync(target, join(fx.repoDir, path)); - g("add", "--", "src/source.ts", "src/[literal].ts", "escape.txt", ...Object.keys(aliases)); + g("add", "-A"); g("commit", "-q", "-m", "add citation aliases"); - const commit = g("rev-parse", "HEAD"); - writeFileSync(join(fx.repoDir, "future.ts"), "added after the manifest commit\n"); - g("add", "--", "future.ts"); - g("commit", "-q", "-m", "add future source"); - writeFileSync(join(fx.repoDir, "generated.ts"), "untracked generated source\n"); - return { ...fx, commit }; + return { ...fx, commit: g("rev-parse", "HEAD") }; } export function makeInput(fx: Fixture, overrides: { text?: string; timeout_ms?: number } = {}): RunnerInput { diff --git a/test/runner.test.ts b/test/runner.test.ts index 6df6e04..5ac03b5 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -1,4 +1,4 @@ -import { existsSync, readdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { rm } from "node:fs/promises"; import { isBuiltin } from "node:module"; import { dirname, join } from "node:path"; @@ -276,14 +276,14 @@ describe("citation validation", () => { }); }); -describe("committed citation aliases", () => { +describe("symlink citations", () => { const fx = makeCitationFixture(); const checkouts = { manifest: makeInput(fx).manifest, repos_root: fx.reposRoot }; - const cite = (path: string, end_line = 3) => finalAnswer([{ repository: REPO, commit: fx.commit.slice(0, 7), path, start_line: 2, end_line }]); + 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"])( - "cites the tracked source target of %s", (path) => { + "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 }, @@ -291,71 +291,18 @@ describe("committed citation aliases", () => { }, ); - it("treats tracked filenames as literal paths rather than Git pathspec patterns", () => { - const verdict = validateSubmission(cite("src/[literal].ts", 2), checkouts); - expect(verdict.ok && verdict.result.status === "final_answer" && verdict.result.evidence[0]?.path).toBe("src/[literal].ts"); - }); - - it.each(["metadata.txt", "metadata-dir/config"])("rejects the committed metadata alias %s", (path) => { - const verdict = validateSubmission(cite(path), checkouts); - expect(verdict.ok).toBe(false); - expect(!verdict.ok && verdict.problems[0]).toContain("resolves inside .git"); - }); - 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"], - ["generated-alias.ts", "not a tracked regular file at manifest commit"], - ["generated.ts", "not a tracked regular file at manifest commit"], - ["future-alias.ts", "not a tracked regular file at manifest commit"], - ["future.ts", "not a tracked regular file at manifest commit"], - ])("rejects the unavailable source target %s", (path, message) => { + ])("rejects %s", (path, message) => { const verdict = validateSubmission(cite(path), checkouts); expect(verdict.ok).toBe(false); expect(!verdict.ok && verdict.problems[0]).toContain(message); }); - - it("checks line bounds against the manifest blob despite working-tree changes", async () => { - const changed = makeCitationFixture(); - try { - writeFileSync(join(changed.repoDir, "src/source.ts"), "first\nsecond\nthird\nfourth\nfifth\n"); - const verdict = validateSubmission(finalAnswer([{ - repository: REPO, commit: changed.commit, path: "source-alias.ts", start_line: 4, end_line: 4, - }]), { manifest: makeInput(changed).manifest, repos_root: changed.reposRoot }); - expect(verdict.ok).toBe(false); - expect(!verdict.ok && verdict.problems[0]).toContain("src/source.ts (3 lines)"); - } finally { - await rm(dirname(changed.reposRoot), { recursive: true, force: true }); - } - }); - - it("requires a regular blob at the manifest commit even if a symlink becomes a working-tree file", async () => { - const changed = makeCitationFixture(); - try { - unlinkSync(join(changed.repoDir, "source-alias.ts")); - writeFileSync(join(changed.repoDir, "source-alias.ts"), "first\nsecond\nthird\n"); - const verdict = validateSubmission(finalAnswer([{ - repository: REPO, commit: changed.commit, path: "source-alias.ts", start_line: 2, end_line: 3, - }]), { manifest: makeInput(changed).manifest, repos_root: changed.reposRoot }); - expect(verdict.ok).toBe(false); - expect(!verdict.ok && verdict.problems[0]).toContain("not a tracked regular file at manifest commit"); - } finally { - await rm(dirname(changed.reposRoot), { recursive: true, force: true }); - } - }); - - it("reads the manifest repository independently of ambient Git redirection", () => { - const previous = process.env.GIT_DIR; - process.env.GIT_DIR = join(fx.scratchDir, "not-a-git-repository"); - try { - expect(validateSubmission(cite("source-alias.ts"), checkouts).ok).toBe(true); - } finally { - if (previous === undefined) delete process.env.GIT_DIR; - else process.env.GIT_DIR = previous; - } - }); }); const RUNNER_PACKAGES = new Set(["@earendil-works/pi-agent-core", "@earendil-works/pi-ai", "typebox", "zod"]);