security: XSS, device IDOR, upload authorization and password-reset type juggling

[edwh]

Fixes the findings from an adversarial review of develop, including the parts of the
iFixit disclosure that were never actually closed. Each fix has a test written against the
unpatched code first.

The two that matter most

Unauthenticated account takeover via the password reset (UserController::reset).
filter_var() returns false for an array; Laravel binds false as integer 0; MySQL
compares a VARCHAR column to 0 numerically, coercing every non-numeric token to 0. So
POST /user/reset with recovery[]=1 turned "match nothing" into "match a real user's live
recovery code", set that user's password, and rendered their email back. Confirmed against
MySQL: SELECT 'a3f5c1' = 0 → 1, SELECT 'a3f5c1' = '0' → 0.

Victim selection is constrained — ->first() has no ORDER BY, so it hits the lowest-id
user holding a non-null recovery, not a chosen target — but it is unauthenticated, silent,
and pollable with a GET until a token goes live. Worth knowing: registration mints a
recovery token, and so did changing your password, so live codes were routine.

Stored XSS on a public page, three requests from a fresh account. Descriptions
(free_text) are Quill HTML rendered raw, with no sanitiser and no CSP. Register → POST /api/v2/groups (no role check, and it makes you a host) → POST /api/v2/events against
any approved group (createEventv2 only checked that you host some group — there was a
TODO saying so). Event visibility is inherited from the group's approved flag, so it went
public with no moderation, and /party/view/{id} needs no login.

The disclosure's audit-log XSS (F005) was only half fixed

The reported line — the modified-values cell — was escaped. Line 8 of the same file was not:
it renders @lang(...metadata, $audit->getMetadata()), and @lang compiles to a bare echo
while the translator does not escape :placeholder values. Two of those values are
attacker-controlled: user_name (any display name) and audit_url (the request URL with
its query string — develop uses the stock UrlResolver; SanitisedUrlResolver only
exists on nuxt-client). It fires for any host, coordinator or admin reviewing a group's
history, which is precisely the reported scenario.

Also fixed

updateDevicev2	Authorised against an eventid from the request body, then mutated a device from the URL path. Name your own event and overwrite/reassign any device in the database. Now derives the event from the record, as deleteDevicev2 already did.
3 × imageUpload	Group, party and device uploads had no authorization beyond being logged in, while each controller's delete-image sibling had a check. The group one purges the existing image first, so any account could replace any group's photo.
api_token	A five-year, JS-readable bearer credential that nothing ever rotated, so a token stolen via XSS survived a password reset. Now rotated on password change and reset; cookie marked Secure over HTTPS (httpOnly has to stay off — the Vue client reads it).
Flash messages	now_following, delete_succeeded, soft_deleted, you_have_joined interpolate names into HTML strings rendered with {!! !!}.
Client translator	The Vue translator has the same unescaped-placeholder bug in a plain .replace(). Added escapeHtml and used it where a group name reaches v-html.
getEventv2	No auth and no approval gate, unlike its sibling — an unmoderated event's exact lat/lng (possibly a home address) was fetchable by id.
Exports	Written into public/ under a guessable name and never deleted, so a privileged export (which includes unapproved events' rows) was left fetchable by anyone. Now storage/app/exports, deleted after send. Plus CSV formula-injection prefixing.
Network logo	The one upload bypassing the content-sniffed jpg/png/gif allowlist, in a controller with zero validate() calls — a crafted SVG would land in the webroot.
/test/check-auth	Live, unauthenticated, resolved identity from a plaintext unencrypted authenticated cookie with no session check. Removed, with its orphaned service.
CALENDAR_HASH, RoleController	Loose != on a hash; and a guard that authorised the route's role then edited whichever role a hidden formId named.

Implementation notes for review

• Sanitisation is a model mutator, not controller code, so every write path (v2 API, web
  forms, imports, seeders) is covered once. This matters because free_text has two
  independent render paths — the Blade modals and ReadMore.vue's v-html fed from the
  API — so a Blade-only fix would have left the second one live.
• stevebauman/purify is the package the iFixit fork used, which should ease reconciling
  with their tree. Its default cache is Laravel's cache, not a serializer path, so CI needs
  nothing writable.
• Escaping is applied to the interpolated values, not the sinks, so the intended markup
  in the translation strings still renders.
• createEventv2 checks network coordinators separately, since their authority comes from
  the group's networks rather than users_groups.

Destructive actions moved off GET

Laravel's CSRF middleware only covers POST/PUT/PATCH/DELETE, so a delete behind a GET route
could be triggered by any page an admin loaded. /group/delete/{id}, /skills/delete/{id},
/tags/delete/{id}, /brands/delete/{id} and the three image deletes are POST now.

The delete controls are anchors marked data-method="post", submitted as CSRF-carrying
forms by a delegated handler in app.js. Two approaches were tried and rejected first, both
caught by the browser tests: a nested <form> (browsers discard it, so the button submitted
the edit action) and formaction on a submit button (it then sat ahead of the save button,
and the tag edit test clicks .btn-create, button[type="submit"], which resolves in DOM
order — so editing a tag deleted it). Worth knowing if anyone adds another one of these.

Invite-acceptance and join links stay GET on purpose: they are followed from email and carry
single-use hashes rather than acting on an arbitrary id. Also removed the route for
PartyController::deleteimage, which has no such method.

Sanitiser allowlist is derived from the live data, not the package default

Shipping HTMLPurifier's default allowlist would have silently degraded existing descriptions
the first time each host saved one. A survey of the anonymised production dump found ~5,100
links carrying target=, ~4,500 rel=, ~7,100 inline style, ~1,300 class, plus div,
hr, sub, sup, strike, font and a few tables — none of which the default permits. The
same survey found zero <script>, <iframe>, <embed>, <object>, <form>,
javascript: URL or on* handler, so nothing legitimate depends on what we block.

config/purify.php widens elements and attributes to match, while keeping the parts of
HTMLPurifier that do the security work: on* handlers are never allowed, URIs are limited to
safe schemes, and style is filtered to an allowlist of CSS properties. position is
excluded deliberately — absolute positioning in user content is a UI-redressing vector.
target= is allowed with HTML.TargetNoopener, so links currently lacking rel="noopener"
gain it.

Verified by running 60 real descriptions through it: what still gets removed is empty
class="", Facebook/Draft.js data-* junk, Word and VML remnants
(<v:imagedata src="file:///C:/Users/…">), Google Docs id="docs-internal-guid-*" and CSS
position. Genuine markup round-trips.

Deliberately not in this PR

• No CSP. The app has a lot of inline script; adding one needs its own change with real
  browser testing rather than being smuggled into a fix PR.
• getGroupv2 still returns a group's contact email with no approval gate. Confirmed as
  intended behaviour ("contact this group").
• Historical audits rows may still hold credentials from before $auditExclude landed.
  Not reachable through any route, so it's a data-remediation task.

One unrelated fix

tests/Integration/utils.js picked an event date by clicking the last day cell of the last
calendar row, which for July resolved to the 26th. Run on the 30th, the event was in the past
and the "Invite volunteers" test failed, since that menu item is gated on the event being
upcoming. Develop last went green on 15 July, when the 26th was still ahead — this fails on
develop today too. The helper now always steps a month, so the chosen day cannot straddle
today.

Verification

PHPUnit 550 green in CI; Jest green in CI; the full Playwright suite run locally, 50 passed in
7.1m. PHPUnit structurally cannot catch the delete-button bugs above — those tests POST
straight to the route and never render the markup — so UI changes here need a browser run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hd9V5FKGKbghx4ztS3rPtF

[sonarqubecloud]

Quality Gate failed

Failed conditions
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE