feat(webhook): smart pre-dispatch filters + WaId-aware contact matching#379
feat(webhook): smart pre-dispatch filters + WaId-aware contact matching#379tobiasstrebitzer wants to merge 4 commits into
Conversation
The descendant selector `.modal-body label` leaked into nested labels (e.g. the filter builder's inputs), restyling controls it shouldn't. Scope it to `.modal-body > label` so only top-level modal field labels are affected.
Add optional smart filters that refine WHETHER a subscribed webhook fires, evaluated per event before delivery. Conditions (AND-combined) match on sender/recipient/body/type/mentions/fromMe/hasMedia/isGroup with is/isNot/contains/equals/matches operators; message-only conditions are skipped for non-message events, so a webhook subscribed to several families behaves sanely without per-event filter sets. A webhook with no filters behaves exactly as before (additive, zero-config). - filters/: family-aware field registry, evaluator, class-validator validation - entity column + DTO field + migration for the persisted filters - wire evaluateFilters into dispatch matching - dashboard FilterBuilder UI on the webhooks page (sender picker backed by a session-chats query) + i18n strings - unit coverage for the evaluator/validation and dispatch, plus a webhooks e2e suite
Match id filters (sender/recipient/mentions) by the engine-neutral WaId key instead of the raw JID, reusing the same identity primitives as the engine and MessageService. An engine-emitted id (any of @c.us / @s.whatsapp.net / @lid, optional :device suffix, a lid resolved to its phone) and a user-typed filter value (bare digits or a JID) both collapse to one neutral key, so: - a phone filter matches the same person across user dialects, and - a lid-addressed actor (e.g. an unresolved @lid group participant) now matches a phone filter once the persistent lid->phone table knows the mapping - previously a silent miss. WebhookService takes the cross-session LidMappingStore (optional) and threads a resolver into evaluateFilters at dispatch; the evaluator stays a pure function (resolver optional) so unit contexts need no store. No new filter fields, no UI change - the dashboard keeps storing the neutral <digits>@c.us value.
d2fffa3 to
08d2bda
Compare
rmyndharis
left a comment
There was a problem hiding this comment.
Thanks for this — the filter subsystem is genuinely well-built: the WaId-neutral contact matching, the portable jsonb/text migration, the validation wiring, and the test coverage are all solid. I'm holding it on one blocker before it can merge.
Blocker — ReDoS via the matches operator (remote event-loop freeze)
The matches operator compiles a user-supplied regex and runs RegExp.test() synchronously on the main event loop inside WebhookService.dispatch (filter-evaluator.ts safeRegexTest → webhook.service.ts dispatch filter), which runs on every inbound message. The isPotentiallyCatastrophicRegex heuristic only catches (X+)+-shaped patterns, and the length caps (MAX_REGEX_LENGTH=200, MAX_REGEX_INPUT_LENGTH=4096) don't bound the damage. Confirmed bypasses (heuristic returns false, then the loop freezes):
(a|a)+b— alternation overlap, 7 chars →.test()against ~40 input chars ran ~91 seconds..*.*.*.*x$— polynomial → still running after a 30s kill against the 4096-char-truncated body.
Because webhook create/update isn't admin-gated, any API key that can manage a session's webhooks can plant such a filter; one inbound message body then stalls the entire process (all sessions, HTTP, queue) — a remote DoS.
A vm/timeout won't fix it (V8 doesn't interrupt a synchronous regex). Any one of these unblocks:
- Drop the
matchesoperator —contains/equalscover most needs; zero ReDoS surface, no new dependency. re2— linear-time, backtracking-free engine; keepsmatches(adds a native dep).worker_threadwith a hard timeout — terminate a regex run that exceeds N ms.
Please also add tests for an alternation-overlap pattern ((a|a)+) and a polynomial one (.*.*.*x$) that assert either rejection at validation or a bounded evaluation time — the current single (a+)+ test passes while the real bypasses sail through.
Everything else looks merge-ready — happy to re-review as soon as the regex path is bounded.
The `matches` operator compiled a user-supplied regex and ran it synchronously on the event loop for every inbound message. The `isPotentiallyCatastrophicRegex` heuristic only caught nested-quantifier shapes like `(a+)+`; alternation-overlap (`(a|a)+b`) and polynomial (`.*.*.*x$`) patterns sailed through and froze the whole process on a single message. Since webhook config isn't admin-gated, any session- managing key could plant such a filter - a remote DoS. Remove `matches` entirely: `contains`/`equals` cover the real use cases at zero ReDoS surface and no new dependency. Validation now rejects the operator outright, and the regex length/input caps and heuristic are gone. Drops the operator from the dashboard FilterBuilder, the API client type, and the `matches`/`regexPlaceholder` i18n keys across all 9 locales.
|
@rmyndharis kudos for spotting this, I agree on dropping regex/matches for now., keeps the filter path simple and easy to maintain - happy to revisit a linear-time engine like re2 later if regex demand shows up. Validation now rejects the operator outright, and safeRegexTest plus the length caps and heuristic are gone, so no user-supplied pattern hits new RegExp() on the event loop anymore. Also pulled it from the dashboard FilterBuilder, the API client type, and the i18n keys across all 9 locales. |
Optional, additive pre-dispatch filters for webhook triggers. A webhook with no filters behaves
exactly as before (zero extra clicks, zero behaviour change). When filters are present, every
condition must match (AND) for the webhook to fire. Builds on the typed-
WaIdwork from #374 (thepersistent
lid -> phonetable) and the Baileys sync hardening from #375.Motivation
Webhook triggers fire on event type only (
message.received, etc.). There are legitimate reasons topre-filter before delivery (Gmail-forwarding-style rules): only fire when sender = A, body contains
"invoice", it's a group, and so on. This cuts webhook congestion and over-emitting, and - in the age of
AI - controls token/cost downstream.
message.*is the natural starting point; the design isevent-family aware so
session/groupcan slot in later without touching the evaluator.A second motivation is correctness of who a filter matches. WhatsApp addresses the same person
through several dialects (
@c.us,@s.whatsapp.net,@lid), and a privacy-id (@lid) sender's numberisn't a phone at all. A naive JID compare silently misses the same contact across dialects, and misses a
lid-addressed sender entirely. This resolves ids through the typed-
WaIdlayer + the persistentlid -> phonetable from #374, so a phone filter matches the person everywhere.What ships
sender,recipient,body,type,mentions,isGroup,fromMe,hasMedia. Operators:is/isNot(ids, enums),contains/equals/matches(text, regex),with optional
caseSensitive. Message-only conditions are skipped on non-message events, so a*-subscribed webhook still fires on session events.sender/recipient/mentionsare matched bythe neutral
WaIdkey, not the raw JID. An engine-emitted id (any dialect, optional:device, a lidthe table resolves to its phone) and a user-typed value (bare digits or a JID) both collapse to one
neutral key. So a phone filter matches the person across
@c.us/@s.whatsapp.netand any@lidthat resolves to that phone - previously a silent miss.filtersJSON column onwebhooks(portable migration:jsonbonPostgres,
texton SQLite, idempotenthasColumnguard). Exposed asfilterson the create/updateDTOs and the response shape.
backed by a session-chats query, enum chips, regex/case-sensitive text). On the webhook list, the
"N filters" badge opens a hover/focus popover summarising the configured conditions.
Commits
fix(dashboard): scope .modal-body label to direct children- unrelated CSS fix carried along.feat(webhook): smart pre-dispatch filters- filters subsystem (family-aware field registry,evaluator, class-validator validation), entity column + DTO field + migration, dispatch wiring,
FilterBuilder UI + badge popover + i18n, unit coverage + a new webhooks e2e suite.
feat(webhook): WaId-aware id matching via the lid->phone table- replaces raw-JID compare withWaId-neutral keys; injects the cross-sessionLidMappingStoreintoWebhookServiceand threads alid resolver into the evaluator (which stays a pure function - resolver optional).
Key design decisions
dispatch/withSafeFetchstructure (fix(webhook): scope by-id ops and GET /webhooks to the session (close cross-session IDOR) #323/fix(security): pin outbound webhook and media fetches to the SSRF-validated IP #338/fix(webhook): deliver session lifecycle events and key them per occurrence #335) with no delivery-layer churn, preserving theSSRF IP-pin on both the direct and queued paths.
WaId.fromEngineJid/WaId.fromUserInput/.toNeutral()(src/engine/identity/wa-id.value.ts) - the same identity layer the engine andMessageServiceuse. The dispatch-time resolver mirrors the adapter'sresolvePhone(
baileys-session-store.ts), backed byLidMappingStore.getCached.WebhookModuleimportsEngineModule(already exportsLidMappingStoreService); the store is@Optional()-injected so unit/queue contexts work without it.Verified no circular dependency: the full
AppModuleboots in the e2e suite.User/Chat/Group taxonomy. The dashboard stores the neutral
<digits>@c.usvalue; all dialect and lidnormalisation happens server-side in the evaluator.
Acceptance / testing
npm run build,npm run lint: clean.npm test: 1013 unit tests. Notable new coverage:filter-evaluator.spec.ts: cross-dialect (@c.usvs@s.whatsapp.net), bare-number <-> JID,:devicestripping, and a lid-resolution hit + no-resolver miss control (incl.mentions).webhook.service.spec.ts: dispatch-level filter behaviour for every field/operator, plus anend-to-end lid sender resolves via the table -> phone filter fires (else a silent miss) test.
npm run test:e2e: 26 (incl. the newwebhooks.e2e-spec.ts: CRUD, auth, SSRF reject, HMAC on thewire, header sanitisation, and filter behaviour; booting it confirms no circular module dep).
tsc -bclean,eslint srcclean (only pre-existing warnings elsewhere),vite buildclean.
CHANGELOG [Unreleased]entry added.Manual check (optional, Baileys)
Create a webhook with
sender is <phone>. Have a group participant who is lid-only message in. With thelid -> phonetable populated (via #372 inbound capture / sync), the webhook now fires for thatparticipant - where a raw-JID compare would have missed them.
Non-goals / follow-ups
@lidvalue to match a resolved@c.us) is out of scopeMessageService's forward-only resolution.session/groupevent families: the registry is ready for them; not wired in this PR.