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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 = '';
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user