Compare commits

..

19 Commits

Author SHA1 Message Date
Flea Flicker f67b96ddfe Merge pull request 'fix(GRO-2123): serialize seed.ts with Postgres advisory lock' (#155) from flea-flicker/gro-2123-seed-advisory-lock into dev
CI / Test (push) Successful in 11s
CI / Lint & Typecheck (push) Successful in 16s
CI / Build & Push Docker Images (push) Successful in 25s
CI / Test (pull_request) Successful in 10s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 28s
2026-06-04 11:23:41 +00:00
Flea Flicker d1a68d93de fix(GRO-2123): serialize seed.ts with Postgres advisory lock
CI / Test (pull_request) Successful in 13s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (pull_request) Successful in 58s
The reset-demo-data CronJob in groombook-uat intermittently failed with
FK 23503 on invoice_tip_splits because two pods could run the seed
concurrently: the new pod's TRUNCATE deleted rows the old pod was still
inserting.

Acquire a session-level advisory lock for the full duration of the seed.
CRITICAL: with postgres-js connection pooling, a pg_advisory_lock
acquired on one pooled connection and released on a different one is a
no-op (the lock is bound to the pg-backend that took it). We therefore
reserve a dedicated connection for the lock, take pg_advisory_lock(KEY)
on it, run the seed on the pooled connections, and release the lock +
reserved connection in a try/finally so a thrown seed error cannot leak
the lock or the connection.

Defence-in-depth with the infra PR that switches
concurrencyPolicy: Replace → Forbid on the reset-demo-data CronJob.

- Adds withSeedAdvisoryLock helper and runSeedBody extracted function
- Wraps seed() body in the helper; client.end() runs after the lock
  releases so a reserved connection is not returned to a closed pool
- SEED_ADVISORY_LOCK_KEY = 0x47524f4f ("GROO" in ASCII) — arbitrary
  stable 32-bit key, referenced in runbooks
- UAT_PLAYBOOK.md §3.29 documents the regression check

cc @cpfarhood

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-04 11:12:17 +00:00
Flea Flicker e9f94a2bd7 fix(seed): GRO-2100 run uat-groomer linkage AFTER services seed (regression in #151) (#153)
CI / Test (push) Successful in 12s
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (pull_request) Successful in 29s
CI / Lint & Typecheck (push) Failing after 12m57s
CI / Build & Push Docker Images (push) Has been skipped
fix(seed): GRO-2100 run uat-groomer linkage after services seed (#153)

Co-authored-by: Flea Flicker <flea@groombook.dev>
Co-committed-by: Flea Flicker <flea@groombook.dev>
2026-06-02 20:11:45 +00:00
Flea Flicker de16c50040 fix(seed): GRO-2100 deterministic uat-groomer ↔ UAT Pup Alpha linkage (#151)
CI / Test (pull_request) Successful in 13s
CI / Lint & Typecheck (pull_request) Successful in 18s
CI / Build & Push Docker Images (pull_request) Successful in 45s
CI / Test (push) Successful in 2m20s
CI / Lint & Typecheck (push) Successful in 2m25s
CI / Build & Push Docker Images (push) Successful in 28s
2026-06-02 18:09:31 +00:00
Flea Flicker fc6c6ef752 fix(db): make services seed idempotent across resets (GRO-2064, GRO-2033 close-out) (#148)
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 28s
CI / Test (pull_request) Successful in 14s
CI / Lint & Typecheck (pull_request) Successful in 20s
CI / Build & Push Docker Images (pull_request) Successful in 39s
2026-06-02 04:54:33 +00:00
The Dogfather 1a6a54cc84 security(audit): log owner-bypass reads in GET /pets/:id/profile-summary (GRO-2062) (#146)
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Successful in 16s
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 40s
CI / Build & Push Docker Images (pull_request) Successful in 27s
QA-approved (gb_lint) + CTO-approved. Defense-in-depth audit row on staff owner-bypass. GRO-2063.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-02 04:20:23 +00:00
Flea Flicker 1f888ac716 security(audit): log owner-bypass reads in GET /pets/:id/profile-summary (GRO-2062)
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (pull_request) Successful in 1m16s
Adds a defense-in-depth audit row to impersonationAuditLogs when the
staff-side owner-bypass path fires. Mirrors the failure-isolation
pattern in src/middleware/portalAudit.ts: insert failures are logged
and swallowed so a working read can never turn into a 500.

- New writeOwnerBypassAudit helper called only when isOwner === true.
- No DB migration; petId + actorStaffId go inside metadata jsonb.
- resolveImpersonationClientId stays pure (no audit side effects).
- Positive + negative tests + a cross-tenant regression test.
- UAT_PLAYBOOK.md §3.19d: TC-API-3.19d documents the audit assertion.

Parent tracking: GRO-2062 (Paperclip).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 04:10:58 +00:00
Flea Flicker 91eb2ccf71 fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052) (#143)
CI / Test (push) Successful in 11s
CI / Lint & Typecheck (push) Successful in 15s
CI / Test (pull_request) Successful in 9s
CI / Lint & Typecheck (pull_request) Successful in 14s
CI / Build & Push Docker Images (push) Successful in 36s
CI / Build & Push Docker Images (pull_request) Successful in 26s
fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052)

Ports the Better-Auth user-table auto-provision branch from canonical apps/api into the deployed ./src/middleware/rbac.ts so the owner-bypass in pets.ts is reachable for Better-Auth email/password customers. OIDC account branch retained as backward-compat fallback. Adds 5 rbac.test.ts cases and UAT_PLAYBOOK pre-condition docs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Flea Flicker <flea@groombook.dev>
Co-committed-by: Flea Flicker <flea@groombook.dev>
2026-06-02 02:40:43 +00:00
The Dogfather a2b09ba502 fix(pets): port owner-bypass into deployed tree (GRO-2013) (#139)
CI / Test (push) Successful in 13s
CI / Lint & Typecheck (push) Successful in 16s
CI / Build & Push Docker Images (push) Successful in 1m5s
CI / Test (pull_request) Successful in 16s
CI / Lint & Typecheck (pull_request) Successful in 2m25s
CI / Build & Push Docker Images (pull_request) Failing after 32s
2026-06-01 20:06:24 +00:00
Flea Flicker 4322fb2a00 Merge pull request 'fix(db): re-register 0034/0036 schema changes via idempotent 0039/0040 (GRO-2033)' (#140) from flea/gro-2033-idempotent-pet-profile-migrations into dev
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Failing after 14m2s
CI / Build & Push Docker Images (push) Has been skipped
Merge PR #140: fix(db): re-register 0034/0036 schema changes via idempotent 0039/0040 (GRO-2033)
2026-06-01 20:00:41 +00:00
Paperclip 27accb9b39 fix(db): re-register 0034/0036 schema changes via idempotent 0039/0040 (GRO-2033)
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 1m11s
Prod cumulative promotion 2026.06.01-7667288 (PR #596) revealed that
0034_extend_pet_profile_columns (temperament_score + 3 jsonb cols) and
0036_add_missing_coat_type_values (short/medium/silky) were silently
skipped on the prod database, leaving the seed/reset path with:

  Seed failed: PostgresError: column "temperament_score" does not exist

## Root cause: drizzle high-water-mark, same shape as GRO-1999

drizzle-orm@0.38.4 `pg-core/dialect.js#migrate` only applies a journal
entry when its `folderMillis` is strictly greater than the most recent
`__drizzle_migrations.created_at`:

  if (!lastDbMigration || Number(lastDbMigration.created_at) < migration.folderMillis) {
    // apply SQL + record hash
  }

`packages/db/migrations/meta/_journal.json` has 0033's when at
1779500000000 (2026-05-23) — but 0034 was registered with when
1751140800000 (2025-06-28) and 0036 with 1751480000000 (2025-07-02).
Both are below the 0033 watermark, so on the prod DB (whose newest
applied migration was 0033) drizzle silently skipped 0034 and 0036.
0038 (when 1780000000000) was above the watermark, so it applied — and
the migrate Job exits 0 with 'migrations applied successfully!'. The
schema didn't change. GRO-1999 documented the same bug for 0037 → 0038.

UAT/dev are unaffected because their watermarks were already below the
0034/0036 entries when those originally ran.

## Fix

Add two new idempotent migrations with monotonic 'when':

- 0039_extend_pet_profile_columns_idempotent.sql, when 1780000000001:
    ALTER TABLE pets ADD COLUMN IF NOT EXISTS temperament_score integer;
    -- + temperament_flags jsonb, medical_alerts jsonb, preferred_cuts jsonb
- 0040_register_missing_coat_type_values.sql, when 1780000000002:
    ALTER TYPE coat_type ADD VALUE IF NOT EXISTS 'short';
    -- + 'medium', 'silky'

Both are 'IF NOT EXISTS' — safe no-ops on UAT/dev where 0034/0036
applied normally, and effective forward-fix on prod where they were
skipped. Do NOT modify 0034/0036 in place (per the GRO-1999 pattern):
UAT/dev have already applied them and re-running would fail.

## Verification

- packages/db/migrations/meta/_journal.json now has 41 entries with idx
  39 and 40 strictly monotonic in 'when'.
- python3 -c 'import json; json.load(open(...))' parses cleanly.
- ALTER TYPE ADD VALUE IF NOT EXISTS is permitted inside a tx on
  PostgreSQL 18.3 (prod cluster image confirmed via CNPG status).

## UAT Playbook

No user-visible behaviour change — schema only. Existing TC-API-3.8 / 3.9 /
3.11 / 3.13 (extended pet profile) and 3.19a (profile summary) continue to
pass and now ALSO act as smoke tests after the prod image roll-forward.

## Refs

- Issue: GRO-2033
- Same-shape prior bug: GRO-1999 (0037 → 0038), commit 423d4bf
- Mitigation: groombook/infra PR #597 (suspend prod reset-demo-data
  CronJob while this lands)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-01 19:36:22 +00:00
The Dogfather 9903b51931 fix(pets): customer can view own pet profile summary (GRO-2013) (#135)
CI / Test (push) Successful in 16s
CI / Lint & Typecheck (push) Failing after 14m15s
CI / Build & Push Docker Images (push) Has been skipped
Adds an owner-bypass in the profile-summary handler for customers signed in via Better Auth, using the existing X-Impersonation-Session-Id portal session header. When a groomer-role staff row carries a valid impersonation session whose clientId matches the pet's clientId, skip groomerLinkageCheck and serve the summary. Otherwise fall through to the existing linkage check.

Resolves a 403 Forbidden where the customer (auto-provisioned by resolveStaffMiddleware as a 'groomer' staff row with no appointment linkage) could not read their own pet's profile.

Scope: GRO-2013 profile-summary endpoint only — no rbac.ts/schema/Dockerfile changes.

Tests: 6 new cases (owner-bypass, no-header, cross-tenant, expired, manager regression, linked-groomer regression); 294/294 pass.

UAT_PLAYBOOK.md: TC-API-3.19a/b/c.

Closes GRO-2013.

Co-authored-by: The Dogfather <20+gb_dogfather@noreply.git.farh.net>
Co-committed-by: The Dogfather <20+gb_dogfather@noreply.git.farh.net>
2026-06-01 18:40:25 +00:00
Flea Flicker fee62c895d fix(api): GRO-2014 — profile-summary 500 → 404/401/JSON-500 (#137)
CI / Lint & Typecheck (push) Successful in 16s
CI / Test (push) Successful in 16s
CI / Build & Push Docker Images (push) Successful in 46s
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 14s
CI / Build & Push Docker Images (pull_request) Failing after 18s
2026-06-01 18:16:29 +00:00
Flea Flicker 2251a172e3 docs(UAT_PLAYBOOK): document canonical source-of-truth for UAT seed passwords (GRO-2000) (#132)
CI / Lint & Typecheck (push) Failing after 5s
CI / Test (push) Successful in 12s
CI / Build & Push Docker Images (push) Has been skipped
CI / Test (pull_request) Successful in 11s
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Build & Push Docker Images (pull_request) Failing after 19s
2026-06-01 17:11:12 +00:00
Flea Flicker 1d28adb71a Merge pull request 'fix(docker): bake pnpm via npm to remove Corepack runtime downloads (GRO-1981)' (#129) from flea-flicker/gro-1985-bake-pnpm-offline into dev
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 1m10s
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 14s
CI / Build & Push Docker Images (pull_request) Successful in 24s
Self-merge per SDLC Phase 1 Step 4 — CTO review approved by gb_dogfather, CI 3/3 green, QA approved by gb_lint. Closes GRO-1985.

cc @cpfarhood
2026-06-01 16:24:41 +00:00
The Dogfather 7f8a1f4bcd Merge pull request 'fix(db): register extra_large via migration 0038 (GRO-1999)' (#130) from flea/gro-1999-migration-0038 into dev
CI / Test (push) Successful in 13s
CI / Test (pull_request) Successful in 14s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (pull_request) Successful in 37s
CI / Lint & Typecheck (push) Successful in 2m23s
CI / Build & Push Docker Images (push) Successful in 32s
2026-06-01 14:49:46 +00:00
Paperclip 423d4bf72d fix(db): register extra_large via migration 0038 (GRO-1999)
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 1m27s
GRO-1979 added 0037_add_extra_large_to_pet_size_category with a journal
'when' of 1751500000000 — below the 0033 high-water mark (1779500000000)
on existing UAT/persistent DBs. Drizzle only applies a migration when its
journal.when is strictly greater than max(applied created_at), so 0037
was silently skipped, leaving pet_size_category without 'extra_large'
and crashing the UAT seed-test-data job (22P02 enum error).

This adds 0038 with a monotonic 'when' (1780000000000) so it applies on
both existing UAT/persistent DBs and fresh DBs. Statement is idempotent
(ADD VALUE IF NOT EXISTS) and a single auto-commit DDL (ADD VALUE cannot
run inside a transaction block).

Do not modify 0033/0034/0036/0037 — re-registering extra_large is correct
since the drizzle PetSizeCategory type and seed.ts both use that value.

GRO-2004

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-01 14:41:27 +00:00
Flea Flicker 3e547b8568 fix(docker): bake pnpm via npm to remove Corepack runtime downloads (GRO-1981)
CI / Test (pull_request) Successful in 17s
CI / Lint & Typecheck (pull_request) Successful in 23s
CI / Build & Push Docker Images (pull_request) Successful in 1m14s
The GRO-1983 fast restoration swapped Corepack's pnpm shim for a real
`npm install -g pnpm@9.15.4` binary, which is the right move. But the
GRO-1997 evidence gate still showed the first `reset-demo-data` pod
(...-nh7vg) hitting `getaddrinfo EAI_AGAIN registry.npmjs.org` before a
retry succeeded — the cache was writable, the cold-cache registry
download wasn't eliminated. This is the durable fix:

1. `ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0` in `base` and `runner`:
   defence in depth so a Corepack shim can never silently re-download
   pnpm, even if it is somehow re-introduced.

2. `ENV HOME=/tmp` in the `migrate`, `seed`, and `reset` stages:
   under `readOnlyRootFilesystem: true` + `runAsUser: 1000`, the
   default HOME path is read-only, and pnpm fails the first time it
   tries to write a config or state file. The job pods already mount a
   writable emptyDir at `/tmp`; point HOME there.

3. CI smoke tests for `seed` and `reset` images (matching the existing
   `migrate` smoke): point `registry.npmjs.org` at 127.0.0.1 in a
   throwaway container, assert `which pnpm` resolves to
   `/usr/local/bin/pnpm` (real binary, not shim), and that `pnpm
   --version` succeeds without network egress. If Corepack ever sneaks
   back in, CI catches it on every PR.

The vestigial `RUN mkdir -p /home/node/.cache/node/corepack` in the
`builder` stage (mentioned in the spec) was already removed in GRO-1909
(commit 0a3eb8a), so nothing to do there.

Follow-on cleanup of the per-job `COREPACK_HOME` env vars and
`node-cache` emptyDir mounts in `groombook/infra` is intentionally
deferred to a coordinated infra PR once the new image is deployed —
keeping the existing infra in place during the transition avoids a
flag-day.

GRO-1985, hardening follow-up to GRO-1984 / GRO-1983.
Closes parent: GRO-1981.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-01 14:02:38 +00:00
Flea Flicker a9bac033fd docs(UAT_PLAYBOOK): add TC-API-3.28 for pet_size_category enum (GRO-1999) (#127)
CI / Test (push) Successful in 13s
CI / Lint & Typecheck (push) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 36s
CI / Test (pull_request) Successful in 10s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 37s
2026-06-01 13:50:16 +00:00
15 changed files with 1644 additions and 60 deletions
+29
View File
@@ -156,3 +156,32 @@ jobs:
${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/reset:latest' || '' }}
cache-from: type=registry,ref=git.farh.net/groombook/cache:reset
cache-to: type=registry,ref=git.farh.net/groombook/cache:reset,mode=max
- name: Smoke test seed image (blackhole npmjs.org)
run: |
set -euo pipefail
IMAGE="git.farh.net/groombook/seed:${{ steps.version.outputs.tag }}"
docker pull "$IMAGE"
# GRO-1985: pnpm must be a real binary, not a Corepack shim, and must
# not try to reach registry.npmjs.org on invocation.
docker run --rm \
--add-host registry.npmjs.org:127.0.0.1 \
--entrypoint="" \
"$IMAGE" \
sh -c 'set -e; test "$(which pnpm)" = "/usr/local/bin/pnpm"; pnpm --version'
echo "seed image: pnpm resolves to /usr/local/bin/pnpm and runs offline ✓"
- name: Smoke test reset image (blackhole npmjs.org)
run: |
set -euo pipefail
IMAGE="git.farh.net/groombook/reset:${{ steps.version.outputs.tag }}"
docker pull "$IMAGE"
# GRO-1985: pnpm must be a real binary, not a Corepack shim, and must
# not try to reach registry.npmjs.org on invocation. Validates the
# hard requirement from the issue: reset runs offline.
docker run --rm \
--add-host registry.npmjs.org:127.0.0.1 \
--entrypoint="" \
"$IMAGE" \
sh -c 'set -e; test "$(which pnpm)" = "/usr/local/bin/pnpm"; echo "HOME=$HOME"; pnpm --version'
echo "reset image: pnpm resolves to /usr/local/bin/pnpm, HOME=/tmp, runs offline ✓"
+13 -1
View File
@@ -3,8 +3,12 @@ FROM node:22-alpine AS base
# invocations of `pnpm` work without DNS access to registry.npmjs.org.
# The corepack shim delegates to corepack, which re-validates against
# npmjs.org on first use — that fails in air-gapped UAT seed/migrate/reset
# Jobs. GRO-1983 / GRO-1889 / GRO-1909.
# Jobs. GRO-1983 / GRO-1889 / GRO-1909 / GRO-1981 / GRO-1985.
RUN npm install -g pnpm@9.15.4
# Belt-and-braces: disable Corepack's download fallback so that even if a
# Corepack shim is somehow invoked at runtime, it will not try to fetch
# pnpm from registry.npmjs.org. Belt for the real-binary trousers. GRO-1985.
ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0
WORKDIR /app
# Install deps
@@ -26,6 +30,8 @@ RUN pnpm --filter @groombook/types build && \
# Runtime
FROM node:22-alpine AS runner
RUN npm install -g pnpm@9.15.4
# Same defence-in-depth as base: no Corepack fallback. GRO-1985.
ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0
WORKDIR /app
ENV NODE_ENV=production
@@ -46,12 +52,18 @@ CMD ["node", "dist/index.js"]
# Migrate stage — runs drizzle-kit migrate against the database
FROM builder AS migrate
# pnpm needs a writable HOME for any config/state it writes. With
# readOnlyRootFilesystem: true and runAsUser: 1000, /home/node is read-only.
# The job pods mount a writable emptyDir at /tmp; point HOME there. GRO-1985.
ENV HOME=/tmp
CMD ["pnpm", "--filter", "@groombook/db", "migrate"]
# Seed stage — populates the database with test data
FROM builder AS seed
ENV HOME=/tmp
CMD ["pnpm", "--filter", "@groombook/db", "seed"]
# Reset stage — drops all tables, re-runs migrations, and re-seeds
FROM builder AS reset
ENV HOME=/tmp
CMD ["pnpm", "--filter", "@groombook/db", "reset"]
+74
View File
@@ -19,6 +19,45 @@ GroomBook API is a Hono-based REST service (TypeScript/Node.js) powering the pet
- OIDC authentication provider configured
- Seed data present (clients, pets, services, staff)
### Source of truth for UAT passwords (GRO-2000)
The `UAT_SUPER_PASSWORD` / `UAT_GROOMER_PASSWORD` / `UAT_TESTER_PASSWORD` / `UAT_CUSTOMER_PASSWORD` env vars the test orchestrator uses **must** be pulled from the live `seed-uat-passwords` Secret in the UAT cluster — never from a captured shell value, a previous run's `.env`, or a copy of the SealedSecret committed before the latest rotation.
**Canonical recipe** (works from any host with `kubectl` + cluster credentials):
```bash
SUPER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
-o jsonpath='{.data.super-password}' | base64 -d)
GROOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
-o jsonpath='{.data.groomer-password}' | base64 -d)
TESTER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
-o jsonpath='{.data.tester-password}' | base64 -d)
CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
-o jsonpath='{.data.customer-password}' | base64 -d)
```
**Why:** the Bitnami SealedSecret `apps/overlays/uat/ss-seed-uat-passwords.yaml` (in `groombook/infra`) is the single source of truth. The UAT `reset-demo-data` CronJob re-hashes these values into the `account` table on every run (idempotent — GRO-1977). A captured env var from a previous generation will not match the current hash, producing 401 `INVALID_EMAIL_OR_PASSWORD`. If the live login still 401s after pulling from the SealedSecret, the seed Job is stale — trigger `kubectl create job --from=cronjob/reset-demo-data -n groombook-uat manual-seed-$$` and retry.
**How to apply:** at the start of every UAT run that touches TC-API-1.4 / 1.5 / 1.6 / 1.7 / 3.18 / 3.21 / 3.23, refresh these four env vars from the cluster before issuing the sign-in request.
### rbac auto-provision for Better-Auth customers (GRO-2052)
> Applies to TC-API-3.16 / 3.19a / 3.19b / 3.19c (customer-as-owner profile-summary paths) and any future case where the test user authenticates via Better-Auth email/password and the route relies on `resolveStaffMiddleware` to resolve a `staff` row.
**Pre-condition (rbac auto-provision):** The test user must have a row in the Better-Auth `user` table (email/password sign-in creates this automatically — see TC-API-1.6 / 1.7). On first authenticated call, `resolveStaffMiddleware` (`./src/middleware/rbac.ts`) auto-provisions a `groomer` staff row keyed by `staff.user_id = user.id` (Better-Auth branch fires before the legacy OIDC `account` branch).
**Verify the auto-provision fired** by querying the DB after the first authenticated call:
```sql
SELECT user_id, role FROM staff WHERE user_id = '<test-user-id>';
```
Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the OIDC `account` branch and 403'd, or the user has no `user` row — fix the test sign-in path before re-running.
**Why this matters:** without the auto-provision branch, Better-Auth email/password customers (e.g. `uat-customer@groombook.dev`) have no `account` row for the OIDC providers, so `resolveStaffMiddleware` falls through to `403 "Forbidden: no staff record found for authenticated user"` *before* `pets.ts` can run the owner-bypass added in GRO-2013. The owner-bypass code is unreachable unless the auto-provision has fired. A green TC-API-3.19a therefore implicitly proves the auto-provision worked; if 3.19a fails with the pre-fix 403, the auto-provision branch is missing from the deployed `./src` tree (see [GRO-2052](/GRO/issues/GRO-2052)).
**How to apply:** for every run of TC-API-3.16 / 3.19a / 3.19b / 3.19c, sign in via TC-API-1.6 (email+password) first to guarantee the `user` row exists, then run the profile-summary call, then assert the `staff` row above before declaring pass.
## Test Cases
### 4.0 Health Check
@@ -104,6 +143,13 @@ GroomBook API is a Hono-based REST service (TypeScript/Node.js) powering the pet
| TC-API-3.17 | Get pet profile summary — groomer restricted | GET /api/pets/{id}/profile-summary as groomer with no pet linkage | 403 Forbidden |
| TC-API-3.18 | Get pet profile summary — visitCount returns full count | GET /api/pets/{id}/profile-summary with 2+ completed appointments | visitCount >= 2 (not capped at 1) |
| TC-API-3.19 | Get pet profile summary — upcomingAppointment excludes past | GET /api/pets/{id}/profile-summary with a past confirmed/scheduled appointment | upcomingAppointment is null (past appointments filtered by startTime >= now) |
| TC-API-3.19a | Get pet profile summary — customer owner-bypass (GRO-2013) | Sign in as `uat-customer@groombook.dev`; `POST /api/portal/session-from-auth`; then `GET /api/pets/{ownPetId}/profile-summary` with header `X-Impersonation-Session-Id: {sessionId}` for either of the customer's seeded pets (`c0000001-0000-0000-0000-000000000002` UAT Pup Alpha, `c0000001-0000-0000-0000-000000000003` UAT Pup Beta) | 200 OK, aggregated profile returned (owner-bypass: customer with valid portal session for pet's clientId is allowed even though rbac.ts auto-provisions them as a `groomer` staff row with no appointment linkage) |
| TC-API-3.19b | Get pet profile summary — customer cross-tenant blocked (GRO-2013) | Sign in as `uat-customer@groombook.dev`; reuse the customer's sessionId from TC-API-3.19a; `GET /api/pets/{otherClientPetId}/profile-summary` for a pet owned by a different client (`c0000002-...` or any non-customer pet) | 403 Forbidden (owner-bypass requires session.clientId === pet.clientId) |
| TC-API-3.19c | Get pet profile summary — customer without portal session header | Same as TC-API-3.19a but omit the `X-Impersonation-Session-Id` header | 403 Forbidden (no owner-bypass without valid portal session) |
| TC-API-3.19d | Get pet profile summary — owner-bypass writes audit row (GRO-2063) | Same setup as TC-API-3.19a (sign in as `uat-customer@groombook.dev`, establish a portal session for the customer's own clientId, call `GET /api/pets/{ownPetId}/profile-summary` with `X-Impersonation-Session-Id: {sessionId}` and a 200 OK response). Then call `GET /api/impersonation/sessions/{sessionId}/audit-log` and confirm there is exactly one entry with `action === "read_profile_summary"`, `pageVisited` matching the profile-summary path, and `metadata` containing `petId` and `actorStaffId` for the customer. Repeat TC-API-3.19b (cross-tenant attempt) and confirm NO new `read_profile_summary` row was written for the cross-tenant attempt. | 200 OK on the profile-summary call AND an audit log entry is present with the correct shape (defense-in-depth audit row; bypass attempts against other clients must NOT log) |
| TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) |
| TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) |
| TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` |
#### Seed Data Verification (GRO-1898)
@@ -120,6 +166,7 @@ GroomBook API is a Hono-based REST service (TypeScript/Node.js) powering the pet
| TC-API-3.26 | Verify 25-35% medicalAlerts distribution | GET /api/pets (first 30 pets), count how many have non-empty medicalAlerts | Ratio is 25-35% (seed uses rand() < 0.3 for ~30% distribution) |
| TC-API-3.27 | Verify coat_type enum has all seed values | After UAT seed completes, inspect the coat_type enum on the UAT DB — it must contain: short, medium, long, double, wire, silky, curly, hairless | UAT seed jobs (`reset-demo-data`, `seed-test-data`) complete 1/1 with no `enum_in` error; coat_type includes all 8 values used by seed.ts `coatTypePool` |
| TC-API-3.28 | Verify pet_size_category enum has all seed values | After UAT seed completes, inspect the pet_size_category enum on the UAT DB — it must contain: small, medium, large, extra_large | UAT seed jobs (`reset-demo-data`, `seed-test-data`) complete 1/1 with no `enum_in` error; pet_size_category includes all 4 values used by seed.ts `petSizeCategoryPool` (regression for GRO-1999, mirrors TC-API-3.27) |
| TC-API-3.29 | Verify `reset-demo-data` CronJob does not fail with FK 23503 on `invoice_tip_splits` (GRO-2123) | Trigger the CronJob manually: `kubectl create job --from=cronjob/reset-demo-data verify-gro2123 -n groombook-uat`. Wait for pod to terminate. Inspect logs: `kubectl logs -n groombook-uat -l job-name=verify-gro2123` | Pod reaches `Completed` state; logs show `✓ Acquired seed advisory lock` and `✓ Released seed advisory lock` from `seed.ts`; no `PostgresError: … violates foreign key constraint "invoice_tip_splits_invoice_id_invoices_id_fk"` (code 23503); final counts unchanged (500 clients, ~4000 invoices) |
### 4.4 Appointment Scheduling
@@ -146,6 +193,33 @@ GroomBook API is a Hono-based REST service (TypeScript/Node.js) powering the pet
| TC-API-5.4 | Update service | PATCH /api/services/{id} with updated fields | 200 OK, service updated |
| TC-API-5.5 | Delete service | DELETE /api/services/{id} | 200 OK, service deleted |
#### 4.5.1 Seed/Reset idempotency (GRO-2064)
Services seeding is now keyed on the deterministic `services.id` (not `name`) and
the reset path now `TRUNCATE`s `services` alongside the other dynamic tables.
This means:
- Running the seed Job twice in a row (no reset in between) converges to the
same catalogue — no `services_pkey` collision.
- A `pnpm reset` followed by `pnpm seed` (or a CronJob reset fire) leaves the
catalogue exactly matching `servicesDef` (10 rows, ids `b0000001-…-001`
`…-00a`), regardless of any stale rows that were present beforehand.
- Mixed `seedKnownUsers` + full `seed()` invocations are safe — the
`demoSvcs` subset (Bath & Brush, Full Groom Small/Medium, Nail Trim) is
keyed on ids `…-001`, `…-002`, `…-003`, `…-005` and the upsert target
is `services.id`, so the same-id / different-name collision that broke
GRO-2033 (id `…-004` = "Nail Trim" vs servicesDef `…-004` =
"Full Groom — Large") cannot recur.
**UAT regression** (verify after a new image is rolled out):
| # | Scenario | Steps | Expected |
|---|----------|-------|----------|
| TC-SEED-1 | Reset → seed converges | `kubectl -n groombook exec deploy/api -- pnpm reset && pnpm seed` | Seed completes 1/1, `services` count = 10, all ids match `servicesDef` |
| TC-SEED-2 | Idempotent re-seed | Re-run `pnpm seed` without reset | Seed completes 1/1, no `services_pkey` errors, `services` count still 10 |
| TC-SEED-3 | Catalogue matches servicesDef | `psql -c "SELECT id, name FROM services ORDER BY id"` | Rows `…-001``…-00a` with names "Bath & Brush"…"Sanitary Trim" exactly as in `servicesDef` |
| TC-SEED-4 | Demo subset coexists | Run `seedKnownUsers` then full `seed` | No collision, demo subset (4 services) ends up with the same rows the full seed would write |
### 4.6 Staff Management
| # | Scenario | Steps | Expected |
@@ -44,6 +44,7 @@ interface MockState {
groomingLogs: Record<string, unknown>[];
staffMembers: Record<string, unknown>[];
services: Record<string, unknown>[];
impersonationSessions: Record<string, unknown>[];
}
let mock: MockState;
@@ -168,6 +169,19 @@ function resetMock() {
{ id: "service-1", name: "Full Groom", description: null, basePriceCents: 6000, durationMinutes: 120, active: true, createdAt: new Date(), updatedAt: new Date() },
{ id: "service-2", name: "Bath & Brush", description: null, basePriceCents: 4000, durationMinutes: 60, active: true, createdAt: new Date(), updatedAt: new Date() },
],
impersonationSessions: [
{
id: "sess-owner",
staffId: "staff-groomer-id",
clientId: CLIENT_ID,
reason: "sso-bridge",
status: "active",
startedAt: new Date("2024-11-01"),
endedAt: null,
expiresAt: new Date("2099-01-01T00:00:00Z"),
createdAt: new Date("2024-11-01"),
},
],
};
}
@@ -177,6 +191,7 @@ vi.mock("../db/index.js", () => {
const groomingVisitLogs = new Proxy({ _name: "groomingVisitLogs" }, { get: (t, p) => p === "_name" ? "groomingVisitLogs" : {} });
const staff = new Proxy({ _name: "staff" }, { get: (t, p) => p === "_name" ? "staff" : {} });
const services = new Proxy({ _name: "services" }, { get: (t, p) => p === "_name" ? "services" : {} });
const impersonationSessions = new Proxy({ _name: "impersonationSessions" }, { get: (t, p) => p === "_name" ? "impersonationSessions" : {} });
// Tracks { [tableName]: { [alias]: SQLExpression } } for the current select() call
let selectedColumns: Record<string, Record<string, unknown>> = {};
@@ -248,6 +263,7 @@ vi.mock("../db/index.js", () => {
if (name === "groomingVisitLogs") return makeChainable(mock.groomingLogs);
if (name === "staff") return makeChainable(mock.staffMembers);
if (name === "services") return makeChainable(mock.services);
if (name === "impersonationSessions") return makeChainable(mock.impersonationSessions);
return makeChainable([]);
},
};
@@ -261,6 +277,7 @@ vi.mock("../db/index.js", () => {
groomingVisitLogs,
staff,
services,
impersonationSessions,
and: vi.fn((a: unknown, b: unknown) => [a, b]),
desc: vi.fn((c: unknown) => c),
eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })),
@@ -399,4 +416,102 @@ describe("GET /:id/profile-summary — empty history", () => {
expect(body.recentGroomingHistory).toEqual([]);
expect(body.lastVisitDate).toBeNull();
});
});
describe("GET /:id/profile-summary — owner-bypass via X-Impersonation-Session-Id (GRO-2013)", () => {
beforeEach(resetMock);
// Simulates the rbac.ts auto-provisioned "groomer" that a customer gets on first login:
// role=groomer, no linkage to any appointment.
const CUSTOMER_STAFF: StaffRow = {
id: "staff-customer-id",
oidcSub: null,
userId: "user-customer-id",
role: "groomer",
isSuperUser: false,
name: "UAT Customer",
email: "uat-customer@groombook.dev",
active: true,
icalToken: null,
createdAt: new Date(),
updatedAt: new Date(),
};
it("customer with valid portal session for pet's client returns 200 (owner-bypass)", async () => {
const app = makeApp(CUSTOMER_STAFF);
// Groomer has no appointment linkage — proves the bypass is via portal session, not linkage.
mock.appointments = [];
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-owner" },
});
expect(res.status).toBe(200);
const body = await res.json();
expect(body.id).toBe(PET_ID);
expect(body.name).toBe("Biscuit");
expect(body.clientId).toBe(CLIENT_ID);
});
it("customer without X-Impersonation-Session-Id header still gets 403 (no bypass)", async () => {
const app = makeApp(CUSTOMER_STAFF);
mock.appointments = [];
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(403);
});
it("customer with portal session for a DIFFERENT client gets 403 (cross-tenant blocked)", async () => {
const app = makeApp(CUSTOMER_STAFF);
mock.appointments = [];
mock.impersonationSessions = [
{
id: "sess-other-client",
staffId: "staff-customer-id",
clientId: "00000000-0000-0000-0000-000000000099", // different from CLIENT_ID
reason: "sso-bridge",
status: "active",
startedAt: new Date("2024-11-01"),
endedAt: null,
expiresAt: new Date("2099-01-01T00:00:00Z"),
createdAt: new Date("2024-11-01"),
},
];
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-other-client" },
});
expect(res.status).toBe(403);
});
it("customer with expired portal session still gets 403", async () => {
const app = makeApp(CUSTOMER_STAFF);
mock.appointments = [];
mock.impersonationSessions = [
{
id: "sess-expired",
staffId: "staff-customer-id",
clientId: CLIENT_ID,
reason: "sso-bridge",
status: "active",
startedAt: new Date("2024-01-01"),
endedAt: null,
expiresAt: new Date("2024-02-01T00:00:00Z"), // expired long ago
createdAt: new Date("2024-01-01"),
},
];
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-expired" },
});
expect(res.status).toBe(403);
});
it("manager does NOT need the impersonation header (existing role check still works)", async () => {
const app = makeApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
});
it("groomer with linkage to pet's client still works (regression — no regression from bypass)", async () => {
const app = makeApp(GROOMER);
// GROOMER fixture has appointments linked to staff-groomer-id in the mock state
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
});
});
+27 -12
View File
@@ -636,21 +636,28 @@ async function seedKnownUsers() {
}
}
// ── Services: idempotent upsert using name as unique key ─────────────────────
// UNIQUE constraint on services.name (migration 0020) must exist first.
// Uses b0000001-... IDs to match main seed servicesDef for same-named services.
// ── Services: idempotent upsert keyed on `id` ─────────────────────────────
// GRO-2064: previously keyed on `services.name` while writing a
// deterministic `id`. If a stale row existed with the same `id` but a
// different `name`, PostgreSQL raised `services_pkey` (id collision)
// before the name-targeted ON CONFLICT could fire. Switch the conflict
// target to `services.id` so deterministic ids always win; pair with
// `TRUNCATE services … CASCADE` above so each reset rebuilds the
// catalogue from `servicesDef` cleanly. GRO-2033 close-out.
// Id↔name map MUST stay in sync with `servicesDef` (the canonical source
// of truth in the main `seed()` function).
const demoSvcs = [
{ id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 },
{ id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 },
{ id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 },
{ id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
{ id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
];
for (const svc of demoSvcs) {
await db.insert(schema.services)
.values({ ...svc, active: true })
.onConflictDoUpdate({
target: schema.services.name,
set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
target: schema.services.id,
set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
});
}
console.log(`✓ Seeded ${demoSvcs.length} services`);
@@ -757,7 +764,13 @@ async function seed() {
({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false })
);
await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
// GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue
// from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows
// (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the
// same `id`) would otherwise cause the deterministic upsert to PK-collide
// on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE
// CASCADE handles appointments/invoices FKs to services.id.
await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers];
for (const s of allStaff) {
@@ -828,9 +841,11 @@ async function seed() {
}
// ── Services ──
// Upsert services using name as unique key. With deterministic IDs in
// servicesDef and TRUNCATE clearing downstream tables first, this is
// idempotent: first run inserts, subsequent runs update existing rows.
// GRO-2064: key the upsert on `services.id` (not `name`) so deterministic
// ids always win, and rely on the TRUNCATE above to clear stale rows before
// the catalogue is rebuilt. The previous name-targeted upsert failed with
// `services_pkey` when a prior run had left a row with the same id but a
// different name (CTO review on infra PR #605, rev #4230).
const serviceIds: string[] = [];
for (const s of servicesDef) {
serviceIds.push(s.id);
@@ -844,8 +859,8 @@ async function seed() {
active: true,
})
.onConflictDoUpdate({
target: schema.services.name,
set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
target: schema.services.id,
set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
});
}
console.log(`✓ Created ${servicesDef.length} services`);
+37 -1
View File
@@ -1,7 +1,7 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod/v3";
import { and, desc, eq, exists, getDb, gte, groomingVisitLogs, or, pets, appointments, staff, services, sql } from "../db/index.js";
import { and, desc, eq, exists, getDb, gte, groomingVisitLogs, impersonationSessions, or, pets, appointments, staff, services, sql } from "../db/index.js";
import type { AppEnv } from "../middleware/rbac.js";
import {
getPresignedUploadUrl,
@@ -307,10 +307,38 @@ async function groomerLinkageCheck(
return !!linkage;
}
/**
* Resolves the clientId from the X-Impersonation-Session-Id header, if present and active.
* Used by staff routes to allow a customer (auto-provisioned as a `groomer` staff row
* by rbac.ts) to access their own pet's data when they are the rightful owner.
*
* Returns null when the header is missing, the session is unknown/expired/ended, or the
* session exists but has no clientId — callers should treat null as "no owner-bypass".
*/
async function resolveImpersonationClientId(
db: ReturnType<typeof getDb>,
c: { req: { header: (name: string) => string | undefined } }
): Promise<string | null> {
const sessionId = c.req.header("X-Impersonation-Session-Id");
if (!sessionId) return null;
const [session] = await db
.select({ clientId: impersonationSessions.clientId, status: impersonationSessions.status, expiresAt: impersonationSessions.expiresAt })
.from(impersonationSessions)
.where(eq(impersonationSessions.id, sessionId))
.limit(1);
if (!session) return null;
if (session.status !== "active") return null;
if (session.expiresAt <= new Date()) return null;
return session.clientId;
}
/**
* GET /:id/profile-summary
* Returns aggregated profile: basic pet fields + grooming history + visit stats + upcoming appointment.
* Groomer RBAC: same visibility rules as GET /:id.
* Owner-bypass (GRO-2013): a customer who supplies a valid X-Impersonation-Session-Id
* for the pet's owning client may read their own pet's summary, even though rbac.ts
* auto-provisions them as a `groomer` staff row with no appointment linkage.
*/
petsRouter.get("/:id/profile-summary", async (c) => {
const db = getDb();
@@ -321,7 +349,15 @@ petsRouter.get("/:id/profile-summary", async (c) => {
const [row] = await db.select().from(pets).where(eq(pets.id, petId));
if (!row) return c.json({ error: "Not found" }, 404);
// Owner-bypass: customer with a valid portal session for this pet's client
// is allowed to view their own pet's profile summary (GRO-2013).
let isOwner = false;
if (isGroomer) {
const ownerClientId = await resolveImpersonationClientId(db, c);
isOwner = !!ownerClientId && ownerClientId === row.clientId;
}
if (isGroomer && !isOwner) {
const hasLinkage = await groomerLinkageCheck(db, row.clientId, staffRow);
if (!hasLinkage) return c.json({ error: "Forbidden" }, 403);
}
@@ -0,0 +1,4 @@
-- GRO-1999: 0037 was skipped on existing DBs due to a below-high-water-mark
-- journal timestamp. Re-register extra_large with a monotonic timestamp so
-- the existing UAT/persistent DBs apply it. Idempotent.
ALTER TYPE "pet_size_category" ADD VALUE IF NOT EXISTS 'extra_large';
@@ -0,0 +1,27 @@
-- Migration: 0039_extend_pet_profile_columns_idempotent.sql
-- GRO-2033: re-register the temperament/medical/preferred-cuts columns from
-- 0034 with an idempotent ADD COLUMN IF NOT EXISTS + a monotonic journal
-- `when` (1780000000001), above the 0033 high-water mark (1779500000000)
-- and above the most recent applied migration 0038 (1780000000000).
--
-- 0034_extend_pet_profile_columns.sql was authored on 2026-05-28 with
-- `when` = 1751140800000 (2025-06-28) — *below* the 0033 high-water mark
-- of 1779500000000 (2026-05-23). drizzle-orm@0.38.4
-- (pg-core/dialect.js#migrate) only applies a migration when
-- `migration.folderMillis > lastDbMigration.created_at`, so on prod —
-- whose last applied entry was 0033 at created_at=1779500000000 — 0034
-- was silently skipped, leaving `pets.temperament_score` (and friends)
-- missing. The migrate Job still exits 0 ("migrations applied
-- successfully!") because the journal high watermark *was* advanced by
-- 0038, but no schema change ever ran for 0034. Seed/reset then crash on:
-- PostgresError: column "temperament_score" does not exist (42703)
--
-- Same pattern as GRO-1999 (0037 → 0038): do NOT modify 0034 in-place
-- (UAT/dev have already applied it via their lower watermarks). Add a
-- new idempotent migration with a monotonic `when` instead so existing
-- DBs apply it cleanly and fresh DBs are a no-op-after-no-op.
ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "temperament_score" integer;
ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "temperament_flags" jsonb DEFAULT '[]';
ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "medical_alerts" jsonb DEFAULT '[]';
ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "preferred_cuts" jsonb DEFAULT '[]';
@@ -0,0 +1,26 @@
-- Migration: 0040_register_missing_coat_type_values.sql
-- GRO-2033: re-register the 'short' / 'medium' / 'silky' coat_type enum
-- values that 0036 added with `when` = 1751480000000 — *below* the 0033
-- high-water mark of 1779500000000. drizzle-orm@0.38.4
-- (pg-core/dialect.js#migrate) silently skipped 0036 on prod for the same
-- reason it skipped 0034 (see 0039). 0036 itself was idempotent
-- (`ADD VALUE IF NOT EXISTS`), but its journal entry was never applied,
-- so the values are not in the prod enum.
--
-- Same pattern as GRO-1999 (0037 → 0038) and 0039: do NOT modify 0036 in
-- place. Add a new entry with a monotonic `when` (1780000000002) so
-- existing prod re-applies it; UAT/dev are a safe no-op because the
-- statements are `IF NOT EXISTS` and the values are already there.
--
-- Postgres restriction: `ALTER TYPE ... ADD VALUE` cannot run inside a
-- transaction block, so we emit individual auto-commit DDL statements
-- (no BEGIN/COMMIT). drizzle-kit migrate executes inside a tx; with
-- `ADD VALUE IF NOT EXISTS` Postgres is permissive and treats it as a
-- regular DDL statement that *can* run inside a tx in 9.6+ when no new
-- value is actually added. If you ever rename this to add a value that
-- doesn't exist on every target DB, lift it out of the journal
-- transaction (single-statement file) — see GRO-1999 commit 423d4bf.
ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'short';
ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'medium';
ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'silky';
+21
View File
@@ -260,6 +260,27 @@
"when": 1751500000000,
"tag": "0037_add_extra_large_to_pet_size_category",
"breakpoints": true
},
{
"idx": 38,
"version": "7",
"when": 1780000000000,
"tag": "0038_register_extra_large_pet_size_category",
"breakpoints": true
},
{
"idx": 39,
"version": "7",
"when": 1780000000001,
"tag": "0039_extend_pet_profile_columns_idempotent",
"breakpoints": true
},
{
"idx": 40,
"version": "7",
"when": 1780000000002,
"tag": "0040_register_missing_coat_type_values",
"breakpoints": true
}
]
}
+243 -17
View File
@@ -401,7 +401,9 @@ const servicesDef = [
*
* In seedKnownUsers() this replaces the inline UAT-staff block.
*/
async function seedUatStaffAccounts(db: ReturnType<typeof drizzle>) {
async function seedUatStaffAccounts(
db: ReturnType<typeof drizzle>,
): Promise<string | null> {
// ── Staff: UAT Super User (oidcSub from SEED_UAT_SUPER_OIDC_SUB env var) ──
const uatSuperOidcSub = process.env.SEED_UAT_SUPER_OIDC_SUB;
if (uatSuperOidcSub) {
@@ -668,6 +670,132 @@ async function seedUatStaffAccounts(db: ReturnType<typeof drizzle>) {
console.log(`✓ Created UAT pet '${pet.name}' with extended fields`);
}
}
// ── GRO-2100: deterministic uat-groomer ↔ pet linkage ───────────────────────
// The UAT groomer (`uat-groomer@groombook.dev`, staffId 00000000-0000-0000-0000-000000000004)
// needs at least one linked pet/appointment or GRO-1987 TC-UAT-2/3 cannot run
// (the pet profile-summary endpoint returns 404 instead of 200/403).
//
// We deterministically link the UAT groomer to the UAT customer's first pet
// ("UAT Pup Alpha") and leave the second pet ("UAT Pup Beta") UNLINKED so
// TC-UAT-2 (200) and TC-UAT-3 (403) can both hardcode the stable petIds.
//
// The linkage call itself is performed by the caller AFTER the `services`
// catalogue has been seeded (this helper runs before services exist,
// which previously caused the linkage to be silently skipped on every
// reset). GRO-2100 follow-up.
return uatCustomerClientId;
}
/**
* GRO-2100: create a deterministic completed appointment linking the UAT groomer
* to "UAT Pup Alpha" (c0000001-0000-0000-0000-000000000002). "UAT Pup Beta"
* (c0000001-0000-0000-0000-000000000003) is intentionally left UNLINKED so
* GRO-1987 TC-UAT-3 can verify the 403 forbidden response.
*
* Idempotent: the deterministic appointment id (`a0000001-…-0001`) is the
* upsert key, so re-running the seed on every reset-demo-data CronJob
* (hourly per apps/overlays/uat/reset-cronjob.yaml) is safe.
*/
async function seedUatGroomerLinkage(
db: ReturnType<typeof drizzle>,
customerClientId: string | null,
): Promise<void> {
const uatGroomerEmail = "uat-groomer@groombook.dev";
const LINKED_PET_ID = "c0000001-0000-0000-0000-000000000002"; // UAT Pup Alpha
const APPT_ID = "a0000001-0000-0000-0000-000000000001";
// Skip silently if the UAT Customer client wasn't created (non-UAT seed
// profile, e.g. seedKnownUsers() in an env without the UAT personas).
if (!customerClientId) {
return;
}
// Only run if the UAT groomer staff record actually exists — dev/test seeds
// that don't set SEED_UAT_STAFF_OIDC_SUB should not crash.
const [uatGroomerStaff] = await db
.select({ id: schema.staff.id })
.from(schema.staff)
.where(eq(schema.staff.email, uatGroomerEmail))
.limit(1);
if (!uatGroomerStaff) {
return;
}
// Skip if this exact appointment already exists (idempotent on re-seed).
const [existing] = await db
.select({ id: schema.appointments.id })
.from(schema.appointments)
.where(eq(schema.appointments.id, APPT_ID))
.limit(1);
if (existing) {
console.log(`✓ GRO-2100: uat-groomer linkage appointment already exists — skipping`);
return;
}
// Skip if the linked pet hasn't been seeded yet (defensive: caller should
// ensure pets exist; if the helper is re-ordered later we don't want to
// crash here).
const [linkedPet] = await db
.select({ id: schema.pets.id })
.from(schema.pets)
.where(eq(schema.pets.id, LINKED_PET_ID))
.limit(1);
if (!linkedPet) {
console.warn(`⚠ GRO-2100: UAT Pup Alpha (${LINKED_PET_ID}) not found — skipping uat-groomer linkage`);
return;
}
// The "Bath & Brush" service id is stable across the reset; falls back to
// any active service if it has not been seeded yet (e.g. seedKnownUsers
// runs in isolation).
const BATH_AND_BRUSH_ID = "b0000001-0000-0000-0000-000000000001";
const [bathService] = await db
.select({ id: schema.services.id })
.from(schema.services)
.where(eq(schema.services.id, BATH_AND_BRUSH_ID))
.limit(1);
let serviceId: string;
if (bathService) {
serviceId = bathService.id;
} else {
const [fallback] = await db
.select({ id: schema.services.id })
.from(schema.services)
.where(eq(schema.services.active, true))
.limit(1);
if (!fallback) {
console.warn(`⚠ GRO-2100: no active services found — skipping uat-groomer linkage`);
return;
}
serviceId = fallback.id;
}
// Schedule the completed appointment 7 days ago so the profile-summary's
// "recentGroomingHistory" window (last 10) reliably includes it.
const startTime = new Date();
startTime.setDate(startTime.getDate() - 7);
startTime.setHours(10, 0, 0, 0);
const endTime = new Date(startTime.getTime() + 45 * 60 * 1000);
await db.insert(schema.appointments).values({
id: APPT_ID,
clientId: customerClientId,
petId: LINKED_PET_ID,
serviceId,
staffId: uatGroomerStaff.id,
batherStaffId: null,
status: "completed",
startTime,
endTime,
notes: "GRO-2100: deterministic uat-groomer linkage for TC-UAT-2/3.",
priceCents: null,
confirmationStatus: "confirmed",
});
console.log(
`✓ GRO-2100: linked uat-groomer (${uatGroomerStaff.id}) → UAT Pup Alpha (${LINKED_PET_ID}) via appointment ${APPT_ID}`,
);
}
// ── Known-users-only seed (prod/demo) ───────────────────────────────────────
@@ -745,27 +873,40 @@ async function seedKnownUsers() {
// ── UAT staff accounts + Better Auth credentials (shared impl) ──────────────
// Extracted into seedUatStaffAccounts() so it runs in both seedKnownUsers()
// and the full seed() UAT branch.
await seedUatStaffAccounts(db);
const uatCustomerClientId = await seedUatStaffAccounts(db);
// ── Services: idempotent upsert using name as unique key ─────────────────────
// UNIQUE constraint on services.name (migration 0020) must exist first.
// Uses b0000001-... IDs to match main seed servicesDef for same-named services.
// ── Services: idempotent upsert keyed on `id` ─────────────────────────────
// GRO-2064: previously keyed on `services.name` while writing a
// deterministic `id`. If a stale row existed with the same `id` but a
// different `name`, PostgreSQL raised `services_pkey` (id collision)
// before the name-targeted ON CONFLICT could fire. Switch the conflict
// target to `services.id` so deterministic ids always win; pair with
// `TRUNCATE services … CASCADE` above so each reset rebuilds the
// catalogue from `servicesDef` cleanly. GRO-2033 close-out.
// Id↔name map MUST stay in sync with `servicesDef` (the canonical source
// of truth in the main `seed()` function).
const demoSvcs = [
{ id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 },
{ id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 },
{ id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 },
{ id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
{ id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
];
for (const svc of demoSvcs) {
await db.insert(schema.services)
.values({ ...svc, active: true })
.onConflictDoUpdate({
target: schema.services.name,
set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
target: schema.services.id,
set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
});
}
console.log(`✓ Seeded ${demoSvcs.length} services`);
// GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run
// AFTER services are seeded (this helper looks up an active service id
// to attach to the appointment; on a fresh reset there are none yet at
// the time seedUatStaffAccounts() returns).
await seedUatGroomerLinkage(db, uatCustomerClientId);
// ── Client: Demo Client ──
const [existingClient] = await db
.select()
@@ -835,6 +976,63 @@ async function seedKnownUsers() {
// ── Main seed ────────────────────────────────────────────────────────────────
// ── GRO-2123: serialize reset+seed with a Postgres advisory lock ────────
// The reset-demo-data CronJob runs on an hourly schedule. With
// concurrencyPolicy=Replace, a new pod can start while the previous one
// is still mid-seed; the new pod's TRUNCATE then deletes rows the old pod
// is still inserting, producing FK 23503 errors non-deterministically
// (see GRO-2123: invoice_tip_splits → invoices).
//
// We hold a session-level advisory lock for the full duration of the
// seed so that overlapping invocations block then proceed in order —
// not skip. The key is a stable 32-bit constant so it can be referenced
// from runbooks without ambiguity and binds to the single-argument
// `pg_advisory_lock(int)` form, which postgres-js serializes as a plain
// number (no bigint type plumbing required).
const SEED_ADVISORY_LOCK_KEY = 0x47524f4f; // "GROO" in ASCII — arbitrary, stable
/**
* Reserve a dedicated connection from `pool`, take the seed advisory lock
* on it, run `fn`, and release the lock + connection in a try/finally.
*
* CRITICAL: with postgres-js connection pooling, a session-level
* `pg_advisory_lock(KEY)` acquired on one pooled connection and released
* on a *different* one is a no-op (the lock is bound to the session /
* pg-backend that took it). We therefore reserve a dedicated connection
* for the lock and release it from the same reserved connection. The
* seed work itself still runs on the pooled connections.
*/
async function withSeedAdvisoryLock<T>(
pool: ReturnType<typeof postgres>,
fn: () => Promise<T>,
): Promise<T> {
const lockConnection = await pool.reserve();
let lockHeld = false;
try {
await lockConnection`SELECT pg_advisory_lock(${SEED_ADVISORY_LOCK_KEY})`;
lockHeld = true;
console.log(`✓ Acquired seed advisory lock (key=${SEED_ADVISORY_LOCK_KEY})`);
const result = await fn();
await lockConnection`SELECT pg_advisory_unlock(${SEED_ADVISORY_LOCK_KEY})`;
lockHeld = false;
console.log(`✓ Released seed advisory lock`);
return result;
} finally {
if (lockHeld) {
try {
await lockConnection`SELECT pg_advisory_unlock(${SEED_ADVISORY_LOCK_KEY})`;
} catch (err) {
console.error("Failed to release seed advisory lock during cleanup:", err);
}
}
try {
lockConnection.release();
} catch (err) {
console.error("Failed to release reserved lock connection:", err);
}
}
}
async function seed() {
const url = process.env.DATABASE_URL;
if (!url) {
@@ -852,6 +1050,22 @@ async function seed() {
const client = postgres(url, { max: 5 });
const db = drizzle(client, { schema });
// GRO-2123: hold the seed advisory lock for the full body of runSeedBody.
// See the withSeedAdvisoryLock comment for why a reserved connection is
// required (postgres-js pooling would silently drop the lock otherwise).
await withSeedAdvisoryLock(client, async () => {
return await runSeedBody(client, db, profile, cfg);
});
await client.end();
}
async function runSeedBody(
client: ReturnType<typeof postgres>,
db: ReturnType<typeof drizzle>,
profile: SeedProfile,
cfg: ProfileConfig,
): Promise<void> {
console.log(`Seeding Groom Book database (profile: ${profile})...\n`);
// ── Staff ──
@@ -868,7 +1082,13 @@ async function seed() {
({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false })
);
await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
// GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue
// from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows
// (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the
// same `id`) would otherwise cause the deterministic upsert to PK-collide
// on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE
// CASCADE handles appointments/invoices FKs to services.id.
await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers];
for (const s of allStaff) {
@@ -916,12 +1136,14 @@ async function seed() {
// ── UAT staff accounts + Better Auth credentials (shared impl) ──────────────
// Seeds deterministic UAT staff with numeric OIDC subs and Better Auth credentials.
// Must run AFTER random staff are created so upserts land correctly.
await seedUatStaffAccounts(db);
const uatCustomerClientId = await seedUatStaffAccounts(db);
// ── Services ──
// Upsert services using name as unique key. With deterministic IDs in
// servicesDef and TRUNCATE clearing downstream tables first, this is
// idempotent: first run inserts, subsequent runs update existing rows.
// GRO-2064: key the upsert on `services.id` (not `name`) so deterministic
// ids always win, and rely on the TRUNCATE above to clear stale rows before
// the catalogue is rebuilt. The previous name-targeted upsert failed with
// `services_pkey` when a prior run had left a row with the same id but a
// different name (CTO review on infra PR #605, rev #4230).
const serviceIds: string[] = [];
for (const s of servicesDef) {
serviceIds.push(s.id);
@@ -935,12 +1157,18 @@ async function seed() {
active: true,
})
.onConflictDoUpdate({
target: schema.services.name,
set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
target: schema.services.id,
set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
});
}
console.log(`✓ Created ${servicesDef.length} services`);
// GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run
// AFTER services are seeded (this helper looks up an active service id
// to attach to the appointment; on a fresh reset there are none yet at
// the time seedUatStaffAccounts() returns).
await seedUatGroomerLinkage(db, uatCustomerClientId);
// ── Clients & Pets ──
const now = new Date();
const appointmentsBackDate = new Date(now);
@@ -1459,8 +1687,6 @@ async function seed() {
}
console.log(`✓ Created ${visitLogCount} grooming visit logs`);
console.log("\nSeed complete!");
await client.end();
}
seed().catch((err) => {
+647
View File
@@ -0,0 +1,647 @@
/**
* Pet Profile Summary Tests
*
* Covers GET /api/pets/:id/profile-summary in the deployed tree (root src/).
*
* Two suites share one mock harness:
*
* 1. GRO-2013 owner-bypass (the deployed-tree port of #135):
* A customer who is auto-provisioned as a `groomer` staff row by rbac.ts
* (with no appointment linkage) may still read their own pet's summary
* when they supply a valid X-Impersonation-Session-Id whose clientId
* matches the pet's clientId.
*
* 2. GRO-2014 error handling (deployed tree):
* - Empty-body 500 must never escape the route — the onError handler
* converts unhandled errors into a structured JSON 500.
* - Malformed UUIDs must return 404 (not 500 via a Postgres uuid cast).
* - Missing staff context must return 401 (not TypeError on staffRow.id).
* - Pet not found must return 404.
* - Groomer with no appointment linkage must return 403.
* - Manager and groomer with linkage must receive the summary body.
*
* Deployed tree handler: src/routes/pets.ts. The mock queries the
* `appointments` table (the live schema) for visit history, not
* `groomingVisitLogs`.
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
import { Hono } from "hono";
import type { AppEnv, StaffRow } from "../middleware/rbac.js";
// ─── Staff fixtures ──────────────────────────────────────────────────────────
const MANAGER: StaffRow = {
id: "staff-manager-id",
oidcSub: "oidc-manager-sub",
userId: null,
role: "manager",
isSuperUser: true,
name: "Manager McManager",
email: "manager@example.com",
active: true,
icalToken: null,
createdAt: new Date(),
updatedAt: new Date(),
};
const GROOMER: StaffRow = {
...MANAGER,
id: "staff-groomer-id",
oidcSub: "oidc-groomer-sub",
role: "groomer",
isSuperUser: false,
name: "Groomer Gary",
email: "groomer@example.com",
};
/**
* Mirrors the auto-provisioned "groomer" staff row rbac.ts creates for an
* OIDC user (e.g. uat-customer@groombook.dev) on first login: role=groomer,
* no appointment linkage.
*/
const CUSTOMER_STAFF: StaffRow = {
...MANAGER,
id: "staff-customer-id",
oidcSub: null,
userId: "user-customer-id",
role: "groomer",
name: "UAT Customer",
email: "uat-customer@groombook.dev",
};
// ─── Mutable mock state ─────────────────────────────────────────────────────
const CLIENT_ID = "c0000001-0000-0000-0000-000000000001";
const PET_ID = "c0000001-0000-0000-0000-000000000002";
const OTHER_CLIENT_PET_ID = "c0000002-0000-0000-0000-000000000099";
const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001";
const futureDate = () => new Date(Date.now() + 30 * 60_000);
const pastDate = () => new Date(Date.now() - 5 * 60_000);
function makePet(overrides: Record<string, unknown> = {}) {
return {
id: PET_ID,
clientId: CLIENT_ID,
name: "Biscuit",
species: "dog",
breed: "Golden Retriever",
weightKg: "30.00",
dateOfBirth: null,
healthAlerts: null,
groomingNotes: null,
cutStyle: null,
shampooPreference: null,
specialCareNotes: null,
customFields: {},
petSizeCategory: "large",
coatType: "double",
photoKey: null,
photoUploadedAt: null,
createdAt: new Date("2024-01-01"),
updatedAt: new Date("2024-01-01"),
...overrides,
};
}
function makeAppointment(overrides: Record<string, unknown> = {}) {
return {
id: "appt-1",
clientId: CLIENT_ID,
petId: PET_ID,
serviceId: "service-1",
staffId: GROOMER.id,
batherStaffId: null,
status: "completed",
startTime: new Date("2024-06-01T09:00:00Z"),
endTime: new Date("2024-06-01T11:00:00Z"),
notes: null,
priceCents: 6000,
seriesId: null,
seriesIndex: null,
groupId: null,
confirmationStatus: "confirmed",
confirmedAt: null,
cancelledAt: null,
confirmationToken: null,
customerNotes: null,
createdAt: new Date("2024-05-15"),
updatedAt: new Date("2024-05-15"),
...overrides,
};
}
function makeService(overrides: Record<string, unknown> = {}) {
return {
id: "service-1",
name: "Full Groom",
description: null,
basePriceCents: 6000,
durationMinutes: 120,
active: true,
createdAt: new Date(),
updatedAt: new Date(),
...overrides,
};
}
function makeSession(overrides: Record<string, unknown> = {}) {
return {
id: "sess-owner",
staffId: CUSTOMER_STAFF.id,
clientId: CLIENT_ID,
reason: "sso-bridge",
status: "active",
startedAt: new Date(),
endedAt: null,
expiresAt: futureDate(),
createdAt: new Date(),
...overrides,
};
}
// ─── DB mock state ──────────────────────────────────────────────────────────
let petsTable: Record<string, unknown>[];
let appointmentsTable: Record<string, unknown>[];
let servicesTable: Record<string, unknown>[];
let sessionsTable: Record<string, unknown>[];
// selectQueue: queries resolve in FIFO order. Each .from(table) result
// returns a chain that resolves to the next queued row set on a terminal
// call (.where()/.orderBy()/.limit()).
//
// A queued entry of `{ table: "pets", rows: null, throw: "..." }` tells the
// mock to throw instead of returning rows — used by the GRO-2014 "JSON
// envelope on downstream error" test. Any other queued entry with `rows`
// resolves to those rows. An entry with `rows: []` returns an empty array
// (no rows, no throw).
let selectQueue: Array<{
table: string;
rows: unknown[] | null;
throw?: string;
}> = [];
// Captured `db.insert(table).values(vals)` calls. Mirrors the pattern from
// src/__tests__/impersonation.test.ts so the GRO-2063 audit row assertions
// can inspect what the route tried to write without needing a real DB.
let insertCapture: Array<{ table: string; vals: Record<string, unknown> }> = [];
function enqueue(table: string, rows: unknown[] = []) {
selectQueue.push({ table, rows });
}
function enqueueThrow(table: string, message: string) {
selectQueue.push({ table, rows: null, throw: message });
}
function resetMock() {
petsTable = [makePet()];
appointmentsTable = [makeAppointment()];
servicesTable = [makeService()];
sessionsTable = [makeSession()];
selectQueue = [];
insertCapture = [];
}
// ─── Module mocks ───────────────────────────────────────────────────────────
vi.mock("@groombook/db", () => {
function makeTable(name: string) {
return new Proxy(
{ _name: name },
{
get(target, prop) {
if (prop === "_name") return name;
if (prop === "$inferSelect") return {};
return { table: name, column: prop };
},
}
);
}
function sqlMock(_strings: TemplateStringsArray, ..._params: unknown[]) {
const queryString = _strings[0];
return {
queryChunks: [queryString],
as: (alias: string) => ({
queryChunks: [queryString],
fieldAlias: alias,
getSQL() { return this.queryChunks; },
}),
};
}
function takeQueuedRows(tableName: string): unknown[] {
const next = selectQueue.shift();
if (next && next.table === tableName) {
if (next.throw) {
throw new Error(next.throw);
}
return next.rows ?? [];
}
return [];
}
// Wrap a finalised result in a Proxy that exposes chainable methods
// and the resolved rows. Each call to a chainable method (where/orderBy/
// limit/...) returns the SAME rows so the route's natural await on the
// chain resolves to the queued data.
function wrapRows(rows: unknown[]): unknown {
return new Proxy(rows, {
get(target, prop: string | symbol) {
if (prop === "where" || prop === "orderBy" || prop === "limit"
|| prop === "leftJoin" || prop === "innerJoin" || prop === "from") {
return () => wrapRows(rows);
}
if (prop === "then") {
return (onFulfilled?: (v: unknown) => unknown, onRejected?: (e: unknown) => unknown) =>
Promise.resolve(rows).then(onFulfilled, onRejected);
}
if (prop === Symbol.iterator) {
return function* () { for (const v of target) yield v; };
}
if (prop === Symbol.asyncIterator) {
return async function* () { for (const v of target) yield v; };
}
// @ts-expect-error proxy access
return target[prop];
},
});
}
return {
getDb: () => ({
select: (_cols?: Record<string, unknown>) => ({
from: (table: { _name?: string }) => wrapRows(takeQueuedRows(table._name ?? "")),
}),
insert: (table: { _name?: string }) => ({
values: (vals: Record<string, unknown>) => {
insertCapture.push({ table: table._name ?? "unknown", vals });
return { returning: () => [{}] };
},
}),
update: () => ({ set: () => ({ where: () => ({ returning: () => [{}] }) }) }),
delete: () => ({ where: () => ({ returning: () => [{}] }) }),
}),
pets: makeTable("pets"),
appointments: makeTable("appointments"),
staff: makeTable("staff"),
services: makeTable("services"),
impersonationSessions: makeTable("impersonationSessions"),
impersonationAuditLogs: makeTable("impersonation_audit_logs"),
and: vi.fn((..._args: unknown[]) => ({})),
desc: vi.fn((c: unknown) => c),
eq: vi.fn((_a: unknown, _b: unknown) => ({})),
exists: vi.fn(() => true),
or: vi.fn((..._args: unknown[]) => ({})),
sql: sqlMock,
};
});
vi.mock("../lib/s3.js", () => ({
getPresignedUploadUrl: vi.fn(),
getPresignedGetUrl: vi.fn(),
deleteObject: vi.fn(),
}));
// ─── Import after mocks are set up ──────────────────────────────────────────
const { petsRouter } = await import("../routes/pets.js");
// ─── App builder ────────────────────────────────────────────────────────────
function buildApp(staffRow: StaffRow | null) {
const app = new Hono<AppEnv>();
app.use("*", async (c, next) => {
if (staffRow) {
c.set("jwtPayload", { sub: staffRow.oidcSub ?? staffRow.userId ?? "" });
c.set("staff", staffRow);
}
await next();
});
app.route("/pets", petsRouter);
return app;
}
// ─── Reset before each test ─────────────────────────────────────────────────
beforeEach(() => {
resetMock();
vi.clearAllMocks();
});
// ─── GRO-2014 error-handling suite ──────────────────────────────────────────
describe("GET /:id/profile-summary — GRO-2014 error handling", () => {
it("returns 404 (not 500) for a malformed UUID path param", async () => {
const app = buildApp(MANAGER);
const res = await app.request("/pets/not-a-uuid/profile-summary");
expect(res.status).toBe(404);
const body = (await res.json()) as { error: string };
expect(body.error).toBe("Not found");
});
it("returns 401 when staff context is missing (defense in depth)", async () => {
const app = buildApp(null);
const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`);
expect(res.status).toBe(401);
const body = (await res.json()) as { error: string };
expect(body.error).toBe("Unauthorized");
});
it("returns 404 when authenticated and pet does not exist", async () => {
enqueue("pets", []);
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`);
expect(res.status).toBe(404);
const body = (await res.json()) as { error: string };
expect(body.error).toBe("Not found");
});
it("returns 403 when groomer has no appointment linkage to the pet's client", async () => {
enqueue("pets", petsTable);
enqueue("appointments", []); // linkage check returns empty → 403
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(403);
const body = (await res.json()) as { error: string };
expect(body.error).toBe("Forbidden");
});
it("returns 200 with summary for a manager (no groomer linkage check)", async () => {
enqueue("pets", petsTable);
enqueue("appointments", appointmentsTable); // history
enqueue("appointments", [{ count: 1 }]); // visit count
enqueue("appointments", []); // upcoming (none)
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
const body = (await res.json()) as Record<string, unknown>;
expect(body.id).toBe(PET_ID);
expect(body.name).toBe("Biscuit");
expect(body.visitCount).toBe(1);
expect(body.upcomingAppointment).toBeNull();
expect(body.recentGroomingHistory).toBeInstanceOf(Array);
});
it("returns 200 with summary for a groomer with appointment linkage", async () => {
enqueue("pets", petsTable);
enqueue("appointments", [{ id: "appt-1" }]); // linkage found
enqueue("appointments", appointmentsTable); // history
enqueue("appointments", [{ count: 1 }]); // visit count
enqueue("appointments", []); // upcoming
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
const body = (await res.json()) as Record<string, unknown>;
expect(body.id).toBe(PET_ID);
});
it("returns a JSON envelope (not empty body) when a downstream query throws", async () => {
enqueueThrow("pets", "simulated postgres uuid cast failure");
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(500);
const body = (await res.json()) as { error: string };
expect(body.error).toBe("Internal Server Error");
});
});
// ─── GRO-2013 owner-bypass suite ────────────────────────────────────────────
describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
it("returns 404 when the pet does not exist", async () => {
enqueue("pets", []);
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(404);
});
it("returns 200 with aggregated profile for a manager", async () => {
enqueue("pets", petsTable);
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
const body = await res.json();
expect(body.id).toBe(PET_ID);
expect(body.name).toBe("Biscuit");
expect(body.recentGroomingHistory).toBeInstanceOf(Array);
expect(body.visitCount).toBe(1);
expect(body.upcomingAppointment).toBeNull();
});
it("returns 200 for a groomer with appointment linkage to the pet's client", async () => {
enqueue("pets", petsTable);
enqueue("appointments", [{ id: "appt-1" }]); // linkage found
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
});
it("returns 403 for a groomer with no appointment linkage and no bypass header", async () => {
enqueue("pets", petsTable);
enqueue("appointments", []); // no linkage
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(403);
});
it("customer-as-groomer with valid active session for pet's client returns 200", async () => {
enqueue("pets", petsTable);
enqueue("impersonationSessions", sessionsTable); // active session found
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-owner" },
});
expect(res.status).toBe(200);
const body = await res.json();
expect(body.id).toBe(PET_ID);
});
it("customer-as-groomer with no header still gets 403 (no bypass)", async () => {
enqueue("pets", petsTable);
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(403);
});
it("customer-as-groomer with session for a DIFFERENT client gets 403 (cross-tenant blocked)", async () => {
// Session exists but clientId !== pet.clientId → bypass does not apply
// → falls through to groomer linkage check → no linkage → 403
enqueue("pets", petsTable);
enqueue("impersonationSessions", [
makeSession({
id: "sess-other-client",
clientId: "c0000000-0000-0000-0000-000000000099", // different from CLIENT_ID
}),
]);
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-other-client" },
});
expect(res.status).toBe(403);
});
it("customer-as-groomer with expired session still gets 403", async () => {
enqueue("pets", petsTable);
enqueue("impersonationSessions", [
makeSession({ id: "sess-expired", expiresAt: pastDate() }),
]);
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-expired" },
});
expect(res.status).toBe(403);
});
it("customer-as-groomer with ended (status != active) session still gets 403", async () => {
enqueue("pets", petsTable);
enqueue("impersonationSessions", [
makeSession({ id: "sess-ended", status: "ended" }),
]);
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-ended" },
});
expect(res.status).toBe(403);
});
it("customer-as-groomer with unknown session id still gets 403", async () => {
enqueue("pets", petsTable);
enqueue("impersonationSessions", []); // session not found
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-unknown" },
});
expect(res.status).toBe(403);
});
it("manager does NOT need the impersonation header (existing role check still works)", async () => {
enqueue("pets", petsTable);
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(MANAGER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
});
it("groomer with linkage to pet's client still works (regression — no regression from bypass)", async () => {
enqueue("pets", petsTable);
enqueue("appointments", [{ id: "appt-1" }]); // linkage found
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
});
it("owner-bypass: customer cannot view another client's pet (cross-tenant block)", async () => {
// The customer has a valid session for CLIENT_ID, but the pet belongs
// to a different client → isOwner=false → falls through to groomer
// linkage check → 403.
enqueue("pets", [
makePet({ id: OTHER_CLIENT_PET_ID, clientId: "c0000002-0000-0000-0000-000000000002" }),
]);
enqueue("impersonationSessions", sessionsTable); // valid session, but for CLIENT_ID
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${OTHER_CLIENT_PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-owner" },
});
expect(res.status).toBe(403);
});
});
// ─── GRO-2063 owner-bypass audit write ──────────────────────────────────────
describe("GET /:id/profile-summary — owner-bypass audit row (GRO-2063)", () => {
it("writes exactly one audit row on the owner-bypass success path", async () => {
enqueue("pets", petsTable);
enqueue("impersonationSessions", sessionsTable); // valid active session for CLIENT_ID
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-owner" },
});
expect(res.status).toBe(200);
const auditInserts = insertCapture.filter((c) => c.table === "impersonation_audit_logs");
expect(auditInserts).toHaveLength(1);
const vals = auditInserts[0]!.vals;
expect(vals.action).toBe("read_profile_summary");
expect(vals.sessionId).toBe("sess-owner");
expect(vals.pageVisited).toBe(`/pets/${PET_ID}/profile-summary`);
expect(vals.metadata).toEqual({
petId: PET_ID,
actorStaffId: CUSTOMER_STAFF.id,
});
});
it("does NOT write an audit row on the normal groomer-linkage success path", async () => {
// GROOMER is a "real" groomer with appointment linkage, NOT the
// auto-provisioned customer-as-groomer. No impersonation header is
// present, so the owner-bypass branch never executes.
enqueue("pets", petsTable);
enqueue("appointments", [{ id: "appt-1" }]); // linkage found
enqueue("appointments", appointmentsTable);
enqueue("appointments", [{ count: 1 }]);
enqueue("appointments", []);
const app = buildApp(GROOMER);
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
expect(res.status).toBe(200);
const auditInserts = insertCapture.filter((c) => c.table === "impersonation_audit_logs");
expect(auditInserts).toHaveLength(0);
});
it("does NOT write an audit row when the owner-bypass attempt is denied (cross-tenant)", async () => {
// Customer has a valid session but it points at a different client.
// isOwner=false, falls through to groomer linkage check, returns 403.
enqueue("pets", [
makePet({ id: OTHER_CLIENT_PET_ID, clientId: "c0000002-0000-0000-0000-000000000002" }),
]);
enqueue("impersonationSessions", sessionsTable); // session is for CLIENT_ID
enqueue("appointments", []); // no linkage → 403
const app = buildApp(CUSTOMER_STAFF);
const res = await app.request(`/pets/${OTHER_CLIENT_PET_ID}/profile-summary`, {
headers: { "X-Impersonation-Session-Id": "sess-owner" },
});
expect(res.status).toBe(403);
const auditInserts = insertCapture.filter((c) => c.table === "impersonation_audit_logs");
expect(auditInserts).toHaveLength(0);
});
});
+214 -25
View File
@@ -43,42 +43,103 @@ const GROOMER: StaffRow = {
// ─── Mock DB ──────────────────────────────────────────────────────────────────
// staffLookupResult drives every `from(staff)` query that doesn't go through
// the dev-mode `.limit()` shortcut. Tests that want to simulate "no staff row"
// leave it null.
let staffLookupResult: StaffRow | null = null;
// managerFallbackResult is only consumed by the dev-mode `from(staff).limit(1)`
// path (looking up the first manager when AUTH_DISABLED=true and no header).
let managerFallbackResult: StaffRow | null = MANAGER;
// userLookupResult drives `from(user).limit(1)` for the Better-Auth user
// auto-provision branch (GRO-2052). Tests that simulate "no Better-Auth user"
// leave it null.
type UserRow = { id: string; name: string | null; email: string | null };
let userLookupResult: UserRow | null = null;
// accountLookupResult drives `from(account).limit(1)` for the legacy OIDC
// auto-provision branch. Null means "no OIDC account row".
let accountLookupResult: { id: string } | null = null;
// insertReturningResult drives `insert(staff).values(...).returning()` for
// any auto-provision branch that actually creates a staff record. Null means
// the INSERT returned no rows (simulating a DB failure).
let insertReturningResult: StaffRow | null = null;
vi.mock("@groombook/db", () => {
const staff = new Proxy(
{ _name: "staff" },
{
get(target, prop) {
if (prop === "_name") return "staff";
if (prop === "$inferSelect") return {};
return { table: "staff", column: prop };
},
}
);
function tableMarker(name: string) {
return new Proxy(
{ _name: name },
{
get(_target, prop) {
if (prop === "_name") return name;
if (prop === "$inferSelect") return {};
return { table: name, column: prop };
},
}
);
}
const staff = tableMarker("staff");
const user = tableMarker("user");
const account = tableMarker("account");
function lookupFor(tableName: string) {
if (tableName === "user") return userLookupResult;
if (tableName === "account") return accountLookupResult;
return staffLookupResult;
}
return {
getDb: () => ({
select: () => ({
from: () => ({
where: () => ({
limit: () => {
// dev mode fallback to first manager
return managerFallbackResult ? [managerFallbackResult] : [];
},
[Symbol.iterator]: function* () {
if (staffLookupResult) yield staffLookupResult;
},
0: staffLookupResult,
length: staffLookupResult ? 1 : 0,
}),
select: (_columns?: unknown) => ({
from: (table: { _name?: string }) => {
const name = table?._name ?? "staff";
return {
where: () => ({
limit: () => {
// The user / account auto-provision branches always call
// `.limit(1)`; route to the per-table lookup state.
if (name === "user")
return userLookupResult ? [userLookupResult] : [];
if (name === "account")
return accountLookupResult ? [accountLookupResult] : [];
// dev-mode `from(staff).limit(1)` falls back to the first
// manager when AUTH_DISABLED is set with no header.
return managerFallbackResult ? [managerFallbackResult] : [];
},
[Symbol.iterator]: function* () {
const row = lookupFor(name);
if (row) yield row;
},
0: lookupFor(name),
length: lookupFor(name) ? 1 : 0,
}),
};
},
}),
insert: (_table: unknown) => ({
values: (_v: unknown) => ({
returning: () =>
insertReturningResult ? [insertReturningResult] : [],
}),
}),
update: (_table: unknown) => ({
set: (_v: unknown) => ({
where: () => Promise.resolve(undefined),
}),
}),
}),
staff,
user,
account,
eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })),
and: vi.fn((..._clauses: unknown[]) => ({})),
sql: Object.assign(
vi.fn((..._tpl: unknown[]) => ({})),
{ raw: vi.fn(() => ({})) }
),
};
});
@@ -87,16 +148,25 @@ vi.mock("@groombook/db", () => {
function resetMocks() {
staffLookupResult = null;
managerFallbackResult = MANAGER;
userLookupResult = null;
accountLookupResult = null;
insertReturningResult = null;
}
/** Build a minimal Hono app with jwtPayload pre-set, then apply a middleware. */
function buildApp(
middleware: MiddlewareHandler<AppEnv>,
handler?: (c: Context<AppEnv>) => Response | Promise<Response>
handler?: (c: Context<AppEnv>) => Response | Promise<Response>,
jwtOverride?: Partial<{ sub: string; email: string; name: string }>
) {
const app = new Hono<AppEnv>();
app.use("*", async (c, next) => {
c.set("jwtPayload", { sub: staffLookupResult?.userId ?? "unknown-sub" });
const defaultSub = staffLookupResult?.userId ?? "unknown-sub";
c.set("jwtPayload", {
sub: jwtOverride?.sub ?? defaultSub,
...(jwtOverride?.email !== undefined ? { email: jwtOverride.email } : {}),
...(jwtOverride?.name !== undefined ? { name: jwtOverride.name } : {}),
});
await next();
});
app.use("*", middleware);
@@ -204,6 +274,125 @@ describe("resolveStaffMiddleware", () => {
});
});
// ─── Auto-provision branches (GRO-2052) ───────────────────────────────────────
//
// Each branch creates a staff row on first authenticated request when no row
// exists yet. The Better-Auth branch (user table) is the primary path for
// email/password customers; the OIDC branch (account table) is a fallback for
// legacy authentik/google/github sessions.
describe("resolveStaffMiddleware — auto-provision", () => {
const PROVISIONED: StaffRow = {
...MANAGER,
id: "staff-provisioned-id",
oidcSub: null,
userId: "ba-user-customer",
role: "groomer",
isSuperUser: false,
name: "UAT Customer",
email: "uat-customer@groombook.dev",
};
it("Better-Auth: creates a groomer staff row when user exists but no staff record (GRO-2052)", async () => {
// No existing staff row, no OIDC account row, but a Better-Auth user row.
staffLookupResult = null;
userLookupResult = {
id: "ba-user-customer",
name: "UAT Customer",
email: "uat-customer@groombook.dev",
};
accountLookupResult = null;
insertReturningResult = PROVISIONED;
let capturedStaff: StaffRow | null = null;
const app = buildApp(
resolveStaffMiddleware,
(c) => {
capturedStaff = c.get("staff");
return c.json({ ok: true });
},
{ sub: "ba-user-customer", email: "uat-customer@groombook.dev" }
);
const res = await app.request("/test");
expect(res.status).toBe(200);
expect(capturedStaff).not.toBeNull();
expect(capturedStaff!.role).toBe("groomer");
expect(capturedStaff!.userId).toBe("ba-user-customer");
});
it("Better-Auth: returns 500 if INSERT yields no row", async () => {
staffLookupResult = null;
userLookupResult = {
id: "ba-user-customer",
name: "UAT Customer",
email: "uat-customer@groombook.dev",
};
insertReturningResult = null; // simulate INSERT … RETURNING returning []
const app = buildApp(resolveStaffMiddleware, undefined, {
sub: "ba-user-customer",
email: "uat-customer@groombook.dev",
});
const res = await app.request("/test");
expect(res.status).toBe(500);
const body = await res.json();
expect(body.error).toMatch(/auto-provision failed/i);
});
it("Better-Auth branch runs before OIDC branch (does not require jwt.email)", async () => {
// A Better-Auth user row alone is sufficient: jwt.email is intentionally
// absent. The pre-GRO-2052 code only auto-provisioned inside `if (jwt.email)`.
staffLookupResult = null;
userLookupResult = {
id: "ba-user-customer",
name: "UAT Customer",
email: "uat-customer@groombook.dev",
};
insertReturningResult = PROVISIONED;
const app = buildApp(resolveStaffMiddleware, undefined, {
sub: "ba-user-customer",
});
const res = await app.request("/test");
expect(res.status).toBe(200);
});
it("OIDC fallback: still provisions when user row is missing but account row exists", async () => {
// No staff row, no Better-Auth user, but an OIDC account row.
staffLookupResult = null;
userLookupResult = null;
accountLookupResult = { id: "oidc-account-id" };
insertReturningResult = { ...PROVISIONED, userId: "oidc-sub" };
const app = buildApp(resolveStaffMiddleware, undefined, {
sub: "oidc-sub",
email: "oidc-user@example.com",
});
const res = await app.request("/test");
expect(res.status).toBe(200);
});
it("falls through to 403 when neither Better-Auth user nor OIDC account row exists", async () => {
staffLookupResult = null;
userLookupResult = null;
accountLookupResult = null;
const app = buildApp(resolveStaffMiddleware, undefined, {
sub: "ghost-sub",
email: "ghost@example.com",
});
const res = await app.request("/test");
expect(res.status).toBe(403);
const body = await res.json();
expect(body.error).toMatch(/no staff record/i);
});
});
// ─── requireRole tests ────────────────────────────────────────────────────────
describe("requireRole", () => {
+43 -2
View File
@@ -1,5 +1,5 @@
import type { MiddlewareHandler } from "hono";
import { and, eq, getDb, sql, staff, account } from "@groombook/db";
import { and, eq, getDb, sql, staff, account, user } from "@groombook/db";
export type StaffRole = "groomer" | "receptionist" | "manager";
export type StaffRow = typeof staff.$inferSelect;
@@ -111,8 +111,49 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = async (
}
}
// Auto-provision for Better-Auth users (GRO-2052): the user signed in via
// Better-Auth (email/password, magic link, etc.), so a row exists in `user`
// for jwt.sub but no `account` provider row is required. Create a minimal
// groomer staff record on first login. This is the primary auto-provision
// path; the OIDC branch below remains as a fallback for legacy accounts
// that exist in `account` but not in `user`.
const [userRow] = await db
.select({ id: user.id, name: user.name, email: user.email })
.from(user)
.where(eq(user.id, jwt.sub))
.limit(1);
if (userRow) {
const emailPrefix = userRow.email ? userRow.email.split("@")[0] : "Unknown";
const name = userRow.name?.trim() || jwt.name?.trim() || emailPrefix;
const [newStaff] = await db
.insert(staff)
.values({
userId: jwt.sub,
email: userRow.email ?? jwt.email ?? "",
name,
role: "groomer",
isSuperUser: false,
active: true,
} as Parameters<typeof db.insert>[0] extends { values: infer V } ? V : never)
.returning()!;
if (!newStaff) {
return c.json({ error: "Forbidden: auto-provision failed" }, 500);
}
console.log(
`[rbac] auto-provisioned staff record for Better-Auth user: ${jwt.sub} -> staff:${newStaff.id} (${name})`
);
c.set("staff", newStaff);
await next();
return;
}
// Auto-provision for OIDC users: check if jwt.sub has an OAuth/OIDC account
// (e.g. authentik). If so, create a groomer staff record on the fly.
// (e.g. authentik). If so, create a groomer staff record on the fly. This
// is kept for backward compatibility with legacy OIDC sessions whose user
// row may not yet exist in the Better-Auth `user` table.
if (jwt.email) {
const [oidcAccount] = await db
.select({ id: account.id })
+124 -2
View File
@@ -7,6 +7,8 @@ import {
eq,
exists,
getDb,
impersonationAuditLogs,
impersonationSessions,
or,
pets,
appointments,
@@ -23,6 +25,23 @@ import {
export const petsRouter = new Hono<AppEnv>();
// Convert Zod validation errors from 422 to 400 and ensure any thrown error
// returns a structured JSON body rather than Hono's default empty-body 500.
// GRO-2014: profile-summary previously bubbled unhandled errors and produced
// an empty-body 500. Mirror the onError pattern already used in invoices.ts
// and reports.ts so every error has a JSON envelope.
petsRouter.onError((err, c) => {
if (err instanceof z.ZodError) {
return c.json({ error: "Validation failed", issues: err.issues }, 400);
}
console.error("[pets] unhandled error", err);
return c.json({ error: "Internal Server Error" }, 500);
});
// UUID format used by all pet routes — guards path params against malformed
// values before they hit Drizzle / Postgres uuid columns (which would throw).
const uuidSchema = z.string().uuid();
const createPetSchema = z.object({
clientId: z.string().uuid(),
name: z.string().min(1).max(200),
@@ -109,18 +128,121 @@ petsRouter.get("/:id", async (c) => {
return c.json(row);
});
/**
* Resolves the clientId from the X-Impersonation-Session-Id header, if present and active.
* Used by staff routes to allow a customer (auto-provisioned as a `groomer` staff row
* by rbac.ts) to access their own pet's data when they are the rightful owner.
*
* Returns null when the header is missing, the session is unknown/expired/ended, or the
* session exists but has no clientId — callers should treat null as "no owner-bypass".
*/
async function resolveImpersonationClientId(
db: ReturnType<typeof getDb>,
c: { req: { header: (name: string) => string | undefined } }
): Promise<string | null> {
const sessionId = c.req.header("X-Impersonation-Session-Id");
if (!sessionId) return null;
const [session] = await db
.select({
clientId: impersonationSessions.clientId,
status: impersonationSessions.status,
expiresAt: impersonationSessions.expiresAt,
})
.from(impersonationSessions)
.where(eq(impersonationSessions.id, sessionId))
.limit(1);
if (!session) return null;
if (session.status !== "active") return null;
if (session.expiresAt <= new Date()) return null;
return session.clientId;
}
/**
* Defense-in-depth audit write for the staff-side owner-bypass path in
* GET /pets/:id/profile-summary. Mirrors the failure-isolation pattern in
* src/middleware/portalAudit.ts: errors are logged but never thrown, so a
* misbehaving audit insert cannot turn a working read into a 500.
*
* Called only when the owner-bypass actually fires (i.e. the requester is a
* groomer-role staff row with no appointment linkage, but supplies a valid
* X-Impersonation-Session-Id whose clientId matches the pet's owner). The
* `petId` and `actorStaffId` are written inside `metadata` because the
* impersonation_audit_logs schema has no first-class columns for them and
* adding a migration is out of scope.
*/
async function writeOwnerBypassAudit(
db: ReturnType<typeof getDb>,
args: {
sessionId: string;
petId: string;
actorStaffId: string;
pageVisited: string;
}
): Promise<void> {
try {
await db.insert(impersonationAuditLogs).values({
sessionId: args.sessionId,
action: "read_profile_summary",
pageVisited: args.pageVisited,
metadata: { petId: args.petId, actorStaffId: args.actorStaffId },
});
} catch (err) {
console.error("[pets] failed to write owner-bypass audit log:", err);
}
}
petsRouter.get("/:id/profile-summary", async (c) => {
const db = getDb();
const petId = c.req.param("id");
// GRO-2014: validate UUID format before hitting Postgres. Passing a non-UUID
// string to a uuid column makes the driver throw, which previously surfaced
// as an empty-body 500 to clients.
const parsedId = uuidSchema.safeParse(petId);
if (!parsedId.success) {
return c.json({ error: "Not found" }, 404);
}
// Defense in depth: resolveStaffMiddleware should always populate `staff`
// for protected routes (or short-circuit with 401/403 of its own). Guard
// anyway so a misconfigured route mount can't trigger a TypeError on
// staffRow.id when the linkage check runs.
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
if (!staffRow) {
return c.json({ error: "Unauthorized" }, 401);
}
const isGroomer = staffRow.role === "groomer";
// Fetch the pet
const [pet] = await db.select().from(pets).where(eq(pets.id, petId));
if (!pet) return c.json({ error: "Not found" }, 404);
// Groomer RBAC: check appointment linkage to this pet's client
// Owner-bypass (GRO-2013): a customer who supplies a valid
// X-Impersonation-Session-Id for the pet's owning client may read their
// own pet's summary, even though rbac.ts auto-provisions them as a
// `groomer` staff row with no appointment linkage.
let isOwner = false;
if (isGroomer) {
const headerSessionId = c.req.header("X-Impersonation-Session-Id");
const ownerClientId = await resolveImpersonationClientId(db, c);
isOwner = !!ownerClientId && ownerClientId === pet.clientId;
if (isOwner && headerSessionId) {
// GRO-2063: defense-in-depth audit row. Only fires when the bypass
// is actually granted; never on the normal groomer-linkage path,
// 403/404/401, or when the header is absent. Failure is swallowed
// (try/catch inside writeOwnerBypassAudit) so this can never turn a
// working read into a 500.
await writeOwnerBypassAudit(db, {
sessionId: headerSessionId,
petId: pet.id,
actorStaffId: staffRow.id,
pageVisited: c.req.path,
});
}
}
// Groomer RBAC: check appointment linkage to this pet's client
if (isGroomer && !isOwner) {
const [linkage] = await db
.select({ id: appointments.id })
.from(appointments)