mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 16:19:09 +00:00
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.
This commit is contained in:
parent
00a99c86c4
commit
df1f4a0e5d
6 changed files with 130 additions and 24 deletions
|
|
@ -31,7 +31,7 @@ from ..dashboard.workflow_approval import (
|
||||||
from ..tools.slack_thread_reply import build_workflow_approval_blocks
|
from ..tools.slack_thread_reply import build_workflow_approval_blocks
|
||||||
from ..utils.dashboard_links import dashboard_workflow_approval_url
|
from ..utils.dashboard_links import dashboard_workflow_approval_url
|
||||||
from ..utils.github_app import (
|
from ..utils.github_app import (
|
||||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
)
|
)
|
||||||
|
|
@ -912,7 +912,7 @@ async def _run_with_workflow_token(
|
||||||
thread_id,
|
thread_id,
|
||||||
)
|
)
|
||||||
if not await refresh_proxy_token(
|
if not await refresh_proxy_token(
|
||||||
thread_id, permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
thread_id, permissions=CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||||
):
|
):
|
||||||
logger.error(
|
logger.error(
|
||||||
"SECURITY: proxy token downscope fully failed for thread %s; the sandbox "
|
"SECURITY: proxy token downscope fully failed for thread %s; the sandbox "
|
||||||
|
|
|
||||||
|
|
@ -114,8 +114,7 @@ from .utils.authorship import (
|
||||||
from .utils.dashboard_links import dashboard_plan_url, dashboard_thread_url
|
from .utils.dashboard_links import dashboard_plan_url, dashboard_thread_url
|
||||||
from .utils.deferred_model import make_model_or_defer
|
from .utils.deferred_model import make_model_or_defer
|
||||||
from .utils.github_app import (
|
from .utils.github_app import (
|
||||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
PROXY_TOKEN_PERMISSION_LADDER,
|
||||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
|
||||||
PermissionMap,
|
PermissionMap,
|
||||||
get_github_app_installation_token_with_expiry,
|
get_github_app_installation_token_with_expiry,
|
||||||
)
|
)
|
||||||
|
|
@ -215,18 +214,25 @@ async def _resolve_proxy_token(
|
||||||
)
|
)
|
||||||
return token, expires_at, permissions
|
return token, expires_at, permissions
|
||||||
|
|
||||||
token, expires_at = await get_github_app_installation_token_with_expiry(
|
# Walk from the richest scope to the guaranteed core so an installation that
|
||||||
permissions=RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
# hasn't granted workflows:write / actions:read degrades instead of failing.
|
||||||
log_errors=False,
|
ladder = PROXY_TOKEN_PERMISSION_LADDER
|
||||||
)
|
last = len(ladder) - 1
|
||||||
if token:
|
for index, scope in enumerate(ladder):
|
||||||
return token, expires_at, RUNTIME_PROXY_TOKEN_PERMISSIONS
|
token, expires_at = await get_github_app_installation_token_with_expiry(
|
||||||
|
permissions=scope,
|
||||||
logger.warning("Retrying GitHub proxy token mint without optional Actions read permission")
|
log_errors=index == last,
|
||||||
token, expires_at = await get_github_app_installation_token_with_expiry(
|
)
|
||||||
permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
if not token:
|
||||||
)
|
continue
|
||||||
return token, expires_at, BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS if token else None
|
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:
|
async def _resolve_snapshot_id_for_repo(repo: dict[str, str] | None) -> str | None:
|
||||||
|
|
|
||||||
|
|
@ -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
|
# 5-minute refresh window (``github_proxy.PROXY_TOKEN_REFRESH_WINDOW``) so a
|
||||||
# near-expiry proxy refresh still mints a genuinely fresh token.
|
# near-expiry proxy refresh still mints a genuinely fresh token.
|
||||||
_TOKEN_CACHE_MARGIN = timedelta(minutes=10)
|
_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",
|
"contents": "write",
|
||||||
"pull_requests": "write",
|
"pull_requests": "write",
|
||||||
"issues": "write",
|
"issues": "write",
|
||||||
"checks": "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] = {
|
RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||||
**BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
**CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
"actions": "read",
|
"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] = {
|
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||||
**BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
**CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
"workflows": "write",
|
"workflows": "write",
|
||||||
}
|
}
|
||||||
|
PROXY_TOKEN_PERMISSION_LADDER: tuple[dict[str, str], ...] = (
|
||||||
|
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
|
CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
|
)
|
||||||
|
|
||||||
PermissionMap = Mapping[str, str]
|
PermissionMap = Mapping[str, str]
|
||||||
PermissionKey = tuple[tuple[str, str], ...]
|
PermissionKey = tuple[tuple[str, str], ...]
|
||||||
|
|
|
||||||
|
|
@ -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:
|
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["actions"] == "read"
|
||||||
assert github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS.get("actions") != "write"
|
assert github_app.RUNTIME_PROXY_TOKEN_PERMISSIONS.get("actions") != "write"
|
||||||
assert "actions" not in github_app.WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_installation_token_includes_permissions(monkeypatch: pytest.MonkeyPatch) -> None:
|
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_ID", "1")
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,8 @@ import pytest
|
||||||
|
|
||||||
from agent.integrations.langsmith import _configure_github_proxy
|
from agent.integrations.langsmith import _configure_github_proxy
|
||||||
from agent.utils.github_app import (
|
from agent.utils.github_app import (
|
||||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
CORE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||||
|
PROXY_TOKEN_PERMISSION_LADDER,
|
||||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
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[0].kwargs["log_errors"] is False
|
||||||
assert mock_get_token.await_args_list[1].kwargs["permissions"] == (
|
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_proxy.assert_called_once_with("sandbox-123", "ghs_install")
|
||||||
mock_record.assert_called_once_with(
|
mock_record.assert_called_once_with(
|
||||||
"thread-123",
|
"thread-123",
|
||||||
"expires",
|
"expires",
|
||||||
repositories=None,
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_skips_proxy_for_non_langsmith(self) -> None:
|
async def test_skips_proxy_for_non_langsmith(self) -> None:
|
||||||
"""Non-langsmith sandboxes should skip proxy configuration."""
|
"""Non-langsmith sandboxes should skip proxy configuration."""
|
||||||
|
|
|
||||||
|
|
@ -415,8 +415,9 @@ async def test_workflow_push_restoration_falls_back_when_actions_read_unavailabl
|
||||||
|
|
||||||
assert refreshed[0]["workflows"] == "write"
|
assert refreshed[0]["workflows"] == "write"
|
||||||
assert refreshed[1]["actions"] == "read"
|
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 "actions" not in refreshed[2]
|
||||||
|
assert "workflows" not in refreshed[2]
|
||||||
|
|
||||||
|
|
||||||
async def test_non_workflow_push_runs_without_approval(monkeypatch: pytest.MonkeyPatch) -> None:
|
async def test_non_workflow_push_runs_without_approval(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue