From e9da186b5f399665a7fc6203bb87a15e1be47aa7 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Mon, 13 Jul 2026 14:24:37 -0400 Subject: [PATCH] fix(open-swe): port core GitHub-App scope fallback (#1701), workflows:write kept out of standing scope (#181) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: fall back to core GitHub App scope when optional grants missing (#1701) * fix: fall back to core GitHub App scope when optional grants missing Proxy-token minting requested workflows:write and actions:read in the permission set used for every sandbox. GitHub 422s a token request that asks for a permission the installation hasn't granted, so any install without workflows:write failed to mint a token and every run died in before-agent setup with "GitHub App installation token is unavailable". _resolve_proxy_token now walks a permission ladder (full -> +workflows -> core) and returns the first scope that mints, recording the granted scope so hourly proxy refreshes stay consistent. A missing optional grant now degrades to the install-time core scope instead of failing the run; workflow-file HITL pushes still require workflows:write and fail at push time when it is absent. * refactor: flatten proxy-token ladder loop with continue --------- Co-authored-by: open-swe[bot] (cherry picked from commit f53caff1aa24a7b29d851b267aa3bdfe62c1e935) Sea Haven fork deviation: upstream #1701 folds workflows:write into the standing BASE/RUNTIME scope. This fork deliberately keeps workflows:write OUT of the standing permission ladder (RUNTIME = core + actions:read; LADDER = (RUNTIME, CORE)) so the sandbox proxy token cannot push .github/workflows/* during normal operation. workflows:write is minted only transiently by WorkflowPushGuardMiddleware for an approved HITL push and dropped on restore, preserving token scope as a backstop for the workflow- push approval control. Security-reviewed (agentic fan-out + GPT-4.1 cross review); the standing-scope-carries-workflows:write bypass was blocked. * fix(open-swe): harden proxy-token restore and mint error handling Two low-severity follow-ups from the security review of the #1701 port. Restore the recorded baseline scope after a workflow-push elevation instead of a hardcoded RUNTIME. An install granted workflows:write but not actions:read resolves its standing token to core; hardcoding RUNTIME on restore requested the ungranted actions:read, 422'd, and fired a false "SECURITY: failed to downscope" error on every approved workflow push before the core fallback recovered. The guard now captures the run's recorded scope before elevating (via the new get_recorded_proxy_permissions) and restores exactly that, falling back to the guaranteed core scope only when the baseline restore fails. Classify installation-token mint failures. get_github_app_installation_token_ with_expiry now treats HTTP 422 (a permission the installation hasn't granted) as the ladder's expected descend signal and keeps it at debug, while a non-422 failure (network/5xx/timeout) is surfaced at WARNING even when errors are otherwise suppressed — so a transient blip no longer silently downscopes a whole run under a debug-only trace. The reduced-scope warning no longer asserts a missing grant as the sole cause. * chore(triage): mark upstream #1701 landed on this branch Ported via PR #181 as Option A (workflows:write kept out of the standing proxy-token scope). Regenerated triage.md from triage.jsonl. --------- Co-authored-by: Ramon Nogueira --- agent/middleware/workflow_push_guard.py | 36 ++++++---- agent/server.py | 34 +++++---- agent/utils/github_app.py | 46 ++++++++++-- agent/utils/github_proxy.py | 17 +++++ docs/upstream-sync/triage.jsonl | 2 +- docs/upstream-sync/triage.md | 2 +- tests/test_github_app.py | 95 ++++++++++++++++++++++++- tests/test_proxy_auth.py | 61 +++++++++++++++- tests/test_workflow_push_guard.py | 44 +++++++++++- 9 files changed, 298 insertions(+), 39 deletions(-) diff --git a/agent/middleware/workflow_push_guard.py b/agent/middleware/workflow_push_guard.py index 42f1ee7a..ccaab820 100644 --- a/agent/middleware/workflow_push_guard.py +++ b/agent/middleware/workflow_push_guard.py @@ -31,11 +31,11 @@ from ..dashboard.workflow_approval import ( from ..tools.slack_thread_reply import build_workflow_approval_blocks from ..utils.dashboard_links import dashboard_workflow_approval_url from ..utils.github_app import ( - BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, RUNTIME_PROXY_TOKEN_PERMISSIONS, WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS, ) -from ..utils.github_proxy import refresh_proxy_token +from ..utils.github_proxy import get_recorded_proxy_permissions, refresh_proxy_token from ..utils.sandbox_state import SANDBOX_BACKENDS from ..utils.slack import post_slack_thread_reply_with_ts @@ -876,6 +876,13 @@ async def _run_with_workflow_token( if sandbox_type != "langsmith": return await run() + # Capture the run's standing scope *before* elevating so we restore exactly + # what the install resolved to (RUNTIME, or CORE when actions:read is absent) + # rather than a hardcoded RUNTIME that 422s on installs lacking actions:read. + # The standing ladder never carries workflows:write, so this is always a real + # downscope. Fall back to RUNTIME if nothing was recorded yet. + baseline_scope = get_recorded_proxy_permissions(thread_id) or RUNTIME_PROXY_TOKEN_PERMISSIONS + elevated = await refresh_proxy_token( thread_id, permissions=WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS ) @@ -904,21 +911,22 @@ async def _run_with_workflow_token( try: return await run() finally: - restored = await refresh_proxy_token(thread_id, permissions=RUNTIME_PROXY_TOKEN_PERMISSIONS) - if not restored: + restored = await refresh_proxy_token(thread_id, permissions=baseline_scope) + if not restored and baseline_scope != CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS: logger.error( - "SECURITY: failed to downscope proxy token for thread %s after an approved " - "workflow push; retrying without actions:read.", + "SECURITY: failed to restore baseline proxy scope for thread %s after an " + "approved workflow push; retrying at the guaranteed core scope.", + thread_id, + ) + restored = await refresh_proxy_token( + thread_id, permissions=CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS + ) + if not restored: + logger.error( + "SECURITY: proxy token downscope fully failed for thread %s; the sandbox " + "may retain workflows:write for the remainder of this run.", thread_id, ) - if not await refresh_proxy_token( - thread_id, permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS - ): - logger.error( - "SECURITY: proxy token downscope fully failed for thread %s; the sandbox " - "may retain workflows:write for the remainder of this run.", - thread_id, - ) class WorkflowPushGuardMiddleware(AgentMiddleware): diff --git a/agent/server.py b/agent/server.py index 628c1135..72658ec7 100644 --- a/agent/server.py +++ b/agent/server.py @@ -114,8 +114,7 @@ from .utils.authorship import ( from .utils.dashboard_links import dashboard_plan_url, dashboard_thread_url from .utils.deferred_model import make_model_or_defer from .utils.github_app import ( - BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, - RUNTIME_PROXY_TOKEN_PERMISSIONS, + PROXY_TOKEN_PERMISSION_LADDER, PermissionMap, get_github_app_installation_token_with_expiry, ) @@ -215,18 +214,25 @@ async def _resolve_proxy_token( ) return token, expires_at, permissions - token, expires_at = await get_github_app_installation_token_with_expiry( - permissions=RUNTIME_PROXY_TOKEN_PERMISSIONS, - log_errors=False, - ) - if token: - return token, expires_at, RUNTIME_PROXY_TOKEN_PERMISSIONS - - logger.warning("Retrying GitHub proxy token mint without optional Actions read permission") - token, expires_at = await get_github_app_installation_token_with_expiry( - permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS - ) - return token, expires_at, BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS if token else None + # Walk from the richest scope to the guaranteed core so an installation that + # hasn't granted workflows:write / actions:read degrades instead of failing. + ladder = PROXY_TOKEN_PERMISSION_LADDER + last = len(ladder) - 1 + for index, scope in enumerate(ladder): + token, expires_at = await get_github_app_installation_token_with_expiry( + permissions=scope, + log_errors=index == last, + ) + if not token: + continue + if index: + logger.warning( + "GitHub proxy token minted with reduced scope %s; a higher-privilege " + "scope was unavailable (missing grant or transient mint failure)", + sorted(scope), + ) + return token, expires_at, scope + return None, None, None async def _resolve_snapshot_id_for_repo(repo: dict[str, str] | None) -> str | None: diff --git a/agent/utils/github_app.py b/agent/utils/github_app.py index cbdbfe3f..3a488683 100644 --- a/agent/utils/github_app.py +++ b/agent/utils/github_app.py @@ -26,20 +26,35 @@ GITHUB_APP_INSTALLATION_ID = os.environ.get("GITHUB_APP_INSTALLATION_ID", "") # 5-minute refresh window (``github_proxy.PROXY_TOKEN_REFRESH_WINDOW``) so a # near-expiry proxy refresh still mints a genuinely fresh token. _TOKEN_CACHE_MARGIN = timedelta(minutes=10) -BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = { +# Granted on every installation at install time, so a token scoped to these +# always mints — the terminal rung of the fallback ladder. +CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = { "contents": "write", "pull_requests": "write", "issues": "write", "checks": "write", } +# `actions:read` (CI-log reads) is a later addition an installation may not have +# accepted. GitHub 422s a mint that requests an ungranted permission, so +# ``_resolve_proxy_token`` walks ``PROXY_TOKEN_PERMISSION_LADDER`` high→low and +# degrades to the guaranteed core scope instead of failing the whole run. RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = { - **BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + **CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, "actions": "read", } +# `workflows:write` is deliberately kept OUT of the standing runtime scope: the +# sandbox proxy token cannot push `.github/workflows/*` during normal operation. +# ``WorkflowPushGuardMiddleware`` mints this elevated scope only for an approved +# HITL workflow-file push and restores the standing scope afterwards, so token +# scope stays a backstop for the approval control rather than a standing grant. WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = { - **BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + **CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, "workflows": "write", } +PROXY_TOKEN_PERMISSION_LADDER: tuple[dict[str, str], ...] = ( + RUNTIME_PROXY_TOKEN_PERMISSIONS, + CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, +) PermissionMap = Mapping[str, str] PermissionKey = tuple[tuple[str, str], ...] @@ -174,9 +189,32 @@ async def get_github_app_installation_token_with_expiry( if isinstance(token, str) and token and parsed is not None: _TOKEN_CACHE[key] = (token, expires_at, parsed - _TOKEN_CACHE_MARGIN) return token, expires_at + except httpx.HTTPStatusError as exc: + status = exc.response.status_code if exc.response is not None else None + if status == 422: + # Expected when the installation hasn't granted a requested permission: + # the ladder caller descends to a smaller scope. Not a transient error, + # so keep it at debug even on the terminal rung. + logger.debug( + "GitHub App token mint rejected for requested scope (HTTP 422)", exc_info=True + ) + elif log_errors: + logger.exception("Failed to get GitHub App installation token") + else: + # A non-422 failure is NOT a missing grant; surface it so a transient + # error doesn't silently downscope a run that would otherwise qualify. + logger.warning( + "GitHub App token mint failed (HTTP %s); scope may degrade transiently", + status, + exc_info=True, + ) + return None, None except Exception: if log_errors: logger.exception("Failed to get GitHub App installation token") else: - logger.debug("Failed to get GitHub App installation token", exc_info=True) + logger.warning( + "GitHub App token mint failed unexpectedly; scope may degrade transiently", + exc_info=True, + ) return None, None diff --git a/agent/utils/github_proxy.py b/agent/utils/github_proxy.py index c92762a7..20e6f89f 100644 --- a/agent/utils/github_proxy.py +++ b/agent/utils/github_proxy.py @@ -91,6 +91,23 @@ def clear_proxy_token_expiry(thread_id: str | None) -> None: _PROXY_TOKEN_EXPIRY.pop(thread_id, None) +def get_recorded_proxy_permissions(thread_id: str | None) -> dict[str, str] | None: + """The permission scope last minted for ``thread_id``'s proxy token, if any. + + Lets a caller that temporarily elevates the proxy scope restore the exact + baseline the run resolved to (e.g. an install granted workflows:write but not + actions:read resolves to core), instead of guessing a fixed scope that may + 422 on restore. + """ + if not thread_id: + return None + record = _PROXY_TOKEN_EXPIRY.get(thread_id) + if record is None: + return None + *_, permission_key = _unpack_proxy_token_record(record) + return dict(permission_key) if permission_key else None + + def _unpack_proxy_token_record(record: tuple[Any, ...]) -> ProxyTokenRecord: expires_at, recorded_at, repositories, *rest = record permissions = rest[0] if rest else () diff --git a/docs/upstream-sync/triage.jsonl b/docs/upstream-sync/triage.jsonl index 9c7c29dc..cb7d05b4 100644 --- a/docs/upstream-sync/triage.jsonl +++ b/docs/upstream-sync/triage.jsonl @@ -78,7 +78,7 @@ {"sha": "7f7af715", "pr": 1684, "subject": "feat: auto-load scoped AGENTS on reads (#1684)", "disposition": "landed", "reason": "ported in #129 (SubdirAgentsReadMiddleware)", "branch": "subdir-agents", "local_sha": null, "updated": "2026-07-08T22:58:22Z"} {"sha": "88b62322", "pr": 1685, "subject": "feat: add platform issue reporting tool (#1685)", "disposition": "landed", "reason": "ported in #129 (report_platform_issue tool)", "branch": "small-tools", "local_sha": null, "updated": "2026-07-08T22:58:22Z"} {"sha": "90cb6caa", "pr": 1681, "subject": "feat: terse Slack replies, share long content via plan-review page (#1681)", "disposition": "landed", "reason": "terse Slack + long-content-via-plan-page; conflicts w/ fork prompt + diverged plan stack", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-09T17:10:22Z"} -{"sha": "f53caff1", "pr": 1701, "subject": "fix: fall back to core GitHub App scope when optional grants missing (#1701)", "disposition": "deferred", "reason": "FLAG-HUMAN: GitHub-App permission-ladder degrade (auth surface); heavy conflict on diverged github_app.py/_resolve_proxy_token", "branch": "github-app-scope", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} +{"sha": "f53caff1", "pr": 1701, "subject": "fix: fall back to core GitHub App scope when optional grants missing (#1701)", "disposition": "landed", "reason": "Ported as Option A: workflows:write kept OUT of standing scope, minted only transiently by the workflow-push guard (security-reviewed); PR #181", "branch": "chore/port-github-app-scope-fallback", "local_sha": null, "updated": "2026-07-13T18:15:30Z"} {"sha": "73a9e8b5", "pr": 1693, "subject": "chore(deps): bump the minor-and-patch group across 1 directory with 19 updates (#1693)", "disposition": "wont-merge", "reason": "dev at-or-ahead on 17/19; group fights dev's pinned langsmith==0.9.7 (#115) and carries an upstream plan-route test", "branch": "", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} {"sha": "9cd7e464", "pr": 1700, "subject": "Fix workflow approval visibility (#1700)", "disposition": "wont-merge", "reason": "superseded — dev's list_workflow_approvals_for_thread already enforces owner-only 403", "branch": "", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} {"sha": "22e024cb", "pr": 1704, "subject": "fix: link issue PRs and prompt repo conventions (#1704)", "disposition": "deferred", "reason": "issue/PR linking + repo-convention prompt; clean but prompt-conflict risk vs #113", "branch": "webhook-issue-linking", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} diff --git a/docs/upstream-sync/triage.md b/docs/upstream-sync/triage.md index 90722286..83cef60f 100644 --- a/docs/upstream-sync/triage.md +++ b/docs/upstream-sync/triage.md @@ -61,6 +61,7 @@ Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred ro | `7f7af715` | #1684 | feat: auto-load scoped AGENTS on reads (#1684) | Landed | ported in #129 (SubdirAgentsReadMiddleware) | subdir-agents | | `88b62322` | #1685 | feat: add platform issue reporting tool (#1685) | Landed | ported in #129 (report_platform_issue tool) | small-tools | | `90cb6caa` | #1681 | feat: terse Slack replies, share long content via plan-review page (#1681) | Landed | terse Slack + long-content-via-plan-page; conflicts w/ fork prompt + diverged plan stack | plan-approval | +| `f53caff1` | #1701 | fix: fall back to core GitHub App scope when optional grants missing (#1701) | Landed | Ported as Option A: workflows:write kept OUT of standing scope, minted only transiently by the workflow-push guard (security-reviewed); PR #181 | chore/port-github-app-scope-fallback | | `c3292d82` | #1611 | bake sfw binary into sandbox image | Won't merge | already in dev | | | `48bf712b` | #1609 | show message timestamps | Won't merge | already in dev | | | `85c0f63e` | #1620 | clickable shared PR header | Won't merge | already in dev | | @@ -92,7 +93,6 @@ Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred ro | `48217b68` | #1489 | feat(open-swe): add E2B sandbox provider (#1489) | Deferred | additive E2B provider; separable but ships on the async sandbox.py base | sandbox-refactor | | `67abf5b0` | #1659 | fix: surface attributed PR creation failures (#1659) | Deferred | PR-attribution-failure guard (new mw, safe imports); heavy conflict on diverged open_pull_request.py | pr-attribution | | `5003c953` | #1683 | feat: open Linear-triggered PRs as the triggering user (#1683) | Deferred | FLAG-HUMAN: adds linear to resolve_github_token per-user OAuth branch (auth surface); depends on #1626 linear.py | linear-pr-as-user | -| `f53caff1` | #1701 | fix: fall back to core GitHub App scope when optional grants missing (#1701) | Deferred | FLAG-HUMAN: GitHub-App permission-ladder degrade (auth surface); heavy conflict on diverged github_app.py/_resolve_proxy_token | github-app-scope | | `22e024cb` | #1704 | fix: link issue PRs and prompt repo conventions (#1704) | Deferred | issue/PR linking + repo-convention prompt; clean but prompt-conflict risk vs #113 | webhook-issue-linking | | `27b0ddeb` | #1708 | feat: add GPT-5.6 OpenAI models (#1708) | Deferred | FLAG-HUMAN: adds OpenAI GPT-5.6 to the model picker; fork's picker is Bedrock/Fireworks-only — needs a product decision before adopting OpenAI models. Gateway (#155) can route OpenAI if adopted. | model-picker | | `62e0ca2d` | #1709 | fix: stale admin model defaults after model upgrades (#1709) | Deferred | stale admin model-default cleanup in team_settings after model upgrades; applies to fork's default-model resolution. | model-picker | diff --git a/tests/test_github_app.py b/tests/test_github_app.py index 97563e48..ec3e43ec 100644 --- a/tests/test_github_app.py +++ b/tests/test_github_app.py @@ -1,8 +1,10 @@ from __future__ import annotations +import logging from datetime import UTC, datetime, timedelta from typing import Any +import httpx import pytest from agent.utils import github_app @@ -148,12 +150,103 @@ async def test_installation_token_can_be_scoped_to_repository_ids( def test_runtime_proxy_token_permissions_include_optional_read_only_actions() -> None: - assert "actions" not in github_app.BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS + assert "actions" not in github_app.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS assert github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS["actions"] == "read" assert github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS.get("actions") != "write" assert "actions" not in github_app.WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS +def test_workflows_write_is_never_in_the_standing_scope() -> None: + """workflows:write is guard-only; the standing token must never carry it, so + token scope stays a backstop for the workflow-push HITL approval control.""" + assert "workflows" not in github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS + assert "workflows" not in github_app.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS + assert github_app.WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS["workflows"] == "write" + assert all("workflows" not in scope for scope in github_app.PROXY_TOKEN_PERMISSION_LADDER) + + +def test_core_proxy_token_permissions_exclude_optional_grants() -> None: + """The terminal ladder rung must only ask for install-time permissions.""" + core = github_app.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS + assert "workflows" not in core + assert "actions" not in core + assert core["contents"] == "write" + + +def test_proxy_token_ladder_descends_to_core() -> None: + """Ladder goes most→least privileged so a missing grant degrades gracefully.""" + ladder = github_app.PROXY_TOKEN_PERMISSION_LADDER + assert ladder == ( + github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS, + github_app.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + ) + assert [len(scope) for scope in ladder] == sorted( + (len(scope) for scope in ladder), reverse=True + ) + + +class _HTTPStatusErrorClient: + """Raises an ``HTTPStatusError`` with a configurable status on mint.""" + + status = 500 + + def __init__(self, **kwargs: Any) -> None: + pass + + async def __aenter__(self) -> _HTTPStatusErrorClient: + return self + + async def __aexit__(self, exc_type: object, exc: object, tb: object) -> None: + return None + + async def post(self, url: str, **kwargs: Any) -> Any: + request = httpx.Request("POST", url) + response = httpx.Response(type(self).status, request=request) + raise httpx.HTTPStatusError("mint failed", request=request, response=response) + + +@pytest.mark.asyncio +async def test_missing_grant_422_descends_quietly( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A 422 (ungranted permission) is the ladder's expected descend signal, so it + must not be logged as a transient failure even on a non-terminal rung.""" + + class Client(_HTTPStatusErrorClient): + status = 422 + + _configure(monkeypatch, Client) + + with caplog.at_level(logging.DEBUG, logger="agent.utils.github_app"): + token, _ = await github_app.get_github_app_installation_token_with_expiry( + permissions={"actions": "read"}, log_errors=False + ) + + assert token is None + assert not any(r.levelno >= logging.WARNING for r in caplog.records) + + +@pytest.mark.asyncio +async def test_transient_mint_error_warns_even_when_errors_suppressed( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A non-422 failure is not a missing grant; it must surface at WARNING even on + a non-terminal rung so a blip doesn't silently downscope a whole run.""" + + class Client(_HTTPStatusErrorClient): + status = 503 + + _configure(monkeypatch, Client) + + with caplog.at_level(logging.DEBUG, logger="agent.utils.github_app"): + token, _ = await github_app.get_github_app_installation_token_with_expiry( + permissions={"actions": "read"}, log_errors=False + ) + + assert token is None + assert any(r.levelno == logging.WARNING for r in caplog.records) + + @pytest.mark.asyncio async def test_installation_token_includes_permissions(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(github_app, "GITHUB_APP_ID", "1") diff --git a/tests/test_proxy_auth.py b/tests/test_proxy_auth.py index 69ced5e6..a00d7ca0 100644 --- a/tests/test_proxy_auth.py +++ b/tests/test_proxy_auth.py @@ -10,7 +10,8 @@ import pytest from agent.integrations.langsmith import _configure_github_proxy from agent.utils.github_app import ( - BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + PROXY_TOKEN_PERMISSION_LADDER, RUNTIME_PROXY_TOKEN_PERMISSIONS, ) @@ -233,16 +234,70 @@ class TestCreateSandboxWithProxy: ) assert mock_get_token.await_args_list[0].kwargs["log_errors"] is False assert mock_get_token.await_args_list[1].kwargs["permissions"] == ( - BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS + CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS ) mock_proxy.assert_called_once_with("sandbox-123", "ghs_install") mock_record.assert_called_once_with( "thread-123", "expires", repositories=None, - permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + permissions=CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, ) + @pytest.mark.asyncio + async def test_ladder_walks_every_rung_and_never_requests_workflows(self) -> None: + """The standing ladder degrades RUNTIME→CORE and must never ask for + workflows:write; that grant is minted only transiently by the push guard.""" + with ( + patch( + "agent.server.get_github_app_installation_token_with_expiry", + new_callable=AsyncMock, + side_effect=[(None, None), ("ghs_install", "expires")], + ) as mock_get_token, + patch("agent.server.create_sandbox") as mock_create, + patch("agent.server._configure_github_proxy") as mock_proxy, + patch("agent.server.record_proxy_token_expiry") as mock_record, + patch.dict("os.environ", {"SANDBOX_TYPE": "langsmith", "LANGSMITH_API_KEY": "ls-key"}), + ): + mock_create.return_value = MagicMock(id="sandbox-123") + + from agent.server import _create_sandbox_with_proxy + + await _create_sandbox_with_proxy(thread_id="thread-123") + + scopes = [call.kwargs["permissions"] for call in mock_get_token.await_args_list] + assert scopes == list(PROXY_TOKEN_PERMISSION_LADDER) + assert all("workflows" not in scope for scope in scopes) + mock_proxy.assert_called_once_with("sandbox-123", "ghs_install") + mock_record.assert_called_once_with( + "thread-123", + "expires", + repositories=None, + permissions=CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS, + ) + + @pytest.mark.asyncio + async def test_raises_only_when_even_core_scope_fails(self) -> None: + """A hard failure requires every ladder rung — including core — to fail.""" + with ( + patch( + "agent.server.get_github_app_installation_token_with_expiry", + new_callable=AsyncMock, + return_value=(None, None), + ) as mock_get_token, + patch("agent.server.create_sandbox") as mock_create, + patch("agent.server._configure_github_proxy"), + patch.dict("os.environ", {"SANDBOX_TYPE": "langsmith", "LANGSMITH_API_KEY": "ls-key"}), + ): + mock_create.return_value = MagicMock(id="sandbox-123") + + from agent.server import _create_sandbox_with_proxy + + with pytest.raises(ValueError, match="installation token is unavailable"): + await _create_sandbox_with_proxy(thread_id="thread-123") + + assert mock_get_token.await_count == len(PROXY_TOKEN_PERMISSION_LADDER) + @pytest.mark.asyncio async def test_skips_proxy_for_non_langsmith(self) -> None: """Non-langsmith sandboxes should skip proxy configuration.""" diff --git a/tests/test_workflow_push_guard.py b/tests/test_workflow_push_guard.py index a471bdf0..3ce890a2 100644 --- a/tests/test_workflow_push_guard.py +++ b/tests/test_workflow_push_guard.py @@ -415,8 +415,50 @@ async def test_workflow_push_restoration_falls_back_when_actions_read_unavailabl assert refreshed[0]["workflows"] == "write" assert refreshed[1]["actions"] == "read" - assert refreshed[2] == guard.BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS + assert refreshed[2] == guard.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS assert "actions" not in refreshed[2] + assert "workflows" not in refreshed[2] + + +async def test_workflow_push_restores_recorded_baseline_scope( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Restore targets the run's recorded baseline scope (here: core, because the + install never granted actions:read) — not a hardcoded RUNTIME that would 422.""" + guard.SANDBOX_BACKENDS["thread-1"] = _Backend() + refreshed: list[dict[str, str]] = [] + + async def fake_approved(thread_id: str, fingerprint: str) -> bool: + return True + + async def fake_refresh(thread_id: str | None, *, permissions: dict[str, str]) -> bool: + refreshed.append(dict(permissions)) + return True + + async def fake_find_approval(*args: Any, **kwargs: Any) -> dict[str, Any] | None: + return None + + monkeypatch.setattr(guard, "workflow_push_approved", fake_approved) + monkeypatch.setattr(guard, "find_workflow_push_approval", fake_find_approval) + monkeypatch.setattr(guard, "refresh_proxy_token", fake_refresh) + monkeypatch.setattr( + guard, + "get_recorded_proxy_permissions", + lambda _thread_id: dict(guard.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS), + ) + + async def handler(_request: Any) -> ToolMessage: + return ToolMessage(content="pushed", tool_call_id="call-1") + + await guard.WorkflowPushGuardMiddleware().awrap_tool_call(_Request(), handler) + + # Elevate to workflows:write, then restore straight to the recorded core scope: + # no spurious RUNTIME attempt (which would 422 for this install) and no false + # "failed to downscope" error. + assert refreshed[0]["workflows"] == "write" + assert refreshed[1] == guard.CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS + assert "workflows" not in refreshed[1] + assert len(refreshed) == 2 async def test_non_workflow_push_runs_without_approval(monkeypatch: pytest.MonkeyPatch) -> None: