fix(trust): Update config.isTrustedFolder (#7373)

This commit is contained in:
shrutip90
2025-08-28 19:41:33 -07:00
committed by GitHub
parent 9037f25df7
commit 2a0e69d833
11 changed files with 67 additions and 135 deletions

View File

@@ -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)!',
);

View File

@@ -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 = {

View File

@@ -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<number>(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);

View File

@@ -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(
<ToolConfirmationMessage
confirmationDetails={details}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);
expect(lastFrame()).toContain(alwaysAllowText);
});
it('should NOT show "allow always" when folder is untrusted', () => {
const mockConfig = {
isTrustedFolder: () => false,

View File

@@ -56,7 +56,7 @@ export const ToolConfirmationMessage: React.FC<
onConfirm(outcome);
};
const isTrustedFolder = config.isTrustedFolder() !== false;
const isTrustedFolder = config.isTrustedFolder();
useKeypress(
(key) => {

View File

@@ -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();

View File

@@ -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 {

View File

@@ -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(),

View File

@@ -56,7 +56,7 @@ export class McpClientManager {
* them with the `ToolRegistry`.
*/
async discoverAllMcpTools(cliConfig: Config): Promise<void> {
if (cliConfig.isTrustedFolder() === false) {
if (!cliConfig.isTrustedFolder()) {
return;
}
await this.stop();

View File

@@ -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', () => {

View File

@@ -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
}