diff --git a/packages/web/server/lib/fs/DOCUMENTATION.md b/packages/web/server/lib/fs/DOCUMENTATION.md index 07b166fd..ae4cdb2b 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. + - 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 96f07fda..140f9436 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -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, diff --git a/packages/web/server/lib/fs/routes.test.js b/packages/web/server/lib/fs/routes.test.js index 6055ce8b..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', @@ -682,8 +682,94 @@ 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, async () => ({ + directory: '/real/proj', + requestedDirectory: '/home/user/proj', + })); + 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 reveal', () => { it.each([ ['linux', 'xdg-open', ['/repo']], 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 }); }); }); });