mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-06 17:38:35 +00:00
fix: filter webhook route events
This commit is contained in:
parent
6e562efd37
commit
f67506dadf
@ -1968,8 +1968,10 @@ Use the HMAC header when the sender supports request signing. Bearer-style heade
|
|||||||
"provider": "github",
|
"provider": "github",
|
||||||
"secret": "${GITHUB_WEBHOOK_SECRET}",
|
"secret": "${GITHUB_WEBHOOK_SECRET}",
|
||||||
"to": "discord:repo-events",
|
"to": "discord:repo-events",
|
||||||
|
"events": ["pull_request"],
|
||||||
|
"actions": ["opened", "synchronize", "reopened", "ready_for_review"],
|
||||||
"thread": "github:{{ github.repository_full_name }}:{{ github.pull_request.number or github.issue.number or github.ref }}",
|
"thread": "github:{{ github.repository_full_name }}:{{ github.pull_request.number or github.issue.number or github.ref }}",
|
||||||
"prompt": "Handle {{ github.event }} {{ github.action }} for {{ github.repository_full_name }}.\n\n{{ body }}"
|
"prompt": "Review {{ github.repository_full_name }} PR #{{ github.pull_request.number }} after {{ github.action }}.\n\n{{ body }}"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -1977,6 +1979,7 @@ Use the HMAC header when the sender supports request signing. Bearer-style heade
|
|||||||
```
|
```
|
||||||
|
|
||||||
For `provider: "github"`, nanobot validates GitHub's `X-Hub-Signature-256` HMAC header and deduplicates deliveries by `X-GitHub-Delivery` for `dedupeTtlS` seconds.
|
For `provider: "github"`, nanobot validates GitHub's `X-Hub-Signature-256` HMAC header and deduplicates deliveries by `X-GitHub-Delivery` for `dedupeTtlS` seconds.
|
||||||
|
Use `events` and `actions` to keep setup pings, issue events, or unrelated PR actions from starting an agent turn.
|
||||||
|
|
||||||
### Template data
|
### Template data
|
||||||
|
|
||||||
@ -2000,10 +2003,12 @@ Common template variables:
|
|||||||
| `webhooks.enabled` | `true` | Enables the webhook subsystem. |
|
| `webhooks.enabled` | `true` | Enables the webhook subsystem. |
|
||||||
| `webhooks.routes.<name>.enabled` | `true` | Enables one route. Route names may contain letters, numbers, `_`, `.`, and `-`. |
|
| `webhooks.routes.<name>.enabled` | `true` | Enables one route. Route names may contain letters, numbers, `_`, `.`, and `-`. |
|
||||||
| `webhooks.routes.<name>.path` | `/webhooks/<name>` | HTTP path served by the gateway. `/health` is reserved. |
|
| `webhooks.routes.<name>.path` | `/webhooks/<name>` | HTTP path served by the gateway. `/health` is reserved. |
|
||||||
| `webhooks.routes.<name>.provider` | `generic` | `generic` or `github`. Provider controls signature and context handling. |
|
| `webhooks.routes.<name>.provider` | `generic` | Registered webhook provider. Built-ins are `generic` and `github`; provider controls signature and context handling. |
|
||||||
| `webhooks.routes.<name>.auth` | `secret` | `secret` or `none`. |
|
| `webhooks.routes.<name>.auth` | `secret` | `secret` or `none`. |
|
||||||
| `webhooks.routes.<name>.secret` | empty | Shared secret or signing secret. Use `${ENV_VAR}` placeholders for real deployments. |
|
| `webhooks.routes.<name>.secret` | empty | Shared secret or signing secret. Use `${ENV_VAR}` placeholders for real deployments. |
|
||||||
| `webhooks.routes.<name>.to` | empty | Required target address in `channel:chat` format. |
|
| `webhooks.routes.<name>.to` | empty | Required target address in `channel:chat` format. |
|
||||||
|
| `webhooks.routes.<name>.events` | `[]` | Optional provider event-name allowlist. For GitHub this matches `X-GitHub-Event`, such as `pull_request`. |
|
||||||
|
| `webhooks.routes.<name>.actions` | `[]` | Optional JSON payload `action` allowlist, such as `opened` or `synchronize`. |
|
||||||
| `webhooks.routes.<name>.thread` | empty | Optional Jinja template for the session key. Defaults to `to`. |
|
| `webhooks.routes.<name>.thread` | empty | Optional Jinja template for the session key. Defaults to `to`. |
|
||||||
| `webhooks.routes.<name>.prompt` | empty | Optional Jinja template for the inbound agent message. |
|
| `webhooks.routes.<name>.prompt` | empty | Optional Jinja template for the inbound agent message. |
|
||||||
| `webhooks.routes.<name>.sender` | `webhook` | Sender ID placed on the inbound message. |
|
| `webhooks.routes.<name>.sender` | `webhook` | Sender ID placed on the inbound message. |
|
||||||
|
|||||||
@ -313,6 +313,8 @@ class WebhookRouteConfig(Base):
|
|||||||
to: str = "" # Required when enabled, e.g. "websocket:github" or "telegram:12345".
|
to: str = "" # Required when enabled, e.g. "websocket:github" or "telegram:12345".
|
||||||
thread: str = "" # Optional explicit session key; defaults to ``to``.
|
thread: str = "" # Optional explicit session key; defaults to ``to``.
|
||||||
prompt: str = "" # Optional Jinja template; a generic event summary is used when empty.
|
prompt: str = "" # Optional Jinja template; a generic event summary is used when empty.
|
||||||
|
events: list[str] = Field(default_factory=list) # Optional provider event-name allowlist.
|
||||||
|
actions: list[str] = Field(default_factory=list) # Optional JSON payload action allowlist.
|
||||||
sender: str = "webhook"
|
sender: str = "webhook"
|
||||||
max_body_bytes: int = Field(default=1_048_576, ge=1024, le=10_485_760)
|
max_body_bytes: int = Field(default=1_048_576, ge=1024, le=10_485_760)
|
||||||
dedupe_ttl_s: int = Field(default=3_600, ge=0, le=86_400)
|
dedupe_ttl_s: int = Field(default=3_600, ge=0, le=86_400)
|
||||||
@ -328,6 +330,8 @@ class WebhookRouteConfig(Base):
|
|||||||
raise ValueError("webhook route path must start with '/'")
|
raise ValueError("webhook route path must start with '/'")
|
||||||
if "?" in self.path or "#" in self.path or any(ch.isspace() for ch in self.path):
|
if "?" in self.path or "#" in self.path or any(ch.isspace() for ch in self.path):
|
||||||
raise ValueError("webhook route path must be a clean absolute path")
|
raise ValueError("webhook route path must be a clean absolute path")
|
||||||
|
self.events = _clean_webhook_filter("events", self.events)
|
||||||
|
self.actions = _clean_webhook_filter("actions", self.actions)
|
||||||
return self
|
return self
|
||||||
|
|
||||||
|
|
||||||
@ -387,6 +391,13 @@ def _lazy_default(module_path: str, class_name: str) -> Any:
|
|||||||
return getattr(module, class_name)()
|
return getattr(module, class_name)()
|
||||||
|
|
||||||
|
|
||||||
|
def _clean_webhook_filter(name: str, values: list[str]) -> list[str]:
|
||||||
|
cleaned = [value.strip() for value in values]
|
||||||
|
if any(not value for value in cleaned):
|
||||||
|
raise ValueError(f"webhook route {name} must not contain empty values")
|
||||||
|
return cleaned
|
||||||
|
|
||||||
|
|
||||||
class ToolsConfig(Base):
|
class ToolsConfig(Base):
|
||||||
"""Tools configuration.
|
"""Tools configuration.
|
||||||
|
|
||||||
|
|||||||
@ -155,6 +155,16 @@ class WebhookRouter:
|
|||||||
remote=remote,
|
remote=remote,
|
||||||
)
|
)
|
||||||
delivery_id = context.get("delivery_id")
|
delivery_id = context.get("delivery_id")
|
||||||
|
if not _route_filter_allows(route, context):
|
||||||
|
return {
|
||||||
|
"ok": True,
|
||||||
|
"queued": False,
|
||||||
|
"ignored": True,
|
||||||
|
"route": name,
|
||||||
|
"event": context.get("event_name") or "",
|
||||||
|
"action": context.get("action") or "",
|
||||||
|
"delivery_id": delivery_id or None,
|
||||||
|
}
|
||||||
prompt = _render_prompt(route, context)
|
prompt = _render_prompt(route, context)
|
||||||
channel, chat_id = _parse_target(route.to)
|
channel, chat_id = _parse_target(route.to)
|
||||||
thread = _render_thread(route, context) or route.to
|
thread = _render_thread(route, context) or route.to
|
||||||
@ -311,6 +321,7 @@ def _template_context(
|
|||||||
provider_context = _webhook_provider(route.provider).context(headers, event)
|
provider_context = _webhook_provider(route.provider).context(headers, event)
|
||||||
event_name = provider_context.pop("event_name", "")
|
event_name = provider_context.pop("event_name", "")
|
||||||
delivery_id = provider_context.pop("delivery_id", "")
|
delivery_id = provider_context.pop("delivery_id", "")
|
||||||
|
action = _event_action(event, provider_context)
|
||||||
return {
|
return {
|
||||||
"route": {
|
"route": {
|
||||||
"name": name,
|
"name": name,
|
||||||
@ -326,6 +337,7 @@ def _template_context(
|
|||||||
"body": body_text,
|
"body": body_text,
|
||||||
"headers": _safe_headers(headers),
|
"headers": _safe_headers(headers),
|
||||||
"remote": remote or "",
|
"remote": remote or "",
|
||||||
|
"action": action,
|
||||||
"github": provider_context.get("github", {}),
|
"github": provider_context.get("github", {}),
|
||||||
"event_name": event_name,
|
"event_name": event_name,
|
||||||
"delivery_id": delivery_id,
|
"delivery_id": delivery_id,
|
||||||
@ -333,6 +345,33 @@ def _template_context(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _event_action(event: Mapping[str, Any], provider_context: Mapping[str, Any]) -> str:
|
||||||
|
github = provider_context.get("github")
|
||||||
|
if isinstance(github, Mapping):
|
||||||
|
action = github.get("action")
|
||||||
|
if isinstance(action, str):
|
||||||
|
return action
|
||||||
|
action = event.get("action")
|
||||||
|
return action if isinstance(action, str) else ""
|
||||||
|
|
||||||
|
|
||||||
|
def _route_filter_allows(route: WebhookRouteConfig, context: Mapping[str, Any]) -> bool:
|
||||||
|
return _filter_matches(route.events, context.get("event_name")) and _filter_matches(
|
||||||
|
route.actions, context.get("action")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _filter_matches(allowed: list[str], value: Any) -> bool:
|
||||||
|
if not allowed:
|
||||||
|
return True
|
||||||
|
normalized = _filter_value(value)
|
||||||
|
return normalized in {_filter_value(item) for item in allowed}
|
||||||
|
|
||||||
|
|
||||||
|
def _filter_value(value: Any) -> str:
|
||||||
|
return value.strip().lower() if isinstance(value, str) else ""
|
||||||
|
|
||||||
|
|
||||||
def _generic_context(headers: Mapping[str, str], _payload: Mapping[str, Any]) -> dict[str, Any]:
|
def _generic_context(headers: Mapping[str, str], _payload: Mapping[str, Any]) -> dict[str, Any]:
|
||||||
return {
|
return {
|
||||||
"event_name": headers.get("x-nanobot-event", ""),
|
"event_name": headers.get("x-nanobot-event", ""),
|
||||||
|
|||||||
@ -274,6 +274,76 @@ async def test_github_webhook_validates_signature_and_dedupes_delivery() -> None
|
|||||||
assert msg.metadata["webhook"]["event"] == "pull_request"
|
assert msg.metadata["webhook"]["event"] == "pull_request"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_github_webhook_event_and_action_filters_ignore_unmatched_events() -> None:
|
||||||
|
bus = MessageBus()
|
||||||
|
router = WebhookRouter(
|
||||||
|
WebhooksConfig(
|
||||||
|
routes={
|
||||||
|
"github": WebhookRouteConfig(
|
||||||
|
provider="github",
|
||||||
|
secret="github-secret",
|
||||||
|
to="discord:repo-events",
|
||||||
|
events=["pull_request"],
|
||||||
|
actions=["opened", "synchronize"],
|
||||||
|
)
|
||||||
|
}
|
||||||
|
),
|
||||||
|
bus,
|
||||||
|
)
|
||||||
|
|
||||||
|
async def send(payload: dict[str, object], event: str, delivery: str):
|
||||||
|
body = json.dumps(payload).encode()
|
||||||
|
return await router.handle(
|
||||||
|
method="POST",
|
||||||
|
path="/webhooks/github",
|
||||||
|
headers={
|
||||||
|
"X-Hub-Signature-256": _sig("github-secret", body),
|
||||||
|
"X-GitHub-Event": event,
|
||||||
|
"X-GitHub-Delivery": delivery,
|
||||||
|
},
|
||||||
|
body=body,
|
||||||
|
)
|
||||||
|
|
||||||
|
ping = await send({"zen": "Keep it logically awesome."}, "ping", "ping-1")
|
||||||
|
closed = await send(
|
||||||
|
{
|
||||||
|
"action": "closed",
|
||||||
|
"repository": {"full_name": "HKUDS/nanobot"},
|
||||||
|
"pull_request": {"title": "Add webhook support"},
|
||||||
|
},
|
||||||
|
"pull_request",
|
||||||
|
"pr-closed-1",
|
||||||
|
)
|
||||||
|
opened = await send(
|
||||||
|
{
|
||||||
|
"action": "opened",
|
||||||
|
"repository": {"full_name": "HKUDS/nanobot"},
|
||||||
|
"pull_request": {"title": "Add webhook support"},
|
||||||
|
},
|
||||||
|
"pull_request",
|
||||||
|
"pr-opened-1",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ping is not None
|
||||||
|
assert ping.status == 202
|
||||||
|
assert ping.body["queued"] is False
|
||||||
|
assert ping.body["ignored"] is True
|
||||||
|
assert ping.body["event"] == "ping"
|
||||||
|
assert closed is not None
|
||||||
|
assert closed.status == 202
|
||||||
|
assert closed.body["queued"] is False
|
||||||
|
assert closed.body["ignored"] is True
|
||||||
|
assert closed.body["action"] == "closed"
|
||||||
|
assert opened is not None
|
||||||
|
assert opened.status == 202
|
||||||
|
assert opened.body["queued"] is True
|
||||||
|
assert bus.inbound_size == 1
|
||||||
|
msg = await bus.consume_inbound()
|
||||||
|
assert "Event: pull_request" in msg.content
|
||||||
|
assert "Action: opened" in msg.content
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_github_webhook_rejects_invalid_signature() -> None:
|
async def test_github_webhook_rejects_invalid_signature() -> None:
|
||||||
bus = MessageBus()
|
bus = MessageBus()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user