diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 222adf4c2ea1..2c1c03fe8494 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -500,6 +500,28 @@ async def _handle_webhook(self, request: "web.Request") -> "web.Response": {"status": "ignored", "event": event_type} ) + # Check action filter (e.g. gate a pull_request route to just + # opened/reopened/synchronize so a single human operation doesn't + # spawn a run on every action GitHub emits). + # + # Fail-open on a missing/empty action: the `and action` clause means a + # delivery whose payload carries no `action` field is processed rather + # than ignored, even when an allow-list is configured. Real GitHub + # pull_request deliveries always carry an action; dropping an event we + # cannot classify would be worse than processing it. + allowed_actions = route_config.get("actions", []) + action = payload.get("action", "") + if allowed_actions and action and action not in allowed_actions: + logger.debug( + "[webhook] Ignoring action %s for route %s (allowed: %s)", + action, + route_name, + allowed_actions, + ) + return web.json_response( + {"status": "ignored", "action": action} + ) + # Format prompt from template prompt_template = route_config.get("prompt", "") prompt = self._render_prompt( diff --git a/scripts/release.py b/scripts/release.py index 36cc15008a0b..7b50ccc4d1fc 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -114,6 +114,7 @@ "github@aldo.pw": "aldoeliacim", "max@c60spaceship.com": "MaxFreedomPollard", "achaljhawar03@gmail.com": "achaljhawar", + "casey@geeknest.com": "cwest", "claytonchew@ClaytonMacMiniM4.local": "claytonchew", "hbentel@gmail.com": "hbentel", "JustinBao@outlook.com": "justinbao19", diff --git a/tests/gateway/test_webhook_integration.py b/tests/gateway/test_webhook_integration.py index 9312ac0e999f..c97b5c277b04 100644 --- a/tests/gateway/test_webhook_integration.py +++ b/tests/gateway/test_webhook_integration.py @@ -143,6 +143,152 @@ async def _capture(event: MessageEvent): assert event.message_id == "gh-delivery-001" +# =================================================================== +# Test 1b: Action filter +# =================================================================== + +class TestActionFilter: + """A route may declare an ``actions`` allow-list to gate which payload + actions trigger a run. This prevents a route subscribed to a whole event + (e.g. ``pull_request``) from firing on every action GitHub emits for one + human operation (opened, closed, reopened, synchronize, ...).""" + + async def _post(self, routes, action): + """POST a PR payload with the given action; return (resp_json, events).""" + secret = "gh-webhook-test-secret" + adapter = _make_adapter(routes) + + captured_events: list[MessageEvent] = [] + + async def _capture(event: MessageEvent): + captured_events.append(event) + + adapter.handle_message = _capture + + app = _create_app(adapter) + payload = {**GITHUB_PR_PAYLOAD, "action": action} + body = json.dumps(payload).encode() + sig = _github_signature(body, secret) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/webhooks/github-pr", + data=body, + headers={ + "Content-Type": "application/json", + "X-GitHub-Event": "pull_request", + "X-Hub-Signature-256": sig, + "X-GitHub-Delivery": f"gh-delivery-{action}", + }, + ) + data = await resp.json() + + # Let any asyncio.create_task fire + await asyncio.sleep(0.05) + return data, captured_events + + @pytest.mark.asyncio + async def test_disallowed_action_is_ignored(self): + """An action absent from the allow-list is dropped before dispatch.""" + routes = { + "github-pr": { + "secret": "gh-webhook-test-secret", + "events": ["pull_request"], + "actions": ["opened", "reopened", "synchronize"], + "prompt": "Review PR #{number}", + "deliver": "log", + } + } + data, events = await self._post(routes, "closed") + assert data["status"] == "ignored" + assert data["action"] == "closed" + assert len(events) == 0 + + @pytest.mark.asyncio + async def test_allowed_action_passes(self): + """An action on the allow-list passes through to dispatch.""" + routes = { + "github-pr": { + "secret": "gh-webhook-test-secret", + "events": ["pull_request"], + "actions": ["opened", "reopened", "synchronize"], + "prompt": "Review PR #{number}", + "deliver": "log", + } + } + for action in ("opened", "synchronize"): + data, events = await self._post(routes, action) + assert data["status"] == "accepted", action + assert len(events) == 1, action + + @pytest.mark.asyncio + async def test_no_actions_filter_allows_all(self): + """With no ``actions`` configured, every action passes (backward-compat).""" + routes = { + "github-pr": { + "secret": "gh-webhook-test-secret", + "events": ["pull_request"], + "prompt": "Review PR #{number}", + "deliver": "log", + } + } + for action in ("opened", "closed", "reopened"): + data, events = await self._post(routes, action) + assert data["status"] == "accepted", action + assert len(events) == 1, action + + @pytest.mark.asyncio + async def test_missing_action_fails_open(self): + """A delivery with no ``action`` field is processed even when an + allow-list is set (fail-open). Real GitHub pull_request deliveries + always carry an action; dropping an unclassifiable event would be worse + than processing it. This pins the ``and action`` clause of the guard so + a future tightening to ``action not in allowed_actions`` can't silently + start dropping action-less deliveries.""" + secret = "gh-webhook-test-secret" + routes = { + "github-pr": { + "secret": secret, + "events": ["pull_request"], + "actions": ["opened", "reopened", "synchronize"], + "prompt": "Review PR #{number}", + "deliver": "log", + } + } + adapter = _make_adapter(routes) + + captured_events: list[MessageEvent] = [] + + async def _capture(event: MessageEvent): + captured_events.append(event) + + adapter.handle_message = _capture + + app = _create_app(adapter) + # Payload with the action key removed entirely. + payload = {k: v for k, v in GITHUB_PR_PAYLOAD.items() if k != "action"} + assert "action" not in payload + body = json.dumps(payload).encode() + sig = _github_signature(body, secret) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/webhooks/github-pr", + data=body, + headers={ + "Content-Type": "application/json", + "X-GitHub-Event": "pull_request", + "X-Hub-Signature-256": sig, + "X-GitHub-Delivery": "gh-delivery-no-action", + }, + ) + data = await resp.json() + + await asyncio.sleep(0.05) + assert data["status"] == "accepted" + assert len(captured_events) == 1 + + # =================================================================== # Test 2: Skills injected into prompt # =================================================================== diff --git a/website/docs/user-guide/messaging/webhooks.md b/website/docs/user-guide/messaging/webhooks.md index d7678ba49f85..ad358c46602b 100644 --- a/website/docs/user-guide/messaging/webhooks.md +++ b/website/docs/user-guide/messaging/webhooks.md @@ -79,6 +79,7 @@ Routes define how different webhook sources are handled. Each route is a named e | Property | Required | Description | |----------|----------|-------------| | `events` | No | List of event types to accept (e.g. `["pull_request"]`). If empty, all events are accepted. Event type is read from `X-GitHub-Event`, `X-GitLab-Event`, or `event_type` in the payload. | +| `actions` | No | List of payload `action` values to accept (e.g. `["opened", "reopened", "synchronize"]`). If empty, all actions are accepted. A delivery whose payload has no `action` field is always accepted (fail-open), even when this list is set. Use this to gate a route subscribed to a whole event — a single human operation on a pull request emits several deliveries (`opened`, `closed`, `reopened`, `synchronize`, …), and without an allow-list every one of them triggers a run. | | `secret` | **Yes** | HMAC secret for signature validation. Falls back to the global `secret` if not set on the route. Set to `"INSECURE_NO_AUTH"` for testing only (skips validation). | | `prompt` | No | Template string with dot-notation payload access (e.g. `{pull_request.title}`). If omitted, the full JSON payload is dumped into the prompt. | | `skills` | No | List of skill names to load for the agent run. | @@ -98,6 +99,7 @@ platforms: routes: github-pr: events: ["pull_request"] + actions: ["opened", "reopened", "synchronize"] secret: "github-webhook-secret" prompt: | Review this pull request: