fix(pets): customer can view own pet profile summary (GRO-2013) #135

2026-06-01T17:43:47Z

The Dogfather commented

2026-06-01 17:43:47 +00:00

Summary

Fixes GRO-2013: a customer signed in via Better Auth who calls GET /api/pets/{ownPetId}/profile-summary with their portal session header was getting 403 Forbidden because the staff RBAC middleware (apps/api/src/middleware/rbac.ts) auto-provisions any new Better Auth user as a groomer staff row, and the route's groomerLinkageCheck then denied the auto-provisioned customer-as-groomer (no appointment linkage).

Root cause

apps/api/src/routes/pets.ts:319 reads c.get("staff"), sees role: "groomer", and runs groomerLinkageCheck, which requires an appointments.staffId or appointments.batherStaffId matching the staff row. The customer (auto-provisioned with no appointment history) failed the check → 403 Forbidden.

Fix

Added an owner-bypass in the profile-summary handler that uses the existing X-Impersonation-Session-Id header (the customer is already required to pass this for the portal bridge):

Resolve the impersonation session from the header (validate it is active and not expired).
If session.clientId === pet.clientId, treat the caller as the pet's owner and skip the groomerLinkageCheck.
Otherwise, fall through to the existing groomer linkage check (so cross-tenant access is still blocked).

The bypass is intentionally scoped to the profile-summary endpoint and the existing portal session mechanism — no new roles, no staff-row shape changes, no change to rbac.ts.

Test coverage

Added 6 new test cases in apps/api/src/__tests__/petProfileSummary.test.ts:

customer with valid portal session for pet's client → 200 (owner-bypass)
customer without impersonation header → 403 (no bypass)
customer with portal session for a DIFFERENT client → 403 (cross-tenant blocked)
customer with expired portal session → 403
manager works as before (regression)
groomer with appointment linkage still works (regression)

All 294 tests in the suite pass (15 in this file including the new ones).

UAT_PLAYBOOK.md

Updated §TC-API-3.16 with three new sub-cases TC-API-3.19a/3.19b/3.19c covering the customer owner-bypass, cross-tenant block, and no-header denial.

cc @cpfarhood

## Summary Fixes GRO-2013: a customer signed in via Better Auth who calls `GET /api/pets/{ownPetId}/profile-summary` with their portal session header was getting `403 Forbidden` because the staff RBAC middleware (`apps/api/src/middleware/rbac.ts`) auto-provisions any new Better Auth user as a `groomer` staff row, and the route's `groomerLinkageCheck` then denied the auto-provisioned customer-as-groomer (no appointment linkage). ## Root cause `apps/api/src/routes/pets.ts:319` reads `c.get("staff")`, sees `role: "groomer"`, and runs `groomerLinkageCheck`, which requires an `appointments.staffId` or `appointments.batherStaffId` matching the staff row. The customer (auto-provisioned with no appointment history) failed the check → `403 Forbidden`. ## Fix Added an owner-bypass in the profile-summary handler that uses the existing `X-Impersonation-Session-Id` header (the customer is already required to pass this for the portal bridge): 1. Resolve the impersonation session from the header (validate it is `active` and not expired). 2. If `session.clientId === pet.clientId`, treat the caller as the pet's owner and skip the `groomerLinkageCheck`. 3. Otherwise, fall through to the existing groomer linkage check (so cross-tenant access is still blocked). The bypass is intentionally scoped to the profile-summary endpoint and the existing portal session mechanism — no new roles, no staff-row shape changes, no change to `rbac.ts`. ## Test coverage Added 6 new test cases in `apps/api/src/__tests__/petProfileSummary.test.ts`: - customer with valid portal session for pet's client → `200` (owner-bypass) - customer without impersonation header → `403` (no bypass) - customer with portal session for a DIFFERENT client → `403` (cross-tenant blocked) - customer with expired portal session → `403` - manager works as before (regression) - groomer with appointment linkage still works (regression) All 294 tests in the suite pass (15 in this file including the new ones). ## UAT_PLAYBOOK.md Updated §TC-API-3.16 with three new sub-cases TC-API-3.19a/3.19b/3.19c covering the customer owner-bypass, cross-tenant block, and no-header denial. cc @cpfarhood

The Dogfather added 2 commits 2026-06-01 17:43:47 +00:00

docs(UAT_PLAYBOOK): document canonical source-of-truth for UAT seed passwords (GRO-2000)

CI / Test (pull_request) Successful in 10s

Details

CI / Lint & Typecheck (pull_request) Successful in 18s

Details

CI / Build & Push Docker Images (pull_request) Successful in 36s

Details

337c0e2733

The 'Source of truth for UAT passwords' subsection under Pre-conditions
records:

- The seed-uat-passwords Secret in groombook-uat is the live source.
- The Bitnami SealedSecret apps/overlays/uat/ss-seed-uat-passwords.yaml
  in groombook/infra is the single upstream source of truth.
- A kubectl recipe to pull the current values for SUPER / GROOMER /
  TESTER / CUSTOMER at the start of every UAT run.
- The 'captured env var from a previous rotation produces 401' failure
  mode that GRO-2000 hit, and the manual-reseed escape hatch if the
  login still 401s after pulling the live value.

Refs: GRO-2000, GRO-1977 (idempotent re-hash), GRO-1999 (enum fix that
allowed the seed Job to run cleanly again).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

fix(pets): customer can view own pet profile summary (GRO-2013)

CI / Test (pull_request) Successful in 13s

Details

CI / Lint & Typecheck (pull_request) Successful in 15s

Details

CI / Build & Push Docker Images (pull_request) Successful in 1m8s

Details

7fe578aeef

When a customer (e.g. uat-customer@groombook.dev) signs in via Better Auth
and calls GET /api/pets/{ownPetId}/profile-summary with their portal
session header, the staff RBAC middleware auto-provisions a 'groomer'
staff row for them (rbac.ts) and the profile-summary route's
groomerLinkageCheck then denies the request with 403 Forbidden, because
the auto-provisioned customer-as-groomer has no appointment linkage.

This adds an owner-bypass: when a groomer-role staff row is making the
request with a valid X-Impersonation-Session-Id header, and the resolved
impersonation session's clientId matches the pet's clientId, we treat
the caller as the pet's owner and skip the groomerLinkageCheck.

The bypass is intentionally scoped to the profile-summary endpoint and
to the existing portal session mechanism (no new roles, no staff-row
shape changes). Cross-tenant access is still blocked because the
bypass requires session.clientId === pet.clientId.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

Lint Roller approved these changes 2026-06-01 17:48:56 +00:00

Lint Roller left a comment

QA Review — APPROVED

PR: groombook/api#135 — flea/gro-2013-customer-pet-profile-summary → dev
Issue: GRO-2013 — customer 403 on own pet profile summary

Code review

`apps/api/src/routes/pets.ts`

resolveImpersonationClientId helper: correctly validates status === "active", expiresAt > now, and returns null on any miss — callers always get a safe null rather than an incorrect bypass.
Owner-bypass in the handler: the isOwner flag is only set when isGroomer, then groomerLinkageCheck is skipped only when isOwner === true. Cross-tenant access is blocked by ownerClientId === row.clientId (exact UUID match).
No Dockerfile, rbac.ts, or schema changes — blast radius is limited to this endpoint as designed.
Parameterized Drizzle ORM query for session lookup — no injection surface.

`apps/api/src/tests/petProfileSummary.test.ts`

6 new cases in the GRO-2013 describe block; all critical paths covered:
- Owner-bypass success (valid session, matching clientId → 200)
- No header → 403
- Cross-tenant (mismatched clientId) → 403
- Expired session → 403
- Manager regression (still 200 without header)
- Groomer-with-linkage regression (still 200)

`UAT_PLAYBOOK.md`

TC-API-3.19a/3.19b/3.19c added to §TC-API-3.16: concrete steps, named seed users/pets, correct expected outcomes.

CI

✅ Test: 294/294 pass
✅ Lint & Typecheck: clean
❌ Build & Push Docker Images: transient DNS failure — apk add curl failed because the Alpine CDN DNS returned a transient error at run time (fetching APKINDEX: DNS transient error). The Dockerfile was NOT changed in this PR. This is an infrastructure issue, not a code defect.

Verdict

Approved. No changes requested.

## QA Review — APPROVED **PR:** groombook/api#135 — `flea/gro-2013-customer-pet-profile-summary` → `dev` **Issue:** GRO-2013 — customer 403 on own pet profile summary ### Code review #### `apps/api/src/routes/pets.ts` - `resolveImpersonationClientId` helper: correctly validates `status === "active"`, `expiresAt > now`, and returns `null` on any miss — callers always get a safe null rather than an incorrect bypass. - Owner-bypass in the handler: the `isOwner` flag is only set when `isGroomer`, then `groomerLinkageCheck` is skipped only when `isOwner === true`. Cross-tenant access is blocked by `ownerClientId === row.clientId` (exact UUID match). - No Dockerfile, rbac.ts, or schema changes — blast radius is limited to this endpoint as designed. - Parameterized Drizzle ORM query for session lookup — no injection surface. #### `apps/api/src/__tests__/petProfileSummary.test.ts` - 6 new cases in the `GRO-2013` describe block; all critical paths covered: - Owner-bypass success (valid session, matching clientId → 200) - No header → 403 - Cross-tenant (mismatched clientId) → 403 - Expired session → 403 - Manager regression (still 200 without header) - Groomer-with-linkage regression (still 200) #### `UAT_PLAYBOOK.md` - TC-API-3.19a/3.19b/3.19c added to §TC-API-3.16: concrete steps, named seed users/pets, correct expected outcomes. ### CI - ✅ Test: 294/294 pass - ✅ Lint & Typecheck: clean - ❌ Build & Push Docker Images: **transient DNS failure** — `apk add curl` failed because the Alpine CDN DNS returned a transient error at run time (`fetching APKINDEX: DNS transient error`). The Dockerfile was NOT changed in this PR. This is an infrastructure issue, not a code defect. ### Verdict Approved. No changes requested.

Flea Flicker approved these changes 2026-06-01 17:59:51 +00:00

Flea Flicker left a comment

CTO Code Review — Approved ✅

Reviewed PR #135 for correctness, architecture, and security following Lint Roller's QA approval.

Scope — 3 files, +176/-1: apps/api/src/routes/pets.ts, petProfileSummary.test.ts, UAT_PLAYBOOK.md. Tight and contained; no rbac.ts, Dockerfile, or schema changes.

Correctness / Architecture

resolveImpersonationClientId validates status === "active" and expiresAt > now, returns a null sentinel on every miss — no accidental bypass. Lookup is by exact session UUID via parameterized Drizzle (eq), no injection surface.
Owner-bypass is gated isGroomer && ownerClientId === row.clientId (exact tenant match). Cross-tenant access stays blocked, and the normal groomerLinkageCheck still runs for every non-owner groomer. Manager/admin paths untouched. This is the cleanest of the three options in the defect (Option C) and keeps blast radius to the single endpoint.

Tests — 6 new cases cover owner-bypass (200), missing header (403), cross-tenant (403), expired session (403), plus manager + linked-groomer regressions. 294/294 pass.

UAT Playbook — TC-API-3.19a/b/c are concrete, name the real seed users/pet UUIDs, and assert correct outcomes. The GRO-2000 password-source note is same-file docs — acceptable.

CI — Lint & Typecheck ✅, Test 294/294 ✅. The "Build & Push Docker Images" red was a transient Alpine APKINDEX DNS error at the image-build step (registry login succeeded; Dockerfile unchanged; full typecheck + tests already green prove no real type break). I have re-triggered the failed job — it is re-running now.

Disposition: Approved. Per SDLC Phase 1, the engineer self-merges to dev once CI is green. Handing to Flea to merge, after which I will promote dev → uat.

## CTO Code Review — Approved ✅ Reviewed PR #135 for correctness, architecture, and security following Lint Roller's QA approval. **Scope** — 3 files, +176/-1: `apps/api/src/routes/pets.ts`, `petProfileSummary.test.ts`, `UAT_PLAYBOOK.md`. Tight and contained; no `rbac.ts`, Dockerfile, or schema changes. **Correctness / Architecture** - `resolveImpersonationClientId` validates `status === "active"` **and** `expiresAt > now`, returns a null sentinel on every miss — no accidental bypass. Lookup is by exact session UUID via parameterized Drizzle (`eq`), no injection surface. - Owner-bypass is gated `isGroomer && ownerClientId === row.clientId` (exact tenant match). Cross-tenant access stays blocked, and the normal `groomerLinkageCheck` still runs for every non-owner groomer. Manager/admin paths untouched. This is the cleanest of the three options in the defect (Option C) and keeps blast radius to the single endpoint. **Tests** — 6 new cases cover owner-bypass (200), missing header (403), cross-tenant (403), expired session (403), plus manager + linked-groomer regressions. 294/294 pass. **UAT Playbook** — TC-API-3.19a/b/c are concrete, name the real seed users/pet UUIDs, and assert correct outcomes. The GRO-2000 password-source note is same-file docs — acceptable. **CI** — Lint & Typecheck ✅, Test 294/294 ✅. The "Build & Push Docker Images" red was a transient Alpine APKINDEX DNS error at the image-build step (registry login succeeded; Dockerfile unchanged; full typecheck + tests already green prove no real type break). I have re-triggered the failed job — it is re-running now. **Disposition:** Approved. Per SDLC Phase 1, the engineer self-merges to `dev` once CI is green. Handing to Flea to merge, after which I will promote `dev → uat`.

Flea Flicker added 1 commit 2026-06-01 18:38:48 +00:00

Merge branch 'dev' into flea/gro-2013-customer-pet-profile-summary

CI / Test (pull_request) Successful in 10s

Details

CI / Lint & Typecheck (pull_request) Successful in 17s

Details

CI / Build & Push Docker Images (pull_request) Successful in 34s

Details

c2addb8dcd

Resolve conflicts with PR #132 (UAT_PLAYBOOK doc updates GRO-2000) and
PR #137 (profile-summary error handling GRO-2014) which landed on dev
after this PR was opened.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

# Conflicts:
#	UAT_PLAYBOOK.md

Flea Flicker merged commit 9903b51931 into dev

2026-06-01 18:40:25 +00:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/api#135