Production readiness review — October 18, 2026

Follow-up implementation: see PRODUCTION-FIXES.md for the October 19 code changes, new migration, tests, and remaining host acceptance work. The findings below describe the reviewed version before that implementation.

Recommendation: hold production launch until the security defects and financial recordkeeping gaps below are resolved and the production deployment checks pass. The recent successful workflow tests remain useful, but do not cover these gaps.

This review inspected the current source, schema, server rules, configuration, dependencies, and deployment documentation. Composer's locked dependency audit returned no published advisories or abandoned packages. One security defect was reproduced using a disposable local database; that database was removed and its server stopped. No development database records or application code were changed during this review. The earlier 231-check suite and 54 development checks were not rerun as part of this review.

1. P1 — Revoke reset links when account security details change

Evidence: classes/MaintenanceService.php:110–137 revokes sessions on account changes but leaves password reset tokens intact. classes/AuthService.php:150–181 accepts a token based on its user ID and expiry without binding it to the account version or email that existed when it was issued.

Confirmed reproduction: issue a valid token for old@example.test, change the account email to new@example.test through MaintenanceService, then submit the old token. The reset succeeds. Someone holding a link delivered to the old mailbox can therefore regain access while the token remains valid, up to 30 minutes.

Fix: invalidate existing tokens transactionally on email and other security changes; bind issued tokens to the relevant account version so concurrent issuance cannot leave an old token valid. Cover email changes, disable/reactivate, role changes, and concurrent reset issuance with regression tests.

2. P1 — Replace the vulnerable browser dependency

Evidence: xsl/content.xsl:41, xsl/login.xsl:36, and xsl/createAccount.xsl:37 load js/jquery-3.2.0.min.js.

This version predates fixes for published cross-site scripting vulnerabilities in HTML manipulation. The upstream advisories describe vulnerable operations and affected versions; this review does not claim a demonstrated application exploit. Composer audit does not cover bundled JavaScript files.

Fix: select a patched, supported jQuery version, review the bundled plugins for compatibility, and rerun browser tests. Track browser dependencies in a manifest and include them in dependency checks.

Primary sources:

3. P1 — Complete financial history and corrections before real dues use

Evidence: classes/Roster.php:66–71 displays charges, summed payments, and balances, but no itemized payment history. classes/RosterService.php:62–79 supports inserting charges and payments, without a correction, void, or reversal workflow. migrations/002_roster.sql:30–49 has no recorded-by actor, correction reason, or payment reference fields.

A payment or charge entered incorrectly currently requires direct database work. The user also cannot inspect each payment's date and details in the application.

Fix: add itemized payment history and financial export/reconciliation; record who entered each item and a reference/method where appropriate; implement permissioned, audited adjustments or reversals that preserve the original entry. Test balance recalculation, repeated requests, and concurrent corrections. This concerns the manual ledger already in scope; online payment processing is a separate feature.

4. P1/P2 — Preserve and complete the audit trail

Evidence: classes/Journal.php:38–44 lets an administrator truncate the complete journal. classes/RosterService.php:44–61 does not audit membership or season changes. Financial messages at lines 67 and 77 omit amounts and the new financial record's ID. classes/Audit.php:3–7 catches write failures; financial writes commit before their audit messages are attempted. Administrative account changes also lack corresponding detailed audit entries.

Consequences: changing club access may leave no journal event, and the journal can be erased. A financial save can succeed while its audit record is lost.

Fix: remove unrestricted journal clearing, define retention and protected archival, record actor/entity/before-and-after details for privileged changes, and write financial audit events in the same transaction or a transactional outbox. Keep authentication available if its logging backend fails, with failure monitoring.

5. P2 — Verify email ownership before relying on it for club access

Evidence: classes/AuthService.php:86–113 immediately activates a self-registered account without verifying its mailbox. classes/RosterService.php:44–55 assigns membership to an existing username or email with no verified-email check.

An account can be registered using someone else's email. Signup alone does not grant membership, but an administrator assigning that expected email could select an account controlled by the wrong person.

Fix: implement mailbox verification or an invitation acceptance flow, and require verified identity before assignment by email. Email changes should require fresh verification and invalidate outstanding reset links. Until then, an explicit independent identity-check procedure is needed for assignments.

6. Release gate — Verify production isolation, transport, and secrets

These are deployment checks, not claims that a public leak currently exists.

classes/Base.php:145–153 has no database TLS certificate/verification options. The approved non-TLS development connection does not establish a production transport design. If production uses a remote database, configure verified TLS or an explicitly secured private transport; use a restricted runtime database account and a separate migration account.
classes/Base.php:432–440 automatically prefers xml/settings.local.xml before environment overrides. Build a clean production artifact that excludes local secrets, .tools backups and control files, .git, tests, and IDE files. Current workspace changes include untracked application classes and migrations; ensure the release captures every required file.
Apache private-directory rules depend on rewrite configuration. IIS and the development router have different rules; no Nginx configuration is provided. Verify private paths and backup filenames return a denial on the actual host. Prefer placing private code, secrets, and backups outside the document root.
main.php:20–30 detects HTTPS only through $_SERVER['HTTPS']. If TLS terminates at a proxy, configure a trusted proxy/server arrangement; otherwise HTTPS visitors can be rejected or cookies can be misconfigured. Do not trust arbitrary client-supplied forwarding headers.
Confirm production environment mode, secure cookies, PHP error-display settings, HTTPS enforcement, and secret provisioning. Add HSTS at the HTTPS server and a tested Content Security Policy; the latter needs work on inline scripts and existing JavaScript links.
Perform a production-host invitation/reset test using the configured application URL and sender. Earlier SMTP acceptance and local tests do not establish inbox delivery or the final hosting setup. This review did not test production DNS.

7. Release gate — Make backup, monitoring, and deployment repeatable

The verified manual backups prove a backup can be restored; the repository does not demonstrate an automated production backup/restore policy, alerting setup, or release pipeline. These may exist outside the project and should be verified.

Before launch:

Schedule protected backups outside the application document root, define retention and recovery objectives, and periodically restore-test them.
Monitor application failures, audit failures, SMTP delivery, database health, disk capacity, and failed backup jobs.
Schedule bin/cleanup-auth.php as documented; ensure scheduled jobs use the correct environment and credentials.
Create a reproducible release with platform-requirement checks, dependency audits, isolated regression tests, migration verification, and a recovery plan.
Test representative roster sizes. Club rosters/dues and exports currently load complete result sets; large clubs need paging, query/performance testing, and export limits or background generation as appropriate.

Product decisions and lower-priority improvements

All club management currently belongs to global site administrators, who can access every club. This matches the implemented scope. If each club will manage itself, scoped club-administrator roles and cross-club authorization tests are needed before that operating model launches.
Active members can see their club's other active members' email addresses (RosterService.php:22–24). Confirm that this is the intended directory/privacy policy; consider restricted visibility or opt-in contacts.
Add MFA and recent-password confirmation for sensitive administrator actions. These are additional protections, not a claim that current session checks fail.
Add season editing with history, clearer payment error recovery, keyboard/mobile usability checks, and repair the garbled username validation text in AuthService.php:95. These should follow the security and ledger work.

Recommended order: reset-token invalidation; browser dependency update; durable financial history/corrections and audit; verified identity; production deployment and operational verification. Resolve the club-administration and directory-privacy decisions before enabling real club users.

9.9 KiB Raw Permalink Blame History Unescape Escape