From 93820f833f53a3f4edaf063715f6ce7b8d1c185c Mon Sep 17 00:00:00 2001 From: shishu314 Date: Tue, 2 Sep 2025 12:01:22 -0400 Subject: [PATCH] Fix(cli) - Remove Foldertrust Feature Flag (#7420) Co-authored-by: Shi Shu --- docs/cli/configuration.md | 4 - packages/cli/src/config/config.test.ts | 228 +----------------- packages/cli/src/config/config.ts | 6 +- packages/cli/src/config/settings.ts | 1 - packages/cli/src/config/settingsSchema.ts | 9 - .../cli/src/config/trustedFolders.test.ts | 1 - packages/cli/src/config/trustedFolders.ts | 4 +- packages/cli/src/ui/hooks/useFolderTrust.ts | 5 +- 8 files changed, 14 insertions(+), 244 deletions(-) diff --git a/docs/cli/configuration.md b/docs/cli/configuration.md index bc114f178b..73f6c1410d 100644 --- a/docs/cli/configuration.md +++ b/docs/cli/configuration.md @@ -228,10 +228,6 @@ Settings are organized into categories. All settings should be placed within the #### `security` -- **`security.folderTrust.featureEnabled`** (boolean): - - **Description:** Enable folder trust feature for enhanced security. - - **Default:** `false` - - **`security.folderTrust.enabled`** (boolean): - **Description:** Setting to track whether Folder trust is enabled. - **Default:** `false` diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 79cf6d29c7..95f334173e 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -4,15 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - describe, - it, - expect, - vi, - beforeEach, - afterEach, - type Mock, -} from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'node:os'; import * as path from 'node:path'; import { ShellTool, EditTool, WriteFileTool } from '@google/gemini-cli-core'; @@ -1492,40 +1484,6 @@ describe('loadCliConfig model selection', () => { }); }); -describe('loadCliConfig folderTrustFeature', () => { - const originalArgv = process.argv; - - beforeEach(() => { - vi.resetAllMocks(); - vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); - vi.stubEnv('GEMINI_API_KEY', 'test-api-key'); - }); - - afterEach(() => { - process.argv = originalArgv; - vi.unstubAllEnvs(); - vi.restoreAllMocks(); - }); - - it('should be false by default', async () => { - process.argv = ['node', 'script.js']; - const settings: Settings = {}; - const argv = await parseArguments({} as Settings); - const config = await loadCliConfig(settings, [], 'test-session', argv); - expect(config.getFolderTrustFeature()).toBe(false); - }); - - it('should be true when settings.folderTrustFeature is true', async () => { - process.argv = ['node', 'script.js']; - const argv = await parseArguments({} as Settings); - const settings: Settings = { - security: { folderTrust: { featureEnabled: true } }, - }; - const config = await loadCliConfig(settings, [], 'test-session', argv); - expect(config.getFolderTrustFeature()).toBe(true); - }); -}); - describe('loadCliConfig folderTrust', () => { const originalArgv = process.argv; @@ -1541,12 +1499,11 @@ describe('loadCliConfig folderTrust', () => { vi.restoreAllMocks(); }); - it('should be false if folderTrustFeature is false and folderTrust is false', async () => { + it('should be false when folderTrust is false', async () => { process.argv = ['node', 'script.js']; const settings: Settings = { security: { folderTrust: { - featureEnabled: false, enabled: false, }, }, @@ -1556,43 +1513,12 @@ describe('loadCliConfig folderTrust', () => { expect(config.getFolderTrust()).toBe(false); }); - it('should be false if folderTrustFeature is true and folderTrust is false', async () => { + it('should be true when folderTrust is true', async () => { process.argv = ['node', 'script.js']; const argv = await parseArguments({} as Settings); const settings: Settings = { security: { folderTrust: { - featureEnabled: true, - enabled: false, - }, - }, - }; - const config = await loadCliConfig(settings, [], 'test-session', argv); - expect(config.getFolderTrust()).toBe(false); - }); - - it('should be false if folderTrustFeature is false and folderTrust is true', async () => { - process.argv = ['node', 'script.js']; - const argv = await parseArguments({} as Settings); - const settings: Settings = { - security: { - folderTrust: { - featureEnabled: false, - enabled: true, - }, - }, - }; - const config = await loadCliConfig(settings, [], 'test-session', argv); - expect(config.getFolderTrust()).toBe(false); - }); - - it('should be true when folderTrustFeature is true and folderTrust is true', async () => { - process.argv = ['node', 'script.js']; - const argv = await parseArguments({} as Settings); - const settings: Settings = { - security: { - folderTrust: { - featureEnabled: true, enabled: true, }, }, @@ -1600,6 +1526,14 @@ describe('loadCliConfig folderTrust', () => { const config = await loadCliConfig(settings, [], 'test-session', argv); expect(config.getFolderTrust()).toBe(true); }); + + it('should be false by default', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments({} as Settings); + const settings: Settings = {}; + const config = await loadCliConfig(settings, [], 'test-session', argv); + expect(config.getFolderTrust()).toBe(false); + }); }); describe('loadCliConfig with includeDirectories', () => { @@ -1959,143 +1893,3 @@ describe('loadCliConfig approval mode', () => { }); }); }); - -describe('loadCliConfig trustedFolder', () => { - const originalArgv = process.argv; - - beforeEach(() => { - vi.resetAllMocks(); - vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); - vi.stubEnv('GEMINI_API_KEY', 'test-api-key'); - process.argv = ['node', 'script.js']; // Reset argv for each test - }); - - afterEach(() => { - process.argv = originalArgv; - vi.unstubAllEnvs(); - vi.restoreAllMocks(); - }); - - const testCases = [ - // Cases where folderTrustFeature is false (feature disabled) - { - folderTrustFeature: false, - folderTrust: true, - isWorkspaceTrusted: true, - expectedFolderTrust: false, - expectedIsTrustedFolder: true, - description: - 'feature disabled, folderTrust true, workspace trusted -> behave as trusted', - }, - { - folderTrustFeature: false, - folderTrust: true, - isWorkspaceTrusted: false, - expectedFolderTrust: false, - expectedIsTrustedFolder: true, - description: - 'feature disabled, folderTrust true, workspace not trusted -> behave as trusted', - }, - { - folderTrustFeature: false, - folderTrust: false, - isWorkspaceTrusted: true, - expectedFolderTrust: false, - expectedIsTrustedFolder: true, - description: - 'feature disabled, folderTrust false, workspace trusted -> behave as trusted', - }, - { - folderTrustFeature: false, - folderTrust: false, - isWorkspaceTrusted: false, - expectedFolderTrust: false, - expectedIsTrustedFolder: true, - description: - 'feature disabled, folderTrust false, workspace not trusted -> behave as trusted', - }, - // Cases where folderTrustFeature is true (feature enabled) - { - folderTrustFeature: true, - folderTrust: true, - isWorkspaceTrusted: true, - expectedFolderTrust: true, - expectedIsTrustedFolder: true, - description: - 'feature enabled, folderTrust true, workspace trusted -> behave as trusted', - }, - { - folderTrustFeature: true, - folderTrust: true, - isWorkspaceTrusted: false, - expectedFolderTrust: true, - expectedIsTrustedFolder: false, - description: - 'feature enabled, folderTrust true, workspace not trusted -> behave as not trusted', - }, - { - folderTrustFeature: true, - folderTrust: true, - isWorkspaceTrusted: undefined, - expectedFolderTrust: true, - expectedIsTrustedFolder: true, - description: - '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: isWorkspaceTrustedValue, - expectedFolderTrust, - expectedIsTrustedFolder, - description, - } of testCases) { - it(`should correctly set folderTrust and isTrustedFolder when ${description}`, async () => { - (isWorkspaceTrusted as Mock).mockImplementation((settings: Settings) => { - const folderTrustFeature = - settings.security?.folderTrust?.featureEnabled ?? false; - const folderTrustSetting = - settings.security?.folderTrust?.enabled ?? true; - const folderTrustEnabled = folderTrustFeature && folderTrustSetting; - - if (!folderTrustEnabled) { - return true; - } - return isWorkspaceTrustedValue; // This is the part that comes from the test case - }); - const argv = await parseArguments({} as Settings); - const settings: Settings = { - security: { - folderTrust: { - featureEnabled: folderTrustFeature, - enabled: folderTrust, - }, - }, - }; - const config = await loadCliConfig(settings, [], 'test-session', argv); - - expect(config.getFolderTrust()).toBe(expectedFolderTrust); - expect(config.isTrustedFolder()).toBe(expectedIsTrustedFolder); - }); - } -}); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 0673fecb6d..3977301235 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -384,10 +384,7 @@ export async function loadCliConfig( const ideMode = settings.ide?.enabled ?? false; - const folderTrustFeature = - settings.security?.folderTrust?.featureEnabled ?? false; - const folderTrustSetting = settings.security?.folderTrust?.enabled ?? true; - const folderTrust = folderTrustFeature && folderTrustSetting; + const folderTrust = settings.security?.folderTrust?.enabled ?? false; const trustedFolder = isWorkspaceTrusted(settings) ?? true; const allExtensions = annotateActiveExtensions( @@ -613,7 +610,6 @@ export async function loadCliConfig( summarizeToolOutput: settings.model?.summarizeToolOutput, ideMode, chatCompression: settings.model?.chatCompression, - folderTrustFeature, folderTrust, interactive, trustedFolder, diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 205cfc6c0a..f7fb67ad92 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -74,7 +74,6 @@ const MIGRATION_MAP: Record = { mcpServerCommand: 'mcp.serverCommand', allowMCPServers: 'mcp.allowed', excludeMCPServers: 'mcp.excluded', - folderTrustFeature: 'security.folderTrust.featureEnabled', folderTrust: 'security.folderTrust.enabled', selectedAuthType: 'security.auth.selectedType', useExternalAuth: 'security.auth.useExternal', diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index f884a332af..2cebe7392c 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -640,15 +640,6 @@ export const SETTINGS_SCHEMA = { description: 'Settings for folder trust.', showInDialog: false, properties: { - featureEnabled: { - type: 'boolean', - label: 'Folder Trust Feature', - category: 'Security', - requiresRestart: true, - default: false, - description: 'Enable folder trust feature for enhanced security.', - showInDialog: true, - }, enabled: { type: 'boolean', label: 'Folder Trust', diff --git a/packages/cli/src/config/trustedFolders.test.ts b/packages/cli/src/config/trustedFolders.test.ts index bf03682f59..23abee438f 100644 --- a/packages/cli/src/config/trustedFolders.test.ts +++ b/packages/cli/src/config/trustedFolders.test.ts @@ -180,7 +180,6 @@ describe('isWorkspaceTrusted', () => { const mockSettings: Settings = { security: { folderTrust: { - featureEnabled: true, enabled: true, }, }, diff --git a/packages/cli/src/config/trustedFolders.ts b/packages/cli/src/config/trustedFolders.ts index faec621d5f..87271cf4be 100644 --- a/packages/cli/src/config/trustedFolders.ts +++ b/packages/cli/src/config/trustedFolders.ts @@ -155,10 +155,8 @@ export function saveTrustedFolders( /** Is folder trust feature enabled per the current applied settings */ export function isFolderTrustEnabled(settings: Settings): boolean { - const folderTrustFeature = - settings.security?.folderTrust?.featureEnabled ?? false; const folderTrustSetting = settings.security?.folderTrust?.enabled ?? false; - return folderTrustFeature && folderTrustSetting; + return folderTrustSetting; } export function isWorkspaceTrusted(settings: Settings): boolean | undefined { diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 7136f177e9..b74d860f14 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -23,14 +23,11 @@ export const useFolderTrust = ( const [isRestarting, setIsRestarting] = useState(false); const folderTrust = settings.merged.security?.folderTrust?.enabled; - const folderTrustFeature = - settings.merged.security?.folderTrust?.featureEnabled; useEffect(() => { const trusted = isWorkspaceTrusted({ security: { folderTrust: { - featureEnabled: folderTrustFeature, enabled: folderTrust, }, }, @@ -38,7 +35,7 @@ export const useFolderTrust = ( setIsTrusted(trusted); setIsFolderTrustDialogOpen(trusted === undefined); onTrustChange(trusted); - }, [onTrustChange, folderTrust, folderTrustFeature]); + }, [onTrustChange, folderTrust]); const handleFolderTrustSelect = useCallback( (choice: FolderTrustChoice) => {