From 0f74e3b918242f57fac01a54db4b74c99681ea60 Mon Sep 17 00:00:00 2001 From: Brynne Taylor <7542439+brynne8@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:16:48 +0800 Subject: [PATCH 1/2] fix(files): list the changed files of a project behind a link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git rev-parse --show-toplevel` answers with the repository's real path, while the caller asks about the cwd it was given: `/tmp` and `/var/folders/…` are links on macOS, so the two spell the same folder differently. Every path git reported was then resolved against the real root and kept only if it matched the caller's cwd as a string, which never happens through a link — a project reached that way had an empty changed-file list: no changed badge, no line counts, no entries at all. - `resolveExistingPath()` (`lib/paths.ts`) resolves the nearest existing ancestor and keeps the remainder as written, so a deleted file works too. `samePath()` stays lexical and says so; this is the resolving half. - `getGitStatus()` rebases every reported path onto the cwd it was asked about, and takes its line-stat pathspec from the resolved cwd. - `getGitFileDiff()` compares and derives its pathspec from the resolved pair, so a path named through a link still resolves to the file Git means, and a path that leaves the repository stays refused. --- docs/agents/files-and-access.md | 1 + lib/git-changes.test.mjs | 103 ++++++++++++++++++++++++++++++++ lib/git-changes.ts | 42 ++++++++++--- lib/paths.test.mjs | 31 ++++++++++ lib/paths.ts | 25 +++++++- 5 files changed, 194 insertions(+), 8 deletions(-) diff --git a/docs/agents/files-and-access.md b/docs/agents/files-and-access.md index 94bef4bacb..095791524e 100644 --- a/docs/agents/files-and-access.md +++ b/docs/agents/files-and-access.md @@ -8,6 +8,7 @@ - Two places switch or create worktrees, one component (`components/ProjectWorktreePicker.tsx`) on the sidebar's one listing of its cwd (`GET /api/worktrees?cwd=`, refetched when the cwd moves). The files' worktree box (the section below the sessions or the Files tab, whichever the layout shows; in the head above the explorer, beside the project box below the sessions, under it in the tab) switches, creates and removes; it shows at the top of a git checkout, a disabled box says why in a subdirectory of one (its root has the worktrees), and a folder outside any repository has no worktree box at all, the project box taking the whole width (the listing's `isGit` tells the two apart). A folder the listing last found outside git shows no "checking" box while it is listed again, which would only go and widen the project box: the browser keeps the last 50 such folders (`loadNonGitCwds()` / `rememberCwdGitStatus()` in `lib/sidebar-prefs.ts`, read-modify-write so other tabs' stay; one found in git leaves), so a reload knows them before its listing answers, the default dated folder included; only a folder this browser never listed shows the box once. The bar above a fresh composer (`components/NewSessionContextBar.tsx`) shows the same list under the same rule, and removes nothing. Both create through the sidebar (`createWorktree()`, also `SessionSidebarControl.createWorktree()`: `POST` with the project root); the files' picker then moves the sidebar's cwd there, the bar starts the new session there; opening either menu refetches the listing. Removal is `DELETE` with the project root; a dirty checkout answers 409 `dirty` and the picker asks before `force`. A group's "+" asks nothing: it starts in the current worktree or another project's root. Every GET `allowFileRoot()`s each listed worktree (the in-memory grant from `addWorktree` does not survive a restart), so a worktree picked there is browsable before it has a session. Clicking a session still moves the cwd to its worktree implicitly. - The sidebar names its cwd's project with `projectForCwd()` (`lib/new-session-context.ts`): a folder it just validated, the listing of that cwd, a checkout in the listing, then the session list. The listing is still the previous cwd's for the commit after a move, and its checkouts count only when it is a checkout's (`isTopLevel`): a subdirectory's lists the repo's checkouts under the subdirectory's own key, so a click on a root session right after it gave that session the subdirectory's key, and `handleCwdChange` closed it as a project switch and rewrote the URL to `/` (#1289). - git prints POSIX-style absolute paths even on Windows: every path read from git goes through `toNativePath()` (`lib/paths.ts`) before it is compared or returned, and paths are compared with `samePath()`, never `===` (otherwise `isTopLevel` is always false on Windows and the worktree switcher hides). Branch names are not paths and keep their forward slashes. Browser code cannot apply Node path rules, so `/api/worktrees` resolves `currentWorktreePath` server-side and the sidebar uses it for highlighting and the removal fallback. +- `git rev-parse --show-toplevel` answers with the repository's real path, while the cwd a caller gives may reach it through a link (`/tmp` and `/var/folders/…` are links on macOS): `lib/git-changes.ts` compares the two with `resolveExistingPath()` (`lib/paths.ts`, the nearest existing ancestor resolved and the rest kept as written), so a project behind a link still lists its changed files and still answers a diff, while a file that resolves outside the repository is still refused. ## File access allow-list - `/api/files` is not a general filesystem browser. Allowed roots: session cwds, their resolved project roots, and roots added with `allowFileRoot()`, which `/api/cwd/validate` and `/api/worktrees` call when they make a location browsable. "Use default directory" too: `/api/default-cwd` only creates `~/pi-cwd/YYYYMMDD`, and the sidebar selects it through `/api/cwd/validate`. diff --git a/lib/git-changes.test.mjs b/lib/git-changes.test.mjs index 54c264547b..c14db55b50 100644 --- a/lib/git-changes.test.mjs +++ b/lib/git-changes.test.mjs @@ -1,10 +1,22 @@ import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import nodePath from "node:path"; import test from "node:test"; +import { createJiti } from "jiti"; + +const posix = process.platform !== "win32"; async function loadSubject() { return import("./git-status.ts"); } +async function loadChanges() { + const jiti = createJiti(import.meta.url, { tsconfigPaths: true }); + return jiti.import("./git-changes.ts"); +} + test("parses null-delimited Git status entries including renames", async () => { const { parseGitPorcelainV1 } = await loadSubject(); const entries = parseGitPorcelainV1([ @@ -50,3 +62,94 @@ test("classifies Git status for explorer badges", async () => { assert.deepEqual(classify("UU"), { status: "conflict", code: "C" }); assert.deepEqual(classify(" D"), { status: "deleted", code: "D" }); }); + +/** A repository with one modified tracked file, and a link that leads to it. */ +function linkedRepo() { + const root = fs.mkdtempSync(nodePath.join(os.tmpdir(), "pi-web-git-linked-")); + const real = nodePath.join(root, "real-repo"); + fs.mkdirSync(real, { recursive: true }); + fs.writeFileSync(nodePath.join(real, "a.txt"), "one\n"); + const git = (...args) => execFileSync("git", ["-C", real, ...args], { stdio: "ignore" }); + git("init", "-q"); + for (const [key, value] of [ + ["user.name", "pi-web test"], + ["user.email", "test@example.invalid"], + ["core.autocrlf", "false"], + ["core.fsmonitor", "false"], + ]) git("config", "--local", key, value); + git("add", "--", "a.txt"); + git("commit", "-qm", "seed"); + fs.writeFileSync(nodePath.join(real, "a.txt"), "two\n"); + const link = nodePath.join(root, "link-repo"); + fs.symlinkSync(real, link); + return { root, real, link }; +} + +test("a repository reached through a link reports files under the folder it was given", async (t) => { + if (!posix) { t.skip("a symlink of one's own needs privileges on Windows"); return; } + const { getGitStatus, getGitFileDiff } = await loadChanges(); + const { root, link } = linkedRepo(); + try { + // What a file listing of that folder hands to a row: the link's own prefix. + const listed = nodePath.join(link, "a.txt"); + const status = await getGitStatus(link); + + assert.equal(status.isGitRepository, true); + const entry = status.files.find((file) => file.filePath === listed); + assert.ok(entry, `a row's path carries its change; listed ${JSON.stringify(status.files.map((file) => file.filePath))}`); + assert.equal(entry.status, "modified"); + // The line stats follow the folder as well, so the header's counts are this folder's. + assert.ok(status.additions >= 1, `additions: ${status.additions}`); + + // And the diff route's lookup answers for the same path. + const diff = await getGitFileDiff(link, listed); + assert.equal(diff.supported, true); + assert.ok(diff.patch.includes("+two")); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test("a repository above the folder asked about contributes no rows", async (t) => { + if (!posix) { t.skip("a symlink of one's own needs privileges on Windows"); return; } + const { getGitStatus } = await loadChanges(); + const { root, real, link } = linkedRepo(); + try { + // A subfolder of the work tree still sees the file below it, and nothing above. + const sub = nodePath.join(real, "sub"); + fs.mkdirSync(sub); + fs.writeFileSync(nodePath.join(sub, "b.txt"), "b\n"); + execFileSync("git", ["-C", real, "add", "--", "sub/b.txt"], { stdio: "ignore" }); + execFileSync("git", ["-C", real, "commit", "-qm", "sub"], { stdio: "ignore" }); + fs.writeFileSync(nodePath.join(sub, "b.txt"), "b changed\n"); + + const inside = await getGitStatus(nodePath.join(link, "sub")); + assert.deepEqual(inside.files.map((file) => file.filePath), [nodePath.join(link, "sub", "b.txt")]); + + // A folder beside the work tree is not in it: the upward search finds nothing (the + // temporary folder above is no repository), so there is no view of changes to show. + const outside = nodePath.join(root, "outside"); + fs.mkdirSync(outside); + const empty = await getGitStatus(outside); + assert.equal(empty.isGitRepository, false); + assert.deepEqual(empty.files, []); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test("a file outside the linked repository is still refused", async (t) => { + if (!posix) { t.skip("a symlink of one's own needs privileges on Windows"); return; } + const { getGitFileDiff } = await loadChanges(); + const { root, link } = linkedRepo(); + try { + // The link does not widen what may be read: a file that is not in the + // repository stays out, however it is named. + const outside = nodePath.join(root, "outside.txt"); + fs.writeFileSync(outside, "not ours\n"); + const diff = await getGitFileDiff(link, outside); + assert.equal(diff.supported, false); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/lib/git-changes.ts b/lib/git-changes.ts index d42c7a9fe0..c6e2e80e6d 100644 --- a/lib/git-changes.ts +++ b/lib/git-changes.ts @@ -13,6 +13,7 @@ import { parseGitPorcelainV1, type GitPorcelainEntry, } from "./git-status"; +import { resolveExistingPath } from "./paths"; const execFileAsync = promisify(execFile); const GIT_TIMEOUT_MS = 10_000; @@ -115,13 +116,22 @@ export async function getGitStatus(cwd: string): Promise { }; } + // Git names the repository by its real path, while the caller asked about a cwd that + // may reach it through a link (`/tmp` and `/var/folders/…` are links on macOS). Every + // path reported here is therefore rebased onto the cwd as given: the file tree lists + // what it was given, and a row can only carry its change if the two spell the path the + // same way. `repositoryRoot` stays what Git says, since it is the repository's own + // name and is where the git commands run. + const resolvedCwd = resolveExistingPath(cwd); const [entries, trackedLineStats] = await Promise.all([ readStatusEntries(repositoryRoot), - readTrackedLineStats(repositoryRoot, cwd), + readTrackedLineStats(repositoryRoot, resolvedCwd), ]); const files = entries.flatMap((entry): GitFileStatus[] => { - const filePath = path.resolve(repositoryRoot, entry.path); - if (!isWithinPath(cwd, filePath)) return []; + const relativeToCwd = path.relative(resolvedCwd, path.resolve(repositoryRoot, entry.path)); + if (!relativeToCwd || relativeToCwd === ".." || relativeToCwd.startsWith(`..${path.sep}`) + || path.isAbsolute(relativeToCwd)) return []; + const filePath = path.join(cwd, relativeToCwd); const classified = classifyGitStatus(entry); return [{ filePath, @@ -206,18 +216,36 @@ async function findFileRepositoryRoot(cwd: string, filePath: string): Promise { const repositoryRoot = await findFileRepositoryRoot(cwd, filePath); - if (!repositoryRoot || !isWithinPath(repositoryRoot, filePath)) return { supported: false }; + if (!repositoryRoot) return { supported: false }; + // The repository names itself by its real path, while the file may have been named + // through a link (`/tmp` and `/var/folders/…` are links on macOS): the same place, + // spelled two ways. Both spellings are tried, and one that leaves the repository is + // dropped — a file outside the repository the caller asked about stays out. + const realRoot = resolveExistingPath(repositoryRoot); + const realFilePath = resolveExistingPath(filePath); + const relativePaths = [ + path.relative(repositoryRoot, path.resolve(filePath)), + path.relative(realRoot, realFilePath), + ] + .filter((relative) => relative && relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)) + .map(toGitPath); + if (relativePaths.length === 0) return { supported: false }; // Reading the index runs a repository-configured fsmonitor hook. Opening a // file in another checkout (one nested in the cwd, say) must not run its hook, // as expanding that folder in the file tree does not (lib/file-tree-visibility.ts); // the cwd's own repository keeps its fsmonitor. - const gitConfig = isWithinPath(repositoryRoot, cwd) ? [] : ["-c", "core.fsmonitor=false"]; + const gitConfig = isWithinPath(repositoryRoot, cwd) || isWithinPath(realRoot, resolveExistingPath(cwd)) + ? [] : ["-c", "core.fsmonitor=false"]; const resolvedFilePath = path.resolve(filePath); - const relativePath = toGitPath(path.relative(repositoryRoot, resolvedFilePath)); const entries = await readStatusEntries(repositoryRoot, gitConfig); - const entry = entries.find((candidate) => candidate.path === relativePath); + // The path as given comes first: a link inside the repository is a link of its own + // to git, and the file it names is the one the caller asked about. + const entry = relativePaths + .map((relative) => entries.find((candidate) => candidate.path === relative)) + .find((candidate) => candidate); if (!entry) return { supported: false }; + const relativePath = entry.path; const { status } = classifyGitStatus(entry); if (status === "deleted") { diff --git a/lib/paths.test.mjs b/lib/paths.test.mjs index b36178cd24..2c185c67c1 100644 --- a/lib/paths.test.mjs +++ b/lib/paths.test.mjs @@ -1,4 +1,7 @@ import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; const isWindows = process.platform === "win32"; @@ -79,3 +82,31 @@ test("filePathFromApiSegments restores UNC roots", async () => { "/192.0.2.1/share/dir", ); }); + +test("resolveExistingPath names a folder by the path the filesystem gives it", async () => { + const { resolveExistingPath } = await loadSubject(); + const root = mkdtempSync(join(tmpdir(), "pi-web-paths-")); + try { + const real = join(root, "real"); + mkdirSync(real); + writeFileSync(join(real, "a.txt"), "a\n"); + const link = join(root, "link"); + symlinkSync(real, link); + + // The same place, whatever string it was reached by — which is what git and the + // file listing disagree about on macOS, where /tmp and /var/folders are links. + assert.equal(resolveExistingPath(link), realpathSync(real)); + assert.equal(resolveExistingPath(real), realpathSync(real)); + assert.equal(resolveExistingPath(link), resolveExistingPath(real)); + + // A path whose end does not exist yet keeps that part as written. + assert.equal( + resolveExistingPath(join(link, "gone.txt")), + join(realpathSync(real), "gone.txt"), + ); + // And a path that exists nowhere still answers with itself, resolved. + assert.equal(resolveExistingPath("/no/such/place/anywhere"), "/no/such/place/anywhere"); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/lib/paths.ts b/lib/paths.ts index 6cb385a8fc..8a79dee41e 100644 --- a/lib/paths.ts +++ b/lib/paths.ts @@ -1,4 +1,5 @@ -import { normalize, parse, sep } from "path"; +import { realpathSync } from "fs"; +import { dirname, join, normalize, parse, relative, resolve, sep } from "path"; // ============================================================================ // Path primitives. @@ -78,3 +79,25 @@ export function samePath(a: string, b: string): boolean { } return normalizedA === normalizedB; } + +/** + * One path with every symlink in it that exists resolved, the missing remainder kept + * as written. `samePath()` compares lexically and leaves symlinks to the caller, and + * this is what that means in practice: `/tmp` and `/var/folders/…` are links on + * macOS, so a folder reached through one is not the same *string* as what the + * filesystem and git name it, though it is the same place. A deleted file has nothing + * of its own to resolve, so its nearest existing ancestor is what gets resolved. + */ +export function resolveExistingPath(target: string): string { + const absolute = resolve(target); + let candidate = absolute; + for (;;) { + try { + return join(realpathSync(candidate), relative(candidate, absolute)); + } catch { + const parent = dirname(candidate); + if (parent === candidate) return absolute; + candidate = parent; + } + } +} From cd1073841e49af9f9ea814d8e477f8b801a9a0e9 Mon Sep 17 00:00:00 2001 From: Brynne Taylor <7542439+brynne8@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:24:54 +0800 Subject: [PATCH 2/2] chore: re-run the e2e job (unrelated navigation flake)