mirror of
https://github.com/usestrix/strix.git
synced 2026-08-21 18:52:47 +02:00
auth: guard token refresh across processes (Greptile P1)
The threading.Lock only serialized refreshes within one process, so two Strix processes sharing one ChatGPT login could both spend the single-use refresh token and leave one with invalid_grant. Add a best-effort cross-process file lock (flock) around the re-read/refresh/persist, layered over the in-process lock and degrading to it where flock is unavailable. The re-read inside the guard means a caller that loses the race adopts the token the winner just rotated instead of exchanging the dead one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
097f4c6b14
commit
9f54b2f144
+39
-4
@@ -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")
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user