fix(webhook): add sender_blocklist to prevent self-triggered loops - #52824
fix(webhook): add sender_blocklist to prevent self-triggered loops#52824cacheburner-agent wants to merge 1 commit into
Conversation
5de8460 to
a096dcf
Compare
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Adds a sender_blocklist feature to the webhook adapter to prevent self-triggered loops (e.g. the bot's own comments triggering a re-response).
Looks Good
_login()helper safely handles both dict and string sender fields- Checks both
senderandactorfields (covers GitHub's alternate payload shapes) - Logging includes route name and sender for debugging unexpected silence
- Intentionally omits
payload.user.loginper the comment (semantics vary by provider) - Comprehensive test suite: blocked sender, allowed sender, empty blocklist, actor fallback, string sender field
- Clean, focused implementation
Reviewed by Hermes Agent
When a webhook route's agent posts a reply to an external service (e.g.
a review comment on a pull request), the service fires a new event for
that comment. With no sender filter, this triggers another agent run,
which posts another comment — a feedback loop that runs until manually
killed. Reproduced on v0.17.0: ~100 spam comments in under two minutes.
Route config now accepts an optional sender_blocklist list. The adapter
checks sender identity before dispatching and returns 200
{status: ignored, reason: sender_blocklist} with no agent spawned.
Zero effect on routes that omit sender_blocklist.
Sender identity is read from payload.sender.login (GitHub/GitLab
standard) and payload.actor.login (GitHub alternate events) via a
type-safe helper that handles both dict and plain-string field shapes
without raising AttributeError. payload.user.login was considered as a
fallback but omitted — its semantics vary by provider and risk matching
the wrong identity.
A bot-identity approach (like the _is_own_message guard in NousResearch#52667 for
Telegram) cannot work here: the webhook adapter is provider-agnostic
and has no inherent self-identity to compare against.
Mirrors the existing events filter above it in structure and response
shape. Logs at INFO so ignored events appear in the default gateway log
without -v.
Tests: 5 new cases (blocked sender, allowed sender, empty blocklist
no-op, actor field fallback, plain-string sender field safety).
a096dcf to
19f5862
Compare
|
Latest push was just a rebase on @cacheburner and I tested this on our production gateway. The
Code is stable and ready for review. |
|
Thanks for this PR — webhook payload filtering has now landed on main via #60944 (salvage of @evelynburger's #57544), which generalizes this: route-level payload filters ( |
|
Amazing, thanks! |
Problem
When a webhook route's agent posts a reply to an external service (e.g. a review comment on a pull request), the service fires a new webhook event for that comment. The adapter has no way to filter by sender, so the event triggers another agent run, which posts another comment — a feedback loop that runs until manually killed.
This is the same class of bug fixed for Telegram in #52667 and tracked for Discord in openclaw#15874. The webhook platform was not covered.
Fix
Route config now accepts an optional
sender_blocklistlist. Before dispatching an agent run, the adapter checks the event payload's sender identity against the list and returns200 {status: ignored, reason: sender_blocklist}with no agent spawned. Zero effect on routes that omitsender_blocklist.Example config:
Why not a bot-identity check (like #52667)?
The Telegram fix compares against a known bot ID retrieved from the platform API at startup. That approach requires knowing the platform's identity model.
The webhook adapter is intentionally provider-agnostic — it accepts GitHub, GitLab, Bitbucket, Stripe, or any HTTP service — and has no inherent concept of self. There is no universal identity to check against.
A GitHub-specific approach (querying the authenticated user via the GitHub API at startup) was considered and rejected: it would hardcode a provider assumption into a provider-agnostic adapter and silently fail on GitLab or any other service.
Implementation details
Sender identity is read from:
payload.sender.login— GitHub/GitLab standard fieldpayload.actor.login— GitHub alternate eventspayload.user.loginwas considered as a fallback but omitted intentionally: its semantics vary by provider and risk matching the wrong identity (e.g. PR author vs. event actor). Conservative is safer.Both fields are extracted via a type-safe helper that handles dict
{"login": "..."}" and plain-string"username"shapes without raisingAttributeError` on non-standard payloads.Logs at
INFO(notDEBUG) so ignored events appear in the default gateway log without-v. The log message includes a hint for users debugging unexpected silence.Mirrors the existing
eventsfilter immediately above it in structure and response shape.Tradeoffs
sender_blocklistadds a config field, which is a larger blast radius than a pure identity check would be. It is opt-in with zero effect when unset, but it is a new surface users need to know about.Note: a self-loop burst could temporarily hit rate limits on the same route before the fix kicks in (rate limiting runs before payload parsing). Acceptable tradeoff but worth being aware of.
If there is a cleaner approach that avoids the config addition — or if the webhook platform is not the intended surface for agent-driven workflows that post back to the originating service — feedback is welcome before this merges.
Tests
5 new cases in
tests/gateway/test_webhook_adapter.py:200 status=ignored, no agent run started202)actorfield fallback works whensenderis absentsenderfield handled withoutAttributeError