diff --git a/packages/ui/src/stores/useFilesViewTabsStore.test.ts b/packages/ui/src/stores/useFilesViewTabsStore.test.ts index 1937e3e1..661959fc 100644 --- a/packages/ui/src/stores/useFilesViewTabsStore.test.ts +++ b/packages/ui/src/stores/useFilesViewTabsStore.test.ts @@ -29,6 +29,20 @@ describe('useFilesViewTabsStore', () => { expect(useFilesViewTabsStore.getState().byRoot[root]?.expandedPaths).toEqual(['/repo/src']); }); + test('rejects realpath children of workspace symlinks (issue 2627)', () => { + const root = '/workspace'; + const store = useFilesViewTabsStore.getState(); + + store.toggleExpandedPath(root, '/workspace/pkg'); + store.toggleExpandedPath(root, '/real/pkg/src'); + store.toggleExpandedPath(root, '/workspace/pkg/src'); + + expect(useFilesViewTabsStore.getState().byRoot[root]?.expandedPaths).toEqual([ + '/workspace/pkg', + '/workspace/pkg/src', + ]); + }); + test('removes stale expanded paths by prefix without closing files', () => { const root = '/repo'; const store = useFilesViewTabsStore.getState(); diff --git a/packages/web/server/lib/fs/DOCUMENTATION.md b/packages/web/server/lib/fs/DOCUMENTATION.md index bc853ce9..6c82a832 100644 --- a/packages/web/server/lib/fs/DOCUMENTATION.md +++ b/packages/web/server/lib/fs/DOCUMENTATION.md @@ -35,3 +35,4 @@ Own filesystem API behavior for the web server runtime, including workspace-boun ## Notes for contributors - Keep filesystem policy (workspace root checks, error mapping, exec timeout behavior) inside this module, not in the composition root. - If adding new `/api/fs/*` endpoints, add them in `routes.js` and extend this document. +- `GET /api/fs/list` may resolve symlinks with `realpath` to read directory contents, but the response `path` and each entry `path` must stay in the caller's requested path space (`path.join(requestedPath, name)`). Returning real paths breaks file-tree expansion for directories reached through workspace symlinks. diff --git a/packages/web/server/lib/fs/routes.js b/packages/web/server/lib/fs/routes.js index 81a29d8e..95887cf5 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -454,7 +454,7 @@ export const registerFsRoutes = (app, dependencies) => { // Non-cacheable commands always execute and are never stored. const runCommandWithGitReadCache = async ({ shell, shellFlag, command, resolvedCwd }) => { const cacheable = gitReadCacheTtlMs > 0 && isCacheableGitReadCommand(command); - const cacheKey = cacheable ? `${resolvedCwd}${normalizeCommand(command)}` : null; + const cacheKey = cacheable ? `${resolvedCwd}${normalizeCommand(command)}` : null; if (cacheKey) { const cached = gitReadCache.get(cacheKey); @@ -1296,6 +1296,11 @@ 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 = ''; const isPlansDirectory = (value) => { @@ -1305,7 +1310,8 @@ export const registerFsRoutes = (app, dependencies) => { }; try { - resolvedPath = await realpathCache.resolve(path.resolve(normalizeDirectoryPath(rawPath))); + requestedPath = path.resolve(normalizeDirectoryPath(rawPath)); + resolvedPath = await realpathCache.resolve(requestedPath); const stats = await fsPromises.stat(resolvedPath); if (!stats.isDirectory()) { @@ -1364,8 +1370,8 @@ export const registerFsRoutes = (app, dependencies) => { const entries = await Promise.all( dirents.map(async (dirent) => { - const entryPath = path.join(resolvedPath, dirent.name); - if (respectGitignore && ignoredPaths.has(entryPath)) { + const physicalEntryPath = path.join(resolvedPath, dirent.name); + if (respectGitignore && ignoredPaths.has(physicalEntryPath)) { return null; } @@ -1374,7 +1380,7 @@ export const registerFsRoutes = (app, dependencies) => { if (!isDirectory && isSymbolicLink) { try { - const linkStats = await fsPromises.stat(entryPath); + const linkStats = await fsPromises.stat(physicalEntryPath); isDirectory = linkStats.isDirectory(); } catch { isDirectory = false; @@ -1383,7 +1389,7 @@ export const registerFsRoutes = (app, dependencies) => { return { name: dirent.name, - path: entryPath, + path: path.join(requestedPath, dirent.name), isDirectory, isFile: dirent.isFile(), isSymbolicLink, @@ -1392,19 +1398,23 @@ export const registerFsRoutes = (app, dependencies) => { ); return res.json({ - path: resolvedPath, + path: requestedPath, entries: entries.filter(Boolean), }); } catch (error) { const err = error; const code = err && typeof err === 'object' && 'code' in err ? err.code : undefined; - const isPlansPath = code === 'ENOENT' && (isPlansDirectory(resolvedPath) || isPlansDirectory(rawPath)); + const isPlansPath = code === 'ENOENT' && ( + isPlansDirectory(resolvedPath) + || isPlansDirectory(requestedPath) + || isPlansDirectory(rawPath) + ); if (code !== 'ENOENT') { console.error('Failed to list directory:', error); } if (code === 'ENOENT') { if (isPlansPath) { - return res.json({ path: resolvedPath || rawPath, entries: [] }); + return res.json({ path: requestedPath || resolvedPath || rawPath, entries: [] }); } return res.status(404).json({ error: 'Directory not found' }); } diff --git a/packages/web/server/lib/fs/routes.test.js b/packages/web/server/lib/fs/routes.test.js index 3d815e78..576c0964 100644 --- a/packages/web/server/lib/fs/routes.test.js +++ b/packages/web/server/lib/fs/routes.test.js @@ -635,3 +635,78 @@ describe('fs raw download Content-Disposition', () => { expect(cd).toContain("filename*=UTF-8''readme.txt"); }); }); + +describe('fs list symlink path space (issue 2627)', () => { + 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: '/workspace' }), + buildAugmentedPath: () => '/usr/bin', + resolveGitBinaryForSpawn: () => 'git', + openchamberUserConfigRoot: '/home/user/.config', + }); + return getRoute('GET', '/api/fs/list'); + }; + + const callList = async (handler, query) => { + const res = createMockResponse(); + await handler({ query }, res); + return res; + }; + + it('keeps entry paths in the requested path space when listing through a symlink', async () => { + const dirents = [ + { + name: 'src', + isDirectory: () => true, + isSymbolicLink: () => false, + isFile: () => false, + }, + { + name: 'README.md', + isDirectory: () => false, + isSymbolicLink: () => false, + isFile: () => true, + }, + ]; + const fsPromises = { + realpath: vi.fn(async (targetPath) => ( + targetPath === '/workspace/pkg' ? '/real/pkg' : targetPath + )), + stat: vi.fn(async () => ({ isDirectory: () => true })), + readdir: vi.fn(async () => dirents), + }; + const handler = registerList(fsPromises); + + const res = await callList(handler, { path: '/workspace/pkg' }); + + expect(res.statusCode).toBe(200); + expect(res.body.path).toBe('/workspace/pkg'); + expect(res.body.entries).toEqual([ + { + name: 'src', + path: '/workspace/pkg/src', + isDirectory: true, + isFile: false, + isSymbolicLink: false, + }, + { + name: 'README.md', + path: '/workspace/pkg/README.md', + isDirectory: false, + isFile: true, + isSymbolicLink: false, + }, + ]); + expect(fsPromises.readdir).toHaveBeenCalledWith('/real/pkg', { withFileTypes: true }); + }); +});