diff --git a/packages/cli/src/config/auth.test.ts b/packages/cli/src/config/auth.test.ts index e2ce6841b1..fb951f8aa4 100644 --- a/packages/cli/src/config/auth.test.ts +++ b/packages/cli/src/config/auth.test.ts @@ -39,6 +39,7 @@ describe('validateAuthMethod', () => { }); it('should return an error message if GEMINI_API_KEY is not set', () => { + vi.stubEnv('GEMINI_API_KEY', ''); expect(validateAuthMethod(AuthType.USE_GEMINI)).toBe( 'GEMINI_API_KEY environment variable not found. Add that to your environment and try again (no reload needed if using .env)!', ); diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 9a5962996a..7403277f35 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -2004,28 +2004,16 @@ describe('loadCliConfig trustedFolder', () => { description: 'feature disabled, folderTrust false, workspace trusted -> behave as trusted', }, - - // Cases where folderTrustFeature is true but folderTrust setting is false { - folderTrustFeature: true, - folderTrust: false, - isWorkspaceTrusted: true, - expectedFolderTrust: false, - expectedIsTrustedFolder: true, - description: - 'feature on, folderTrust false, workspace trusted -> behave as trusted', - }, - { - folderTrustFeature: true, + folderTrustFeature: false, folderTrust: false, isWorkspaceTrusted: false, expectedFolderTrust: false, expectedIsTrustedFolder: true, description: - 'feature on, folderTrust false, workspace not trusted -> behave as trusted', + 'feature disabled, folderTrust false, workspace not trusted -> behave as trusted', }, - - // Cases where feature is fully enabled (folderTrustFeature and folderTrust are true) + // Cases where folderTrustFeature is true (feature enabled) { folderTrustFeature: true, folderTrust: true, @@ -2033,7 +2021,7 @@ describe('loadCliConfig trustedFolder', () => { expectedFolderTrust: true, expectedIsTrustedFolder: true, description: - 'feature on, folderTrust on, workspace trusted -> is trusted', + 'feature enabled, folderTrust true, workspace trusted -> behave as trusted', }, { folderTrustFeature: true, @@ -2042,28 +2030,46 @@ describe('loadCliConfig trustedFolder', () => { expectedFolderTrust: true, expectedIsTrustedFolder: false, description: - 'feature on, folderTrust on, workspace NOT trusted -> is NOT trusted', + 'feature enabled, folderTrust true, workspace not trusted -> behave as not trusted', }, { folderTrustFeature: true, folderTrust: true, isWorkspaceTrusted: undefined, expectedFolderTrust: true, - expectedIsTrustedFolder: undefined, + expectedIsTrustedFolder: true, description: - 'feature on, folderTrust on, workspace trust unknown -> is unknown', + 'feature enabled, folderTrust false, workspace trust unknown -> behave as trusted', + }, + { + folderTrustFeature: true, + folderTrust: false, + isWorkspaceTrusted: true, + expectedFolderTrust: false, + expectedIsTrustedFolder: true, + description: + 'feature enabled, folderTrust false, workspace trusted -> behave as trusted', + }, + { + folderTrustFeature: true, + folderTrust: false, + isWorkspaceTrusted: false, + expectedFolderTrust: false, + expectedIsTrustedFolder: true, + description: + 'feature enabled, folderTrust false, workspace not trusted -> behave as trusted', }, ]; for (const { folderTrustFeature, folderTrust, - isWorkspaceTrusted: mockTrustValue, + isWorkspaceTrusted: isWorkspaceTrustedValue, expectedFolderTrust, expectedIsTrustedFolder, description, } of testCases) { - it(`should be correct for: ${description}`, async () => { + it(`should correctly set folderTrust and isTrustedFolder when ${description}`, async () => { (isWorkspaceTrusted as Mock).mockImplementation((settings: Settings) => { const folderTrustFeature = settings.security?.folderTrust?.featureEnabled ?? false; @@ -2074,7 +2080,7 @@ describe('loadCliConfig trustedFolder', () => { if (!folderTrustEnabled) { return true; } - return mockTrustValue; // This is the part that comes from the test case + return isWorkspaceTrustedValue; // This is the part that comes from the test case }); const argv = await parseArguments({} as Settings); const settings: Settings = { diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 5fbf632aab..42323fa129 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -107,6 +107,7 @@ import { appEvents, AppEvent } from '../utils/events.js'; import { isNarrowWidth } from './utils/isNarrowWidth.js'; import { useWorkspaceMigration } from './hooks/useWorkspaceMigration.js'; import { WorkspaceMigrationDialog } from './components/WorkspaceMigrationDialog.js'; +import { isWorkspaceTrusted } from '../config/trustedFolders.js'; const CTRL_EXIT_PROMPT_DURATION_MS = 1000; // Maximum number of queued messages to display in UI to prevent performance issues @@ -204,7 +205,7 @@ const App = ({ config, settings, startupWarnings = [], version }: AppProps) => { const [footerHeight, setFooterHeight] = useState(0); const [corgiMode, setCorgiMode] = useState(false); const [isTrustedFolderState, setIsTrustedFolder] = useState( - config.isTrustedFolder(), + isWorkspaceTrusted(settings.merged), ); const [currentModel, setCurrentModel] = useState(config.getModel()); const [shellModeActive, setShellModeActive] = useState(false); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 32fac4c659..967a301feb 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -143,24 +143,6 @@ describe('ToolConfirmationMessage', () => { expect(lastFrame()).toContain(alwaysAllowText); }); - it('should show "allow always" when folder trust is undefined', () => { - const mockConfig = { - isTrustedFolder: () => undefined, - getIdeMode: () => false, - } as unknown as Config; - - const { lastFrame } = renderWithProviders( - , - ); - - expect(lastFrame()).toContain(alwaysAllowText); - }); - it('should NOT show "allow always" when folder is untrusted', () => { const mockConfig = { isTrustedFolder: () => false, diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index acddae111b..e3c48d9415 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -56,7 +56,7 @@ export const ToolConfirmationMessage: React.FC< onConfirm(outcome); }; - const isTrustedFolder = config.isTrustedFolder() !== false; + const isTrustedFolder = config.isTrustedFolder(); useKeypress( (key) => { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 0b1c20234f..aa86599976 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -675,69 +675,47 @@ describe('Server Config (config.ts)', () => { }); describe('setApprovalMode with folder trust', () => { + const baseParams: ConfigParameters = { + sessionId: 'test', + targetDir: '.', + debugMode: false, + model: 'test-model', + cwd: '.', + }; + it('should throw an error when setting YOLO mode in an untrusted folder', () => { - const config = new Config({ - sessionId: 'test', - targetDir: '.', - debugMode: false, - model: 'test-model', - cwd: '.', - trustedFolder: false, // Untrusted - }); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(false); expect(() => config.setApprovalMode(ApprovalMode.YOLO)).toThrow( 'Cannot enable privileged approval modes in an untrusted folder.', ); }); it('should throw an error when setting AUTO_EDIT mode in an untrusted folder', () => { - const config = new Config({ - sessionId: 'test', - targetDir: '.', - debugMode: false, - model: 'test-model', - cwd: '.', - trustedFolder: false, // Untrusted - }); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(false); expect(() => config.setApprovalMode(ApprovalMode.AUTO_EDIT)).toThrow( 'Cannot enable privileged approval modes in an untrusted folder.', ); }); it('should NOT throw an error when setting DEFAULT mode in an untrusted folder', () => { - const config = new Config({ - sessionId: 'test', - targetDir: '.', - debugMode: false, - model: 'test-model', - cwd: '.', - trustedFolder: false, // Untrusted - }); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(false); expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); }); it('should NOT throw an error when setting any mode in a trusted folder', () => { - const config = new Config({ - sessionId: 'test', - targetDir: '.', - debugMode: false, - model: 'test-model', - cwd: '.', - trustedFolder: true, // Trusted - }); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); expect(() => config.setApprovalMode(ApprovalMode.YOLO)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.AUTO_EDIT)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); }); it('should NOT throw an error when setting any mode if trustedFolder is undefined', () => { - const config = new Config({ - sessionId: 'test', - targetDir: '.', - debugMode: false, - model: 'test-model', - cwd: '.', - trustedFolder: undefined, // Undefined - }); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); // isTrustedFolder defaults to true expect(() => config.setApprovalMode(ApprovalMode.YOLO)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.AUTO_EDIT)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 0b7b9cdc7b..2f052e30bf 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -572,7 +572,7 @@ export class Config { } setApprovalMode(mode: ApprovalMode): void { - if (this.isTrustedFolder() === false && mode !== ApprovalMode.DEFAULT) { + if (!this.isTrustedFolder() && mode !== ApprovalMode.DEFAULT) { throw new Error( 'Cannot enable privileged approval modes in an untrusted folder.', ); @@ -730,8 +730,18 @@ export class Config { return this.folderTrust; } - isTrustedFolder(): boolean | undefined { - return this.trustedFolder; + isTrustedFolder(): boolean { + // isWorkspaceTrusted in cli/src/config/trustedFolder.js returns undefined + // when the file based trust value is unavailable, since it is mainly used + // in the initialization for trust dialogs, etc. Here we return true since + // config.isTrustedFolder() is used for the main business logic of blocking + // tool calls etc in the rest of the application. + // + // Default value is true since we load with trusted settings to avoid + // restarts in the more common path. If the user chooses to mark the folder + // as untrusted, the CLI will restart and we will have the trust value + // reloaded. + return this.trustedFolder ?? true; } setIdeMode(value: boolean): void { diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index 6095381841..9611f823be 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -55,33 +55,6 @@ describe('McpClientManager', () => { expect(mockedMcpClient.discover).toHaveBeenCalledOnce(); }); - it('should discover tools if isTrustedFolder is undefined', async () => { - const mockedMcpClient = { - connect: vi.fn(), - discover: vi.fn(), - disconnect: vi.fn(), - getStatus: vi.fn(), - }; - vi.mocked(McpClient).mockReturnValue( - mockedMcpClient as unknown as McpClient, - ); - const manager = new McpClientManager( - { - 'test-server': {}, - }, - '', - {} as ToolRegistry, - {} as PromptRegistry, - false, - {} as WorkspaceContext, - ); - await manager.discoverAllMcpTools({ - isTrustedFolder: () => undefined, - } as unknown as Config); - expect(mockedMcpClient.connect).toHaveBeenCalledOnce(); - expect(mockedMcpClient.discover).toHaveBeenCalledOnce(); - }); - it('should not discover tools if folder is not trusted', async () => { const mockedMcpClient = { connect: vi.fn(), diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 3f25102af6..dcb4589106 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -56,7 +56,7 @@ export class McpClientManager { * them with the `ToolRegistry`. */ async discoverAllMcpTools(cliConfig: Config): Promise { - if (cliConfig.isTrustedFolder() === false) { + if (!cliConfig.isTrustedFolder()) { return; } await this.stop(); diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index 5fc64cbaa6..0e9396b040 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -598,6 +598,8 @@ describe('DiscoveredMCPTool', () => { inputSchema, undefined, true, + undefined, + { isTrustedFolder: () => true } as any, ); const invocation = trustedTool.build({ param: 'mock' }); expect( @@ -809,25 +811,6 @@ describe('DiscoveredMCPTool', () => { expect(confirmation).not.toBe(false); expect(confirmation).toHaveProperty('type', 'mcp'); }); - - it('should return false if trust is true and folder trust is undefined', async () => { - // The check is `isTrustedFolder() !== false`, so `undefined` should pass - const trustedTool = new DiscoveredMCPTool( - mockCallableToolInstance, - serverName, - serverToolName, - baseDescription, - inputSchema, - undefined, - true, // trust = true - undefined, - mockConfig(undefined) as any, // isTrustedFolder = undefined - ); - const invocation = trustedTool.build({ param: 'mock' }); - expect( - await invocation.shouldConfirmExecute(new AbortController().signal), - ).toBe(false); - }); }); describe('DiscoveredMCPToolInvocation', () => { diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 5bc48a1e7d..66eee800bb 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -82,9 +82,7 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< const serverAllowListKey = this.serverName; const toolAllowListKey = `${this.serverName}.${this.serverToolName}`; - const isTrustedFolder = this.cliConfig?.isTrustedFolder() !== false; - - if (this.trust && isTrustedFolder) { + if (this.cliConfig?.isTrustedFolder() && this.trust) { return false; // server is trusted, no confirmation needed }