Skip to content

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

Merged
galz10 merged 20 commits intomainfrom
galzahavi/fix/api-auth
Dec 13, 2025
Merged

fix(auth): prioritize GEMINI_API_KEY env var and skip unnecessary key…#14745
galz10 merged 20 commits intomainfrom
galzahavi/fix/api-auth

Conversation

@galz10
Copy link
Copy Markdown
Collaborator

@galz10 galz10 commented Dec 8, 2025

Summary

This PR updates the authentication logic to prioritize the GEMINI_API_KEY environment variable over the system keychain. This prevents unnecessary keychain access prompts for users who have configured the CLI via environment variables.

Details

Previously, the application would unconditionally attempt to load credentials from the system keychain on startup, even if a valid GEMINI_API_KEY was present in the environment. This often triggered system prompts (e.g., macOS Keychain Access) or errors in environments where keychain access is restricted or unwanted.

This change refactors useAuth.ts to:

  1. Check process.env['GEMINI_API_KEY'] first.
  2. If the environment variable is present, use it immediately and skip the keychain read.
  3. Only fall back to loadApiKey() (keychain access) if the environment variable is missing.

Additionally, ApiAuthDialog.tsx has been updated to initialize the text buffer with the environment variable value if available, improving the UI experience.

Related Issues

Related to internal ticket: manual-request-prevent-keychain-access

How to Validate

  1. Setup Environment Variable:

    export GEMINI_API_KEY="your-test-api-key"
  2. Run the CLI:

    npm run start
  3. Expected Result:

    • The application should start without prompting for keychain access.
    • If the API Key dialog appears (e.g. forced), the key should be pre-filled.
  4. Verify Fallback (Edge Case):

    • Unset the environment variable (unset GEMINI_API_KEY).
    • Run the CLI.
    • Verify that the application still correctly loads stored credentials from the keychain if they exist.

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any)
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt
    • Windows
      • npm run
      • npx
      • Docker
    • Linux
      • npm run
      • npx
      • Docker

…chain access

When GEMINI_API_KEY is present in the environment, the application now uses it immediately without attempting to load credentials from the system keychain. This prevents unnecessary keychain access prompts for users who rely solely on environment variables.
@galz10 galz10 requested a review from a team as a code owner December 8, 2025 22:01
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @galz10, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the authentication mechanism by ensuring that the GEMINI_API_KEY environment variable takes precedence over any locally stored API keys. This change streamlines the API key retrieval process, making it more efficient and user-friendly by immediately leveraging environment-defined keys for both application logic and UI presentation.

Highlights

  • Environment Variable Prioritization: The GEMINI_API_KEY environment variable is now prioritized over locally stored keys when determining the API key to use for authentication.
  • UI Pre-fill Enhancement: The API key input dialog will automatically pre-fill with the GEMINI_API_KEY from environment variables, if available, improving user experience by reducing manual input.
  • Optimized API Key Loading: The reloadApiKey function has been optimized to immediately use the GEMINI_API_KEY environment variable if present, thereby skipping potentially unnecessary asynchronous operations to load a stored key.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to prioritize the GEMINI_API_KEY environment variable and optimize authentication by skipping unnecessary key loading. While the intention is good, the implementation introduces a subtle but important bug in both modified files. The use of the logical OR operator (||) to check for the environment variable's presence causes an explicitly empty GEMINI_API_KEY to be ignored, incorrectly falling back to a stored key. My review includes comments with high severity to address this by using checks that correctly handle an empty string as a valid, intentional value, ensuring the environment variable behavior is predictable and correct.

Comment thread packages/cli/src/ui/auth/ApiAuthDialog.tsx Outdated
Comment thread packages/cli/src/ui/auth/useAuth.ts Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 8, 2025

Size Change: +1.16 kB (+0.01%)

Total Size: 21.6 MB

Filename Size Change
./bundle/gemini.js 21.6 MB +1.16 kB (+0.01%)
ℹ️ View Unchanged
Filename Size
./bundle/sandbox-macos-permissive-closed.sb 1.03 kB
./bundle/sandbox-macos-permissive-open.sb 890 B
./bundle/sandbox-macos-permissive-proxied.sb 1.31 kB
./bundle/sandbox-macos-restrictive-closed.sb 3.29 kB
./bundle/sandbox-macos-restrictive-open.sb 3.36 kB
./bundle/sandbox-macos-restrictive-proxied.sb 3.56 kB

compressed-size-action

@galz10 galz10 force-pushed the galzahavi/fix/api-auth branch from 0ea18e4 to 37f086b Compare December 10, 2025 21:41
@galz10 galz10 force-pushed the galzahavi/fix/api-auth branch from c6dd376 to bd6fd6c Compare December 11, 2025 00:09
@jacob314
Copy link
Copy Markdown
Contributor

when I login with oauth and then log in with a key I still get the dialog to enter an API key even though I already have one exported in my ENV. I should not see the API KEY entry dialog at all if GEMINI_API_KEY is set

- Updates ApiAuthDialog to use Ctrl+C for clearing stored keys instead of Ctrl+K.
- Modifies AuthDialog to skip the API key input dialog if GEMINI_API_KEY is present in the environment, even on re-authentication.
- Adds a key to ApiAuthDialog in DialogManager to ensure the component re-mounts when the default value changes.
- Updates tests to verify that empty environment variables are handled correctly and that the dialog is skipped when env vars are present.
@galz10
Copy link
Copy Markdown
Collaborator Author

galz10 commented Dec 12, 2025

when I login with oauth and then log in with a key I still get the dialog to enter an API key even though I already have one exported in my ENV. I should not see the API KEY entry dialog at all if GEMINI_API_KEY is set

Fixed

@galz10 galz10 force-pushed the galzahavi/fix/api-auth branch from fda2897 to f83f83f Compare December 12, 2025 20:33
@jacob314
Copy link
Copy Markdown
Contributor

This review was generated by Gemini CLI.

Reviewing the changes to prioritize GEMINI_API_KEY.

Logic & Correctness:
The prioritization logic in useAuth.ts and the flow through AuthDialog -> useAuth effect -> Authenticated looks correct.

  • The empty string case (GEMINI_API_KEY="") is handled correctly: useAuth sees the empty key, transitions to AwaitingApiKeyInput, and allows the user to enter a key.

Tests:
In packages/cli/src/ui/auth/ApiAuthDialog.test.tsx:
The shift from calls[0] to calls[1] for mockedUseKeypress is correct due to the new hook call in the parent component.

  • Suggestion: It would be helpful to add a small comment explaining that calls[0] is the dialog's handler (Ctrl+C) and calls[1] is the TextInput's handler to prevent future confusion.

Code Unreachability:
In ApiAuthDialog.tsx, you added:

const initialApiKey = process.env['GEMINI_API_KEY'] || defaultValue;

It seems this line might effectively be dead code because:

  1. If GEMINI_API_KEY is set and truthy, useAuth transitions directly to Authenticated, so this dialog is never shown.
  2. If GEMINI_API_KEY is empty, useAuth transitions to AwaitingApiKeyInput, but the env var is empty, so it falls back to defaultValue (which is also empty).
    This is harmless, but worth noting that the pre-fill behavior likely never triggers in practice.

React:

  • handleClear in ApiAuthDialog.tsx awaits clearApiKey() before setting state. Ensure this doesn't cause issues if the component unmounts during the async call (though unlikely in this modal flow).

Overall, looks good!

Comment thread packages/cli/src/ui/auth/ApiAuthDialog.tsx Outdated
@jacob314
Copy link
Copy Markdown
Contributor

In packages/cli/src/ui/auth/ApiAuthDialog.tsx, you use the isMounted ref pattern. While acceptable here, consider using a cancellable promise pattern for better async management.

Example:

// const pendingPromise = useRef<{ cancel: () => void } | null>(null);
// useEffect(() => () => pendingPromise.current?.cancel(), []);
//
// const handleClear = () => {
//   pendingPromise.current?.cancel();
//   const { promise, cancel } = makeCancellable(clearApiKey());
//   pendingPromise.current = { cancel };
//   promise.then(() => buffer.setText(''));
// };

Copy link
Copy Markdown
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved once the two comments mentioned are addressed.

Copy link
Copy Markdown
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

galz10 and others added 2 commits December 12, 2025 15:28
   - Implement cancelable promise pattern for API key clearing to safely handle component unmounting
   - Transition to using shared keyMatchers for the clear input command to ensure consistent keyboard shortcuts
@galz10 galz10 enabled auto-merge December 12, 2025 23:28
@galz10 galz10 added this pull request to the merge queue Dec 13, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025
@galz10 galz10 added this pull request to the merge queue Dec 13, 2025
Merged via the queue into main with commit 57c7b9c Dec 13, 2025
20 checks passed
@galz10 galz10 deleted the galzahavi/fix/api-auth branch December 13, 2025 02:02
thacio added a commit to thacio/auditaria that referenced this pull request Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants