diff --git a/agent/middleware/workflow_push_guard.py b/agent/middleware/workflow_push_guard.py index c0124b83..42f1ee7a 100644 --- a/agent/middleware/workflow_push_guard.py +++ b/agent/middleware/workflow_push_guard.py @@ -42,6 +42,13 @@ from ..utils.slack import post_slack_thread_reply_with_ts logger = logging.getLogger(__name__) _WORKFLOW_PREFIX = ".github/workflows/" +# Appended to every block reason so the agent always sees how to recover. The guard blocks +# because it cannot inspect the push, not because the command is dangerous; re-issuing it as a +# single plain git command makes it inspectable and lets it through. +_BLOCK_REMEDY = ( + " Re-issue it as a single plain command with no shell operators (no &&, ;, |, (), etc.) — " + "`git push origin `, or `git -C push origin ` to target a subdirectory." +) _SHELL_OPERATORS = {";", "|", "||", "&"} _REF_NAME = re.compile(r"^[A-Za-z0-9._/@+-]+$") _GIT_OBJECT_ID = re.compile(r"^[0-9a-fA-F]{40,64}$") @@ -307,10 +314,7 @@ def _parse_git_push(command: str) -> ParsedGitPush | _BlockedGitPush | None: # No push the token scan can normalize. Fail closed if one is hidden behind shell # metacharacters/expansion; otherwise leave the command untouched. if _maybe_obfuscated_git_push(stripped): - return _BlockedGitPush( - "possible git push obscured by shell metacharacters; issue it as a plain " - "`git push origin `" - ) + return _BlockedGitPush("possible git push obscured by shell metacharacters") return None if _UNSAFE_RAW_COMMAND.search(stripped) or "&" in stripped.replace("&&", ""): return _BlockedGitPush("unsafe shell characters in git push command") @@ -714,7 +718,13 @@ def _blocked_message( ) -> ToolMessage: status = "rejected" if already_rejected else "approval_required" if blocked: - error = change.blocked_reason or "This git push command is not recognized as safe." + error = ( + (change.blocked_reason or "This git push command is not recognized as safe.").rstrip( + "." + ) + + "." + + _BLOCK_REMEDY + ) error_type = "WorkflowPushBlocked" elif stale: error = ( diff --git a/tests/test_workflow_push_guard.py b/tests/test_workflow_push_guard.py index ac38e1ea..a471bdf0 100644 --- a/tests/test_workflow_push_guard.py +++ b/tests/test_workflow_push_guard.py @@ -990,3 +990,30 @@ async def test_unreadable_head_tree_blocks(monkeypatch: pytest.MonkeyPatch) -> N assert result.status == "error" payload = json.loads(str(result.content)) assert payload["error_type"] == "WorkflowPushBlocked" + # The block must tell the agent how to recover, or it just retries other chained forms. + assert "git push origin " in payload["error"] + assert "git -C push" in payload["error"] + + +def test_blocked_message_appends_actionable_remedy() -> None: + change = guard.WorkflowPushChange( + fingerprint="fp", + repo="acme/app", + branch="feature", + files=[], + head_sha="deadbeef", + remote="origin", + local_ref="feature", + remote_ref="feature", + fixed_command="", + blocked=True, + blocked_reason="chained shell commands", + ) + message = guard._blocked_message(change, blocked=True) + payload = json.loads(str(message.content)) + error = payload["error"] + # Original reason is preserved, and the remedy is appended exactly once. + assert error.startswith("chained shell commands.") + assert "git push origin " in error + assert "git -C push" in error + assert error.count("Re-issue it as a single plain command") == 1