# Adversarial codebase audit — 2026-07-03

> Read-only diagnosis of DentalPin `main` @ `e19e081`. Nothing here was applied to the
> codebase. Produced by 8 parallel specialist audit passes (multi-tenancy, RBAC, module
> isolation, event bus, copilot/LLM boundary, backend core, frontend, documentation).
> Each pass was told to verify against source before flagging; 8 top findings were then
> spot-verified against the actual code (marked **✓ verified** below).

**Scope:** 376 Python files · 22 modules · 70 Vue components · ~255 API call sites checked.

**Triage:** ~17 critical · ~55 high · ~70 medium · ~50 low. Counts are approximate because
several passes independently reported the same root cause — deduped into the systemic
section, which is where the real signal is.

---

## Five root causes, found from five directions

These weren't flagged once. Different audit passes arrived at the same underlying defect
independently. That convergence is the strongest evidence each is real and load-bearing.

### S1 — Install state is tracked in the DB and consumed by nothing
*(module-isolation #1/#2 · event-bus #3 · copilot #6 · backend M-modscan)*

The plugin system records each module's state in `ModuleRecord`, but neither the boot
sequence nor the loader ever reads it.

- `backend/docker-entrypoint.sh:27` runs `alembic upgrade heads` unconditionally on every
  boot, so an uninstalled module's dropped tables are **re-created on the next restart**
  while its state stays `UNINSTALLED`.
- `backend/app/core/plugins/loader.py:142` mounts routers, subscribes event handlers, and
  registers copilot tools for **every module discovered on disk** — install state never
  checked.
- `backend/app/core/plugins/processor.py:259` (`_remove`) drops tables but never
  unsubscribes handlers, unmounts routers, or unregisters tools.

Net effect: after uninstalling `recalls`, its handlers keep firing against dropped tables
on every appointment (errors swallowed by the bus), its routes 500 instead of 404, its
copilot tools stay callable, and a restart resurrects its schema. This is issue #56's
"cosmetic uninstall" reintroduced one layer up — and it means the per-tenant
`modules_enabled` direction (ADR 0012) has no enforcement point today.

### S2 — Events publish inside uncommitted transactions
*(event-bus #2 · backend H-evt1)*

`get_db` commits only at request end, but nearly every publisher does `flush()` →
`publish()`, and handlers run inline in **separate sessions**. So handlers read/write
against data the publisher hasn't committed. Concretely: `billing.on_payment_refunded`
recomputes invoice status but the `Refund` row is invisible → **invoice stays "paid" after
a refund**, deterministically. The recalls auto-link handler sets an FK to an uncommitted
appointment → fails or deadlocks. Budget-line handlers commit rows in their own session
that survive a later rollback → phantom plan↔budget divergence.

Only `verifactu` and `copilot` do it right (commit-before-publish, with comments explaining
why) — the correct pattern is known but not systematized. There is no transactional outbox
for the bus.

### S3 — Money paths are check-then-act with no DB backstop
*(backend C1–C4, H-pay/H-bill)*

Every monetary invariant is enforced in Python between a read and a write, with no lock and
no database constraint. Two concurrent requests (a double-click) each pass the check and
both write: refund cap (`net_paid` goes negative), invoice over-payment (`INVOICE_PAID`
fires twice), allocation sum, partial-invoicing quantity. Worst: the invoice-number unique
index the model comment claims "replaced" the constraint **does not exist in any
migration** — duplicate fiscal invoice numbers are representable, a direct
Spanish-compliance break. The money *columns* are correctly `Numeric(12,2)` with
positive-amount checks; it's the *cross-row* invariants that have no teeth.

### S4 — The CI gates CLAUDE.md advertises don't do what it says
*(module-isolation #3 · backend H-api3 · doc-drift #2 · event-bus #8)*

CLAUDE.md: "Cross-module FKs are allowed only when the target is in `depends`. CI rejects
migrations otherwise." **No such check exists** — and the isolation test explicitly skips
`migrations/` directories. That's how a cross-module FK
`appointment_treatments.planned_treatment_item_id → planned_treatment_items.id`
(agenda→treatment_plan, not in `depends`, and a dependency cycle) slipped in undetected.
Separately, the docs-coverage / catalog generator's `PUBLISH_RE` regex misses enum-based
and variable-dispatched publishes, so `docs/events-catalog.md` lists ~19 live events as
"publisher: none" — inviting an agent to delete load-bearing events — and the "feral event"
CI gate is blind to them.

### S5 — Silent failure is the default posture, front to back
*(frontend, pervasive · event-bus #15)*

The bus swallows every handler exception with only a log. The frontend `useApi`
global-toasts only 403/5xx/network — **400/404/409/422 rethrow silently**, so any handler
without try/catch fails with no user feedback. Read paths render false-empty on error (a
failed calendar fetch looks like a free week; a failed odontogram load renders a healthy
32-tooth mouth). Write paths lose data quietly (clinical-note text wiped on failed save;
treatment surface edits dropped with a success toast). Destructive actions with no undo
endpoint fire on one click with no confirm. Across the product, the failure mode is "looks
fine, did nothing" — the most dangerous mode in a clinical/fiscal tool.

---

## Findings by dimension

Critical and high in full; medium/low condensed (the passes already triaged them, and they
run to the hundreds).

### Backend core, money & compliance — 4 critical · ~15 high

- **CRITICAL ✓ verified — Invoice-number uniqueness does not exist; the model comment lies.**
  `billing/models.py:182` claims `ix_invoices_clinic_number_unique` "replaced" the
  constraint. Grep of every billing migration (`bil_0001…0004`) confirms no such index and
  no unique on `(series_id, sequential_number)`. Duplicate fiscal invoice numbers are
  representable — silent, and a Spanish-compliance break. One-line migration fixes it.
- **CRITICAL — Refund cap is a racy Python check with no DB constraint.**
  `payments/workflow.py:281`. Two concurrent `POST /{payment_id}/refunds` both read
  `already_refunded=0`, both pass `≤ payment.amount`, both insert → 200 € refunded on a
  100 € payment; `net_paid` negative; downstream reports corrupt. No `FOR UPDATE`, no
  `Σrefund ≤ payment` constraint.
- **CRITICAL — Invoice over-payment is check-then-act, unlocked.**
  `billing/router.py:857`. Concurrent `POST /invoices/{id}/payments` both see full
  `balance_due`, both insert; invoice collects 2× its total, `INVOICE_PAID` fires twice.
- **CRITICAL — Concurrent issue of one draft → burned number + ghost VeriFactu record.**
  `billing/router.py:575` + `workflow.py:110`. `status=="draft"` read without a row lock;
  both txns pass, series lock hands them N and N+1, both run the VeriFactu hook → one
  invoice, two chained AEAT records, a permanent sequential gap, an alta for a non-existent
  document.
- **HIGH — Patient medical history is hard-deleted.**
  `patients_clinical/service.py:105,142,181,220,248,276` all `db.delete(row)` — no
  `deleted_at`, no audit — despite the module's own "most sensitive surface" note. A deleted
  penicillin-allergy row is unrecoverable; `PUT /medical-history` wipes-and-reinserts all
  rows, destroying provenance. Medico-legal exposure.
- **HIGH — Login rate-limit bypassable via spoofed `X-Forwarded-For`.**
  `docker-compose.yml:50` (`--forwarded-allow-ips *`) + `auth/router.py:52` keys the 5/min
  limiter on `get_remote_address`. Rotating a fake XFF gives each request a fresh bucket.
  Pin to the proxy CIDR.
- **HIGH — `SEED_ON_STARTUP` mints a hardcoded-credential admin with no env guard.**
  `scripts/seed_demo.py:635` (`hash_password("demo1234")`, no `ENVIRONMENT` check), wired
  next to `ENVIRONMENT=production` in `docker-compose.coolify.yml:35`. Flip it to self-heal
  a wiped volume and you get a known-credential superuser on prod, silently.
- **HIGH — Any clinic admin can install/uninstall modules and SIGTERM the process.**
  `plugins/router.py:160–251` gate on per-clinic `admin.clinic.write`, but the ops are
  process-global; `/-/restart` kills PID 1.
- **HIGH — Credit-note math inverts absolute discounts.**
  `billing/workflow.py:565` negates `unit_price` but copies `discount_value` verbatim → a
  90 €-net line credits 110 € net, wrong `cuota_total` chained and submitted to AEAT. No
  cumulative credit-note cap either.
- **HIGH — VeriFactu: crash between AEAT submit and commit → duplicate submission.**
  `verifactu/services/submission_queue.py:129` marks `sending` + flush but never commits
  before the network call (commit only at :264). A crash after AEAT accepts rolls back to
  `pending`, resubmits, gets duplicate-rejections that then block all invoice issuing
  clinic-wide. Regenerating a non-head rejected record breaks the hash chain
  (`hook.py:446`).
- **HIGH — Notifications outbox: `SKIP LOCKED` neutralized → duplicate sends, lost
  messages.** `notifications/gateway.py:203–246`. Per-message commit inside the loop
  releases locks on the rest of the batch; a second replica re-sends. A vendor `failed`
  receipt doesn't touch `next_attempt_at`, so it's re-dispatched. A crash between the
  `sending` commit and the result strands the row forever — the promised reaper was never
  built.

<details><summary>Medium & low — rounding, crypto, config, conventions (~30)</summary>

- **Line-item rounding mismatch** `billing/service.py:568` — lines rounded on insert, totals
  summed from unrounded values; PDF lines don't sum to total, trips AEAT cross-validation.
  Uses HALF_EVEN, not Spanish-customary HALF_UP.
- **VAT rate stored as Float** `billing/models.py:237` (also catalog/budget) — float
  artifacts split desglose groups and print `varios` in gestoría export. Use `Numeric(5,2)`.
- **Earned-ledger idempotency broken for NULL session path** `payments/models.py:237` —
  `UNIQUE(treatment_id, source_session_id)` is NULLS DISTINCT, so `ON CONFLICT DO NOTHING`
  is a no-op; re-marking a treatment doubles patient debt.
- **Accounting export keys payments by `invoice.issue_date`** `accounting_export/service.py:93`
  — April cobros land in March, Feb-invoice cobros never appear, refunds not exported.
- **SECRET_KEY has no strength validator** `config.py:13` — SHA-256'd unsalted into the
  Fernet key for SMTP passwords AND VeriFactu certs; rotating it bricks stored creds and
  degrades SMTP to unauthenticated silently.
- **BUDGET_PUBLIC_SECRET_KEY falls back to SECRET_KEY** `budget/public_router.py:70` —
  collapses the two keys ADR-0006 promised to isolate; absent from Coolify compose.
- **Scheduler has no multi-worker guard** `scheduler.py:39` — in-process AsyncIOScheduler in
  every process, no leader election. `--workers N` sends digests N× and drains the verifactu
  queue concurrently.
- **Clinic-configured From address is dead code** `email/service.py:188` — always stamps
  `noreply@dentalpin.com` → SPF/DMARC misalignment.
- **`invoice.balance_due` removed but still read** `notifications/handlers.py:428` — every
  `invoice.sent` raises AttributeError, swallowed; "email invoice" silently sends nothing.
- **Two 422 response shapes** `main.py:149` — Pydantic `{detail}` vs hand-raised
  `{data,message,errors}`; frontend reading `message` renders undefined.
- **PII to stdout by default** `events/bus.py:56`, `email/providers/console.py:46` — full
  payloads and rendered emails at INFO; a deploy that forgets `EMAIL_PROVIDER=smtp` streams
  health data to stdout.
- DELETE returns 200+body in 4 routers (convention 204); HTTPException raised inside 22
  service methods (breaks agent-tool wrapping); `DENTALPIN_DEV_MODULE_SCAN` defaults True in
  prod; readiness 503 leaks `str(exc)`; CORS reflects arbitrary Origin if `ALLOWED_ORIGINS`
  has `*`.

</details>

### Frontend coherence & affordances — 8 critical · ~27 high

- **CRITICAL ✓ verified — Periodontogram "Descartar borrador" crashes, then blocks all new
  sessions.** `periodontogram/…/usePeriodontogramSession.ts:158` calls `api.delete(...)`;
  `useApi` exposes only `del`. TypeError before any request, no catch, modal already closed
  — then `uq_perio_snap_one_draft_per_patient` blocks starting any new perio session for
  that patient. One-word fix.
- **CRITICAL — Payments: Enter key creates duplicate payments.**
  `payments/…/PaymentCreateModal.vue:228` gates only on `canSubmit`, never `isSubmitting`;
  no in-flight guard. Enter twice on the "cobrar" flow = two `POST /payments`. No void; undo
  requires a refund.
- **CRITICAL — Public budget accept/reject fakes success.**
  `budget/…/p/budget/[token].vue:173` uses raw `$fetch` with `catch { return false }`; the
  page closes the modal unconditionally. Patient signs, request fails, patient believes they
  accepted — clinic never receives it.
- **CRITICAL — Odontogram: failed chart load renders a healthy 32-tooth mouth.**
  `odontogram/…/useOdontogramData.ts:83` sets error console-only; the chart defaults every
  tooth to `healthy` when `teeth=[]`.
- **CRITICAL — VeriFactu AEAT remediation fails silently (TypeError in the catch).**
  `verifactu/…/InvoiceVerifactuSlot.vue:74` — a local `errorMessage` computed shadows the
  imported helper; the catch calls the ComputedRef as a function.
- **CRITICAL — Dead pagination: Nuxt UI v2 API on a v4 component.**
  `media/…/DocumentGallery.vue:229` +2 use `v-model`+`page-count` (v4 needs
  `v-model:page`/`items-per-page`). Documents 13+ and pipeline rows 21+ unreachable.
- **CRITICAL — Migration import broken for real files (10 s client timeout).**
  `migration_import/…/DataMigrationPage.vue:129` uses `api.post` (10 s) for `.dpm` files the
  backend accepts up to 5 GB. Execute is also unconfirmed, uncaught, and double-fireable
  with no rollback in v1.
- **HIGH — `useApi` silently drops `params`; hard 10 s timeout; swallows 4xx.**
  `frontend/app/composables/useApi.ts:33,47,91`. The dropped `params` makes the VeriFactu
  queue tabs (Pendientes/Rechazadas/Fallos) all render the same unfiltered list. The 4xx
  swallow is the root of most silent-failure findings.
- **HIGH — `patients` layer (`depends: []`) bare-renders 6 other layers' components.**
  `patients/…/AdministrationTab.vue:227` +6 use budget/billing/media/odontogram/
  patients_clinical/patient_timeline components directly, gated by permission only — never
  module-active state, never via `ModuleSlot`. Uninstall would silently blank
  patient-detail sections.
- **HIGH — Cabinets settings fully ungated; the gating constant doesn't exist.**
  `agenda.cabinets.read/write` enforced backend-side but absent from `permissions.ts`; the
  page has zero `can()` checks. Receptionists see create/edit/delete buttons that 403.
- **HIGH — Actions that submit editable data the backend silently discards.**
  `billing/…/invoices/new.vue:152` sends editable `billing_name/tax_id/…` that `InvoiceCreate`
  lacks (snapshotted from patient). Same class: plan "Activar" always 400s, plans search is
  a no-op, pipeline contact buttons never render (`PatientBrief` lacks `phone`).

<details><summary>High & medium — data loss, unconfirmed destructive actions, i18n, drift (~50)</summary>

- **Silent data-loss on failed save:** clinical-note composer wipes typed text;
  treatment-edit modal drops surface edits (success toast, nothing saved); perio
  measurements lost inside a closed exam; timeline caps at 20; recalls call-list hard-capped
  at 50 no pager; odontogram chart truncated at 50.
- **Destructive, no confirm, no undo endpoint:** odontogram treatment delete (invoiced
  treatment deletes like a draft), appointment cancel from modal footer, clinical-note delete
  (3 of 5 surfaces one-click), recall cancel/mark-done, plan-item delete (cascades to
  odontogram + budget line).
- **Double-submit duplicate side effects:** notifications reply Enter path (duplicate
  WhatsApp), whatsapp "Send test" (Kapso bills per message), budget "Enviar" + accept
  signature.
- **Fiscal display drift:** credit-note creation client-side marks original `cancelled`,
  server creates a draft and doesn't cancel; `GET /invoices/{id}/payments` typed `Payment[]`
  but backend returns a leaner schema → date "-", method "undefined".
- **Dead affordance:** payment "detail" menu → `/payments/{id}`, a page in no layer → 404.
- **i18n regressions in EN UI:** invoice-series settings (30 keys) and legal-guardian form
  (24 keys) ES-only; 16+ wrong-prefix error keys render raw; media ships zero locale files.
- **Decimal money serializes as JSON strings** but host TS types declare `number` —
  consumers survive only via `Number()`; new arithmetic string-concatenates.
- **CI blind spot:** `ci.yml` regenerates modules.json with all 22 layers, so CI never tests
  the shipped 17-layer set; cross-module affordances gated by permission only, never
  module-active state.

</details>

### Event bus — 3 critical · 5 high

- **CRITICAL ✓ verified — Media patient-archive cascade has never worked.**
  `media/__init__.py:65` handler is `_on_patient_archived(self, db, data)`, but the bus
  (`events/bus.py:58`) calls `handler(data)` → `TypeError` every time. Even fixed, it reads
  `data["clinic_id"]` but the publisher (`patients/service.py:285`) sends `{"patient_id":…}`
  only. Archiving a patient never archives their documents (a GDPR cascade); the bus
  swallows the exception.
- **CRITICAL — Publish-before-commit breaks refunds, deadlocks recall auto-link.**
  The S2 root cause, concretely: `billing/events.py:25`, `recalls/events.py:39`.
- **CRITICAL — Uninstall never unsubscribes handlers or unmounts routers.**
  The S1 root cause on the bus: `loader.py:142` / `processor.py:259`. `service.py`'s comment
  claiming uninstalled handlers "do not fire" is false.
- **HIGH — Budget reminders marked "sent" but no message is sent.**
  `budget/workflow.py:585` publishes `budget.reminder_sent`; notifications never subscribes
  it, only `patient_timeline` does. Patient receives nothing, cooldown prevents retry, staff
  see a timeline that lies.
- **HIGH — Fire-and-forget `create_task`: droppable + commit race.**
  `notifications/handlers.py:28,190` — task refs discarded (GC mid-flight), and the task
  races the publisher's commit. Intermittently missing confirmation emails.
- **HIGH — Appointment clinical notes never reach the patient timeline.**
  `clinical_notes/service.py:263` publishes `appointment_{clinical,administrative}_created`
  with zero consumers; timeline subscribes only the four non-appointment note events.
- **HIGH — `events-catalog.md` misreports ~19 events' publishers.**
  The S4 regex bug: `generate_catalogs.py:86`'s `PUBLISH_RE` misses variable dispatch, the
  `OdontogramEventType` class, and gateway indirection.

<details><summary>Medium & low — dead enum members, dedup, naming, docs (~13)</summary>

- **`item_completed_without_note` fires on every completion** `treatment_plan/service.py:970`
  — the promised reconciliation doesn't exist; every completed item shows "completado sin
  nota" even when a note was captured.
- **`patient_timeline` has no dedup** despite its own CLAUDE.md requiring
  `(event_type, source_id)` dedup — any re-emission writes duplicate cards.
- **Outbound WhatsApp never reaches the timeline** — gateway dual-publishes legacy events
  only for `channel==email`.
- **11 dead enum members** never published (INVOICE_CREATED, PAYMENT_VOIDED contradicting
  the payments redesign, DOCUMENT_ARCHIVED vs the DOCUMENT_DELETED actually published, etc.).
- **Bus posture:** failures terminal and invisible, in-memory only, no retry/dead-letter —
  wrong for consistency-critical reactions. A handler that subscribes during publish mutates
  the list under iteration → latent RuntimeError.
- Naming incoherence: 2- vs 3-segment names; `patient.medical_updated` published by
  `patients_clinical` not `patients`.

</details>

### Module isolation & migrations — 2 critical · 6 high

- **CRITICAL ✓ verified — Boot-time `alembic upgrade heads` resurrects uninstalled modules.**
  `docker-entrypoint.sh:27` / `alembic_paths.py:20`. Uninstall verifactu → tables dropped →
  restart → `vfy_0006` looks unapplied → tables recreated while state stays `UNINSTALLED`.
- **CRITICAL — Loader mounts every module regardless of install state.**
  `loader.py:142` / `registry.py:37`. `is_loaded()` means "code on disk," so
  `migration_import` gating verifactu writes on `is_loaded("verifactu")` is always True.
- **HIGH — The advertised cross-module-FK CI gate does not exist.**
  Nothing inspects `ForeignKey` targets against `manifest.depends`, and
  `test_module_isolation.py:124` skips `migrations/`.
- **HIGH — Deliberate isolation evasion via raw SQL.**
  `payments/service.py:565` joins odontogram/catalog/users in raw `text()` (comment admits
  it launders undeclared deps); `patients/service.py:95` reads agenda's `appointments` — an
  inverted dependency.
- **HIGH — agenda ↔ treatment_plan: undeclared import + cross-module FK + cycle.**
  `agenda/service.py:22` imports a module not in `depends`; `agenda/models.py:166` FKs
  `planned_treatment_items.id`; treatment_plan depends on agenda → schema-level cycle.
- **HIGH — whatsapp_kapso registers its channel adapter at import time.**
  `whatsapp_kapso/__init__.py:30` — the adapter is live even though the module was never
  installed; uninstall cleanup undone on next boot.
- **HIGH — 11 modules violate "each module owns its Alembic branch."**
  patients…notifications thread through one linear chain (`branch_labels=None`).
  Mitigant: `uninstall` permanently blocks these, so the removable flag isn't a lie — but
  none can ever be made removable without a graph rewrite.

<details><summary>Medium & low — ordering hazards, cross-reaches, auto_install policy (~6)</summary>

- **`ag_0002` ALTERs a table its revision graph doesn't guarantee exists** — fresh-install
  success is an Alembic traversal accident.
- **agenda (core) imports removable schedules' internals** `agenda/kanban_service.py:90` —
  after uninstall hits dropped tables; `except: continue` risks InFailedSQLTransaction.
- **billing → reports runtime import, circular** `billing/router.py:1121`.
- **auto_install policy incoherence** — schedules/recalls/copilot ship `auto_install=True`
  against the stated opt-in policy; `manifest.py:41` defaults it True.
- **Validator checks isolation one direction only** — `tp_0002` creates clinical_notes'
  tables in treatment_plan's chain; flipping clinical_notes to removable would leave tables
  behind on uninstall.
- migration_import over-declares `depends` on schedules → spuriously blocks uninstalling it.

</details>

### Copilot / LLM trust boundary — 1 high · 8 medium

- **HIGH ✓ verified — Tool error strings bypass redaction → PII to the cloud LLM.**
  `registry.py:171` / `orchestrator.py:228` / `redaction.py:167`. The redactor tokenizes
  only known-key values; tool failures surface as `{"error": <string>}` and `"error"` isn't
  a known key — so a Pydantic validation error echoing a bad email/phone/name verbatim, or
  any handler exception carrying row data, is streamed to OpenAI in cleartext.
- **MEDIUM — `permissions=[]` is the default → an ungated tool is silently exposed.**
  `tool.py:46` / `registry.py:148`. Over an empty list, `all(...)` is True and the deny loop
  never runs. No registration-time validation requires a permission.
- **MEDIUM — The write-confirmation boundary rests on honest `category` labels.**
  `orchestrator.py:225`. READ auto-executes; only WRITE/DESTRUCTIVE suspend. `category` is
  self-declared with no cross-check against whether the handler mutates.

<details><summary>Medium & low — over-tokenization, digest leaks, money coercion (~9)</summary>

- **Generic `"name"` key over-tokenizes non-PII** `redaction.py:40` — catalog/cabinet names
  get tokenized, so the agent can't match "empaste" to a catalog item; catalog search via
  copilot is effectively broken under redaction (the default).
- **Uninstalled/disabled modules keep their tools callable** (S1 on the copilot surface).
- **Morning digest creates a new Agent + AgentSession every run** `tasks.py:96` — unbounded
  row growth per recipient per hour.
- **Digest has no "already sent today" guard** — a restart or multi-worker deploy re-sends.
- **Free-text tool arguments reach the cloud untokenized** — `note`/`reason_note`/
  `custom_message` aren't in the key set.
- `jsonify` coerces Decimal money to float; `send_notification` (WRITE) vs `send_budget`
  (DESTRUCTIVE) label the same irreversible send inconsistently; "38 tools" is really 36.

**Verified sound:** the registry chokepoint genuinely enforces RBAC/guardrails/audit in
order; `clinic_id` is always server-sourced, never from tool args; tool permission strings
match their HTTP routes; off-books axis separation is honored and tested; the confirmation
flow is owner-scoped.

</details>

### RBAC & permissions — 1 high · 3 medium

- **HIGH ✓ verified — `create_user` trusts a caller-supplied `clinic_id`.**
  `core/auth/router.py:395`. `require_permission("admin.users.write")` checks only the
  caller's role in their *own* clinic, but the body's `clinic_id` is written verbatim into a
  new `ClinicMembership` with an arbitrary role. An admin of Clinic A can POST
  `{clinic_id: <Clinic B>, role: "admin"}` and mint an admin membership in Clinic B.
  `update_user`/`delete_user` scope correctly — this one doesn't.
- **MEDIUM — Front-desk roles can author and delete clinical notes.**
  `clinical_notes/__init__.py` grants receptionist/hygienist/assistant `notes.write`, which
  gates create, edit, *and* delete (no separate delete permission).
- **MEDIUM — Role inversion in media.**
  `media/__init__.py` — receptionist/assistant get `media.*` (can delete clinical photos)
  while the hygienist is read-only and can't upload one.

<details><summary>Medium & low — cross-module coupling, dead permission, frontend drift (~5)</summary>

- **agenda endpoint gated by clinical_notes' permission** `agenda/router.py:512` — if
  clinical_notes is uninstalled, the endpoint silently becomes admin-only.
- **`admin.users.read` is dead** — declared in core perms and frontend, enforced by no
  endpoint; `GET /users` requires `write`.
- 26 hardcoded permission strings in the frontend bypass `PERMISSIONS`; several view-only
  settings pages gate on `admin.clinic.write` instead of `.read`.
- Most pages carry only `auth` middleware, no permission gate (UI-only, backend gated).

**Verified sound:** the wildcard matcher has no `startswith` bug (`billing.*` ≠
`billing_x.y`); module-name prefixing composes correctly; install/uninstall/user-management
are admin-only.

</details>

### Multi-tenancy isolation — 2 medium · fragile patterns

> No critical/high cross-tenant read or write is exploitable via HTTP or copilot tools —
> `get_clinic_context`, the tool-registry chokepoint, media path construction, and the
> WhatsApp webhook signature check all hold. The two mediums are real disclosures gated by
> knowing a non-enumerable target UUID.

- **MEDIUM ✓ verified — Recalls: create trusts `patient_id`; list/export join `Patient`
  unscoped.** `recalls/service.py:231` inserts with the caller's `clinic_id` but never checks
  the supplied `patient_id`; the list query (`service.py:131`) joins `Patient` with no clinic
  predicate. A clinic-A user creates a recall pointing at a known clinic-B patient UUID, then
  reads back that patient's name/phone in the list or CSV.
- **MEDIUM — Agenda: planned-item validation skipped when appointment has no `patient_id`.**
  `agenda/service.py:403,455`. Validation runs only `if planned_item_ids and
  data.get("patient_id")`, but `patient_id` is optional. Omitting the patient links a
  caller-supplied planned item via a naked `db.get()`; a later GET eager-loads clinic B's
  treatment-plan data.

<details><summary>Fragile-but-not-exploitable — defense-in-depth gaps (~5)</summary>

- **patients_clinical service layer has zero scoping** — getters like `get_allergy(db, id)`
  take no `clinic_id` despite docstrings claiming otherwise; isolation lives entirely in the
  router. The first direct caller (a copilot tool or event handler) would return clinic B's
  medical history. Worth hardening proactively.
- notifications/budget event handlers fetch by id trusting payload `clinic_id` with no
  filter — internal-only today.
- Several `create` paths store caller-supplied FKs without ownership validation — dangling
  reference smell, no disclosure.

</details>

### Documentation drift — several high · misleads agents

- **HIGH ✓ verified — CLAUDE.md's "source of truth" RBAC snippet is flat wrong.**
  `CLAUDE.md` shows the pre-modular `clinical.*` scheme; `permissions.py:44` reality is
  dentist = `agents.view/supervise`, hygienist/assistant/receptionist = empty core grants,
  everything else merged from module manifests via `get_role_permissions()`. The "Adding a
  permission" recipe points at the wrong place.
- **HIGH — 4 modules with events have no `events.md`; the CI check is regex-blind.**
  agenda, odontogram, patients_clinical, clinical_notes publish events but have no
  `events.md`. `check_docs_coverage.py:165`'s `PUBLISH_RE` has no capture group for
  `EventType.X`, so the "events.md required" rule never fires. Its promised shared helpers
  (`_docs_lib.py`, `docs_index.py`) don't exist.

<details><summary>Medium & low — stale commits, glossary, catalogs (~12)</summary>

- **18 files carry placeholder `last_verified_commit: 0000000`** (catalog, notifications, all
  6 verifactu screens, both locales) — defeats the staleness-badge mechanism.
- **Genuinely stale screens** where the .vue changed after the recorded SHA with
  behavior-changing commits: patients/detail, reports, payments dashboard.
- **events-catalog.md factually wrong** — odontogram/clinical_notes events show "declared but
  unused" while actively published (the S4 regex bug).
- **Glossary drift:** "Payment" still defined as "money against an invoice" contradicting the
  patient-centric model; zero entries for WhatsApp/copilot/nudges/notifications.
- **CHANGELOG gaps:** patient_timeline, patients_clinical, clinical_notes, migration_import
  have code after their last CHANGELOG touch.
- `docs/portal/` is allowlisted by CI but omitted from the CLAUDE.md + README folder tables.

</details>

---

## Top 8 fixes by leverage

1. **S1** — consume `ModuleRecord` state in the loader and entrypoint (filter branch heads
   and module mounting by install state; split `is_loaded()` into `is_discovered()` /
   `is_installed()`; add an uninstall→restart round-trip test).
2. **Invoice-number unique index** — one migration to create the index the model already
   claims exists.
3. **`FOR UPDATE` + DB CHECK caps** on refunds / invoice payments / allocations.
4. **`create_user`** — validate `clinic_id` against the caller's admin memberships.
5. **Media handler** — fix the signature and add `clinic_id` to the `patient.archived`
   payload.
6. **`useApi`** — forward `query`, make the timeout per-call overridable, document the
   toast-or-rethrow contract.
7. **Commit-before-publish** across the event bus (or a transactional outbox).
8. **Docs** — fix the CLAUDE.md RBAC snippet and the catalog-generator regex (so live events
   stop reading as unused).
