diff --git a/common/src/testing/mocks/filesystem.ts b/common/src/testing/mocks/filesystem.ts index 6c9703622e..bf2af29bde 100644 --- a/common/src/testing/mocks/filesystem.ts +++ b/common/src/testing/mocks/filesystem.ts @@ -2,7 +2,7 @@ import { mock } from 'bun:test' import type { CodebuffFileSystem } from '../../types/filesystem' import type { Mock } from 'bun:test' -import type { PathLike , Stats } from 'node:fs' +import type { PathLike, Stats } from 'node:fs' export interface CreateMockFsOptions { files?: Record @@ -14,6 +14,7 @@ export interface CreateMockFsOptions { path: string, options?: { recursive?: boolean }, ) => Promise + realpathImpl?: (path: string) => Promise statImpl?: (path: string) => Promise } @@ -31,6 +32,7 @@ export interface MockFsWithMocks { options?: { recursive?: boolean }, ) => Promise > + realpath: Mock<(path: PathLike) => Promise> stat: Mock<(path: PathLike) => Promise> } @@ -43,6 +45,7 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs { readdirImpl, writeFileImpl, mkdirImpl, + realpathImpl, statImpl, } = options @@ -79,6 +82,20 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs { return undefined } + const defaultRealpath = async (path: PathLike): Promise => { + const pathStr = String(path) + const isKnownPath = + pathStr in writtenFiles || + pathStr in directories || + createdDirs.has(pathStr) + + if (!isKnownPath) { + throw new Error(`Path not found: ${pathStr}`) + } + + return pathStr + } + const defaultStat = async (path: PathLike): Promise => { const pathStr = String(path) const isFile = pathStr in writtenFiles @@ -134,6 +151,10 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs { mkdirImpl(String(path), opts) : defaultMkdir + const realpathFn = realpathImpl + ? async (path: PathLike) => realpathImpl(String(path)) + : defaultRealpath + const statFn = statImpl ? async (path: PathLike) => statImpl(String(path)) : defaultStat @@ -143,6 +164,7 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs { readdir: mock(readdirFn), writeFile: mock(writeFileFn), mkdir: mock(mkdirFn), + realpath: mock(realpathFn), stat: mock(statFn), } as unknown as MockFs } @@ -153,6 +175,7 @@ export function restoreMockFs(mockFs: MockFs): void { mocks.readdir.mockRestore() mocks.writeFile.mockRestore() mocks.mkdir.mockRestore() + mocks.realpath.mockRestore() mocks.stat.mockRestore() } @@ -162,5 +185,6 @@ export function clearMockFs(mockFs: MockFs): void { mocks.readdir.mockClear() mocks.writeFile.mockClear() mocks.mkdir.mockClear() + mocks.realpath.mockClear() mocks.stat.mockClear() } diff --git a/common/src/types/filesystem.ts b/common/src/types/filesystem.ts index 6fa64e1168..4506b5c87e 100644 --- a/common/src/types/filesystem.ts +++ b/common/src/types/filesystem.ts @@ -6,5 +6,11 @@ import type fs from 'fs' */ export type CodebuffFileSystem = Pick< typeof fs.promises, - 'mkdir' | 'readdir' | 'readFile' | 'stat' | 'unlink' | 'writeFile' + | 'mkdir' + | 'readdir' + | 'readFile' + | 'realpath' + | 'stat' + | 'unlink' + | 'writeFile' > diff --git a/sdk/src/__tests__/list-directory.test.ts b/sdk/src/__tests__/list-directory.test.ts new file mode 100644 index 0000000000..5637a75b4c --- /dev/null +++ b/sdk/src/__tests__/list-directory.test.ts @@ -0,0 +1,224 @@ +import { describe, expect, it, mock } from 'bun:test' + +import path from 'path' + +import { listDirectory } from '../tools/list-directory' + +import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' +import type { Dirent, PathLike } from 'node:fs' + +const PROJECT_ROOT = path.resolve('workspace', 'project') + +function createFs(realpaths: Record) { + const readdir = mock(async (_path: PathLike) => { + return [ + { + name: 'index.ts', + isDirectory: () => false, + isFile: () => true, + }, + ] as Dirent[] + }) + + const fs = { + realpath: mock(async (path: PathLike) => { + const pathString = String(path) + return realpaths[pathString] ?? pathString + }), + readdir, + } as unknown as CodebuffFileSystem + + return { fs, readdir } +} + +describe('listDirectory', () => { + it('allows listing the project root itself', async () => { + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + }) + + const result = await listDirectory({ + directoryPath: '.', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result[0]).toEqual({ + type: 'json', + value: { + files: ['index.ts'], + directories: [], + path: '.', + }, + }) + expect(readdir).toHaveBeenCalledWith(PROJECT_ROOT, { + withFileTypes: true, + }) + }) + + it('lists a directory inside the project and preserves the requested path', async () => { + const childPath = path.join(PROJECT_ROOT, 'src') + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + [childPath]: childPath, + }) + + const result = await listDirectory({ + directoryPath: 'src', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + files: ['index.ts'], + directories: [], + path: 'src', + }, + }, + ]) + expect(readdir).toHaveBeenCalledWith(childPath, { + withFileTypes: true, + }) + }) + + it('returns the normal list error when the requested directory is missing', async () => { + const missingPath = path.join(PROJECT_ROOT, 'missing') + const readdir = mock(async (_path: PathLike) => [] as Dirent[]) + const fs = { + realpath: mock(async (requestedPath: PathLike) => { + const requestedPathString = String(requestedPath) + if (requestedPathString === missingPath) { + throw new Error( + `ENOENT: no such file or directory, realpath '${missingPath}'`, + ) + } + return requestedPathString + }), + readdir, + } as unknown as CodebuffFileSystem + + const result = await listDirectory({ + directoryPath: 'missing', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + errorMessage: `Failed to list directory: ENOENT: no such file or directory, realpath '${missingPath}'`, + }, + }, + ]) + expect(readdir).not.toHaveBeenCalled() + }) + + it('rejects sibling paths that only share the project prefix', async () => { + const siblingPath = path.resolve(PROJECT_ROOT, '..', 'project-evil') + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + [siblingPath]: siblingPath, + }) + + const result = await listDirectory({ + directoryPath: '../project-evil', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + errorMessage: + "Invalid path: Path '../project-evil' is outside the project directory.", + }, + }, + ]) + expect(readdir).not.toHaveBeenCalled() + }) + + it('rejects the project parent directory', async () => { + const parentPath = path.dirname(PROJECT_ROOT) + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + [parentPath]: parentPath, + }) + + const result = await listDirectory({ + directoryPath: '..', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + errorMessage: + "Invalid path: Path '..' is outside the project directory.", + }, + }, + ]) + expect(readdir).not.toHaveBeenCalled() + }) + + it('rejects directories that escape through a symlink', async () => { + const symlinkPath = path.join(PROJECT_ROOT, 'link') + const outsidePath = path.resolve(PROJECT_ROOT, '..', 'outside') + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + [symlinkPath]: outsidePath, + }) + + const result = await listDirectory({ + directoryPath: 'link', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + errorMessage: + "Invalid path: Path 'link' is outside the project directory.", + }, + }, + ]) + expect(readdir).not.toHaveBeenCalled() + }) + + it('allows a symlink that resolves inside the project', async () => { + const symlinkPath = path.join(PROJECT_ROOT, 'link') + const realTarget = path.join(PROJECT_ROOT, 'src') + const { fs, readdir } = createFs({ + [PROJECT_ROOT]: PROJECT_ROOT, + [symlinkPath]: realTarget, + }) + + const result = await listDirectory({ + directoryPath: 'link', + projectPath: PROJECT_ROOT, + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + files: ['index.ts'], + directories: [], + path: 'link', + }, + }, + ]) + expect(readdir).toHaveBeenCalledWith(realTarget, { + withFileTypes: true, + }) + }) +}) diff --git a/sdk/src/tools/list-directory.ts b/sdk/src/tools/list-directory.ts index 3bf66fa968..53f12b660e 100644 --- a/sdk/src/tools/list-directory.ts +++ b/sdk/src/tools/list-directory.ts @@ -2,6 +2,7 @@ import * as path from 'path' import type { CodebuffToolOutput } from '@codebuff/common/tools/list' import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' +import { isPathInside } from '@codebuff/common/util/path' export async function listDirectory(params: { directoryPath: string @@ -11,9 +12,23 @@ export async function listDirectory(params: { const { directoryPath, projectPath, fs } = params try { - const resolvedPath = path.resolve(projectPath, directoryPath) + const projectRoot = path.resolve(projectPath) + const resolvedPath = path.resolve(projectRoot, directoryPath) + const realProjectRoot = await fs.realpath(projectRoot) + const realResolvedPath = await fs.realpath(resolvedPath) - const entries = await fs.readdir(resolvedPath, { + if (!isPathInside(realProjectRoot, realResolvedPath)) { + return [ + { + type: 'json', + value: { + errorMessage: `Invalid path: Path '${directoryPath}' is outside the project directory.`, + }, + }, + ] + } + + const entries = await fs.readdir(realResolvedPath, { withFileTypes: true, })