From 5c1ec46b6f4bd4d70afaca89ccab4be8a4b4f453 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 02:25:14 +0100 Subject: [PATCH] fix: keep a root=live listing inside the session folder (release review) The lookup judges liveWorktree.path lexically and only when the pane moves, while the route's probe and readdir both follow symlinks. A symlink put at that path, or at a parent of it, without moving the pane re-rooted the listing outside workingDir. Names and sizes leaked, while file-content and file-raw still refused the paths. The route now resolves both folders on every request and walks the worktree only when it lies strictly inside the resolved workingDir, otherwise it walks workingDir. Only the check uses the resolved paths: the walk keeps the session's spelling, so a workspace reached through a symlink still gets paths relative to workingDir. Co-Authored-By: Claude Opus 5.5 --- src/live-worktree.ts | 17 +++-- src/session.ts | 3 +- src/types/session.ts | 3 +- src/web/routes/file-routes.ts | 26 ++++++- test/routes/file-routes-live-worktree.test.ts | 72 ++++++++++++++++++- 5 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/live-worktree.ts b/src/live-worktree.ts index 997cef43..c7a688d4 100644 --- a/src/live-worktree.ts +++ b/src/live-worktree.ts @@ -13,12 +13,17 @@ * than by running `git worktree list`: this runs from the pane watcher whenever a directory * changes, and a file read starts no process, no repository config and no filter. * - * Only a folder at or below `workingDir` is ever an answer, and that is the whole security - * argument: the file routes already serve everything under `workingDir`, so following a pane - * there widens nothing, and a planted `.git` file can only move where the listing STARTS inside - * that same folder. A worktree outside it (`git worktree add ../feature`) stays unfollowed, since - * listing it would need a second confinement root. Nothing here is a confinement decision: the - * routes keep judging every path against `workingDir`. + * Only a folder strictly below `workingDir` is ever an answer: the file routes already serve + * everything under it, so following a pane there widens nothing, and a planted `.git` file can + * only move where the listing STARTS inside that same folder. A worktree outside it + * (`git worktree add ../feature`) stays unfollowed, since listing it would need a second + * confinement root. + * + * That answer is lexical and only as fresh as the last time the pane moved: a symlink can take the + * folder's place afterwards without moving the pane. So nothing here is a confinement decision. + * `GET /api/sessions/:id/files` resolves the folder on every request and walks it only while it + * really lies inside `workingDir` (`canWalkLiveWorktree()` in file-routes.ts), and the routes keep + * judging every path against `workingDir`. * * Probes are bounded (#516). The folder can sit on a mount that stopped answering, and an * `unknown` answer stops the walk rather than being read as "no `.git` here". diff --git a/src/session.ts b/src/session.ts index 0b93da0b..5b4c1f6c 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1582,7 +1582,8 @@ export class Session extends EventEmitter { /** * The linked git worktree inside `workingDir` that {@link paneCwd} is in, when there is one * (`live-worktree.ts`). The Files panel lists it instead of the main checkout. Runtime-only and - * never a confinement root, like `paneCwd`: it is always a folder the file routes already serve. + * never a confinement root, like `paneCwd`: it is judged lexically when the pane moves, and the + * listing route re-checks the real folder on every request. */ get liveWorktree(): LiveWorktree | undefined { return this._liveWorktree; diff --git a/src/types/session.ts b/src/types/session.ts index 476fa40b..d4a8043e 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -854,7 +854,8 @@ export interface SessionState { * the session's folder. Derived from {@link paneCwd} by `live-worktree.ts`, so it is * runtime-only and absent wherever `paneCwd` is, and absent for a worktree outside * `workingDir`: the file routes never serve those. NOT a confinement root; the Files panel - * only starts its listing here. + * only starts its listing here, after `GET .../files` has checked that the real folder still + * lies inside `workingDir`, since a symlink can replace it without moving the pane. */ liveWorktree?: LiveWorktree; /** Remote execution metadata, present when this session runs over SSH through local tmux */ diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index d0530f39..4d2a2b3c 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -782,6 +782,26 @@ export function isPathWithinRoot(root: string, candidate: string): boolean { return rel === '' || (!isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${sep}`)); } +/** + * Whether a `root=live` listing may start at a session's `liveWorktree` (#765). `findLiveWorktree()` + * judged that path lexically when the pane moved, and a symlink can take its place, or a parent's, + * without moving the pane. The probe and `readdir` both follow one, and the walk's `path`s only look + * relative to `workingDir`, so this is the request-time half of the rule, the same one + * `validateSessionFilePath()` applies to a file: resolved, the worktree must lie strictly inside the + * resolved `workingDir`. Only the CHECK uses the resolved paths: the walk keeps the session's spelling, + * or a workspace reached through a symlink would stop producing paths relative to `workingDir`. + * Probed first (#516); both realpaths then walk the components that probe has just answered for. + */ +async function canWalkLiveWorktree(workingDir: string, worktreePath: string): Promise { + if ((await probePathKind(worktreePath, { pastCap: true })) !== 'directory') return false; + try { + const [realRoot, realWorktree] = await Promise.all([fs.realpath(workingDir), fs.realpath(worktreePath)]); + return realWorktree !== realRoot && isPathWithinRoot(realRoot, realWorktree); + } catch { + return false; + } +} + function findMatchingPickerRoot(roots: FilesystemBrowseRoot[], candidate: string): FilesystemBrowseRoot | undefined { return roots .filter((root) => isPathWithinRoot(root.path, candidate)) @@ -1606,14 +1626,14 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even const maxDepth = Math.min(parseInt(depth || '5', 10), 10); const includeHidden = showHidden === 'true'; const workingDir = session.workingDir; - // `root=live` starts the walk at the linked worktree the pane is in (#765). That folder is - // inside `workingDir` (`live-worktree.ts`), and every `path` below stays relative to + // `root=live` starts the walk at the linked worktree the pane is in (#765), once that folder + // proves to be inside `workingDir` right now, and every `path` below stays relative to // `workingDir`, so file-content, file-raw and the edit route need nothing new and judge these // paths against the same root as any other. Only the search predicate reads a path relative // to the listing, since that is the tree the person is looking at. const liveWorktree = session.liveWorktree; const walkRoot = - root === 'live' && liveWorktree && (await probePathKind(liveWorktree.path, { pastCap: true })) === 'directory' + root === 'live' && liveWorktree && (await canWalkLiveWorktree(workingDir, liveWorktree.path)) ? liveWorktree.path : workingDir; // null for an empty/whitespace query, which is what keeps the default diff --git a/test/routes/file-routes-live-worktree.test.ts b/test/routes/file-routes-live-worktree.test.ts index f8799130..a5b8a86e 100644 --- a/test/routes/file-routes-live-worktree.test.ts +++ b/test/routes/file-routes-live-worktree.test.ts @@ -5,7 +5,7 @@ * `workingDir`: that is what keeps the preview, raw and edit routes working unchanged, inside the * same confinement. Real filesystem on purpose, since the walk is the thing under test. */ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -18,6 +18,8 @@ describe('Files panel listing of a live worktree', () => { let harness: RouteTestHarness; let repo: string; let worktree: string; + /** Removed after each test: a folder of the server user's outside the session folder. */ + let outside: string | undefined; const list = async (query = '') => { const res = await harness.app.inject({ @@ -46,8 +48,30 @@ describe('Files panel listing of a live worktree', () => { afterEach(async () => { await harness.app.close(); rmSync(repo, { recursive: true, force: true }); + if (outside) rmSync(outside, { recursive: true, force: true }); + outside = undefined; }); + /** A folder outside the session folder, holding a visible and a hidden file and a subfolder. */ + const makeOutsideFolder = () => { + outside = mkdtempSync(join(homedir(), 'live-worktree-outside-')); + mkdirSync(join(outside, 'inner')); + writeFileSync(join(outside, '.hidden-outside'), 'secret'); + writeFileSync(join(outside, 'inner', 'outside-file.txt'), 'secret'); + return outside; + }; + const expectSessionFolderListing = (data: { + root: string; + worktree: unknown; + tree: Array<{ name: string; children?: Array<{ path: string }> }>; + }) => { + expect(data.root).toBe(repo); + expect(JSON.stringify(data.tree)).not.toContain('outside'); + expect(data.tree.find((node) => node.name === 'src')?.children?.[0].path).toBe('src/main-only.ts'); + // Kept so the panel's comparison with the session's state settles instead of reloading forever. + expect(data.worktree).toEqual({ name: 'probe', path: worktree }); + }; + it('lists the worktree with paths relative to the session folder', async () => { const data = await list('&root=live'); expect(data.root).toBe(worktree); @@ -86,6 +110,52 @@ describe('Files panel listing of a live worktree', () => { expect(data.worktree).toEqual({ name: 'probe', path: worktree }); }); + it('walks the session folder when a symlink replaced the worktree with a folder outside it', async () => { + rmSync(worktree, { recursive: true }); + symlinkSync(makeOutsideFolder(), worktree); + expectSessionFolderListing(await list('&root=live&showHidden=true')); + }); + + it('does not find anything outside the session folder through such a symlink when searching', async () => { + rmSync(worktree, { recursive: true }); + symlinkSync(makeOutsideFolder(), worktree); + const data = await list(`&root=live&showHidden=true&q=${encodeURIComponent('outside')}`); + expect(data.root).toBe(repo); + expect(data.matches).toEqual([]); + }); + + it('walks the session folder when a parent of the worktree is the symlink', async () => { + const parent = join(repo, '.claude/worktrees'); + rmSync(parent, { recursive: true }); + mkdirSync(join(makeOutsideFolder(), 'probe')); + writeFileSync(join(outside as string, 'probe', 'outside-file.txt'), 'secret'); + symlinkSync(outside as string, parent); + expectSessionFolderListing(await list('&root=live&showHidden=true')); + }); + + it('walks the session folder when the worktree path is a symlink back to that folder', async () => { + rmSync(worktree, { recursive: true }); + symlinkSync(repo, worktree); + expectSessionFolderListing(await list('&root=live')); + }); + + it('still lists a worktree of a workspace reached through a symlink, in the workspace spelling', async () => { + const link = `${repo}-link`; + symlinkSync(repo, link); + try { + const spelled = join(link, PREFIX); + Object.assign(harness.ctx._session, { + workingDir: link, + liveWorktree: { name: 'probe', path: spelled }, + }); + const data = await list('&root=live'); + expect(data.root).toBe(spelled); + expect(data.tree[0].children[0].path).toBe(`${PREFIX}/src/worktree-only.ts`); + } finally { + rmSync(link); + } + }); + it('searches the worktree: the query reads paths in the listing, the result in the session folder', async () => { const data = await list(`&root=live&q=${encodeURIComponent('src/*.ts')}`); expect(data.root).toBe(worktree);