diff --git a/agent/reviewer.py b/agent/reviewer.py index 70a8b8aa..915b1f2f 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -637,11 +637,16 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: config["metadata"]["github_token_expires_at"] = new_expires_at github_token = _token - sandbox_backend = await ensure_sandbox_for_thread(thread_id) + 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( + thread_id, + github_proxy_token=github_proxy_token, + ) work_dir = await aresolve_sandbox_work_dir(sandbox_backend) - repo_config = config["configurable"].get("repo") or {} repo_owner = str(repo_config.get("owner", "")) repo_name = str(repo_config.get("name", "")) base_sha = str(config["configurable"].get("base_sha", "") or "") diff --git a/agent/server.py b/agent/server.py index 8bbe6371..4c31d5b1 100644 --- a/agent/server.py +++ b/agent/server.py @@ -119,36 +119,35 @@ async def _start_langsmith_sandbox_if_needed(sandbox_backend: SandboxBackendProt await asyncio.to_thread(sandbox.start) -async def _create_sandbox_with_proxy() -> SandboxBackendProtocol: - """Create a new sandbox with GitHub proxy auth configured. - - Uses create_sandbox (generic factory) so non-langsmith providers still work. - For langsmith sandboxes, configures the proxy with the installation token. - """ +async def _create_sandbox_with_proxy( + github_proxy_token: str | None = None, +) -> SandboxBackendProtocol: + """Create a new sandbox with GitHub proxy auth configured.""" sandbox_backend = await asyncio.to_thread(create_sandbox) sandbox_type = os.getenv("SANDBOX_TYPE", "langsmith") if sandbox_type == "langsmith": - installation_token = await get_github_app_installation_token() - if not installation_token: + token = github_proxy_token or await get_github_app_installation_token() + if not token: msg = "Cannot configure proxy: GitHub App installation token is unavailable" logger.error(msg) raise ValueError(msg) await _start_langsmith_sandbox_if_needed(sandbox_backend) - await asyncio.to_thread(_configure_github_proxy, sandbox_backend.id, installation_token) + await asyncio.to_thread(_configure_github_proxy, sandbox_backend.id, token) return sandbox_backend async def _refresh_github_proxy( sandbox_backend: SandboxBackendProtocol, + github_proxy_token: str | None = None, ) -> None: """Refresh GitHub proxy credentials for reused LangSmith sandboxes.""" if os.getenv("SANDBOX_TYPE", "langsmith") != "langsmith": return - installation_token = await get_github_app_installation_token() - if not installation_token: + token = github_proxy_token or await get_github_app_installation_token() + if not token: logger.warning( "Skipping GitHub proxy refresh for sandbox %s: installation token unavailable", sandbox_backend.id, @@ -157,16 +156,17 @@ async def _refresh_github_proxy( current_backend = unwrap_sandbox_backend(sandbox_backend) await _start_langsmith_sandbox_if_needed(current_backend) - await asyncio.to_thread(_configure_github_proxy, current_backend.id, installation_token) + await asyncio.to_thread(_configure_github_proxy, current_backend.id, token) async def _refresh_github_proxy_or_recreate( sandbox_backend: SandboxBackendProtocol, thread_id: str, + github_proxy_token: str | None = None, ) -> SandboxBackendProtocol: """Refresh proxy credentials, recreating stale LangSmith sandboxes on failure.""" try: - await _refresh_github_proxy(sandbox_backend) + await _refresh_github_proxy(sandbox_backend, github_proxy_token) except Exception: # noqa: BLE001 logger.warning( "Failed to refresh GitHub proxy for sandbox %s on thread %s, recreating sandbox", @@ -174,7 +174,7 @@ async def _refresh_github_proxy_or_recreate( thread_id, exc_info=True, ) - return await _recreate_sandbox(thread_id) + return await _recreate_sandbox(thread_id, github_proxy_token=github_proxy_token) return sandbox_backend @@ -186,7 +186,11 @@ async def _configure_git_identity(sandbox_backend: SandboxBackendProtocol) -> No ) -async def _recreate_sandbox(thread_id: str) -> SandboxBackendProtocol: +async def _recreate_sandbox( + thread_id: str, + *, + github_proxy_token: str | None = None, +) -> SandboxBackendProtocol: """Recreate a sandbox after a connection failure. Sets the SANDBOX_CREATING sentinel and creates a fresh sandbox @@ -195,7 +199,10 @@ async def _recreate_sandbox(thread_id: str) -> SandboxBackendProtocol: """ await client.threads.update(thread_id=thread_id, metadata=_creating_metadata()) try: - sandbox_backend = set_sandbox_backend(thread_id, await _create_sandbox_with_proxy()) + sandbox_backend = set_sandbox_backend( + thread_id, + await _create_sandbox_with_proxy(github_proxy_token), + ) except Exception: logger.exception("Failed to recreate sandbox after connection failure") await client.threads.update(thread_id=thread_id, metadata=_RESET_METADATA) @@ -204,7 +211,9 @@ async def _recreate_sandbox(thread_id: str) -> SandboxBackendProtocol: async def check_or_recreate_sandbox( - sandbox_backend: SandboxBackendProtocol, thread_id: str + sandbox_backend: SandboxBackendProtocol, + thread_id: str, + github_proxy_token: str | None = None, ) -> SandboxBackendProtocol: """Check if a cached sandbox is reachable; recreate it if not. @@ -221,7 +230,7 @@ async def check_or_recreate_sandbox( "Cached sandbox is no longer reachable for thread %s, recreating", thread_id, ) - sandbox_backend = await _recreate_sandbox(thread_id) + sandbox_backend = await _recreate_sandbox(thread_id, github_proxy_token=github_proxy_token) return sandbox_backend @@ -272,7 +281,11 @@ def graph_loaded_for_execution(config: RunnableConfig) -> bool: ) -async def ensure_sandbox_for_thread(thread_id: str) -> SandboxBackendProtocol: +async def ensure_sandbox_for_thread( + thread_id: str, + *, + github_proxy_token: str | None = None, +) -> SandboxBackendProtocol: """Get-or-create a healthy sandbox bound to ``thread_id``. Implements the four-state lifecycle described in AGENTS.md: @@ -297,14 +310,18 @@ async def ensure_sandbox_for_thread(thread_id: str) -> SandboxBackendProtocol: if sandbox_backend: logger.info("Using cached sandbox backend for thread %s", thread_id) original_sandbox_id = sandbox_backend.id - sandbox_backend = await check_or_recreate_sandbox(sandbox_backend, thread_id) + sandbox_backend = await check_or_recreate_sandbox( + sandbox_backend, thread_id, github_proxy_token + ) if sandbox_backend.id == original_sandbox_id: - sandbox_backend = await _refresh_github_proxy_or_recreate(sandbox_backend, thread_id) + sandbox_backend = await _refresh_github_proxy_or_recreate( + sandbox_backend, thread_id, github_proxy_token + ) elif sandbox_id is None: logger.info("Creating new sandbox for thread %s", thread_id) await client.threads.update(thread_id=thread_id, metadata=_creating_metadata()) try: - sandbox_backend = await _create_sandbox_with_proxy() + sandbox_backend = await _create_sandbox_with_proxy(github_proxy_token) logger.info("Sandbox created: %s", sandbox_backend.id) except Exception: logger.exception("Failed to create sandbox") @@ -322,7 +339,7 @@ async def ensure_sandbox_for_thread(thread_id: str) -> SandboxBackendProtocol: logger.warning("Failed to connect to existing sandbox %s, creating new one", sandbox_id) await client.threads.update(thread_id=thread_id, metadata=_creating_metadata()) try: - sandbox_backend = await _create_sandbox_with_proxy() + sandbox_backend = await _create_sandbox_with_proxy(github_proxy_token) created_replacement_sandbox = True except Exception: logger.exception("Failed to create replacement sandbox") @@ -330,10 +347,12 @@ async def ensure_sandbox_for_thread(thread_id: str) -> SandboxBackendProtocol: raise if not created_replacement_sandbox: original_sandbox_id = sandbox_backend.id - sandbox_backend = await check_or_recreate_sandbox(sandbox_backend, thread_id) + sandbox_backend = await check_or_recreate_sandbox( + sandbox_backend, thread_id, github_proxy_token + ) if sandbox_backend.id == original_sandbox_id: sandbox_backend = await _refresh_github_proxy_or_recreate( - sandbox_backend, thread_id + sandbox_backend, thread_id, github_proxy_token ) sandbox_backend = set_sandbox_backend(thread_id, sandbox_backend) diff --git a/agent/utils/github_app.py b/agent/utils/github_app.py index ba708aa6..4d45a360 100644 --- a/agent/utils/github_app.py +++ b/agent/utils/github_app.py @@ -5,6 +5,8 @@ from __future__ import annotations import logging import os import time +from collections.abc import Sequence +from typing import Any import httpx import jwt @@ -28,26 +30,35 @@ def _generate_app_jwt() -> str: return jwt.encode(payload, private_key, algorithm="RS256") -async def get_github_app_installation_token() -> str | None: - """Exchange the GitHub App JWT for an installation access token. - - Returns: - Installation access token string, or None if unavailable. - """ - token, _ = await get_github_app_installation_token_with_expiry() +async def get_github_app_installation_token( + *, + repository_ids: Sequence[int] | None = None, + repositories: Sequence[str] | None = None, +) -> str | None: + """Exchange the GitHub App JWT for an installation access token.""" + token, _ = await get_github_app_installation_token_with_expiry( + repository_ids=repository_ids, + repositories=repositories, + ) return token -async def get_github_app_installation_token_with_expiry() -> tuple[str | None, str | None]: - """Exchange the GitHub App JWT for an installation access token and its expiry. - - Returns ``(token, expires_at)`` where ``expires_at`` is the ISO-8601 string - returned by GitHub (typically 1 hour out). Either value may be ``None``. - """ +async def get_github_app_installation_token_with_expiry( + *, + repository_ids: Sequence[int] | None = None, + repositories: Sequence[str] | None = None, +) -> tuple[str | None, str | None]: + """Exchange the GitHub App JWT for an installation access token and its expiry.""" if not GITHUB_APP_ID or not GITHUB_APP_PRIVATE_KEY or not GITHUB_APP_INSTALLATION_ID: logger.debug("GitHub App env vars not fully configured, skipping app token") return None, None + body: dict[str, Any] = {} + if repository_ids: + body["repository_ids"] = list(repository_ids) + elif repositories: + body["repositories"] = list(repositories) + try: app_jwt = _generate_app_jwt() async with httpx.AsyncClient() as client: @@ -58,6 +69,7 @@ async def get_github_app_installation_token_with_expiry() -> tuple[str | None, s "Accept": "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28", }, + json=body or None, ) response.raise_for_status() data = response.json() diff --git a/agent/webapp.py b/agent/webapp.py index f38dea4d..4256d4e7 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -1668,6 +1668,46 @@ async def fetch_github_pr_metadata(pr_ref: GitHubPrRef, *, token: str) -> dict[s return data if isinstance(data, dict) else None +def _repo_private_from_pr_metadata(pr_metadata: dict[str, Any]) -> bool | None: + repo = pr_metadata.get("base", {}).get("repo") + if isinstance(repo, dict) and isinstance(repo.get("private"), bool): + return repo["private"] + return None + + +def _repo_id_from_pr_metadata(pr_metadata: dict[str, Any]) -> int | None: + repo = pr_metadata.get("base", {}).get("repo") + repo_id = repo.get("id") if isinstance(repo, dict) else None + return repo_id if isinstance(repo_id, int) else None + + +def _repo_private_from_payload(payload: dict[str, Any]) -> bool | None: + repo = payload.get("repository") + private = repo.get("private") if isinstance(repo, dict) else None + return private if isinstance(private, bool) else None + + +def _repo_id_from_payload(payload: dict[str, Any]) -> int | None: + repo = payload.get("repository") + repo_id = repo.get("id") if isinstance(repo, dict) else None + return repo_id if isinstance(repo_id, int) else None + + +async def _reviewer_token_for_repo( + repo_config: dict[str, str], + *, + repo_private: bool | None, + repo_id: int | None = None, +) -> tuple[str | None, str | None]: + if repo_private is False: + if repo_id is not None: + return await get_github_app_installation_token_with_expiry(repository_ids=[repo_id]) + repo_name = repo_config.get("name") + if repo_name: + return await get_github_app_installation_token_with_expiry(repositories=[repo_name]) + return await get_github_app_installation_token_with_expiry() + + async def trigger_pr_review_from_ref( pr_ref: GitHubPrRef, *, @@ -1681,6 +1721,8 @@ async def trigger_pr_review_from_ref( if not await _is_repo_enabled_for_review(repo_config): return {"success": False, "error": "Repository not enabled for review"} + # Full token to read PR metadata (privacy/id aren't in the trigger ref); + # re-scoped below once we know whether the repo is public. app_token, app_token_expires_at = await get_github_app_installation_token_with_expiry() if not app_token: logger.warning("No GitHub App token available for PR reviewer request") @@ -1690,6 +1732,17 @@ async def trigger_pr_review_from_ref( if not pr_metadata: return {"success": False, "error": "Could not fetch pull request metadata"} + repo_private = _repo_private_from_pr_metadata(pr_metadata) + repo_id = _repo_id_from_pr_metadata(pr_metadata) + app_token, app_token_expires_at = await _reviewer_token_for_repo( + repo_config, + repo_private=repo_private, + repo_id=repo_id, + ) + if not app_token: + logger.warning("No GitHub App token available for PR reviewer request") + return {"success": False, "error": "No GitHub App token available"} + base_sha = pr_metadata.get("base", {}).get("sha", "") head = pr_metadata.get("head", {}) head_sha = head.get("sha", "") @@ -1742,6 +1795,7 @@ async def trigger_pr_review_from_ref( base_sha=base_sha, head_sha=head_sha, branch_name=branch_name, + repo_private=repo_private, slack_channel_id=slack_channel_id, slack_thread_ts=slack_thread_ts, ) @@ -1781,6 +1835,7 @@ def _build_reviewer_configurable( base_sha: str, head_sha: str, branch_name: str, + repo_private: bool | None = None, re_review: bool = False, last_reviewed_sha: str = "", slack_channel_id: str = "", @@ -1801,6 +1856,8 @@ def _build_reviewer_configurable( } if branch_name: configurable["branch_name"] = branch_name + if repo_private is not None: + configurable["repo_private"] = repo_private if last_reviewed_sha: configurable["last_reviewed_sha"] = last_reviewed_sha if slack_channel_id and slack_thread_ts: @@ -1836,6 +1893,8 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou "owner": repo.get("owner", {}).get("login", ""), "name": repo.get("name", ""), } + repo_private = _repo_private_from_payload(payload) + repo_id = _repo_id_from_payload(payload) pr_number = pull_request.get("number") pr_url = pull_request.get("html_url", "") or pull_request.get("url", "") branch_name = pull_request.get("head", {}).get("ref", "") @@ -1881,7 +1940,11 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou return last_reviewed_sha = existing_last_reviewed_sha - app_token, app_token_expires_at = await get_github_app_installation_token_with_expiry() + app_token, app_token_expires_at = await _reviewer_token_for_repo( + repo_config, + repo_private=repo_private, + repo_id=repo_id, + ) if not app_token: logger.warning("No GitHub App token available for reviewer dispatch") return @@ -1917,6 +1980,7 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou base_sha=base_sha, head_sha=head_sha, branch_name=branch_name, + repo_private=repo_private, re_review=is_re_review, last_reviewed_sha=last_reviewed_sha, ) @@ -2204,6 +2268,8 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: "owner": repo.get("owner", {}).get("login", "") or repo.get("owner", {}).get("name", ""), "name": repo.get("name", ""), } + repo_private = _repo_private_from_payload(payload) + repo_id = _repo_id_from_payload(payload) if not repo_config["owner"] or not repo_config["name"]: logger.warning("Push to %s ignored: repository owner/name missing from payload", head_ref) return @@ -2216,7 +2282,11 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: ) return - app_token, app_token_expires_at = await get_github_app_installation_token_with_expiry() + app_token, app_token_expires_at = await _reviewer_token_for_repo( + repo_config, + repo_private=repo_private, + repo_id=repo_id, + ) if not app_token: logger.warning("No GitHub App token for push re-review on %s", head_ref) return @@ -2231,6 +2301,21 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: ) return + # Push payloads normally carry repo privacy/id; fall back to PR metadata. + # If the repo turns out public, re-scope the token so reviewer.py doesn't + # proxy a full-installation token for a public PR. + if repo_private is None: + repo_private = _repo_private_from_pr_metadata(pr) + repo_id = repo_id or _repo_id_from_pr_metadata(pr) + if repo_private is False: + app_token, app_token_expires_at = await _reviewer_token_for_repo( + repo_config, + repo_private=repo_private, + repo_id=repo_id, + ) + if not app_token: + logger.warning("No GitHub App token for push re-review on %s", head_ref) + return pr_number = pr.get("number") pr_url = pr.get("html_url") or pr.get("url") or "" base_sha = pr.get("base", {}).get("sha", "") @@ -2332,6 +2417,7 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: base_sha=base_sha, head_sha=head_sha, branch_name=head_ref, + repo_private=repo_private, re_review=True, last_reviewed_sha=last_reviewed_sha if isinstance(last_reviewed_sha, str) else "", ) @@ -2599,6 +2685,8 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None: "owner": repo.get("owner", {}).get("login", ""), "name": repo.get("name", ""), } + repo_private = _repo_private_from_payload(payload) + repo_id = _repo_id_from_payload(payload) pr_number = pull_request.get("number") if not isinstance(pr_number, int): return @@ -2610,7 +2698,11 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None: if metadata is None or metadata.get("kind") != REVIEWER_THREAD_KIND: return - app_token, app_token_expires_at = await get_github_app_installation_token_with_expiry() + app_token, app_token_expires_at = await _reviewer_token_for_repo( + repo_config, + repo_private=repo_private, + repo_id=repo_id, + ) if not app_token: return try: @@ -2669,6 +2761,7 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None: base_sha=base_sha, head_sha=head_sha, branch_name=branch_name, + repo_private=repo_private, re_review=True, ) configurable.update( diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 00000000..795d8798 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from agent.utils import github_app + + +class _FakeResponse: + def raise_for_status(self) -> None: + pass + + def json(self) -> dict[str, str]: + return {"token": "token", "expires_at": "expires"} + + +class _FakeAsyncClient: + last_post: dict[str, Any] | None = None + + async def __aenter__(self) -> _FakeAsyncClient: + return self + + async def __aexit__(self, exc_type: object, exc: object, tb: object) -> None: + return None + + async def post(self, url: str, **kwargs: Any) -> _FakeResponse: + type(self).last_post = {"url": url, **kwargs} + return _FakeResponse() + + +@pytest.mark.asyncio +async def test_installation_token_can_be_scoped_to_repository_ids( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(github_app, "GITHUB_APP_ID", "1") + monkeypatch.setattr(github_app, "GITHUB_APP_PRIVATE_KEY", "key") + monkeypatch.setattr(github_app, "GITHUB_APP_INSTALLATION_ID", "2") + monkeypatch.setattr(github_app, "_generate_app_jwt", lambda: "jwt") + monkeypatch.setattr(github_app.httpx, "AsyncClient", _FakeAsyncClient) + + token, expires_at = await github_app.get_github_app_installation_token_with_expiry( + repository_ids=[123] + ) + + assert token == "token" + assert expires_at == "expires" + assert _FakeAsyncClient.last_post is not None + assert _FakeAsyncClient.last_post["json"] == {"repository_ids": [123]} + + +@pytest.mark.asyncio +async def test_installation_token_omits_scope_for_full_installation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(github_app, "GITHUB_APP_ID", "1") + monkeypatch.setattr(github_app, "GITHUB_APP_PRIVATE_KEY", "key") + monkeypatch.setattr(github_app, "GITHUB_APP_INSTALLATION_ID", "2") + monkeypatch.setattr(github_app, "_generate_app_jwt", lambda: "jwt") + monkeypatch.setattr(github_app.httpx, "AsyncClient", _FakeAsyncClient) + + await github_app.get_github_app_installation_token_with_expiry() + + assert _FakeAsyncClient.last_post is not None + assert _FakeAsyncClient.last_post["json"] is None diff --git a/tests/test_pr_ready_auto_review.py b/tests/test_pr_ready_auto_review.py index 849034af..ae1dcc97 100644 --- a/tests/test_pr_ready_auto_review.py +++ b/tests/test_pr_ready_auto_review.py @@ -10,10 +10,19 @@ import pytest from agent import webapp -def _pr_payload(*, action: str, draft: bool, author: str = "alice") -> dict[str, Any]: +def _pr_payload( + *, + action: str, + draft: bool, + author: str = "alice", + private: bool | None = None, +) -> dict[str, Any]: + repository: dict[str, Any] = {"owner": {"login": "lc"}, "name": "repo", "id": 123} + if private is not None: + repository["private"] = private return { "action": action, - "repository": {"owner": {"login": "lc"}, "name": "repo"}, + "repository": repository, "pull_request": { "number": 7, "html_url": "https://github.com/lc/repo/pull/7", @@ -56,6 +65,55 @@ async def test_pr_ready_non_draft_triggers_run(monkeypatch: pytest.MonkeyPatch) assert kwargs["config"]["configurable"]["pr_number"] == 7 +@pytest.mark.asyncio +async def test_pr_ready_public_repo_uses_scoped_reviewer_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + fake_client = MagicMock() + fake_client.runs.create = AsyncMock() + 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) + 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) + monkeypatch.setattr(webapp, "get_profile", AsyncMock(return_value=None)) + monkeypatch.setattr(webapp, "get_team_settings", AsyncMock(return_value={})) + + 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" + _, kwargs = fake_client.runs.create.await_args + assert kwargs["config"]["configurable"]["repo_private"] is False + + +@pytest.mark.asyncio +async def test_pr_ready_private_repo_uses_full_reviewer_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + fake_client = MagicMock() + fake_client.runs.create = AsyncMock() + 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, "set_reviewer_thread_metadata", AsyncMock()) + monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False)) + monkeypatch.setattr(webapp, "get_client", lambda url: fake_client) + monkeypatch.setattr(webapp, "get_profile", AsyncMock(return_value=None)) + monkeypatch.setattr(webapp, "get_team_settings", AsyncMock(return_value={})) + + await webapp.process_github_pr_ready(_pr_payload(action="opened", draft=False, private=True)) + + get_token.assert_awaited_once_with() + _, kwargs = fake_client.runs.create.await_args + assert kwargs["config"]["configurable"]["repo_private"] is True + + @pytest.mark.asyncio async def test_pr_ready_for_review_triggers_run(monkeypatch: pytest.MonkeyPatch) -> None: fake_client = MagicMock() diff --git a/tests/test_proxy_auth.py b/tests/test_proxy_auth.py index 221c4c83..dfc40fb3 100644 --- a/tests/test_proxy_auth.py +++ b/tests/test_proxy_auth.py @@ -327,7 +327,7 @@ class TestRefreshProxyOnSandboxReuse: assert sandbox is replacement_sandbox mock_proxy.assert_called_once_with("sandbox-stale", "ghs_fresh") - mock_recreate.assert_awaited_once_with("thread-123") + mock_recreate.assert_awaited_once_with("thread-123", github_proxy_token=None) @pytest.mark.asyncio async def test_starts_stopped_langsmith_sandbox_before_proxy_refresh(self) -> None: diff --git a/tests/test_reviewer_watch.py b/tests/test_reviewer_watch.py index 2c4d9b22..032bc7ae 100644 --- a/tests/test_reviewer_watch.py +++ b/tests/test_reviewer_watch.py @@ -3,18 +3,31 @@ from __future__ import annotations from typing import Any -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, call, patch import pytest from agent import webapp -def _push_payload(*, ref: str, after: str, owner: str = "lc", name: str = "repo") -> dict[str, Any]: +def _push_payload( + *, + ref: str, + after: str, + owner: str = "lc", + name: str = "repo", + private: bool | None = None, + repo_id: int | None = None, +) -> dict[str, Any]: + repository: dict[str, Any] = {"owner": {"login": owner}, "name": name} + if private is not None: + repository["private"] = private + if repo_id is not None: + repository["id"] = repo_id return { "ref": ref, "after": after, - "repository": {"owner": {"login": owner}, "name": name}, + "repository": repository, "sender": {"login": "alice", "id": 7}, } @@ -309,6 +322,139 @@ async def test_push_event_idempotent_when_head_unchanged() -> None: fake_client.runs.create.assert_not_called() +@pytest.mark.asyncio +async def test_reviewer_token_for_repo_public_scopes_by_id() -> None: + get_token = AsyncMock(return_value=("scoped", "exp")) + with patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token): + token, expires = await webapp._reviewer_token_for_repo( + {"owner": "lc", "name": "repo"}, repo_private=False, repo_id=123 + ) + assert (token, expires) == ("scoped", "exp") + get_token.assert_awaited_once_with(repository_ids=[123]) + + +@pytest.mark.asyncio +async def test_reviewer_token_for_repo_public_scopes_by_name_without_id() -> None: + get_token = AsyncMock(return_value=("scoped", "exp")) + with patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token): + await webapp._reviewer_token_for_repo( + {"owner": "lc", "name": "repo"}, repo_private=False, repo_id=None + ) + get_token.assert_awaited_once_with(repositories=["repo"]) + + +@pytest.mark.asyncio +async def test_reviewer_token_for_repo_private_uses_full_token() -> None: + get_token = AsyncMock(return_value=("full", "exp")) + with patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token): + await webapp._reviewer_token_for_repo( + {"owner": "lc", "name": "repo"}, repo_private=True, repo_id=123 + ) + get_token.assert_awaited_once_with() + + +@pytest.mark.asyncio +async def test_reviewer_token_for_repo_unknown_privacy_uses_full_token() -> None: + get_token = AsyncMock(return_value=("full", "exp")) + with patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token): + await webapp._reviewer_token_for_repo( + {"owner": "lc", "name": "repo"}, repo_private=None, repo_id=123 + ) + get_token.assert_awaited_once_with() + + +@pytest.mark.asyncio +async def test_push_event_public_repo_uses_scoped_token() -> None: + payload = _push_payload(ref="refs/heads/feat-x", after="newsha", private=False, repo_id=123) + pr = { + "number": 7, + "html_url": "https://github.com/lc/repo/pull/7", + "title": "T", + "head": {"sha": "newsha", "ref": "feat-x"}, + "base": {"sha": "basesha", "ref": "main"}, + } + fake_client = MagicMock() + fake_client.runs.create = AsyncMock() + get_token = AsyncMock(return_value=("scoped-token", "exp")) + persist = AsyncMock(return_value="enc") + + with ( + patch( + "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + ), + patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token), + patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr), + patch( + "agent.webapp._get_thread_metadata_safe", + new_callable=AsyncMock, + return_value={"kind": "reviewer", "watch": True}, + ), + patch( + "agent.webapp._ensure_thread_exists_for_metadata", + new_callable=AsyncMock, + return_value=True, + ), + patch("agent.webapp.persist_encrypted_github_token", persist), + 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), + patch("agent.webapp.is_thread_active", new_callable=AsyncMock, return_value=False), + patch("agent.webapp.get_client", return_value=fake_client), + ): + await webapp.process_github_push_event(payload) + + get_token.assert_awaited_once_with(repository_ids=[123]) + assert persist.await_args.args[1] == "scoped-token" + _, kwargs = fake_client.runs.create.await_args + assert kwargs["config"]["configurable"]["repo_private"] is False + + +@pytest.mark.asyncio +async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> None: + payload = _push_payload(ref="refs/heads/feat-x", after="newsha") + pr = { + "number": 7, + "html_url": "https://github.com/lc/repo/pull/7", + "title": "T", + "head": {"sha": "newsha", "ref": "feat-x"}, + "base": {"sha": "basesha", "ref": "main", "repo": {"private": False, "id": 456}}, + } + fake_client = MagicMock() + fake_client.runs.create = AsyncMock() + get_token = AsyncMock(side_effect=[("full-token", "e1"), ("scoped-token", "e2")]) + persist = AsyncMock(return_value="enc") + + with ( + patch( + "agent.webapp._is_repo_enabled_for_review", new_callable=AsyncMock, return_value=True + ), + patch("agent.webapp.get_github_app_installation_token_with_expiry", get_token), + patch("agent.webapp._fetch_open_pr_for_branch", new_callable=AsyncMock, return_value=pr), + patch( + "agent.webapp._get_thread_metadata_safe", + new_callable=AsyncMock, + return_value={"kind": "reviewer", "watch": True}, + ), + patch( + "agent.webapp._ensure_thread_exists_for_metadata", + new_callable=AsyncMock, + return_value=True, + ), + patch("agent.webapp.persist_encrypted_github_token", persist), + 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), + patch("agent.webapp.is_thread_active", new_callable=AsyncMock, return_value=False), + patch("agent.webapp.get_client", return_value=fake_client), + ): + 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" + _, kwargs = fake_client.runs.create.await_args + assert kwargs["config"]["configurable"]["repo_private"] is False + + @pytest.mark.asyncio async def test_pr_close_disables_watch() -> None: captured: list[Any] = [] diff --git a/tests/test_stale_sandbox_creating.py b/tests/test_stale_sandbox_creating.py index 2c7ed57d..1088457e 100644 --- a/tests/test_stale_sandbox_creating.py +++ b/tests/test_stale_sandbox_creating.py @@ -67,7 +67,7 @@ async def test_fresh_sandbox_creating_waits_for_other_worker() -> None: {"metadata": {"sandbox_id": "sandbox-existing", "sandbox_creating_at": fresh_at}}, ] - async def passthrough(sb, _thread_id): + async def passthrough(sb, _thread_id, _github_proxy_token=None): return sb with (