Skip to content

Don't mutate the caller's events in triggerBatch - #251

Open
OsamaAnsar wants to merge 1 commit into
pusher:masterfrom
OsamaAnsar:fix/trigger-batch-mutates-input
Open

OsamaAnsar wants to merge 1 commit into
pusher:masterfrom
OsamaAnsar:fix/trigger-batch-mutates-input

Conversation

@OsamaAnsar

Copy link
Copy Markdown

Summary

triggerBatch mutates the array the caller passes in. lib/events.js:57-61 overwrites batch[i].data in place with the JSON string, or with the ciphertext for private-encrypted- channels.

Effects:

  • Reusing or retrying the same batch encrypts encrypted events a second time, so receivers cannot decrypt them. Reproduced with post stubbed to capture the body: the first call decrypts to {"hello":"world"}, the second call decrypts to the first call's ciphertext JSON string.
  • For non-encrypted events, the caller's data object silently becomes a string.

trigger() doesn't have this problem: it builds a new event object for each request.

Fix

triggerBatch now builds a new array of new event objects ({ ...event, data: <encoded> }) and posts that, leaving the caller's events untouched. The request body is the same as before (same keys, same order), and encrypt / ensureJSON are called exactly as before. The copy is shallow, which is enough since only the top-level data property is replaced.

Test plan

Added to tests/integration/pusher/trigger.js:

  • #triggerBatch (plain client): the events passed in are unchanged after the call, and data is still an object.
  • #triggerBatch (with encryptionMasterKey): the same batch is triggered twice, and both request bodies contain an encrypted event that decrypts to the original data. The caller's events are still unchanged afterwards.

Both fail without the fix (I reverted only lib/events.js and re-ran: 2 failing, the second with the encrypted data decrypting to the ciphertext JSON string rather than the original data) and pass with it.

Also ran, on Node 24.19.0:

  • npm run local-test (tests/integration/**): 131 passing, 0 failing
  • npm run lint (eslint + prettier --check): clean

Not run: tests/remote/** (part of npm test), since it needs real Pusher credentials.

CHANGELOG

  • [FIXED] triggerBatch no longer modifies the events passed in, so a batch can be reused or retried without double-encrypting events on encrypted channels.

triggerBatch overwrote data on each event in the array it was given with the JSON string, or the ciphertext for private-encrypted- channels. Triggering the same batch twice (for example when retrying) therefore encrypted the data a second time, and receivers could not decrypt it. Build new event objects for the request body instead, as trigger() already does.
@github-actions github-actions Bot added the Stale label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant