mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-02 12:03:15 +00:00
fix: stop storing GitHub tokens in metadata (#1405)
* fix: stop storing GitHub tokens in metadata Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com> * fix: bound in-process GitHub token cache with 24h TTL + sweep --------- Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com> Co-authored-by: open-swe[bot] <johannes@langchain.dev>
This commit is contained in:
Brace Sproul
GitHub
parent
ce24a47cdf
commit
2070a770c2
16 changed files with 228 additions and 349 deletions
|
|
@ -98,7 +98,7 @@ Supported model IDs and per-model effort/reasoning rules live in `agent/dashboar
|
|||
|
||||
### Auth
|
||||
|
||||
- **GitHub**: dual-mode. User OAuth tokens are encrypted-at-rest in thread metadata (`agent/encryption.py`, `utils/auth.py:resolve_github_token`). When no user token is available, falls back to a GitHub App installation token (`utils/github_app.py`). The installation token is also what configures the LangSmith sandbox's GitHub proxy.
|
||||
- **GitHub**: dual-mode. User OAuth tokens are encrypted at rest in the dashboard OAuth store and cached only in process during a run (`utils/auth.py:resolve_github_token`, `utils/github_token.py`). When no user token is available, falls back to a GitHub App installation token (`utils/github_app.py`). The installation token is also what configures the LangSmith sandbox's GitHub proxy.
|
||||
- **Webhooks**: GitHub signatures verified in `utils/github_comments.py:verify_github_signature`; Slack/Linear handled in their respective utils.
|
||||
- **Dashboard / UI**: GitHub OAuth login lives in `agent/dashboard/oauth.py` and `routes.py` (`/auth/login`, `/auth/callback`, `/auth/logout`, `/me`).
|
||||
|
||||
|
|
|
|||
|
|
@ -14,12 +14,10 @@ from fastapi import HTTPException
|
|||
from langgraph_sdk.errors import InternalServerError
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from ..utils.auth import persist_encrypted_github_token
|
||||
from ..utils.thread_ops import is_thread_active, langgraph_client, queue_message_for_thread
|
||||
from .message_adapter import state_messages_to_ui
|
||||
from .options import SUPPORTED_MODEL_IDS, model_supports_effort
|
||||
from .profiles import OAUTH_TOKENS_NAMESPACE, get_profile, get_valid_access_token
|
||||
from .profiles import _get_value as get_oauth_record
|
||||
from .profiles import get_profile, get_valid_access_token
|
||||
from .user_mappings import email_for_login
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -86,17 +84,10 @@ def _parse_repo(full_name: str | None) -> dict[str, str] | None:
|
|||
return {"owner": owner, "name": name}
|
||||
|
||||
|
||||
async def _persist_dashboard_github_token(thread_id: str, login: str) -> None:
|
||||
async def _ensure_dashboard_github_token(login: str) -> None:
|
||||
token = await get_valid_access_token(login)
|
||||
if not token:
|
||||
raise HTTPException(401, "github token unavailable, re-login required")
|
||||
record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, login)
|
||||
expires_at = record.get("token_expires_at") if isinstance(record, dict) else None
|
||||
await persist_encrypted_github_token(
|
||||
thread_id,
|
||||
token,
|
||||
expires_at=expires_at if isinstance(expires_at, str) else None,
|
||||
)
|
||||
|
||||
|
||||
def _thread_owner_login(metadata: dict[str, Any]) -> str | None:
|
||||
|
|
@ -323,7 +314,7 @@ async def _start_agent_run(
|
|||
client = langgraph_client()
|
||||
await client.threads.create(thread_id=thread_id, metadata=metadata, if_exists="do_nothing")
|
||||
await client.threads.update(thread_id=thread_id, metadata=metadata)
|
||||
await _persist_dashboard_github_token(thread_id, login)
|
||||
await _ensure_dashboard_github_token(login)
|
||||
|
||||
configurable: dict[str, Any] = {
|
||||
"thread_id": thread_id,
|
||||
|
|
@ -401,7 +392,7 @@ async def send_dashboard_message(
|
|||
thread if isinstance(thread, dict) else {"thread_id": thread_id, "metadata": metadata}
|
||||
)
|
||||
|
||||
await _persist_dashboard_github_token(thread_id, login)
|
||||
await _ensure_dashboard_github_token(login)
|
||||
profile = await get_profile(login) or {}
|
||||
thread_source = _thread_source(metadata)
|
||||
configurable: dict[str, Any] = {
|
||||
|
|
|
|||
|
|
@ -64,7 +64,8 @@ from .tools import (
|
|||
)
|
||||
from .utils.agents_md import fetch_agents_md
|
||||
from .utils.auth import resolve_github_token
|
||||
from .utils.github_token import get_github_token_from_thread
|
||||
from .utils.github_app import get_github_app_installation_token_with_expiry
|
||||
from .utils.github_token import cache_github_token_for_thread, get_github_token_from_thread
|
||||
from .utils.model import DEFAULT_LLM_REASONING, make_model, provider_model_kwargs
|
||||
from .utils.sandbox_paths import aresolve_sandbox_work_dir
|
||||
|
||||
|
|
@ -622,22 +623,24 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
logger.info("No thread_id or not for execution, returning reviewer agent without sandbox")
|
||||
return create_deep_agent(system_prompt="", tools=[]).with_config(config)
|
||||
|
||||
repo_config = config["configurable"].get("repo") or {}
|
||||
github_token: str | None = None
|
||||
if config["configurable"].get("source"):
|
||||
cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread(
|
||||
thread_id
|
||||
)
|
||||
if cached_token and cached_encrypted:
|
||||
config["metadata"]["github_token_encrypted"] = cached_encrypted
|
||||
config["metadata"]["github_token_expires_at"] = cached_expires_at
|
||||
cached_token, _cached_expires_at = await get_github_token_from_thread(thread_id)
|
||||
if cached_token:
|
||||
github_token = cached_token
|
||||
else:
|
||||
_token, new_encrypted, new_expires_at = await resolve_github_token(config, thread_id)
|
||||
config["metadata"]["github_token_encrypted"] = new_encrypted
|
||||
config["metadata"]["github_token_expires_at"] = new_expires_at
|
||||
github_token = _token
|
||||
try:
|
||||
github_token, _expires_at = await resolve_github_token(config, thread_id)
|
||||
except RuntimeError:
|
||||
github_token, expires_at = await get_github_app_installation_token_with_expiry(
|
||||
repositories=[str(repo_config.get("name"))] if repo_config.get("name") else None
|
||||
)
|
||||
if github_token:
|
||||
cache_github_token_for_thread(thread_id, github_token, expires_at=expires_at)
|
||||
else:
|
||||
raise
|
||||
|
||||
repo_config = config["configurable"].get("repo") or {}
|
||||
repo_private = config["configurable"].get("repo_private")
|
||||
github_proxy_token = github_token if repo_private is False else None
|
||||
sandbox_backend = await ensure_sandbox_for_thread(
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ the reviewer's tools and webhook handlers go through.
|
|||
Why thread metadata: it survives sandbox eviction, is queryable cross-thread
|
||||
via the langgraph SDK (a future UI lists all reviewer threads by filtering on
|
||||
``metadata.kind == "reviewer"``), and matches existing patterns the codebase
|
||||
already uses for ``sandbox_id``, ``github_token_encrypted``, etc.
|
||||
already uses for durable non-secret run state like ``sandbox_id``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
|
|||
|
|
@ -410,9 +410,7 @@ async def get_agent(config: RunnableConfig) -> Pregel:
|
|||
tools=[],
|
||||
).with_config(config)
|
||||
|
||||
github_token, new_encrypted, new_expires_at = await resolve_github_token(config, thread_id)
|
||||
config["metadata"]["github_token_encrypted"] = new_encrypted
|
||||
config["metadata"]["github_token_expires_at"] = new_expires_at
|
||||
github_token, _expires_at = await resolve_github_token(config, thread_id)
|
||||
triggering_user_identity = await asyncio.to_thread(
|
||||
resolve_triggering_user_identity, config, github_token
|
||||
)
|
||||
|
|
|
|||
|
|
@ -13,9 +13,8 @@ from langgraph.config import get_config
|
|||
from langgraph.graph.state import RunnableConfig
|
||||
from langgraph_sdk import get_client
|
||||
|
||||
from ..encryption import encrypt_token
|
||||
from .github_app import get_github_app_installation_token_with_expiry
|
||||
from .github_token import get_github_token_from_thread
|
||||
from .github_token import cache_github_token_for_thread, get_github_token_from_thread
|
||||
from .linear import comment_on_linear_issue
|
||||
from .slack import post_slack_thread_reply
|
||||
|
||||
|
|
@ -287,24 +286,18 @@ async def leave_failure_comment(
|
|||
raise ValueError(f"Unknown source: {source}")
|
||||
|
||||
|
||||
async def persist_encrypted_github_token(
|
||||
def _cache_resolved_github_token(
|
||||
thread_id: str, token: str, expires_at: str | None = None
|
||||
) -> str:
|
||||
"""Encrypt a GitHub token and store it (and its expiry) on the thread metadata."""
|
||||
encrypted = encrypt_token(token)
|
||||
metadata: dict[str, Any] = {
|
||||
"github_token_encrypted": encrypted,
|
||||
"github_token_expires_at": expires_at,
|
||||
}
|
||||
await client.threads.update(thread_id=thread_id, metadata=metadata)
|
||||
return encrypted
|
||||
) -> tuple[str, str | None]:
|
||||
cache_github_token_for_thread(thread_id, token, expires_at=expires_at)
|
||||
return token, expires_at
|
||||
|
||||
|
||||
async def save_encrypted_token_from_email(
|
||||
async def resolve_token_from_email(
|
||||
email: str | None,
|
||||
source: str,
|
||||
) -> tuple[str, str, str | None]:
|
||||
"""Resolve, encrypt, and store a GitHub token based on user email."""
|
||||
) -> tuple[str, str | None]:
|
||||
"""Resolve and cache a GitHub token based on user email."""
|
||||
config = get_config()
|
||||
configurable = config.get("configurable", {})
|
||||
thread_id = configurable.get("thread_id")
|
||||
|
|
@ -363,23 +356,15 @@ async def save_encrypted_token_from_email(
|
|||
raise ValueError(f"No token found: {error}")
|
||||
|
||||
expires_at = auth_result.get("expires_at") if isinstance(auth_result, dict) else None
|
||||
encrypted = await persist_encrypted_github_token(thread_id, token, expires_at=expires_at)
|
||||
return token, encrypted, expires_at
|
||||
return _cache_resolved_github_token(
|
||||
thread_id, token, expires_at=expires_at if isinstance(expires_at, str) else None
|
||||
)
|
||||
|
||||
|
||||
async def _resolve_dashboard_user_token(
|
||||
thread_id: str, github_login: str
|
||||
) -> tuple[str, str, str | None] | None:
|
||||
"""Resolve a per-user GitHub token from the dashboard OAuth store.
|
||||
|
||||
Returns the ``(token, encrypted, expires_at)`` tuple, or ``None`` when the
|
||||
user has no valid token (never linked, or expired/revoked beyond refresh).
|
||||
|
||||
The thread-metadata token cache is intentionally NOT consulted here: Slack
|
||||
thread ids are shared across everyone in a conversation, so a cached token
|
||||
from a prior triggering user would impersonate the current ``github_login``.
|
||||
We always resolve by login from the dashboard store instead.
|
||||
"""
|
||||
) -> tuple[str, str | None] | None:
|
||||
"""Resolve a per-user GitHub token from the dashboard OAuth store."""
|
||||
login = github_login.strip()
|
||||
if not login:
|
||||
raise ValueError("missing github_login")
|
||||
|
|
@ -392,13 +377,13 @@ async def _resolve_dashboard_user_token(
|
|||
return None
|
||||
record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, login)
|
||||
expires_at = record.get("token_expires_at") if isinstance(record, dict) else None
|
||||
expires_at = expires_at if isinstance(expires_at, str) else None
|
||||
encrypted = await persist_encrypted_github_token(thread_id, token, expires_at=expires_at)
|
||||
return token, encrypted, expires_at
|
||||
return _cache_resolved_github_token(
|
||||
thread_id, token, expires_at=expires_at if isinstance(expires_at, str) else None
|
||||
)
|
||||
|
||||
|
||||
async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str, str | None]:
|
||||
"""Get a GitHub App installation token and persist it for the thread."""
|
||||
async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str | None]:
|
||||
"""Get a GitHub App installation token and cache it for the thread."""
|
||||
bot_token, expires_at = await get_github_app_installation_token_with_expiry()
|
||||
if not bot_token:
|
||||
raise RuntimeError(
|
||||
|
|
@ -409,13 +394,10 @@ async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str, str
|
|||
logger.info(
|
||||
"Using GitHub App installation token for thread %s (bot-token-only mode)", thread_id
|
||||
)
|
||||
encrypted = await persist_encrypted_github_token(thread_id, bot_token, expires_at=expires_at)
|
||||
return bot_token, encrypted, expires_at
|
||||
return _cache_resolved_github_token(thread_id, bot_token, expires_at=expires_at)
|
||||
|
||||
|
||||
async def resolve_github_token(
|
||||
config: RunnableConfig, thread_id: str
|
||||
) -> tuple[str, str, str | None]:
|
||||
async def resolve_github_token(config: RunnableConfig, thread_id: str) -> tuple[str, str | None]:
|
||||
"""Resolve a GitHub token from the run config based on the source.
|
||||
|
||||
Routes to the correct auth method depending on whether the run was
|
||||
|
|
@ -425,10 +407,6 @@ async def resolve_github_token(
|
|||
X_SERVICE_AUTH_JWT_SECRET), the GitHub App installation token is used
|
||||
for all operations instead of per-user OAuth tokens.
|
||||
|
||||
Returns:
|
||||
(github_token, new_encrypted, expires_at) tuple. ``expires_at`` is the
|
||||
ISO-8601 expiry persisted alongside the ciphertext, or ``None``.
|
||||
|
||||
Raises:
|
||||
RuntimeError: If source is missing or token resolution fails.
|
||||
"""
|
||||
|
|
@ -462,18 +440,16 @@ async def resolve_github_token(
|
|||
|
||||
try:
|
||||
if source == "github":
|
||||
cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread(
|
||||
thread_id
|
||||
)
|
||||
if cached_token and cached_encrypted:
|
||||
return cached_token, cached_encrypted, cached_expires_at
|
||||
cached_token, cached_expires_at = await get_github_token_from_thread(thread_id)
|
||||
if cached_token:
|
||||
return cached_token, cached_expires_at
|
||||
from ..dashboard.user_mappings import email_for_login
|
||||
|
||||
email = await email_for_login(github_login)
|
||||
if not email:
|
||||
raise ValueError(f"No email mapping found for GitHub user '{github_login}'")
|
||||
return await save_encrypted_token_from_email(email, source)
|
||||
return await save_encrypted_token_from_email(configurable.get("user_email"), source)
|
||||
return await resolve_token_from_email(email, source)
|
||||
return await resolve_token_from_email(configurable.get("user_email"), source)
|
||||
except ValueError as exc:
|
||||
logger.error("GitHub auth failed for thread %s: %s", thread_id, str(exc))
|
||||
raise RuntimeError(str(exc)) from exc
|
||||
|
|
|
|||
|
|
@ -4,51 +4,40 @@ from __future__ import annotations
|
|||
|
||||
import logging
|
||||
from collections.abc import Mapping
|
||||
from datetime import UTC, datetime
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
|
||||
from langgraph.config import get_config
|
||||
from langgraph_sdk import get_client
|
||||
from langgraph_sdk.errors import NotFoundError
|
||||
|
||||
from ..encryption import decrypt_token
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_GITHUB_TOKEN_METADATA_KEY = "github_token_encrypted"
|
||||
_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY = "github_token_expires_at"
|
||||
# Treat tokens with <= this many seconds remaining as expired so we re-auth
|
||||
# before kicking off long agent runs.
|
||||
_GITHUB_TOKEN_EXPIRY_SKEW_SECONDS = 60
|
||||
# Hard cap on how long an entry stays cached regardless of the token's own
|
||||
# expiry, so entries for threads that are never read again don't accumulate.
|
||||
_GITHUB_TOKEN_MAX_TTL = timedelta(hours=24)
|
||||
# thread_id -> (token, token_expires_at, cached_at)
|
||||
_GITHUB_TOKEN_CACHE: dict[str, tuple[str, str | None, datetime]] = {}
|
||||
|
||||
|
||||
class GitHubAuthError(Exception):
|
||||
"""Raised when a GitHub call returns 401, signalling a stale/revoked token."""
|
||||
|
||||
|
||||
# Treat tokens with <= this many seconds remaining as expired so we re-auth
|
||||
# before kicking off long agent runs.
|
||||
_GITHUB_TOKEN_EXPIRY_SKEW_SECONDS = 60
|
||||
|
||||
client = get_client()
|
||||
|
||||
|
||||
def _read_encrypted_github_token(metadata: dict[str, Any]) -> str | None:
|
||||
encrypted_token = metadata.get(_GITHUB_TOKEN_METADATA_KEY)
|
||||
return encrypted_token if isinstance(encrypted_token, str) and encrypted_token else None
|
||||
|
||||
|
||||
def _decrypt_github_token(encrypted_token: str | None) -> str | None:
|
||||
if not encrypted_token:
|
||||
return None
|
||||
|
||||
return decrypt_token(encrypted_token)
|
||||
def cache_github_token_for_thread(
|
||||
thread_id: str, token: str, expires_at: str | None = None
|
||||
) -> None:
|
||||
"""Cache a GitHub token in process for the current thread."""
|
||||
if not thread_id or not token:
|
||||
return
|
||||
now = datetime.now(UTC)
|
||||
_GITHUB_TOKEN_CACHE[thread_id] = (token, expires_at, now)
|
||||
_evict_expired(now=now)
|
||||
|
||||
|
||||
def _is_expired(expires_at: Any, *, now: datetime | None = None) -> bool:
|
||||
"""Return True when ``expires_at`` is past (or close to) ``now``.
|
||||
|
||||
Accepts ISO-8601 strings (with or without trailing Z) and unix timestamps.
|
||||
Unparseable values are treated as not expired so we don't break callers
|
||||
that haven't started persisting an expiry yet.
|
||||
"""
|
||||
"""Return True when ``expires_at`` is past (or close to) ``now``."""
|
||||
if expires_at is None:
|
||||
return False
|
||||
|
||||
|
|
@ -78,76 +67,59 @@ def _is_expired(expires_at: Any, *, now: datetime | None = None) -> bool:
|
|||
return (parsed - current).total_seconds() <= _GITHUB_TOKEN_EXPIRY_SKEW_SECONDS
|
||||
|
||||
|
||||
def _read_token_if_fresh(metadata: dict[str, Any]) -> str | None:
|
||||
"""Decrypt the cached token only if it has not expired."""
|
||||
encrypted = _read_encrypted_github_token(metadata)
|
||||
if not encrypted:
|
||||
def _entry_expired(expires_at: str | None, cached_at: datetime, *, now: datetime) -> bool:
|
||||
"""Expired when past the token's own expiry or the 24h cache cap."""
|
||||
if now - cached_at >= _GITHUB_TOKEN_MAX_TTL:
|
||||
return True
|
||||
return _is_expired(expires_at, now=now)
|
||||
|
||||
|
||||
def _evict_expired(*, now: datetime | None = None) -> None:
|
||||
current = now or datetime.now(UTC)
|
||||
stale = [
|
||||
tid
|
||||
for tid, (_token, expires_at, cached_at) in _GITHUB_TOKEN_CACHE.items()
|
||||
if _entry_expired(expires_at, cached_at, now=current)
|
||||
]
|
||||
for tid in stale:
|
||||
_GITHUB_TOKEN_CACHE.pop(tid, None)
|
||||
|
||||
|
||||
def _cached_token_if_fresh(thread_id: str | None) -> tuple[str | None, str | None]:
|
||||
if not thread_id:
|
||||
return None, None
|
||||
cached = _GITHUB_TOKEN_CACHE.get(thread_id)
|
||||
if not cached:
|
||||
return None, None
|
||||
token, expires_at, cached_at = cached
|
||||
if _entry_expired(expires_at, cached_at, now=datetime.now(UTC)):
|
||||
_GITHUB_TOKEN_CACHE.pop(thread_id, None)
|
||||
logger.info("Cached GitHub token for thread %s has expired; re-resolving", thread_id)
|
||||
return None, None
|
||||
return token, expires_at
|
||||
|
||||
|
||||
def _thread_id_from_config(run_config: Mapping[str, Any]) -> str | None:
|
||||
configurable = run_config.get("configurable", {})
|
||||
if not isinstance(configurable, Mapping):
|
||||
return None
|
||||
if _is_expired(metadata.get(_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY)):
|
||||
return None
|
||||
return _decrypt_github_token(encrypted)
|
||||
thread_id = configurable.get("thread_id")
|
||||
return thread_id if isinstance(thread_id, str) and thread_id else None
|
||||
|
||||
|
||||
def get_github_token(run_config: Mapping[str, Any] | None = None) -> str | None:
|
||||
"""Resolve a GitHub token from run metadata.
|
||||
|
||||
Pass ``run_config`` when LangGraph runnable config is already available (e.g. after
|
||||
``get_config()`` in callers). Omit to read from ``get_config()`` (required runnable
|
||||
context). Returns ``None`` for tokens whose ``github_token_expires_at`` is past.
|
||||
"""
|
||||
"""Resolve the current thread's GitHub token from process memory."""
|
||||
resolved = run_config if run_config is not None else get_config()
|
||||
return _read_token_if_fresh(resolved.get("metadata", {}))
|
||||
token, _expires_at = _cached_token_if_fresh(_thread_id_from_config(resolved))
|
||||
return token
|
||||
|
||||
|
||||
async def get_github_token_from_thread(
|
||||
thread_id: str,
|
||||
) -> tuple[str | None, str | None, str | None]:
|
||||
"""Resolve a GitHub token from LangGraph thread metadata.
|
||||
|
||||
Returns ``(None, None, None)`` when no token is cached or when the cached
|
||||
token's ``github_token_expires_at`` has elapsed — callers must treat the
|
||||
cache as missing in that case and re-resolve. On a fresh hit, returns the
|
||||
decrypted token, its ciphertext, and the persisted expiry (or ``None``).
|
||||
"""
|
||||
try:
|
||||
thread = await client.threads.get(thread_id)
|
||||
except NotFoundError:
|
||||
logger.debug("Thread %s not found while looking up GitHub token", thread_id)
|
||||
return None, None, None
|
||||
except Exception: # noqa: BLE001
|
||||
logger.exception("Failed to fetch thread metadata for %s", thread_id)
|
||||
return None, None, None
|
||||
|
||||
metadata = (thread or {}).get("metadata", {})
|
||||
encrypted_token = _read_encrypted_github_token(metadata)
|
||||
if not encrypted_token:
|
||||
return None, None, None
|
||||
expires_at_raw = metadata.get(_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY)
|
||||
if _is_expired(expires_at_raw):
|
||||
logger.info("Cached GitHub token for thread %s has expired; re-resolving", thread_id)
|
||||
return None, None, None
|
||||
|
||||
token = _decrypt_github_token(encrypted_token)
|
||||
if token:
|
||||
logger.info("Found GitHub token in thread metadata for thread %s", thread_id)
|
||||
expires_at = expires_at_raw if isinstance(expires_at_raw, str) else None
|
||||
return token, encrypted_token, expires_at
|
||||
async def get_github_token_from_thread(thread_id: str) -> tuple[str | None, str | None]:
|
||||
"""Resolve the current process's cached GitHub token for a thread."""
|
||||
return _cached_token_if_fresh(thread_id)
|
||||
|
||||
|
||||
async def invalidate_cached_github_token(thread_id: str) -> None:
|
||||
"""Clear a cached GitHub token from thread metadata.
|
||||
|
||||
Called when a downstream GitHub API call returns 401, so the next run
|
||||
re-resolves a fresh token instead of replaying the revoked ciphertext.
|
||||
"""
|
||||
try:
|
||||
await client.threads.update(
|
||||
thread_id=thread_id,
|
||||
metadata={
|
||||
_GITHUB_TOKEN_METADATA_KEY: None,
|
||||
_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY: None,
|
||||
},
|
||||
)
|
||||
logger.info("Invalidated cached GitHub token for thread %s", thread_id)
|
||||
except Exception:
|
||||
logger.exception("Failed to invalidate cached GitHub token for thread %s", thread_id)
|
||||
"""Clear a cached GitHub token for a thread."""
|
||||
_GITHUB_TOKEN_CACHE.pop(thread_id, None)
|
||||
logger.info("Invalidated cached GitHub token for thread %s", thread_id)
|
||||
|
|
|
|||
|
|
@ -52,7 +52,6 @@ from .reviewer_publish import fetch_pr_review_threads
|
|||
from .reviewer_reconcile import reconcile_findings_with_review_threads
|
||||
from .utils.auth import (
|
||||
is_bot_token_only_mode,
|
||||
persist_encrypted_github_token,
|
||||
resolve_github_token_from_email,
|
||||
)
|
||||
from .utils.comments import get_recent_comments
|
||||
|
|
@ -74,7 +73,11 @@ from .utils.github_comments import (
|
|||
verify_github_signature,
|
||||
)
|
||||
from .utils.github_org_membership import INTERNAL_BOT_LOGINS, is_user_active_org_member
|
||||
from .utils.github_token import get_github_token_from_thread, invalidate_cached_github_token
|
||||
from .utils.github_token import (
|
||||
cache_github_token_for_thread,
|
||||
get_github_token_from_thread,
|
||||
invalidate_cached_github_token,
|
||||
)
|
||||
from .utils.linear import post_linear_trace_comment
|
||||
from .utils.linear_team_repo_map import LINEAR_TEAM_TO_REPO
|
||||
from .utils.multimodal import dedupe_urls, extract_image_urls, fetch_image_block
|
||||
|
|
@ -1725,11 +1728,7 @@ async def trigger_pr_review_from_ref(
|
|||
if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client):
|
||||
return {"success": False, "error": "Could not create reviewer thread"}
|
||||
|
||||
try:
|
||||
await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
except Exception:
|
||||
logger.warning("Could not persist bot token for reviewer thread %s", thread_id)
|
||||
return {"success": False, "error": "Could not persist reviewer token"}
|
||||
cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
|
||||
pr_meta: ReviewerPRMeta = {
|
||||
"owner": pr_ref.owner,
|
||||
|
|
@ -1919,11 +1918,7 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou
|
|||
if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client):
|
||||
return
|
||||
|
||||
try:
|
||||
await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
except Exception:
|
||||
logger.warning("Could not persist bot token for reviewer thread %s", thread_id)
|
||||
return
|
||||
cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
|
||||
await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True, head_sha=head_sha)
|
||||
|
||||
|
|
@ -1986,9 +1981,9 @@ async def process_github_pr_ready(payload: dict[str, Any]) -> None:
|
|||
author_login or "<unknown>",
|
||||
)
|
||||
return
|
||||
# Use source="github" so the auth resolver finds the bot token persisted on
|
||||
# the thread; "github_auto" would fall through to the email-based path,
|
||||
# which has no user_email to route on for webhook-triggered runs.
|
||||
# Use source="github" so the reviewer resolver can use the GitHub App token;
|
||||
# "github_auto" would fall through to the email-based path, which has no
|
||||
# user_email to route on for webhook-triggered runs.
|
||||
await _dispatch_first_review_from_pr_payload(payload, source="github")
|
||||
|
||||
|
||||
|
|
@ -2267,11 +2262,7 @@ async def process_github_push_event(payload: dict[str, Any]) -> None:
|
|||
langgraph_client = get_client(url=LANGGRAPH_URL)
|
||||
if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client):
|
||||
return
|
||||
try:
|
||||
await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
except Exception:
|
||||
logger.warning("Could not persist bot token for reviewer thread %s", thread_id)
|
||||
return
|
||||
cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
try:
|
||||
threads = await fetch_pr_review_threads(
|
||||
owner=repo_config["owner"],
|
||||
|
|
@ -2341,24 +2332,20 @@ async def _refresh_thread_github_token_after_401(thread_id: str, email: str) ->
|
|||
|
||||
|
||||
async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str | None:
|
||||
"""Resolve and persist a GitHub token for a thread when available.
|
||||
"""Resolve and cache a GitHub token for a thread when available.
|
||||
|
||||
Skips the cached ciphertext when its ``github_token_expires_at`` is past.
|
||||
In bot-token-only mode, returns a fresh GitHub App installation token
|
||||
instead of resolving per-user OAuth tokens.
|
||||
"""
|
||||
if is_bot_token_only_mode():
|
||||
bot_token, expires_at = await get_github_app_installation_token_with_expiry()
|
||||
if bot_token:
|
||||
try:
|
||||
await persist_encrypted_github_token(thread_id, bot_token, expires_at=expires_at)
|
||||
except Exception:
|
||||
logger.warning("Could not persist bot token for thread %s", thread_id)
|
||||
cache_github_token_for_thread(thread_id, bot_token, expires_at=expires_at)
|
||||
return bot_token
|
||||
logger.warning("Bot-token-only mode but GitHub App token unavailable")
|
||||
return None
|
||||
|
||||
github_token, _encrypted_token, _expires_at = await get_github_token_from_thread(thread_id)
|
||||
github_token, _expires_at = await get_github_token_from_thread(thread_id)
|
||||
if github_token:
|
||||
return github_token
|
||||
|
||||
|
|
@ -2367,12 +2354,10 @@ async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str
|
|||
if not github_token:
|
||||
return None
|
||||
|
||||
try:
|
||||
await persist_encrypted_github_token(
|
||||
thread_id, github_token, expires_at=auth_result.get("expires_at")
|
||||
)
|
||||
except Exception:
|
||||
logger.warning("Could not persist GitHub token for thread %s", thread_id)
|
||||
expires_at = auth_result.get("expires_at")
|
||||
cache_github_token_for_thread(
|
||||
thread_id, github_token, expires_at=expires_at if isinstance(expires_at, str) else None
|
||||
)
|
||||
return github_token
|
||||
|
||||
|
||||
|
|
@ -2586,11 +2571,7 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None:
|
|||
)
|
||||
if not app_token:
|
||||
return
|
||||
try:
|
||||
await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
except Exception:
|
||||
logger.warning("Could not persist bot token for reviewer thread %s", thread_id)
|
||||
return
|
||||
cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at)
|
||||
|
||||
threads = await fetch_pr_review_threads(
|
||||
owner=repo_config["owner"],
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ async def test_agent_uses_profile_subagent_model_override() -> None:
|
|||
patch(
|
||||
"agent.server.resolve_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("ghp", "enc", None),
|
||||
return_value=("ghp", None),
|
||||
),
|
||||
patch("agent.server.resolve_triggering_user_identity", return_value=None),
|
||||
patch(
|
||||
|
|
@ -112,7 +112,7 @@ async def test_agent_subagent_inherits_profile_model_override_without_explicit_p
|
|||
patch(
|
||||
"agent.server.resolve_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("ghp", "enc", None),
|
||||
return_value=("ghp", None),
|
||||
),
|
||||
patch("agent.server.resolve_triggering_user_identity", return_value=None),
|
||||
patch(
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ def _stub_dashboard_store(
|
|||
*,
|
||||
token: str | None,
|
||||
expires_at: str | None = "2099-01-01T00:00:00Z",
|
||||
cached: tuple[str | None, str | None, str | None] = (None, None, None),
|
||||
cached: tuple[str | None, str | None] = (None, None),
|
||||
) -> None:
|
||||
from agent.dashboard import profiles
|
||||
|
||||
|
|
@ -73,11 +73,7 @@ def _stub_dashboard_store(
|
|||
async def fake_get_value(namespace, key):
|
||||
return {"token_expires_at": expires_at}
|
||||
|
||||
async def fake_persist(thread_id: str, tok: str, expires_at: str | None = None):
|
||||
return "enc"
|
||||
|
||||
monkeypatch.setattr(auth, "get_github_token_from_thread", fake_get_from_thread)
|
||||
monkeypatch.setattr(auth, "persist_encrypted_github_token", fake_persist)
|
||||
monkeypatch.setattr(profiles, "get_valid_access_token", fake_get_valid)
|
||||
monkeypatch.setattr(profiles, "_get_value", fake_get_value)
|
||||
|
||||
|
|
@ -88,10 +84,9 @@ def test_resolve_github_token_slack_uses_dashboard_store(
|
|||
_stub_dashboard_store(monkeypatch, token="user-tok")
|
||||
monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False)
|
||||
|
||||
token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
token, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
|
||||
assert token == "user-tok"
|
||||
assert encrypted == "enc"
|
||||
assert expires_at == "2099-01-01T00:00:00Z"
|
||||
|
||||
|
||||
|
|
@ -103,11 +98,11 @@ def test_resolve_github_token_slack_ignores_stale_thread_cache(
|
|||
_stub_dashboard_store(
|
||||
monkeypatch,
|
||||
token="bob-token",
|
||||
cached=("alice-token", "alice-enc", "2099-01-01T00:00:00Z"),
|
||||
cached=("alice-token", "2099-01-01T00:00:00Z"),
|
||||
)
|
||||
monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False)
|
||||
|
||||
token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
token, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
|
||||
assert token == "bob-token"
|
||||
|
||||
|
|
@ -133,7 +128,7 @@ def test_resolve_github_token_per_user_wins_over_bot_only_mode(
|
|||
|
||||
monkeypatch.setattr(auth, "_resolve_bot_installation_token", fail_bot)
|
||||
|
||||
token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
token, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
assert token == "user-tok"
|
||||
|
||||
|
||||
|
|
@ -144,12 +139,12 @@ def test_resolve_github_token_slack_no_token_falls_back_to_bot_in_bot_only_mode(
|
|||
monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True)
|
||||
|
||||
async def fake_bot(thread_id: str):
|
||||
return ("bot-tok", "bot-enc", None)
|
||||
return ("bot-tok", None)
|
||||
|
||||
monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot)
|
||||
|
||||
token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
assert (token, encrypted, expires_at) == ("bot-tok", "bot-enc", None)
|
||||
token, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1"))
|
||||
assert (token, expires_at) == ("bot-tok", None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("source", ["github", "linear"])
|
||||
|
|
@ -159,10 +154,10 @@ def test_resolve_github_token_bot_only_mode_non_slack_uses_bot(
|
|||
monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True)
|
||||
|
||||
async def fake_bot(thread_id: str):
|
||||
return ("bot-tok", "bot-enc", None)
|
||||
return ("bot-tok", None)
|
||||
|
||||
monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot)
|
||||
|
||||
config = {"configurable": {"source": source, "github_login": "octo", "thread_id": "t1"}}
|
||||
token, _, _ = asyncio.run(auth.resolve_github_token(config, "t1"))
|
||||
token, _ = asyncio.run(auth.resolve_github_token(config, "t1"))
|
||||
assert token == "bot-tok"
|
||||
|
|
|
|||
|
|
@ -305,11 +305,8 @@ def test_process_github_review_finding_reply_uses_rereview_config(monkeypatch) -
|
|||
async def fake_get_token_with_expiry() -> tuple[str, str]:
|
||||
return "app-token", "2026-01-01T00:00:00Z"
|
||||
|
||||
async def fake_persist_token(
|
||||
thread_id: str, token: str, *, expires_at: str | None = None
|
||||
) -> str:
|
||||
captured["persist"] = (thread_id, token, expires_at)
|
||||
return "encrypted"
|
||||
def fake_cache_token(thread_id: str, token: str, *, expires_at: str | None = None) -> None:
|
||||
captured["cache"] = (thread_id, token, expires_at)
|
||||
|
||||
async def fake_fetch_threads(**_kwargs: object) -> list[dict[str, object]]:
|
||||
return []
|
||||
|
|
@ -346,7 +343,7 @@ def test_process_github_review_finding_reply_uses_rereview_config(monkeypatch) -
|
|||
monkeypatch.setattr(
|
||||
webapp, "get_github_app_installation_token_with_expiry", fake_get_token_with_expiry
|
||||
)
|
||||
monkeypatch.setattr(webapp, "persist_encrypted_github_token", fake_persist_token)
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_token)
|
||||
monkeypatch.setattr(webapp, "fetch_pr_review_threads", fake_fetch_threads)
|
||||
monkeypatch.setattr(webapp, "reconcile_findings_with_review_threads", fake_reconcile)
|
||||
monkeypatch.setattr(webapp, "list_reviewer_findings", fake_list_findings)
|
||||
|
|
@ -393,11 +390,8 @@ def test_process_github_review_finding_reply_queues_reply_body_when_active(monke
|
|||
async def fake_get_token_with_expiry() -> tuple[str, str]:
|
||||
return "app-token", "2026-01-01T00:00:00Z"
|
||||
|
||||
async def fake_persist_token(
|
||||
_thread_id: str, _token: str, *, expires_at: str | None = None
|
||||
) -> str:
|
||||
def fake_cache_token(_thread_id: str, _token: str, *, expires_at: str | None = None) -> None:
|
||||
captured["expires_at"] = expires_at
|
||||
return "encrypted"
|
||||
|
||||
async def fake_fetch_threads(**_kwargs: object) -> list[dict[str, object]]:
|
||||
return []
|
||||
|
|
@ -427,7 +421,7 @@ def test_process_github_review_finding_reply_queues_reply_body_when_active(monke
|
|||
monkeypatch.setattr(
|
||||
webapp, "get_github_app_installation_token_with_expiry", fake_get_token_with_expiry
|
||||
)
|
||||
monkeypatch.setattr(webapp, "persist_encrypted_github_token", fake_persist_token)
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_token)
|
||||
monkeypatch.setattr(webapp, "fetch_pr_review_threads", fake_fetch_threads)
|
||||
monkeypatch.setattr(webapp, "reconcile_findings_with_review_threads", fake_reconcile)
|
||||
monkeypatch.setattr(webapp, "list_reviewer_findings", fake_list_findings)
|
||||
|
|
@ -739,13 +733,12 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None:
|
|||
async def fake_get_github_app_installation_token_with_expiry() -> tuple[str | None, str | None]:
|
||||
return "app-token", None
|
||||
|
||||
async def fake_persist_encrypted_github_token(
|
||||
def fake_cache_github_token(
|
||||
thread_id: str, token: str, *, expires_at: str | None = None
|
||||
) -> str:
|
||||
captured["persist_thread_id"] = thread_id
|
||||
captured["persist_token"] = token
|
||||
captured["persist_expires_at"] = expires_at
|
||||
return "encrypted-token"
|
||||
) -> None:
|
||||
captured["cache_thread_id"] = thread_id
|
||||
captured["cache_token"] = token
|
||||
captured["cache_expires_at"] = expires_at
|
||||
|
||||
async def fake_is_thread_active(thread_id: str) -> bool:
|
||||
captured["active_thread_id"] = thread_id
|
||||
|
|
@ -774,9 +767,7 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None:
|
|||
"get_github_app_installation_token_with_expiry",
|
||||
fake_get_github_app_installation_token_with_expiry,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
webapp, "persist_encrypted_github_token", fake_persist_encrypted_github_token
|
||||
)
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_github_token)
|
||||
monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active)
|
||||
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", fake_set_reviewer_thread_metadata)
|
||||
monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClient())
|
||||
|
|
@ -806,8 +797,8 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None:
|
|||
"thread_id": captured["thread_id"],
|
||||
"if_exists": "do_nothing",
|
||||
}
|
||||
assert captured["persist_token"] == "app-token"
|
||||
assert captured["persist_thread_id"] == captured["thread_id"]
|
||||
assert captured["cache_token"] == "app-token"
|
||||
assert captured["cache_thread_id"] == captured["thread_id"]
|
||||
assert "https://github.com/langchain-ai/open-swe/pull/1244" in prompt
|
||||
assert "Base SHA: base-sha" in prompt
|
||||
assert "Head SHA: head-sha" in prompt
|
||||
|
|
@ -836,13 +827,12 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None:
|
|||
"head": {"sha": "head-sha", "ref": "feature-branch"},
|
||||
}
|
||||
|
||||
async def fake_persist_encrypted_github_token(
|
||||
def fake_cache_github_token(
|
||||
thread_id: str, token: str, *, expires_at: str | None = None
|
||||
) -> str:
|
||||
captured["persist_thread_id"] = thread_id
|
||||
captured["persist_token"] = token
|
||||
captured["persist_expires_at"] = expires_at
|
||||
return "encrypted-token"
|
||||
) -> None:
|
||||
captured["cache_thread_id"] = thread_id
|
||||
captured["cache_token"] = token
|
||||
captured["cache_expires_at"] = expires_at
|
||||
|
||||
async def fake_is_thread_active(thread_id: str) -> bool:
|
||||
captured["active_thread_id"] = thread_id
|
||||
|
|
@ -875,9 +865,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None:
|
|||
fake_get_github_app_installation_token_with_expiry,
|
||||
)
|
||||
monkeypatch.setattr(webapp, "fetch_github_pr_metadata", fake_fetch_github_pr_metadata)
|
||||
monkeypatch.setattr(
|
||||
webapp, "persist_encrypted_github_token", fake_persist_encrypted_github_token
|
||||
)
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_github_token)
|
||||
monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active)
|
||||
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", fake_set_reviewer_thread_metadata)
|
||||
monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClient())
|
||||
|
|
@ -906,7 +894,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None:
|
|||
"if_exists": "do_nothing",
|
||||
}
|
||||
assert captured["metadata_token"] == "app-token"
|
||||
assert captured["persist_token"] == "app-token"
|
||||
assert captured["cache_token"] == "app-token"
|
||||
assert "Base SHA: base-sha" in prompt
|
||||
assert "Head SHA: head-sha" in prompt
|
||||
assert config["source"] == "slack"
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
Covers:
|
||||
- (a) expired-cache reads return None / fall through to re-auth
|
||||
- (b) 401 on a downstream GitHub call invalidates the cached ciphertext and
|
||||
- (b) 401 on a downstream GitHub call invalidates the cached token and
|
||||
triggers a fresh resolve in the webapp
|
||||
- (c) ``publish_review`` invalidates the cached token and returns a clean
|
||||
failure when GitHub responds 401
|
||||
|
|
@ -13,25 +13,16 @@ from __future__ import annotations
|
|||
import asyncio
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from agent.utils import github_comments, github_token
|
||||
|
||||
_TEST_FERNET_KEY = "GMI8FNqVnhFzVfKDUTpGAUq8a2cm14kU0SyXzMTM4Yc="
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _set_encryption_key(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("TOKEN_ENCRYPTION_KEY", _TEST_FERNET_KEY)
|
||||
|
||||
|
||||
def _encrypted(token: str) -> str:
|
||||
from agent.encryption import encrypt_token
|
||||
|
||||
return encrypt_token(token)
|
||||
def _clear_token_cache() -> None:
|
||||
github_token._GITHUB_TOKEN_CACHE.clear()
|
||||
|
||||
|
||||
# (a) expired-cache reads -----------------------------------------------------
|
||||
|
|
@ -57,73 +48,65 @@ def test_is_expired_treats_unparseable_as_not_expired() -> None:
|
|||
assert github_token._is_expired("not-a-date") is False
|
||||
|
||||
|
||||
def test_get_github_token_returns_none_for_expired_run_metadata() -> None:
|
||||
def test_get_github_token_returns_none_for_expired_cache() -> None:
|
||||
past = (datetime.now(UTC) - timedelta(hours=1)).isoformat()
|
||||
metadata = {
|
||||
"github_token_encrypted": _encrypted("ghp_secret"),
|
||||
"github_token_expires_at": past,
|
||||
}
|
||||
assert github_token.get_github_token({"metadata": metadata}) is None
|
||||
github_token.cache_github_token_for_thread("tid", "ghp_secret", expires_at=past)
|
||||
assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) is None
|
||||
|
||||
|
||||
def test_get_github_token_returns_decrypted_for_fresh_metadata() -> None:
|
||||
def test_get_github_token_returns_fresh_cached_token() -> None:
|
||||
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
||||
metadata = {
|
||||
"github_token_encrypted": _encrypted("ghp_secret"),
|
||||
"github_token_expires_at": future,
|
||||
}
|
||||
assert github_token.get_github_token({"metadata": metadata}) == "ghp_secret"
|
||||
github_token.cache_github_token_for_thread("tid", "ghp_secret", expires_at=future)
|
||||
assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) == "ghp_secret"
|
||||
|
||||
|
||||
def test_get_github_token_returns_decrypted_when_no_expires_at() -> None:
|
||||
metadata = {"github_token_encrypted": _encrypted("ghp_secret")}
|
||||
assert github_token.get_github_token({"metadata": metadata}) == "ghp_secret"
|
||||
def test_get_github_token_returns_cached_token_when_no_expires_at() -> None:
|
||||
github_token.cache_github_token_for_thread("tid", "ghp_secret")
|
||||
assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) == "ghp_secret"
|
||||
|
||||
|
||||
def test_cached_token_expires_after_max_ttl() -> None:
|
||||
"""A token with no/far expiry is still dropped once it's older than the 24h cap."""
|
||||
far_future = (datetime.now(UTC) + timedelta(days=30)).isoformat()
|
||||
old_cached_at = datetime.now(UTC) - timedelta(hours=25)
|
||||
github_token._GITHUB_TOKEN_CACHE["tid"] = ("ghp_secret", far_future, old_cached_at)
|
||||
assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) is None
|
||||
|
||||
|
||||
def test_cache_write_sweeps_other_expired_entries() -> None:
|
||||
"""Writing one entry evicts unrelated entries that have passed their expiry."""
|
||||
past = (datetime.now(UTC) - timedelta(hours=1)).isoformat()
|
||||
github_token.cache_github_token_for_thread("stale", "ghp_stale", expires_at=past)
|
||||
github_token.cache_github_token_for_thread("fresh", "ghp_fresh")
|
||||
assert "stale" not in github_token._GITHUB_TOKEN_CACHE
|
||||
assert "fresh" in github_token._GITHUB_TOKEN_CACHE
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_github_token_from_thread_skips_expired() -> None:
|
||||
past = (datetime.now(UTC) - timedelta(hours=1)).isoformat()
|
||||
fake_client = AsyncMock()
|
||||
fake_client.threads.get.return_value = {
|
||||
"metadata": {
|
||||
"github_token_encrypted": _encrypted("ghp_revoked"),
|
||||
"github_token_expires_at": past,
|
||||
}
|
||||
}
|
||||
with patch.object(github_token, "client", fake_client):
|
||||
token, encrypted, expires_at = await github_token.get_github_token_from_thread("tid")
|
||||
github_token.cache_github_token_for_thread("tid", "ghp_revoked", expires_at=past)
|
||||
token, expires_at = await github_token.get_github_token_from_thread("tid")
|
||||
assert token is None
|
||||
assert encrypted is None
|
||||
assert expires_at is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_github_token_from_thread_returns_fresh() -> None:
|
||||
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
||||
enc = _encrypted("ghp_live")
|
||||
fake_client = AsyncMock()
|
||||
fake_client.threads.get.return_value = {
|
||||
"metadata": {
|
||||
"github_token_encrypted": enc,
|
||||
"github_token_expires_at": future,
|
||||
}
|
||||
}
|
||||
with patch.object(github_token, "client", fake_client):
|
||||
token, encrypted, expires_at = await github_token.get_github_token_from_thread("tid")
|
||||
github_token.cache_github_token_for_thread("tid", "ghp_live", expires_at=future)
|
||||
token, expires_at = await github_token.get_github_token_from_thread("tid")
|
||||
assert token == "ghp_live"
|
||||
assert encrypted == enc
|
||||
assert expires_at == future
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_invalidate_cached_github_token_clears_metadata() -> None:
|
||||
fake_client = AsyncMock()
|
||||
with patch.object(github_token, "client", fake_client):
|
||||
await github_token.invalidate_cached_github_token("tid-42")
|
||||
fake_client.threads.update.assert_awaited_once_with(
|
||||
thread_id="tid-42",
|
||||
metadata={"github_token_encrypted": None, "github_token_expires_at": None},
|
||||
)
|
||||
async def test_invalidate_cached_github_token_clears_cache() -> None:
|
||||
github_token.cache_github_token_for_thread("tid-42", "ghp_live")
|
||||
await github_token.invalidate_cached_github_token("tid-42")
|
||||
token, expires_at = await github_token.get_github_token_from_thread("tid-42")
|
||||
assert token is None
|
||||
assert expires_at is None
|
||||
|
||||
|
||||
# (b) 401 on a downstream GitHub call -----------------------------------------
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ def _patch_dispatch_deps(monkeypatch: pytest.MonkeyPatch, fake_client: Any) -> N
|
|||
AsyncMock(return_value=("token", None)),
|
||||
)
|
||||
monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(webapp, "persist_encrypted_github_token", AsyncMock(return_value="enc"))
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", MagicMock())
|
||||
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock())
|
||||
monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False))
|
||||
monkeypatch.setattr(webapp, "get_client", lambda url: fake_client)
|
||||
|
|
@ -74,8 +74,8 @@ async def test_pr_ready_public_repo_uses_scoped_reviewer_token(
|
|||
get_token = AsyncMock(return_value=("scoped-token", "expires"))
|
||||
monkeypatch.setattr(webapp, "get_github_app_installation_token_with_expiry", get_token)
|
||||
monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True))
|
||||
persist_token = AsyncMock(return_value="enc")
|
||||
monkeypatch.setattr(webapp, "persist_encrypted_github_token", persist_token)
|
||||
cache_token = MagicMock()
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", cache_token)
|
||||
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock())
|
||||
monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False))
|
||||
monkeypatch.setattr(webapp, "get_client", lambda url: fake_client)
|
||||
|
|
@ -85,8 +85,8 @@ async def test_pr_ready_public_repo_uses_scoped_reviewer_token(
|
|||
await webapp.process_github_pr_ready(_pr_payload(action="opened", draft=False, private=False))
|
||||
|
||||
get_token.assert_awaited_once_with(repository_ids=[123])
|
||||
persist_token.assert_awaited_once()
|
||||
assert persist_token.await_args.args[1] == "scoped-token"
|
||||
cache_token.assert_called_once()
|
||||
assert cache_token.call_args.args[1] == "scoped-token"
|
||||
_, kwargs = fake_client.runs.create.await_args
|
||||
assert kwargs["config"]["configurable"]["repo_private"] is False
|
||||
|
||||
|
|
@ -100,7 +100,7 @@ async def test_pr_ready_private_repo_uses_full_reviewer_token(
|
|||
get_token = AsyncMock(return_value=("full-token", "expires"))
|
||||
monkeypatch.setattr(webapp, "get_github_app_installation_token_with_expiry", get_token)
|
||||
monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(webapp, "persist_encrypted_github_token", AsyncMock(return_value="enc"))
|
||||
monkeypatch.setattr(webapp, "cache_github_token_for_thread", MagicMock())
|
||||
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock())
|
||||
monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False))
|
||||
monkeypatch.setattr(webapp, "get_client", lambda url: fake_client)
|
||||
|
|
|
|||
|
|
@ -209,7 +209,7 @@ class TestRefreshProxyOnSandboxReuse:
|
|||
patch(
|
||||
"agent.server.resolve_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("ghp", "enc", None),
|
||||
return_value=("ghp", None),
|
||||
),
|
||||
patch(
|
||||
"agent.server.get_sandbox_id_from_metadata",
|
||||
|
|
@ -258,7 +258,7 @@ class TestRefreshProxyOnSandboxReuse:
|
|||
patch(
|
||||
"agent.server.resolve_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("ghp", "enc", None),
|
||||
return_value=("ghp", None),
|
||||
),
|
||||
patch(
|
||||
"agent.server.get_sandbox_id_from_metadata",
|
||||
|
|
|
|||
|
|
@ -75,7 +75,7 @@ async def test_reviewer_uses_cached_thread_token_for_slack_review_request() -> N
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("app-token", "encrypted-token", None),
|
||||
return_value=("app-token", None),
|
||||
) as mock_get_thread_token,
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock) as mock_resolve_token,
|
||||
patch(
|
||||
|
|
@ -95,7 +95,7 @@ async def test_reviewer_uses_cached_thread_token_for_slack_review_request() -> N
|
|||
|
||||
metadata = config["metadata"]
|
||||
assert isinstance(metadata, dict)
|
||||
assert metadata["github_token_encrypted"] == "encrypted-token"
|
||||
assert "github_token_encrypted" not in metadata
|
||||
mock_get_thread_token.assert_awaited_once_with("reviewer-thread-id")
|
||||
mock_resolve_token.assert_not_called()
|
||||
middleware = create_agent.call_args.kwargs["middleware"]
|
||||
|
|
@ -290,7 +290,7 @@ async def test_reviewer_inlines_agents_md_into_system_prompt() -> None:
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -650,7 +650,7 @@ async def test_reviewer_injects_pr_review_threads_into_first_review_context() ->
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -722,7 +722,7 @@ async def test_reviewer_injects_pr_review_threads_into_re_review_context() -> No
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -786,7 +786,7 @@ async def test_reviewer_omits_threads_block_when_fetch_returns_empty() -> None:
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -846,7 +846,7 @@ async def test_reviewer_continues_when_thread_fetch_raises() -> None:
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -914,7 +914,7 @@ async def test_reviewer_populates_diff_line_set_from_github_api() -> None:
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -980,7 +980,7 @@ async def test_reviewer_leaves_validation_disabled_when_diff_fetch_fails() -> No
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
@ -1042,7 +1042,7 @@ async def test_reviewer_injects_pr_title_and_body_into_context() -> None:
|
|||
patch(
|
||||
"agent.reviewer.get_github_token_from_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", "encrypted-token", None),
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock),
|
||||
patch(
|
||||
|
|
|
|||
|
|
@ -188,11 +188,7 @@ async def test_push_event_queues_when_thread_active_even_if_pr_diff_unchanged()
|
|||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"agent.webapp.persist_encrypted_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value="enc",
|
||||
),
|
||||
patch("agent.webapp.cache_github_token_for_thread"),
|
||||
patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock),
|
||||
patch("agent.webapp.is_thread_active", new_callable=AsyncMock, return_value=True),
|
||||
patch("agent.webapp.queue_message_for_thread", new=queue_message),
|
||||
|
|
@ -257,11 +253,7 @@ async def test_push_event_triggers_re_review_run_when_watching() -> None:
|
|||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"agent.webapp.persist_encrypted_github_token",
|
||||
new_callable=AsyncMock,
|
||||
return_value="enc",
|
||||
),
|
||||
patch("agent.webapp.cache_github_token_for_thread"),
|
||||
patch(
|
||||
"agent.webapp.set_reviewer_thread_metadata",
|
||||
new_callable=AsyncMock,
|
||||
|
|
@ -384,7 +376,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None:
|
|||
fake_client = MagicMock()
|
||||
fake_client.runs.create = AsyncMock()
|
||||
get_token = AsyncMock(return_value=("scoped-token", "exp"))
|
||||
persist = AsyncMock(return_value="enc")
|
||||
cache_token = MagicMock()
|
||||
|
||||
with (
|
||||
patch(
|
||||
|
|
@ -402,7 +394,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None:
|
|||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
patch("agent.webapp.persist_encrypted_github_token", persist),
|
||||
patch("agent.webapp.cache_github_token_for_thread", cache_token),
|
||||
patch("agent.webapp.fetch_pr_review_threads", new_callable=AsyncMock, return_value=[]),
|
||||
patch("agent.webapp.reconcile_findings_with_review_threads", new_callable=AsyncMock),
|
||||
patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock),
|
||||
|
|
@ -412,7 +404,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None:
|
|||
await webapp.process_github_push_event(payload)
|
||||
|
||||
get_token.assert_awaited_once_with(repository_ids=[123])
|
||||
assert persist.await_args.args[1] == "scoped-token"
|
||||
assert cache_token.call_args.args[1] == "scoped-token"
|
||||
_, kwargs = fake_client.runs.create.await_args
|
||||
assert kwargs["config"]["configurable"]["repo_private"] is False
|
||||
|
||||
|
|
@ -430,7 +422,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No
|
|||
fake_client = MagicMock()
|
||||
fake_client.runs.create = AsyncMock()
|
||||
get_token = AsyncMock(side_effect=[("full-token", "e1"), ("scoped-token", "e2")])
|
||||
persist = AsyncMock(return_value="enc")
|
||||
cache_token = MagicMock()
|
||||
|
||||
with (
|
||||
patch(
|
||||
|
|
@ -448,7 +440,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No
|
|||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
patch("agent.webapp.persist_encrypted_github_token", persist),
|
||||
patch("agent.webapp.cache_github_token_for_thread", cache_token),
|
||||
patch("agent.webapp.fetch_pr_review_threads", new_callable=AsyncMock, return_value=[]),
|
||||
patch("agent.webapp.reconcile_findings_with_review_threads", new_callable=AsyncMock),
|
||||
patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock),
|
||||
|
|
@ -458,7 +450,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No
|
|||
await webapp.process_github_push_event(payload)
|
||||
|
||||
assert get_token.await_args_list == [call(), call(repository_ids=[456])]
|
||||
assert persist.await_args.args[1] == "scoped-token"
|
||||
assert cache_token.call_args.args[1] == "scoped-token"
|
||||
_, kwargs = fake_client.runs.create.await_args
|
||||
assert kwargs["config"]["configurable"]["repo_private"] is False
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue