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
This commit is contained in:
@@ -25,6 +25,7 @@ Own filesystem API behavior for the web server runtime, including workspace-boun
|
|||||||
- `GET /api/fs/list`
|
- `GET /api/fs/list`
|
||||||
- Owns exec job queue state (`execJobs`) and lifecycle/TTL pruning.
|
- Owns exec job queue state (`execJobs`) and lifecycle/TTL pruning.
|
||||||
- Enforces workspace boundary checks with active project + worktree fallback support.
|
- 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`
|
- `createFsSearchRuntime({ fsPromises, path, spawn, resolveGitBinaryForSpawn })` from `search.js`
|
||||||
- Returns `{ searchFilesystemFiles(rootPath, options) }`.
|
- Returns `{ searchFilesystemFiles(rootPath, options) }`.
|
||||||
- Supports fuzzy matching, hidden-file handling, and optional `git check-ignore` filtering.
|
- Supports fuzzy matching, hidden-file handling, and optional `git check-ignore` filtering.
|
||||||
|
|||||||
@@ -206,11 +206,11 @@ const resolveWorkspacePath = ({ targetPath, baseDirectory, path, os, normalizeDi
|
|||||||
const resolvedBase = path.resolve(baseDirectory || os.homedir());
|
const resolvedBase = path.resolve(baseDirectory || os.homedir());
|
||||||
|
|
||||||
if (isPathWithinRoot(resolved, resolvedBase, path, os)) {
|
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)) {
|
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' };
|
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);
|
const candidateResolved = path.resolve(candidate);
|
||||||
if (isPathWithinRoot(resolved, candidateResolved, path, os)) {
|
if (isPathWithinRoot(resolved, candidateResolved, path, os)) {
|
||||||
return { ok: true, base: candidateResolved, resolved };
|
return { ok: true, base: candidateResolved, resolved, insideWorkspace: true };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -333,7 +333,7 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
return resolveWorkspacePathFromContext({
|
const resolved = await resolveWorkspacePathFromContext({
|
||||||
req,
|
req,
|
||||||
targetPath,
|
targetPath,
|
||||||
resolveProjectDirectory,
|
resolveProjectDirectory,
|
||||||
@@ -342,6 +342,49 @@ const resolveReadPathFromContext = async ({ req, targetPath, scope, resolveProje
|
|||||||
normalizeDirectoryPath,
|
normalizeDirectoryPath,
|
||||||
openchamberUserConfigRoot,
|
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 }) => {
|
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 });
|
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);
|
const stats = await fsPromises.stat(canonicalPath);
|
||||||
if (!stats.isFile()) {
|
if (!stats.isFile()) {
|
||||||
@@ -835,7 +885,14 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
return res.status(400).json({ error: resolved.error });
|
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);
|
const stats = await fsPromises.stat(canonicalPath);
|
||||||
if (!stats.isFile()) {
|
if (!stats.isFile()) {
|
||||||
@@ -899,7 +956,14 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
return res.status(400).json({ error: resolved.error });
|
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);
|
const stats = await fsPromises.stat(canonicalPath);
|
||||||
if (!stats.isFile()) {
|
if (!stats.isFile()) {
|
||||||
@@ -977,7 +1041,14 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
return res.status(400).json({ error: resolved.error });
|
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);
|
const stats = await fsPromises.stat(canonicalPath);
|
||||||
if (!stats.isFile()) {
|
if (!stats.isFile()) {
|
||||||
@@ -1464,7 +1535,13 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
try {
|
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);
|
resolvedPath = await realpathCache.resolve(requestedPath);
|
||||||
|
|
||||||
const stats = await fsPromises.stat(resolvedPath);
|
const stats = await fsPromises.stat(resolvedPath);
|
||||||
@@ -1524,8 +1601,8 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
|
|
||||||
const entries = await Promise.all(
|
const entries = await Promise.all(
|
||||||
dirents.map(async (dirent) => {
|
dirents.map(async (dirent) => {
|
||||||
const physicalEntryPath = path.join(resolvedPath, dirent.name);
|
const entryPath = path.join(requestedPath, dirent.name);
|
||||||
if (respectGitignore && ignoredPaths.has(physicalEntryPath)) {
|
if (respectGitignore && ignoredPaths.has(entryPath)) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -204,6 +204,26 @@ const registerRaw = (fsPromises) => {
|
|||||||
return getRoute('GET', '/api/fs/raw');
|
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 registerMkdir = (fsPromises) => {
|
||||||
const { app, getRoute } = createRouteRegistry();
|
const { app, getRoute } = createRouteRegistry();
|
||||||
registerFsRoutes(app, {
|
registerFsRoutes(app, {
|
||||||
@@ -681,6 +701,135 @@ describe('fs read', () => {
|
|||||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('Read retry exhausted for /repo/file.txt'));
|
expect(warn).toHaveBeenCalledWith(expect.stringContaining('Read retry exhausted for /repo/file.txt'));
|
||||||
warn.mockRestore();
|
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', () => {
|
describe('fs reveal', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user