mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-07-13 03:20:33 -07:00
fix(core): prevent path traversal in custome command file injection (#27234)
This commit is contained in:
@@ -413,6 +413,113 @@ describe('readPathFromWorkspace', () => {
|
||||
).rejects.toThrow('Path not found in workspace: not-found.txt');
|
||||
});
|
||||
|
||||
it('should prevent path traversal outside the workspace via relative paths', async () => {
|
||||
mock({
|
||||
[CWD]: {},
|
||||
[OUTSIDE_DIR]: {
|
||||
'secret.txt': 'secrets',
|
||||
},
|
||||
});
|
||||
const config = createMockConfig(CWD);
|
||||
// Attempt to traverse out of CWD to OUTSIDE_DIR
|
||||
const relativeTraversal = path.join('..', 'outside', 'secret.txt');
|
||||
await expect(
|
||||
readPathFromWorkspace(relativeTraversal, config),
|
||||
).rejects.toThrow(`Path not found in workspace: ${relativeTraversal}`);
|
||||
});
|
||||
|
||||
it('should prevent symlink escape outside the workspace', async () => {
|
||||
mock({
|
||||
[CWD]: {
|
||||
'malicious-link': mock.symlink({
|
||||
path: path.join(OUTSIDE_DIR, 'secret.txt'),
|
||||
}),
|
||||
},
|
||||
[OUTSIDE_DIR]: {
|
||||
'secret.txt': 'secrets',
|
||||
},
|
||||
});
|
||||
const config = createMockConfig(CWD);
|
||||
// Even if the link is in the workspace, its target is not.
|
||||
await expect(
|
||||
readPathFromWorkspace('malicious-link', config),
|
||||
).rejects.toThrow('Path not found in workspace: malicious-link');
|
||||
});
|
||||
|
||||
it('should block symlink escape inside a directory expansion (defense-in-depth)', async () => {
|
||||
mock({
|
||||
[CWD]: {
|
||||
'allowed-dir': {
|
||||
'legit.txt': 'legit content',
|
||||
'malicious-link.txt': mock.symlink({
|
||||
path: path.join(OUTSIDE_DIR, 'secret.txt'),
|
||||
}),
|
||||
},
|
||||
},
|
||||
[OUTSIDE_DIR]: {
|
||||
'secret.txt': 'secrets',
|
||||
},
|
||||
});
|
||||
const mockFileService = {
|
||||
filterFiles: vi.fn((files) => files),
|
||||
} as unknown as FileDiscoveryService;
|
||||
const config = createMockConfig(CWD, [], mockFileService);
|
||||
const result = await readPathFromWorkspace('allowed-dir', config);
|
||||
const resultText = result
|
||||
.map((p) => {
|
||||
if (typeof p === 'string') return p;
|
||||
if (typeof p === 'object' && p && 'text' in p) return p.text;
|
||||
return '';
|
||||
})
|
||||
.join('');
|
||||
|
||||
// Legit content should be there
|
||||
expect(resultText).toContain('legit content');
|
||||
// Secret content should NOT be there, but a skip message SHOULD be
|
||||
expect(resultText).not.toContain('secrets');
|
||||
expect(resultText).toContain(
|
||||
'--- Skipped malicious-link.txt: traverses outside workspace ---',
|
||||
);
|
||||
});
|
||||
|
||||
it('should push multiple skip messages if multiple traversals are found in a directory', async () => {
|
||||
mock({
|
||||
[CWD]: {
|
||||
'bad-dir': {
|
||||
'link1.txt': mock.symlink({ path: path.join(OUTSIDE_DIR, 's1.txt') }),
|
||||
'link2.txt': mock.symlink({ path: path.join(OUTSIDE_DIR, 's2.txt') }),
|
||||
'good.txt': 'good content',
|
||||
},
|
||||
},
|
||||
[OUTSIDE_DIR]: {
|
||||
's1.txt': 'secret1',
|
||||
's2.txt': 'secret2',
|
||||
},
|
||||
});
|
||||
const mockFileService = {
|
||||
filterFiles: vi.fn((files) => files),
|
||||
} as unknown as FileDiscoveryService;
|
||||
const config = createMockConfig(CWD, [], mockFileService);
|
||||
const result = await readPathFromWorkspace('bad-dir', config);
|
||||
const resultText = result
|
||||
.map((p) => {
|
||||
if (typeof p === 'string') return p;
|
||||
if (typeof p === 'object' && p && 'text' in p) return p.text;
|
||||
return '';
|
||||
})
|
||||
.join('');
|
||||
|
||||
expect(resultText).toContain('good content');
|
||||
expect(resultText).toContain(
|
||||
'--- Skipped link1.txt: traverses outside workspace ---',
|
||||
);
|
||||
expect(resultText).toContain(
|
||||
'--- Skipped link2.txt: traverses outside workspace ---',
|
||||
);
|
||||
expect(resultText).not.toContain('secret1');
|
||||
expect(resultText).not.toContain('secret2');
|
||||
});
|
||||
|
||||
// mock-fs permission simulation is unreliable on Windows.
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
'should return an error string if reading a file with no permissions',
|
||||
|
||||
@@ -40,6 +40,12 @@ export async function readPathFromWorkspace(
|
||||
const searchDirs = workspace.getDirectories();
|
||||
for (const dir of searchDirs) {
|
||||
const potentialPath = path.resolve(dir, pathStr);
|
||||
|
||||
// Security check: ensure the resolved path is actually within the workspace.
|
||||
if (!workspace.isPathWithinWorkspace(potentialPath)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
await fs.access(potentialPath);
|
||||
absolutePath = potentialPath;
|
||||
@@ -81,6 +87,15 @@ export async function readPathFromWorkspace(
|
||||
);
|
||||
|
||||
for (const filePath of finalFiles) {
|
||||
// Defense in depth: validate each file found within the directory.
|
||||
if (!workspace.isPathWithinWorkspace(filePath)) {
|
||||
const relativePathForDisplay = path.relative(absolutePath, filePath);
|
||||
allParts.push({
|
||||
text: `--- Skipped ${relativePathForDisplay}: traverses outside workspace ---\n\n`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const relativePathForDisplay = path.relative(absolutePath, filePath);
|
||||
allParts.push({ text: `--- ${relativePathForDisplay} ---\n` });
|
||||
const result = await processSingleFileContent(
|
||||
|
||||
Reference in New Issue
Block a user