fix(auth): prioritize GEMINI_API_KEY env var and skip unnecessary key… (#14745)

This commit is contained in:
Gal Zahavi
2025-12-12 17:50:21 -08:00
committed by GitHub
parent 942bcfc61e
commit 57c7b9ccce
10 changed files with 122 additions and 19 deletions

View File

@@ -582,6 +582,13 @@ Logging in with Google... Restarting Gemini CLI to continue.
settings.merged.security?.auth?.selectedType &&
!settings.merged.security?.auth?.useExternal
) {
// We skip validation for Gemini API key here because it might be stored
// in the keychain, which we can't check synchronously.
// The useAuth hook handles validation for this case.
if (settings.merged.security.auth.selectedType === AuthType.USE_GEMINI) {
return;
}
const error = validateAuthMethod(
settings.merged.security.auth.selectedType,
);

View File

@@ -12,8 +12,18 @@ import {
useTextBuffer,
type TextBuffer,
} from '../components/shared/text-buffer.js';
import { clearApiKey } from '@google/gemini-cli-core';
// Mocks
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@google/gemini-cli-core')>();
return {
...actual,
clearApiKey: vi.fn().mockResolvedValue(undefined),
};
});
vi.mock('../hooks/useKeypress.js', () => ({
useKeypress: vi.fn(),
}));
@@ -37,7 +47,8 @@ describe('ApiAuthDialog', () => {
let mockBuffer: TextBuffer;
beforeEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
vi.stubEnv('GEMINI_API_KEY', '');
mockBuffer = {
text: '',
lines: [''],
@@ -91,7 +102,9 @@ describe('ApiAuthDialog', () => {
({ keyName, sequence, expectedCall, args }) => {
mockBuffer.text = 'submitted-key'; // Set for the onSubmit case
render(<ApiAuthDialog onSubmit={onSubmit} onCancel={onCancel} />);
const keypressHandler = mockedUseKeypress.mock.calls[0][0];
// calls[0] is the ApiAuthDialog's useKeypress (Ctrl+C handler)
// calls[1] is the TextInput's useKeypress (typing handler)
const keypressHandler = mockedUseKeypress.mock.calls[1][0];
keypressHandler({
name: keyName,
@@ -117,4 +130,20 @@ describe('ApiAuthDialog', () => {
expect(lastFrame()).toContain('Invalid API Key');
});
it('calls clearApiKey and clears buffer when Ctrl+C is pressed', async () => {
render(<ApiAuthDialog onSubmit={onSubmit} onCancel={onCancel} />);
// calls[0] is the ApiAuthDialog's useKeypress (Ctrl+C handler)
const keypressHandler = mockedUseKeypress.mock.calls[0][0];
await keypressHandler({
name: 'c',
ctrl: true,
meta: false,
shift: false,
});
expect(clearApiKey).toHaveBeenCalled();
expect(mockBuffer.setText).toHaveBeenCalledWith('');
});
});

View File

@@ -5,11 +5,15 @@
*/
import type React from 'react';
import { useRef, useEffect } from 'react';
import { Box, Text } from 'ink';
import { theme } from '../semantic-colors.js';
import { TextInput } from '../components/shared/TextInput.js';
import { useTextBuffer } from '../components/shared/text-buffer.js';
import { useUIState } from '../contexts/UIStateContext.js';
import { clearApiKey, debugLogger } from '@google/gemini-cli-core';
import { useKeypress } from '../hooks/useKeypress.js';
import { keyMatchers, Command } from '../keyMatchers.js';
interface ApiAuthDialogProps {
onSubmit: (apiKey: string) => void;
@@ -27,9 +31,20 @@ export function ApiAuthDialog({
const { mainAreaWidth } = useUIState();
const viewportWidth = mainAreaWidth - 8;
const pendingPromise = useRef<{ cancel: () => void } | null>(null);
useEffect(
() => () => {
pendingPromise.current?.cancel();
},
[],
);
const initialApiKey = defaultValue;
const buffer = useTextBuffer({
initialText: defaultValue || '',
initialCursorOffset: defaultValue?.length || 0,
initialText: initialApiKey || '',
initialCursorOffset: initialApiKey?.length || 0,
viewport: {
width: viewportWidth,
height: 4,
@@ -44,6 +59,41 @@ export function ApiAuthDialog({
onSubmit(value);
};
const handleClear = () => {
pendingPromise.current?.cancel();
let isCancelled = false;
const wrappedPromise = new Promise<void>((resolve, reject) => {
clearApiKey().then(
() => !isCancelled && resolve(),
(error) => !isCancelled && reject(error),
);
});
pendingPromise.current = {
cancel: () => {
isCancelled = true;
},
};
return wrappedPromise
.then(() => {
buffer.setText('');
})
.catch((err) => {
debugLogger.debug('Failed to clear API key:', err);
});
};
useKeypress(
async (key) => {
if (keyMatchers[Command.CLEAR_INPUT](key)) {
await handleClear();
}
},
{ isActive: true },
);
return (
<Box
borderStyle="round"
@@ -89,7 +139,7 @@ export function ApiAuthDialog({
)}
<Box marginTop={1}>
<Text color={theme.text.secondary}>
(Press Enter to submit, Esc to cancel)
(Press Enter to submit, Esc to cancel, Ctrl+C to clear stored key)
</Text>
</Box>
</Box>

View File

@@ -232,6 +232,21 @@ describe('AuthDialog', () => {
);
});
it('skips API key dialog if env var is present but empty', async () => {
mockedValidateAuthMethod.mockReturnValue(null);
process.env['GEMINI_API_KEY'] = ''; // Empty string
// props.settings.merged.security.auth.selectedType is undefined here
renderWithProviders(<AuthDialog {...props} />);
const { onSelect: handleAuthSelect } =
mockedRadioButtonSelect.mock.calls[0][0];
await handleAuthSelect(AuthType.USE_GEMINI);
expect(props.setAuthState).toHaveBeenCalledWith(
AuthState.Unauthenticated,
);
});
it('shows API key dialog on initial setup if no env var is present', async () => {
mockedValidateAuthMethod.mockReturnValue(null);
// process.env['GEMINI_API_KEY'] is not set
@@ -247,7 +262,7 @@ describe('AuthDialog', () => {
);
});
it('shows API key dialog on re-auth to allow editing', async () => {
it('skips API key dialog on re-auth if env var is present (cannot edit)', async () => {
mockedValidateAuthMethod.mockReturnValue(null);
process.env['GEMINI_API_KEY'] = 'test-key-from-env';
// Simulate that the user has already authenticated once
@@ -260,7 +275,7 @@ describe('AuthDialog', () => {
await handleAuthSelect(AuthType.USE_GEMINI);
expect(props.setAuthState).toHaveBeenCalledWith(
AuthState.AwaitingApiKeyInput,
AuthState.Unauthenticated,
);
});

View File

@@ -116,9 +116,6 @@ export function AuthDialog({
return;
}
if (authType) {
const isInitialAuthSelection =
!settings.merged.security?.auth?.selectedType;
await clearCachedCredentialFile();
settings.setValue(scope, 'security.auth.selectedType', authType);
@@ -135,7 +132,7 @@ export function AuthDialog({
}
if (authType === AuthType.USE_GEMINI) {
if (isInitialAuthSelection && process.env['GEMINI_API_KEY']) {
if (process.env['GEMINI_API_KEY'] !== undefined) {
setAuthState(AuthState.Unauthenticated);
return;
} else {

View File

@@ -12,7 +12,7 @@ exports[`ApiAuthDialog > renders correctly 1`] = `
│ │ Paste your API key here │ │
│ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ │
│ (Press Enter to submit, Esc to cancel)
│ (Press Enter to submit, Esc to cancel, Ctrl+C to clear stored key)
│ │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯"
`;

View File

@@ -40,8 +40,8 @@ vi.mock('../../config/auth.js', () => ({
describe('useAuth', () => {
beforeEach(() => {
vi.resetAllMocks();
process.env['GEMINI_API_KEY'] = '';
process.env['GEMINI_DEFAULT_AUTH_TYPE'] = '';
delete process.env['GEMINI_API_KEY'];
delete process.env['GEMINI_DEFAULT_AUTH_TYPE'];
});
afterEach(() => {

View File

@@ -55,11 +55,15 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
);
const reloadApiKey = useCallback(async () => {
const envKey = process.env['GEMINI_API_KEY'];
if (envKey !== undefined) {
setApiKeyDefaultValue(envKey);
return envKey;
}
const storedKey = (await loadApiKey()) ?? '';
const envKey = process.env['GEMINI_API_KEY'] ?? '';
const key = envKey || storedKey;
setApiKeyDefaultValue(key);
return key; // Return the key for immediate use
setApiKeyDefaultValue(storedKey);
return storedKey;
}, []);
useEffect(() => {

View File

@@ -166,6 +166,7 @@ export const DialogManager = ({
return (
<Box flexDirection="column">
<ApiAuthDialog
key={uiState.apiKeyDefaultValue}
onSubmit={uiActions.handleApiKeySubmit}
onCancel={uiActions.handleApiKeyCancel}
error={uiState.authError}

View File

@@ -66,7 +66,7 @@ export async function createContentGeneratorConfig(
authType: AuthType | undefined,
): Promise<ContentGeneratorConfig> {
const geminiApiKey =
(await loadApiKey()) || process.env['GEMINI_API_KEY'] || undefined;
process.env['GEMINI_API_KEY'] || (await loadApiKey()) || undefined;
const googleApiKey = process.env['GOOGLE_API_KEY'] || undefined;
const googleCloudProject =
process.env['GOOGLE_CLOUD_PROJECT'] ||