From 73c2f7bf239fefecefbc055772c61305ab540238 Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Wed, 5 Aug 2026 11:21:43 +0300 Subject: [PATCH 1/3] fix(server): allow reading files through workspace-internal symlinks Read-family fs routes (stat/read/raw/serve) rejected files whose canonical (realpath) target escaped the project root, so a symlinked folder inside the workspace (e.g. ~/test_folder -> /shared/test_folder) listed fine but every file open failed with "Failed to open file". Resolve symlinks before the containment check: paths that are lexically inside the active workspace stay readable even when their realpath target lives outside it, while direct paths outside the workspace (including traversal and canonical-path requests) remain rejected and write/exec keep the strict canonical boundary. The directory listing now returns entry paths under the requested (user-visible) directory so the file tree hands back addressable paths instead of canonical ones. Refs OPE-235 --- packages/web/server/lib/fs/DOCUMENTATION.md | 1 + packages/web/server/lib/fs/routes.js | 99 +++++++++++-- packages/web/server/lib/fs/routes.test.js | 149 ++++++++++++++++++++ 3 files changed, 238 insertions(+), 11 deletions(-) diff --git a/packages/web/server/lib/fs/DOCUMENTATION.md b/packages/web/server/lib/fs/DOCUMENTATION.md index 07b166fd..e2941b97 100644 --- a/packages/web/server/lib/fs/DOCUMENTATION.md +++ b/packages/web/server/lib/fs/DOCUMENTATION.md @@ -25,6 +25,7 @@ Own filesystem API behavior for the web server runtime, including workspace-boun - `GET /api/fs/list` - Owns exec job queue state (`execJobs`) and lifecycle/TTL pruning. - Enforces workspace boundary checks with active project + worktree fallback support. + - Read-family endpoints (`stat`, `read`, `raw`, `serve`) resolve symlinks before serving and allow paths that are lexically inside the active workspace even when their canonical (realpath) target lives outside it through a workspace-internal symlink (for example `~/test_folder -> /shared/test_folder`). Paths that are not inside the workspace lexically are still rejected before any symlink resolution, and write/exec operations keep the strict canonical containment check. - `createFsSearchRuntime({ fsPromises, path, spawn, resolveGitBinaryForSpawn })` from `search.js` - Returns `{ searchFilesystemFiles(rootPath, options) }`. - Supports fuzzy matching, hidden-file handling, and optional `git check-ignore` filtering. diff --git a/packages/web/server/lib/fs/routes.js b/packages/web/server/lib/fs/routes.js index 96f07fda..34124d85 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -206,11 +206,11 @@ const resolveWorkspacePath = ({ targetPath, baseDirectory, path, os, normalizeDi const resolvedBase = path.resolve(baseDirectory || os.homedir()); if (isPathWithinRoot(resolved, resolvedBase, path, os)) { - return { ok: true, base: resolvedBase, resolved }; + return { ok: true, base: resolvedBase, resolved, insideWorkspace: true }; } if (isPathWithinRoot(resolved, openchamberUserConfigRoot, path, os)) { - return { ok: true, base: path.resolve(openchamberUserConfigRoot), resolved }; + return { ok: true, base: path.resolve(openchamberUserConfigRoot), resolved, insideWorkspace: true }; } return { ok: false, error: 'Path is outside of active workspace' }; @@ -239,7 +239,7 @@ const resolveWorkspacePathFromWorktrees = async ({ targetPath, baseDirectory, pa } const candidateResolved = path.resolve(candidate); if (isPathWithinRoot(resolved, candidateResolved, path, os)) { - return { ok: true, base: candidateResolved, resolved }; + return { ok: true, base: candidateResolved, resolved, insideWorkspace: true }; } } } catch (error) { @@ -333,7 +333,7 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje }); } - return resolveWorkspacePathFromContext({ + const resolved = await resolveWorkspacePathFromContext({ req, targetPath, resolveProjectDirectory, @@ -342,6 +342,49 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje normalizeDirectoryPath, openchamberUserConfigRoot, }); + if (resolved.ok || resolved.error !== 'Path is outside of active workspace') { + return resolved; + } + + // The active project directory is validated with fs.realpath, so the base + // is canonical while the client (and the file tree) addresses files under + // the user-visible, possibly symlinked root (a workspace-internal symlinked + // folder, or a project root that is itself a symlink). Accept paths that + // are lexically inside the raw directory the client sent; symlink + // resolution happens afterwards, so direct paths outside the workspace + // remain rejected and the canonical containment check still applies to + // every path that is not inside the workspace. + const rawHeaderDirectory = typeof req.get === 'function' ? req.get('x-opencode-directory') : null; + const rawHeaderEncoding = typeof req.get === 'function' ? req.get('x-opencode-directory-encoding') : null; + const decodedHeaderDirectory = rawHeaderDirectory && rawHeaderEncoding === 'uri' + ? (() => { + try { + return decodeURIComponent(rawHeaderDirectory); + } catch { + return rawHeaderDirectory; + } + })() + : rawHeaderDirectory; + const queryDirectory = Array.isArray(req.query?.directory) + ? req.query.directory[0] + : req.query?.directory; + const lexicalBase = [decodedHeaderDirectory, queryDirectory] + .find((value) => typeof value === 'string' && value.trim().length > 0); + if (lexicalBase) { + const lexical = resolveWorkspacePath({ + targetPath, + baseDirectory: lexicalBase, + path, + os, + normalizeDirectoryPath, + openchamberUserConfigRoot, + }); + if (lexical.ok) { + return lexical; + } + } + + return resolved; }; const runCommandInDirectory = ({ shell, shellFlag, command, resolvedCwd, spawn, buildAugmentedPath, commandTimeoutMs }) => { @@ -785,7 +828,14 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const canonicalPath = await fsPromises.realpath(resolved.resolved); + const [canonicalPath, canonicalBase] = await Promise.all([ + fsPromises.realpath(resolved.resolved), + fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), + ]); + + if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { + return res.status(403).json({ error: 'Access to file denied' }); + } const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -835,7 +885,14 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const canonicalPath = await fsPromises.realpath(resolved.resolved); + const [canonicalPath, canonicalBase] = await Promise.all([ + fsPromises.realpath(resolved.resolved), + fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), + ]); + + if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { + return res.status(403).json({ error: 'Access to file denied' }); + } const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -899,7 +956,14 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const canonicalPath = await fsPromises.realpath(resolved.resolved); + const [canonicalPath, canonicalBase] = await Promise.all([ + fsPromises.realpath(resolved.resolved), + fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), + ]); + + if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { + return res.status(403).json({ error: 'Access to file denied' }); + } const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -977,7 +1041,14 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const canonicalPath = await fsPromises.realpath(resolved.resolved); + const [canonicalPath, canonicalBase] = await Promise.all([ + fsPromises.realpath(resolved.resolved), + fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), + ]); + + if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { + return res.status(403).json({ error: 'Access to file denied' }); + } const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -1464,7 +1535,13 @@ export const registerFsRoutes = (app, dependencies) => { }; try { - requestedPath = path.resolve(normalizeDirectoryPath(rawPath)); + // 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)); resolvedPath = await realpathCache.resolve(requestedPath); const stats = await fsPromises.stat(resolvedPath); @@ -1524,8 +1601,8 @@ export const registerFsRoutes = (app, dependencies) => { const entries = await Promise.all( dirents.map(async (dirent) => { - const physicalEntryPath = path.join(resolvedPath, dirent.name); - if (respectGitignore && ignoredPaths.has(physicalEntryPath)) { + const entryPath = path.join(requestedPath, dirent.name); + if (respectGitignore && ignoredPaths.has(entryPath)) { return null; } diff --git a/packages/web/server/lib/fs/routes.test.js b/packages/web/server/lib/fs/routes.test.js index 64467af7..9e801d9a 100644 --- a/packages/web/server/lib/fs/routes.test.js +++ b/packages/web/server/lib/fs/routes.test.js @@ -204,6 +204,26 @@ 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, { @@ -681,6 +701,135 @@ describe('fs read', () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining('Read retry exhausted for /repo/file.txt')); warn.mockRestore(); }); + + it('reads files inside the workspace whose canonical path escapes through a symlinked directory', async () => { + // ~/test_folder -> /outside/shared: the requested path is lexically inside + // the workspace, the realpath is not. The read must follow the symlink + // instead of rejecting it as an outside path. + const fsPromises = { + realpath: vi.fn(async (targetPath) => { + if (targetPath === '/repo/link/file.txt') return '/outside/shared/file.txt'; + return targetPath; + }), + stat: vi.fn(async () => ({ isFile: () => true, size: 5 })), + readFile: vi.fn(async () => 'hello'), + }; + const handler = registerRead(fsPromises); + + const res = await callRead(handler, { path: '/repo/link/file.txt' }); + + expect(res.statusCode).toBe(200); + expect(res.body).toBe('hello'); + expect(fsPromises.readFile).toHaveBeenCalledWith('/outside/shared/file.txt', 'utf8'); + }); + + it('rejects reads of canonical paths outside the workspace that no workspace symlink reaches', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const fsPromises = { + stat: vi.fn(async () => ({ isFile: () => true, size: 6 })), + readFile: vi.fn(async () => 'secret'), + }; + const handler = registerRead(fsPromises); + + const res = await callRead(handler, { path: '/outside/shared/file.txt' }); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ error: 'Path is outside of active workspace' }); + expect(fsPromises.readFile).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('reads files under a symlinked project root addressed via the client-sent lexical directory', async () => { + // /home/user/proj -> /real/proj: the validated base is canonical but the + // client (and the file tree) address files under the lexical root. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const fsPromises = { + realpath: vi.fn(async (targetPath) => { + if (targetPath === '/home/user/proj') return '/real/proj'; + if (targetPath === '/home/user/proj/file.txt') return '/real/proj/file.txt'; + return targetPath; + }), + stat: vi.fn(async () => ({ isFile: () => true, size: 4 })), + readFile: vi.fn(async () => 'data'), + }; + const handler = registerRead(fsPromises); + const res = createMockResponse(); + + await handler({ + query: { path: '/home/user/proj/file.txt' }, + get: (name) => (name === 'x-opencode-directory' ? '/home/user/proj' : undefined), + }, res); + + expect(res.statusCode).toBe(200); + expect(res.body).toBe('data'); + expect(fsPromises.readFile).toHaveBeenCalledWith('/real/proj/file.txt', 'utf8'); + warn.mockRestore(); + }); + + it('rejects path traversal that escapes the workspace even when it passes through a symlinked directory', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const fsPromises = { + realpath: vi.fn(async (targetPath) => { + if (targetPath === '/repo/link') return '/outside/shared'; + return targetPath; + }), + stat: vi.fn(async () => ({ isFile: () => true, size: 6 })), + readFile: vi.fn(async () => 'secret'), + }; + const handler = registerRead(fsPromises); + + const res = await callRead(handler, { path: '/repo/sub/../../etc/passwd' }); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ error: 'Path is outside of active workspace' }); + expect(fsPromises.readFile).not.toHaveBeenCalled(); + 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', () => { From 256722eeea8a41ffc206586d430f3bb10f0f0a27 Mon Sep 17 00:00:00 2001 From: herjarsa Date: Fri, 28 Aug 2026 09:41:31 +0200 Subject: [PATCH 2/3] 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']], From 3e0a9622aa985c19a95ed360a551c183ff9c3d38 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 29 Aug 2026 00:53:06 +0300 Subject: [PATCH 3/3] refactor(server): own the requested project directory in the runtime Drop the canonical-containment 403 guard and the extra realpath(base) the read routes (stat/read/raw/serve) had gained. Every workspace resolution returns insideWorkspace: true and outside-file grants use base = dirname(canonicalPath), so the guard could never fire; the flag had no remaining reader and is gone with it. The read routes are back to the single realpath(resolved.resolved) they had before. Move the lexical-base fallback out of the inline header parsing in routes.js. x-opencode-directory decoding belongs to project-directory-runtime, so resolveProjectDirectory now also returns requestedDirectory, the pre-realpath candidate that validated. resolveWorkspacePathFromContext retries against it when the canonical base rejects a path, which keeps files under a symlinked project root addressable without a second copy of the header/query parsing. --- packages/web/server/lib/fs/DOCUMENTATION.md | 2 +- packages/web/server/lib/fs/routes.js | 112 ++++++------------ packages/web/server/lib/fs/routes.test.js | 9 +- .../lib/opencode/project-directory-runtime.js | 27 +++-- .../project-directory-runtime.test.js | 44 +++++-- 5 files changed, 88 insertions(+), 106 deletions(-) diff --git a/packages/web/server/lib/fs/DOCUMENTATION.md b/packages/web/server/lib/fs/DOCUMENTATION.md index e2941b97..ae4cdb2b 100644 --- a/packages/web/server/lib/fs/DOCUMENTATION.md +++ b/packages/web/server/lib/fs/DOCUMENTATION.md @@ -25,7 +25,7 @@ Own filesystem API behavior for the web server runtime, including workspace-boun - `GET /api/fs/list` - Owns exec job queue state (`execJobs`) and lifecycle/TTL pruning. - Enforces workspace boundary checks with active project + worktree fallback support. - - Read-family endpoints (`stat`, `read`, `raw`, `serve`) resolve symlinks before serving and allow paths that are lexically inside the active workspace even when their canonical (realpath) target lives outside it through a workspace-internal symlink (for example `~/test_folder -> /shared/test_folder`). Paths that are not inside the workspace lexically are still rejected before any symlink resolution, and write/exec operations keep the strict canonical containment check. + - The active project directory is validated with `fs.realpath`, so when the project root is itself a symlink the workspace base no longer matches the paths the client sends. Workspace resolution therefore retries against the raw directory the client requested (`requestedDirectory` from `resolveProjectDirectory`) before falling back to worktree roots. Symlinks are still resolved afterwards, and write/exec routes keep their canonical containment check against the resolved base. - `createFsSearchRuntime({ fsPromises, path, spawn, resolveGitBinaryForSpawn })` from `search.js` - Returns `{ searchFilesystemFiles(rootPath, options) }`. - Supports fuzzy matching, hidden-file handling, and optional `git check-ignore` filtering. diff --git a/packages/web/server/lib/fs/routes.js b/packages/web/server/lib/fs/routes.js index e6d8c9f0..140f9436 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -206,11 +206,11 @@ const resolveWorkspacePath = ({ targetPath, baseDirectory, path, os, normalizeDi const resolvedBase = path.resolve(baseDirectory || os.homedir()); if (isPathWithinRoot(resolved, resolvedBase, path, os)) { - return { ok: true, base: resolvedBase, resolved, insideWorkspace: true }; + return { ok: true, base: resolvedBase, resolved }; } if (isPathWithinRoot(resolved, openchamberUserConfigRoot, path, os)) { - return { ok: true, base: path.resolve(openchamberUserConfigRoot), resolved, insideWorkspace: true }; + return { ok: true, base: path.resolve(openchamberUserConfigRoot), resolved }; } return { ok: false, error: 'Path is outside of active workspace' }; @@ -239,7 +239,7 @@ const resolveWorkspacePathFromWorktrees = async ({ targetPath, baseDirectory, pa } const candidateResolved = path.resolve(candidate); if (isPathWithinRoot(resolved, candidateResolved, path, os)) { - return { ok: true, base: candidateResolved, resolved, insideWorkspace: true }; + return { ok: true, base: candidateResolved, resolved }; } } } catch (error) { @@ -267,6 +267,27 @@ const resolveWorkspacePathFromContext = async ({ req, targetPath, resolveProject return resolved; } + // The active project directory is validated with fs.realpath, so the base is + // canonical while the client (and the file tree) addresses files under the + // user-visible root, which may itself be a symlink. Retry against the raw + // directory the client asked for so those paths stay addressable. Symlink + // resolution still happens afterwards, and the routes that need canonical + // containment re-check it against this base. + const requestedBase = resolvedProject.requestedDirectory; + if (typeof requestedBase === 'string' && requestedBase && requestedBase !== resolvedProject.directory) { + const lexical = resolveWorkspacePath({ + targetPath, + baseDirectory: requestedBase, + path, + os, + normalizeDirectoryPath, + openchamberUserConfigRoot, + }); + if (lexical.ok) { + return lexical; + } + } + return resolveWorkspacePathFromWorktrees({ targetPath, baseDirectory: resolvedProject.directory, @@ -333,7 +354,7 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje }); } - const resolved = await resolveWorkspacePathFromContext({ + return resolveWorkspacePathFromContext({ req, targetPath, resolveProjectDirectory, @@ -342,49 +363,6 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje normalizeDirectoryPath, openchamberUserConfigRoot, }); - if (resolved.ok || resolved.error !== 'Path is outside of active workspace') { - return resolved; - } - - // The active project directory is validated with fs.realpath, so the base - // is canonical while the client (and the file tree) addresses files under - // the user-visible, possibly symlinked root (a workspace-internal symlinked - // folder, or a project root that is itself a symlink). Accept paths that - // are lexically inside the raw directory the client sent; symlink - // resolution happens afterwards, so direct paths outside the workspace - // remain rejected and the canonical containment check still applies to - // every path that is not inside the workspace. - const rawHeaderDirectory = typeof req.get === 'function' ? req.get('x-opencode-directory') : null; - const rawHeaderEncoding = typeof req.get === 'function' ? req.get('x-opencode-directory-encoding') : null; - const decodedHeaderDirectory = rawHeaderDirectory && rawHeaderEncoding === 'uri' - ? (() => { - try { - return decodeURIComponent(rawHeaderDirectory); - } catch { - return rawHeaderDirectory; - } - })() - : rawHeaderDirectory; - const queryDirectory = Array.isArray(req.query?.directory) - ? req.query.directory[0] - : req.query?.directory; - const lexicalBase = [decodedHeaderDirectory, queryDirectory] - .find((value) => typeof value === 'string' && value.trim().length > 0); - if (lexicalBase) { - const lexical = resolveWorkspacePath({ - targetPath, - baseDirectory: lexicalBase, - path, - os, - normalizeDirectoryPath, - openchamberUserConfigRoot, - }); - if (lexical.ok) { - return lexical; - } - } - - return resolved; }; const runCommandInDirectory = ({ shell, shellFlag, command, resolvedCwd, spawn, buildAugmentedPath, commandTimeoutMs }) => { @@ -828,14 +806,7 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const [canonicalPath, canonicalBase] = await Promise.all([ - fsPromises.realpath(resolved.resolved), - fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), - ]); - - if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { - return res.status(403).json({ error: 'Access to file denied' }); - } + const canonicalPath = await fsPromises.realpath(resolved.resolved); const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -885,14 +856,7 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const [canonicalPath, canonicalBase] = await Promise.all([ - fsPromises.realpath(resolved.resolved), - fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), - ]); - - if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { - return res.status(403).json({ error: 'Access to file denied' }); - } + const canonicalPath = await fsPromises.realpath(resolved.resolved); const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -956,14 +920,7 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const [canonicalPath, canonicalBase] = await Promise.all([ - fsPromises.realpath(resolved.resolved), - fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), - ]); - - if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { - return res.status(403).json({ error: 'Access to file denied' }); - } + const canonicalPath = await fsPromises.realpath(resolved.resolved); const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -1041,14 +998,7 @@ export const registerFsRoutes = (app, dependencies) => { return res.status(400).json({ error: resolved.error }); } - const [canonicalPath, canonicalBase] = await Promise.all([ - fsPromises.realpath(resolved.resolved), - fsPromises.realpath(resolved.base).catch(() => path.resolve(resolved.base)), - ]); - - if (!isPathWithinRoot(canonicalPath, canonicalBase, path, os) && !resolved.insideWorkspace) { - return res.status(403).json({ error: 'Access to file denied' }); - } + const canonicalPath = await fsPromises.realpath(resolved.resolved); const stats = await fsPromises.stat(canonicalPath); if (!stats.isFile()) { @@ -1521,6 +1471,10 @@ 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 = ''; diff --git a/packages/web/server/lib/fs/routes.test.js b/packages/web/server/lib/fs/routes.test.js index 67941895..35640af8 100644 --- a/packages/web/server/lib/fs/routes.test.js +++ b/packages/web/server/lib/fs/routes.test.js @@ -165,7 +165,7 @@ const registerUpload = (fsPromises) => { return getRoute('POST', '/api/fs/upload'); }; -const registerRead = (fsPromises) => { +const registerRead = (fsPromises, resolveProjectDirectory = async () => ({ directory: '/repo' })) => { const { app, getRoute } = createRouteRegistry(); registerFsRoutes(app, { os: { homedir: () => '/home/user' }, @@ -177,7 +177,7 @@ const registerRead = (fsPromises) => { spawn: vi.fn(), crypto: { randomUUID: () => 'job-0' }, normalizeDirectoryPath: (p) => p, - resolveProjectDirectory: async () => ({ directory: '/repo' }), + resolveProjectDirectory, buildAugmentedPath: () => '/usr/bin', resolveGitBinaryForSpawn: () => 'git', openchamberUserConfigRoot: '/home/user/.config', @@ -733,7 +733,10 @@ describe('fs read', () => { stat: vi.fn(async () => ({ isFile: () => true, size: 4 })), readFile: vi.fn(async () => 'data'), }; - const handler = registerRead(fsPromises); + const handler = registerRead(fsPromises, async () => ({ + directory: '/real/proj', + requestedDirectory: '/home/user/proj', + })); const res = createMockResponse(); await handler({ diff --git a/packages/web/server/lib/opencode/project-directory-runtime.js b/packages/web/server/lib/opencode/project-directory-runtime.js index 2e289752..eb3e0ef9 100644 --- a/packages/web/server/lib/opencode/project-directory-runtime.js +++ b/packages/web/server/lib/opencode/project-directory-runtime.js @@ -44,7 +44,12 @@ export const createProjectDirectoryRuntime = (dependencies) => { return { ok: false, error: 'Specified path is not a directory' }; } const realPath = await realpathCache.resolve(resolved); - return { ok: true, directory: realPath }; + // `requestedDirectory` is the pre-realpath candidate the caller asked + // for. Callers that address files in the user-visible path space (the + // file tree, the read-family FS routes) need it when the project root + // is itself a symlink and the canonical `directory` no longer contains + // the paths the client sends. + return { ok: true, directory: realPath, requestedDirectory: resolved }; } catch (error) { const err = error; if (err && typeof err === 'object' && err.code === 'ENOENT') { @@ -71,11 +76,11 @@ export const createProjectDirectoryRuntime = (dependencies) => { for (const candidate of requested) { const validated = await validateDirectoryPath(candidate); if (validated.ok) { - return { directory: validated.directory, error: null }; + return { directory: validated.directory, requestedDirectory: validated.requestedDirectory, error: null }; } lastError = validated.error; } - return { directory: null, error: lastError }; + return { directory: null, requestedDirectory: null, error: lastError }; } const readSettings = typeof getReadSettingsFromDiskMigrated === 'function' @@ -93,27 +98,27 @@ export const createProjectDirectoryRuntime = (dependencies) => { if (typeof settings.lastDirectory === 'string' && settings.lastDirectory.trim()) { const validated = await validateDirectoryPath(settings.lastDirectory); if (validated.ok) { - return { directory: validated.directory, error: null }; + return { directory: validated.directory, requestedDirectory: validated.requestedDirectory, error: null }; } } const projects = sanitizeProjects(settings.projects) || []; if (projects.length === 0) { - return { directory: null, error: 'Directory parameter or active project is required' }; + return { directory: null, requestedDirectory: null, error: 'Directory parameter or active project is required' }; } const activeId = typeof settings.activeProjectId === 'string' ? settings.activeProjectId : ''; const active = projects.find((project) => project.id === activeId) || projects[0]; if (!active || !active.path) { - return { directory: null, error: 'Directory parameter or active project is required' }; + return { directory: null, requestedDirectory: null, error: 'Directory parameter or active project is required' }; } const validated = await validateDirectoryPath(active.path); if (!validated.ok) { - return { directory: null, error: validated.error }; + return { directory: null, requestedDirectory: null, error: validated.error }; } - return { directory: validated.directory, error: null }; + return { directory: validated.directory, requestedDirectory: validated.requestedDirectory, error: null }; }; const resolveOptionalProjectDirectory = async (req) => { @@ -126,18 +131,18 @@ export const createProjectDirectoryRuntime = (dependencies) => { const requested = [headerDirectory, queryDirectory].filter(Boolean); if (requested.length === 0) { - return { directory: null, error: null }; + return { directory: null, requestedDirectory: null, error: null }; } let lastError = null; for (const candidate of requested) { const validated = await validateDirectoryPath(candidate); if (validated.ok) { - return { directory: validated.directory, error: null }; + return { directory: validated.directory, requestedDirectory: validated.requestedDirectory, error: null }; } lastError = validated.error; } - return { directory: null, error: lastError }; + return { directory: null, requestedDirectory: null, error: lastError }; }; return { diff --git a/packages/web/server/lib/opencode/project-directory-runtime.test.js b/packages/web/server/lib/opencode/project-directory-runtime.test.js index 2a12a79a..9ee8d879 100644 --- a/packages/web/server/lib/opencode/project-directory-runtime.test.js +++ b/packages/web/server/lib/opencode/project-directory-runtime.test.js @@ -26,7 +26,7 @@ describe('project directory runtime', () => { const runtime = createTestRuntime(); const result = await runtime.validateDirectoryPath('/home/user/project'); - expect(result).toEqual({ ok: true, directory: '/home/user/project' }); + expect(result).toEqual({ ok: true, directory: '/home/user/project', requestedDirectory: '/home/user/project' }); }); it('resolves symlinks via fsPromises.realpath', async () => { @@ -39,7 +39,7 @@ describe('project directory runtime', () => { const result = await runtime.validateDirectoryPath('/symlink/path/to/project'); - expect(result).toEqual({ ok: true, directory: '/real/path/to/project' }); + expect(result).toEqual({ ok: true, directory: '/real/path/to/project', requestedDirectory: '/symlink/path/to/project' }); }); it('returns error when candidate is empty', async () => { @@ -125,7 +125,11 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); - expect(result).toEqual({ directory: '/real/workspace/project', error: null }); + expect(result).toEqual({ + directory: '/real/workspace/project', + requestedDirectory: '/home/user/workspace/project', + error: null, + }); }); it('decodes marked x-opencode-directory header values', async () => { @@ -153,7 +157,7 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); expect(validatedPath).toBe(pathWithUnicode); - expect(result).toEqual({ directory: pathWithUnicode, error: null }); + expect(result).toEqual({ directory: pathWithUnicode, requestedDirectory: pathWithUnicode, error: null }); }); it('preserves raw percent sequences without directory encoding marker', async () => { @@ -177,7 +181,7 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); expect(validatedPath).toBe(rawPath); - expect(result).toEqual({ directory: rawPath, error: null }); + expect(result).toEqual({ directory: rawPath, requestedDirectory: rawPath, error: null }); }); it('falls back to query directory when an unmarked encoded header is invalid', async () => { @@ -199,7 +203,7 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); - expect(result).toEqual({ directory: validPath, error: null }); + expect(result).toEqual({ directory: validPath, requestedDirectory: validPath, error: null }); }); it('resolves symlinks in query directory parameter', async () => { @@ -217,7 +221,11 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); - expect(result).toEqual({ directory: '/real/workspace/project', error: null }); + expect(result).toEqual({ + directory: '/real/workspace/project', + requestedDirectory: '/home/user/workspace/project', + error: null, + }); }); it('resolves symlinks in lastDirectory from settings', async () => { @@ -238,7 +246,11 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); - expect(result).toEqual({ directory: '/real/workspace/project', error: null }); + expect(result).toEqual({ + directory: '/real/workspace/project', + requestedDirectory: '/home/user/workspace/project', + error: null, + }); }); it('resolves symlinks in active project path from settings', async () => { @@ -261,7 +273,11 @@ describe('project directory runtime', () => { const result = await runtime.resolveProjectDirectory(req); - expect(result).toEqual({ directory: '/real/workspace/project', error: null }); + expect(result).toEqual({ + directory: '/real/workspace/project', + requestedDirectory: '/home/user/workspace/project', + error: null, + }); }); }); @@ -276,7 +292,7 @@ describe('project directory runtime', () => { const result = await runtime.resolveOptionalProjectDirectory(req); - expect(result).toEqual({ directory: null, error: null }); + expect(result).toEqual({ directory: null, requestedDirectory: null, error: null }); }); it('resolves symlinks when directory is provided', async () => { @@ -294,7 +310,11 @@ describe('project directory runtime', () => { const result = await runtime.resolveOptionalProjectDirectory(req); - expect(result).toEqual({ directory: '/real/workspace/project', error: null }); + expect(result).toEqual({ + directory: '/real/workspace/project', + requestedDirectory: '/symlink/workspace/project', + error: null, + }); }); it('preserves raw percent sequences without directory encoding marker', async () => { @@ -318,7 +338,7 @@ describe('project directory runtime', () => { const result = await runtime.resolveOptionalProjectDirectory(req); expect(validatedPath).toBe(rawPath); - expect(result).toEqual({ directory: rawPath, error: null }); + expect(result).toEqual({ directory: rawPath, requestedDirectory: rawPath, error: null }); }); }); });