From 3e0a9622aa985c19a95ed360a551c183ff9c3d38 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 29 Aug 2026 00:53:06 +0300 Subject: [PATCH] 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 }); }); }); });