Fix volunteer PII leak on public event pages#879
Open
edwh wants to merge 3 commits into
Open
Conversation
Party::expandVolunteers() was serialising the full User Eloquent model into the page HTML (via json_encode in the Blade template). Because User::$hidden only contained password and remember_token, any visitor could read api_token, calendar_hash, latitude, longitude, recovery, and mediawiki tokens directly from the page source. Fixes: 1. User::$hidden — add api_token, calendar_hash, recovery, recovery_expires, mediawiki, latitude, longitude (defence-in-depth; protects every serialisation point). 2. Party::expandVolunteers() — replace the full User model assignment with an explicit whitelist of only the fields the frontend needs (id, name, email when showEmails, user_skills shape). Never put an Eloquent model directly into page-embedded JSON. 3. ApiController::getUserInfo() — extend the belt-and-suspenders makeHidden() call to cover all credential fields, not just api_token. Tests (VolunteerPiiTest.php): - testEventPageDoesNotLeakVolunteerCredentials: loads the public event page HTML and asserts sentinel values are absent while volunteer name is still present. - testUserModelHiddenFieldsExcludeCredentials: asserts every sensitive field is in $hidden via User::toArray(). - testExpandVolunteersEmitsSafeShape: asserts expandVolunteers() returns a plain array (not an Eloquent model) and contains no sensitive keys. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ints
- Call unsetRelation('volunteer') before assigning the safe array; without
this, Eloquent's toArray() merges relationsToArray() last, silently
overwriting the safe array with the full User model in JSON output
- Add null guard for orphaned skill FK rows (deleted skillName record)
- Fix inverted $showEmails in EventController::listVolunteers — was
negated so non-hosts received emails and hosts did not
- Add latitude/longitude to makeHidden() in ApiController::getUserInfo()
and UserController::mapUserAndAuditToUserChange() (Zapier endpoint)
- Update testExpandVolunteersEmitsSafeShape to assert via json_encode so
the Eloquent relation-bypass is actually caught (PHP array-access was
a false pass — it reads $attributes, not the serialised output)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add api_token, calendar_hash, recovery, recovery_expires, latitude, longitude, and password to User::$auditExclude so they are never written to the audits table on any model update. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 tasks
|
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.
Summary
Party::expandVolunteers()was serialising the full User Eloquent model into event page HTML, exposingapi_token,calendar_hash,recovery,latitude, andlongitudeto anonymous visitorsexpandVolunteers()to emit a whitelist-only$safeUserarray and callunsetRelation('volunteer')to prevent Eloquent'srelationsToArray()clobbering it duringjson_encodeUser::\$hiddento cover all credential and PII fields\$showEmailsinEventController::listVolunteers(was negated — showed emails to non-hosts)latitude/longitudetomakeHidden()belt-and-suspenders guards inApiControllerandUserController(Zapier endpoint)User::\$auditExcludeso they are never written to the audit logTest plan
VolunteerPiiTest— page HTML does not leak credentials,User::\$hiddencovers all sensitive fields,expandVolunteersemits safe shape viajson_encodeUserAuditExcludeTest— credential fields absent from auditold_values/new_values🤖 Generated with Claude Code