From 00a99c86c420c249b2e69b116dd3cc80f85db798 Mon Sep 17 00:00:00 2001 From: "seahaven-openswe[bot]" <296972425+seahaven-openswe[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:16:26 -0400 Subject: [PATCH 1/4] chore: port question-answering prompt clarification (#1680) (#178) Port upstream 304032fa: clarify that information-only answers should check out relevant repos first for context, answer fully inline, and only post a concise summary to Slack threads. The fork already carried the shared-base Slack guidance; this adds the missing TASK_EXECUTION_SECTION update and its test coverage. Refs: #146 Co-authored-by: amoussa1229 <166072409+amoussa1229@users.noreply.github.com> --- agent/prompt.py | 2 +- docs/upstream-sync/triage.jsonl | 2 +- docs/upstream-sync/triage.md | 2 +- tests/test_github_comment_prompts.py | 10 ++++++++++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/agent/prompt.py b/agent/prompt.py index 61826685..450acce8 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -199,7 +199,7 @@ If a Slack- or GitHub-triggered request asks you to review a GitHub pull request **For code-change tasks:** Understand the task and explore relevant files first. Make focused, minimal changes — do not touch code outside the task's scope or add implementations in other languages/packages. Verify with linters and only the tests related to your changes. Then commit, push, and (when a PR is warranted) open/update the draft PR — see Committing below. -**For information-only requests:** Gather what you need and answer in the source channel. Never leave a question unanswered. Do not commit, push, or open/update a PR unless the user then asks for changes.""" +**For information-only requests:** First identify any relevant git repositories and check them out before answering, so your response is grounded in current repo state. Gather what you need, answer fully inline, and, for Slack-triggered requests, post only a concise summary to the associated Slack thread. Never leave a question unanswered. Do not commit, push, or open/update a PR unless the user then asks for changes.""" CORRIDOR_PROMPT = """--- diff --git a/docs/upstream-sync/triage.jsonl b/docs/upstream-sync/triage.jsonl index bf884fe8..9c7c29dc 100644 --- a/docs/upstream-sync/triage.jsonl +++ b/docs/upstream-sync/triage.jsonl @@ -72,7 +72,7 @@ {"sha": "3dbc0282", "pr": 1676, "subject": "fix: preserve plan redirects after login (#1676)", "disposition": "landed", "reason": "FLAG-HUMAN: follow-on to landed #1668 refining sanitize_redirect_to (open-redirect auth surface); not a dup", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-09T17:10:22Z"} {"sha": "c75cbb1f", "pr": 1677, "subject": "feat: re-add Fable 5 with an admin toggle to disable it (#1677)", "disposition": "landed", "reason": "Ported + Bedrock-converted onto dev via feat/readd-fable5-bedrock; anthropic: Fable ID mapped to bedrock_converse:us.anthropic.claude-fable-5.", "branch": "fable-admin-toggle", "local_sha": null, "updated": "2026-07-10T17:07:19Z"} {"sha": "bb104d93", "pr": 1679, "subject": "fix: submit plan comments with cmd enter (#1679)", "disposition": "landed", "reason": "applies clean but edits fork-diverged PlanReview.tsx (#130); needs UI/e2e validation — separate PR", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-09T17:10:22Z"} -{"sha": "304032fa", "pr": 1680, "subject": "chore: clarify question answering prompt (#1680)", "disposition": "deferred", "reason": "reword Slack info-only answer guidance; conflicts w/ fork's customized Slack prompt", "branch": "prompt-tweaks", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} +{"sha": "304032fa", "pr": 1680, "subject": "chore: clarify question answering prompt (#1680)", "disposition": "landed", "reason": "reword Slack info-only answer guidance; conflicts w/ fork's customized Slack prompt", "branch": "prompt-tweaks", "local_sha": null, "updated": "2026-07-13T17:06:27Z"} {"sha": "5003c953", "pr": 1683, "subject": "feat: open Linear-triggered PRs as the triggering user (#1683)", "disposition": "deferred", "reason": "FLAG-HUMAN: adds linear to resolve_github_token per-user OAuth branch (auth surface); depends on #1626 linear.py", "branch": "linear-pr-as-user", "local_sha": null, "updated": "2026-07-08T20:14:42Z"} {"sha": "feb7ac98", "pr": 1689, "subject": "feat(web): surface thread sandbox ID with touch-friendly menu (#1689)", "disposition": "landed", "reason": "Ported to dev via feat/thread-sandbox-id-sidebar.", "branch": "dashboard-ui", "local_sha": null, "updated": "2026-07-10T16:48:54Z"} {"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"} diff --git a/docs/upstream-sync/triage.md b/docs/upstream-sync/triage.md index 01f37d07..90722286 100644 --- a/docs/upstream-sync/triage.md +++ b/docs/upstream-sync/triage.md @@ -56,6 +56,7 @@ Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred ro | `3dbc0282` | #1676 | fix: preserve plan redirects after login (#1676) | Landed | FLAG-HUMAN: follow-on to landed #1668 refining sanitize_redirect_to (open-redirect auth surface); not a dup | plan-approval | | `c75cbb1f` | #1677 | feat: re-add Fable 5 with an admin toggle to disable it (#1677) | Landed | Ported + Bedrock-converted onto dev via feat/readd-fable5-bedrock; anthropic: Fable ID mapped to bedrock_converse:us.anthropic.claude-fable-5. | fable-admin-toggle | | `bb104d93` | #1679 | fix: submit plan comments with cmd enter (#1679) | Landed | applies clean but edits fork-diverged PlanReview.tsx (#130); needs UI/e2e validation — separate PR | plan-approval | +| `304032fa` | #1680 | chore: clarify question answering prompt (#1680) | Landed | reword Slack info-only answer guidance; conflicts w/ fork's customized Slack prompt | prompt-tweaks | | `feb7ac98` | #1689 | feat(web): surface thread sandbox ID with touch-friendly menu (#1689) | Landed | Ported to dev via feat/thread-sandbox-id-sidebar. | dashboard-ui | | `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 | @@ -90,7 +91,6 @@ Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred ro | `4f8bc2dd` | #1692 | refactor: simplify open-swe agent sandbox lifecycle (#1692) | Deferred | FLAG-HUMAN: structural rewrite of ensure_sandbox_for_thread (drops __creating__ 4-case sentinel) | sandbox-refactor | | `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 | -| `304032fa` | #1680 | chore: clarify question answering prompt (#1680) | Deferred | reword Slack info-only answer guidance; conflicts w/ fork's customized Slack prompt | prompt-tweaks | | `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 | diff --git a/tests/test_github_comment_prompts.py b/tests/test_github_comment_prompts.py index f87d95c4..e08e4e01 100644 --- a/tests/test_github_comment_prompts.py +++ b/tests/test_github_comment_prompts.py @@ -148,11 +148,21 @@ def test_construct_system_prompt_does_not_require_pr_for_questions() -> None: assert "Do not create commits, branches, or pull requests for questions" in prompt assert "For information-only requests" in prompt + assert "check them out before answering" in prompt + assert "answer fully inline" in prompt assert "open or update a draft PR when the user asks for one" in prompt assert "Always Create PRs Policy Override" not in prompt assert "Always push, open/update the draft PR" not in prompt +def test_shared_base_summarizes_slack_information_answers() -> None: + from agent.prompt import OPEN_SWE_SHARED_BASE + + assert "Slack-triggered information-only answers" in OPEN_SWE_SHARED_BASE + assert "post only a concise summary" in OPEN_SWE_SHARED_BASE + assert "complete answer inline" in OPEN_SWE_SHARED_BASE + + def test_construct_system_prompt_includes_always_create_prs_override() -> None: prompt = construct_system_prompt(working_dir="/workspace", create_prs=True) From df1f4a0e5d8d4e3db5eb22a7e5d2139eeab580c5 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Wed, 8 Jul 2026 09:06:53 -0400 Subject: [PATCH 2/4] 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: From 45567e007f295c46cb29247c438fe87cf69462dd Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Mon, 13 Jul 2026 13:40:39 -0400 Subject: [PATCH 3/4] fix(open-swe): harden proxy-token restore and mint error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/middleware/workflow_push_guard.py | 32 ++++++++----- agent/server.py | 4 +- agent/utils/github_app.py | 25 +++++++++- agent/utils/github_proxy.py | 17 +++++++ tests/test_github_app.py | 64 +++++++++++++++++++++++++ tests/test_workflow_push_guard.py | 41 ++++++++++++++++ 6 files changed, 168 insertions(+), 15 deletions(-) diff --git a/agent/middleware/workflow_push_guard.py b/agent/middleware/workflow_push_guard.py index 50b8b0b5..ccaab820 100644 --- a/agent/middleware/workflow_push_guard.py +++ b/agent/middleware/workflow_push_guard.py @@ -35,7 +35,7 @@ from ..utils.github_app import ( 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, ) - if not await refresh_proxy_token( + restored = await refresh_proxy_token( thread_id, permissions=CORE_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, - ) + ) + 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, + ) class WorkflowPushGuardMiddleware(AgentMiddleware): diff --git a/agent/server.py b/agent/server.py index 0fe3beab..72658ec7 100644 --- a/agent/server.py +++ b/agent/server.py @@ -227,8 +227,8 @@ async def _resolve_proxy_token( continue if index: logger.warning( - "GitHub proxy token minted with reduced scope %s; installation is " - "missing higher-privilege grants", + "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 diff --git a/agent/utils/github_app.py b/agent/utils/github_app.py index f6907c98..3a488683 100644 --- a/agent/utils/github_app.py +++ b/agent/utils/github_app.py @@ -189,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/tests/test_github_app.py b/tests/test_github_app.py index f5f923d1..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 @@ -183,6 +185,68 @@ def test_proxy_token_ladder_descends_to_core() -> None: ) +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_workflow_push_guard.py b/tests/test_workflow_push_guard.py index b6737810..3ce890a2 100644 --- a/tests/test_workflow_push_guard.py +++ b/tests/test_workflow_push_guard.py @@ -420,6 +420,47 @@ async def test_workflow_push_restoration_falls_back_when_actions_read_unavailabl 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: # Head contains workflow files, but they are identical to the trusted remote base, # so no approval is required for a code-only change. From be379e6d90e15faadad829243323b0e5accdc53e Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Mon, 13 Jul 2026 14:15:55 -0400 Subject: [PATCH 4/4] 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. --- docs/upstream-sync/triage.jsonl | 2 +- docs/upstream-sync/triage.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 |