diff --git a/strix/auth/codex.py b/strix/auth/codex.py index 6b5551e9..3843428b 100644 --- a/strix/auth/codex.py +++ b/strix/auth/codex.py @@ -32,6 +32,8 @@ from typing import TYPE_CHECKING, Any if TYPE_CHECKING: + from collections.abc import Iterator + from openai import AsyncOpenAI @@ -77,6 +79,36 @@ _EXPIRY_SKEW_S = 300 _refresh_lock = threading.Lock() +@contextlib.contextmanager +def _refresh_guard() -> Iterator[None]: + """Serialize a token refresh within and across Strix processes. + + The in-process lock covers concurrent agents in one process; a best-effort + file lock (``flock``) covers concurrent Strix processes sharing one login, so + two of them can't both spend the single-use refresh token and leave one with + ``invalid_grant``. Degrades to the in-process lock alone where ``flock`` is + unavailable (e.g. Windows). + """ + with _refresh_lock: + try: + import fcntl + + lock_path = store.AUTH_PATH.with_suffix(".lock") + lock_path.parent.mkdir(parents=True, exist_ok=True) + handle = lock_path.open("w") + except (ImportError, OSError): + yield + return + try: + with contextlib.suppress(OSError): + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + yield + finally: + with contextlib.suppress(OSError): + fcntl.flock(handle.fileno(), fcntl.LOCK_UN) + handle.close() + + class CodexAuthError(Exception): """A Codex auth step failed. ``code`` is a stable, machine-readable reason.""" @@ -295,16 +327,19 @@ def _near_expiry(record: dict[str, Any]) -> bool: def get_valid_token() -> tuple[str, str]: """Return ``(access_token, account_id)``, refreshing if near expiry. - Refreshes under a lock and re-reads the store after acquiring it, so that - when many agents fire at once only one refresh happens — OpenAI invalidates a - refresh token as soon as it is used, so a concurrent stampede would fail. + Refreshes under a within- and cross-process lock and re-reads the store after + acquiring it, so that when many agents (or parallel Strix processes sharing + one login) fire at once only one refresh happens — OpenAI invalidates a + refresh token as soon as it is used, so a concurrent stampede would fail. The + re-read means a caller that loses the race picks up the token the winner just + rotated instead of exchanging the now-dead one. """ record = read_record() if record is None: raise CodexAuthError("not_authenticated", "not signed in; run: strix auth login") if not _near_expiry(record): return record["access"], record["account_id"] - with _refresh_lock: + with _refresh_guard(): record = read_record() if record is None: raise CodexAuthError("not_authenticated", "not signed in; run: strix auth login") diff --git a/tests/test_codex_auth.py b/tests/test_codex_auth.py index 28682db2..5feacbd0 100644 --- a/tests/test_codex_auth.py +++ b/tests/test_codex_auth.py @@ -183,6 +183,49 @@ def test_get_valid_token_refreshes_and_persists_rotation(monkeypatch: pytest.Mon assert codex.read_record()["refresh"] == "r2" +def test_get_valid_token_uses_token_rotated_by_another_process( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Simulate a parallel Strix process rotating the token while we wait for the + # refresh guard: the pre-guard read sees the stale token, the in-guard read + # sees the winner's fresh one, so we must NOT exchange the now-dead refresh. + records = [ + { + "type": "oauth", + "provider": "codex", + "access": "stale", + "refresh": "r1", + "account_id": "acct", + "expires_at": time.time() - 10, + }, + { + "type": "oauth", + "provider": "codex", + "access": "fresh-from-other-process", + "refresh": "r2", + "account_id": "acct", + "expires_at": time.time() + 3600, + }, + ] + calls = {"n": 0} + + def _fake_read() -> dict[str, Any]: + record = records[min(calls["n"], len(records) - 1)] + calls["n"] += 1 + return record + + def _boom(_payload: dict[str, str]) -> dict[str, Any]: + msg = "must not refresh a token another process already rotated" + raise AssertionError(msg) + + monkeypatch.setattr(codex, "read_record", _fake_read) + monkeypatch.setattr(codex, "_post_form", _boom) + + access, account_id = codex.get_valid_token() + assert access == "fresh-from-other-process" + assert account_id == "acct" + + def test_get_valid_token_raises_when_not_signed_in() -> None: with pytest.raises(codex.CodexAuthError) as exc: codex.get_valid_token()