Skip to content

fix(auth): prevent infinite auth loop from file contention, headless keyring, and supervisor state drops (#28341) - #29448

Merged
DavidAPierce merged 18 commits into
google-gemini:mainfrom
villahernandez-coder:FixBug-cla-561555286
Sep 28, 2026
Merged

DavidAPierce merged 18 commits into
google-gemini:mainfrom
villahernandez-coder:FixBug-cla-561555286

Conversation

@villahernandez-coder

@villahernandez-coder villahernandez-coder commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes an infinite authentication loop affecting users on Windows, WSL, and headless environments (#28341). This change resolves file contention collisions with companion tools (like the Gemini Code Assist VS Code extension), provides automatic fallback to encrypted file storage when libsecret is headless or locked in WSL, and guarantees security.auth.selectedType is preserved across supervisor relaunch transitions (exit code 199).

Details

  1. Atomic File I/O & Read Tolerance:
    • Both oauth_creds.json (cacheCredentials in oauth2.ts) and settings.json (updateSettingsFilePreservingFormat in commentJson.ts) now write atomically via sibling temp files (.${filename}.${pid}.${timestamp}.tmp) followed by atomic rename operations.
    • Added readFileWithRetry to handle transient file lock conditions (e.g. EBUSY / EPERM / partial reads on Windows NTFS) during concurrent access by external editors and extensions.
  2. Headless Linux & WSL Keyring Fallback:
    • Detects WSL (WSL_DISTRO_NAME / WSL_INTEROP) and headless environments without active GUI prompters (!DISPLAY && !WAYLAND_DISPLAY), cleanly bypassing libsecret / @github/keytar deadlocks and safely falling back to FileKeychain (~/.gemini/token-storage.json).
    • Hardened KeychainService.isKeychainFunctional probe against unhandled rejections during test cycles.
  3. Supervisor State Persistence Across Relaunches (exit code 199):
    • relaunchApp({ overrideAuthType }) now emits an auth-selected-type IPC event to the supervisor parent before exit, which injects GEMINI_CLI_AUTH_OVERRIDE into the spawned child process. This prevents security.auth.selectedType from reverting to undefined if disk synchronization is mid-flight.
  4. AuthDialog Premature File Purge Removal:
    • AuthDialog no longer unconditionally executes clearCachedCredentialFile() upon choosing Google Login, only clearing credentials when switching away from a differing configured authentication type.
    • Asynchronous relaunchApp calls inside timers are properly caught and handled.

Related Issues

Fixes #28341

How to Validate

Ran full unit and integration test suites:

  • npm test -w @google/gemini-cli-core -- src/services/keychainService.test.ts src/code_assist/oauth2.test.ts src/code_assist/oauth-credential-storage.test.ts (All 65 tests passed)
  • npm test -w @google/gemini-cli -- src/ui/auth/AuthDialog.test.tsx src/utils/processUtils.test.ts src/config/settings.test.ts src/utils/commentJson.test.ts (All 154 tests passed)
  • npm run typecheck & npm run lint (Passed with 0 errors/warnings)

Pre-Merge Checklist

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

…keyring, and supervisor state drops

- Implement atomic writes and retry loops for settings.json and oauth_creds.json
- Automatically fallback to FileKeychain in WSL and headless environments
- Preserve selectedType state across supervisor relaunches (exit code 199)
- Prevent premature oauth credentials file deletion in AuthDialog

Fixes b/561555286 (google-gemini#28341)
@villahernandez-coder
villahernandez-coder requested a review from a team as a code owner September 22, 2026 17:51
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 addresses critical stability issues causing infinite authentication loops on Windows, WSL, and headless environments. By implementing atomic file I/O, robust retry mechanisms for file reads, and a more reliable state persistence strategy during supervisor relaunches, the changes ensure that authentication settings remain consistent and accessible even under concurrent access or environment-specific constraints.

Highlights

  • Atomic File Operations: Implemented atomic file writing using temporary files and rename operations for oauth_creds.json and settings.json to prevent data corruption and handle file contention.
  • WSL/Headless Keyring Fallback: Added detection for WSL and headless Linux environments to bypass native libsecret keyring and safely fallback to file-based storage, preventing deadlocks.
  • Supervisor State Persistence: Ensured security.auth.selectedType is preserved across supervisor relaunches by injecting an IPC event that sets an environment variable override in the child process.
  • Robust File Access: Introduced readFileWithRetry to handle transient file lock conditions (e.g., EBUSY/EPERM) on Windows NTFS systems.
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 the 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 counterproductive. 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.

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. ↩

@github-actions github-actions Bot added the size/m A medium sized PR label Sep 22, 2026
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

📊 PR Size: size/L

  • Lines changed: 612
  • Additions: +546
  • Deletions: -66
  • Files changed: 16

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

Copy link
Copy Markdown
Contributor

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 introduces support for overriding the selected authentication type across application relaunches, implements atomic file operations for settings and credentials to prevent corruption, and adds a bypass for the native keychain in WSL and headless Linux environments. Feedback on these changes highlights a missing check for headless environments in isHeadlessLinuxOrWsl, a potential race condition when sending IPC messages before process exit, and a blocking busy-wait loop in the file read retry logic that should be optimized using Atomics.wait.

Comment thread packages/core/src/services/keychainService.ts Outdated
Comment thread packages/cli/src/utils/processUtils.ts Outdated
Comment thread packages/cli/src/utils/commentJson.ts Outdated
@gemini-cli gemini-cli Bot added priority/p1 Important and should be addressed in the near term. area/core Issues related to User Interface, OS Support, Core Functionality labels Sep 22, 2026
…s keyring, IPC drain, and sleep loop

- Add headless Linux display check (!DISPLAY && !WAYLAND_DISPLAY) to isHeadlessLinuxOrWsl
- Await process.send completion callback before relaunch cleanup to prevent IPC drops
- Replace CPU busy-wait loop in readFileWithRetry with Atomics.wait
@github-actions github-actions Bot added the size/l A large sized PR label Sep 22, 2026
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/Gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces support for overriding the selected authentication type across application relaunches via the GEMINI_CLI_AUTH_OVERRIDE environment variable, implements atomic file writing and retries for settings and credentials to prevent corruption, and adds detection for WSL/headless Linux environments to bypass native keychain usage and avoid libsecret lockups. Feedback on the changes suggests mutating process.env['GEMINI_CLI_AUTH_OVERRIDE'] directly in relaunchAppInChildProcess to ensure the override persists across subsequent iterations of the relaunch loop, as newEnv is recreated on each iteration.

Comment thread packages/cli/src/utils/relaunch.ts
…ss runner

- Set process.env.GEMINI_CLI_AUTH_OVERRIDE directly upon receiving auth-selected-type IPC message in relaunchAppInChildProcess and index.ts
- Clean up cacheCredentials conflict resolution in oauth2.ts
- Add unit test for auth override persistence in relaunch.test.ts
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness improvements to the Gemini CLI, including support for overriding the authentication type via the GEMINI_CLI_AUTH_OVERRIDE environment variable during relaunch transitions, atomic file writes and read retries for settings and credential caching to prevent file corruption, and bypassing the native keychain in WSL or headless Linux environments to avoid libsecret lockups. The review feedback highlights two critical issues: a potential race condition where the process exits before the auth-selected-type IPC message is fully flushed, and an issue where the authentication override is ignored if a stale configuration already exists on disk.

Comment thread packages/cli/src/utils/processUtils.ts
Comment thread packages/cli/src/config/settings.ts Outdated
…visor auth override

- Add 500ms fallback timeout in relaunchApp when process.send experiences channel backpressure
- Unconditionally apply GEMINI_CLI_AUTH_OVERRIDE from supervisor over stale disk settings
- Add unit tests for process.send backpressure timeout and unconditional auth override
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces support for overriding the selected authentication type via the GEMINI_CLI_AUTH_OVERRIDE environment variable during application relaunches, improves file write robustness with atomic operations and retries, and adds a fallback to file-based keychains in WSL and headless Linux environments to prevent libsecret lockups. The review feedback highlights a critical prototype pollution vulnerability in the settings merging logic (applyKeyDiff) when handling untrusted workspace configurations, and recommends refactoring the synchronous file-retry helper to use asynchronous operations to avoid blocking the event loop.

Comment thread packages/cli/src/utils/commentJson.ts
Comment thread packages/cli/src/utils/commentJson.ts Outdated
…ENOENT read retries

- Guard applyKeyDiff and preserveCommentsOnPropertyDeletion against dangerous prototype keys (__proto__, constructor, prototype)
- Immediately rethrow ENOENT in readFileWithRetry without redundant backoff retries
- Add unit tests for prototype pollution resistance and ENOENT handling in commentJson.test.ts
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 enhances the CLI's robustness and security by supporting authentication overrides via the GEMINI_CLI_AUTH_OVERRIDE environment variable during relaunch, implementing atomic file writes and read-with-retry logic in commentJson.ts, preventing prototype pollution, and bypassing the native keychain in WSL or headless Linux environments. The review feedback recommends extending the read-with-retry mechanism to oauth_creds.json in the core package to mitigate potential file contention issues on Windows/WSL.

Comment thread packages/core/src/code_assist/oauth2.ts
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness, security, and compatibility improvements. It adds support for propagating authentication overrides (GEMINI_CLI_AUTH_OVERRIDE) across application relaunches via IPC messages and environment variables. To prevent file corruption and handle transient file locks, it implements atomic file writes and read retries for settings and OAuth credentials. It also adds prototype pollution protection when updating JSON settings and automatically bypasses the native keychain in WSL or headless Linux environments to avoid libsecret lockups. Comprehensive unit tests have been added to validate these changes. No review comments were provided, so I have no additional feedback to offer.

Note: Security Review did not run due to the size of the PR.

…ory filesystem errors

- Format packages/cli/src/utils/commentJson.test.ts to satisfy Prettier

- Explicitly import vitest test primitives in processUtils.test.ts

- Handle EACCES alongside ENOENT in eval-inventory.ts for restricted path stat checks

- Safeguard empty file handling in commentJson.ts atomic updates

- Keep userOriginalSettings synchronized with supervisor auth override in settings.ts
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces support for environment variable overrides (GEMINI_CLI_AUTH_OVERRIDE) during application relaunch, bypasses the native keychain in WSL or headless Linux environments to prevent lockups, and implements safer file operations (atomic writes and retries) to avoid corruption and handle transient errors. It also adds prototype pollution protection when updating JSON settings. The review feedback highlights that the synchronous busy-wait loop used for retrying file reads in commentJson.ts blocks the Node.js event loop and wastes CPU, suggesting the use of Atomics.wait on a SharedArrayBuffer for a more efficient synchronous sleep.

Comment thread packages/cli/src/utils/commentJson.ts Outdated
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness, security, and compatibility improvements. Key changes include supporting authentication type overrides across supervisor relaunches via IPC and environment variables, implementing atomic file writes and read retries for settings and OAuth credentials to prevent corruption, protecting against prototype pollution in JSON settings updates, and bypassing native keychains in WSL/headless Linux environments to avoid libsecret lockups. I have no additional feedback to provide as no review comments were submitted.

Note: Security Review did not run due to the size of the PR.

…2E lock contention

- Utilize native fs.rmSync maxRetries and retryDelay options for atomic directory deletion

- Remove disallowed main-thread Atomics.wait and eliminate long busy-wait loops in test-rig cleanup
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness and security improvements across the codebase. Key changes include supporting an authentication type override via the GEMINI_CLI_AUTH_OVERRIDE environment variable (propagated across application relaunches via IPC), protecting against prototype pollution in JSON settings by ignoring dangerous keys, implementing atomic writes and read retries for settings and OAuth credentials, and automatically bypassing the native keychain in WSL or headless Linux environments to prevent lockups. I have no feedback to provide as there are no review comments to address.

Note: Security Review did not run due to the size of the PR.

@DavidAPierce

Copy link
Copy Markdown
Contributor

Findings

1. 🔴 Critical: CI Test Failures in keychainService.test.ts (9 tests failing across Linux, macOS, and Windows)

  • Locations: packages/core/src/services/keychainService.ts:99-126, packages/core/src/services/keychainService.test.ts:302-332
  • Issue: The CI job Testing: CI/Test (Linux) - 24.x, others (and Mac/Windows equivalents) fails with 9 test failures in keychainService.test.ts:
    FAIL src/services/keychainService.test.ts > KeychainService > isAvailable > should return true and emit telemetry on successful functional test with native keychain
    AssertionError: expected "spy" to be called at least once
    > expect(mockKeytar.setPassword).toHaveBeenCalled();
    
  • Root Cause:
    1. isHeadlessLinuxOrWsl() defines headless Linux as os.platform() === 'linux' && !process.env['DISPLAY'] && !process.env['WAYLAND_DISPLAY'].
    2. In keychainService.test.ts, the top-level beforeEach mocks os.platform() to return 'linux'.
    3. In GitHub Actions runners, neither DISPLAY nor WAYLAND_DISPLAY is set.
    4. As a result, isHeadlessLinuxOrWsl() evaluates to true for all KeychainService test runs in CI, unconditionally bypassing native keychain loading (getNativeKeychain()) and falling back to FileKeychain.
    5. The existing tests in keychainService.test.ts that verify functional probing and native keytar behavior are skipped and fail.
  • Design Concern:
    keychainService.ts already has a 2-second timeout probe (isKeychainFunctional) designed specifically to detect non-responsive Secret Service / D-Bus on headless Linux/WSL/SSH without deadlocking. Completely bypassing native keychain based purely on !DISPLAY also breaks setups where headless Linux machines have D-Bus secret service daemons configured without X11.
  • Recommendation: Rather than globally bypassing native keytar on all headless Linux environments, either rely on the existing 2s probe in isKeychainFunctional or ensure the test environment properly mocks DISPLAY / os.platform() and cleans up environment stubs via vi.unstubAllEnvs() in afterEach.

2. 🔴 Critical: Synchronous Busy-Wait Loops on the Node.js Main Thread

  • Locations: packages/cli/src/utils/commentJson.ts:41-45, packages/test-utils/src/test-rig.ts:437-440
  • Issue:
    // packages/cli/src/utils/commentJson.ts
    const end = Date.now() + 25 * attempt;
    while (Date.now() < end) {
      /* empty */
    }
    // packages/test-utils/src/test-rig.ts
    const delay = Math.min(50 * (i + 1), 500);
    const start = Date.now();
    while (Date.now() - start < delay) {
      /* busy wait */
    }
  • Impact: Running a synchronous while (Date.now() < end) busy-wait loop on Node's single-threaded event loop spins the CPU at 100%, freezes terminal I/O, halts Ink UI frame rendering, and delays timers.
  • Recommendation: In commentJson.ts, avoid blocking the main thread with tight loops. If synchronous retry is necessary, use a non-busy wait or make the file updater async. In test-rig.ts, avoid burning up to 500ms in a CPU-bound busy-wait.

3. 🟡 High: Sticky GEMINI_CLI_AUTH_OVERRIDE Prevents Future Settings Changes

  • Locations: packages/cli/src/utils/relaunch.ts:74-76, packages/cli/src/config/settings.ts:907-931
  • Issue:
    When the supervisor receives auth-selected-type, it sets:
    process.env['GEMINI_CLI_AUTH_OVERRIDE'] = msg.authType;
    newEnv['GEMINI_CLI_AUTH_OVERRIDE'] = msg.authType;
    In settings.ts, GEMINI_CLI_AUTH_OVERRIDE unconditionally overrides userSettings.security.auth.selectedType.
    Because GEMINI_CLI_AUTH_OVERRIDE is never deleted from process.env or from newEnv, it remains set permanently in the supervisor process. Any subsequent supervisor relaunch or user attempt to switch authentication types in settings.json within that session will be overridden by the stale environment variable.
  • Recommendation: Consume and delete process.env['GEMINI_CLI_AUTH_OVERRIDE'] in settings.ts once loaded (or clear it in relaunchAppInChildProcess after spawning the child) so it only applies as a one-time handoff across the supervisor relaunch.

4. 🟢 Improvement: Non-Idiomatic Callback Parameter in readOAuthCredsWithRetry

  • Location: packages/core/src/code_assist/oauth2.ts:693-696
  • Issue:
    export async function readOAuthCredsWithRetry(
      filePath: string,
      getMaxRetries: () => number = () => 3,
    ): Promise<string>
  • Recommendation: Passing a function closure getMaxRetries: () => number instead of a standard maxRetries: number = 3 (or an options object { retries?: number }) is non-standard and adds unnecessary overhead. A constant or numeric parameter is cleaner and more idiomatic.

…d auth override

- Refactor keychainService to bypass only in WSL, relying on isKeychainFunctional timeout probe for headless Linux
- Clean up test environment stubs with vi.unstubAllEnvs() in keychainService.test.ts
- Remove synchronous busy-wait CPU spinning in commentJson.ts and test-rig.ts
- Consume and delete GEMINI_CLI_AUTH_OVERRIDE once read in settings.ts and clear after spawn in relaunch supervisor
- Use numeric maxRetries parameter in readOAuthCredsWithRetry
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness and security improvements across the CLI and core packages. Key changes include handling authentication overrides across application relaunches, preventing prototype pollution in JSON settings updates, bypassing native keychains in WSL environments to avoid lockups, and implementing atomic file writes and retries for settings and OAuth credentials. Additionally, test cleanup was simplified using native retry options. The review feedback suggests refactoring the synchronous file read retry logic in commentJson.ts to use asynchronous operations (fs.promises.readFile) to prevent blocking the event loop.

Note: Security Review did not run due to the size of the PR.

Comment thread packages/cli/src/utils/commentJson.ts Outdated
- Read settings files directly via fs.readFileSync to avoid synchronous retry overhead and event loop blocking
@villahernandez-coder

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 introduces several robustness, security, and environment-compatibility improvements. Key updates include implementing atomic file writes and retry mechanisms for settings and OAuth credentials to prevent corruption, protecting against prototype pollution in JSON settings parsing, and bypassing the native keychain in WSL environments to avoid libsecret lockups. Additionally, it adds support for passing authentication overrides across application relaunches via IPC, simplifies test directory cleanup using native fs.rmSync retries, and handles EACCES errors in the evaluation inventory script. I have no feedback to provide as there are no review comments to assess.

Note: Security Review did not run due to the size of the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core Issues related to User Interface, OS Support, Core Functionality priority/p1 Important and should be addressed in the near term. size/l A large sized PR size/m A medium sized PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infinite auth loop

2 participants