clinicpro/docs/audit-backlog.md

Backend Audit Backlog — ClinicPro

Durable backlog for the backend-audit branch (prompt: .claude/prompt/full-backend-audit.md). This file is the source of truth — TodoWrite is volatile and does NOT survive a session restart. Status legend: ✅ done · ▶ doing · ☐ todo. Order: Critical → High → Medium → Low. Every fix needs a regression test that fails without the fix and passes with it.

Last full scan: 2026-06-28 (5 parallel investigators: idor/massassign, auth-surface, api/quality, db-integrity, perf).

---

✅ DONE (committed on backend-audit)

#	Task	Cat	Commit
D1	Functional test infra (ApiTestCase + db_test + JWT helpers)	test	30c5fbe
D2	repositoryClass on all entities w/ custom repo	orm	05fde81
D3	regression: every custom-repo entity maps its repositoryClass	test	372bea4
D4	IDOR — ownership on GET weekly-schedule	security	90736c8
D5	IDOR — ownership on GET date-override (single)	security	0932930
D6	onDelete on required FKs (Clinic, Doctor, ClinicDoctorInvitation, DoctorInsurance, DoctorSecretary, ClinicSubscription, SubscriptionPeriod)	db	36b87e9
D7	N+1 — batch-fetch pending payments in expiry loop	perf	e174046
D8	missing indexes (appointment expiry, session dates, user status)	perf	8b96753
D9	repair phpstan config	devops	e456809
D10	priv-esc — commission_percent/active admin-only on PATCH representation	security	6bd49c2
D11	C1 payment callback verifies gateway-confirmed amount vs stored amount (anti underpayment / RefNum-replay)	security	(this commit)

---

☐ CRITICAL

None outstanding.

---

☐ HIGH

#	Task	File:line	Cat	How to test
✅H1	IDOR write: createAppointment trusts request doctor_uuid, no scope check — any staff books onto any doctor's calendar	src/Appointment/Controller/MyAppointmentsController.php:59	security-idor	DONE — canBookForDoctor() scope gate + tests/Appointment/BookingScopeTest
✅H2	No UNIQUE (doctor_id, slot_start) on Appointment → double-booking race (index is non-unique)	src/Appointment/Entity/Appointment.php	db-unique	DONE — nullable unique active_slot_key (occupying = pending/confirmed, mirrors isSlotTaken); bookAtomically catches the unique violation + expires lapsed pendings in-txn; all 3 booking paths (online/my/admin) routed through it. Migration backfills one row per slot (non-destructive). tests/Appointment/SlotUniquenessTest. NB: backfill surfaced a real pre-existing double-booked slot in dev data (doctor 1764, two expired rows) — harmless (both expired = key NULL).
✅H3	Payment.referenceId not unique → same gateway callback credited twice	src/Payment/Entity/Payment.php:61-62	db-unique	DONE — unique index on reference_id (NULL until success → no collision) + callback pre-check rejects replays. tests/Payment/PaymentCallbackAmountTest::testReplayedGatewayReferenceIsRejected
✅H4	FinancialBreakdown.payment onDelete CASCADE on non-nullable FK → deleting a Payment destroys ledger rows; should be RESTRICT	src/Settlement/Entity/FinancialBreakdown.php:28-30	db-ondelete	DONE — onDelete RESTRICT + migration. tests/Settlement/FinancialBreakdownIntegrityTest
✅H5	Insurance pricing/coverage modeled as raw int FKs (no FK/onDelete) → orphan rows on delete	src/Insurance/Entity/EntityInsurancePricing.php · TenantInsurance.php · TenantServiceCoverage.php	db-ondelete	DONE (entity-owner path) — entity_id is polymorphic (doctor|clinic) so no DB FK is possible; added TenantInsuranceCleanupService::purgeForEntity() wired into doctor + clinic DELETE (purges tenant_insurances + pricing + coverage). tests/Insurance/TenantInsuranceCleanupTest. Residual (→ M20-adjacent): orphans when an insurance category itself is deleted (insurance_id ref) or a service_item is deleted (service_item_id ref) — different deletion paths, lower freq.
✅H6	N+1: clinic doctors list lazy-loads specialties per doctor	src/Doctor/Repository/DoctorRepository.php (findByClinicWithFilters)	perf-nplus1	DONE — addSelect('s') + Paginator(fetchJoinCollection:true). Added ApiTestCase::countQueries() helper. tests/Doctor/ClinicDoctorListNPlusOneTest (query count constant vs doctor count; verified 4→10 without fix).
✅H7	N+1: comments list lazy-loads likes→user, replies (recursive), author realName	src/Rating/Repository/CommentRepository.php (findApprovedRootsByDoctor)	perf-nplus1	DONE — two fetch-join passes (roots+user+likes; replies+their user+likes+one more reply level) hydrate everything toArray() touches → bounded queries for a 2-level thread. Functional correctness test tests/Rating/CommentListNPlusOneTest (like counts, approved-only replies). Query-count assertion unreliable through HTTP here, so correctness-tested.
✅H8	N+1: insurance service-coverage serviceItemRepo->find() per row in array_map	src/Insurance/Controller/InsuranceController.php:406-420	perf-nplus1	DONE — batch findBy(['id' => $ids]) + uuid map. Verified by functional correctness test (tests/Insurance/ServiceCoverageNPlusOneTest); query-count assertion was unreliable for this endpoint (identity-map), so correctness-tested instead.
✅H9	Unbounded list: listClaims loads ALL tenant claims, no LIMIT/pagination	src/Billing/Controller/BillingController.php · ClaimRepository.php	perf-pagination	DONE — findByTenant(page,limit) + countByTenant (shared QB), default limit 50/max 100. Additive data.meta (envelope unchanged → backward compatible). tests/Billing/ClaimsListPaginationTest.
✅H10	Unbounded list: wallet/transactions loads ALL user transactions	src/Settlement/Controller/SettlementController.php · WalletTransactionRepository.php	perf-pagination	DONE — finding overstated: findByUser already defaulted to limit=50 (bounded, just page-less). Added page/offset + countByUser + data.meta. tests/Settlement/WalletTransactionsPaginationTest.

---

☐ MEDIUM

#	Task	File:line	Cat	How to test
✅M1	Mass-assign: claim approve/pay lets owning tenant set arbitrary approved_rials/paid_rials + force status	src/Billing/Controller/BillingController.php	security-massassign	DONE — status-jump part was already guarded by canTransitionTo (false alarm). Added bounds: approved ∈ [0, claimed], paid ∈ [0, approved] → 422. tests/Billing/ClaimAmountBoundsTest.
✅M2	IDOR read: GET /insurance/{id} (showDoctorInsurance) leaks any doctor's insurance incl. negotiated price by id enumeration	src/Insurance/Controller/InsuranceController.php:499	security-idor	DONE — auth+ownership (owner/admin). tests/Insurance/DoctorInsuranceOwnershipTest
✅M3	IDOR read: showAddress loads any DoctorAddress by id, no owner check	src/Doctor/Controller/DoctorController.php:559	security-idor	DONE — ownership mirror of PATCH/DELETE. tests/Doctor/DoctorAddressOwnershipTest
✅M4	IDOR write: ClinicService createItem/updateItem bind staff via global staffRepo->findByUuid(), no entity-scope check (cross-tenant staff binding)	src/ClinicService/Controller/ClinicServiceController.php:157,195	security-idor	DONE — staff must match tenant (entity_type/id) → 422. tests/ClinicService/ServiceItemStaffOwnershipTest
✅M5	IDOR read: AppointmentSettings list endpoints leak any doctor's config — listOverrides, listHolidays, availableLocations (no ownership on {doctorUuid})	src/Appointment/Controller/AppointmentSettingsController.php:150,256,349	security-idor	DONE — ownership added to listOverrides/listHolidays/availableLocations. tests/Appointment/AppointmentSettingsListOwnershipTest
✅M6	OTP send-code has no per-mobile/per-uuid cap, only per-IP (5/hr) → SMS flood from rotating IPs	src/Auth/Controller/AuthController.php:136-153 · OtpService.php:59-77 · rate_limiter.yaml:4-7	security-ratelimit	DONE — per-mobile limiter (5/hr) added alongside per-IP. tests/Auth/SendCodeMobileRateLimitTest (6 reqs / 6 IPs → 6th 429).
✅M7	Refresh token not rotated on use (same raw token 30d), never re-checks user status	src/Auth/Controller/AuthController.php:477-480 · TokenService.php:33-45	security-auth	DONE — single-use rotation (revoke old + issue new) + suspended-user (status!=1) rejected. tests/Auth/RefreshTokenRotationTest.
M8	N+1: secretary list lazy-loads secretary/doctor ManyToOne per row	src/Secretary/Controller/SecretaryController.php:192,213	perf-nplus1	Profiler secretary list → ~2 queries/row
M9	N+1: billing claims lazy items + insuranceRepo->find() per claim in enrichClaims	src/Billing/Controller/BillingController.php:~49,68	perf-nplus1	GET claims → items+insurance query/claim
✅M10	Unbounded list: listMine settlements findByUser no limit	src/Settlement/Controller/SettlementController.php:193	perf-pagination	DONE — page/limit + countByUser + data.meta. tests/Settlement/SettlementListPaginationTest
✅M11	Unbounded list: admin pendingComments findPending no limit	src/Rating/Controller/RatingController.php:350	perf-pagination	DONE — findPending(limit,offset)+countPending+meta. tests/Rating/CommentPaginationTest::testAdminPendingListPaginates
✅M12	Unbounded list: public listComments per-doctor no limit	src/Rating/Controller/RatingController.php:270	perf-pagination	DONE — Paginator(fetchJoinCollection) page/limit + countApprovedRootsByDoctor + meta. tests/Rating/CommentPaginationTest::testPublicListPaginates
⚠️M13	Missing index: ServiceItem.section_id FK fully unindexed (entity has zero indexes)	src/ClinicService/Entity/ServiceItem.php:23-24	perf-index	EXPLAIN findBySection → no full scan
✅M14	Missing index: WalletTransaction(user_id, type) composite for per-type SUM	src/Settlement/Entity/WalletTransaction.php:38-39	perf-index	DONE — added composite idx_wallet_user_type + migration. InfraSmokeTest::testWalletUserTypeIndexExists.
⚠️M15	Missing index: ClinicDoctorInvitation.doctor_id FK unindexed	src/ClinicInvitation/Entity/ClinicDoctorInvitation.php:41-42	perf-index	FALSE POSITIVE — doctor_id has a FK → auto-indexed (IDX_26DCCFEB87F4FB17 exists). No change.
M16	User.email and User.nationalCode not unique → duplicate identities	src/Auth/Entity/User.php:31-32,37-38	db-unique	Insert two users same email/national_code → 2nd rejected. ⚠️ check dup data first
M17	Payment.gatewayToken not unique	src/Payment/Entity/Payment.php:58-59	db-unique	Two payments same gateway_token → rejected
M18	DateOverride no UNIQUE (doctor_id, date) → ambiguous schedule	src/Appointment/Entity/DateOverride.php:12	db-unique	Two overrides same doctor+date → rejected
M19	FinancialBreakdown no unique (payment_id, source) → double-accounting	src/Settlement/Entity/FinancialBreakdown.php:28-33	db-unique	Two breakdowns same payment+source → rejected
M20	Soft-ref FKs orphan (Billing/ClinicService): ClaimItem.invoice_item_id, Claim.insurance_id, Tariff.service_item_id, TenantServiceCoverage.service_item_id, DoctorAddress.clinicId, polymorphic SmsWallet, ClinicStaff	src/Billing/Entity/ClaimItem.php:22 · Claim.php:48 · ClinicService/Entity/Tariff.php:23 · Insurance/Entity/TenantServiceCoverage.php:26 · Doctor/Entity/DoctorAddress.php:32 · Sms/Entity/SmsWallet.php:18-22 · Staff/Entity/ClinicStaff.php:22-26	db-ondelete	Delete parent → child handled (cascade/restrict/set-null), no orphan
M21	~30 ad-hoc raw error codes not in ErrorCodes.php (USER_NOT_FOUND, INVALID_ROLE, SLOT_TAKEN, DUPLICATE_REQUEST, ERR_GONE, ERR_ACCESS_DENIED, …) → message() returns "خطای ناشناخته"	src/Admin/Controller/AdminApiController.php (many) · MyAppointmentsController.php:41-80 · PreRegistrationController.php:66 · CategoryController.php:36-40 · ClinicInvitationController.php:203	quality-errorcodes	Trigger each → code present in ErrorCodes.php

