Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions apps/api/scripts/backfill_bug092_target_hospital.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
"""One-off backfill for BUG-092 (error.md).

`scripts/seed_demo.py::_make_persona` never set `PatientProfile.
target_hospital_id` before this fix, so any DB already seeded from a
pre-fix run has every patient stuck at `target_hospital_id=NULL`, which
the org-scope filter (`src/services/clinician.py`, ISS-022/PR #22) treats
as invisible to every clinician. `_main` in `seed_demo.py` is idempotent by
early-exiting once `CLINICIAN_EMAIL` exists, so simply re-running the seed
does NOT repair already-seeded rows — this script does that repair
in-place instead.

Scope: this is a local single-tenant demo/dev DB with exactly one
organization and one clinician account (per ADR-049 — org-scope policy
itself is unchanged). Every `patient_profiles` row with a NULL
`target_hospital_id` is assigned to that one clinician's `organization_id`
(there is no other org for them to plausibly belong to in this DB). This
script is not meant to run against a multi-tenant/production DB with more
than one organization — it refuses to guess in that case.

Run from apps/api:
uv run python -m scripts.backfill_bug092_target_hospital
"""

from __future__ import annotations

import asyncio

from sqlalchemy import select

from src.db import SessionLocal
from src.models.patient_profile import PatientProfile
from src.models.user import Organization, User


async def _main() -> None:
async with SessionLocal() as db:
org_count_row = await db.execute(select(Organization.id))
org_ids = [row[0] for row in org_count_row.all()]
if len(org_ids) != 1:
print(
f"refusing to backfill: expected exactly 1 organization in this "
f"DB, found {len(org_ids)} — this script only handles the "
f"single-tenant local demo DB shape"
)
return

clinician_row = await db.execute(
select(User).where(User.email == "clinician@neurosync.demo")
)
clinician = clinician_row.scalar_one_or_none()
if clinician is None:
print("no demo clinician found — nothing to backfill")
return
if clinician.organization_id is None:
print("demo clinician has no organization_id — nothing to backfill")
return
org_id = clinician.organization_id

rows = await db.execute(
select(PatientProfile, User)
.join(User, User.id == PatientProfile.user_id)
.where(
User.role == "patient",
PatientProfile.target_hospital_id.is_(None),
)
)
pairs = rows.all()
if not pairs:
print("no NULL target_hospital_id rows among patients — nothing to do")
return

for profile, _user in pairs:
profile.target_hospital_id = org_id

await db.commit()
print(f"backfilled {len(pairs)} patient_profiles.target_hospital_id -> {org_id}")


if __name__ == "__main__":
asyncio.run(_main())
10 changes: 8 additions & 2 deletions apps/api/scripts/seed_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ async def _make_clinician(db, settings: Settings, org_id: uuid.UUID) -> None:
)


