Resolve error thrown for sensitive values (#17826)

This commit is contained in:
christine betts
2026-01-29 09:59:26 -05:00
committed by GitHub
parent eb17d94e91
commit 42eedc9184
2 changed files with 76 additions and 2 deletions

View File

@@ -398,6 +398,35 @@ describe('extensionSettings', () => {
expect(actualContent).toBe('VAR1="a value with spaces"\n'); 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 () => { it('should not attempt to clear secrets if keychain is unavailable', async () => {
// Arrange // Arrange
const mockIsAvailable = vi.fn().mockResolvedValue(false); const mockIsAvailable = vi.fn().mockResolvedValue(false);
@@ -738,5 +767,42 @@ describe('extensionSettings', () => {
const lines = actualContent.split('\n').filter((line) => line.length > 0); const lines = actualContent.split('\n').filter((line) => line.length > 0);
expect(lines).toHaveLength(3); // Should only have the three variables 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
});
}); });
}); });

View File

@@ -112,7 +112,7 @@ export async function maybePromptForSettings(
const nonSensitiveSettings: Record<string, string> = {}; const nonSensitiveSettings: Record<string, string> = {};
for (const setting of settings) { for (const setting of settings) {
const value = allSettings[setting.envVar]; const value = allSettings[setting.envVar];
if (value === undefined) { if (value === undefined || value === '') {
continue; continue;
} }
if (setting.sensitive) { if (setting.sensitive) {
@@ -230,7 +230,15 @@ export async function updateSetting(
); );
if (settingToUpdate.sensitive) { 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; return;
} }