Security hardening: booking auth, offering exposure, payments, invites (#31–#37) #38

Merged
thatguygriff merged 1 commits from feature/security-fixes into main 2026-06-09 20:11:34 +00:00
Owner

Summary

Security fixes from a pen-test review of the plugin's access control, injection surface, and information/payment-data exposure. Closes #31, #32, #33, #34, #35, #36, #37.

The SQL layer (parameterized throughout) and output escaping were already sound; these changes address logic-level authorization gaps, a disclosure surface, a booking race, and a couple of hardening items.

Changes

Issue Severity Fix
#31 High BookingEndpoint::book() no longer trusts a client offering_id: a slot-tied offering is authoritative, and any offering used must belong to the slot's instructor — closing a free/misrouted-payment bypass.
#32 Medium offerings, offerings/{id}/questions, and policies read endpoints now require book_lesson instead of __return_true (no anonymous consumer exists). Offering::toArray() also omits etransfer_email from listings as defense-in-depth.
#33 Medium Slots are claimed atomically (UPDATE … WHERE is_booked = 0) before a lesson is inserted, preventing a double-booking / double-charge race.
#34 Low Availability slot creation rejects an offering the instructor doesn't own.
#35 Low A single weekly booking is capped (MAX_WEEKLY_OCCURRENCES = 12) and only creates lessons for slots it actually claimed.
#36 Low Stripe secret/webhook keys are write-only in the settings UI; a blank submit keeps the stored value and secrets are never echoed back into HTML.
#37 Low Pending invites expire after 14 days (Invite::isAcceptable), enforced at registration and surfaced on the admin invites list.

Notes

  • AvailabilityEndpoint now takes an OfferingRepository (wired in RestRegistrar).
  • The Stripe /payments/webhook endpoint remains __return_true by design — it is signature-verified and called server-to-server by Stripe, which has no WP session.
  • The relevant e-transfer email is still delivered to the student via the authenticated createIntent response for their own registration.

Tests

Adds BookingEndpointTest (offering-substitution rejection, claim/409 race, happy path) plus Invite, Offering, and AvailabilityRepository coverage, and minimal WP_REST_Request/WP_REST_Response stubs to the test bootstrap.

composer test (200 tests, 572 assertions), composer lint (PHPStan L6), composer cs (PHPCS) all green.

🤖 Generated with Claude Code

## Summary Security fixes from a pen-test review of the plugin's access control, injection surface, and information/payment-data exposure. Closes #31, #32, #33, #34, #35, #36, #37. The SQL layer (parameterized throughout) and output escaping were already sound; these changes address logic-level authorization gaps, a disclosure surface, a booking race, and a couple of hardening items. ## Changes | Issue | Severity | Fix | |---|---|---| | #31 | High | `BookingEndpoint::book()` no longer trusts a client `offering_id`: a slot-tied offering is authoritative, and any offering used must belong to the slot's instructor — closing a free/misrouted-payment bypass. | | #32 | Medium | `offerings`, `offerings/{id}/questions`, and `policies` read endpoints now require `book_lesson` instead of `__return_true` (no anonymous consumer exists). `Offering::toArray()` also omits `etransfer_email` from listings as defense-in-depth. | | #33 | Medium | Slots are claimed atomically (`UPDATE … WHERE is_booked = 0`) before a lesson is inserted, preventing a double-booking / double-charge race. | | #34 | Low | Availability slot creation rejects an offering the instructor doesn't own. | | #35 | Low | A single weekly booking is capped (`MAX_WEEKLY_OCCURRENCES = 12`) and only creates lessons for slots it actually claimed. | | #36 | Low | Stripe secret/webhook keys are write-only in the settings UI; a blank submit keeps the stored value and secrets are never echoed back into HTML. | | #37 | Low | Pending invites expire after 14 days (`Invite::isAcceptable`), enforced at registration and surfaced on the admin invites list. | ## Notes - `AvailabilityEndpoint` now takes an `OfferingRepository` (wired in `RestRegistrar`). - The Stripe `/payments/webhook` endpoint remains `__return_true` by design — it is signature-verified and called server-to-server by Stripe, which has no WP session. - The relevant e-transfer email is still delivered to the student via the authenticated `createIntent` response for their own registration. ## Tests Adds `BookingEndpointTest` (offering-substitution rejection, claim/409 race, happy path) plus `Invite`, `Offering`, and `AvailabilityRepository` coverage, and minimal `WP_REST_Request`/`WP_REST_Response` stubs to the test bootstrap. `composer test` (200 tests, 572 assertions), `composer lint` (PHPStan L6), `composer cs` (PHPCS) all green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
thatguygriff added 1 commit 2026-06-09 20:08:59 +00:00
Harden booking, offering exposure, payments, and invites
CI / No Debug Code (pull_request) Successful in 3s
CI / Tests (PHP 8.1) (pull_request) Successful in 49s
CI / Coding Standards (pull_request) Successful in 55s
CI / PHPStan (pull_request) Successful in 1m7s
CI / Tests (PHP 8.3) (pull_request) Successful in 1m41s
CI / Tests (PHP 8.2) (pull_request) Successful in 44s
CI / Build Plugin Zip (pull_request) Has been skipped
061d09e034
Security fixes from a pen-test review (issues #31–#37):

- #31 Booking no longer trusts a client-supplied offering_id: a slot-tied
  offering is authoritative and any offering used must belong to the slot's
  instructor, closing a free/misrouted-payment bypass.
- #34 Availability slot creation rejects an offering the instructor does not
  own (AvailabilityEndpoint now takes OfferingRepository).
- #32 Offering/question/policy listing endpoints now require book_lesson
  instead of being public (no anonymous consumer exists); Offering::toArray
  also omits etransfer_email from listings as defense-in-depth.
- #33 Slots are claimed atomically (UPDATE ... WHERE is_booked = 0) before a
  lesson is inserted, preventing a double-booking race.
- #35 A single weekly booking is capped (MAX_WEEKLY_OCCURRENCES) and only
  creates lessons for slots it actually claimed.
- #36 Stripe secret/webhook keys are write-only in the settings UI and a blank
  submit keeps the stored value; secrets are never echoed back into HTML.
- #37 Pending invites expire after 14 days (Invite::isAcceptable), enforced at
  registration and surfaced on the admin invites list.

Adds BookingEndpointTest plus Invite/Offering/AvailabilityRepository coverage
and minimal WP_REST_Request/WP_REST_Response stubs. composer test (200),
lint, and cs all green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thatguygriff force-pushed feature/security-fixes from 5140e76347 to 061d09e034 2026-06-09 20:08:59 +00:00 Compare
thatguygriff merged commit 693246c1c1 into main 2026-06-09 20:11:34 +00:00
thatguygriff deleted branch feature/security-fixes 2026-06-09 20:11:34 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Unsupervised/unsupervised-scheduler#38