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; + } + } +}