---

☐ LOW

#	Task	File:line	Cat
L1	IDOR: updateSession skips assertPatientGate (ownership OK, gate inconsistent)	src/Patient/Controller/PatientController.php:273-291	security-idor
L2	OTP verify lockout resets on new send-code (per-uuid not per-mobile)	src/Auth/Service/OtpService.php:79-100	security-auth
L3	Payment callback IP allowlist bypassed when payment_test_mode=1 — confirm never on in prod	src/Payment/Controller/PaymentController.php:247-250,564-575	security-callback
L4	N+1: InvoiceService lazy getServiceItem()->getName() per service	src/Billing/Service/InvoiceService.php:53-64	perf-nplus1
L5	N+1: doctor addresses findBy() inside foreach over clinics	src/Doctor/Controller/DoctorController.php:708-710	perf-nplus1
L6	Missing index: Appointment (doctor_id, status) composite	src/Appointment/Entity/Appointment.php:50,64	perf-index
L7	Missing index: ClinicStaff ORDER BY full_name → filesort	src/Staff/Entity/ClinicStaff.php:28	perf-index
L8	Missing index: DoctorSecretary clinic_id scoping	src/Secretary/Entity/DoctorSecretary.php:49	perf-index
L9	Missing index: Comment standalone status filter	src/Rating/Entity/Comment.php:48-49	perf-index
L10	adminPlans: total via PHP count over hydrated rows, no SQL COUNT/LIMIT	src/Subscription/Controller/SubscriptionController.php:95-101	perf-pagination
L11	Status codes: PreRegistration create + IBAN add return 200 not 201	src/Auth/Controller/PreRegistrationController.php:73 · Representation/Controller/RepresentationActionController.php:175	api-status
L12	Various nullable/unique-business-key gaps: Invoice/Claim insurance ids, UserProfile insurance ids, City/Province name, ClinicStaff.nationalCode, MobileVerificationOtp, SmsSettings, SmsLog.templateUuid	(see db scan)	db-unique/ondelete

---

☐ EPICS (design debt — cross-repo, defer; do NOT quick-fix)

#	Task	Scope	Note
E1	Double-nest response contract — ~12 controllers return success(['data'=>X]) → {data:{data:X}}, others flat. Inconsistent envelope.	AppointmentSettings, Appointment, Rating, Settlement, Secretary, Sms, Location, Doctor, Blog, DoctorService/Specialty/Tag, Representation, UserProfile, Clinic, Insurance, Billing	Documented pitfall in CLAUDE.md; clients (nobat724_front, clinic-pro-tauri) already read data.data. Unifying breaks all 3 clients → needs coordinated cross-repo change + versioning. NOT a quick bug.
E2	No DTO/validator on sensitive writes — admin create, auth flows, payment verify, booking read raw json_decode arrays	Admin, Auth, Payment, Appointment, Settlement controllers	Introduce request DTOs + validator incrementally. Large.
E3	Fat controllers — AdminApiController (1938 LOC), MyAppointmentsController booking, RepresentationActionController (835), DoctorController/ClinicController	extract per-domain Services	SOLID refactor; lower urgency than security/db.
E4	CI — no .github/workflows; add phpunit + phpstan + migrate-on-empty-db	devops	prompt var §DevOps
E5	phpstan baseline dirty — 41 pre-existing errors across the codebase (D9 only repaired the config so it runs). Audit fixes must not add new ones; cleaning the 41 is its own task.	devops	ddev exec php vendor/bin/phpstan analyse → 41 errors (e.g. SlotCalculatorService.php:233, SubscriptionController.php:33)
E6	No test DB isolation — ApiTestCase doesn't reset/rollback db_test between tests/runs, so rows accumulate; count/time-based assertions are fragile (hit twice this session). Add per-test transaction rollback or a DB reset.	test	tests rely on random keys + relaxed assertions as a workaround

---

Tallies

• Security: 16 (1 critical, 1 high IDOR-write, rest med/low) · Performance: 20 · DB-integrity: 33 · API/quality: 28 (mostly E1 nesting) · DevOps: 1 epic.
• Already fixed (pre-audit-session): 10.

Progress (this audit session)

• ✅ CRITICAL: 1/1 (C1)
• ✅ HIGH: 10/10 (H1–H10) — the prompt's hard completion gate (no Critical/High remaining) is MET.
• ☐ MEDIUM: 0/21 · ☐ LOW: 0/12 · ☐ Epics E1–E6.
• 11 task-commits, each with a regression test + docs; full suite 36 tests / 82 assertions green; phpstan adds 0 new errors over the 41-error baseline (E5).