diff --git a/agent/dashboard/thread_api.py b/agent/dashboard/thread_api.py index 299210a9..aae4bf2b 100644 --- a/agent/dashboard/thread_api.py +++ b/agent/dashboard/thread_api.py @@ -8,7 +8,7 @@ import binascii import json import logging import os -from collections.abc import AsyncIterator +from collections.abc import AsyncIterator, Mapping from datetime import UTC, datetime from typing import Any @@ -726,7 +726,7 @@ async def _sidebar_active_thread_summary( client: Any, active_thread_id: str | None, *, - fallback_threads: Mapping[str, ThreadLike], + fallback_threads: Mapping[str, dict[str, Any]], visible_thread_ids: set[str], login: str, email: str | None, @@ -752,11 +752,17 @@ async def _sidebar_active_thread_summary( _assert_thread_readable(metadata) except HTTPException: return None + # Refreshing persists latest-run metadata back to the thread; only do that + # when the caller owns it, mirroring the is_owner gate on the single-thread + # read path. A non-owner viewing a shared thread reads its last-known state + # without mutating it. + owns_thread = _user_owns_thread(metadata, login, email) summary = await _summarize_thread( client, thread, owner_login=None if include_all else login, owner_email=None if include_all else email, + refresh_active_run=owns_thread, ) return summary, _is_thread_resolved(metadata) diff --git a/agent/middleware/pr_creation_guard.py b/agent/middleware/pr_creation_guard.py index 27b6429d..2541356a 100644 --- a/agent/middleware/pr_creation_guard.py +++ b/agent/middleware/pr_creation_guard.py @@ -68,6 +68,9 @@ def _shell_command_argument(tokens: list[str], shell_index: int) -> str | None: if token == "-c" or ( token.startswith("-") and not token.startswith("--") and "c" in token[1:] ): + _, _, glued = token[1:].partition("c") + if glued: + return glued if index + 1 < len(tokens) and tokens[index + 1] not in _SHELL_SEPARATORS: return tokens[index + 1] return None diff --git a/agent/middleware/pr_verdict_guard.py b/agent/middleware/pr_verdict_guard.py index 78ebbd90..ad95883c 100644 --- a/agent/middleware/pr_verdict_guard.py +++ b/agent/middleware/pr_verdict_guard.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os import re import shlex from collections.abc import Awaitable, Callable, Mapping @@ -14,6 +15,9 @@ from langgraph.prebuilt.tool_node import ToolCallRequest from langgraph.types import Command _SHELL_SEPARATORS = {";", "&&", "||", "|", "&"} +_SHELL_EXECUTABLES = {"bash", "dash", "sh", "zsh"} +_MAX_SHELL_EXPANSION_DEPTH = 3 +_SHELL_EXPANSION_DEPTH_LIMIT_TOKEN = "__pr_verdict_guard_shell_expansion_depth_limit__" _GITHUB_REVIEWS_ENDPOINT = re.compile( r"(?:^|/)repos/[^/\s]+/[^/\s]+/pulls/\d+/reviews(?:/\d+/events)?/?$" ) @@ -55,13 +59,62 @@ def _tool_call_id(request: ToolCallRequest) -> str | None: return None -def _shell_tokens(command: str) -> list[str]: +def _split_shell_tokens(command: str) -> list[str]: try: return shlex.split(command, posix=True) except ValueError: return command.split() +def _executable_name(token: str) -> str: + return os.path.basename(token.strip("'\"")) + + +def _shell_command_argument(tokens: list[str], shell_index: int) -> str | None: + for index, token in enumerate(tokens[shell_index + 1 :], start=shell_index + 1): + if token in _SHELL_SEPARATORS: + return None + if token == "-c" or ( + token.startswith("-") and not token.startswith("--") and "c" in token[1:] + ): + _, _, glued = token[1:].partition("c") + if glued: + return glued + if index + 1 < len(tokens) and tokens[index + 1] not in _SHELL_SEPARATORS: + return tokens[index + 1] + return None + return None + + +def _has_nested_shell_command(tokens: list[str]) -> bool: + return any( + _executable_name(token) in _SHELL_EXECUTABLES + and _shell_command_argument(tokens, index) is not None + for index, token in enumerate(tokens) + ) + + +def _expand_nested_shell_tokens(tokens: list[str], depth: int = 0) -> list[str]: + if depth >= _MAX_SHELL_EXPANSION_DEPTH: + if _has_nested_shell_command(tokens): + return [*tokens, _SHELL_EXPANSION_DEPTH_LIMIT_TOKEN] + return tokens + + expanded = list(tokens) + for index, token in enumerate(tokens): + if _executable_name(token) not in _SHELL_EXECUTABLES: + continue + inner_command = _shell_command_argument(tokens, index) + if inner_command is None: + continue + expanded.extend(_expand_nested_shell_tokens(_split_shell_tokens(inner_command), depth + 1)) + return expanded + + +def _shell_tokens(command: str) -> list[str]: + return _expand_nested_shell_tokens(_split_shell_tokens(command)) + + def _subtokens_after(tokens: list[str], index: int) -> list[str]: subtokens: list[str] = [] for token in tokens[index + 1 :]: @@ -73,7 +126,7 @@ def _subtokens_after(tokens: list[str], index: int) -> list[str]: def _contains_gh_pr_review_verdict(tokens: list[str]) -> bool: for index, token in enumerate(tokens): - if token != "gh": + if _executable_name(token) != "gh": continue subtokens = _subtokens_after(tokens, index) is_pr_review = any( @@ -92,7 +145,7 @@ def _contains_gh_pr_review_verdict(tokens: list[str]) -> bool: def _contains_gh_api_review_verdict(tokens: list[str]) -> bool: for index, token in enumerate(tokens): - if token != "gh": + if _executable_name(token) != "gh": continue subtokens = _subtokens_after(tokens, index) if "api" not in subtokens: @@ -111,7 +164,7 @@ def _contains_gh_api_review_verdict(tokens: list[str]) -> bool: def _contains_direct_review_verdict(tokens: list[str]) -> bool: for index, token in enumerate(tokens): - if token != "curl": + if _executable_name(token) != "curl": continue subtokens = _subtokens_after(tokens, index) if not any(_GITHUB_REVIEWS_URL.search(subtoken) for subtoken in subtokens): @@ -129,7 +182,13 @@ def is_pr_verdict_fallback_command(command: str) -> bool: ``event=APPROVE|REQUEST_CHANGES`` to a ``/pulls/N/reviews`` endpoint, and ``curl`` to the reviews URL with a verdict event in the body) but is intentionally fail-open: shell aliases, ``gh`` aliases, ``--input`` - JSON-file bodies, and non-curl HTTP clients will not be blocked. This is + JSON-file bodies, and non-curl HTTP clients will not be blocked. The + ``-c`` form of a known shell (``bash``/``dash``/``sh``/``zsh``, whether + space-separated or glued as ``-c'...'``) is expanded to a fixed depth and + quoted/path-prefixed executables are normalized, so those do not slip + through; other shells (``ash``/``busybox``/``fish``) and stdin-fed + programs (``... | bash``, here-strings, ``bash -s``) remain fail-open. + This is acceptable because the threat model is an honest agent papering over a ``publish_review`` limitation or refusal, not an adversary trying to bypass the guardrail. Plain ``gh pr review`` (no verdict flag) and @@ -138,7 +197,8 @@ def is_pr_verdict_fallback_command(command: str) -> bool: """ tokens = _shell_tokens(command) return ( - _contains_gh_pr_review_verdict(tokens) + _SHELL_EXPANSION_DEPTH_LIMIT_TOKEN in tokens + or _contains_gh_pr_review_verdict(tokens) or _contains_gh_api_review_verdict(tokens) or _contains_direct_review_verdict(tokens) ) diff --git a/tests/github/test_pr_creation_guard.py b/tests/github/test_pr_creation_guard.py index 7aa7d4f0..a993bd41 100644 --- a/tests/github/test_pr_creation_guard.py +++ b/tests/github/test_pr_creation_guard.py @@ -43,6 +43,8 @@ def test_detects_pr_creation_fallback_commands() -> None: "/usr/bin/curl -X POST https://api.github.com/repos/langchain-ai/open-swe/pulls -d '{}'" ) assert is_pr_creation_fallback_command("bash -c 'gh pr create --draft'") + assert is_pr_creation_fallback_command("bash -c'gh pr create --draft'") + assert is_pr_creation_fallback_command("zsh -lc'gh pr create --draft'") assert is_pr_creation_fallback_command("GH_TOKEN=dummy sh -c 'gh pr create --draft'") assert is_pr_creation_fallback_command( "zsh -lc 'gh api repos/langchain-ai/open-swe/pulls -X POST -f title=x'" diff --git a/tests/github/test_pr_verdict_guard.py b/tests/github/test_pr_verdict_guard.py index d86908df..c3c1fd81 100644 --- a/tests/github/test_pr_verdict_guard.py +++ b/tests/github/test_pr_verdict_guard.py @@ -60,6 +60,28 @@ def test_detects_curl_review_verdicts() -> None: ) +def test_detects_nested_and_normalized_verdict_commands() -> None: + assert is_pr_verdict_fallback_command("/usr/bin/gh pr review 12 --approve") + assert is_pr_verdict_fallback_command( + "/usr/bin/curl -X POST https://api.github.com/repos/o/r/pulls/5/reviews " + '-d \'{"event": "APPROVE"}\'' + ) + assert is_pr_verdict_fallback_command("bash -c 'gh pr review 12 --approve'") + assert is_pr_verdict_fallback_command("bash -c'gh pr review 12 --approve'") + assert is_pr_verdict_fallback_command("zsh -lc'gh pr review 12 --approve'") + assert is_pr_verdict_fallback_command("GH_TOKEN=dummy sh -c 'gh pr review -a'") + assert is_pr_verdict_fallback_command( + "zsh -lc 'gh api repos/o/r/pulls/5/reviews -X POST -f event=REQUEST_CHANGES'" + ) + assert is_pr_verdict_fallback_command( + 'bash -c "curl -X POST https://api.github.com/repos/o/r/pulls/5/reviews ' + '-d \'{\\"event\\": \\"APPROVE\\"}\'"' + ) + assert is_pr_verdict_fallback_command( + 'bash -c \'sh -c "dash -c \\"zsh -c \\\\\\"gh pr review 12 --approve\\\\\\"\\""\'' + ) + + def test_allows_safe_review_commands() -> None: assert not is_pr_verdict_fallback_command("gh pr review 12 --comment -b 'looks good'") assert not is_pr_verdict_fallback_command("gh pr review -c") @@ -69,6 +91,8 @@ def test_allows_safe_review_commands() -> None: assert not is_pr_verdict_fallback_command("gh api repos/o/r/pulls/5/reviews") assert not is_pr_verdict_fallback_command("gh pr create --draft") assert not is_pr_verdict_fallback_command("curl https://api.github.com/repos/o/r/pulls/5") + assert not is_pr_verdict_fallback_command("bash -c 'gh pr review 12 --comment -b ok'") + assert not is_pr_verdict_fallback_command("/usr/bin/gh pr view 12 --json reviews") async def test_middleware_blocks_execute_verdict_fallbacks() -> None: