164 lines
12 KiB
Markdown
164 lines
12 KiB
Markdown
# Hardening issue ledger (verified findings)
|
|
|
|
Every item below was found by one audit agent and **re-confirmed by an independent adversarial
|
|
verifier** reading the code (and, for H-01, probing the running dev server). Evidence is file:line at
|
|
the time of the audit (2026-07-16) — re-locate if lines drifted, but do not re-litigate the finding.
|
|
|
|
**This file is the loop's progress state.** When a phase fixes an item, tick it `[x]` and append
|
|
`— fixed in <commit>` on the same line. `LOOP-PROMPT.md` reads this file to decide what's left.
|
|
|
|
---
|
|
|
|
## Phase 0 — auth gate & session liveness
|
|
|
|
- [ ] **H-01 (blocker) — the auth gate never executes at runtime.**
|
|
`client/middleware.ts` is correct as written (PUBLIC_PATHS = `['/login']`, redirect on dead token)
|
|
but never runs: live cookie-less `GET /fa/admin` → 200 full HTML, bare `GET /` → 404 (no next-intl
|
|
locale redirect either) — reproduced under Turbopack **and** `--webpack`. Cause: Next.js infers the
|
|
workspace root from a stray `C:\Users\Lenovo\pnpm-lock.yaml` (startup warning names it), so the
|
|
middleware file is never bound. Also: Next 16 deprecates `middleware.ts` for `proxy.ts`, and the
|
|
docs place the file "at the same level as `pages` or `app`" — for this repo that's `client/src/`.
|
|
Fix: pin `turbopack.root` (+ `outputFileTracingRoot`) in `client/next.config.mjs`; migrate the file
|
|
to `src/proxy.ts`; **prove the gate runs with a runtime probe** (curl DoD).
|
|
|
|
- [ ] **H-02 (blocker) — `isTokenAlive` can never read the real token (JWE).**
|
|
Server: `server/src/Infrastructure/Baya.Infrastructure.Identity/Jwt/JwtService.cs:116-128` —
|
|
`EncryptingCredentials(Aes128KW, Aes128CbcHmacSha256)` ⇒ the access token is a 5-segment JWE.
|
|
Client: `client/src/lib/auth/token.ts:24-38` JSON-parses segment 1 ⇒ always `null` ⇒
|
|
`isTokenAlive` always `false` for real tokens. Consequences: (a) once H-01 is fixed the middleware
|
|
would redirect-loop **logged-in** users; (b) `getServerAuthState` (`client/src/lib/auth/server.ts`)
|
|
seeds `isAuthenticated=false` on every hard reload. The wire already returns
|
|
`accessExpiresAt`/`refreshExpiresAt` (`client/src/services/auth/types.ts:62-66`): persist a
|
|
readable companion expiry cookie in `persistAuthTokens` and check that instead of decoding.
|
|
|
|
- [ ] **H-03 (high) — anonymous visitor to a private shell = infinite splash, never a login ask.**
|
|
`useMe()` is enabled only when `isAuthenticated`; for a cookie-less session that's permanently
|
|
`false`, so `useRoleHydration()` never leaves `loading` and `RoleGuard`
|
|
(`client/src/components/auth/RoleGuard.tsx:55`) renders `AuthSplash` forever (splash markup
|
|
confirmed in served HTML). Add an explicit `unauthenticated` state → redirect to `/login?next=…`
|
|
with return-URL honored post-login.
|
|
|
|
## Phase 1 — admin RBAC & staff role scopes (server)
|
|
|
|
- [ ] **H-04 (blocker) — DynamicPermission RBAC is structurally dead; the whole admin API 403s.**
|
|
`DynamicPermissionService.cs:9` passes only the literal role `"admin"`; the seeded personas hold
|
|
`super_admin`/`finance` (`DemoWorldDefinitions.cs:144-145`, distinct constants `RoleNames.cs:13,17`).
|
|
No RoleClaim is ever seeded, and the only claim writer
|
|
(`RoleManagerService.ChangeRolePermissionsAsync`, `RoleManagerService.cs:145-208`) has **no HTTP
|
|
surface** (the `Features/{Role,Admin}` handlers exist but no controller exposes them). Every
|
|
`[Authorize(ConstantPolicies.DynamicPermission)]` endpoint (all Admin* controllers, PlatformConfig,
|
|
Holidays, Audit, SupportAlerts, Reviews-moderate) 403s for both seeded admins. RUNBOOK.md:115-130 and
|
|
refinement-phase-2-report falsely document these personas as working — fix the docs too.
|
|
|
|
- [ ] **H-05 (high) — `BookingRoles.Admin` over-grants: Support/Moderation get clinical + financial access.**
|
|
`Features/Bookings/BookingRoles.cs:8-9` bundles Admin/SuperAdmin/Support/Finance/Moderation; used by
|
|
`GetCareInstructionsQuery.Handler.cs:31,37` (encrypted clinical notes),
|
|
`TransitionBookingStatusCommand.Handler.cs:24-25,51-54` (force `Completed` ⇒ arms the payout
|
|
dispute-window), `GetNursePayableBalanceQuery.Handler.cs:22-24` (contradicts its own comment).
|
|
Controllers carry only blanket `[Authorize]` — the handler set IS the boundary. The codebase's own
|
|
convention is narrower (`PatientAccess.cs:17` = [Admin, SuperAdmin]; `StaffRoles.cs:5-13`
|
|
"narrowest fitting scope"). Split into `ClinicalAccess` / `Financial` sets and update the handlers.
|
|
|
|
## Phase 2 — money-path mock integrity (client)
|
|
|
|
- [ ] **H-06 (blocker) — refunds mock reads the retired bookings-mock store → real cancel 404s.**
|
|
`services/refunds/apis/mockApi.ts:3,88` imports `mockGetBookingForRefund` from
|
|
`services/bookings/apis/mockApi.ts` whose fixture array (ids ~5001-5005, `:340-345` throws 404
|
|
otherwise) is orphaned since `USE_BOOKINGS_MOCK=false`. Cancel CTA renders for every real booking
|
|
(`bookings/[id]/page.tsx:79-104`) then the preview errors (`bookings/[id]/cancel/page.tsx:50-78`).
|
|
|
|
- [ ] **H-07 (blocker) — BNPL wizard runs on a disconnected mock store keyed by the real request id.**
|
|
`checkout/page.tsx:213-224` pushes the **real** `requestId` (payment+bookingRequests are real) into
|
|
the BNPL branch; `services/bnpl/apis/mockApi.ts:5-8` hard-imports `bookingRequestsMockApi` +
|
|
`mockInsertConvertedBooking` (raw mock modules, bypassing both domains' seams); store holds only
|
|
fixture ids 1-2 → 404, or on id collision `settle()` (`:260-317`) fabricates a client-side "booking"
|
|
while the real request expires unpaid. `BNPL_ENABLED=true` (`services/payment/constants.ts:23`) has
|
|
no gating. Stopgap: hide/derive the CTA while `USE_BNPL_MOCK && !USE_BOOKING_REQUESTS_MOCK`; any
|
|
surviving mock must resolve requests through the real `bookingRequestsApi` selector.
|
|
|
|
- [ ] **H-08 (blocker) — verification is 100% mocked while catalog/search are real → invisible nurses.**
|
|
`services/verification/constants.ts:8-9` `USE_VERIFICATION_MOCK=true` gates the WHOLE
|
|
`VerificationApi` (nurse flow + TrustBadge + admin queue, one interface — `types.ts:255-265`).
|
|
The mock's `__mockApproveAll` never flips the real `nurse_profiles.is_verified`
|
|
(`NurseProfile.cs:44-53`, private setter, only the real b6 `VerificationAggregator` calls it), but
|
|
variant activation is real and unconditional (`SetVariantActiveCommand.Handler.cs:15-36`), and the
|
|
search gate requires `IsVerified` (`SearchIndexMaintainer.cs:34,65,94,152`). `PublishGate.tsx:59-68`
|
|
CTA makes no API call at all. Fix: split the seam (nurse half real now — clientApi is 1:1 per the
|
|
registry; admin half stays mocked until REQ-034) and wire/gate the publish UI honestly.
|
|
|
|
- [ ] **H-09 (high) — nurse earnings fabricated although its endpoints are live.**
|
|
`services/payouts/constants.ts:13` `USE_PAYOUTS_MOCK=true`, but REQ-025 was **delivered** —
|
|
`NursePayoutsController.cs:33-48` serves earnings_balance/earnings/{id}, and
|
|
`payouts/apis/clientApi.ts:163-187` already implements them. The flag is held hostage by the admin
|
|
batch methods (REQ-036, still open). Split the seam per-method (nurse reads real, admin mock).
|
|
|
|
- [ ] **H-10 (medium) — payment outcome discards the delivered `bookingId`.**
|
|
`services/payment/apis/clientApi.ts:55-75` types the response as `Omit<…,'bookingId'>` and sets
|
|
`bookingId: null` (line 72) although `BookingRequestDto.cs:46-49` carries it (REQ-017 delivered).
|
|
Kills the confirmation deep links (`checkout/return/page.tsx:84`,
|
|
`confirmation/page.tsx:94-110`) and the targeted cache invalidation (`invalidations.ts:17-24`).
|
|
Read it off the wire; drop the stale REQ-017-pending comments.
|
|
|
|
## Phase 3 — session & error-surface hardening (client)
|
|
|
|
- [ ] **H-11 (high) — logout/login never clear the query cache → cross-account data leak.**
|
|
`useLogout.ts:22-30` removes only `authKeys.all` (`['auth']`); `useVerifyOtp.ts:25-32` /
|
|
`useSelectRole.ts:19-38` touch only auth keys; `queryClient.ts:17-27` is a tab-lifetime singleton;
|
|
domain keys carry no user id. User B logging in after User A on the same device is served A's cached
|
|
patients/bookings/addresses/tickets until staleTime lapses. Fix: `queryClient.clear()` on logout +
|
|
reset on login.
|
|
|
|
- [ ] **H-12 (high) — raw stack-trace error boundary; no route-level error pages.**
|
|
`components/common/ErrorBoundary.tsx:35-58` renders hardcoded-English `error.toString()` + full
|
|
component stack, no retry, TODO-only reporting; used at `CustomerLayout.tsx:78` and
|
|
`TopBarAndSideBarLayout.tsx:104` — and sits BELOW RoleGuard/providers, while **no `error.tsx` or
|
|
`global-error.tsx` exists anywhere** under `src/app`. Replace with a branded i18n recovery card
|
|
(mirror `AuthAccountError`), dev-only stack, and add `[locale]` route-level error files.
|
|
|
|
- [ ] **H-13 (high) — 401/403/5xx/network toasts are hardcoded English.**
|
|
`lib/api/client.ts:57,80,86,91` — four `dispatchToast('…English…')` literals pass through
|
|
`dispatchToast.ts`/`ToastBridge.tsx` verbatim; no keys exist in either messages file. Violates the
|
|
project's golden rule #3 on the fa-default app. Localize via a locale-keyed dictionary (non-React
|
|
call site — the fetch layer already computes the locale).
|
|
|
|
- [ ] **H-14 (medium) — admin read-only consoles unguarded client-side.**
|
|
`admin/audit/page.tsx:25`, `admin/verification/page.tsx:50`, `admin/tickets/page.tsx:43`,
|
|
`admin/roles/page.tsx:50,121` fetch/render with no `useAdminCapabilities()` check — only the
|
|
sidebar (`AdminLayout.tsx:24-33`) hides them. (Server enforcement exists but is the dead
|
|
DynamicPermission policy — H-04.) Add a `CapabilityGuard` early-return with the non-leaking
|
|
access-denied pattern.
|
|
|
|
## Phase 4 — contract completion batch (server; Tier B/C REQs)
|
|
|
|
- [ ] **H-15 (medium) — the deferred REQ set, per the ledger** (`for-backend.md` — re-check each
|
|
status before building; REQ-031's HTTP surface may already exist after Phase 1):
|
|
REQ-019/020/021 customer refunds (cancel command, policy preview + canonical
|
|
`cancellation_policy_code` set, refund-by-booking + decomposition) · REQ-022/023/024 BNPL
|
|
(options/schedule/wallet-installments, eligibility KYC fields) · REQ-026 reviews
|
|
(eligibility/my-review reads — verify what phase 3/4 already delivered) · REQ-027 leftovers
|
|
(`record_access` check; the GET/PUT care_record endpoints already exist —
|
|
`PatientCareRecordsController.cs:44-54`) · REQ-029/030 admin config `updatedAt`/`updatedBy` + audit
|
|
filters · REQ-034 admin verification queue/signed-doc-URL/approve · REQ-035 admin refund
|
|
preview/initiate/approve/reject · REQ-036 admin payout preview/run/retry/record-reference.
|
|
|
|
- [ ] **H-16a (medium, server half) — partner portal has no identity signal or self-scoped reads.**
|
|
REQ-038: `administersPartnerCenterId` on `MeResult` + a seeded demo partner admin; REQ-032/033:
|
|
`centers/me[/nurses|/bookings|/settlement]` split reads + invoice `totalIrr`. `Me` interface
|
|
(`services/auth/types.ts:79-91`) has no partner field; `resolveRoleDestination`
|
|
(`services/auth/routing.ts:32-47`) has no partner branch.
|
|
|
|
## Phase 5 — final de-mock & partner reachability (client)
|
|
|
|
- [ ] **H-16b (medium, client half) — partner routing + portal gate + flip `USE_PARTNER_MOCK`.**
|
|
Add the partner branch to `resolveRoleDestination` off the new `/me` signal; gate the portal at the
|
|
layout level (today `partner/layout.tsx:15` RoleGuard has no `expected`, the mock resolves a
|
|
hardcoded center for ANY caller — `partnerCenter/apis/mockApi.ts:237-243` — and `nurses`/`bookings`
|
|
pages don't even await `useMyPartnerCenter`).
|
|
|
|
- [ ] **H-17 (medium) — patientRecords id-type mismatch; flip the remaining flags.**
|
|
Client `Medication/RoutineItem/CareTask.id: string` (`patientRecords/types.ts:36,45,53`, mock seeds
|
|
'm1'/'r1'/'t1') vs wire `long Id` (`CarePlanDtos.cs:11-15`) — a naive flip 400s every family-record
|
|
save; new items should omit id (server assigns). Then flip every remaining `USE_*_MOCK` (refunds,
|
|
bnpl, payouts-admin, admin, partnerCenter, verification-admin, patientRecords) as its REQs land;
|
|
delete the orphaned cross-mock helpers; update `mocks-registry.md` + `client/CLAUDE.md`.
|