Skip to content

Commit b2dfc48

Browse files
committed
fix(speakers): stop logging access_token and raw filter PII in bulk send debug/error logs
SpeakerService::triggerSendEmails's debug log json_encode()'d the full payload, including access_token - confirmed leaking a live bearer token in a real dev send today. Replaced with summit id, flow_event, speaker_ids count and whether a filter was given. ProcessSpeakersEmailRequestJob::failed()'s error log json_encode()'d the raw filter, which can carry email/full_name PII (valid speaker filter fields). New redactFilterFieldNames() logs only the filter's field names. Found and confirmed via adversarial review of CodeRabbit's full-review findings on PR #595. test(speakers): add red-green verified regression test for filter PII redaction testFailedChunkLogsFilterFieldNamesButNotTheirValues asserts the failed() error log contains the filter's field names but not an email value from it; reverting the redaction makes it fail (Mockery 0 matching calls).
1 parent ae32f90 commit b2dfc48

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

app/Jobs/Emails/PresentationSubmissions/ProcessSpeakersEmailRequestJob.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,15 @@ public function failed(\Throwable $e): void
125125
(
126126
sprintf
127127
(
128-
"ProcessSpeakersEmailRequestJob::failed summit %s flow_event %s: chunk of %s speaker(s) failed (%s: %s); up to %s of them may not have been processed. Speaker ids in the chunk: [%s] filter %s. %s.",
128+
"ProcessSpeakersEmailRequestJob::failed summit %s flow_event %s: chunk of %s speaker(s) failed (%s: %s); up to %s of them may not have been processed. Speaker ids in the chunk: [%s] filter fields %s. %s.",
129129
$this->summit_id,
130130
$flow_event,
131131
count($speaker_ids),
132132
get_class($e),
133133
$e->getMessage(),
134134
count($speaker_ids),
135135
$ids_list,
136-
json_encode($this->filter),
136+
json_encode($this->redactFilterFieldNames($this->filter)),
137137
$resend_hint
138138
)
139139
);
@@ -187,4 +187,19 @@ public function failed(\Throwable $e): void
187187
Log::error($ex);
188188
}
189189
}
190+
191+
/**
192+
* Reduces a raw filter (["email==foo@bar.com", "presentations_selection_plan_id==78"]) to
193+
* just its field names (["email", "presentations_selection_plan_id"]) so error logs never
194+
* carry filter values that may be PII - email and full_name are valid filter fields
195+
* (ISpeakerFilterFields::OPERATORS).
196+
*
197+
* @param mixed $filter
198+
* @return string[]
199+
*/
200+
private function redactFilterFieldNames($filter): array
201+
{
202+
if (empty($filter) || !is_array($filter)) return [];
203+
return array_map(fn($condition) => preg_replace('/[=<>@!].*/', '', (string)$condition), $filter);
204+
}
190205
}

app/Services/Model/Imp/SpeakerService.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,9 +1243,11 @@ public function triggerSendEmails(Summit $summit, array $payload, $filter = null
12431243
(
12441244
sprintf
12451245
(
1246-
"SpeakerService::triggerSendEmails summit %s payload %s process_db_chunk_size %s process_jon_chunk_size %s",
1246+
"SpeakerService::triggerSendEmails summit %s email_flow_event %s speaker_ids_count %s has_filter %s process_db_chunk_size %s process_jon_chunk_size %s",
12471247
$summit->getId(),
1248-
json_encode($payload),
1248+
$payload['email_flow_event'] ?? '',
1249+
isset($payload['speaker_ids']) ? count($payload['speaker_ids']) : 0,
1250+
!is_null($filter) ? 'yes' : 'no',
12491251
$process_db_chunk_size,
12501252
$process_jon_chunk_size
12511253
)

tests/ProcessSpeakersEmailRequestJobFailedHookTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,4 +194,27 @@ public function testFailedChunkWithoutOutcomeRecipientOnlyLogsTheUnprocessedIds(
194194
->withArgs(fn($message) => is_string($message) && str_contains($message, '44, 55'))
195195
->once();
196196
}
197+
198+
public function testFailedChunkLogsFilterFieldNamesButNotTheirValues(): void
199+
{
200+
// email and full_name are valid speaker filter fields (ISpeakerFilterFields::OPERATORS),
201+
// so the raw filter can carry PII. The error log must still say which fields were used
202+
// (useful to reproduce the failed send) without writing the PII value itself.
203+
Queue::fake();
204+
Log::spy();
205+
206+
$job = new ProcessSpeakersEmailRequestJob(self::$summit->getId(), [
207+
'email_flow_event' => 'SUMMIT_SUBMISSIONS_PRESENTATION_SPEAKER_ACCEPTED_ALTERNATE',
208+
'speaker_ids' => [11, 22],
209+
], ['email==foo@bar.com', 'presentations_selection_plan_id==78']);
210+
211+
$job->failed(new \RuntimeException('worker killed mid-chunk'));
212+
213+
Log::shouldHaveReceived('error')
214+
->withArgs(fn($message) => is_string($message)
215+
&& str_contains($message, 'email')
216+
&& str_contains($message, 'presentations_selection_plan_id')
217+
&& !str_contains($message, 'foo@bar.com'))
218+
->once();
219+
}
197220
}

0 commit comments

Comments
 (0)