fix(server): allow reading files through workspace-internal symlinks (#2872)

fix(server): allow reading files through workspace-internal symlinks
2026-08-29 00:53:22 +03:00
parent 25c2e7b3ff 3e0a9622aa
commit a182f4f4ff
5 changed files with 159 additions and 26 deletions
@@ -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.
@@ -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,
@@ -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']],
@@ -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 });
    });
  });
 });