# Clinic CMS Final Engineering Audit

**Date:** 2026-08-12 → 2026-08-13 · **Auditor:** autonomous engineering session (Claude) · **Scope:** full repository, live installation at clinic.tmsoagency.com, test suite, gates, database, backups, scheduler
**Method:** AUDIT → FIND → REPRODUCE → ROOT-CAUSE → FIX → TEST → REGRESSION TEST → RE-AUDIT

---

## 1. Executive Summary

**Overall status: PRODUCTION READY WITH CONDITIONS.**

The codebase is unusually sound at its foundations. Patient files are stored on a private disk and served only through authorised, audited application routes — there is no `public/storage` symlink on the box at all. Money is integer minor-units with signed amounts. Every concurrency-critical invariant is enforced at the database level (unique `slot_key` against double booking, unique `branch_id+mrn`, unique idempotency keys on payments and dispensings), not merely in application code. Module boundaries hold at zero deptrac violations, PHPStan level 6 is clean, and the full test suite finishes green.

The audit found **8 genuine defects**. Six were fixed and regression-protected within this cycle. The remaining two conditions are configuration, not code, and are listed in §17: outgoing mail still writes to a log file (password resets do not reach anyone), and backups never leave the machine.

Nothing found rose to C0/C1: no PHI exposure, no authentication or authorisation bypass, no financial corruption, no cross-branch leakage.

---

## 2. Baseline (captured before any change)

| Item | Value |
|---|---|
| Git | branch `main` @ `16eba80`; working tree carried 2 uncommitted files from a prior session (DrugNameNormaliser salt-only fix + its test) |
| PHP / Laravel | 8.3.30 / 12.64.0 |
| Database | MySQL 8.4.3, app user `clinic_app` (no DDL; migrations via `mysql_migrations`) |
| Node / npm / Composer | 24.16.0 / 11.13.0 / 2.9.4 |
| Environment | `APP_ENV=production`, `APP_DEBUG=false`, session: database driver + encrypted + secure cookie + http-only + lax, cache file, queue database, mail **log**, `LOG_LEVEL=warning` |
| Tests | **1178 passed, 2 FAILED** (3,526 assertions, 1,135 s) |
| PHPStan | level 6 — clean |
| Deptrac | 0 violations, 0 errors |
| Pint | 4 files drifted |
| composer audit | **6 advisories, 1 package** (league/commonmark ≤ 2.8.3, incl. CVE-2026-71478) |
| npm audit | 0 vulnerabilities |
| `system:verify` | PASS with 3 WARN (no queue worker; mail=log; local-only backups) |
| `audit:verify` | **chain broken at 12 of 2,592 entries** |
| Backups | nightly 02:30 dump present same day, `backups:verify` PASS; Windows tasks `Clinic-Scheduler` and `MySQL Daily Backup` present and Ready |

Baseline overall: **8.2 / 10**.

---

## 3. Findings

Severity scale: C0 critical · C1 high · C2 medium · C3 low · C4 informational.

### C2-001 — Two Messaging tests depended on the wall clock
- **Module:** Messaging · **Files:** `MessageDispatchTest.php`, `ReminderSourceTest.php`
- **Symptom:** baseline suite, run at ~23:00, failed 2 tests that pass during office hours.
- **Reproduced:** yes — in the real nightly baseline run.
- **Root cause:** the tests never froze time. "Useful for another hour" reaches past a 23:59 quiet-hours opening when it is already 23:05; "tomorrow plus two hours" is a different calendar day after 22:00, and the reminder window is keyed to the calendar day. **The production logic is correct in both cases** — deferring a message whose window opens before it expires, and day-keying reminders, are the designed behaviours.
- **Fix:** clock pinned to `2026-08-05 10:00 UTC` in both files' `beforeEach` (the framework clears it between tests); the file's own fixtures already assumed that date.
- **Verification:** Messaging suite 51/51 green; both tests now pass at any hour. **FIXED**

### C2-002 — `audit:verify --from` falsely reported the first row of every window
- **Module:** core console · **File:** `app/Console/Commands/VerifyAuditChainCommand.php`
- **Symptom:** `audit:verify --from=147` reported id 148 broken while the full run said 148 was fine.
- **Reproduced:** yes, on the live database.
- **Root cause:** the running hash was seeded `null` on a windowed start instead of from the stored hash at the boundary, so the first row inside every window recomputed against nothing.
- **Fix:** seed from the stored hash of the newest row at/below `--from`.
- **Regression test:** added to `tests/Feature/AuditTrailTest.php` — a windowed check of an intact chain must exit 0, and a window opened just before real tampering must still exit 1. 7/7 green. **FIXED**

### C2-003 — Live audit chain: 12 broken entries, all from day one
- **Module:** audit trail (live data) · ids 10, 54, 55, 65, 68, 74, 103, 106, 116, 122, 125, 147
- **Investigation:** every broken row was created **2026-08-02** — the first day of live setup — and falls exactly in the types whose morph/namespace representation changed later that day (FQCN `Modules\Auth\Models\DoctorProfile`, `App\Models\PersonalAccessToken`, early `billable_service`/`lab_test` rows). With the fixed verifier: **the chain is intact from id 148 (2026-08-02 21:20) through today — 2,445 consecutive entries verified.** The evidence pattern is day-one structural churn, not tampering: damage is bounded to the hours the hash shape was still moving, and nothing after it fails.
- **Disposition:** the rows were **not** rewritten — rewriting audit rows to make hashes pass is forging the trail, which is the one repair this table must never receive. The exception is documented (here, in `.env`, and in the session memory), and the nightly check is pinned past it: `AUDIT_VERIFY_FROM=147`. `audit:verify --from=0` still checks all of history on demand and will always list these 12. **RESOLVED — DOCUMENTED**

### C2-004 — league/commonmark carried 6 security advisories
- **Root cause:** transitive dependency of laravel/framework at 2.8.3; advisories include CVE-2026-71478 (unsafe-link filter bypass).
- **Fix:** targeted lock update 2.8.3 → **2.10.0** (same major, patch-safe). `composer audit`: **0 advisories**. Full suite green afterwards. **FIXED**

### C3-001 — The audit chain was never verified on a schedule
- **Symptom:** `audit:verify` existed but only ran when somebody typed it; a broken chain would surface months late, when it can no longer be explained.
- **Fix:** scheduled daily at 08:10 (after the 08:00 backup check), reading its start from `config('audit.chain.verify_from')` — a real config key, not a raw `env()` call, so customer installs that cache config keep working. Registration confirmed via `schedule:list`. **FIXED**

### C3-002 — Format drift and stale documentation
- Pint: 4 drifted files fixed (two test files, two lang files' line endings).
- `docs/DEPLOYMENT.md` still stated the test suite cannot run on this host and `clinic_test_app` was dropped — both false since the account was recreated. Rewritten to the current truth, including the two standing rules (one test run at a time; never `config:cache` here). **FIXED**

### C4-001 — Two controllers write Eloquent directly
- `ScheduleController` (rotas) and `PaymentMethodController`. Both are configuration CRUD: authorization lives in their FormRequests, validation is complete, and all three models carry the `Auditable` trait, so the audit trail is written regardless. Not refactored — moving working, guarded config-CRUD into services would be stylistic churn. **DOCUMENTED**

### C4-002 — Queue driver `database`, no worker
- Verified: **nothing in the application implements `ShouldQueue`** (0 pending, 0 failed jobs on live) — currently harmless. The day SMTP plus queued notifications arrive, a worker becomes mandatory; `system:verify` already warns about exactly this. **DOCUMENTED**

---

## 4. Major Architectural Changes

None required. Deptrac: 0 violations before and after; controllers delegate to services on every business workflow (patient creation, booking, billing, dispensing, lab ordering all have exactly one service-level source of truth, shared by web and API entry points — verified for public booking and billing specifically).

## 5. Security Improvements

- league/commonmark advisories eliminated (2.10.0).
- Audit chain now monitored nightly; verifier's windowed mode made trustworthy.
- Verified working as designed (no change needed): no `public/storage` symlink; all uploads on the `private` disk; both file-download routes authorise per-request; portal upload links are 238-bit tokens, hash-stored, throttled 30/min (view) and 10/min (upload), MIME sniffed via finfo against an allow-list, stored under random ULID names; public booking API throttled 6/hour on store; status lookup never names the patient; security headers (CSP `script-src 'self'`, HSTS, nosniff, DENY, same-origin referrer) present; exception envelope leaks no SQL/path/class in production and carries X-Request-Id; secrets sweep of app/config clean; audit values redact passwords/tokens/secrets.

## 6. Performance Improvements

No blind optimisation performed. Verified statically: `Model::preventLazyLoading` outside production (N+1s fail tests before they ship); hot indexes present (`branch_id+name_normalized`, `branch_id+phone_normalized`, blind `national_id_index`, appointment/doctor/patient starts_at composites, invoice line/settlement composites); dashboard aggregates use single grouped queries. Load behaviour under production traffic: **UNVERIFIED** (no load harness on a live box).

## 7. Database Improvements

None needed this cycle. Verified: FKs restrict-on-delete for clinical rows; business invariants at DB level (unique `slot_key`, `branch_id+mrn`, `branch_id+reference`, `branch_id+idempotency_key` on payments and dispensings); append-only enforcement on `audit_logs` and `stock_transactions` at the model layer; migrations ledger (78) exactly matches files on disk (78).

## 8. API Improvements

None needed. Verified: everything on `api/v1` behind `auth:sanctum` + `throttle:api` (guest surface separated behind `throttle:api-guest` with tighter per-route throttles); one response envelope `{success, code, message, data, meta, errors}` with stable machine codes; `PatientResource` exposes ULID, never the internal id, and never `national_id`; `X-Branch` honoured only with `branches.switch`, anything else a uniform 403. OpenAPI docs freshness: **UNVERIFIED**.

## 9. Patient Data / PHI Improvements

None needed — see §5. National ID encrypted at rest with a blind index for search/duplicate detection.

## 10. Appointment Improvements

None needed. Double booking is impossible past the DB (`slot_key` unique index) even if the row lock misses; both were already regression-tested. All entry points (desk, API, public page, widget) resolve to the same `AppointmentService`.

## 11. Billing Improvements

None needed this cycle (the department split, check-in fee, refund-netted revenue and draft-vs-issued semantics all landed and were verified earlier the same week). Idempotent payments and overpayment refusal re-confirmed by the full suite.

## 12. Laboratory Improvements

None needed. Own-lab OFF path verified live earlier this week: no internal invoice, request prints as an external recommendation. Charges flow only through the `ChargeCollector` port — the module holds no invoice logic.

## 13. Pharmacy Improvements

None needed this cycle (FEFO under row locks, append-only ledger with in-lock `balance_after`, expired stock never allocated or counted, all-or-nothing dispensing, reversal restocks the originating batch and never silently edits the bill). One pre-existing parser bug ("three times daily" read as once daily) was found and fixed by walkthrough earlier the same day, with a 20-case regression table.

## 14. Backup Improvements

- Verified: nightly 02:30 box dump present and `backups:verify` passes daily at 08:00; in-app encrypted pipeline (`backups:run`, 03:15) present; `docs/RESTORE.md` exists and names the migrations-ledger trap.
- **Not improved (blocked on credentials):** nothing leaves the machine — see §17/§18. A full restore drill was **not** run on the live box (deliberate; procedure documented). **UNVERIFIED: restore.**

---

## 15. Tests

| | Before | After |
|---|---|---|
| Tests | 1178 passed / **2 failed** | **1181 passed / 0 failed** |
| Assertions | 3,526 | 3,531 |
| Duration | 1,135 s | 1,076 s |

Delta: +1 new regression test (windowed audit verification), 2 formerly failing tests fixed at the root (pinned clock). All other gates after fixes: PHPStan L6 clean · Deptrac 0 · Pint pass · composer audit 0 · npm audit 0 · frontend build green (validated into a scratch directory so live assets were never touched) · `system:verify` PASS with the 3 known WARNs · `schedule:list` shows the new 08:10 audit check.

Process note, in the interest of honest reporting: one full-suite run was invalidated mid-audit because a targeted test run was started concurrently against the shared `clinic_test` schema (a documented hazard of this box, re-triggered by the auditor). The corrupted run was discarded and the final numbers above come from a clean, exclusive re-run.

---

## 16. Quality Scores

A 10 would mean deeply verified, not merely tidy-looking. Areas marked ± were verified by code-reading and existing tests but not exercised under production-like load or via an authenticated browser sweep.

| Area | Before | After | Notes |
|---|---|---|---|
| Architecture | 9 | 9 | Deptrac 0/0; no violation found to fix |
| Code Quality | 8.5 | 9 | Drift + advisory cleared; line-by-line only where targeted |
| Security | 8 | 8.5 | commonmark patched; surface sweep clean |
| PHI Security | 9 | 9 | Every serving path authorises; no public exposure exists |
| Auth & Authorization | 9 | 9 | Fortify+2FA, per-surface rate limiters, policies, status-outer-gate |
| API Quality | 8.5 | 8.5 | Envelope + resources verified; OpenAPI unchecked |
| Database Integrity | 9 | 9 | Invariants live in the schema |
| Branch / Tenant Isolation | 8.5 | 8.5 | Global scope + header rules verified; no dedicated cross-branch matrix suite |
| Performance ± | 7.5 | 7.5 | Static verification only |
| Testing | 8 | 9 | Both flaky roots removed; suite deterministic at any hour |
| Reliability | 8 | 8.5 | Chain now monitored; idempotency everywhere it matters |
| Error Handling | 9 | 9 | One envelope, no internals leaked, request-id correlation |
| Billing Integrity | 9 | 9 | Signed amounts, idempotent, refunds net |
| Appointment Reliability | 9 | 9 | DB-level backstop |
| Backup / DR | 6 | 6.5 | Verified nightly + monitored, but local-only and no restore drill |
| Deployment Readiness | 8 | 8.5 | `system:verify` honest; docs corrected |
| Maintainability | 9 | 9 | Docs, boundaries, comment discipline |
| UX / Frontend ± | 7.5 | 7.5 | Build + CSP verified; runtime console sweep needs a login |

**Overall: BEFORE 8.2 / 10 → AFTER 8.6 / 10.**

---

## 17. Remaining Issues

1. **Mail is `log` (C2, deliberate but live-affecting).** Password resets and booking confirmations reach nobody. Next step: enter SMTP credentials in Settings → Mail; then decide whether notifications should queue (which requires a worker — see C4-002).
2. **Backups never leave the machine (C2).** A disk failure or fire takes the data and every backup with it. Next step: provision an S3-compatible bucket and set `BACKUP_S3_BUCKET` + keys; the pipeline, encryption, prune and read-back verification are already built and inert until then.
3. **Restore drill (UNVERIFIED).** Backups are verified for existence/freshness/size daily, but a full restore has not been rehearsed on this box. `docs/RESTORE.md` is the script; running it against a scratch schema some quiet afternoon would close the loop.
4. **Load/performance under real traffic and an authenticated frontend console sweep (UNVERIFIED).**

## 18. Blocked Items

- **Off-box backup destination** — requires bucket credentials only the operator can create. Everything else is ready.
- **SMTP configuration** — requires the clinic's mail credentials; never to be typed by the auditor.

Nothing else was blocked; no destructive change was needed anywhere.

## 19. Production Readiness Decision

**READY WITH CONDITIONS.**

The application is safe and correct for daily clinical use as deployed: PHI is protected, money adds up, double-booking and double-dispensing are structurally impossible, failures render politely without leaking internals, and every gate is green. The two conditions are operational, not code: until SMTP is configured the system cannot reach patients or reset passwords by email, and until an off-box bucket exists the disaster-recovery story ends at this machine's power supply. Both are single-sitting fixes once credentials exist.

---

*Changed in this audit (13 files, uncommitted at time of writing): `VerifyAuditChainCommand.php` (+`config/audit.php`, `routes/console.php`, `.env` pin), two Messaging test files (pinned clock), `tests/Feature/AuditTrailTest.php` (+1 regression), `composer.lock` (commonmark 2.10.0), `docs/DEPLOYMENT.md` (truth restored), Pint touch-ups on two lang files and one pharmacy test, plus the inherited `DrugNameNormaliser` salt-only fix and its `AllergyCheckTest` cases, verified green here.*