Skip to content

Commit 57c7b9c

Browse files
galdawavegaldawave
authored
fix(auth): prioritize GEMINI_API_KEY env var and skip unnecessary key… (google-gemini#14745)
1 parent 942bcfc commit 57c7b9c

10 files changed

Lines changed: 122 additions & 19 deletions

File tree

‎packages/cli/src/ui/AppContainer.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,13 @@ Logging in with Google... Restarting Gemini CLI to continue.
582582
settings.merged.security?.auth?.selectedType &&
583583
!settings.merged.security?.auth?.useExternal
584584
) {
585+
// We skip validation for Gemini API key here because it might be stored
586+
// in the keychain, which we can't check synchronously.
587+
// The useAuth hook handles validation for this case.
588+
if (settings.merged.security.auth.selectedType === AuthType.USE_GEMINI) {
589+
return;
590+
}
591+
585592
const error = validateAuthMethod(
586593
settings.merged.security.auth.selectedType,
587594
);

‎packages/cli/src/ui/auth/ApiAuthDialog.test.tsx‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,18 @@ import {
1212
useTextBuffer,
1313
type TextBuffer,
1414
} from '../components/shared/text-buffer.js';
15+
import { clearApiKey } from '@google/gemini-cli-core';
1516

1617
// Mocks
18+
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
19+
const actual =
20+
await importOriginal<typeof import('@google/gemini-cli-core')>();
21+
return {
22+
...actual,
23+
clearApiKey: vi.fn().mockResolvedValue(undefined),
24+
};
25+
});
26+
1727
vi.mock('../hooks/useKeypress.js', () => ({
1828
useKeypress: vi.fn(),
1929
}));
@@ -37,7 +47,8 @@ describe('ApiAuthDialog', () => {
3747
let mockBuffer: TextBuffer;
3848

3949
beforeEach(() => {
40-
vi.resetAllMocks();
50+
vi.clearAllMocks();
51+
vi.stubEnv('GEMINI_API_KEY', '');
4152
mockBuffer = {
4253
text: '',
4354
lines: [''],
@@ -91,7 +102,9 @@ describe('ApiAuthDialog', () => {
91102
({ keyName, sequence, expectedCall, args }) => {
92103
mockBuffer.text = 'submitted-key'; // Set for the onSubmit case
93104
render(<ApiAuthDialog onSubmit={onSubmit} onCancel={onCancel} />);
94-
const keypressHandler = mockedUseKeypress.mock.calls[0][0];
105+
// calls[0] is the ApiAuthDialog's useKeypress (Ctrl+C handler)
106+
// calls[1] is the TextInput's useKeypress (typing handler)
107+
const keypressHandler = mockedUseKeypress.mock.calls[1][0];
95108

96109
keypressHandler({
97110
name: keyName,
@@ -117,4 +130,20 @@ describe('ApiAuthDialog', () => {
117130

118131
expect(lastFrame()).toContain('Invalid API Key');
119132
});
133+
134+
it('calls clearApiKey and clears buffer when Ctrl+C is pressed', async () => {
135+
render(<ApiAuthDialog onSubmit={onSubmit} onCancel={onCancel} />);
136+
// calls[0] is the ApiAuthDialog's useKeypress (Ctrl+C handler)
137+
const keypressHandler = mockedUseKeypress.mock.calls[0][0];
138+
139+
await keypressHandler({
140+
name: 'c',
141+
ctrl: true,
142+
meta: false,
143+
shift: false,
144+
});
145+
146+
expect(clearApiKey).toHaveBeenCalled();
147+
expect(mockBuffer.setText).toHaveBeenCalledWith('');
148+
});
120149
});

‎packages/cli/src/ui/auth/ApiAuthDialog.tsx‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@
55
*/
66

77
import type React from 'react';
8+
import { useRef, useEffect } from 'react';
89
import { Box, Text } from 'ink';
910
import { theme } from '../semantic-colors.js';
1011
import { TextInput } from '../components/shared/TextInput.js';
1112
import { useTextBuffer } from '../components/shared/text-buffer.js';
1213
import { useUIState } from '../contexts/UIStateContext.js';
14+
import { clearApiKey, debugLogger } from '@google/gemini-cli-core';
15+
import { useKeypress } from '../hooks/useKeypress.js';
16+
import { keyMatchers, Command } from '../keyMatchers.js';
1317

1418
interface ApiAuthDialogProps {
1519
onSubmit: (apiKey: string) => void;
@@ -27,9 +31,20 @@ export function ApiAuthDialog({
2731
const { mainAreaWidth } = useUIState();
2832
const viewportWidth = mainAreaWidth - 8;
2933

34+
const pendingPromise = useRef<{ cancel: () => void } | null>(null);
35+
36+
useEffect(
37+
() => () => {
38+
pendingPromise.current?.cancel();
39+
},
40+
[],
41+
);
42+
43+
const initialApiKey = defaultValue;
44+
3045
const buffer = useTextBuffer({
31-
initialText: defaultValue || '',
32-
initialCursorOffset: defaultValue?.length || 0,
46+
initialText: initialApiKey || '',
47+
initialCursorOffset: initialApiKey?.length || 0,
3348
viewport: {
3449
width: viewportWidth,
3550
height: 4,
@@ -44,6 +59,41 @@ export function ApiAuthDialog({
4459
onSubmit(value);
4560
};
4661

62+
const handleClear = () => {
63+
pendingPromise.current?.cancel();
64+
65+
let isCancelled = false;
66+
const wrappedPromise = new Promise<void>((resolve, reject) => {
67+
clearApiKey().then(
68+
() => !isCancelled && resolve(),
69+
(error) => !isCancelled && reject(error),
70+
);
71+
});
72+
73+
pendingPromise.current = {
74+
cancel: () => {
75+
isCancelled = true;
76+
},
77+
};
78+
79+
return wrappedPromise
80+
.then(() => {
81+
buffer.setText('');
82+
})
83+
.catch((err) => {
84+
debugLogger.debug('Failed to clear API key:', err);
85+
});
86+
};
87+
88+
useKeypress(
89+
async (key) => {
90+
if (keyMatchers[Command.CLEAR_INPUT](key)) {
91+
await handleClear();
92+
}
93+
},
94+
{ isActive: true },
95+
);
96+
4797
return (
4898
<Box
4999
borderStyle="round"
@@ -89,7 +139,7 @@ export function ApiAuthDialog({
89139
)}
90140
<Box marginTop={1}>
91141
<Text color={theme.text.secondary}>
92-
(Press Enter to submit, Esc to cancel)
142+
(Press Enter to submit, Esc to cancel, Ctrl+C to clear stored key)
93143
</Text>
94144
</Box>
95145
</Box>

‎packages/cli/src/ui/auth/AuthDialog.test.tsx‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,21 @@ describe('AuthDialog', () => {
232232
);
233233
});
234234

235+
it('skips API key dialog if env var is present but empty', async () => {
236+
mockedValidateAuthMethod.mockReturnValue(null);
237+
process.env['GEMINI_API_KEY'] = ''; // Empty string
238+
// props.settings.merged.security.auth.selectedType is undefined here
239+
240+
renderWithProviders(<AuthDialog {...props} />);
241+
const { onSelect: handleAuthSelect } =
242+
mockedRadioButtonSelect.mock.calls[0][0];
243+
await handleAuthSelect(AuthType.USE_GEMINI);
244+
245+
expect(props.setAuthState).toHaveBeenCalledWith(
246+
AuthState.Unauthenticated,
247+
);
248+
});
249+
235250
it('shows API key dialog on initial setup if no env var is present', async () => {
236251
mockedValidateAuthMethod.mockReturnValue(null);
237252
// process.env['GEMINI_API_KEY'] is not set
@@ -247,7 +262,7 @@ describe('AuthDialog', () => {
247262
);
248263
});
249264

250-
it('shows API key dialog on re-auth to allow editing', async () => {
265+
it('skips API key dialog on re-auth if env var is present (cannot edit)', async () => {
251266
mockedValidateAuthMethod.mockReturnValue(null);
252267
process.env['GEMINI_API_KEY'] = 'test-key-from-env';
253268
// Simulate that the user has already authenticated once
@@ -260,7 +275,7 @@ describe('AuthDialog', () => {
260275
await handleAuthSelect(AuthType.USE_GEMINI);
261276

262277
expect(props.setAuthState).toHaveBeenCalledWith(
263-
AuthState.AwaitingApiKeyInput,
278+
AuthState.Unauthenticated,
264279
);
265280
});
266281

‎packages/cli/src/ui/auth/AuthDialog.tsx‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,6 @@ export function AuthDialog({
116116
return;
117117
}
118118
if (authType) {
119-
const isInitialAuthSelection =
120-
!settings.merged.security?.auth?.selectedType;
121-
122119
await clearCachedCredentialFile();
123120

124121
settings.setValue(scope, 'security.auth.selectedType', authType);
@@ -135,7 +132,7 @@ export function AuthDialog({
135132
}
136133

137134
if (authType === AuthType.USE_GEMINI) {
138-
if (isInitialAuthSelection && process.env['GEMINI_API_KEY']) {
135+
if (process.env['GEMINI_API_KEY'] !== undefined) {
139136
setAuthState(AuthState.Unauthenticated);
140137
return;
141138
} else {

‎packages/cli/src/ui/auth/__snapshots__/ApiAuthDialog.test.tsx.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ exports[`ApiAuthDialog > renders correctly 1`] = `
1212
│ │ Paste your API key here │ │
1313
│ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ │
1414
│ │
15-
│ (Press Enter to submit, Esc to cancel) │
15+
│ (Press Enter to submit, Esc to cancel, Ctrl+C to clear stored key) │
1616
│ │
1717
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯"
1818
`;

‎packages/cli/src/ui/auth/useAuth.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ vi.mock('../../config/auth.js', () => ({
4040
describe('useAuth', () => {
4141
beforeEach(() => {
4242
vi.resetAllMocks();
43-
process.env['GEMINI_API_KEY'] = '';
44-
process.env['GEMINI_DEFAULT_AUTH_TYPE'] = '';
43+
delete process.env['GEMINI_API_KEY'];
44+
delete process.env['GEMINI_DEFAULT_AUTH_TYPE'];
4545
});
4646

4747
afterEach(() => {

‎packages/cli/src/ui/auth/useAuth.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,15 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
5555
);
5656

5757
const reloadApiKey = useCallback(async () => {
58+
const envKey = process.env['GEMINI_API_KEY'];
59+
if (envKey !== undefined) {
60+
setApiKeyDefaultValue(envKey);
61+
return envKey;
62+
}
63+
5864
const storedKey = (await loadApiKey()) ?? '';
59-
const envKey = process.env['GEMINI_API_KEY'] ?? '';
60-
const key = envKey || storedKey;
61-
setApiKeyDefaultValue(key);
62-
return key; // Return the key for immediate use
65+
setApiKeyDefaultValue(storedKey);
66+
return storedKey;
6367
}, []);
6468

6569
useEffect(() => {

‎packages/cli/src/ui/components/DialogManager.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ export const DialogManager = ({
166166
return (
167167
<Box flexDirection="column">
168168
<ApiAuthDialog
169+
key={uiState.apiKeyDefaultValue}
169170
onSubmit={uiActions.handleApiKeySubmit}
170171
onCancel={uiActions.handleApiKeyCancel}
171172
error={uiState.authError}

‎packages/core/src/core/contentGenerator.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export async function createContentGeneratorConfig(
6666
authType: AuthType | undefined,
6767
): Promise<ContentGeneratorConfig> {
6868
const geminiApiKey =
69-
(await loadApiKey()) || process.env['GEMINI_API_KEY'] || undefined;
69+
process.env['GEMINI_API_KEY'] || (await loadApiKey()) || undefined;
7070
const googleApiKey = process.env['GOOGLE_API_KEY'] || undefined;
7171
const googleCloudProject =
7272
process.env['GOOGLE_CLOUD_PROJECT'] ||

0 commit comments

Comments
 (0)