diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index f5e9567b19..ccf6b58fb1 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1340,11 +1340,6 @@ Logging in with Google... Restarting Gemini CLI to continue. } } - const parsedCommand = parseSlashCommand( - submittedValue, - slashCommands ?? [], - ); - const isSlash = isSlashCommand(submittedValue.trim()); const isIdle = streamingState === StreamingState.Idle; const isAgentRunning = @@ -1352,6 +1347,10 @@ Logging in with Google... Restarting Gemini CLI to continue. isToolExecuting(pendingHistoryItems); if (isSlash && isAgentRunning) { + const parsedCommand = parseSlashCommand( + submittedValue, + slashCommands ?? [], + ); const commandToExecute = parsedCommand.commandToExecute; if (commandToExecute?.isSafeConcurrent) { void handleSlashCommand(submittedValue); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx index 5034fa8a7e..c909c6d7f8 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx @@ -1130,18 +1130,19 @@ describe('useSlashCommandProcessor', () => { }); expect(spySetContext).toHaveBeenLastCalledWith('extB'); - // 3. Run /plan (Default) + // 3. Run /help (Concurrent global command) + spySetContext.mockClear(); + await act(async () => { + await hook.current.handleSlashCommand('/help'); + }); + // A concurrent command should NOT modify the active extension context + expect(spySetContext).not.toHaveBeenCalled(); + + // 4. Run /plan (Default) await act(async () => { await hook.current.handleSlashCommand('/plan my task'); }); expect(spySetContext).toHaveBeenLastCalledWith(undefined); - - // 4. Run /clear (Global) - await act(async () => { - await hook.current.handleSlashCommand('/help'); - }); - // Context should still be undefined - expect(spySetContext).toHaveBeenLastCalledWith(undefined); }); }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 830c6fecbc..035c65d7e7 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -371,7 +371,7 @@ export const useSlashCommandProcessor = ( extensionContext, } = parseSlashCommand(trimmed, commands); - if (config) { + if (config && commandToExecute && !commandToExecute.isSafeConcurrent) { if (extensionContext) { if (config.hasExtensionPlanDir(extensionContext)) { config.setActiveExtensionContext(extensionContext); diff --git a/packages/core/.geminiignore b/packages/core/.geminiignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/core/.gitignore b/packages/core/.gitignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 89c41150bf..c7729760b9 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3369,8 +3369,13 @@ describe('Plans Directory Initialization', () => { writeSpy.mockRestore(); }); - it('should throw a security violation if the resolved plan directory is outside the project root', async () => { - vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined); + it('should throw a security violation and verify mkdirSync runs before realpathSync (TOCTOU mitigation)', async () => { + const callOrder: string[] = []; + + vi.spyOn(fs, 'mkdirSync').mockImplementation(() => { + callOrder.push('mkdirSync'); + return undefined; + }); const config = new Config({ ...baseParams, @@ -3386,14 +3391,19 @@ describe('Plans Directory Initialization', () => { .spyOn(fs, 'realpathSync') .mockImplementation((p: fs.PathLike) => { const pStr = p.toString(); - if (pStr.includes('plans')) return '/outside/the/project/root/plans'; + // Ignore the calls from storage/initialization + if (pStr.includes('plans')) { + callOrder.push('realpathSync'); + return '/outside/the/project/root/plans'; + } return pStr; }); try { expect(() => config.getPlansDir()).toThrow( - /Security violation: Resolved plan directory.*is outside the project root/, + /Security violation: Resolved plan directory.*is outside both the project root.*and the global configuration directory/, ); + expect(callOrder).toEqual(['mkdirSync', 'realpathSync']); } finally { realpathSpy.mockRestore(); } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index f7f985323b..9758ea85ee 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -6,6 +6,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; +import { SecurityError } from '../core/errors.js'; import { SandboxPolicyManager } from '../policy/sandboxPolicyManager.js'; import { inspect } from 'node:util'; import process from 'node:process'; @@ -451,7 +452,6 @@ import { A2AClientManager } from '../agents/a2a-client-manager.js'; import { type McpContext } from '../tools/mcp-client.js'; import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; import { getErrorMessage } from '../utils/errors.js'; -import { LRUCache } from 'mnemonist'; export type { FileFilteringOptions }; export { @@ -775,9 +775,15 @@ export class Config implements McpContext, AgentLoopContext { private readonly extensionsEnabled: boolean; private mcpServers: Record | undefined; private readonly mcpEnablementCallbacks?: McpEnablementCallbacks; + /** + * The persistent context used to route tools (e.g. plans, tasks) to the correct + * extension directory. + * This is a persistent session state (similar to `approvalMode`). It remains active + * across multiple LLM turns until explicitly changed by another command. + */ private activeExtensionContext?: string; - private initializedPlanDirs = new LRUCache(20); - private plansDirCache = new LRUCache(20); + private initializedPlanDirs = new Set(); + private plansDirCache = new Map(); private readonly extensionPlanDirs: Record; private userMemory: string | HierarchicalMemory; private geminiMdFileCount: number; @@ -2244,8 +2250,9 @@ export class Config implements McpContext, AgentLoopContext { } getActiveExtensionPlanDir(): string | undefined { - if (this.activeExtensionContext) { - return this.extensionPlanDirs[this.activeExtensionContext]; + const context = this.getActiveExtensionContext(); + if (context) { + return this.extensionPlanDirs[context]; } return undefined; } @@ -2264,23 +2271,31 @@ export class Config implements McpContext, AgentLoopContext { } try { - const realPlansDir = resolveToRealPath(plansDir); - const realProjectRoot = resolveToRealPath(this.getTargetDir()); + fs.mkdirSync(plansDir, { recursive: true }); - if (!isSubpath(realProjectRoot, realPlansDir)) { - throw new Error( - `Security violation: Resolved plan directory '${realPlansDir}' is outside the project root '${realProjectRoot}'.`, + const realPlansDir = resolveToRealPath(plansDir); + const realProjectRoot = this.storage.getRealProjectRoot(); + const realGlobalGeminiDir = resolveToRealPath( + Storage.getGlobalGeminiDir(), + ); + + if ( + !isSubpath(realProjectRoot, realPlansDir) && + !isSubpath(realGlobalGeminiDir, realPlansDir) + ) { + throw new SecurityError( + `Security violation: Resolved plan directory '${realPlansDir}' is outside both the project root '${realProjectRoot}' and the global configuration directory.`, ); } - this.initializedPlanDirs.set(plansDir, true); - fs.mkdirSync(realPlansDir, { recursive: true }); + this.initializedPlanDirs.add(plansDir); this.workspaceContext.addDirectory(realPlansDir); } catch (e: unknown) { - const errorMessage = e instanceof Error ? e.message : String(e); - if (errorMessage.includes('Security violation')) { + if (e instanceof SecurityError) { throw e; } + this.initializedPlanDirs.add(plansDir); // Don't try again and spam stderr + const errorMessage = e instanceof Error ? e.message : String(e); process.stderr.write( `Failed to initialize active plan directory at '${plansDir}': ${errorMessage}\n`, ); diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 0cf2c91969..f26f1fcdfc 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -24,7 +24,7 @@ vi.mock('fs', async (importOriginal) => { }); import { Storage } from './storage.js'; -import { GEMINI_DIR, homedir, resolveToRealPath } from '../utils/paths.js'; +import { GEMINI_DIR, homedir } from '../utils/paths.js'; import { ProjectRegistry } from './projectRegistry.js'; import { StorageMigration } from './storageMigration.js'; @@ -104,7 +104,7 @@ describe('Storage - Security', () => { describe('Storage – additional helpers', () => { const projectRoot = resolveToRealPath(path.resolve('/tmp/project')); - const storage = new Storage(projectRoot); + let storage = new Storage(projectRoot); beforeEach(() => { ProjectRegistry.prototype.getShortId = vi @@ -306,12 +306,6 @@ describe('Storage – additional helpers', () => { customDir: '.my-plans', expected: path.resolve(projectRoot, '.my-plans'), }, - { - name: 'custom absolute path outside throws', - customDir: path.resolve('/absolute/path/to/plans'), - expected: '', - expectedError: `Custom plans directory '${path.resolve('/absolute/path/to/plans')}' resolves to '${path.resolve('/absolute/path/to/plans')}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`, - }, { name: 'absolute path that happens to be inside project root', customDir: path.join(projectRoot, 'internal-plans'), @@ -332,32 +326,11 @@ describe('Storage – additional helpers', () => { customDir: undefined, expected: () => storage.getProjectTempPlansDir(), }, - { - name: 'escaping relative path throws', - customDir: '../escaped-plans', - expected: '', - expectedError: `Custom plans directory '../escaped-plans' resolves to '${resolveToRealPath(path.resolve(projectRoot, '../escaped-plans'))}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`, - }, { name: 'hidden directory starting with ..', customDir: '..plans', expected: path.resolve(projectRoot, '..plans'), }, - { - name: 'security escape via symbolic link throws', - customDir: 'symlink-to-outside', - setup: () => { - vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => { - if (p.toString().includes('symlink-to-outside')) { - return path.resolve('/outside/project/root'); - } - return p.toString(); - }); - return () => vi.mocked(fs.realpathSync).mockRestore(); - }, - expected: '', - expectedError: `Custom plans directory 'symlink-to-outside' resolves to '${path.resolve('/outside/project/root')}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`, - }, { name: 'non-existent plan dir in a symlinked project root', customDir: 'new-plans', @@ -378,33 +351,14 @@ describe('Storage – additional helpers', () => { }, expected: path.resolve(projectRoot, 'new-plans'), }, - { - name: 'security escape via symbolic link with non-existent dir throws', - customDir: 'link-to-outside/new-dir', - setup: () => { - vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => { - const pStr = p.toString(); - if (pStr.includes('link-to-outside/new-dir')) { - const err = new Error('ENOENT') as NodeJS.ErrnoException; - err.code = 'ENOENT'; - throw err; - } - if (pStr.includes('link-to-outside')) { - return '/outside/project/root'; - } - return pStr; - }); - return () => vi.mocked(fs.realpathSync).mockRestore(); - }, - expected: '', - expectedError: - "Custom plans directory 'link-to-outside/new-dir' resolves to '/outside/project/root/new-dir', which is outside the project root '/tmp/project'.", - }, ]; testCases.forEach(({ name, customDir, expected, expectedError, setup }) => { it(`should handle ${name}`, async () => { const cleanup = setup?.(); + if (setup) { + storage = new Storage(projectRoot, 'test-session'); + } try { if (name.includes('default behavior')) { await storage.initialize(); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 9c45b6205b..041ecad579 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -12,13 +12,11 @@ import { GEMINI_DIR, homedir, GOOGLE_ACCOUNTS_FILENAME, - isSubpath, resolveToRealPath, normalizePath, } from '../utils/paths.js'; import { ProjectRegistry } from './projectRegistry.js'; import { StorageMigration } from './storageMigration.js'; - export const OAUTH_FILE = 'oauth_creds.json'; const TMP_DIR_NAME = 'tmp'; const BIN_DIR_NAME = 'bin'; @@ -40,6 +38,10 @@ export class Storage { this.realProjectRoot = resolveToRealPath(targetDir); } + getRealProjectRoot(): string { + return this.realProjectRoot; + } + setCustomPlansDir(dir: string | undefined): void { this.customPlansDir = dir; } @@ -325,16 +327,7 @@ export class Storage { getPlansDir(extensionPlanDir?: string): string { const customDir = extensionPlanDir || this.customPlansDir; if (customDir) { - const resolvedPath = path.resolve(this.getProjectRoot(), customDir); - const realResolvedPath = resolveToRealPath(resolvedPath); - - if (!isSubpath(this.realProjectRoot, realResolvedPath)) { - throw new Error( - `Custom plans directory '${customDir}' resolves to '${realResolvedPath}', which is outside the project root '${this.realProjectRoot}'.`, - ); - } - - return resolvedPath; + return path.resolve(this.getProjectRoot(), customDir); } return this.getProjectTempPlansDir(); } diff --git a/packages/core/src/core/errors.ts b/packages/core/src/core/errors.ts new file mode 100644 index 0000000000..ef84be418a --- /dev/null +++ b/packages/core/src/core/errors.ts @@ -0,0 +1,11 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ +export class SecurityError extends Error { + constructor(message: string) { + super(message); + this.name = 'SecurityError'; + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 130ca9c2a5..caebb9cf63 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -276,7 +276,8 @@ export * from './voice/responseFormatter.js'; // Export types from @google/genai export type { Content, Part, FunctionCall } from '@google/genai'; - // Export context types and profiles export * from './context/types.js'; export * from './context/profiles.js'; + +export * from './core/errors.js';