diff options
Diffstat (limited to 'packages/core/src')
26 files changed, 1588 insertions, 120 deletions
diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index f2169790..dcc81b4f 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -19,6 +19,18 @@ import { import { GeminiClient } from '../core/client.js'; import { GitService } from '../services/gitService.js'; +vi.mock('fs', async (importOriginal) => { + const actual = await importOriginal<typeof import('fs')>(); + return { + ...actual, + existsSync: vi.fn().mockReturnValue(true), + statSync: vi.fn().mockReturnValue({ + isDirectory: vi.fn().mockReturnValue(true), + }), + realpathSync: vi.fn((path) => path), + }; +}); + // Mock dependencies that might be called during Config construction or createServerConfig vi.mock('../tools/tool-registry', () => { const ToolRegistryMock = vi.fn(); @@ -219,6 +231,23 @@ describe('Server Config (config.ts)', () => { expect(config.getFileFilteringRespectGitIgnore()).toBe(false); }); + it('should initialize WorkspaceContext with includeDirectories', () => { + const includeDirectories = ['/path/to/dir1', '/path/to/dir2']; + const paramsWithIncludeDirs: ConfigParameters = { + ...baseParams, + includeDirectories, + }; + const config = new Config(paramsWithIncludeDirs); + const workspaceContext = config.getWorkspaceContext(); + const directories = workspaceContext.getDirectories(); + + // Should include the target directory plus the included directories + expect(directories).toHaveLength(3); + expect(directories).toContain(path.resolve(baseParams.targetDir)); + expect(directories).toContain('/path/to/dir1'); + expect(directories).toContain('/path/to/dir2'); + }); + it('Config constructor should set telemetry to true when provided as true', () => { const paramsWithTelemetry: ConfigParameters = { ...baseParams, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index c92fb623..d8bce341 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -50,6 +50,7 @@ import { IdeClient } from '../ide/ide-client.js'; // Re-export OAuth config type export type { MCPOAuthConfig }; +import { WorkspaceContext } from '../utils/workspaceContext.js'; export enum ApprovalMode { DEFAULT = 'default', @@ -172,6 +173,7 @@ export interface ConfigParameters { proxy?: string; cwd: string; fileDiscoveryService?: FileDiscoveryService; + includeDirectories?: string[]; bugCommand?: BugCommandSettings; model: string; extensionContextFilePaths?: string[]; @@ -194,6 +196,7 @@ export class Config { private readonly embeddingModel: string; private readonly sandbox: SandboxConfig | undefined; private readonly targetDir: string; + private readonly workspaceContext: WorkspaceContext; private readonly debugMode: boolean; private readonly question: string | undefined; private readonly fullContext: boolean; @@ -248,6 +251,10 @@ export class Config { params.embeddingModel ?? DEFAULT_GEMINI_EMBEDDING_MODEL; this.sandbox = params.sandbox; this.targetDir = path.resolve(params.targetDir); + this.workspaceContext = new WorkspaceContext( + this.targetDir, + params.includeDirectories ?? [], + ); this.debugMode = params.debugMode; this.question = params.question; this.fullContext = params.fullContext ?? false; @@ -392,6 +399,10 @@ export class Config { return this.targetDir; } + getWorkspaceContext(): WorkspaceContext { + return this.workspaceContext; + } + getToolRegistry(): Promise<ToolRegistry> { return Promise.resolve(this.toolRegistry); } diff --git a/packages/core/src/config/flashFallback.test.ts b/packages/core/src/config/flashFallback.test.ts index cd78cd34..a0034ea1 100644 --- a/packages/core/src/config/flashFallback.test.ts +++ b/packages/core/src/config/flashFallback.test.ts @@ -4,14 +4,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { Config } from './config.js'; import { DEFAULT_GEMINI_MODEL, DEFAULT_GEMINI_FLASH_MODEL } from './models.js'; +import fs from 'node:fs'; + +vi.mock('node:fs'); describe('Flash Model Fallback Configuration', () => { let config: Config; beforeEach(() => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => true, + } as fs.Stats); config = new Config({ sessionId: 'test-session', targetDir: '/test', diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 1da355f4..96ddec1c 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -200,6 +200,9 @@ describe('Gemini Client (client.ts)', () => { getNoBrowser: vi.fn().mockReturnValue(false), getUsageStatisticsEnabled: vi.fn().mockReturnValue(true), getIdeMode: vi.fn().mockReturnValue(false), + getWorkspaceContext: vi.fn().mockReturnValue({ + getDirectories: vi.fn().mockReturnValue(['/test/dir']), + }), getGeminiClient: vi.fn(), setFallbackMode: vi.fn(), }; diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 6337bac2..ecc7c242 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -172,7 +172,6 @@ export class GeminiClient { } private async getEnvironment(): Promise<Part[]> { - const cwd = this.config.getWorkingDir(); const today = new Date().toLocaleDateString(undefined, { weekday: 'long', year: 'numeric', @@ -180,14 +179,35 @@ export class GeminiClient { day: 'numeric', }); const platform = process.platform; - const folderStructure = await getFolderStructure(cwd, { - fileService: this.config.getFileService(), - }); + + const workspaceContext = this.config.getWorkspaceContext(); + const workspaceDirectories = workspaceContext.getDirectories(); + + const folderStructures = await Promise.all( + workspaceDirectories.map((dir) => + getFolderStructure(dir, { + fileService: this.config.getFileService(), + }), + ), + ); + + const folderStructure = folderStructures.join('\n'); + + let workingDirPreamble: string; + if (workspaceDirectories.length === 1) { + workingDirPreamble = `I'm currently working in the directory: ${workspaceDirectories[0]}`; + } else { + const dirList = workspaceDirectories + .map((dir) => ` - ${dir}`) + .join('\n'); + workingDirPreamble = `I'm currently working in the following directories:\n${dirList}`; + } + const context = ` This is the Gemini CLI. We are setting up the context for our chat. Today's date is ${today}. My operating system is: ${platform} - I'm currently working in the directory: ${cwd} + ${workingDirPreamble} ${folderStructure} `.trim(); diff --git a/packages/core/src/test-utils/mockWorkspaceContext.ts b/packages/core/src/test-utils/mockWorkspaceContext.ts new file mode 100644 index 00000000..61497b3e --- /dev/null +++ b/packages/core/src/test-utils/mockWorkspaceContext.ts @@ -0,0 +1,33 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi } from 'vitest'; +import { WorkspaceContext } from '../utils/workspaceContext.js'; + +/** + * Creates a mock WorkspaceContext for testing + * @param rootDir The root directory to use for the mock + * @param additionalDirs Optional additional directories to include in the workspace + * @returns A mock WorkspaceContext instance + */ +export function createMockWorkspaceContext( + rootDir: string, + additionalDirs: string[] = [], +): WorkspaceContext { + const allDirs = [rootDir, ...additionalDirs]; + + const mockWorkspaceContext = { + addDirectory: vi.fn(), + getDirectories: vi.fn().mockReturnValue(allDirs), + isPathWithinWorkspace: vi + .fn() + .mockImplementation((path: string) => + allDirs.some((dir) => path.startsWith(dir)), + ), + } as unknown as WorkspaceContext; + + return mockWorkspaceContext; +} diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 4ff33ff4..b44d7e6f 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -32,6 +32,7 @@ import fs from 'fs'; import os from 'os'; import { ApprovalMode, Config } from '../config/config.js'; import { Content, Part, SchemaUnion } from '@google/genai'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; describe('EditTool', () => { let tool: EditTool; @@ -41,6 +42,7 @@ describe('EditTool', () => { let geminiClient: any; beforeEach(() => { + vi.restoreAllMocks(); tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'edit-tool-test-')); rootDir = path.join(tempDir, 'root'); fs.mkdirSync(rootDir); @@ -54,6 +56,7 @@ describe('EditTool', () => { getTargetDir: () => rootDir, getApprovalMode: vi.fn(), setApprovalMode: vi.fn(), + getWorkspaceContext: () => createMockWorkspaceContext(rootDir), // getGeminiConfig: () => ({ apiKey: 'test-api-key' }), // This was not a real Config method // Add other properties/methods of Config if EditTool uses them // Minimal other methods to satisfy Config type if needed by EditTool constructor or other direct uses: @@ -215,8 +218,9 @@ describe('EditTool', () => { old_string: 'old', new_string: 'new', }; - expect(tool.validateToolParams(params)).toMatch( - /File path must be within the root directory/, + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', ); }); }); @@ -675,4 +679,28 @@ describe('EditTool', () => { ); }); }); + + describe('workspace boundary validation', () => { + it('should validate paths are within workspace root', () => { + const validPath = { + file_path: path.join(rootDir, 'file.txt'), + old_string: 'old', + new_string: 'new', + }; + expect(tool.validateToolParams(validPath)).toBeNull(); + }); + + it('should reject paths outside workspace root', () => { + const invalidPath = { + file_path: '/etc/passwd', + old_string: 'root', + new_string: 'hacked', + }; + const error = tool.validateToolParams(invalidPath); + expect(error).toContain( + 'File path must be within one of the workspace directories', + ); + expect(error).toContain(rootDir); + }); + }); }); diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index fd936611..ff2bc204 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -26,7 +26,6 @@ import { ensureCorrectEdit } from '../utils/editCorrector.js'; import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js'; import { ReadFileTool } from './read-file.js'; import { ModifiableTool, ModifyContext } from './modifiable-tool.js'; -import { isWithinRoot } from '../utils/fileUtils.js'; /** * Parameters for the Edit tool @@ -137,8 +136,10 @@ Expectation for required parameters: return `File path must be absolute: ${params.file_path}`; } - if (!isWithinRoot(params.file_path, this.config.getTargetDir())) { - return `File path must be within the root directory (${this.config.getTargetDir()}): ${params.file_path}`; + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(params.file_path)) { + const directories = workspaceContext.getDirectories(); + return `File path must be within one of the workspace directories: ${directories.join(', ')}`; } return null; diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index 51effe4e..0ee6c0ee 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -9,9 +9,10 @@ import { partListUnionToString } from '../core/geminiRequest.js'; import path from 'path'; import fs from 'fs/promises'; import os from 'os'; -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; // Removed vi +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { Config } from '../config/config.js'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; describe('GlobTool', () => { let tempRootDir: string; // This will be the rootDirectory for the GlobTool instance @@ -23,6 +24,7 @@ describe('GlobTool', () => { getFileService: () => new FileDiscoveryService(tempRootDir), getFileFilteringRespectGitIgnore: () => true, getTargetDir: () => tempRootDir, + getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir), } as unknown as Config; beforeEach(async () => { @@ -243,7 +245,7 @@ describe('GlobTool', () => { path: '../../../../../../../../../../tmp', }; // Definitely outside expect(specificGlobTool.validateToolParams(paramsOutside)).toContain( - "resolves outside the tool's root directory", + 'resolves outside the allowed workspace directories', ); }); @@ -264,6 +266,37 @@ describe('GlobTool', () => { ); }); }); + + describe('workspace boundary validation', () => { + it('should validate search paths are within workspace boundaries', () => { + const validPath = { pattern: '*.ts', path: 'sub' }; + const invalidPath = { pattern: '*.ts', path: '../..' }; + + expect(globTool.validateToolParams(validPath)).toBeNull(); + expect(globTool.validateToolParams(invalidPath)).toContain( + 'resolves outside the allowed workspace directories', + ); + }); + + it('should provide clear error messages when path is outside workspace', () => { + const invalidPath = { pattern: '*.ts', path: '/etc' }; + const error = globTool.validateToolParams(invalidPath); + + expect(error).toContain( + 'resolves outside the allowed workspace directories', + ); + expect(error).toContain(tempRootDir); + }); + + it('should work with paths in workspace subdirectories', async () => { + const params: GlobToolParams = { pattern: '*.md', path: 'sub' }; + const result = await globTool.execute(params, abortSignal); + + expect(result.llmContent).toContain('Found 2 file(s)'); + expect(result.llmContent).toContain('fileC.md'); + expect(result.llmContent).toContain('FileD.MD'); + }); + }); }); describe('sortFileEntries', () => { diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 417495fe..5bcb9778 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -11,7 +11,6 @@ import { SchemaValidator } from '../utils/schemaValidator.js'; import { BaseTool, Icon, ToolResult } from './tools.js'; import { Type } from '@google/genai'; import { shortenPath, makeRelative } from '../utils/paths.js'; -import { isWithinRoot } from '../utils/fileUtils.js'; import { Config } from '../config/config.js'; // Subset of 'Path' interface provided by 'glob' that we can implement for testing @@ -130,8 +129,10 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { params.path || '.', ); - if (!isWithinRoot(searchDirAbsolute, this.config.getTargetDir())) { - return `Search path ("${searchDirAbsolute}") resolves outside the tool's root directory ("${this.config.getTargetDir()}").`; + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(searchDirAbsolute)) { + const directories = workspaceContext.getDirectories(); + return `Search path ("${searchDirAbsolute}") resolves outside the allowed workspace directories: ${directories.join(', ')}`; } const targetDir = searchDirAbsolute || this.config.getTargetDir(); @@ -189,10 +190,27 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { } try { - const searchDirAbsolute = path.resolve( - this.config.getTargetDir(), - params.path || '.', - ); + const workspaceContext = this.config.getWorkspaceContext(); + const workspaceDirectories = workspaceContext.getDirectories(); + + // If a specific path is provided, resolve it and check if it's within workspace + let searchDirectories: readonly string[]; + if (params.path) { + const searchDirAbsolute = path.resolve( + this.config.getTargetDir(), + params.path, + ); + if (!workspaceContext.isPathWithinWorkspace(searchDirAbsolute)) { + return { + llmContent: `Error: Path "${params.path}" is not within any workspace directory`, + returnDisplay: `Path is not within workspace`, + }; + } + searchDirectories = [searchDirAbsolute]; + } else { + // Search across all workspace directories + searchDirectories = workspaceDirectories; + } // Get centralized file discovery service const respectGitIgnore = @@ -200,17 +218,26 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { this.config.getFileFilteringRespectGitIgnore(); const fileDiscovery = this.config.getFileService(); - const entries = (await glob(params.pattern, { - cwd: searchDirAbsolute, - withFileTypes: true, - nodir: true, - stat: true, - nocase: !params.case_sensitive, - dot: true, - ignore: ['**/node_modules/**', '**/.git/**'], - follow: false, - signal, - })) as GlobPath[]; + // Collect entries from all search directories + let allEntries: GlobPath[] = []; + + for (const searchDir of searchDirectories) { + const entries = (await glob(params.pattern, { + cwd: searchDir, + withFileTypes: true, + nodir: true, + stat: true, + nocase: !params.case_sensitive, + dot: true, + ignore: ['**/node_modules/**', '**/.git/**'], + follow: false, + signal, + })) as GlobPath[]; + + allEntries = allEntries.concat(entries); + } + + const entries = allEntries; // Apply git-aware filtering if enabled and in git repository let filteredEntries = entries; @@ -236,7 +263,12 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { } if (!filteredEntries || filteredEntries.length === 0) { - let message = `No files found matching pattern "${params.pattern}" within ${searchDirAbsolute}.`; + let message = `No files found matching pattern "${params.pattern}"`; + if (searchDirectories.length === 1) { + message += ` within ${searchDirectories[0]}`; + } else { + message += ` within ${searchDirectories.length} workspace directories`; + } if (gitIgnoredCount > 0) { message += ` (${gitIgnoredCount} files were git-ignored)`; } @@ -263,7 +295,12 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { const fileListDescription = sortedAbsolutePaths.join('\n'); const fileCount = sortedAbsolutePaths.length; - let resultMessage = `Found ${fileCount} file(s) matching "${params.pattern}" within ${searchDirAbsolute}`; + let resultMessage = `Found ${fileCount} file(s) matching "${params.pattern}"`; + if (searchDirectories.length === 1) { + resultMessage += ` within ${searchDirectories[0]}`; + } else { + resultMessage += ` across ${searchDirectories.length} workspace directories`; + } if (gitIgnoredCount > 0) { resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`; } diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index 01295083..aadc93ae 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -10,6 +10,7 @@ import path from 'path'; import fs from 'fs/promises'; import os from 'os'; import { Config } from '../config/config.js'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; // Mock the child_process module to control grep/git grep behavior vi.mock('child_process', () => ({ @@ -33,6 +34,7 @@ describe('GrepTool', () => { const mockConfig = { getTargetDir: () => tempRootDir, + getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir), } as unknown as Config; beforeEach(async () => { @@ -120,7 +122,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'world' }; const result = await grepTool.execute(params, abortSignal); expect(result.llmContent).toContain( - 'Found 3 matches for pattern "world" in path "."', + 'Found 3 matches for pattern "world" in the workspace directory', ); expect(result.llmContent).toContain('File: fileA.txt'); expect(result.llmContent).toContain('L1: hello world'); @@ -147,7 +149,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'hello', include: '*.js' }; const result = await grepTool.execute(params, abortSignal); expect(result.llmContent).toContain( - 'Found 1 match for pattern "hello" in path "." (filter: "*.js")', + 'Found 1 match for pattern "hello" in the workspace directory (filter: "*.js"):', ); expect(result.llmContent).toContain('File: fileB.js'); expect(result.llmContent).toContain( @@ -179,7 +181,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'nonexistentpattern' }; const result = await grepTool.execute(params, abortSignal); expect(result.llmContent).toContain( - 'No matches found for pattern "nonexistentpattern" in path "."', + 'No matches found for pattern "nonexistentpattern" in the workspace directory.', ); expect(result.returnDisplay).toBe('No matches found'); }); @@ -188,7 +190,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'foo.*bar' }; // Matches 'const foo = "bar";' const result = await grepTool.execute(params, abortSignal); expect(result.llmContent).toContain( - 'Found 1 match for pattern "foo.*bar" in path "."', + 'Found 1 match for pattern "foo.*bar" in the workspace directory:', ); expect(result.llmContent).toContain('File: fileB.js'); expect(result.llmContent).toContain('L1: const foo = "bar";'); @@ -198,7 +200,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'HELLO' }; const result = await grepTool.execute(params, abortSignal); expect(result.llmContent).toContain( - 'Found 2 matches for pattern "HELLO" in path "."', + 'Found 2 matches for pattern "HELLO" in the workspace directory:', ); expect(result.llmContent).toContain('File: fileA.txt'); expect(result.llmContent).toContain('L1: hello world'); @@ -220,6 +222,98 @@ describe('GrepTool', () => { }); }); + describe('multi-directory workspace', () => { + it('should search across all workspace directories when no path is specified', async () => { + // Create additional directory with test files + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.writeFile( + path.join(secondDir, 'other.txt'), + 'hello from second directory\nworld in second', + ); + await fs.writeFile( + path.join(secondDir, 'another.js'), + 'function world() { return "test"; }', + ); + + // Create a mock config with multiple directories + const multiDirConfig = { + getTargetDir: () => tempRootDir, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGrepTool = new GrepTool(multiDirConfig); + const params: GrepToolParams = { pattern: 'world' }; + const result = await multiDirGrepTool.execute(params, abortSignal); + + // Should find matches in both directories + expect(result.llmContent).toContain( + 'Found 5 matches for pattern "world"', + ); + + // Matches from first directory + expect(result.llmContent).toContain('fileA.txt'); + expect(result.llmContent).toContain('L1: hello world'); + expect(result.llmContent).toContain('L2: second line with world'); + expect(result.llmContent).toContain('fileC.txt'); + expect(result.llmContent).toContain('L1: another world in sub dir'); + + // Matches from second directory (with directory name prefix) + const secondDirName = path.basename(secondDir); + expect(result.llmContent).toContain( + `File: ${path.join(secondDirName, 'other.txt')}`, + ); + expect(result.llmContent).toContain('L2: world in second'); + expect(result.llmContent).toContain( + `File: ${path.join(secondDirName, 'another.js')}`, + ); + expect(result.llmContent).toContain('L1: function world()'); + + // Clean up + await fs.rm(secondDir, { recursive: true, force: true }); + }); + + it('should search only specified path within workspace directories', async () => { + // Create additional directory + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.mkdir(path.join(secondDir, 'sub')); + await fs.writeFile( + path.join(secondDir, 'sub', 'test.txt'), + 'hello from second sub directory', + ); + + // Create a mock config with multiple directories + const multiDirConfig = { + getTargetDir: () => tempRootDir, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGrepTool = new GrepTool(multiDirConfig); + + // Search only in the 'sub' directory of the first workspace + const params: GrepToolParams = { pattern: 'world', path: 'sub' }; + const result = await multiDirGrepTool.execute(params, abortSignal); + + // Should only find matches in the specified sub directory + expect(result.llmContent).toContain( + 'Found 1 match for pattern "world" in path "sub"', + ); + expect(result.llmContent).toContain('File: fileC.txt'); + expect(result.llmContent).toContain('L1: another world in sub dir'); + + // Should not contain matches from second directory + expect(result.llmContent).not.toContain('test.txt'); + + // Clean up + await fs.rm(secondDir, { recursive: true, force: true }); + }); + }); + describe('getDescription', () => { it('should generate correct description with pattern only', () => { const params: GrepToolParams = { pattern: 'testPattern' }; @@ -246,6 +340,21 @@ describe('GrepTool', () => { ); }); + it('should indicate searching across all workspace directories when no path specified', () => { + // Create a mock config with multiple directories + const multiDirConfig = { + getTargetDir: () => tempRootDir, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, ['/another/dir']), + } as unknown as Config; + + const multiDirGrepTool = new GrepTool(multiDirConfig); + const params: GrepToolParams = { pattern: 'testPattern' }; + expect(multiDirGrepTool.getDescription(params)).toBe( + "'testPattern' across all workspace directories", + ); + }); + it('should generate correct description with pattern, include, and path', () => { const params: GrepToolParams = { pattern: 'testPattern', diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index 177bd1aa..027ab1b1 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -92,22 +92,23 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> { /** * Checks if a path is within the root directory and resolves it. * @param relativePath Path relative to the root directory (or undefined for root). - * @returns The absolute path if valid and exists. + * @returns The absolute path if valid and exists, or null if no path specified (to search all directories). * @throws {Error} If path is outside root, doesn't exist, or isn't a directory. */ - private resolveAndValidatePath(relativePath?: string): string { - const targetPath = path.resolve( - this.config.getTargetDir(), - relativePath || '.', - ); + private resolveAndValidatePath(relativePath?: string): string | null { + // If no path specified, return null to indicate searching all workspace directories + if (!relativePath) { + return null; + } + + const targetPath = path.resolve(this.config.getTargetDir(), relativePath); - // Security Check: Ensure the resolved path is still within the root directory. - if ( - !targetPath.startsWith(this.config.getTargetDir()) && - targetPath !== this.config.getTargetDir() - ) { + // Security Check: Ensure the resolved path is within workspace boundaries + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(targetPath)) { + const directories = workspaceContext.getDirectories(); throw new Error( - `Path validation failed: Attempted path "${relativePath || '.'}" resolves outside the allowed root directory "${this.config.getTargetDir()}".`, + `Path validation failed: Attempted path "${relativePath}" resolves outside the allowed workspace directories: ${directories.join(', ')}`, ); } @@ -146,10 +147,13 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> { return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`; } - try { - this.resolveAndValidatePath(params.path); - } catch (error) { - return getErrorMessage(error); + // Only validate path if one is provided + if (params.path) { + try { + this.resolveAndValidatePath(params.path); + } catch (error) { + return getErrorMessage(error); + } } return null; // Parameters are valid @@ -174,44 +178,78 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> { }; } - let searchDirAbs: string; try { - searchDirAbs = this.resolveAndValidatePath(params.path); + const workspaceContext = this.config.getWorkspaceContext(); + const searchDirAbs = this.resolveAndValidatePath(params.path); const searchDirDisplay = params.path || '.'; - const matches: GrepMatch[] = await this.performGrepSearch({ - pattern: params.pattern, - path: searchDirAbs, - include: params.include, - signal, - }); + // Determine which directories to search + let searchDirectories: readonly string[]; + if (searchDirAbs === null) { + // No path specified - search all workspace directories + searchDirectories = workspaceContext.getDirectories(); + } else { + // Specific path provided - search only that directory + searchDirectories = [searchDirAbs]; + } - if (matches.length === 0) { - const noMatchMsg = `No matches found for pattern "${params.pattern}" in path "${searchDirDisplay}"${params.include ? ` (filter: "${params.include}")` : ''}.`; + // Collect matches from all search directories + let allMatches: GrepMatch[] = []; + for (const searchDir of searchDirectories) { + const matches = await this.performGrepSearch({ + pattern: params.pattern, + path: searchDir, + include: params.include, + signal, + }); + + // Add directory prefix if searching multiple directories + if (searchDirectories.length > 1) { + const dirName = path.basename(searchDir); + matches.forEach((match) => { + match.filePath = path.join(dirName, match.filePath); + }); + } + + allMatches = allMatches.concat(matches); + } + + let searchLocationDescription: string; + if (searchDirAbs === null) { + const numDirs = workspaceContext.getDirectories().length; + searchLocationDescription = + numDirs > 1 + ? `across ${numDirs} workspace directories` + : `in the workspace directory`; + } else { + searchLocationDescription = `in path "${searchDirDisplay}"`; + } + + if (allMatches.length === 0) { + const noMatchMsg = `No matches found for pattern "${params.pattern}" ${searchLocationDescription}${params.include ? ` (filter: "${params.include}")` : ''}.`; return { llmContent: noMatchMsg, returnDisplay: `No matches found` }; } - const matchesByFile = matches.reduce( + // Group matches by file + const matchesByFile = allMatches.reduce( (acc, match) => { - const relativeFilePath = - path.relative( - searchDirAbs, - path.resolve(searchDirAbs, match.filePath), - ) || path.basename(match.filePath); - if (!acc[relativeFilePath]) { - acc[relativeFilePath] = []; + const fileKey = match.filePath; + if (!acc[fileKey]) { + acc[fileKey] = []; } - acc[relativeFilePath].push(match); - acc[relativeFilePath].sort((a, b) => a.lineNumber - b.lineNumber); + acc[fileKey].push(match); + acc[fileKey].sort((a, b) => a.lineNumber - b.lineNumber); return acc; }, {} as Record<string, GrepMatch[]>, ); - const matchCount = matches.length; + const matchCount = allMatches.length; const matchTerm = matchCount === 1 ? 'match' : 'matches'; - let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${params.pattern}" in path "${searchDirDisplay}"${params.include ? ` (filter: "${params.include}")` : ''}:\n---\n`; + let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${params.pattern}" ${searchLocationDescription}${params.include ? ` (filter: "${params.include}")` : ''}: +--- +`; for (const filePath in matchesByFile) { llmContent += `File: ${filePath}\n`; @@ -334,6 +372,13 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> { ); description += ` within ${shortenPath(relativePath)}`; } + } else { + // When no path is specified, indicate searching all workspace directories + const workspaceContext = this.config.getWorkspaceContext(); + const directories = workspaceContext.getDirectories(); + if (directories.length > 1) { + description += ` across all workspace directories`; + } } return description; } diff --git a/packages/core/src/tools/ls.test.ts b/packages/core/src/tools/ls.test.ts new file mode 100644 index 00000000..fb99d829 --- /dev/null +++ b/packages/core/src/tools/ls.test.ts @@ -0,0 +1,496 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +vi.mock('fs', () => ({ + default: { + statSync: vi.fn(), + readdirSync: vi.fn(), + }, + statSync: vi.fn(), + readdirSync: vi.fn(), +})); +import { LSTool } from './ls.js'; +import { Config } from '../config/config.js'; +import { WorkspaceContext } from '../utils/workspaceContext.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; + +describe('LSTool', () => { + let lsTool: LSTool; + let mockConfig: Config; + let mockWorkspaceContext: WorkspaceContext; + let mockFileService: FileDiscoveryService; + const mockPrimaryDir = '/home/user/project'; + const mockSecondaryDir = '/home/user/other-project'; + + beforeEach(() => { + vi.resetAllMocks(); + + // Mock WorkspaceContext + mockWorkspaceContext = { + getDirectories: vi + .fn() + .mockReturnValue([mockPrimaryDir, mockSecondaryDir]), + isPathWithinWorkspace: vi + .fn() + .mockImplementation( + (path) => + path.startsWith(mockPrimaryDir) || + path.startsWith(mockSecondaryDir), + ), + addDirectory: vi.fn(), + } as unknown as WorkspaceContext; + + // Mock FileService + mockFileService = { + shouldGitIgnoreFile: vi.fn().mockReturnValue(false), + shouldGeminiIgnoreFile: vi.fn().mockReturnValue(false), + } as unknown as FileDiscoveryService; + + // Mock Config + mockConfig = { + getTargetDir: vi.fn().mockReturnValue(mockPrimaryDir), + getWorkspaceContext: vi.fn().mockReturnValue(mockWorkspaceContext), + getFileService: vi.fn().mockReturnValue(mockFileService), + getFileFilteringOptions: vi.fn().mockReturnValue({ + respectGitIgnore: true, + respectGeminiIgnore: true, + }), + } as unknown as Config; + + lsTool = new LSTool(mockConfig); + }); + + describe('parameter validation', () => { + it('should accept valid absolute paths within workspace', () => { + const params = { + path: '/home/user/project/src', + }; + + const error = lsTool.validateToolParams(params); + expect(error).toBeNull(); + }); + + it('should reject relative paths', () => { + const params = { + path: './src', + }; + + const error = lsTool.validateToolParams(params); + expect(error).toBe('Path must be absolute: ./src'); + }); + + it('should reject paths outside workspace with clear error message', () => { + const params = { + path: '/etc/passwd', + }; + + const error = lsTool.validateToolParams(params); + expect(error).toBe( + 'Path must be within one of the workspace directories: /home/user/project, /home/user/other-project', + ); + }); + + it('should accept paths in secondary workspace directory', () => { + const params = { + path: '/home/user/other-project/lib', + }; + + const error = lsTool.validateToolParams(params); + expect(error).toBeNull(); + }); + }); + + describe('execute', () => { + it('should list files in a directory', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['file1.ts', 'file2.ts', 'subdir']; + const mockStats = { + isDirectory: vi.fn(), + mtime: new Date(), + size: 1024, + }; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + const pathStr = path.toString(); + if (pathStr === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + // For individual files + if (pathStr.toString().endsWith('subdir')) { + return { ...mockStats, isDirectory: () => true, size: 0 } as fs.Stats; + } + return { ...mockStats, isDirectory: () => false } as fs.Stats; + }); + + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('[DIR] subdir'); + expect(result.llmContent).toContain('file1.ts'); + expect(result.llmContent).toContain('file2.ts'); + expect(result.returnDisplay).toBe('Listed 3 item(s).'); + }); + + it('should list files from secondary workspace directory', async () => { + const testPath = '/home/user/other-project/lib'; + const mockFiles = ['module1.js', 'module2.js']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + if (path.toString() === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 2048, + } as fs.Stats; + }); + + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('module1.js'); + expect(result.llmContent).toContain('module2.js'); + expect(result.returnDisplay).toBe('Listed 2 item(s).'); + }); + + it('should handle empty directories', async () => { + const testPath = '/home/user/project/empty'; + + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => true, + } as fs.Stats); + vi.mocked(fs.readdirSync).mockReturnValue([]); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toBe( + 'Directory /home/user/project/empty is empty.', + ); + expect(result.returnDisplay).toBe('Directory is empty.'); + }); + + it('should respect ignore patterns', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['test.js', 'test.spec.js', 'index.js']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + const pathStr = path.toString(); + if (pathStr === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 1024, + } as fs.Stats; + }); + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + const result = await lsTool.execute( + { path: testPath, ignore: ['*.spec.js'] }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('test.js'); + expect(result.llmContent).toContain('index.js'); + expect(result.llmContent).not.toContain('test.spec.js'); + expect(result.returnDisplay).toBe('Listed 2 item(s).'); + }); + + it('should respect gitignore patterns', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['file1.js', 'file2.js', 'ignored.js']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + const pathStr = path.toString(); + if (pathStr === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 1024, + } as fs.Stats; + }); + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + (mockFileService.shouldGitIgnoreFile as any).mockImplementation( + (path: string) => path.includes('ignored.js'), + ); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('file1.js'); + expect(result.llmContent).toContain('file2.js'); + expect(result.llmContent).not.toContain('ignored.js'); + expect(result.returnDisplay).toBe('Listed 2 item(s). (1 git-ignored)'); + }); + + it('should respect geminiignore patterns', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['file1.js', 'file2.js', 'private.js']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + const pathStr = path.toString(); + if (pathStr === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 1024, + } as fs.Stats; + }); + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + (mockFileService.shouldGeminiIgnoreFile as any).mockImplementation( + (path: string) => path.includes('private.js'), + ); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('file1.js'); + expect(result.llmContent).toContain('file2.js'); + expect(result.llmContent).not.toContain('private.js'); + expect(result.returnDisplay).toBe('Listed 2 item(s). (1 gemini-ignored)'); + }); + + it('should handle non-directory paths', async () => { + const testPath = '/home/user/project/file.txt'; + + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => false, + } as fs.Stats); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('Path is not a directory'); + expect(result.returnDisplay).toBe('Error: Path is not a directory.'); + }); + + it('should handle non-existent paths', async () => { + const testPath = '/home/user/project/does-not-exist'; + + vi.mocked(fs.statSync).mockImplementation(() => { + throw new Error('ENOENT: no such file or directory'); + }); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('Error listing directory'); + expect(result.returnDisplay).toBe('Error: Failed to list directory.'); + }); + + it('should sort directories first, then files alphabetically', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['z-file.ts', 'a-dir', 'b-file.ts', 'c-dir']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + if (path.toString() === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + if (path.toString().endsWith('-dir')) { + return { + isDirectory: () => true, + mtime: new Date(), + size: 0, + } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 1024, + } as fs.Stats; + }); + + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + const lines = ( + typeof result.llmContent === 'string' ? result.llmContent : '' + ).split('\n'); + const entries = lines.slice(1).filter((line: string) => line.trim()); // Skip header + expect(entries[0]).toBe('[DIR] a-dir'); + expect(entries[1]).toBe('[DIR] c-dir'); + expect(entries[2]).toBe('b-file.ts'); + expect(entries[3]).toBe('z-file.ts'); + }); + + it('should handle permission errors gracefully', async () => { + const testPath = '/home/user/project/restricted'; + + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => true, + } as fs.Stats); + vi.mocked(fs.readdirSync).mockImplementation(() => { + throw new Error('EACCES: permission denied'); + }); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('Error listing directory'); + expect(result.llmContent).toContain('permission denied'); + expect(result.returnDisplay).toBe('Error: Failed to list directory.'); + }); + + it('should validate parameters and return error for invalid params', async () => { + const result = await lsTool.execute( + { path: '../outside' }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('Invalid parameters provided'); + expect(result.returnDisplay).toBe('Error: Failed to execute tool.'); + }); + + it('should handle errors accessing individual files during listing', async () => { + const testPath = '/home/user/project/src'; + const mockFiles = ['accessible.ts', 'inaccessible.ts']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + if (path.toString() === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + if (path.toString().endsWith('inaccessible.ts')) { + throw new Error('EACCES: permission denied'); + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 1024, + } as fs.Stats; + }); + + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + // Spy on console.error to verify it's called + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + // Should still list the accessible file + expect(result.llmContent).toContain('accessible.ts'); + expect(result.llmContent).not.toContain('inaccessible.ts'); + expect(result.returnDisplay).toBe('Listed 1 item(s).'); + + // Verify error was logged + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Error accessing'), + ); + + consoleErrorSpy.mockRestore(); + }); + }); + + describe('getDescription', () => { + it('should return shortened relative path', () => { + const params = { + path: path.join(mockPrimaryDir, 'deeply', 'nested', 'directory'), + }; + + const description = lsTool.getDescription(params); + expect(description).toBe(path.join('deeply', 'nested', 'directory')); + }); + + it('should handle paths in secondary workspace', () => { + const params = { + path: path.join(mockSecondaryDir, 'lib'), + }; + + const description = lsTool.getDescription(params); + expect(description).toBe(path.join('..', 'other-project', 'lib')); + }); + }); + + describe('workspace boundary validation', () => { + it('should accept paths in primary workspace directory', () => { + const params = { path: `${mockPrimaryDir}/src` }; + expect(lsTool.validateToolParams(params)).toBeNull(); + }); + + it('should accept paths in secondary workspace directory', () => { + const params = { path: `${mockSecondaryDir}/lib` }; + expect(lsTool.validateToolParams(params)).toBeNull(); + }); + + it('should reject paths outside all workspace directories', () => { + const params = { path: '/etc/passwd' }; + const error = lsTool.validateToolParams(params); + expect(error).toContain( + 'Path must be within one of the workspace directories', + ); + expect(error).toContain(mockPrimaryDir); + expect(error).toContain(mockSecondaryDir); + }); + + it('should list files from secondary workspace directory', async () => { + const testPath = `${mockSecondaryDir}/tests`; + const mockFiles = ['test1.spec.ts', 'test2.spec.ts']; + + vi.mocked(fs.statSync).mockImplementation((path: any) => { + if (path.toString() === testPath) { + return { isDirectory: () => true } as fs.Stats; + } + return { + isDirectory: () => false, + mtime: new Date(), + size: 512, + } as fs.Stats; + }); + + vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); + + const result = await lsTool.execute( + { path: testPath }, + new AbortController().signal, + ); + + expect(result.llmContent).toContain('test1.spec.ts'); + expect(result.llmContent).toContain('test2.spec.ts'); + expect(result.returnDisplay).toBe('Listed 2 item(s).'); + }); + }); +}); diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index 68a69101..8490f18a 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -11,7 +11,6 @@ import { Type } from '@google/genai'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import { Config, DEFAULT_FILE_FILTERING_OPTIONS } from '../config/config.js'; -import { isWithinRoot } from '../utils/fileUtils.js'; /** * Parameters for the LS tool @@ -129,8 +128,11 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { if (!path.isAbsolute(params.path)) { return `Path must be absolute: ${params.path}`; } - if (!isWithinRoot(params.path, this.config.getTargetDir())) { - return `Path must be within the root directory (${this.config.getTargetDir()}): ${params.path}`; + + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(params.path)) { + const directories = workspaceContext.getDirectories(); + return `Path must be within one of the workspace directories: ${directories.join(', ')}`; } return null; } diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index e06c353a..f4086a2b 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -12,6 +12,7 @@ import fs from 'fs'; import fsp from 'fs/promises'; import { Config } from '../config/config.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; describe('ReadFileTool', () => { let tempRootDir: string; @@ -27,6 +28,7 @@ describe('ReadFileTool', () => { const mockConfigInstance = { getFileService: () => new FileDiscoveryService(tempRootDir), getTargetDir: () => tempRootDir, + getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir), } as unknown as Config; tool = new ReadFileTool(mockConfigInstance); }); @@ -65,8 +67,9 @@ describe('ReadFileTool', () => { it('should return error for path outside root', () => { const outsidePath = path.resolve(os.tmpdir(), 'outside-root.txt'); const params: ReadFileToolParams = { absolute_path: outsidePath }; - expect(tool.validateToolParams(params)).toMatch( - /File path must be within the root directory/, + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', ); }); @@ -261,4 +264,36 @@ describe('ReadFileTool', () => { }); }); }); + + describe('workspace boundary validation', () => { + it('should validate paths are within workspace root', () => { + const params: ReadFileToolParams = { + absolute_path: path.join(tempRootDir, 'file.txt'), + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); + + it('should reject paths outside workspace root', () => { + const params: ReadFileToolParams = { + absolute_path: '/etc/passwd', + }; + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', + ); + expect(error).toContain(tempRootDir); + }); + + it('should provide clear error message with workspace directories', () => { + const outsidePath = path.join(os.tmpdir(), 'outside-workspace.txt'); + const params: ReadFileToolParams = { + absolute_path: outsidePath, + }; + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', + ); + expect(error).toContain(tempRootDir); + }); + }); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 9ba80672..31282c20 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -10,7 +10,6 @@ import { makeRelative, shortenPath } from '../utils/paths.js'; import { BaseTool, Icon, ToolLocation, ToolResult } from './tools.js'; import { Type } from '@google/genai'; import { - isWithinRoot, processSingleFileContent, getSpecificMimeType, } from '../utils/fileUtils.js'; @@ -86,8 +85,11 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { if (!path.isAbsolute(filePath)) { return `File path must be absolute, but was relative: ${filePath}. You must provide an absolute path.`; } - if (!isWithinRoot(filePath, this.config.getTargetDir())) { - return `File path must be within the root directory (${this.config.getTargetDir()}): ${filePath}`; + + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(filePath)) { + const directories = workspaceContext.getDirectories(); + return `File path must be within one of the workspace directories: ${directories.join(', ')}`; } if (params.offset !== undefined && params.offset < 0) { return 'Offset must be a non-negative number'; @@ -145,7 +147,7 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { if (result.error) { return { llmContent: result.error, // The detailed error for LLM - returnDisplay: result.returnDisplay, // User-friendly error + returnDisplay: result.returnDisplay || 'Error reading file', // User-friendly error }; } @@ -163,8 +165,8 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { ); return { - llmContent: result.llmContent, - returnDisplay: result.returnDisplay, + llmContent: result.llmContent || '', + returnDisplay: result.returnDisplay || '', }; } } diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index 641aa705..68bb9b0e 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -13,6 +13,7 @@ import path from 'path'; import fs from 'fs'; // Actual fs for setup import os from 'os'; import { Config } from '../config/config.js'; +import { WorkspaceContext } from '../utils/workspaceContext.js'; vi.mock('mime-types', () => { const lookup = (filename: string) => { @@ -48,11 +49,11 @@ describe('ReadManyFilesTool', () => { let mockReadFileFn: Mock; beforeEach(async () => { - tempRootDir = fs.mkdtempSync( - path.join(os.tmpdir(), 'read-many-files-root-'), + tempRootDir = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'read-many-files-root-')), ); - tempDirOutsideRoot = fs.mkdtempSync( - path.join(os.tmpdir(), 'read-many-files-external-'), + tempDirOutsideRoot = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'read-many-files-external-')), ); fs.writeFileSync(path.join(tempRootDir, '.geminiignore'), 'foo.*'); const fileService = new FileDiscoveryService(tempRootDir); @@ -64,6 +65,8 @@ describe('ReadManyFilesTool', () => { respectGeminiIgnore: true, }), getTargetDir: () => tempRootDir, + getWorkspaceDirs: () => [tempRootDir], + getWorkspaceContext: () => new WorkspaceContext(tempRootDir), } as Partial<Config> as Config; tool = new ReadManyFilesTool(mockConfig); @@ -424,5 +427,54 @@ describe('ReadManyFilesTool', () => { expect(result.returnDisplay).not.toContain('foo.quux'); expect(result.returnDisplay).toContain('bar.ts'); }); + + it('should read files from multiple workspace directories', async () => { + const tempDir1 = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'multi-dir-1-')), + ); + const tempDir2 = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'multi-dir-2-')), + ); + const fileService = new FileDiscoveryService(tempDir1); + const mockConfig = { + getFileService: () => fileService, + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectGeminiIgnore: true, + }), + getWorkspaceContext: () => new WorkspaceContext(tempDir1, [tempDir2]), + getTargetDir: () => tempDir1, + } as Partial<Config> as Config; + tool = new ReadManyFilesTool(mockConfig); + + fs.writeFileSync(path.join(tempDir1, 'file1.txt'), 'Content1'); + fs.writeFileSync(path.join(tempDir2, 'file2.txt'), 'Content2'); + + const params = { paths: ['*.txt'] }; + const result = await tool.execute(params, new AbortController().signal); + const content = result.llmContent as string[]; + if (!Array.isArray(content)) { + throw new Error(`llmContent is not an array: ${content}`); + } + const expectedPath1 = path.join(tempDir1, 'file1.txt'); + const expectedPath2 = path.join(tempDir2, 'file2.txt'); + + expect( + content.some((c) => + c.includes(`--- ${expectedPath1} ---\n\nContent1\n\n`), + ), + ).toBe(true); + expect( + content.some((c) => + c.includes(`--- ${expectedPath2} ---\n\nContent2\n\n`), + ), + ).toBe(true); + expect(result.returnDisplay).toContain( + 'Successfully read and concatenated content from **2 file(s)**', + ); + + fs.rmSync(tempDir1, { recursive: true, force: true }); + fs.rmSync(tempDir2, { recursive: true, force: true }); + }); }); }); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index fb160a79..771577ec 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -302,18 +302,27 @@ Use this tool when the user's query implies needing the content of several files } try { - const entries = await glob( - searchPatterns.map((p) => p.replace(/\\/g, '/')), - { - cwd: this.config.getTargetDir(), - ignore: effectiveExcludes, - nodir: true, - dot: true, - absolute: true, - nocase: true, - signal, - }, - ); + const allEntries = new Set<string>(); + const workspaceDirs = this.config.getWorkspaceContext().getDirectories(); + + for (const dir of workspaceDirs) { + const entriesInDir = await glob( + searchPatterns.map((p) => p.replace(/\\/g, '/')), + { + cwd: dir, + ignore: effectiveExcludes, + nodir: true, + dot: true, + absolute: true, + nocase: true, + signal, + }, + ); + for (const entry of entriesInDir) { + allEntries.add(entry); + } + } + const entries = Array.from(allEntries); const gitFilteredEntries = fileFilteringOptions.respectGitIgnore ? fileDiscovery @@ -346,11 +355,15 @@ Use this tool when the user's query implies needing the content of several files let geminiIgnoredCount = 0; for (const absoluteFilePath of entries) { - // Security check: ensure the glob library didn't return something outside targetDir. - if (!absoluteFilePath.startsWith(this.config.getTargetDir())) { + // Security check: ensure the glob library didn't return something outside the workspace. + if ( + !this.config + .getWorkspaceContext() + .isPathWithinWorkspace(absoluteFilePath) + ) { skippedFiles.push({ path: absoluteFilePath, - reason: `Security: Glob library returned path outside target directory. Base: ${this.config.getTargetDir()}, Path: ${absoluteFilePath}`, + reason: `Security: Glob library returned path outside workspace. Path: ${absoluteFilePath}`, }); continue; } diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 55364197..7f237e3d 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -37,6 +37,7 @@ import * as crypto from 'crypto'; import * as summarizer from '../utils/summarizer.js'; import { ToolConfirmationOutcome } from './tools.js'; import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; describe('ShellTool', () => { let shellTool: ShellTool; @@ -53,6 +54,7 @@ describe('ShellTool', () => { getDebugMode: vi.fn().mockReturnValue(false), getTargetDir: vi.fn().mockReturnValue('/test/dir'), getSummarizeToolOutputConfig: vi.fn().mockReturnValue(undefined), + getWorkspaceContext: () => createMockWorkspaceContext('.'), getGeminiClient: vi.fn(), } as unknown as Config; @@ -105,7 +107,7 @@ describe('ShellTool', () => { vi.mocked(fs.existsSync).mockReturnValue(false); expect( shellTool.validateToolParams({ command: 'ls', directory: 'rel/path' }), - ).toBe('Directory must exist.'); + ).toBe("Directory 'rel/path' is not a registered workspace directory."); }); }); @@ -385,3 +387,37 @@ describe('ShellTool', () => { }); }); }); + +describe('validateToolParams', () => { + it('should return null for valid directory', () => { + const config = { + getCoreTools: () => undefined, + getExcludeTools: () => undefined, + getTargetDir: () => '/root', + getWorkspaceContext: () => + createMockWorkspaceContext('/root', ['/users/test']), + } as unknown as Config; + const shellTool = new ShellTool(config); + const result = shellTool.validateToolParams({ + command: 'ls', + directory: 'test', + }); + expect(result).toBeNull(); + }); + + it('should return error for directory outside workspace', () => { + const config = { + getCoreTools: () => undefined, + getExcludeTools: () => undefined, + getTargetDir: () => '/root', + getWorkspaceContext: () => + createMockWorkspaceContext('/root', ['/users/test']), + } as unknown as Config; + const shellTool = new ShellTool(config); + const result = shellTool.validateToolParams({ + command: 'ls', + directory: 'test2', + }); + expect(result).toContain('is not a registered workspace directory'); + }); +}); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 02fcbb7f..96423af1 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -124,14 +124,19 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> { } if (params.directory) { if (path.isAbsolute(params.directory)) { - return 'Directory cannot be absolute. Must be relative to the project root directory.'; + return 'Directory cannot be absolute. Please refer to workspace directories by their name.'; } - const directory = path.resolve( - this.config.getTargetDir(), - params.directory, + const workspaceDirs = this.config.getWorkspaceContext().getDirectories(); + const matchingDirs = workspaceDirs.filter( + (dir) => path.basename(dir) === params.directory, ); - if (!fs.existsSync(directory)) { - return 'Directory must exist.'; + + if (matchingDirs.length === 0) { + return `Directory '${params.directory}' is not a registered workspace directory.`; + } + + if (matchingDirs.length > 1) { + return `Directory name '${params.directory}' is ambiguous as it matches multiple workspace directories.`; } } return null; diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index b3fdd7a3..27a7c28b 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -30,6 +30,9 @@ import { Schema, } from '@google/genai'; import { spawn } from 'node:child_process'; +import fs from 'node:fs'; + +vi.mock('node:fs'); // Use vi.hoisted to define the mock function so it can be used in the vi.mock factory const mockDiscoverMcpTools = vi.hoisted(() => vi.fn()); @@ -144,6 +147,10 @@ describe('ToolRegistry', () => { let mockConfigGetToolDiscoveryCommand: ReturnType<typeof vi.spyOn>; beforeEach(() => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => true, + } as fs.Stats); config = new Config(baseConfigParams); toolRegistry = new ToolRegistry(config); vi.spyOn(console, 'warn').mockImplementation(() => {}); diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index c33b5fa2..965e9476 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -31,6 +31,7 @@ import { ensureCorrectFileContent, CorrectedEditResult, } from '../utils/editCorrector.js'; +import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; const rootDir = path.resolve(os.tmpdir(), 'gemini-cli-test-root'); @@ -54,6 +55,7 @@ const mockConfigInternal = { getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT), setApprovalMode: vi.fn(), getGeminiClient: vi.fn(), // Initialize as a plain mock function + getWorkspaceContext: () => createMockWorkspaceContext(rootDir), getApiKey: () => 'test-key', getModel: () => 'test-model', getSandbox: () => false, @@ -83,6 +85,7 @@ describe('WriteFileTool', () => { let tempDir: string; beforeEach(() => { + vi.clearAllMocks(); // Create a unique temporary directory for files created outside the root tempDir = fs.mkdtempSync( path.join(os.tmpdir(), 'write-file-test-external-'), @@ -98,6 +101,11 @@ describe('WriteFileTool', () => { ) as Mocked<GeminiClient>; vi.mocked(GeminiClient).mockImplementation(() => mockGeminiClientInstance); + vi.mocked(ensureCorrectEdit).mockImplementation(mockEnsureCorrectEdit); + vi.mocked(ensureCorrectFileContent).mockImplementation( + mockEnsureCorrectFileContent, + ); + // Now that mockGeminiClientInstance is initialized, set the mock implementation for getGeminiClient mockConfigInternal.getGeminiClient.mockReturnValue( mockGeminiClientInstance, @@ -177,8 +185,9 @@ describe('WriteFileTool', () => { file_path: outsidePath, content: 'hello', }; - expect(tool.validateToolParams(params)).toMatch( - /File path must be within the root directory/, + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', ); }); @@ -427,8 +436,8 @@ describe('WriteFileTool', () => { const params = { file_path: outsidePath, content: 'test' }; const result = await tool.execute(params, abortSignal); expect(result.llmContent).toMatch(/Error: Invalid parameters provided/); - expect(result.returnDisplay).toMatch( - /Error: File path must be within the root directory/, + expect(result.returnDisplay).toContain( + 'Error: File path must be within one of the workspace directories', ); }); @@ -616,4 +625,39 @@ describe('WriteFileTool', () => { expect(result.llmContent).not.toMatch(/User modified the `content`/); }); }); + + describe('workspace boundary validation', () => { + it('should validate paths are within workspace root', () => { + const params = { + file_path: path.join(rootDir, 'file.txt'), + content: 'test content', + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); + + it('should reject paths outside workspace root', () => { + const params = { + file_path: '/etc/passwd', + content: 'malicious', + }; + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', + ); + expect(error).toContain(rootDir); + }); + + it('should provide clear error message with workspace directories', () => { + const outsidePath = path.join(tempDir, 'outside-root.txt'); + const params = { + file_path: outsidePath, + content: 'test', + }; + const error = tool.validateToolParams(params); + expect(error).toContain( + 'File path must be within one of the workspace directories', + ); + expect(error).toContain(rootDir); + }); + }); }); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index ae37ca8a..470adf31 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -27,7 +27,7 @@ import { } from '../utils/editCorrector.js'; import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js'; import { ModifiableTool, ModifyContext } from './modifiable-tool.js'; -import { getSpecificMimeType, isWithinRoot } from '../utils/fileUtils.js'; +import { getSpecificMimeType } from '../utils/fileUtils.js'; import { recordFileOperationMetric, FileOperation, @@ -105,8 +105,11 @@ export class WriteFileTool if (!path.isAbsolute(filePath)) { return `File path must be absolute: ${filePath}`; } - if (!isWithinRoot(filePath, this.config.getTargetDir())) { - return `File path must be within the root directory (${this.config.getTargetDir()}): ${filePath}`; + + const workspaceContext = this.config.getWorkspaceContext(); + if (!workspaceContext.isPathWithinWorkspace(filePath)) { + const directories = workspaceContext.getDirectories(); + return `File path must be within one of the workspace directories: ${directories.join(', ')}`; } try { diff --git a/packages/core/src/utils/flashFallback.integration.test.ts b/packages/core/src/utils/flashFallback.integration.test.ts index f5e354a0..9211ad2f 100644 --- a/packages/core/src/utils/flashFallback.integration.test.ts +++ b/packages/core/src/utils/flashFallback.integration.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { Config } from '../config/config.js'; +import fs from 'node:fs'; import { setSimulate429, disableSimulationAfterFallback, @@ -17,10 +18,16 @@ import { DEFAULT_GEMINI_FLASH_MODEL } from '../config/models.js'; import { retryWithBackoff } from './retry.js'; import { AuthType } from '../core/contentGenerator.js'; +vi.mock('node:fs'); + describe('Flash Fallback Integration', () => { let config: Config; beforeEach(() => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.statSync).mockReturnValue({ + isDirectory: () => true, + } as fs.Stats); config = new Config({ sessionId: 'test-session', targetDir: '/test', diff --git a/packages/core/src/utils/workspaceContext.test.ts b/packages/core/src/utils/workspaceContext.test.ts new file mode 100644 index 00000000..67d06b62 --- /dev/null +++ b/packages/core/src/utils/workspaceContext.test.ts @@ -0,0 +1,283 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { WorkspaceContext } from './workspaceContext.js'; + +vi.mock('fs'); + +describe('WorkspaceContext', () => { + let workspaceContext: WorkspaceContext; + // Use path module to create platform-agnostic paths + const mockCwd = path.resolve(path.sep, 'home', 'user', 'project'); + const mockExistingDir = path.resolve( + path.sep, + 'home', + 'user', + 'other-project', + ); + const mockNonExistentDir = path.resolve( + path.sep, + 'home', + 'user', + 'does-not-exist', + ); + const mockSymlinkDir = path.resolve(path.sep, 'home', 'user', 'symlink'); + const mockRealPath = path.resolve(path.sep, 'home', 'user', 'real-directory'); + + beforeEach(() => { + vi.resetAllMocks(); + + // Mock fs.existsSync + vi.mocked(fs.existsSync).mockImplementation((path) => { + const pathStr = path.toString(); + return ( + pathStr === mockCwd || + pathStr === mockExistingDir || + pathStr === mockSymlinkDir || + pathStr === mockRealPath + ); + }); + + // Mock fs.statSync + vi.mocked(fs.statSync).mockImplementation((path) => { + const pathStr = path.toString(); + if (pathStr === mockNonExistentDir) { + throw new Error('ENOENT'); + } + return { + isDirectory: () => true, + } as fs.Stats; + }); + + // Mock fs.realpathSync + vi.mocked(fs.realpathSync).mockImplementation((path) => { + const pathStr = path.toString(); + if (pathStr === mockSymlinkDir) { + return mockRealPath; + } + return pathStr; + }); + }); + + describe('initialization', () => { + it('should initialize with a single directory (cwd)', () => { + workspaceContext = new WorkspaceContext(mockCwd); + const directories = workspaceContext.getDirectories(); + expect(directories).toHaveLength(1); + expect(directories[0]).toBe(mockCwd); + }); + + it('should validate and resolve directories to absolute paths', () => { + const absolutePath = path.join(mockCwd, 'subdir'); + vi.mocked(fs.existsSync).mockImplementation( + (p) => p === mockCwd || p === absolutePath, + ); + vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString()); + + workspaceContext = new WorkspaceContext(mockCwd, [absolutePath]); + const directories = workspaceContext.getDirectories(); + expect(directories).toContain(absolutePath); + }); + + it('should reject non-existent directories', () => { + expect(() => { + new WorkspaceContext(mockCwd, [mockNonExistentDir]); + }).toThrow('Directory does not exist'); + }); + + it('should handle empty initialization', () => { + workspaceContext = new WorkspaceContext(mockCwd, []); + const directories = workspaceContext.getDirectories(); + expect(directories).toHaveLength(1); + expect(directories[0]).toBe(mockCwd); + }); + }); + + describe('adding directories', () => { + beforeEach(() => { + workspaceContext = new WorkspaceContext(mockCwd); + }); + + it('should add valid directories', () => { + workspaceContext.addDirectory(mockExistingDir); + const directories = workspaceContext.getDirectories(); + expect(directories).toHaveLength(2); + expect(directories).toContain(mockExistingDir); + }); + + it('should resolve relative paths to absolute', () => { + // Since we can't mock path.resolve, we'll test with absolute paths + workspaceContext.addDirectory(mockExistingDir); + const directories = workspaceContext.getDirectories(); + expect(directories).toContain(mockExistingDir); + }); + + it('should reject non-existent directories', () => { + expect(() => { + workspaceContext.addDirectory(mockNonExistentDir); + }).toThrow('Directory does not exist'); + }); + + it('should prevent duplicate directories', () => { + workspaceContext.addDirectory(mockExistingDir); + workspaceContext.addDirectory(mockExistingDir); + const directories = workspaceContext.getDirectories(); + expect(directories.filter((d) => d === mockExistingDir)).toHaveLength(1); + }); + + it('should handle symbolic links correctly', () => { + workspaceContext.addDirectory(mockSymlinkDir); + const directories = workspaceContext.getDirectories(); + expect(directories).toContain(mockRealPath); + expect(directories).not.toContain(mockSymlinkDir); + }); + }); + + describe('path validation', () => { + beforeEach(() => { + workspaceContext = new WorkspaceContext(mockCwd, [mockExistingDir]); + }); + + it('should accept paths within workspace directories', () => { + const validPath1 = path.join(mockCwd, 'src', 'file.ts'); + const validPath2 = path.join(mockExistingDir, 'lib', 'module.js'); + + expect(workspaceContext.isPathWithinWorkspace(validPath1)).toBe(true); + expect(workspaceContext.isPathWithinWorkspace(validPath2)).toBe(true); + }); + + it('should reject paths outside workspace', () => { + const invalidPath = path.resolve( + path.dirname(mockCwd), + 'outside-workspace', + 'file.txt', + ); + expect(workspaceContext.isPathWithinWorkspace(invalidPath)).toBe(false); + }); + + it('should resolve symbolic links before validation', () => { + const symlinkPath = path.join(mockCwd, 'symlink-file'); + const realPath = path.join(mockCwd, 'real-file'); + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p === symlinkPath) { + return realPath; + } + return p.toString(); + }); + + expect(workspaceContext.isPathWithinWorkspace(symlinkPath)).toBe(true); + }); + + it('should handle nested directories correctly', () => { + const nestedPath = path.join( + mockCwd, + 'deeply', + 'nested', + 'path', + 'file.txt', + ); + expect(workspaceContext.isPathWithinWorkspace(nestedPath)).toBe(true); + }); + + it('should handle edge cases (root, parent references)', () => { + const rootPath = '/'; + const parentPath = path.dirname(mockCwd); + + expect(workspaceContext.isPathWithinWorkspace(rootPath)).toBe(false); + expect(workspaceContext.isPathWithinWorkspace(parentPath)).toBe(false); + }); + + it('should handle non-existent paths correctly', () => { + const nonExistentPath = path.join(mockCwd, 'does-not-exist.txt'); + vi.mocked(fs.existsSync).mockImplementation((p) => p !== nonExistentPath); + + // Should still validate based on path structure + expect(workspaceContext.isPathWithinWorkspace(nonExistentPath)).toBe( + true, + ); + }); + }); + + describe('getDirectories', () => { + it('should return a copy of directories array', () => { + workspaceContext = new WorkspaceContext(mockCwd); + const dirs1 = workspaceContext.getDirectories(); + const dirs2 = workspaceContext.getDirectories(); + + expect(dirs1).not.toBe(dirs2); // Different array instances + expect(dirs1).toEqual(dirs2); // Same content + }); + }); + + describe('symbolic link security', () => { + beforeEach(() => { + workspaceContext = new WorkspaceContext(mockCwd); + }); + + it('should follow symlinks but validate resolved path', () => { + const symlinkInsideWorkspace = path.join(mockCwd, 'link-to-subdir'); + const resolvedInsideWorkspace = path.join(mockCwd, 'subdir'); + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p === symlinkInsideWorkspace) { + return resolvedInsideWorkspace; + } + return p.toString(); + }); + + expect( + workspaceContext.isPathWithinWorkspace(symlinkInsideWorkspace), + ).toBe(true); + }); + + it('should prevent sandbox escape via symlinks', () => { + const symlinkEscape = path.join(mockCwd, 'escape-link'); + const resolvedOutside = path.resolve(mockCwd, '..', 'outside-file'); + + vi.mocked(fs.existsSync).mockImplementation((p) => { + const pathStr = p.toString(); + return ( + pathStr === symlinkEscape || + pathStr === resolvedOutside || + pathStr === mockCwd + ); + }); + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p.toString() === symlinkEscape) { + return resolvedOutside; + } + return p.toString(); + }); + vi.mocked(fs.statSync).mockImplementation( + (p) => + ({ + isDirectory: () => p.toString() !== resolvedOutside, + }) as fs.Stats, + ); + + workspaceContext = new WorkspaceContext(mockCwd); + expect(workspaceContext.isPathWithinWorkspace(symlinkEscape)).toBe(false); + }); + + it('should handle circular symlinks', () => { + const circularLink = path.join(mockCwd, 'circular'); + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.realpathSync).mockImplementation(() => { + throw new Error('ELOOP: too many symbolic links encountered'); + }); + + // Should handle the error gracefully + expect(workspaceContext.isPathWithinWorkspace(circularLink)).toBe(false); + }); + }); +}); diff --git a/packages/core/src/utils/workspaceContext.ts b/packages/core/src/utils/workspaceContext.ts new file mode 100644 index 00000000..16d1b4c9 --- /dev/null +++ b/packages/core/src/utils/workspaceContext.ts @@ -0,0 +1,127 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * WorkspaceContext manages multiple workspace directories and validates paths + * against them. This allows the CLI to operate on files from multiple directories + * in a single session. + */ +export class WorkspaceContext { + private directories: Set<string>; + + /** + * Creates a new WorkspaceContext with the given initial directory and optional additional directories. + * @param initialDirectory The initial working directory (usually cwd) + * @param additionalDirectories Optional array of additional directories to include + */ + constructor(initialDirectory: string, additionalDirectories: string[] = []) { + this.directories = new Set<string>(); + + this.addDirectoryInternal(initialDirectory); + + for (const dir of additionalDirectories) { + this.addDirectoryInternal(dir); + } + } + + /** + * Adds a directory to the workspace. + * @param directory The directory path to add (can be relative or absolute) + * @param basePath Optional base path for resolving relative paths (defaults to cwd) + */ + addDirectory(directory: string, basePath: string = process.cwd()): void { + this.addDirectoryInternal(directory, basePath); + } + + /** + * Internal method to add a directory with validation. + */ + private addDirectoryInternal( + directory: string, + basePath: string = process.cwd(), + ): void { + const absolutePath = path.isAbsolute(directory) + ? directory + : path.resolve(basePath, directory); + + if (!fs.existsSync(absolutePath)) { + throw new Error(`Directory does not exist: ${absolutePath}`); + } + + const stats = fs.statSync(absolutePath); + if (!stats.isDirectory()) { + throw new Error(`Path is not a directory: ${absolutePath}`); + } + + let realPath: string; + try { + realPath = fs.realpathSync(absolutePath); + } catch (_error) { + throw new Error(`Failed to resolve path: ${absolutePath}`); + } + + this.directories.add(realPath); + } + + /** + * Gets a copy of all workspace directories. + * @returns Array of absolute directory paths + */ + getDirectories(): readonly string[] { + return Array.from(this.directories); + } + + /** + * Checks if a given path is within any of the workspace directories. + * @param pathToCheck The path to validate + * @returns True if the path is within the workspace, false otherwise + */ + isPathWithinWorkspace(pathToCheck: string): boolean { + try { + const absolutePath = path.resolve(pathToCheck); + + let resolvedPath = absolutePath; + if (fs.existsSync(absolutePath)) { + try { + resolvedPath = fs.realpathSync(absolutePath); + } catch (_error) { + return false; + } + } + + for (const dir of this.directories) { + if (this.isPathWithinRoot(resolvedPath, dir)) { + return true; + } + } + + return false; + } catch (_error) { + return false; + } + } + + /** + * Checks if a path is within a given root directory. + * @param pathToCheck The absolute path to check + * @param rootDirectory The absolute root directory + * @returns True if the path is within the root directory, false otherwise + */ + private isPathWithinRoot( + pathToCheck: string, + rootDirectory: string, + ): boolean { + const relative = path.relative(rootDirectory, pathToCheck); + return ( + !relative.startsWith(`..${path.sep}`) && + relative !== '..' && + !path.isAbsolute(relative) + ); + } +} |