Skip to content

Commit ff22edd

Browse files
Merge pull request #142 from gemini-cli-extensions/deterministic_local_path_traversal
Restrict PoC command to only run within it's directory, and on a spec…
2 parents d005c90 + 7827e93 commit ff22edd

4 files changed

Lines changed: 92 additions & 30 deletions

File tree

mcp-server/src/constants.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import path from 'path';
8+
9+
export const SECURITY_DIR_NAME = '.gemini_security';
10+
export const POC_DIR_NAME = 'poc';
11+
12+
export const SECURITY_DIR = path.join(process.cwd(), SECURITY_DIR_NAME);
13+
export const POC_DIR = path.join(SECURITY_DIR, POC_DIR_NAME);

mcp-server/src/index.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import path from 'path';
1414
import { getAuditScope } from './filesystem.js';
1515
import { findLineNumbers } from './security.js';
1616
import { parseMarkdownToDict } from './parser.js';
17+
import { SECURITY_DIR_NAME, POC_DIR_NAME } from './constants.js';
1718

1819
import { runPoc } from './poc.js';
1920

@@ -67,12 +68,12 @@ server.tool(
6768

6869
server.tool(
6970
'convert_report_to_json',
70-
'Converts the Markdown security report into a JSON file named security_report.json in the .gemini_security folder.',
71+
`Converts the Markdown security report into a JSON file named security_report.json in the ${SECURITY_DIR_NAME} folder.`,
7172
{} as any,
7273
(async () => {
7374
try {
74-
const reportPath = path.join(process.cwd(), '.gemini_security/DRAFT_SECURITY_REPORT.md');
75-
const outputPath = path.join(process.cwd(), '.gemini_security/security_report.json');
75+
const reportPath = path.join(process.cwd(), `${SECURITY_DIR_NAME}/DRAFT_SECURITY_REPORT.md`);
76+
const outputPath = path.join(process.cwd(), `${SECURITY_DIR_NAME}/security_report.json`);
7677

7778
const content = await fs.readFile(reportPath, 'utf-8');
7879
const results = parseMarkdownToDict(content);
@@ -113,13 +114,13 @@ server.registerPrompt(
113114
role: 'user' as const,
114115
content: {
115116
type: 'text' as const,
116-
text: `You are a helpful assistant that helps users maintain notes. Your task is to add a new entry to the notes file at '.gemini_security/${notePath}'.
117+
text: `You are a helpful assistant that helps users maintain notes. Your task is to add a new entry to the notes file at '${SECURITY_DIR_NAME}/${notePath}'.
117118
118119
You MUST use the 'ReadFile' and 'WriteFile' tools.
119120
120121
**Workflow:**
121122
122-
1. **Read the file:** First, you MUST attempt to read the file at '.gemini_security/${notePath}' using the 'ReadFile' tool.
123+
1. **Read the file:** First, you MUST attempt to read the file at '${SECURITY_DIR_NAME}/${notePath}' using the 'ReadFile' tool.
123124
124125
2. **Handle the result:**
125126
* **If the file exists:**
@@ -129,7 +130,7 @@ server.registerPrompt(
129130
* Once you have a consistent entry, append it to the content, ensuring it perfectly matches the existing format.
130131
* Use the 'WriteFile' tool to write the **entire updated content** back to the file.
131132
* **If the file does NOT exist (ReadFile returns an error):**
132-
* First, if the '.gemini_security' directory doesn't exist, create it.
133+
* First, if the '${SECURITY_DIR_NAME}' directory doesn't exist, create it.
133134
* This is a new note. You MUST ask the user to define a template for this note.
134135
* Once the user provides a template, construct the initial file content. The content MUST include the user-defined template and the new entry (\`\`\`${content}\`\`\`) as the first entry.
135136
* Use the 'WriteFile' tool to create the new file with the complete initial content.
@@ -168,8 +169,13 @@ server.registerPrompt(
168169
**Workflow:**
169170
170171
1. **Generate PoC:**
171-
* Create a 'poc' directory in '.gemini_security' if it doesn't exist.
172-
* Generate a Node.js script that demonstrates the vulnerability under the '.gemini_security/poc/' directory.
172+
* Create a '${POC_DIR_NAME}' directory in '${SECURITY_DIR_NAME}' if it doesn't exist.
173+
* Generate a Node.js script that demonstrates the vulnerability under the '${SECURITY_DIR_NAME}/${POC_DIR_NAME}/' directory.
174+
* Based on the vulnerability type certain criteria must be met in our script, otherwise generate the PoC to the best of your ability:
175+
* If the vulnerability is a Path Traversal Vulnerability:
176+
* **YOU MUST** Use the 'write_file' tool to create a temporary file '../gcli_secext_temp.txt' directly outside of the project directory.
177+
* The script should then try to read the file using the vulnerability in the user's code.
178+
* **YOU MUST** Use the 'write_file' tool to delete the '../gcli_secext_temp.txt' file after the verification step, regardless of whether the read was successful or not.
173179
* The script should import the user's vulnerable file(s), and demonstrate the vulnerability in their code.
174180
175181
2. **Run PoC:**

mcp-server/src/poc.test.ts

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,55 +6,78 @@
66

77
import { describe, it, vi, expect } from 'vitest';
88
import { runPoc } from './poc.js';
9+
import { POC_DIR } from './constants.js';
910

1011
describe('runPoc', () => {
12+
const mockPath = {
13+
dirname: (p: string) => p.substring(0, p.lastIndexOf('/')),
14+
resolve: (p1: string, p2?: string) => {
15+
if (p2 && p2.startsWith('/')) return p2;
16+
if (p2) return p1 + '/' + p2;
17+
return p1;
18+
},
19+
sep: '/',
20+
};
21+
1122
it('should execute the file at the given path', async () => {
12-
const mockPath = {
13-
dirname: (p: string) => p.substring(0, p.lastIndexOf('/')),
14-
};
1523
const mockExecAsync = vi.fn(async (cmd: string) => {
1624
if (cmd.startsWith('npm install')) {
1725
return { stdout: '', stderr: '' };
1826
}
1927
return { stdout: 'output', stderr: '' };
2028
});
29+
const mockExecFileAsync = vi.fn(async (file: string, args?: string[]) => {
30+
return { stdout: 'output', stderr: '' };
31+
});
2132

2233
const result = await runPoc(
23-
{ filePath: '/tmp/test.js' },
24-
{ fs: {} as any, path: mockPath as any, execAsync: mockExecAsync as any }
34+
{ filePath: `${POC_DIR}/test.js` },
35+
{ fs: {} as any, path: mockPath as any, execAsync: mockExecAsync as any, execFileAsync: mockExecFileAsync as any }
2536
);
2637

27-
expect(mockExecAsync).toHaveBeenCalledTimes(2);
28-
expect(mockExecAsync).toHaveBeenNthCalledWith(
29-
1,
38+
expect(mockExecAsync).toHaveBeenCalledTimes(1);
39+
expect(mockExecAsync).toHaveBeenCalledWith(
3040
'npm install --registry=https://registry.npmjs.org/',
31-
{ cwd: '/tmp' }
41+
{ cwd: POC_DIR }
3242
);
33-
expect(mockExecAsync).toHaveBeenNthCalledWith(2, 'node /tmp/test.js');
43+
expect(mockExecFileAsync).toHaveBeenCalledTimes(1);
44+
expect(mockExecFileAsync).toHaveBeenCalledWith('node', [`${POC_DIR}/test.js`]);
3445
expect((result.content[0] as any).text).toBe(
3546
JSON.stringify({ stdout: 'output', stderr: '' })
3647
);
3748
});
3849

3950
it('should handle execution errors', async () => {
40-
const mockPath = {
41-
dirname: (p: string) => p.substring(0, p.lastIndexOf('/')),
42-
};
4351
const mockExecAsync = vi.fn(async (cmd: string) => {
44-
if (cmd.startsWith('node')) {
45-
throw new Error('Execution failed');
46-
}
4752
return { stdout: '', stderr: '' };
4853
});
54+
const mockExecFileAsync = vi.fn(async (file: string, args?: string[]) => {
55+
throw new Error('Execution failed');
56+
});
4957

5058
const result = await runPoc(
51-
{ filePath: '/tmp/error.js' },
52-
{ fs: {} as any, path: mockPath as any, execAsync: mockExecAsync as any }
59+
{ filePath: `${POC_DIR}/error.js` },
60+
{ fs: {} as any, path: mockPath as any, execAsync: mockExecAsync as any, execFileAsync: mockExecFileAsync as any }
5361
);
5462

5563
expect(result.isError).toBe(true);
5664
expect((result.content[0] as any).text).toBe(
5765
JSON.stringify({ error: 'Execution failed' })
5866
);
5967
});
68+
69+
it('should fail when accessing file outside of allowed directory', async () => {
70+
const mockExecAsync = vi.fn();
71+
const mockExecFileAsync = vi.fn();
72+
73+
const result = await runPoc(
74+
{ filePath: '/tmp/malicious.js' },
75+
{ fs: {} as any, path: mockPath as any, execAsync: mockExecAsync as any, execFileAsync: mockExecFileAsync as any }
76+
);
77+
78+
expect(result.isError).toBe(true);
79+
expect((result.content[0] as any).text).toContain('Security Error: PoC execution is restricted');
80+
expect(mockExecAsync).not.toHaveBeenCalled();
81+
expect(mockExecFileAsync).not.toHaveBeenCalled();
82+
});
6083
});

mcp-server/src/poc.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,49 @@
77
import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
88
import { promises as fs } from 'fs';
99
import path from 'path';
10-
import { exec } from 'child_process';
10+
import { exec, execFile } from 'child_process';
1111
import { promisify } from 'util';
12+
import { POC_DIR } from './constants.js';
1213

1314
const execAsync = promisify(exec);
15+
const execFileAsync = promisify(execFile);
1416

1517
export async function runPoc(
1618
{
1719
filePath,
1820
}: {
1921
filePath: string;
2022
},
21-
dependencies: { fs: typeof fs; path: typeof path; execAsync: typeof execAsync } = { fs, path, execAsync }
23+
dependencies: { fs: typeof fs; path: typeof path; execAsync: typeof execAsync; execFileAsync: typeof execFileAsync } = { fs, path, execAsync, execFileAsync }
2224
): Promise<CallToolResult> {
2325
try {
2426
const pocDir = dependencies.path.dirname(filePath);
2527

28+
// 🛡️ Validate that the filePath is within the safe PoC directory
29+
const resolvedFilePath = dependencies.path.resolve(filePath);
30+
const safePocDir = dependencies.path.resolve(process.cwd(), POC_DIR);
31+
32+
if (!resolvedFilePath.startsWith(safePocDir + dependencies.path.sep)) {
33+
return {
34+
content: [
35+
{
36+
type: 'text',
37+
text: JSON.stringify({
38+
error: `Security Error: PoC execution is restricted to files within '${safePocDir}'. Attempted to access '${resolvedFilePath}'.`,
39+
}),
40+
},
41+
],
42+
isError: true,
43+
};
44+
}
45+
2646
try {
2747
await dependencies.execAsync('npm install --registry=https://registry.npmjs.org/', { cwd: pocDir });
2848
} catch (error) {
29-
// Ignore errors from npm install, as it might fail if no package.json exists,
49+
// 📦 Ignore errors from npm install, as it might fail if no package.json exists,
3050
// but we still want to attempt running the PoC.
3151
}
32-
const { stdout, stderr } = await dependencies.execAsync(`node ${filePath}`);
52+
const { stdout, stderr } = await dependencies.execFileAsync('node', [filePath]);
3353

3454
return {
3555
content: [

0 commit comments

Comments
 (0)