From df1f4a0e5d8d4e3db5eb22a7e5d2139eeab580c5 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Wed, 8 Jul 2026 09:06:53 -0400 Subject: [PATCH] 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. --- agent/middleware/workflow_push_guard.py | 4 +- agent/server.py | 34 ++++++++------ agent/utils/github_app.py | 21 +++++++-- tests/test_github_app.py | 31 ++++++++++++- tests/test_proxy_auth.py | 61 +++++++++++++++++++++++-- tests/test_workflow_push_guard.py | 3 +- 6 files changed, 130 insertions(+), 24 deletions(-) diff --git a/agent/middleware/workflow_push_guard.py b/agent/middleware/workflow_push_guard.py index 42f1ee7a..50b8b0b5 100644 --- a/agent/middleware/workflow_push_guard.py +++ b/agent/middleware/workflow_push_guard.py @@ -31,7 +31,7 @@ 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, ) @@ -912,7 +912,7 @@ async def _run_with_workflow_token( thread_id, ) if not await refresh_proxy_token( - thread_id, permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS + thread_id, permissions=CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS ): logger.error( "SECURITY: proxy token downscope fully failed for thread %s; the sandbox " diff --git a/agent/server.py b/agent/server.py index 628c1135..0fe3beab 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; installation is " + "missing higher-privilege grants", + 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..f6907c98 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], ...] diff --git a/tests/test_github_app.py b/tests/test_github_app.py index 97563e48..f5f923d1 100644 --- a/tests/test_github_app.py +++ b/tests/test_github_app.py @@ -148,12 +148,41 @@ 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 + ) + + @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..b6737810 100644 --- a/tests/test_workflow_push_guard.py +++ b/tests/test_workflow_push_guard.py @@ -415,8 +415,9 @@ 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_non_workflow_push_runs_without_approval(monkeypatch: pytest.MonkeyPatch) -> None: