Skip to content

fix: catch OSError from fcntl.flock(LOCK_UN) in google_oauth.py - #30276

Closed
annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/flock-guard-google-oauth
Closed

annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/flock-guard-google-oauth

Conversation

@annguyenNous

Copy link
Copy Markdown
Contributor

Problem

In agent/google_oauth.py, the fcntl branch of the file-lock release only catches ImportError, not OSError. If fcntl.flock(fd, fcntl.LOCK_UN) raises OSError (e.g. EBADF), the exception propagates uncaught through the outer finally block, masking the original exception from the yield body.

The msvcrt branch (line 236) already correctly catches OSError, confirming this is an oversight — the fcntl branch should mirror it.

Fix

Change except ImportError: to except (ImportError, OSError): on line 230 so both branches handle unlock failures consistently.

Context

This is the same pattern that was fixed in 5 other files (PR #20529, #23819, #23873) — agent/shell_hooks.py, tools/skill_usage.py, tools/memory_tool.py, tools/environments/file_sync.py, hermes_cli/auth.py. This instance in google_oauth.py was missed by those earlier scans because it's nested inside an except ImportError block rather than a standalone finally.

The fcntl branch of the file-lock release only caught ImportError,
not OSError. If flock(LOCK_UN) raises OSError (e.g. EBADF), the
exception propagates uncaught through the outer finally block,
masking the original exception from the yield body.

The msvcrt branch (line 236) already correctly catches OSError,
confirming this is an oversight — the fcntl branch should mirror it.

Fix: change 'except ImportError' to 'except (ImportError, OSError)'
so both branches handle unlock failures consistently.
@alt-glitch alt-glitch added type/bug Something isn't working area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have labels May 22, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Related to the fcntl.flock(LOCK_UN) guard family: #23873, #20529, #16274, #21719. This catches a missed instance in google_oauth.py. If any of those PRs merge first, this should still be applied as it covers a different file.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the missed flock(LOCK_UN) guard.

Automated hermes-sweeper review found this is superseded on current main:

  • Commit 7130d60861a9243301514bff611a9381830d59d8 removed agent/google_oauth.py entirely while removing the Google Gemini CLI / Antigravity OAuth providers (feat(providers): remove google-gemini-cli + google-antigravity OAuth providers #50492).
  • The PR's only changed path, agent/google_oauth.py:230, no longer exists on main; the associated unlock path therefore cannot run.
  • A tracked-tree search finds no remaining agent.google_oauth, GoogleOAuthError, or _credentials_lock reference.

Closing as implemented on main because the vulnerable code path has been removed.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants