Skip to content

Commit a265779

Browse files
authored
fix(tools): prevent path traversal in get_doc and version injection in get_component_api (#8)
## Summary - Fix path traversal issue in `get_doc` tool that allowed reading arbitrary files outside the `docs/` directory - Fix version parameter validation issue in `get_component_api` tool that allowed querying arbitrary npm packages via URL traversal - Add tests for both fixes ## Details ### get_doc - Path traversal `get_doc` joined user-supplied paths with `DOCS_DIR` without checking the resolved path stayed within the docs directory. A path like `../../etc/passwd` could read any file on the host. **Fix:** Resolve the path and verify it starts with `DOCS_DIR` before reading. ### get_component_api - Version injection The `version` parameter was interpolated directly into the npm registry URL. A version like `../../express` resolved to `https://registry.npmjs.org/express`, skipping the hardcoded package allowlist. **Fix:** Validate the version string against `[\da-zA-Z.\-]+`, rejecting anything with slashes or other unexpected characters.
1 parent 074e3a3 commit a265779

4 files changed

Lines changed: 86 additions & 15 deletions

File tree

src/tools/get_component_api/get_component_api.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ export const getComponentApiTool = {
2525
},
2626
handler: async ({ componentName, version = 'latest' }: GetComponentAPIToolPayload) => {
2727
try {
28+
if (!/^[\da-zA-Z.\-]+$/.test(version)) {
29+
return createTextResponse(
30+
`Access denied: version "${version}" contains invalid characters. Use a semver string (e.g., "2.12.0") or "latest".`
31+
);
32+
}
33+
2834
// transform any to kebab-case e.g avatar group (space) or
2935
// avatarGroup or AvatarGroup (camelCase) to avatar-group
3036
let normalizedName = componentName

src/tools/get_docs/get_doc.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,22 @@ const DOCS_DIR = path.join(__dirname, '../../../docs');
99

1010
const docRequestSchema = z.object({
1111
path: z.string().describe('Document path (e.g., "docs/08-Releases.md")'),
12-
lines: z.string().optional().describe('Line range like "1-24" or "26-58" (optional, reads full file if omitted)'),
12+
lines: z
13+
.string()
14+
.optional()
15+
.describe('Line range like "1-24" or "26-58" (optional, reads full file if omitted)'),
1316
});
1417

1518
function readLines(filePath: string, lineRange?: string): string {
1619
const content = fs.readFileSync(filePath, 'utf8');
17-
20+
1821
if (!lineRange) {
1922
return content;
2023
}
2124

2225
const lines = content.split('\n');
23-
const [start, end] = lineRange.split('-').map(n => parseInt(n.trim(), 10));
24-
26+
const [start, end] = lineRange.split('-').map((n) => parseInt(n.trim(), 10));
27+
2528
if (isNaN(start) || isNaN(end) || start < 1 || end < start) {
2629
throw new Error(`Invalid line range: ${lineRange}`);
2730
}
@@ -31,7 +34,8 @@ function readLines(filePath: string, lineRange?: string): string {
3134

3235
export const getDocTool = {
3336
name: 'get_doc',
34-
description: 'Get full content of UI5 Web Components documentation file(s). Supports reading multiple documents and specific line ranges.',
37+
description:
38+
'Get full content of UI5 Web Components documentation file(s). Supports reading multiple documents and specific line ranges.',
3539
inputSchema: {
3640
docs: z
3741
.array(docRequestSchema)
@@ -44,6 +48,12 @@ export const getDocTool = {
4448
for (const doc of docs) {
4549
const normalizedPath = doc.path.replace(/^docs\//, '');
4650
const filePath = path.join(DOCS_DIR, normalizedPath);
51+
const resolvedPath = path.resolve(filePath);
52+
53+
if (!resolvedPath.startsWith(path.resolve(DOCS_DIR))) {
54+
results.push(`Access denied: "${doc.path}" points outside the docs directory.`);
55+
continue;
56+
}
4757

4858
if (!fs.existsSync(filePath)) {
4959
results.push(`❌ Document "${doc.path}" not found.`);
@@ -55,8 +65,9 @@ export const getDocTool = {
5565
results.push(`# ${doc.path}${rangeInfo}\n\n${content}`);
5666
}
5767

58-
const footer = '\n\n**Framework Integration:** If using React, Angular or Vue, also check the get_guidelines tool for framework-specific setup and best practices.';
59-
68+
const footer =
69+
'\n\n**Framework Integration:** If using React, Angular or Vue, also check the get_guidelines tool for framework-specific setup and best practices.';
70+
6071
return createTextResponse(results.join('\n\n---\n\n') + footer);
6172
} catch (error) {
6273
return handleToolError(error, 'Error reading documentation');

test/tools/docs.test.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,24 +40,38 @@ test('getDocTool normalizes paths', async (t) => {
4040
});
4141

4242
test('getDocTool supports line ranges', async (t) => {
43-
const result = await getDocTool.handler({
44-
docs: [{ path: 'docs/09-FAQ.md', lines: '1-10' }]
43+
const result = await getDocTool.handler({
44+
docs: [{ path: 'docs/09-FAQ.md', lines: '1-10' }],
4545
});
4646

4747
t.is(result.content[0].type, 'text');
4848
t.true(result.content[0].text.includes('[lines 1-10]'));
4949
});
5050

5151
test('getDocTool supports multiple documents', async (t) => {
52-
const result = await getDocTool.handler({
53-
docs: [
54-
{ path: 'docs/09-FAQ.md' },
55-
{ path: 'docs/08-Releases.md' }
56-
]
52+
const result = await getDocTool.handler({
53+
docs: [{ path: 'docs/09-FAQ.md' }, { path: 'docs/08-Releases.md' }],
5754
});
5855

5956
t.is(result.content[0].type, 'text');
6057
t.true(result.content[0].text.includes('docs/09-FAQ.md'));
6158
t.true(result.content[0].text.includes('docs/08-Releases.md'));
6259
});
6360

61+
test('getDocTool blocks path traversal with ../', async (t) => {
62+
const result = await getDocTool.handler({ docs: [{ path: '../../etc/passwd' }] });
63+
64+
t.true(result.content[0].text.includes('Access denied'));
65+
});
66+
67+
test('getDocTool blocks path traversal to source code', async (t) => {
68+
const result = await getDocTool.handler({ docs: [{ path: '../src/index.ts' }] });
69+
70+
t.true(result.content[0].text.includes('Access denied'));
71+
});
72+
73+
test('getDocTool blocks path traversal with docs/ prefix', async (t) => {
74+
const result = await getDocTool.handler({ docs: [{ path: 'docs/../../../etc/passwd' }] });
75+
76+
t.true(result.content[0].text.includes('Access denied'));
77+
});

test/tools/get_component_api.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,44 @@ test('returns not found for invalid component', async (t) => {
4040
});
4141

4242
t.true(result.content[0].text.includes('not found'));
43-
});
43+
});
44+
45+
test('blocks version with path traversal', async (t) => {
46+
const result = await getComponentApiTool.handler({
47+
componentName: 'Button',
48+
version: '../../express',
49+
});
50+
51+
t.true(result.content[0].text.includes('Access denied'));
52+
});
53+
54+
test('blocks version with slashes', async (t) => {
55+
const result = await getComponentApiTool.handler({
56+
componentName: 'Button',
57+
version: 'latest/../../lodash',
58+
});
59+
60+
t.true(result.content[0].text.includes('Access denied'));
61+
});
62+
63+
test('allows valid semver version', async (t) => {
64+
global.fetch = async () => ({ ok: false, status: 404 }) as Response;
65+
66+
const result = await getComponentApiTool.handler({
67+
componentName: 'Button',
68+
version: '2.6.0',
69+
});
70+
71+
t.false(result.content[0].text.includes('Access denied'));
72+
});
73+
74+
test('allows valid prerelease version', async (t) => {
75+
global.fetch = async () => ({ ok: false, status: 404 }) as Response;
76+
77+
const result = await getComponentApiTool.handler({
78+
componentName: 'Button',
79+
version: '2.0.0-rc.1',
80+
});
81+
82+
t.false(result.content[0].text.includes('Access denied'));
83+
});

0 commit comments

Comments
 (0)