From 67c782c295c1221219ff98f19037f400a05a0b67 Mon Sep 17 00:00:00 2001 From: Aran Yogesh Date: Thu, 9 Apr 2026 11:45:08 -0700 Subject: [PATCH] fix: block open-swe from approving PRs (#1177) * fix: block open-swe from approving PRs * linting * fix: add case-insensitive APPROVE guard and unit tests --- agent/tools/github_review.py | 18 +++++++++++++--- tests/test_github_review.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 tests/test_github_review.py diff --git a/agent/tools/github_review.py b/agent/tools/github_review.py index b9048856..7f5b71cf 100644 --- a/agent/tools/github_review.py +++ b/agent/tools/github_review.py @@ -93,8 +93,8 @@ def create_pr_review( Args: pull_number: The PR number to review. - body: The review body text (required for APPROVE/REQUEST_CHANGES, optional for COMMENT). - event: The review action - one of APPROVE, REQUEST_CHANGES, or COMMENT. + body: The review body text (required for REQUEST_CHANGES, optional for COMMENT). + event: The review action - one of REQUEST_CHANGES or COMMENT. APPROVE is not allowed. comments: Optional list of review comments. Each comment dict should have: - path (str): The relative file path to comment on. - body (str): The comment text. @@ -107,6 +107,12 @@ def create_pr_review( Returns: Dictionary with success status and the created review data. """ + if event.upper() == "APPROVE": + return { + "success": False, + "error": "APPROVE is not allowed. Use COMMENT or REQUEST_CHANGES.", + } + repo_config = _get_repo_config() if not repo_config: return {"success": False, "error": "No repo config found"} @@ -229,11 +235,17 @@ def submit_pr_review( pull_number: The PR number. review_id: The ID of the pending review to submit. body: Optional body text for the review submission. - event: The review action - one of APPROVE, REQUEST_CHANGES, or COMMENT. + event: The review action - one of REQUEST_CHANGES or COMMENT. APPROVE is not allowed. Returns: Dictionary with success status and the submitted review data. """ + if event.upper() == "APPROVE": + return { + "success": False, + "error": "APPROVE is not allowed. Use COMMENT or REQUEST_CHANGES.", + } + repo_config = _get_repo_config() if not repo_config: return {"success": False, "error": "No repo config found"} diff --git a/tests/test_github_review.py b/tests/test_github_review.py new file mode 100644 index 00000000..2d5c9fd4 --- /dev/null +++ b/tests/test_github_review.py @@ -0,0 +1,40 @@ +"""Tests for github_review tool guards.""" + +from unittest.mock import patch + +from agent.tools.github_review import create_pr_review, submit_pr_review + + +class TestApproveBlocked: + """APPROVE event must be rejected in both create and submit.""" + + def test_create_pr_review_blocks_approve(self): + result = create_pr_review(pull_number=1, body="lgtm", event="APPROVE") + assert result["success"] is False + assert "APPROVE is not allowed" in result["error"] + + def test_submit_pr_review_blocks_approve(self): + result = submit_pr_review(pull_number=1, review_id=1, event="APPROVE") + assert result["success"] is False + assert "APPROVE is not allowed" in result["error"] + + def test_create_pr_review_blocks_approve_lowercase(self): + result = create_pr_review(pull_number=1, body="lgtm", event="approve") + assert result["success"] is False + assert "APPROVE is not allowed" in result["error"] + + def test_submit_pr_review_blocks_approve_mixed_case(self): + result = submit_pr_review(pull_number=1, review_id=1, event="Approve") + assert result["success"] is False + assert "APPROVE is not allowed" in result["error"] + + @patch("agent.tools.github_review._get_repo_config", return_value=None) + def test_create_pr_review_allows_comment(self, _mock): + result = create_pr_review(pull_number=1, body="looks good", event="COMMENT") + # Will fail with "No repo config found" but NOT with the approve error + assert "APPROVE is not allowed" not in result.get("error", "") + + @patch("agent.tools.github_review._get_repo_config", return_value=None) + def test_create_pr_review_allows_request_changes(self, _mock): + result = create_pr_review(pull_number=1, body="fix this", event="REQUEST_CHANGES") + assert "APPROVE is not allowed" not in result.get("error", "")