Skip to content

Commit 10e1ba9

Browse files
fix(sdk): prevent list directory path escapes
1 parent 366311e commit 10e1ba9

4 files changed

Lines changed: 207 additions & 4 deletions

File tree

common/src/testing/mocks/filesystem.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { mock } from 'bun:test'
22

33
import type { CodebuffFileSystem } from '../../types/filesystem'
44
import type { Mock } from 'bun:test'
5-
import type { PathLike , Stats } from 'node:fs'
5+
import type { PathLike, Stats } from 'node:fs'
66

77
export interface CreateMockFsOptions {
88
files?: Record<string, string>
@@ -14,6 +14,7 @@ export interface CreateMockFsOptions {
1414
path: string,
1515
options?: { recursive?: boolean },
1616
) => Promise<string | undefined>
17+
realpathImpl?: (path: string) => Promise<string>
1718
statImpl?: (path: string) => Promise<Stats>
1819
}
1920

@@ -31,6 +32,7 @@ export interface MockFsWithMocks {
3132
options?: { recursive?: boolean },
3233
) => Promise<string | undefined>
3334
>
35+
realpath: Mock<(path: PathLike) => Promise<string>>
3436
stat: Mock<(path: PathLike) => Promise<Stats>>
3537
}
3638

@@ -43,6 +45,7 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs {
4345
readdirImpl,
4446
writeFileImpl,
4547
mkdirImpl,
48+
realpathImpl,
4649
statImpl,
4750
} = options
4851

@@ -79,6 +82,20 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs {
7982
return undefined
8083
}
8184

85+
const defaultRealpath = async (path: PathLike): Promise<string> => {
86+
const pathStr = String(path)
87+
const isKnownPath =
88+
pathStr in writtenFiles ||
89+
pathStr in directories ||
90+
createdDirs.has(pathStr)
91+
92+
if (!isKnownPath) {
93+
throw new Error(`Path not found: ${pathStr}`)
94+
}
95+
96+
return pathStr
97+
}
98+
8299
const defaultStat = async (path: PathLike): Promise<Stats> => {
83100
const pathStr = String(path)
84101
const isFile = pathStr in writtenFiles
@@ -134,6 +151,10 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs {
134151
mkdirImpl(String(path), opts)
135152
: defaultMkdir
136153

154+
const realpathFn = realpathImpl
155+
? async (path: PathLike) => realpathImpl(String(path))
156+
: defaultRealpath
157+
137158
const statFn = statImpl
138159
? async (path: PathLike) => statImpl(String(path))
139160
: defaultStat
@@ -143,6 +164,7 @@ export function createMockFs(options: CreateMockFsOptions = {}): MockFs {
143164
readdir: mock(readdirFn),
144165
writeFile: mock(writeFileFn),
145166
mkdir: mock(mkdirFn),
167+
realpath: mock(realpathFn),
146168
stat: mock(statFn),
147169
} as unknown as MockFs
148170
}
@@ -153,6 +175,7 @@ export function restoreMockFs(mockFs: MockFs): void {
153175
mocks.readdir.mockRestore()
154176
mocks.writeFile.mockRestore()
155177
mocks.mkdir.mockRestore()
178+
mocks.realpath.mockRestore()
156179
mocks.stat.mockRestore()
157180
}
158181

@@ -162,5 +185,6 @@ export function clearMockFs(mockFs: MockFs): void {
162185
mocks.readdir.mockClear()
163186
mocks.writeFile.mockClear()
164187
mocks.mkdir.mockClear()
188+
mocks.realpath.mockClear()
165189
mocks.stat.mockClear()
166190
}

common/src/types/filesystem.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,11 @@ import type fs from 'fs'
66
*/
77
export type CodebuffFileSystem = Pick<
88
typeof fs.promises,
9-
'mkdir' | 'readdir' | 'readFile' | 'stat' | 'unlink' | 'writeFile'
9+
| 'mkdir'
10+
| 'readdir'
11+
| 'readFile'
12+
| 'realpath'
13+
| 'stat'
14+
| 'unlink'
15+
| 'writeFile'
1016
>
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import { describe, expect, it, mock } from 'bun:test'
2+
3+
import { listDirectory } from '../tools/list-directory'
4+
5+
import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem'
6+
import type { Dirent, PathLike } from 'node:fs'
7+
8+
const PROJECT_ROOT = '/workspace/project'
9+
10+
function createFs(realpaths: Record<string, string>) {
11+
const readdir = mock(async (_path: PathLike) => {
12+
return [
13+
{
14+
name: 'index.ts',
15+
isDirectory: () => false,
16+
isFile: () => true,
17+
},
18+
] as Dirent[]
19+
})
20+
21+
const fs = {
22+
realpath: mock(async (path: PathLike) => {
23+
const pathString = String(path)
24+
return realpaths[pathString] ?? pathString
25+
}),
26+
readdir,
27+
} as unknown as CodebuffFileSystem
28+
29+
return { fs, readdir }
30+
}
31+
32+
describe('listDirectory', () => {
33+
it('allows listing the project root itself', async () => {
34+
const { fs, readdir } = createFs({
35+
[PROJECT_ROOT]: PROJECT_ROOT,
36+
})
37+
38+
const result = await listDirectory({
39+
directoryPath: '.',
40+
projectPath: PROJECT_ROOT,
41+
fs,
42+
})
43+
44+
expect(result[0]).toEqual({
45+
type: 'json',
46+
value: {
47+
files: ['index.ts'],
48+
directories: [],
49+
path: '.',
50+
},
51+
})
52+
expect(readdir).toHaveBeenCalledWith(PROJECT_ROOT, {
53+
withFileTypes: true,
54+
})
55+
})
56+
57+
it('lists a directory inside the project and preserves the requested path', async () => {
58+
const { fs, readdir } = createFs({
59+
[PROJECT_ROOT]: PROJECT_ROOT,
60+
[`${PROJECT_ROOT}/src`]: `${PROJECT_ROOT}/src`,
61+
})
62+
63+
const result = await listDirectory({
64+
directoryPath: 'src',
65+
projectPath: PROJECT_ROOT,
66+
fs,
67+
})
68+
69+
expect(result).toEqual([
70+
{
71+
type: 'json',
72+
value: {
73+
files: ['index.ts'],
74+
directories: [],
75+
path: 'src',
76+
},
77+
},
78+
])
79+
expect(readdir).toHaveBeenCalledWith(`${PROJECT_ROOT}/src`, {
80+
withFileTypes: true,
81+
})
82+
})
83+
84+
it('rejects sibling paths that only share the project prefix', async () => {
85+
const siblingPath = '/workspace/project-evil'
86+
const { fs, readdir } = createFs({
87+
[PROJECT_ROOT]: PROJECT_ROOT,
88+
[siblingPath]: siblingPath,
89+
})
90+
91+
const result = await listDirectory({
92+
directoryPath: '../project-evil',
93+
projectPath: PROJECT_ROOT,
94+
fs,
95+
})
96+
97+
expect(result).toEqual([
98+
{
99+
type: 'json',
100+
value: {
101+
errorMessage:
102+
"Invalid path: Path '../project-evil' is outside the project directory.",
103+
},
104+
},
105+
])
106+
expect(readdir).not.toHaveBeenCalled()
107+
})
108+
109+
it('rejects the project parent directory', async () => {
110+
const parentPath = '/workspace'
111+
const { fs, readdir } = createFs({
112+
[PROJECT_ROOT]: PROJECT_ROOT,
113+
[parentPath]: parentPath,
114+
})
115+
116+
const result = await listDirectory({
117+
directoryPath: '..',
118+
projectPath: PROJECT_ROOT,
119+
fs,
120+
})
121+
122+
expect(result).toEqual([
123+
{
124+
type: 'json',
125+
value: {
126+
errorMessage:
127+
"Invalid path: Path '..' is outside the project directory.",
128+
},
129+
},
130+
])
131+
expect(readdir).not.toHaveBeenCalled()
132+
})
133+
134+
it('rejects directories that escape through a symlink', async () => {
135+
const symlinkPath = `${PROJECT_ROOT}/link`
136+
const { fs, readdir } = createFs({
137+
[PROJECT_ROOT]: PROJECT_ROOT,
138+
[symlinkPath]: '/outside',
139+
})
140+
141+
const result = await listDirectory({
142+
directoryPath: 'link',
143+
projectPath: PROJECT_ROOT,
144+
fs,
145+
})
146+
147+
expect(result).toEqual([
148+
{
149+
type: 'json',
150+
value: {
151+
errorMessage:
152+
"Invalid path: Path 'link' is outside the project directory.",
153+
},
154+
},
155+
])
156+
expect(readdir).not.toHaveBeenCalled()
157+
})
158+
})

sdk/src/tools/list-directory.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as path from 'path'
22

33
import type { CodebuffToolOutput } from '@codebuff/common/tools/list'
44
import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem'
5+
import { isPathInside } from '@codebuff/common/util/path'
56

67
export async function listDirectory(params: {
78
directoryPath: string
@@ -11,9 +12,23 @@ export async function listDirectory(params: {
1112
const { directoryPath, projectPath, fs } = params
1213

1314
try {
14-
const resolvedPath = path.resolve(projectPath, directoryPath)
15+
const projectRoot = path.resolve(projectPath)
16+
const resolvedPath = path.resolve(projectRoot, directoryPath)
17+
const realProjectRoot = await fs.realpath(projectRoot)
18+
const realResolvedPath = await fs.realpath(resolvedPath)
1519

16-
const entries = await fs.readdir(resolvedPath, {
20+
if (!isPathInside(realProjectRoot, realResolvedPath)) {
21+
return [
22+
{
23+
type: 'json',
24+
value: {
25+
errorMessage: `Invalid path: Path '${directoryPath}' is outside the project directory.`,
26+
},
27+
},
28+
]
29+
}
30+
31+
const entries = await fs.readdir(realResolvedPath, {
1732
withFileTypes: true,
1833
})
1934

0 commit comments

Comments
 (0)