Compare commits

..

15 Commits

Author SHA1 Message Date
The Dogfather 16c959434b promote(uat): GRO-2013 owner-bypass + GRO-2033 idempotent migrations (dev→uat)
CI / Test (pull_request) Successful in 11s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 41s
Merge dev into uat. Resolves test-file/playbook conflicts created by PR #138's
squash merge by taking dev's superset versions (verified: all GRO-2014 tests +
TC ids preserved, plus GRO-2013 additions). No-ff merge so dev becomes an
ancestor of uat, preventing future squash-divergence conflicts.

Carries:
- GRO-2013 deployed-tree owner-bypass (src/routes/pets.ts, reconciled 20-test file)
- GRO-2033 idempotent migrations 0039/0040

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-01 20:10:51 +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
The Dogfather 23484dc90a promote(uat): GRO-2014 profile-summary error-handling fix (dev→uat) (#138)
CI / Test (push) Successful in 10s
CI / Lint & Typecheck (push) Successful in 16s
CI / Build & Push Docker Images (push) Successful in 39s
2026-06-01 18:27:42 +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
The Dogfather 6a81a52a50 Merge pull request 'Promote dev → uat: UAT seed-password source-of-truth playbook (GRO-2000)' (#134) from dev into uat
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 27s
CI / Test (pull_request) Successful in 11s
CI / Lint & Typecheck (pull_request) Successful in 13s
CI / Build & Push Docker Images (pull_request) Successful in 1m10s
2026-06-01 17:41:47 +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
The Dogfather 5a4b9a98bd Merge pull request 'promote(docker): bake pnpm via npm to remove Corepack runtime downloads (GRO-1981)' (#133) from dev into uat
CI / Test (push) Successful in 12s
CI / Lint & Typecheck (push) Successful in 14s
CI / Build & Push Docker Images (push) Successful in 40s
Promote GRO-1985 (parent GRO-1981) dev->uat. cc @cpfarhood
2026-06-01 16:30:54 +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 f7f88156e1 Merge pull request 'promote(db): register extra_large via migration 0038 to UAT (GRO-2004)' (#131) from dev into uat
CI / Test (push) Successful in 11s
CI / Lint & Typecheck (push) Successful in 15s
CI / Build & Push Docker Images (push) Successful in 35s
2026-06-01 14:52:13 +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
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
The Dogfather 8af5a49d14 Merge pull request 'Promote dev→uat: GRO-1982 pet_size_category extra_large enum migration' (#126) from dev into uat
CI / Test (push) Successful in 13s
CI / Lint & Typecheck (push) Successful in 16s
CI / Build & Push Docker Images (push) Successful in 37s
Promote dev→uat: GRO-1983 seed-job pnpm fix + GRO-1982 extra_large enum migration

Carries the accumulated dev state into uat (PR #125 docker pnpm fix + 0037 migration).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-01 12:44:20 +00:00
10 changed files with 911 additions and 4 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"]
+6
View File
@@ -125,6 +125,12 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
| 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.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)
@@ -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);
});
});
+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,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';
+14
View File
@@ -267,6 +267,20 @@
"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
}
]
}
+569
View File
@@ -0,0 +1,569 @@
/**
* 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;
}> = [];
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 = [];
}
// ─── 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: () => ({ values: () => ({ returning: () => [{}] }) }),
update: () => ({ set: () => ({ where: () => ({ returning: () => [{}] }) }) }),
delete: () => ({ where: () => ({ returning: () => [{}] }) }),
}),
pets: makeTable("pets"),
appointments: makeTable("appointments"),
staff: makeTable("staff"),
services: makeTable("services"),
impersonationSessions: makeTable("impersonationSessions"),
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);
});
});
+75 -2
View File
@@ -7,6 +7,7 @@ import {
eq,
exists,
getDb,
impersonationSessions,
or,
pets,
appointments,
@@ -23,6 +24,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 +127,73 @@ 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;
}
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 ownerClientId = await resolveImpersonationClientId(db, c);
isOwner = !!ownerClientId && ownerClientId === pet.clientId;
}
// Groomer RBAC: check appointment linkage to this pet's client
if (isGroomer && !isOwner) {
const [linkage] = await db
.select({ id: appointments.id })
.from(appointments)