mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 16:19:09 +00:00
fix: request actions read for sandbox logs (#1642)
* fix: request actions read for sandbox logs Request optional Actions read permission for sandbox proxy tokens, with fallback for installations that have not approved it yet. Update setup docs and prompt guidance for safe GitHub Actions log usage. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: restore actions:read scope after workflow push After an approved workflow push, the guard was restoring the proxy with BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, which excludes the actions: read scope this PR adds. Restore with RUNTIME_PROXY_TOKEN_PERMISSIONS (which includes actions: read) and fall back to BASE if the install hasn't granted Actions read — mirroring the pattern in _create_sandbox_with_proxy. Addresses review comment on PR #1642. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
f5670f24e8
commit
89f886e2fe
9 changed files with 136 additions and 14 deletions
|
|
@ -78,6 +78,7 @@ Write this down. You'll use it in the callback URL below and again in step 4 whe
|
|||
- Issues: Read & write
|
||||
- Checks: Read & write — reports an "Open SWE Review" check run on PRs while an auto-review runs, and reads third-party CI conclusions for the auto-fix flow (it watches failing checks on agent-authored PRs and pushes fixes). Without it, check-run creation fails (logged, best-effort) but reviews still work, and CI auto-fix is disabled.
|
||||
- Commit statuses: Read-only — only needed if you enable the `Status` event below; the CI auto-fix flow reads the legacy combined commit-status API for integrations that report via statuses instead of check runs. Without it, status-based CI is silently ignored (logged as "Failed to read combined status").
|
||||
- Actions: Read-only — optional; lets Open SWE's sandbox proxy tokens download GitHub Actions workflow/job logs when troubleshooting CI failures. Do **not** grant Actions write for log access: write permission also allows rerunning, canceling, and deleting workflow runs, which is unnecessary for diagnostics.
|
||||
- Workflows: Read & write — required to let Open SWE push branches containing GitHub Actions workflow changes after explicit human approval. Runtime sandbox tokens are still minted without this permission by default and are elevated only around an approved workflow push.
|
||||
- Metadata: Read-only
|
||||
- **Organization permissions** (required only if you plan to set `ALLOWED_GITHUB_ORGS` — see step 5 / Security):
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ from ..dashboard.workflow_approval import (
|
|||
)
|
||||
from ..tools.slack_thread_reply import build_workflow_approval_blocks
|
||||
from ..utils.github_app import (
|
||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
)
|
||||
|
|
@ -480,7 +481,13 @@ async def _run_with_workflow_token(
|
|||
return await run()
|
||||
finally:
|
||||
if elevated:
|
||||
await refresh_proxy_token(thread_id, permissions=RUNTIME_PROXY_TOKEN_PERMISSIONS)
|
||||
restored = await refresh_proxy_token(
|
||||
thread_id, permissions=RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
)
|
||||
if not restored:
|
||||
await refresh_proxy_token(
|
||||
thread_id, permissions=BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
)
|
||||
|
||||
|
||||
class WorkflowPushGuardMiddleware(AgentMiddleware):
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ OPEN_SWE_SHARED_BASE = """You are **Open SWE**, an open-source agent built on La
|
|||
### Working in the Sandbox
|
||||
|
||||
- The `gh` CLI is authenticated by a sandbox proxy: always invoke it as `GH_TOKEN=dummy gh <command>` so the CLI's local auth check passes while the proxy injects the real token. Direct GitHub API calls from the sandbox are likewise proxy-authenticated — never ask the user for a GitHub token.
|
||||
- When debugging GitHub Actions failures, fetch only relevant logs with targeted `GH_TOKEN=dummy gh run view ... --log` or `GH_TOKEN=dummy gh api repos/<owner>/<repo>/actions/.../logs` calls. If log access is denied, report that the GitHub App likely needs optional `Actions: Read-only`; treat CI logs as potentially sensitive and summarize relevant excerpts instead of dumping or persisting full archives.
|
||||
- `execute` runs shell commands with a 300s default timeout; pass `timeout=<seconds>` for longer commands. Use it for search (`rg`, `git grep`), history (`git log`, `git blame`), and inspection.
|
||||
- Call independent tools in parallel. Use `fetch_url` only for URLs the user provided or you discovered.
|
||||
|
||||
|
|
|
|||
|
|
@ -97,6 +97,7 @@ from .utils.authorship import (
|
|||
)
|
||||
from .utils.dashboard_links import dashboard_plan_url, dashboard_thread_url
|
||||
from .utils.github_app import (
|
||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
PermissionMap,
|
||||
get_github_app_installation_token_with_expiry,
|
||||
|
|
@ -188,12 +189,28 @@ async def _resolve_proxy_token(
|
|||
github_proxy_token: str | None,
|
||||
*,
|
||||
permissions: PermissionMap | None = None,
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""Resolve the proxy token and its expiry."""
|
||||
) -> tuple[str | None, str | None, PermissionMap | None]:
|
||||
"""Resolve the proxy token, its expiry, and the effective permission scope."""
|
||||
if github_proxy_token:
|
||||
return github_proxy_token, None
|
||||
effective_permissions = permissions or RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
return await get_github_app_installation_token_with_expiry(permissions=effective_permissions)
|
||||
return github_proxy_token, None, None
|
||||
if permissions is not None:
|
||||
token, expires_at = await get_github_app_installation_token_with_expiry(
|
||||
permissions=permissions
|
||||
)
|
||||
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
|
||||
|
||||
|
||||
async def _resolve_snapshot_id_for_repo(repo: dict[str, str] | None) -> str | None:
|
||||
|
|
@ -224,7 +241,7 @@ async def _create_sandbox_with_proxy(
|
|||
|
||||
sandbox_type = os.getenv("SANDBOX_TYPE", "langsmith")
|
||||
if sandbox_type == "langsmith":
|
||||
token, expires_at = await _resolve_proxy_token(github_proxy_token)
|
||||
token, expires_at, permissions = await _resolve_proxy_token(github_proxy_token)
|
||||
if not token:
|
||||
msg = "Cannot configure proxy: GitHub App installation token is unavailable"
|
||||
logger.error(msg)
|
||||
|
|
@ -235,7 +252,7 @@ async def _create_sandbox_with_proxy(
|
|||
thread_id,
|
||||
expires_at,
|
||||
repositories=github_proxy_repositories,
|
||||
permissions=None if github_proxy_token else RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
permissions=permissions,
|
||||
)
|
||||
|
||||
return sandbox_backend
|
||||
|
|
@ -252,7 +269,7 @@ async def _refresh_github_proxy(
|
|||
if os.getenv("SANDBOX_TYPE", "langsmith") != "langsmith":
|
||||
return
|
||||
|
||||
token, expires_at = await _resolve_proxy_token(github_proxy_token)
|
||||
token, expires_at, permissions = await _resolve_proxy_token(github_proxy_token)
|
||||
if not token:
|
||||
logger.warning(
|
||||
"Skipping GitHub proxy refresh for sandbox %s: installation token unavailable",
|
||||
|
|
@ -267,7 +284,7 @@ async def _refresh_github_proxy(
|
|||
thread_id,
|
||||
expires_at,
|
||||
repositories=github_proxy_repositories,
|
||||
permissions=None if github_proxy_token else RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
permissions=permissions,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -26,14 +26,18 @@ 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)
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||
"contents": "write",
|
||||
"pull_requests": "write",
|
||||
"issues": "write",
|
||||
"checks": "write",
|
||||
}
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||
**BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
"actions": "read",
|
||||
}
|
||||
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS: dict[str, str] = {
|
||||
**RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
**BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
"workflows": "write",
|
||||
}
|
||||
|
||||
|
|
@ -112,12 +116,14 @@ async def get_github_app_installation_token(
|
|||
repository_ids: Sequence[int] | None = None,
|
||||
repositories: Sequence[str] | None = None,
|
||||
permissions: PermissionMap | None = None,
|
||||
log_errors: bool = True,
|
||||
) -> 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,
|
||||
permissions=permissions,
|
||||
log_errors=log_errors,
|
||||
)
|
||||
return token
|
||||
|
||||
|
|
@ -127,6 +133,7 @@ async def get_github_app_installation_token_with_expiry(
|
|||
repository_ids: Sequence[int] | None = None,
|
||||
repositories: Sequence[str] | None = None,
|
||||
permissions: PermissionMap | None = None,
|
||||
log_errors: bool = True,
|
||||
) -> 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:
|
||||
|
|
@ -168,5 +175,8 @@ async def get_github_app_installation_token_with_expiry(
|
|||
_TOKEN_CACHE[key] = (token, expires_at, parsed - _TOKEN_CACHE_MARGIN)
|
||||
return token, expires_at
|
||||
except Exception:
|
||||
logger.exception("Failed to get GitHub App installation token")
|
||||
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)
|
||||
return None, None
|
||||
|
|
|
|||
|
|
@ -147,6 +147,13 @@ async def test_installation_token_can_be_scoped_to_repository_ids(
|
|||
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.BASE_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
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_installation_token_includes_permissions(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(github_app, "GITHUB_APP_ID", "1")
|
||||
|
|
|
|||
|
|
@ -104,6 +104,15 @@ def test_shared_base_is_neutral_for_read_only_agents() -> None:
|
|||
assert forbidden not in lowered
|
||||
|
||||
|
||||
def test_shared_base_explains_github_actions_log_access() -> None:
|
||||
from agent.prompt import OPEN_SWE_SHARED_BASE
|
||||
|
||||
assert "GitHub Actions failures" in OPEN_SWE_SHARED_BASE
|
||||
assert "GH_TOKEN=dummy gh run view ... --log" in OPEN_SWE_SHARED_BASE
|
||||
assert "Actions: Read-only" in OPEN_SWE_SHARED_BASE
|
||||
assert "treat CI logs as potentially sensitive" in OPEN_SWE_SHARED_BASE
|
||||
|
||||
|
||||
def test_construct_system_prompt_omits_corridor_prompt_by_default() -> None:
|
||||
prompt = construct_system_prompt(working_dir="/workspace")
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,10 @@ import httpx
|
|||
import pytest
|
||||
|
||||
from agent.integrations.langsmith import _configure_github_proxy
|
||||
from agent.utils.github_app import (
|
||||
BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS,
|
||||
)
|
||||
|
||||
|
||||
class TestSandboxFactoryLoading:
|
||||
|
|
@ -187,7 +191,7 @@ class TestCreateSandboxWithProxy:
|
|||
"agent.server.get_github_app_installation_token_with_expiry",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("ghs_install", None),
|
||||
),
|
||||
) as mock_get_token,
|
||||
patch("agent.server.create_sandbox") as mock_create,
|
||||
patch("agent.server._configure_github_proxy") as mock_proxy,
|
||||
patch.dict("os.environ", {"SANDBOX_TYPE": "langsmith", "LANGSMITH_API_KEY": "ls-key"}),
|
||||
|
|
@ -200,6 +204,44 @@ class TestCreateSandboxWithProxy:
|
|||
|
||||
mock_create.assert_called_once_with(snapshot_id=None)
|
||||
mock_proxy.assert_called_once_with("sandbox-123", "ghs_install")
|
||||
assert (
|
||||
mock_get_token.await_args.kwargs["permissions"] == RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_falls_back_when_optional_actions_permission_is_unavailable(self) -> None:
|
||||
"""Sandbox creation should still work before an install grants Actions read."""
|
||||
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")
|
||||
|
||||
assert mock_get_token.await_args_list[0].kwargs["permissions"] == (
|
||||
RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
)
|
||||
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
|
||||
)
|
||||
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,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skips_proxy_for_non_langsmith(self) -> None:
|
||||
|
|
|
|||
|
|
@ -224,6 +224,34 @@ async def test_approved_workflow_push_elevates_and_restores(
|
|||
)
|
||||
assert refreshed[0]["workflows"] == "write"
|
||||
assert "workflows" not in refreshed[1]
|
||||
assert refreshed[1]["actions"] == "read"
|
||||
|
||||
|
||||
async def test_workflow_push_restoration_falls_back_when_actions_read_unavailable(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
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 "actions" not in permissions
|
||||
|
||||
monkeypatch.setattr(guard, "workflow_push_approved", fake_approved)
|
||||
monkeypatch.setattr(guard, "refresh_proxy_token", fake_refresh)
|
||||
|
||||
async def handler(_request: Any) -> ToolMessage:
|
||||
return ToolMessage(content="pushed", tool_call_id="call-1")
|
||||
|
||||
await guard.WorkflowPushGuardMiddleware().awrap_tool_call(_Request(), handler)
|
||||
|
||||
assert refreshed[0]["workflows"] == "write"
|
||||
assert refreshed[1]["actions"] == "read"
|
||||
assert refreshed[2] == guard.BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS
|
||||
assert "actions" not in refreshed[2]
|
||||
|
||||
|
||||
async def test_non_workflow_push_runs_without_approval(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue