From 256722eeea8a41ffc206586d430f3bb10f0f0a27 Mon Sep 17 00:00:00 2001 From: herjarsa Date: Fri, 28 Aug 2026 09:41:31 +0200 Subject: [PATCH] fix(server): drop /api/fs/list hunk per btriapitsyn review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main already returns entry paths under the requested (lexical) directory, fixed separately. Re-applying the original LIST hunk introduced two regressions: shadowing of outer 'let requestedPath' inside the try block, and the gitignore filter comparing lexical entry paths against 'ignoredPaths' built from the canonical realpath. This commit drops the LIST hunk and the two list tests that accompanied it. The read-family fixes (stat/read/raw/serve) stay — those were the actual symlink resolve-before-containment fix and are not affected by the LIST regressions. Refs btriapitsyn on #2872 (2026-08-27). --- packages/web/server/lib/fs/routes.js | 16 ++---- packages/web/server/lib/fs/routes.test.js | 66 ----------------------- 2 files changed, 3 insertions(+), 79 deletions(-) diff --git a/packages/web/server/lib/fs/routes.js b/packages/web/server/lib/fs/routes.js index 34124d85..e6d8c9f0 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -1521,10 +1521,6 @@ export const registerFsRoutes = (app, dependencies) => { ? req.query.path.trim() : os.homedir(); const respectGitignore = req.query.respectGitignore === 'true'; - // Logical (requested) path stays in the caller's path space. Realpath is - // only used to read directory contents — returning real paths for entries - // breaks file-tree expansion when listing through a symlink, because the - // UI rejects expanded paths that fall outside the workspace root. let requestedPath = ''; let resolvedPath = ''; @@ -1535,13 +1531,7 @@ export const registerFsRoutes = (app, dependencies) => { }; try { - // Keep the listing directory canonical (realpath-resolved) so readdir - // and git check-ignore operate on the real directory, but expose entry - // paths under the requested (user-visible) directory. This keeps paths - // inside the workspace addressable when the requested directory is a - // symlink to a folder outside the project root — otherwise the file - // tree hands back canonical paths that the read/stat/raw routes reject. - const requestedPath = path.resolve(normalizeDirectoryPath(rawPath)); + requestedPath = path.resolve(normalizeDirectoryPath(rawPath)); resolvedPath = await realpathCache.resolve(requestedPath); const stats = await fsPromises.stat(resolvedPath); @@ -1601,8 +1591,8 @@ export const registerFsRoutes = (app, dependencies) => { const entries = await Promise.all( dirents.map(async (dirent) => { - const entryPath = path.join(requestedPath, dirent.name); - if (respectGitignore && ignoredPaths.has(entryPath)) { + const physicalEntryPath = path.join(resolvedPath, dirent.name); + if (respectGitignore && ignoredPaths.has(physicalEntryPath)) { return null; } diff --git a/packages/web/server/lib/fs/routes.test.js b/packages/web/server/lib/fs/routes.test.js index 9e801d9a..3656407a 100644 --- a/packages/web/server/lib/fs/routes.test.js +++ b/packages/web/server/lib/fs/routes.test.js @@ -204,26 +204,6 @@ const registerRaw = (fsPromises) => { return getRoute('GET', '/api/fs/raw'); }; -const registerList = (fsPromises) => { - const { app, getRoute } = createRouteRegistry(); - registerFsRoutes(app, { - os: { homedir: () => '/home/user' }, - path: path.posix, - fsPromises: { - realpath: async (targetPath) => targetPath, - ...fsPromises, - }, - spawn: vi.fn(), - crypto: { randomUUID: () => 'job-0' }, - normalizeDirectoryPath: (p) => p, - resolveProjectDirectory: async () => ({ directory: '/repo' }), - buildAugmentedPath: () => '/usr/bin', - resolveGitBinaryForSpawn: () => 'git', - openchamberUserConfigRoot: '/home/user/.config', - }); - return getRoute('GET', '/api/fs/list'); -}; - const registerMkdir = (fsPromises) => { const { app, getRoute } = createRouteRegistry(); registerFsRoutes(app, { @@ -786,52 +766,6 @@ describe('fs read', () => { warn.mockRestore(); }); }); - -describe('fs list', () => { - it('returns entry paths under the requested directory so workspace symlinks stay addressable', async () => { - const fsPromises = { - realpath: vi.fn(async (targetPath) => { - if (targetPath === '/repo/link') return '/outside/shared'; - return targetPath; - }), - stat: vi.fn(async () => ({ isDirectory: () => true })), - readdir: vi.fn(async () => [ - { name: 'file.md', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, - ]), - }; - const handler = registerList(fsPromises); - const res = createMockResponse(); - - await handler({ query: { path: '/repo/link' } }, res); - - expect(res.statusCode).toBe(200); - expect(res.body.path).toBe('/outside/shared'); - expect(res.body.entries).toEqual([ - expect.objectContaining({ name: 'file.md', path: '/repo/link/file.md' }), - ]); - }); - - it('still lists real (non-symlinked) directories with canonical entry paths', async () => { - const fsPromises = { - realpath: vi.fn(async (targetPath) => targetPath), - stat: vi.fn(async () => ({ isDirectory: () => true })), - readdir: vi.fn(async () => [ - { name: 'file.md', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, - ]), - }; - const handler = registerList(fsPromises); - const res = createMockResponse(); - - await handler({ query: { path: '/repo' } }, res); - - expect(res.statusCode).toBe(200); - expect(res.body.path).toBe('/repo'); - expect(res.body.entries).toEqual([ - expect.objectContaining({ name: 'file.md', path: '/repo/file.md' }), - ]); - }); -}); - describe('fs reveal', () => { it.each([ ['linux', 'xdg-open', ['/repo']],