async def _make_persona(db, settings: Settings, p: dict) -> None:
async def _make_persona(db, settings: Settings, p: dict, org_id: uuid.UUID) -> None:
user = User(
email=p["email"],
password_hash=hash_password(DEMO_PASSWORD, settings),
Expand All @@ -304,6 +304,12 @@ async def _make_persona(db, settings: Settings, p: dict) -> None:
aad=_profile_aad(user.id, "emergency_contact"),
settings=settings,
),
# BUG-092 fix: org-scope filter (services/clinician.py, ISS-022/PR
# #22) requires target_hospital_id == actor.organization_id. This
# seed's demo patients belong to the demo clinician's org, so
# they must be assigned to it or the dashboard is structurally
# empty (NULL never matches a real UUID).
target_hospital_id=org_id,
)
)
consent = ConsentSnapshot(
Expand Down Expand Up @@ -398,7 +404,7 @@ async def _main() -> None:

await _make_clinician(db, settings, org.id)
for persona in PERSONAS:
await _make_persona(db, settings, persona)
await _make_persona(db, settings, persona, org.id)

await db.commit()
print(
Expand Down
22 changes: 19 additions & 3 deletions apps/api/src/schemas/clinician.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from typing import Any
from uuid import UUID

from pydantic import BaseModel, ConfigDict, EmailStr, Field
from pydantic import BaseModel, ConfigDict, Field


class RiskBadge(BaseModel):
Expand All @@ -24,7 +24,15 @@ class RiskBadge(BaseModel):

class PatientListItem(BaseModel):
user_id: UUID = Field(alias="userId")
email: EmailStr
# BUG-092 follow-on (discovered live, error.md): plain `str`, not
# `EmailStr` — these values echo an already-existing `User.email` row
# from the DB, not user input being validated. `EmailStr` rejected the
# demo dataset's `@demo` (no-TLD) addresses with a 500 the moment the
# org-scope filter (ISS-022) stopped masking it by returning empty
# first. Response-side re-validation of a value that already exists in
# the DB is not a security control here — it only breaks legitimately
# stored rows that don't fit RFC 5322 assumptions.
email: str
name: str
birth_year: int = Field(alias="birthYear")
is_minor: bool = Field(alias="isMinor")
Expand Down Expand Up @@ -60,7 +68,15 @@ class ConsentSnapshotOut(BaseModel):

class PatientDetail(BaseModel):
user_id: UUID = Field(alias="userId")
email: EmailStr
# BUG-092 follow-on (discovered live, error.md): plain `str`, not
# `EmailStr` — these values echo an already-existing `User.email` row
# from the DB, not user input being validated. `EmailStr` rejected the
# demo dataset's `@demo` (no-TLD) addresses with a 500 the moment the
# org-scope filter (ISS-022) stopped masking it by returning empty
# first. Response-side re-validation of a value that already exists in
# the DB is not a security control here — it only breaks legitimately
# stored rows that don't fit RFC 5322 assumptions.
email: str
name: str
birth_year: int = Field(alias="birthYear")
is_minor: bool = Field(alias="isMinor")
Expand Down
9 changes: 7 additions & 2 deletions apps/api/src/services/clinician.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
"""Clinician read service — decrypts PII and assembles dashboard payloads.

Phase 1a Demo simplification: every authenticated clinician sees every patient.
PRD §0.1 organization-scoped RLS lands in Phase 2.
Org-scoped per PRD §0.1 / ISS-022 (PR #22): a clinician / org_admin may read
only patients whose `PatientProfile.target_hospital_id` matches the actor's
`organization_id`; `super_admin` (service_role) sees all. See
`_can_access_patient` / `_patient_list_filter` for the exact rule. (BUG-092,
error.md: this docstring previously claimed "every authenticated clinician
sees every patient" — stale since PR #22 landed the org-scope filter; fixed
here to match actual behavior.)
"""

from __future__ import annotations
Expand Down
151 changes: 151 additions & 0 deletions apps/api/tests/repro/test_bug_092_org_scope_seed_mismatch.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
"""Regression test for BUG-092 (open) — live-discovered during
PLAN-2026-W31-WEBDASH's real-stack apps/web <-> apps/api <-> DB
verification.

Live repro (2026-07-27, qa): logged in as the documented demo clinician
(`clinician@neurosync.demo`, `docs/PROGRESS.md:126-138`) through the real
`apps/web` HTTP path (login -> cookie -> server-component fetch ->
apps/api), and separately hit `GET /api/v1/clinician/patients` directly
against apps/api with the same bearer token. Both returned an EMPTY list
(`{"success":true,"data":[]}`) even though the demo DB had 5 real seeded
patients. `GET /api/v1/clinician/patients/{id}` and
`GET /api/v1/clinician/sessions/{id}` for real, existing IDs both 404'd.

Root cause: `scripts/seed_demo.py::_make_persona` (PR #13, `d04c808`) never
sets `PatientProfile.target_hospital_id` — every seeded patient has it
`NULL`. `src/services/clinician.py`'s org-scope filter (PR #22, `c718809`,
ISS-022) requires `PatientProfile.target_hospital_id == actor.
organization_id`; the demo clinician DOES have a real `organization_id`.
`NULL == <uuid>` is never true in SQL, so every demo patient is
structurally excluded — contradicting the module's own docstring ("every
authenticated clinician sees every patient").

This test reproduces the live shape (clinician with a real
`organization_id`, patient profile whose `target_hospital_id` mirrors
whatever `seed_demo.py` currently sets it to) against a throwaway schema
(no live/demo DB touched) and asserts the CORRECT, expected behavior —
patients in the clinician's care ARE visible. It was RED (documented the
bug) against pre-fix `target_hospital_id=None`; ADR-049 chose backfill
(seed sets `target_hospital_id=<clinician's organization_id>`, org-scope
policy unchanged) over relaxing the "no-org == visible" policy, so the
fixture below now mirrors that post-fix seed shape and the test is GREEN.
"""

from __future__ import annotations

import uuid
from datetime import UTC, datetime, timedelta

import pytest
import pytest_asyncio
from sqlalchemy.ext.asyncio import AsyncSession

from src.models.patient_profile import PatientProfile

# conftest.py patches `PatientProfile.is_minor`'s column to
# `server_default=text("false")` (a TEST-ONLY convenience for callers that
# omit `is_minor`). SQLAlchemy's `Mapper._insert_cols_as_none` iterates
# every server_default and does a bare `not col.server_default` — a
# `TextClause` has no `__bool__`, so ANY insert of `PatientProfile` (this
# is the first test module in the suite to construct one via the ORM
# directly) raises `TypeError: Boolean value of this clause is not
# defined`. Not part of BUG-092 — a separate, pre-existing conftest gap
# with zero prior coverage — worked around locally here since we always
# pass `is_minor` explicitly and don't need the server_default at all.
from src.models.patient_profile import PatientProfile as _PatientProfile # noqa: E402
from src.models.session import Session
from src.models.user import Organization, User
from src.services.clinician import get_patient_detail, get_session_detail, list_patients

_PatientProfile.__table__.c.is_minor.server_default = None


@pytest_asyncio.fixture
async def clinician_and_patient(db_session: AsyncSession):
org = Organization(id=uuid.uuid4(), name="demo hospital", type="clinic")
db_session.add(org)
await db_session.flush()

clinician = User(
email="clinician@neurosync.demo.test",
password_hash="x",
role="clinician",
organization_id=org.id,
)
db_session.add(clinician)
await db_session.flush()

patient = User(email="patient@demo.test", password_hash="x", role="patient")
db_session.add(patient)
await db_session.flush()

profile = PatientProfile(
user_id=patient.id,
name_encrypted=b"not-really-encrypted-test-blob",
birth_year=1996,
is_minor=False,
gender="female",
# BUG-092 fix (ADR-049, option a): scripts/seed_demo.py now assigns
# the demo clinician's organization_id to every seeded patient's
# target_hospital_id, so this fixture mirrors seed_demo.py's
# post-fix, real, current behavior. Org-scope policy itself
# (services/clinician.py) is unchanged — this data now satisfies
# it instead of violating it.
target_hospital_id=org.id,
)
db_session.add(profile)
await db_session.flush()

session = Session(
patient_id=patient.id,
status="report_ready",
submitted_at=datetime.now(UTC) - timedelta(hours=1),
)
db_session.add(session)
await db_session.flush()

return clinician, patient, session


@pytest.mark.asyncio
async def test_bug092_demo_shaped_patient_is_visible_in_list(
db_session: AsyncSession, clinician_and_patient
):
clinician, patient, _session = clinician_and_patient

items = await list_patients(db_session, actor=clinician, limit=50)

assert any(i.user_id == patient.id for i in items), (
"BUG-092: a patient seeded exactly like scripts/seed_demo.py "
"(target_hospital_id=None) must be visible to the demo clinician "
"(organization_id set) — got an empty/missing list because the "
"org-scope filter (PR #22) excludes NULL-hospital patients"
)


@pytest.mark.asyncio
async def test_bug092_demo_shaped_patient_detail_is_reachable(
db_session: AsyncSession, clinician_and_patient
):
clinician, patient, _session = clinician_and_patient

detail = await get_patient_detail(db_session, actor=clinician, patient_id=patient.id)

assert detail is not None, (
"BUG-092: get_patient_detail 404s (returns None) for a real, "
"existing patient seeded exactly like the live demo data"
)


@pytest.mark.asyncio
async def test_bug092_demo_shaped_session_detail_is_reachable(
db_session: AsyncSession, clinician_and_patient
):
clinician, _patient, session = clinician_and_patient

detail = await get_session_detail(db_session, actor=clinician, session_id=session.id)

assert detail is not None, (
"BUG-092: get_session_detail 404s (returns None) for a real, "
"existing session seeded exactly like the live demo data"
)
Loading