Skip to content

Keep access tokens and raw PII out of the Messenger transport - #49

Open
loevgaard wants to merge 1 commit into
masterfrom
fix/17-pii-in-transport
Open

Keep access tokens and raw PII out of the Messenger transport#49
loevgaard wants to merge 1 commit into
masterfrom
fix/17-pii-in-transport

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes #17

Stacked on #48.

Problem

SendEvent carried the whole Event object. At dispatch time that object holds the pixels including their access tokens, and the raw email addresses, phone numbers, names and dates of birth the application attached, because normalisation and hashing only happen inside Parameters::getPayload(), which the SDK calls later in the handler.

Routing the command to a transport, which is the recommended setup, therefore wrote all of that into the transport's storage, into the failure transport when it failed, and into anything that dumps messages such as messenger:failed:show. Failure transports are often kept indefinitely, so it was a retention problem as well as a secrets problem.

Change

SendEvent now carries the finished payload:

new SendEvent($eventName, $eventId, $payload, $pixelIds, $testEventCode);

built with SendEvent::fromEvent($event). The payload is produced by the SDK at dispatch time, so it is already normalised and hashed, and only pixel ids travel.

Access tokens are resolved when the event is sent, through a new AccessTokenResolverInterface. ConfigurationBasedAccessTokenResolver reads them from the pixels configuration, which is request independent and therefore safe to run in a worker. Applications with their own PixelProviderInterface alias the resolver too, as documented in README.md and UPGRADE.md.

PreparedEvent (internal) hands the precomputed payload back to ClientInterface::sendEvent(), which takes an Event. It extends the SDK's Event, which is explicitly documented as non-final for this kind of extension, and overrides getPayload().

A welcome side effect: everything in the message is now a scalar or an array, so it also survives the Symfony serializer rather than requiring the PHP one.

Tests

The important one serialises a message built from an event carrying an access token, an email address, a phone number and a first name, then asserts none of those four strings appear in the serialised output, while the payload holds the SHA-256 of the email. Plus three tests for the resolver, including the fact that numeric pixel ids become integer array keys in PHP, and three for the rewritten handler.

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.61%. Comparing base (865684c) to head (5c3470c).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
src/Message/PreparedEvent.php 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master      #49      +/-   ##
============================================
+ Coverage     77.35%   77.61%   +0.26%     
- Complexity      140      148       +8     
============================================
  Files            30       32       +2     
  Lines           468      487      +19     
============================================
+ Hits            362      378      +16     
- Misses          106      109       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@loevgaard
loevgaard force-pushed the fix/16-sync-failures branch from 7107a61 to b06fbd4 Compare September 7, 2026 12:03
@loevgaard
loevgaard force-pushed the fix/17-pii-in-transport branch from 9a99ee8 to dcbe1cb Compare September 7, 2026 12:03
@loevgaard
loevgaard force-pushed the fix/16-sync-failures branch from b06fbd4 to db88227 Compare September 7, 2026 12:18
@loevgaard
loevgaard force-pushed the fix/17-pii-in-transport branch from dcbe1cb to af21153 Compare September 7, 2026 12:18
@loevgaard
loevgaard force-pushed the fix/16-sync-failures branch from db88227 to bb2ba25 Compare September 7, 2026 12:46
@loevgaard
loevgaard force-pushed the fix/17-pii-in-transport branch from af21153 to bac5d1e Compare September 7, 2026 12:46
@loevgaard
loevgaard force-pushed the fix/16-sync-failures branch from bb2ba25 to 865684c Compare September 7, 2026 12:51
@loevgaard
loevgaard force-pushed the fix/17-pii-in-transport branch from bac5d1e to c9f1d9d Compare September 7, 2026 12:51
Base automatically changed from fix/16-sync-failures to master September 7, 2026 12:56
SendEvent carried the Event object, so routing it to a transport wrote
the access token and every raw email, phone number and name into that
transport's storage, and into the failure transport on failure. Hashing
only happened later, inside Client::sendEvent().

The command now carries the finished payload and pixel ids only. Access
tokens are resolved at send time through AccessTokenResolverInterface.

Fixes #17
@loevgaard
loevgaard force-pushed the fix/17-pii-in-transport branch from c9f1d9d to 5c3470c Compare September 7, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Access tokens and unhashed PII are serialized into the Messenger transport

1 participant