Security fixes: CSV injection, policy body output, invite token hashing, slot datetime validation #43

2026-06-10T19:39:33Z

thatguygriff commented

2026-06-10 19:39:33 +00:00

Fixes the four actionable findings from the security review pass: #39, #40, #41, #42.

What changed

CSV formula injection in the payments export (#39 — Medium)

PaymentReport::csvLine() now apostrophe-prefixes any field with a leading =, +, -, @, tab, or CR before quoting. Student display names are self-chosen at registration and flow into the CSV the studio admin opens in Excel/Google Sheets — previously a name like =HYPERLINK(...) became a live formula. New test covers hostile names across all trigger characters.

Policy bodies sanitised at output (#40 — defense-in-depth)

PolicyEndpoint::index() runs the body through wp_kses_post() before returning it. The booking/group-class JS renders this HTML raw via innerHTML; protection previously rested entirely on every write path remembering to kses. Now a missed write path can't become stored XSS against students.

Invite tokens hashed at rest (#41)

Only hash('sha256', $token) is stored in us_invites; the registration flow hashes the submitted token for lookup (Invite::hashToken()). The admin Invites page shows the full registration link once, in a notice right after creation — the pending list now shows email + invited date, and a lost link is re-issued by revoke + re-invite. A database leak (backup, SQLi in an unrelated plugin) can no longer be used to redeem pending invites and mint accounts.

Note: existing plaintext pending invites stop matching after this change and must be revoked and re-issued (pre-1.0, no migration).

Availability datetime validation (#42)

New AvailabilitySlot::normalizeDateTime() accepts canonical Y-m-d H:i[:s] and HTML datetime-local (Y-m-d\TH:i[:s]) forms, canonicalises to Y-m-d H:i:s, and rejects everything else via a strict round-trip check (no PHP date rollover: 2026-02-30 is refused). Both creation paths use it — REST returns 400 invalid_datetime, the admin form no-ops like the existing empty-field check — and both now also require end > start. Previously garbage reached the DATETIME column on the single path and threw an unhandled exception inside the weekly-series date arithmetic.

Checks

composer test (204 tests, 594 assertions), PHPStan L6, and PHPCS all green. Feature docs updated (account-registration, payment-reporting, availability-management).

🤖 Generated with Claude Code

Fixes the four actionable findings from the security review pass: #39, #40, #41, #42. ## What changed ### CSV formula injection in the payments export (#39 — Medium) `PaymentReport::csvLine()` now apostrophe-prefixes any field with a leading `=`, `+`, `-`, `@`, tab, or CR before quoting. Student display names are self-chosen at registration and flow into the CSV the studio admin opens in Excel/Google Sheets — previously a name like `=HYPERLINK(...)` became a live formula. New test covers hostile names across all trigger characters. ### Policy bodies sanitised at output (#40 — defense-in-depth) `PolicyEndpoint::index()` runs the body through `wp_kses_post()` before returning it. The booking/group-class JS renders this HTML raw via `innerHTML`; protection previously rested entirely on every write path remembering to kses. Now a missed write path can't become stored XSS against students. ### Invite tokens hashed at rest (#41) Only `hash('sha256', $token)` is stored in `us_invites`; the registration flow hashes the submitted token for lookup (`Invite::hashToken()`). The admin Invites page shows the full registration link **once**, in a notice right after creation — the pending list now shows email + invited date, and a lost link is re-issued by revoke + re-invite. A database leak (backup, SQLi in an unrelated plugin) can no longer be used to redeem pending invites and mint accounts. **Note:** existing plaintext pending invites stop matching after this change and must be revoked and re-issued (pre-1.0, no migration). ### Availability datetime validation (#42) New `AvailabilitySlot::normalizeDateTime()` accepts canonical `Y-m-d H:i[:s]` and HTML `datetime-local` (`Y-m-d\TH:i[:s]`) forms, canonicalises to `Y-m-d H:i:s`, and rejects everything else via a strict round-trip check (no PHP date rollover: `2026-02-30` is refused). Both creation paths use it — REST returns `400 invalid_datetime`, the admin form no-ops like the existing empty-field check — and both now also require `end > start`. Previously garbage reached the DATETIME column on the single path and threw an unhandled exception inside the weekly-series date arithmetic. ## Checks `composer test` (204 tests, 594 assertions), PHPStan L6, and PHPCS all green. Feature docs updated (account-registration, payment-reporting, availability-management). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

thatguygriff added 1 commit 2026-06-10 19:39:33 +00:00

Security fixes: CSV injection, policy body output, invite hashing, slot datetimes

CI / No Debug Code (pull_request) Successful in 3s

Details

CI / Tests (PHP 8.1) (pull_request) Successful in 43s

Details

CI / Tests (PHP 8.3) (pull_request) Successful in 49s

Details

CI / Tests (PHP 8.2) (pull_request) Successful in 59s

Details

CI / Coding Standards (pull_request) Successful in 1m11s

Details

CI / PHPStan (pull_request) Successful in 1m20s

Details

CI / Build Plugin Zip (pull_request) Has been skipped

Details

f3f5c7801f

Four fixes from a security review pass:

- Neutralise CSV formula injection in the payments export: fields with a
  leading =, +, -, @, tab, or CR (e.g. a hostile student display name) are
  apostrophe-prefixed in PaymentReport::csvLine() so they open as text in
  Excel/Google Sheets. Fixes #39.
- Sanitise policy bodies with wp_kses_post at output in
  PolicyEndpoint::index() (the booking JS renders that HTML raw), so a
  future write path that forgets kses can never become stored XSS.
  Fixes #40.
- Store invite tokens hashed (SHA-256) at rest: a database leak can no
  longer redeem pending invites. The registration link is shown once, at
  creation; the pending list shows email/invited date; lookups hash the
  submitted token. Existing plaintext pending invites must be re-issued.
  Fixes #41.
- Validate availability slot datetimes on both creation paths (REST and
  admin form) via AvailabilitySlot::normalizeDateTime(): canonical and
  datetime-local forms normalise to Y-m-d H:i:s, garbage and end <= start
  are rejected (REST 400) instead of reaching the DATETIME column or
  throwing inside the weekly-series date arithmetic. Fixes #42.

composer test (204 tests, 594 assertions), PHPStan L6, and PHPCS all green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

thatguygriff merged commit 63e2fbcc5b into main

2026-06-10 19:51:21 +00:00

thatguygriff deleted branch feature/security-review-fixes

2026-06-10 19:51:21 +00:00

thatguygriff referenced this pull request

2026-06-10 20:17:52 +00:00

Invites never expire (no TTL on pending tokens) #37

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Unsupervised/unsupervised-scheduler#43