10 KiB
Technical Concerns
Analysis Date: 2026-05-05 Revised: 2026-05-05 — verified call sites; demoted false positives (see CHANGELOG at bottom)
Prioritized list of issues found during codebase analysis.
CRITICAL — Security / Data Loss Risk
C1. Race condition on booking creation — no atomic lock
Files: includes/class-availability.php (is_available), api/class-rest-controller.php (create_booking lines 326–362)
Flow: is_available() → Booking::create() → mark_as_booked() — three separate non-transactional operations. Two concurrent POST requests for the same yacht and overlapping dates will both pass the check before either writes. The UNIQUE KEY yacht_date (yacht_id, date) will reject the second mark_as_booked() iteration silently, but both yacht_booking posts are already created. Admin sees two confirmed bookings for the same dates.
Fix: Wrap in a DB transaction with SELECT ... FOR UPDATE on availability rows, or insert a sentinel lock row first and check the result before proceeding.
C2. OAuth credentials stored unencrypted in wp_options
File: integrations/google-calendar/class-oauth-handler.php lines 185–187, 213–215
yacht_booking_gcal_credentials (client_id, client_secret) and yacht_booking_gcal_tokens (access_token, refresh_token) stored as plaintext serialized arrays. Any plugin or user with DB read access can extract a working Google Calendar refresh token. If the database is compromised (via SQL injection elsewhere on the WordPress install or hosting breach), the Google Calendar account is fully exposed.
Fix: Encrypt with WordPress secret keys (AUTH_KEY etc.) before storing; decrypt on read.
HIGH — Reliability
H1. GCal API HTTP errors silently ignored
Files: integrations/google-calendar/class-gcal-service.php, class-oauth-handler.php
Every API call checks is_wp_error() but not the HTTP response code. A 401, 403, 429, or 500 from Google is treated as success — json_decode runs, expected fields like $body['id'] or $body['access_token'] are absent, function returns false. All errors only logged when WP_DEBUG === true. In production: complete silence.
H2. Token refresh has no concurrency protection
File: integrations/google-calendar/class-oauth-handler.php lines 107–128
Simultaneous WP-Cron events both finding the token expired will both call refresh_access_token(). Google may invalidate the first new token when the second refresh request arrives. Result: permanently broken GCal connection requiring manual reauth.
Fix: Wrap refresh in a transient-based lock (set_transient('gcal_refresh_lock', 1, 60)), or use wp_cache_add() semantics.
H3. register_cron_actions() may not be called — cron hooks fire with no registered callbacks
File: integrations/google-calendar/class-sync-controller.php lines 141–153
The constructor registers on_booking_created etc., but register_cron_actions() (which registers the yacht_booking_sync_to_gcal action itself) is a separate static method. Need to verify it is called on every init / plugins_loaded request. If not, scheduled single events silently fire with no handler.
H4. Single calendar for all yachts — GCal events apply to every yacht
File: integrations/google-calendar/class-gcal-service.php lines 196–272
get_calendar_id() returns one shared option (yacht_booking_gcal_calendar_id). pull_from_gcal() calls sync_from_gcal($yacht->ID) for each yacht but each call fetches all events from the same calendar and blocks that yacht for all those dates. Multi-yacht sites will incorrectly block all yachts for every GCal event.
H5. tmp-fix-polish.php in project root reveals dev paths
File: tmp-fix-polish.php
Contains only one line: a path string to another local project (c:\visual studio code\projekty\jachty.pagedev.pl\tmp-fix-polish.php). No PHP tag, no code. If deployed via FTP, the raw path string is publicly accessible at the URL. Should be deleted and excluded from deployment.
MEDIUM — Maintainability / Correctness
M1. Date validation missing on booking creation
File: api/class-rest-controller.php lines 116–122
start_date/end_date use only sanitize_text_field — no validate_callback. An invalid date string like "not-a-date" reaches new \DateTime() in Availability::count_days() without try/catch, producing an uncaught fatal error visible to the public. The /availability endpoint validates dates correctly; /bookings does not.
Fix: Add validate_callback checking \DateTime::createFromFormat('Y-m-d', $param) !== false.
M2. No pagination on get_yachts and get_bookings
File: api/class-rest-controller.php lines 222–245, 429–462
Both endpoints use posts_per_page => -1. Will degrade progressively as bookings accumulate.
M3. N+1 query pattern in booking and yacht lists
File: api/class-rest-controller.php lines 442–458
For each booking: get_post($yacht_id) called individually inside loop. For each yacht: three separate get_post_meta() calls per row. With 20 bookings: 60+ queries per response.
M4. iCal importer ignores STATUS:CANCELLED events
File: integrations/ical/class-ical-import.php parse_ics() method
Parser reads UID, SUMMARY, DTSTART, DTEND but ignores STATUS. Cancelled events in the iCal feed are imported as blocks. The GCal service correctly skips status: cancelled events (line 347 of class-gcal-service.php) — parity needed in iCal importer.
M5. mark_as_blocked() passes null booking_id with %d format
File: includes/class-availability.php lines 98–109
$wpdb->replace() receives null with %d format specifier. In older MySQL/MariaDB this may insert 0 instead of NULL, making it impossible to distinguish a blocked row from one linked to booking ID 0.
M6. OAuth redirect URI hardcoded to production domain
File: integrations/google-calendar/class-oauth-handler.php lines 40, 72
$redirect_uri = 'https://jachty.pagedev.pl/wp-admin/...' — literal string, not admin_url(). OAuth fails completely on staging or local dev. Comment acknowledges this is intentional but makes testing and domain migration fragile.
LOW — Cleanup / Polish
L1. Dead code: Availability::mark_as_available() has bug but is never called
File: includes/class-availability.php lines 123–149
The function ignores its $start_date/$end_date parameters and runs $wpdb->delete() filtered only by yacht_id — which would wipe the entire availability table for that yacht.
Verification: grep across the plugin shows zero call sites. All code that handles cancellation/deletion uses clear_booking_availability($booking_id) (in Admin, Rest_Controller, GCal_Service, ICal_Import), which correctly filters by booking_id.
Risk: Trap for a future developer who reads the method name and decides to use it. Recommend either deleting the function or replacing its body with the commented-out correct implementation that already lives below it (lines 137–146).
L2. Dead form field: yacht_booking_nonce from wp_nonce_field is never read
Files: frontend/class-shortcode.php:329, frontend/class-calendar-widget.php:429
The booking form renders wp_nonce_field('yacht_booking_submit', 'yacht_booking_nonce') as a hidden input, but calendar.js (lines 336–343) builds formData manually from named fields and never includes it. The actual nonce travels via X-WP-Nonce header from yachtBookingData.nonce (a wp_create_nonce('wp_rest') value), which matches what Rest_Controller::create_booking() verifies.
Risk: Code rot — looks like it's doing something but isn't. Either remove the wp_nonce_field call, or wire JS to send it as a fallback for the no-JS case (currently there is no no-JS fallback anyway).
L3. Logging disabled in production
All error_log() calls gated on WP_DEBUG === true. GCal sync errors, token refresh failures, and iCal import problems are silently discarded in production. Consider a persistent log table or option-based error queue surfaced in admin.
L4. No incremental DB migration path
File: includes/class-installer.php
No upgrade() method runs when yacht_booking_version in wp_options differs from YACHT_BOOKING_VERSION. dbDelta() adds missing columns but won't remove old ones or modify existing column types. Future schema changes risk silent failures on existing installs.
L5. yacht_booking CPT exposed to default WP REST API
File: includes/class-booking.php line 48
show_in_rest => true registers the CPT at /wp/v2/yacht_booking. The plugin's own /yacht-booking/v1/ namespace is the intended REST surface. Set show_in_rest => false unless there is a reason for the default WP exposure.
L6. Booking::create() does not validate end > start
File: includes/class-booking.php lines 229–230
A booking with end_date before start_date is stored without error. get_date_range() while-loop exits immediately, no availability rows are created, and the booking is invisible in the calendar but exists as a CPT post.
CHANGELOG
2026-05-05 (revision): Subagent audit produced two false positives that were initially listed as CRITICAL. Verified by tracing call sites:
- Old C1 (
mark_as_availabledeletes all rows) → demoted to L1. The function is never called anywhere in the plugin; all cancellation paths use the correctclear_booking_availability($booking_id). Remains a trap for future devs but no production impact. - Old C3 (POST /bookings nonce mismatch) → demoted to L2. The actual auth path (JS →
X-WP-Nonceheader →wp_verify_nonce(..., 'wp_rest')) is fully consistent. Thewp_nonce_field('yacht_booking_submit', ...)in shortcode/widget renders a hidden input that JS never reads — dead form field, not a security gap.
Race condition (old C2 → C1) and unencrypted OAuth tokens (old C4 → C2) remain valid CRITICAL items.