diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index db527f1ecb..09ed586b82 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -398,6 +398,35 @@ describe('extensionSettings', () => { expect(actualContent).toBe('VAR1="a value with spaces"\n'); }); + it('should not set sensitive settings if the value is empty during initial setup', async () => { + const config: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [ + { + name: 's1', + description: 'd1', + envVar: 'SENSITIVE_VAR', + sensitive: true, + }, + ], + }; + mockRequestSetting.mockResolvedValue(''); + + await maybePromptForSettings( + config, + '12345', + mockRequestSetting, + undefined, + undefined, + ); + + const userKeychain = new KeychainTokenStorage( + `Gemini CLI Extensions test-ext 12345`, + ); + expect(await userKeychain.getSecret('SENSITIVE_VAR')).toBeNull(); + }); + it('should not attempt to clear secrets if keychain is unavailable', async () => { // Arrange const mockIsAvailable = vi.fn().mockResolvedValue(false); @@ -738,5 +767,42 @@ describe('extensionSettings', () => { const lines = actualContent.split('\n').filter((line) => line.length > 0); expect(lines).toHaveLength(3); // Should only have the three variables }); + + it('should delete a sensitive setting if the new value is empty', async () => { + mockRequestSetting.mockResolvedValue(''); + + await updateSetting( + config, + '12345', + 'VAR2', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ); + + const userKeychain = new KeychainTokenStorage( + `Gemini CLI Extensions test-ext 12345`, + ); + expect(await userKeychain.getSecret('VAR2')).toBeNull(); + }); + + it('should not throw if deleting a non-existent sensitive setting with empty value', async () => { + mockRequestSetting.mockResolvedValue(''); + // Ensure it doesn't exist first + const userKeychain = new KeychainTokenStorage( + `Gemini CLI Extensions test-ext 12345`, + ); + await userKeychain.deleteSecret('VAR2'); + + await updateSetting( + config, + '12345', + 'VAR2', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ); + // Should complete without error + }); }); }); diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index 35624d86aa..4ba7d34b35 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -112,7 +112,7 @@ export async function maybePromptForSettings( const nonSensitiveSettings: Record = {}; for (const setting of settings) { const value = allSettings[setting.envVar]; - if (value === undefined) { + if (value === undefined || value === '') { continue; } if (setting.sensitive) { @@ -230,7 +230,15 @@ export async function updateSetting( ); if (settingToUpdate.sensitive) { - await keychain.setSecret(settingToUpdate.envVar, newValue); + if (newValue) { + await keychain.setSecret(settingToUpdate.envVar, newValue); + } else { + try { + await keychain.deleteSecret(settingToUpdate.envVar); + } catch { + // Ignore if secret does not exist + } + } return; }