Scope the config snapshot to files inside the working tree (#1596)

* Scope config snapshot to files inside the working tree

* Record excluded snapshot entries as placeholders instead of links

* Limit linked snapshot content to unmodified tracked files and tracked directories

File targets reached through a link are included only when their content is
unchanged from HEAD, and directory targets only when they contain tracked
files; anything else is recorded as a single placeholder. Adds tests for a
sensitive path that links to a tracked directory, links to untracked
directories, and links to tracked files modified after checkout.
This commit is contained in:
Ashwin Bhat 2026-08-06 10:17:40 -07:00 committed by GitHub
parent 0aee57ab82
commit e1fc925862
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 448 additions and 19 deletions

View File

@ -3,11 +3,16 @@ import {
appendFileSync,
cpSync,
existsSync,
lstatSync,
mkdirSync,
readFileSync,
readlinkSync,
realpathSync,
rmSync,
statSync,
writeFileSync,
} from "fs";
import { dirname } from "path";
import { dirname, join, posix, relative, sep } from "path";
// Paths that are both PR-controllable and read from cwd at CLI startup.
//
@ -30,19 +35,174 @@ const SENSITIVE_PATHS = [
const CLAUDE_PR_EXCLUDE_PATTERN = "/.claude-pr/";
function snapshotSensitivePath(src: string, dest: string): void {
try {
cpSync(src, dest, { recursive: true, dereference: true });
} catch (error) {
// Symlinks whose targets are absent on the PR head (e.g. `.claude/CLAUDE.md`
// -> `../AGENTS.md` when the PR deleted the target) make dereferenced
// copies throw ENOENT. Preserve the symlink for the review snapshot instead.
if (error instanceof Error && "code" in error && error.code === "ENOENT") {
cpSync(src, dest, { recursive: true });
return;
function isSameOrInside(child: string, parent: string): boolean {
return child === parent || child.startsWith(`${parent}${sep}`);
}
// Repository paths (relative to cwd, `/`-separated) that a link may resolve
// to: `files` are tracked files whose working-tree content is unchanged from
// HEAD, `dirs` are directories that contain at least one tracked file.
type TrackedPaths = { files: Set<string>; dirs: Set<string> };
// Built from the superproject only: `git ls-files` reports a submodule as a
// single entry, so paths inside a checked-out submodule are in neither set and
// links into one are recorded as placeholders.
function listTrackedPaths(): TrackedPaths {
const gitPathList = (args: string[]) =>
execFileSync("git", args, { encoding: "utf8", maxBuffer: Infinity })
.split("\0")
.filter(Boolean);
const modified = new Set(
gitPathList([
"diff",
"--name-only",
"-z",
"--relative",
"--ignore-submodules",
"HEAD",
"--",
]),
);
const tracked: TrackedPaths = { files: new Set(), dirs: new Set() };
for (const file of gitPathList(["ls-files", "-z"])) {
if (!modified.has(file)) {
tracked.files.add(file);
}
for (
let dir = posix.dirname(file);
dir !== "." && !tracked.dirs.has(dir);
dir = posix.dirname(dir)
) {
tracked.dirs.add(dir);
}
}
return tracked;
}
// The snapshot is scoped to tracked repository content and never contains
// links. An entry is copied with its content only when all of these hold:
// 1. its real target (through any links) lies inside the working tree;
// 2. no component of the target's path inside the tree is `.git`, and the
// target is not inside the snapshot directory itself;
// 3. the target does not contain a directory already on the entry's own
// path (which would recurse);
// 4. if the entry is reached through a link (it is one, or a directory above
// it inside the sensitive path is), a file target must be tracked in the
// checkout with its content unchanged from HEAD, and a directory target
// must contain at least one tracked file (see listTrackedPaths). Directory
// targets that pass are descended into and their children are checked
// individually.
// Files and directories at their literal, non-linked location are unaffected
// by rule 4 and are copied as-is. Every other entry — targets outside the
// tree, dangling or looping links, git metadata, submodule contents, untracked
// or locally modified files reached through a link — is recorded as a
// placeholder file (see recordPlaceholder), so nothing in the snapshot
// resolves anywhere else.
function shouldSnapshotContent(
entryPath: string,
workTreeRealPath: string,
tracked: TrackedPaths,
): boolean {
try {
const targetRealPath = realpathSync(entryPath);
if (!isSameOrInside(targetRealPath, workTreeRealPath)) {
return false;
}
const targetParts = relative(workTreeRealPath, targetRealPath).split(sep);
if (targetParts.includes(".git") || targetParts[0] === ".claude-pr") {
return false;
}
for (let dir = dirname(entryPath); ; dir = dirname(dir)) {
if (isSameOrInside(realpathSync(dir), targetRealPath)) {
return false;
}
if (dir === dirname(dir)) {
break;
}
}
const literalPath = join(
workTreeRealPath,
relative(process.cwd(), entryPath),
);
if (targetRealPath === literalPath) {
return true;
}
const targetRepoPath = targetParts.join("/");
return statSync(targetRealPath).isDirectory()
? tracked.dirs.has(targetRepoPath)
: tracked.files.has(targetRepoPath);
} catch {
return false;
}
}
// Writes a short regular file at `dest` describing the entry that was left
// out, so the snapshot records that something was there without linking to it.
function recordPlaceholder(src: string, dest: string): void {
console.warn(
`Snapshot: ${src} not included in snapshot; recording a placeholder`,
);
let description = "is not included in this snapshot";
try {
description = `was a symbolic link to ${JSON.stringify(readlinkSync(src))}; the link target is not included in this snapshot`;
} catch {
// Not a link (or no longer present).
}
mkdirSync(dirname(dest), { recursive: true });
writeFileSync(dest, `Snapshot placeholder: ${src} ${description}.\n`);
}
/**
* Copies a sensitive path into the review snapshot. Entries that pass the
* check above are copied dereferenced (reviewers see the effective content);
* every other entry is recorded as a placeholder file, never as a link.
* Applies per entry, including links nested inside a real directory.
*/
function snapshotSensitivePath(
src: string,
dest: string,
workTreeRealPath: string,
tracked: TrackedPaths,
): void {
const excluded: Array<{ src: string; dest: string }> = [];
const keepOrExclude =
(keep: (entry: string) => boolean) =>
(entrySrc: string, entryDest: string) => {
if (keep(entrySrc)) {
return true;
}
excluded.push({ src: entrySrc, dest: entryDest });
return false;
};
try {
cpSync(src, dest, {
recursive: true,
dereference: true,
filter: keepOrExclude((entry) =>
shouldSnapshotContent(entry, workTreeRealPath, tracked),
),
});
} catch (error) {
// Dangling links are normally caught by the filter above. If a target
// disappears between that check and the copy, the dereferencing copy
// throws ENOENT; start over without following links, recording every link
// as a placeholder, instead of failing the restore.
if (
!(error instanceof Error && "code" in error && error.code === "ENOENT")
) {
throw error;
}
rmSync(dest, { recursive: true, force: true });
excluded.length = 0;
cpSync(src, dest, {
recursive: true,
filter: keepOrExclude((entry) => !lstatSync(entry).isSymbolicLink()),
});
}
for (const entry of excluded) {
recordPlaceholder(entry.src, entry.dest);
}
}
function ensureClaudePrExcludedFromGit(): void {
@ -110,11 +270,15 @@ export function restoreConfigFromBase(baseBranch: string): void {
// Snapshot every PR-authored sensitive path into .claude-pr/ before deletion
// so review agents can inspect what the PR changes without those files ever
// being executed. Captured before the security delete so it reflects the
// PR-authored version.
// PR-authored version. Links are followed only to tracked, unmodified content
// inside the working tree; anything else is recorded as a placeholder file,
// so the snapshot itself never contains links.
rmSync(".claude-pr", { recursive: true, force: true });
const workTreeRealPath = realpathSync(process.cwd());
const tracked = listTrackedPaths();
for (const p of SENSITIVE_PATHS) {
if (existsSync(p)) {
snapshotSensitivePath(p, `.claude-pr/${p}`);
if (lstatSync(p, { throwIfNoEntry: false })) {
snapshotSensitivePath(p, `.claude-pr/${p}`, workTreeRealPath, tracked);
}
}
if (existsSync(".claude-pr")) {

View File

@ -5,6 +5,7 @@ import {
lstatSync,
mkdtempSync,
mkdirSync,
readdirSync,
readFileSync,
rmSync,
symlinkSync,
@ -147,7 +148,7 @@ describe("restoreConfigFromBase", () => {
);
});
test("snapshots symlinked sensitive paths even when the PR head target is missing", () => {
test("records dangling links as placeholders, including top-level ones", () => {
setupSymlinkedMainBranch();
git(["checkout", "pr"]);
@ -157,14 +158,229 @@ describe("restoreConfigFromBase", () => {
restoreConfigFromBase("main");
expect(lstatRepoFile(".claude-pr/.claude/CLAUDE.md").isSymbolicLink()).toBe(
true,
);
expectPlaceholder(".claude-pr/CLAUDE.md");
expectPlaceholder(".claude-pr/.claude/CLAUDE.md");
expectNoLinksInSnapshot();
expect(readRepoFile(".claude/settings.json")).toBe(
`${JSON.stringify({ source: "base" })}\n`,
);
});
test("snapshots links to tracked in-tree files as dereferenced content", () => {
setupSymlinkedMainBranch();
git(["checkout", "pr"]);
restoreConfigFromBase("main");
expect(lstatRepoFile(".claude-pr/CLAUDE.md").isFile()).toBe(true);
expect(lstatRepoFile(".claude-pr/.claude/CLAUDE.md").isFile()).toBe(true);
expect(readRepoFile(".claude-pr/CLAUDE.md")).toBe(
"shared agent instructions\n",
);
expect(readRepoFile(".claude-pr/.claude/CLAUDE.md")).toBe(
"shared agent instructions\n",
);
expectNoLinksInSnapshot();
});
test("records CLAUDE.md links to targets outside the working tree as placeholders", () => {
const outsideFile = writeOutsideFile("notes.md", "outside notes\n");
rmSync(join(repoDir, "CLAUDE.md"), { force: true });
symlinkRepoFile("CLAUDE.md", outsideFile);
git(["add", "-A"]);
git(["commit", "-m", "pr links CLAUDE.md outside the repo"]);
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/CLAUDE.md");
expect(readRepoFile(".claude-pr/CLAUDE.md")).not.toBe("outside notes\n");
expect(snapshotRegularFileContents()).not.toContain("outside notes\n");
expectNoLinksInSnapshot();
expect(readRepoFile("CLAUDE.md")).toBe("base claude instructions\n");
});
test("records nested links to targets outside the working tree as placeholders", () => {
const outsideFile = writeOutsideFile(
"secret.txt",
"outside file content\n",
);
writeOutsideFile("dir/inner.txt", "outside dir content\n");
const outsideDir = join(tempDir, "outside", "dir");
symlinkRepoFile(".claude/linked-file.md", outsideFile);
symlinkRepoFile(".claude/linked-dir", outsideDir);
git(["add", "-A"]);
git(["commit", "-m", "pr adds nested links outside the repo"]);
restoreConfigFromBase("main");
expect(readRepoFile(".claude-pr/.claude/settings.json")).toBe(
`${JSON.stringify({ source: "pr" })}\n`,
);
expectPlaceholder(".claude-pr/.claude/linked-file.md");
expectPlaceholder(".claude-pr/.claude/linked-dir");
const contents = snapshotRegularFileContents();
expect(contents).not.toContain("outside file content\n");
expect(contents).not.toContain("outside dir content\n");
expectNoLinksInSnapshot();
expect(readRepoFile(".claude/settings.json")).toBe(
`${JSON.stringify({ source: "base" })}\n`,
);
});
test("records links into git metadata as placeholders", () => {
symlinkRepoFile(".claude/git-config", "../.git/config");
git(["add", "-A"]);
git(["commit", "-m", "pr links into git metadata"]);
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/git-config");
expect(snapshotRegularFileContents()).not.toContain(
readRepoFile(".git/config"),
);
expectNoLinksInSnapshot();
});
test("records relative links that only resolve from inside the snapshot as placeholders", () => {
// Both targets dangle at their source location but would resolve to the
// repository's .git/config if re-created one directory deeper.
symlinkRepoFile(".claude/x", "../../.git/config");
rmSync(join(repoDir, "CLAUDE.md"), { force: true });
symlinkRepoFile("CLAUDE.md", "../.git/config");
git(["add", "-A"]);
git(["commit", "-m", "pr adds relative links"]);
restoreConfigFromBase("main");
const gitConfig = readRepoFile(".git/config");
for (const path of [".claude-pr/.claude/x", ".claude-pr/CLAUDE.md"]) {
expectPlaceholder(path);
expect(readRepoFile(path)).not.toBe(gitConfig);
}
expect(snapshotRegularFileContents()).not.toContain(gitConfig);
expectNoLinksInSnapshot();
});
test("records links into nested git metadata inside the working tree as placeholders", () => {
writeRepoFile("other/.git/config", "nested checkout config\n");
symlinkRepoFile(".claude/x", "../other/.git/config");
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/x");
expect(snapshotRegularFileContents()).not.toContain(
"nested checkout config\n",
);
expectNoLinksInSnapshot();
});
test("records links to untracked in-tree files as placeholders", () => {
writeRepoFile(".env", "untracked env contents\n");
symlinkRepoFile(".claude/env", "../.env");
git(["add", ".claude/env"]);
git(["commit", "-m", "pr links to an untracked file"]);
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/env");
expect(snapshotRegularFileContents()).not.toContain(
"untracked env contents\n",
);
expect(readRepoFile(".claude-pr/.claude/settings.json")).toBe(
`${JSON.stringify({ source: "pr" })}\n`,
);
expectNoLinksInSnapshot();
});
test("records links to tracked files modified after checkout as placeholders", () => {
writeRepoFile(".env", "PLACEHOLDER=1\n");
symlinkRepoFile(".claude/env", "../.env");
git(["add", ".env", ".claude/env"]);
git(["commit", "-m", "pr links to a tracked file"]);
writeRepoFile(".env", "written after checkout\n");
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/env");
expect(snapshotRegularFileContents()).not.toContain(
"written after checkout\n",
);
expectNoLinksInSnapshot();
});
test("snapshots a sensitive path that links to a tracked in-tree directory", () => {
rmSync(join(repoDir, ".claude"), { recursive: true, force: true });
writeRepoFile(
"config/claude/settings.json",
`${JSON.stringify({ source: "linked-dir" })}\n`,
);
writeRepoFile("config/claude/agents/reviewer.md", "reviewer agent\n");
writeRepoFile("docs/agents/writer.md", "writer agent\n");
symlinkRepoFile("config/claude/more-agents", "../../docs/agents");
symlinkRepoFile(".claude", "config/claude");
git(["add", "-A"]);
git(["commit", "-m", "pr links .claude to a tracked directory"]);
writeRepoFile("config/claude/local.txt", "untracked file\n");
writeRepoFile("config/claude/cache/entry.txt", "untracked dir entry\n");
restoreConfigFromBase("main");
expect(lstatRepoFile(".claude-pr/.claude").isDirectory()).toBe(true);
expect(readRepoFile(".claude-pr/.claude/settings.json")).toBe(
`${JSON.stringify({ source: "linked-dir" })}\n`,
);
expect(readRepoFile(".claude-pr/.claude/agents/reviewer.md")).toBe(
"reviewer agent\n",
);
expect(readRepoFile(".claude-pr/.claude/more-agents/writer.md")).toBe(
"writer agent\n",
);
expectPlaceholder(".claude-pr/.claude/local.txt");
expectPlaceholder(".claude-pr/.claude/cache");
const contents = snapshotRegularFileContents();
expect(contents).not.toContain("untracked file\n");
expect(contents).not.toContain("untracked dir entry\n");
expectNoLinksInSnapshot();
expect(lstatRepoFile(".claude").isDirectory()).toBe(true);
expect(readRepoFile(".claude/settings.json")).toBe(
`${JSON.stringify({ source: "base" })}\n`,
);
});
test("records links to untracked in-tree directories as a single placeholder", () => {
writeRepoFile("build/out/a.js", "generated a\n");
writeRepoFile("build/out/b.js", "generated b\n");
symlinkRepoFile(".claude/build", "../build");
git(["add", ".claude/build"]);
git(["commit", "-m", "pr links to an untracked directory"]);
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/build");
const contents = snapshotRegularFileContents();
expect(contents).not.toContain("generated a\n");
expect(contents).not.toContain("generated b\n");
expectNoLinksInSnapshot();
});
test("records links back into a parent directory as placeholders", () => {
symlinkRepoFile(".claude/parent-dir", "..");
git(["add", "-A"]);
git(["commit", "-m", "pr adds a link back to the repo root"]);
restoreConfigFromBase("main");
expectPlaceholder(".claude-pr/.claude/parent-dir");
expect(existsRepoFile(".claude-pr/.claude/parent-dir/src")).toBe(false);
expect(readRepoFile(".claude-pr/.claude/settings.json")).toBe(
`${JSON.stringify({ source: "pr" })}\n`,
);
expectNoLinksInSnapshot();
});
test("does not modify an existing .gitignore", () => {
writeRepoFile(".gitignore", "node_modules\n");
git(["add", ".gitignore"]);
@ -196,6 +412,55 @@ describe("restoreConfigFromBase", () => {
return readFileSync(join(repoDir, path), "utf8");
}
function writeOutsideFile(path: string, contents: string): string {
const fullPath = join(tempDir, "outside", path);
mkdirSync(dirname(fullPath), { recursive: true });
writeFileSync(fullPath, contents);
return fullPath;
}
// Contents of every regular file recorded in the snapshot, without following
// links, so tests can assert what actually got copied into the repository.
function snapshotRegularFileContents(): string[] {
const contents: string[] = [];
const visit = (dir: string) => {
for (const entry of readdirSync(dir)) {
const entryPath = join(dir, entry);
const stats = lstatSync(entryPath);
if (stats.isDirectory()) {
visit(entryPath);
} else if (stats.isFile()) {
contents.push(readFileSync(entryPath, "utf8"));
}
}
};
visit(join(repoDir, ".claude-pr"));
return contents;
}
// The snapshot must never contain links: every entry is a regular file or a
// real directory.
function expectNoLinksInSnapshot(): void {
const visit = (dir: string) => {
for (const entry of readdirSync(dir)) {
const entryPath = join(dir, entry);
const stats = lstatSync(entryPath);
expect(stats.isSymbolicLink()).toBe(false);
if (stats.isDirectory()) {
visit(entryPath);
}
}
};
visit(join(repoDir, ".claude-pr"));
}
function expectPlaceholder(path: string): void {
const stats = lstatRepoFile(path);
expect(stats.isSymbolicLink()).toBe(false);
expect(stats.isFile()).toBe(true);
expect(readRepoFile(path)).toStartWith("Snapshot placeholder: ");
}
function existsRepoFile(path: string): boolean {
return existsSync(join(repoDir, path));
}