fix(fs): open files through workspace symlinks
This commit is contained in:
@@ -40,6 +40,7 @@ Keep `bridge.ts` as a thin orchestration layer that delegates message handling t
|
||||
- active-directory selection across multi-root workspaces
|
||||
- dropped-file parsing and attachment reading
|
||||
- models metadata fetch helper
|
||||
- Read paths are authorized in the requested workspace path space before symlink resolution, matching the web runtime; directly requested outside-workspace paths remain denied.
|
||||
|
||||
The webview CSP permits `blob:` only for `worker-src` so shared UI parsers can run bounded local decompression off the main thread. Blob scripts remain disallowed by `script-src`.
|
||||
|
||||
|
||||
@@ -583,12 +583,8 @@ export const resolveFileReadPath = async (targetPath: string, requestedRoot?: st
|
||||
}
|
||||
|
||||
try {
|
||||
const [canonicalPath, canonicalBase] = await Promise.all([
|
||||
fs.promises.realpath(resolved),
|
||||
fs.promises.realpath(baseRoot).catch(() => path.resolve(baseRoot)),
|
||||
]);
|
||||
|
||||
if (!isPathInside(canonicalPath, canonicalBase)) {
|
||||
const canonicalPath = await fs.promises.realpath(resolved);
|
||||
if (!isPathInside(resolved, path.resolve(baseRoot))) {
|
||||
return { ok: false, status: 403, error: 'Access to file denied' };
|
||||
}
|
||||
|
||||
|
||||
@@ -35,5 +35,6 @@ Own filesystem API behavior for the web server runtime, including workspace-boun
|
||||
## Notes for contributors
|
||||
- Keep filesystem policy (workspace root checks, error mapping, exec timeout behavior) inside this module, not in the composition root.
|
||||
- Filesystem `EPERM`/`EACCES` failures use the stable `reason: "os-permission"` response marker. Policy denials such as workspace-boundary or missing-grant failures must not use that marker because a native folder picker cannot remediate them.
|
||||
- Read-only routes authorize the requested path against the workspace before resolving symlinks. A symlink reached through the workspace may therefore target a file outside it, while a directly requested outside path still requires an exact-path grant. Write routes keep canonical-target boundary checks.
|
||||
- If adding new `/api/fs/*` endpoints, add them in `routes.js` and extend this document.
|
||||
- `GET /api/fs/list` may resolve symlinks with `realpath` to read directory contents, but the response `path` and each entry `path` must stay in the caller's requested path space (`path.join(requestedPath, name)`). Returning real paths breaks file-tree expansion for directories reached through workspace symlinks.
|
||||
|
||||
@@ -758,14 +758,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)) {
|
||||
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()) {
|
||||
@@ -815,14 +808,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)) {
|
||||
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()) {
|
||||
@@ -886,14 +872,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)) {
|
||||
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()) {
|
||||
@@ -971,14 +950,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)) {
|
||||
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()) {
|
||||
|
||||
@@ -341,6 +341,24 @@ describe('fs write', () => {
|
||||
});
|
||||
|
||||
describe('fs read', () => {
|
||||
it('reads workspace files through symlinks that resolve outside the workspace', async () => {
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => {
|
||||
if (targetPath === '/repo/link.txt') return '/shared/target.txt';
|
||||
return targetPath;
|
||||
}),
|
||||
stat: vi.fn(async () => ({ isFile: () => true, size: 6 })),
|
||||
readFile: vi.fn(async () => 'shared'),
|
||||
};
|
||||
const handler = registerRead(fsPromises);
|
||||
|
||||
const res = await callRead(handler, { path: '/repo/link.txt' });
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.body).toBe('shared');
|
||||
expect(fsPromises.readFile).toHaveBeenCalledWith('/shared/target.txt', 'utf8');
|
||||
});
|
||||
|
||||
it('rejects outside workspace reads without a grant', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
const fsPromises = {
|
||||
|
||||
Reference in New Issue
Block a user