mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 19:43:15 +00:00
e9da186b5f
* 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] <open-swe@users.noreply.github.com> (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 <ramon.nogueira@langchain.dev>
305 lines
10 KiB
Python
305 lines
10 KiB
Python
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
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _clear_token_cache() -> Any:
|
|
github_app.clear_app_token_cache()
|
|
yield
|
|
github_app.clear_app_token_cache()
|
|
|
|
|
|
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
|
|
|
|
def __init__(self, **kwargs: Any) -> None:
|
|
pass
|
|
|
|
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()
|
|
|
|
|
|
def _configure(monkeypatch: pytest.MonkeyPatch, client_cls: type) -> 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", client_cls)
|
|
|
|
|
|
class _CountingResponse:
|
|
def __init__(self, expires_at: str) -> None:
|
|
self._expires_at = expires_at
|
|
|
|
def raise_for_status(self) -> None:
|
|
pass
|
|
|
|
def json(self) -> dict[str, str]:
|
|
return {"token": "tok-123", "expires_at": self._expires_at}
|
|
|
|
|
|
class _CountingClient:
|
|
posts = 0
|
|
expires_at = "2099-01-01T00:00:00Z"
|
|
|
|
def __init__(self, **kwargs: Any) -> None:
|
|
pass
|
|
|
|
async def __aenter__(self) -> _CountingClient:
|
|
return self
|
|
|
|
async def __aexit__(self, exc_type: object, exc: object, tb: object) -> None:
|
|
return None
|
|
|
|
async def post(self, url: str, **kwargs: Any) -> _CountingResponse:
|
|
type(self).posts += 1
|
|
return _CountingResponse(type(self).expires_at)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_token_is_cached_until_near_expiry(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
|
|
|
class Client(_CountingClient):
|
|
posts = 0
|
|
expires_at = future
|
|
|
|
_configure(monkeypatch, Client)
|
|
|
|
t1, _ = await github_app.get_github_app_installation_token_with_expiry()
|
|
t2, _ = await github_app.get_github_app_installation_token_with_expiry()
|
|
|
|
assert t1 == t2 == "tok-123"
|
|
assert Client.posts == 1 # second call served from the in-process cache
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_cache_is_scoped_per_repository_set(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
|
|
|
class Client(_CountingClient):
|
|
posts = 0
|
|
expires_at = future
|
|
|
|
_configure(monkeypatch, Client)
|
|
|
|
await github_app.get_github_app_installation_token_with_expiry(repositories=["a"])
|
|
await github_app.get_github_app_installation_token_with_expiry(repositories=["b"])
|
|
await github_app.get_github_app_installation_token_with_expiry(repositories=["a"])
|
|
|
|
assert Client.posts == 2 # distinct scopes mint separately; the repeat is cached
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_near_expiry_token_is_not_cached(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
soon = (datetime.now(UTC) + timedelta(minutes=2)).isoformat()
|
|
|
|
class Client(_CountingClient):
|
|
posts = 0
|
|
expires_at = soon
|
|
|
|
_configure(monkeypatch, Client)
|
|
|
|
await github_app.get_github_app_installation_token_with_expiry()
|
|
await github_app.get_github_app_installation_token_with_expiry()
|
|
|
|
assert Client.posts == 2 # within the safety margin -> re-minted every call
|
|
|
|
|
|
@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]}
|
|
|
|
|
|
def test_runtime_proxy_token_permissions_include_optional_read_only_actions() -> None:
|
|
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")
|
|
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(
|
|
repositories=["open-swe"], permissions={"workflows": "write", "contents": "write"}
|
|
)
|
|
|
|
assert _FakeAsyncClient.last_post is not None
|
|
assert _FakeAsyncClient.last_post["json"] == {
|
|
"repositories": ["open-swe"],
|
|
"permissions": {"contents": "write", "workflows": "write"},
|
|
}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_cache_is_scoped_per_permission_set(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
|
|
|
class Client(_CountingClient):
|
|
posts = 0
|
|
expires_at = future
|
|
|
|
_configure(monkeypatch, Client)
|
|
|
|
await github_app.get_github_app_installation_token_with_expiry(
|
|
permissions={"contents": "write"}
|
|
)
|
|
await github_app.get_github_app_installation_token_with_expiry(
|
|
permissions={"contents": "write", "workflows": "write"}
|
|
)
|
|
await github_app.get_github_app_installation_token_with_expiry(
|
|
permissions={"contents": "write"}
|
|
)
|
|
|
|
assert Client.posts == 2
|
|
|
|
|
|
@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
|