fix(llm): harden subscription credential store and provider labeling

Addresses PR review:
- Extract the shared ~/.strix/subscription-auth.json handling into subscription_store, writing tokens owner-only (0600) from creation via os.open instead of chmod-after-write, closing the window where credentials were briefly world/group-readable.
- Serialize read-modify-write across providers and processes with a reentrant lock, so overlapping ChatGPT/Grok save/logout/refresh operations no longer clobber each other.
- Live/TUI stats label the subscription from the persisted run record (falling back to provider-aware settings), so resumed runs no longer mislabel the provider when STRIX_LLM changes.
This commit is contained in:
yoni
2026-07-29 17:39:26 +00:00
parent 5c94872186
commit 9fd11eedec
7 changed files with 247 additions and 114 deletions
+18
View File
@@ -8,6 +8,7 @@ from agents.models.openai_chatcompletions import OpenAIChatCompletionsModel
from strix.config import grok, subscription
from strix.config.models import StrixProvider
from strix.interface import utils
from strix.report import state as state_mod
@@ -51,3 +52,20 @@ def test_run_record_reports_grok_provider(monkeypatch) -> None: # type: ignore[
record = state_mod.ReportState(run_name="run-test").run_record
assert record["auth_mode"] == "subscription"
assert record["subscription_provider"] == "Grok"
def test_subscription_label_prefers_persisted_provider(monkeypatch) -> None: # type: ignore[no-untyped-def]
settings = mock.MagicMock()
settings.llm.model = "chatgpt/gpt-5.4" # current settings point at ChatGPT
monkeypatch.setattr(utils, "load_settings", lambda: settings)
# A resumed Grok run keeps its persisted provider even though settings changed.
resumed = mock.MagicMock(
run_record={"auth_mode": "subscription", "subscription_provider": "Grok"}
)
assert utils._subscription_label(resumed) == "Grok subscription"
# With no persisted provider, it derives the label from settings (not a
# hardcoded default).
fresh = mock.MagicMock(run_record={})
assert utils._subscription_label(fresh) == "ChatGPT subscription"
+55
View File
@@ -0,0 +1,55 @@
"""Shared subscription credential store: secure writes and cross-provider locking."""
from __future__ import annotations
import stat
from typing import TYPE_CHECKING
from strix.config import codex, grok, subscription_store
if TYPE_CHECKING:
from pathlib import Path
import pytest
def test_write_creates_owner_only_file(tmp_path: Path) -> None:
path = tmp_path / ".strix" / "subscription-auth.json"
subscription_store.write(path, {"grok": {"type": "oauth", "access": "a", "refresh": "r"}})
assert stat.S_IMODE(path.stat().st_mode) == 0o600
# No stray temp file is left behind.
assert not path.with_suffix(".json.tmp").exists()
def test_providers_share_store_without_clobbering(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
store = tmp_path / ".strix" / "subscription-auth.json"
monkeypatch.setattr(codex, "AUTH_PATH", store)
monkeypatch.setattr(grok, "AUTH_PATH", store)
codex.save_record({"type": "oauth", "access": "c", "refresh": "r", "account_id": "acct"})
grok.save_record({"type": "oauth", "access": "g", "refresh": "r"})
data = subscription_store.read(store)
assert data["codex"]["access"] == "c"
assert data["grok"]["access"] == "g"
# Logging one provider out leaves the other's credential intact.
grok.logout()
remaining = subscription_store.read(store)
assert "grok" not in remaining
assert remaining["codex"]["access"] == "c"
def test_guard_is_reentrant(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
store = tmp_path / ".strix" / "subscription-auth.json"
monkeypatch.setattr(grok, "AUTH_PATH", store)
# Persisting while already holding the guard must not deadlock — this mirrors
# a token refresh saving its new record inside the refresh critical section.
with subscription_store.guard(store):
grok.save_record({"type": "oauth", "access": "g", "refresh": "r"})
record = grok.read_record()
assert record is not None
assert record["access"] == "g"