Repository navigation
fix(meta): validate and preserve Coexistence webhook batches - #2749
Open
christianini-debug wants to merge 2 commits into
Open
christianini-debug wants to merge 2 commits into
christianini-debug wants to merge 2 commits into
Conversation
Contributor
Reviewer's GuideThis PR replaces first-item/asynchronous Meta webhook handling with ordered, awaited processing of every entry, change, message, echo, and status, while isolating integration and tenant lookups, preventing duplicate media persistence, and propagating failures. A production-bundled regression suite verifies coexistence routing, directionality, ordering, persistence, error behavior, and EvoHub compatibility. Sequence diagram for ordered Meta coexistence webhook processingsequenceDiagram
participant Meta
participant Controller as MetaController
participant Instance as WhatsAppInstance
participant Service as BusinessStartupService
participant Handler as eventHandler
participant Chatwoot
participant Bot as chatbotController
Meta->>Controller: receiveWebhook(data)
loop each entry and change
Controller->>Controller: instance.findFirst(number, channelIntegration)
Controller->>Instance: connectToWhatsapp(single-change payload)
Instance->>Service: connectToWhatsapp(data)
Service->>Handler: eventHandler(incoming messages)
Handler->>Service: sendDataWebhook(MESSAGES_UPSERT)
Service->>Chatwoot: eventWhatsapp(...)
Service->>Bot: emit(...)
Service->>Handler: eventHandler(message_echoes, isEcho=true)
Handler->>Service: sendDataWebhook(MESSAGES_UPSERT)
end
Controller-->>Meta: success after all processing
Flow diagram for coexistence message and echo routingflowchart TD
A[Webhook change] --> B{messages present?}
B -->|yes| C[eventHandler incoming messages]
B -->|no| D{message_echoes or smb_message_echoes present?}
C --> D
D -->|yes| E[eventHandler echoes with isEcho=true]
D -->|no| F{statuses present?}
E --> F
F -->|yes| G[eventHandler statuses]
F -->|no| H[Await completion]
G --> H
C --> I[Incoming remoteJid from message.from]
E --> J[Outgoing remoteJid from message.to]
I --> K[Persist and emit events]
J --> K
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/api/integrations/channel/meta/meta.controller.ts" line_range="52" />
<code_context>
const instance = await this.prismaRepository.instance.findFirst({
- where: { number: numberId },
+ where: { number: numberId, integration: this.channelIntegration },
});
</code_context>
<issue_to_address>
**Forged webhooks reach tenant instances**
When an unauthenticated caller POSTs a `whatsapp_business_account` payload containing a known phone number ID and forged message data, `receiveWebhook` trusts each change’s `metadata.phone_number_id` to select an instance and passes the submitted payload to its channel; the Meta POST route does not verify a signature, so an unauthenticated caller can forge messages that are persisted or delivered to that tenant’s integrations.
Verify Meta’s `X-Hub-Signature-256` against the raw request body before dispatching any webhook payload.
</issue_to_address>
### Comment 2
<location path="src/api/integrations/channel/meta/whatsapp.business.service.ts" line_range="174" />
<code_context>
- if (!from) return false;
-
- return from === displayPhone || from === phoneNumberId;
+ return !!from && from === displayPhone;
}
</code_context>
<issue_to_address>
**Business messages use wrong conversation**
When a business-originated message has `from` equal to `metadata.phone_number_id`, `isCloudApiFromMe` returns false, so `resolveMessageRemoteId` uses the business ID instead of the customer recipient and treats the message as inbound. The event is stored and delivered in the wrong conversation.
Recognize `phone_number_id` as a business sender in `isCloudApiFromMe`.
</issue_to_address>
### Comment 3
<location path="src/api/integrations/channel/meta/whatsapp.business.service.ts" line_range="151" />
<code_context>
+ await this.eventHandler({ ...content, messages: echoes, statuses: undefined, isEcho: true });
+ }
+ if (Array.isArray(content.statuses) && content.statuses.length) {
+ await this.eventHandler({ ...content, messages: undefined });
+ }
+ }
</code_context>
<issue_to_address>
**Status events are acknowledged early**
When a status update or deletion triggers an asynchronous webhook or Chatwoot dispatch, `messageHandle` does not await `sendDataWebhook` for these events, so `eventHandler` and the controller can acknowledge the request before delivery completes. A rejected dispatch is not propagated, and the status event is lost.
Await each status-update and deletion dispatch in `messageHandle` so delivery failures propagate to the caller.
Also at `src/api/integrations/channel/meta/whatsapp.business.service.ts:866`, `src/api/integrations/channel/meta/whatsapp.business.service.ts:997`.
</issue_to_address>
### Comment 4
<location path="src/api/integrations/channel/meta/whatsapp.business.service.ts" line_range="991" />
<code_context>
message.type === 'reaction'
) {
- await this.messageHandle(content, database, settings);
+ await this.messageHandle({ ...content, messages: [message], statuses: undefined }, database, settings);
} else {
this.logger.warn(`Tipo de mensaje no reconocido: ${message.type}`);
</code_context>
<issue_to_address>
**Contacts get the wrong name**
When a webhook value contains messages from multiple senders and corresponding contact entries, `eventHandler` narrows each call to one message but retains the full contacts array, and `messageHandle` reads `contacts[0]`. Later messages therefore use the first sender’s profile name and can overwrite or emit the wrong contact identity.
Pass the contact matching each message to `messageHandle`, or select the matching contact there instead of always using `contacts[0]`.
Also at `src/api/integrations/channel/meta/whatsapp.business.service.ts:975`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 4 findings to address first, and if the batch iteration or integration filtering is wrong, a webhook change could be delivered to the wrong tenant, duplicate persisted messages, or trigger incorrect outbound webhooks and bot activity. Reverting stops future processing but cannot retract messages, database records, or notifications that were already created.
Blocking findings: src/api/integrations/channel/meta/meta.controller.ts:52, src/api/integrations/channel/meta/whatsapp.business.service.ts:174, src/api/integrations/channel/meta/whatsapp.business.service.ts:151, src/api/integrations/channel/meta/whatsapp.business.service.ts:991
|
|
||
| const instance = await this.prismaRepository.instance.findFirst({ | ||
| where: { number: numberId }, | ||
| where: { number: numberId, integration: this.channelIntegration }, |
Contributor
There was a problem hiding this comment.
🟠 High · Forged webhooks reach tenant instances
When an unauthenticated caller POSTs a whatsapp_business_account payload containing a known phone number ID and forged message data, receiveWebhook trusts each change’s metadata.phone_number_id to select an instance and passes the submitted payload to its channel; the Meta POST route does not verify a signature, so an unauthenticated caller can forge messages that are persisted or delivered to that tenant’s integrations.
Verify Meta’s X-Hub-Signature-256 against the raw request body before dispatching any webhook payload.
Prompt for AI agents
In `src/api/integrations/channel/meta/meta.controller.ts` at line 52:
**Forged webhooks reach tenant instances**
When an unauthenticated caller POSTs a `whatsapp_business_account` payload containing a known phone number ID and forged message data, `receiveWebhook` trusts each change’s `metadata.phone_number_id` to select an instance and passes the submitted payload to its channel; the Meta POST route does not verify a signature, so an unauthenticated caller can forge messages that are persisted or delivered to that tenant’s integrations.
Verify Meta’s `X-Hub-Signature-256` against the raw request body before dispatching any webhook payload.✅ Addressed in 01ef596: The Meta webhook route now requires a valid X-Hub-Signature-256 HMAC computed over the captured raw request body before dispatching the payload.
| await this.eventHandler({ ...content, messages: echoes, statuses: undefined, isEcho: true }); | ||
| } | ||
| if (Array.isArray(content.statuses) && content.statuses.length) { | ||
| await this.eventHandler({ ...content, messages: undefined }); |
Contributor
There was a problem hiding this comment.
🟡 Medium · Status events are acknowledged early
When a status update or deletion triggers an asynchronous webhook or Chatwoot dispatch, messageHandle does not await sendDataWebhook for these events, so eventHandler and the controller can acknowledge the request before delivery completes. A rejected dispatch is not propagated, and the status event is lost.
Await each status-update and deletion dispatch in messageHandle so delivery failures propagate to the caller.
Also at src/api/integrations/channel/meta/whatsapp.business.service.ts:866, src/api/integrations/channel/meta/whatsapp.business.service.ts:997.
Prompt for AI agents
In `src/api/integrations/channel/meta/whatsapp.business.service.ts` at line 151:
**Status events are acknowledged early**
When a status update or deletion triggers an asynchronous webhook or Chatwoot dispatch, `messageHandle` does not await `sendDataWebhook` for these events, so `eventHandler` and the controller can acknowledge the request before delivery completes. A rejected dispatch is not propagated, and the status event is lost.
Await each status-update and deletion dispatch in `messageHandle` so delivery failures propagate to the caller.
Also at `src/api/integrations/channel/meta/whatsapp.business.service.ts:866`, `src/api/integrations/channel/meta/whatsapp.business.service.ts:997`.| message.type === 'reaction' | ||
| ) { | ||
| await this.messageHandle(content, database, settings); | ||
| await this.messageHandle({ ...content, messages: [message], statuses: undefined }, database, settings); |
Contributor
There was a problem hiding this comment.
🟠 High · Contacts get the wrong name
When a webhook value contains messages from multiple senders and corresponding contact entries, eventHandler narrows each call to one message but retains the full contacts array, and messageHandle reads contacts[0]. Later messages therefore use the first sender’s profile name and can overwrite or emit the wrong contact identity.
Pass the contact matching each message to messageHandle, or select the matching contact there instead of always using contacts[0].
Also at src/api/integrations/channel/meta/whatsapp.business.service.ts:975.
Prompt for AI agents
In `src/api/integrations/channel/meta/whatsapp.business.service.ts` at line 991:
**Contacts get the wrong name**
When a webhook value contains messages from multiple senders and corresponding contact entries, `eventHandler` narrows each call to one message but retains the full contacts array, and `messageHandle` reads `contacts[0]`. Later messages therefore use the first sender’s profile name and can overwrite or emit the wrong contact identity.
Pass the contact matching each message to `messageHandle`, or select the matching contact there instead of always using `contacts[0]`.
Also at `src/api/integrations/channel/meta/whatsapp.business.service.ts:975`.✅ Addressed in 01ef596: messageHandle now selects the contact whose wa_id matches the resolved message remote JID instead of using the first contact, preventing names from being attributed to other senders.
Validate Meta POST signatures before dispatch, preserve business ID senders and contact names, and await status delivery. BREAKING CHANGE: Meta POST webhooks require WA_BUSINESS_APP_SECRET. Missing configuration returns 503; invalid signatures return 401.
christianini-debug
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When Meta sends multiple Coexistence app echoes in one webhook, the current parser only processes the first message/change and the controller acknowledges the request before its asynchronous handlers finish. This can silently drop a colleague's phone message or route a later change using the first change's phone number ID.
This builds on #2514, verifies the Meta POST signature before dispatch, and processes every entry, change and message before acknowledging the webhook:
X-Hub-Signature-256over the original request bytes usingWA_BUSINESS_APP_SECRET; fail closed before any controller dispatch.fromMe: true) in the customer's conversation, including when inbound messages and echoes coexist in the same value. Handle empty arrays without hiding the alternate echo array.metadata.phone_number_idas a business sender and match each message to its customer contact bywa_id, including multi-customer batches.For example, a
value.message_echoesarray with two messages addressed to different customers now produces two outgoing events, each with its ownremoteJid, instead of only the first event.Related work
Type of change
Testing
npm ci --no-audit --no-fundnpm run test:coexistence— 33/33 passingnpm run lint:checkDATABASE_PROVIDER=postgresql DATABASE_CONNECTION_URI='postgresql://user:pass@localhost:5432/evolution?schema=public' npm run db:generatenpm run buildgit diff --checkThe regression harness bundles the production Meta service/controller/router, webhook guard, EvoHub controller and base chatbot controller with mocked infrastructure boundaries. The HTTP tests mount the production Meta router on a local Express server, accept a correctly signed UTF-8 payload, and reject invalid signatures, modified bytes, a missing App Secret and unavailable raw bytes before controller dispatch. The GET verification handshake remains covered separately. It covers batch routing across entries/changes and tenants, outgoing/incoming direction, contact isolation, error propagation, media persistence with and without S3, Chatwoot ordering, and native bot ignore/pause behavior.
As a negative control, the batch-message, multi-change routing, contact-isolation and media-persistence tests were also run against unmodified
develop(e273b904): all four fail there and pass with this change. The 15 added review-regression tests were run before the follow-up implementation: 13 failed against the original PR commit (ab781d91), while the valid-signature and GET-handshake controls passed. All 33 tests pass after the follow-up fixes. No database, Meta credentials or external network services are required; HTTP tests bind an ephemeral loopback port.esbuildis declared as a direct test dependency at the version already resolved by the existing lockfile; other package resolutions are unchanged.Migration and compatibility
Breaking configuration change: configure
WA_BUSINESS_APP_SECRETwith the App Secret of the Meta app signing notifications before upgrading a deployment that uses/webhook/meta. This is separate fromWA_BUSINESS_TOKEN_WEBHOOK(the GET verification token) and the Cloud API access token. An unset App Secret returns HTTP 503 on Meta POST webhooks; missing/invalid signatures or unavailable raw request bytes return HTTP 401. The required environment variable and response behavior are documented in.env.example. Meta's official SDK documentation describes the separate GET-token and POST-signature mechanisms.No database migrations or schema changes are required. PostgreSQL client generation/build were checked locally; no live PostgreSQL/MySQL integration is claimed. EvoHub retains its separate
EVOLUTION_HUB_WEBHOOK_SECRETverification path and transport overrides. This change does not add per-instance Meta App Secret configuration.Processing failures can now reject the webhook instead of being silently acknowledged. This does not add replay deduplication or exactly-once delivery, and the existing external webhook delivery/retry layer is unchanged.
Checklist
developbranchScreenshots: not applicable to this backend change.