Compare commits

..

1 Commits

Author SHA1 Message Date
Flea Flicker 707e74e6e1 fix: add missing 'short'/'medium'/'silky' to coat_type enum (GRO-1971)
CI / Test (pull_request) Successful in 12s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 1m2s
Migration 0031_buffer_rules.sql created coat_type enum with values
('smooth', 'double', 'wire', 'curly', 'long', 'hairless') but omitted
'short', 'medium', and 'silky' which are defined in schema.ts and
required by seed.ts.

Added 0035_add_missing_coat_type_values.sql to add the missing values
using ALTER TYPE ... ADD VALUE IF NOT EXISTS.

This resolves the UAT seed failure:
  PostgresError: invalid input value for enum coat_type: "short"
  code: '22P02' routine: 'enum_in' (packages/db/src/seed.ts)

Dev→UAT SDLC: QA (Lint Roller) review, then CTO review.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-05-31 23:08:38 +00:00
14 changed files with 35 additions and 1034 deletions
-29
View File
@@ -156,32 +156,3 @@ 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 ✓"
+9 -19
View File
@@ -1,14 +1,7 @@
FROM node:22-alpine AS base
# Install pnpm as a real binary via npm (not corepack shim) so runtime
# 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 / 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
RUN corepack enable && corepack install -g pnpm@9.15.4
ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0
ENV COREPACK_ENABLE_STRICT=0
WORKDIR /app
# Install deps
@@ -29,9 +22,9 @@ 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
RUN corepack enable && corepack install -g pnpm@9.15.4
ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0
ENV COREPACK_ENABLE_STRICT=0
WORKDIR /app
ENV NODE_ENV=production
@@ -52,18 +45,15 @@ 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
RUN corepack enable && corepack install -g pnpm@9.15.4
ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0
ENV COREPACK_ENABLE_STRICT=0
CMD ["pnpm", "--filter", "@groombook/db", "reset"]
-30
View File
@@ -19,27 +19,6 @@ 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.
## Test Cases
### 4.0 Health Check
@@ -62,8 +41,6 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
| TC-API-1.8 | Email+password — invalid password | POST /api/auth/sign-in/email with wrong password | 400 Bad Request, error returned |
| TC-API-1.9 | Email+password — unknown user | POST /api/auth/sign-in/email with non-existent email | 400 Bad Request, error returned |
| TC-API-1.10 | Auto-provision on first OIDC login | First login as a Better-Auth user with no existing staff record | 200 OK, access granted; groomer staff record auto-created with name/email from user table |
> **Note (GRO-1977):** Seed credential provisioning is idempotent — re-running the seed with updated `SEED_UAT_*_PASSWORD` env vars rotates stored credential hashes. TC-API-1.4 through TC-API-1.7 now return 200 for all 4 UAT personas (previously returned 401 due to frozen-hash bug).
| TC-API-1.11 | Existing staff unaffected by OIDC login | Login as uat-groomer@groombook.dev (email+password), then GET /api/staff to find that record | 200 OK, staff record unchanged — no duplicate created, original role and isSuperUser preserved |
| TC-API-1.12 | Auto-provisioned role and superUser flags | After TC-API-1.10, GET /api/staff and inspect the auto-created record | role = "groomer", isSuperUser = false, active = true |
| TC-API-1.13 | Name fallback — user.name present | Auto-provision where Better-Auth user has name set | Staff name = user.name value from user table |
@@ -125,12 +102,6 @@ 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)
@@ -146,7 +117,6 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
| TC-API-3.25 | Verify 30+ total pets in UAT DB | GET /api/pets then count total | 30+ pets returned (UAT seed creates 500 random-pool + 5 UAT test clients + 2 UAT customer = 507 total) |
| 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) |
### 4.4 Appointment Scheduling
@@ -44,7 +44,6 @@ interface MockState {
groomingLogs: Record<string, unknown>[];
staffMembers: Record<string, unknown>[];
services: Record<string, unknown>[];
impersonationSessions: Record<string, unknown>[];
}
let mock: MockState;
@@ -169,19 +168,6 @@ 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"),
},
],
};
}
@@ -191,7 +177,6 @@ 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>> = {};
@@ -263,7 +248,6 @@ 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([]);
},
};
@@ -277,7 +261,6 @@ 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 })),
@@ -416,102 +399,4 @@ 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);
});
});
@@ -67,7 +67,6 @@ let dbAccounts: AccountRow[] = [];
let dbStaff: StaffRow[] = [];
let insertedUsers: UserRow[] = [];
let insertedAccounts: AccountRow[] = [];
let updatedAccounts: Array<{ id: string; password: string }> = [];
let updatedStaff: Array<{ id: string; userId: string }> = [];
const originalEnv = { ...process.env };
@@ -78,7 +77,6 @@ function resetMock() {
dbStaff = [];
insertedUsers = [];
insertedAccounts = [];
updatedAccounts = [];
updatedStaff = [];
process.env = { ...originalEnv };
}
@@ -175,11 +173,7 @@ async function seedUatCredentials(
);
if (existingAccount) {
// Idempotent update: re-hash the current env password and update the stored hash.
const { hashPassword } = await import("better-auth/crypto");
const passwordHash = await hashPassword(password);
existingAccount.password = passwordHash;
updatedAccounts.push({ id: existingAccount.id, password: passwordHash });
// skip — already has credential account
} else {
// Use Better-Auth's hashPassword so test helper matches production seed.ts
const { hashPassword } = await import("better-auth/crypto");
@@ -318,9 +312,9 @@ describe("seedUatCredentials — credential provisioning logic", () => {
expect(updatedStaff).toHaveLength(0);
});
// ── AC-5: idempotent — does not insert duplicate records ───────────────────
// ── AC-5: idempotent — skips when user already exists ───────────────────────
it("AC-5: re-running does not insert duplicate user or account records", async () => {
it("AC-5: re-running does not duplicate user or account records (idempotent)", async () => {
process.env.SEED_UAT_CUSTOMER_PASSWORD = TEST_PASSWORD;
const preExistingUsers: UserRow[] = [
@@ -336,96 +330,25 @@ describe("seedUatCredentials — credential provisioning logic", () => {
},
];
// First call — nothing inserted (user + account pre-exist)
await seedUatCredentials([UAT_ACCOUNTS[2]!], {
users: preExistingUsers,
accounts: preExistingAccounts,
staff: [],
});
// No inserts — user and account already exist
expect(insertedUsers).toHaveLength(0);
expect(insertedAccounts).toHaveLength(0);
});
// ── AC-5b: password rotation on re-seed ─────────────────────────────────────
it("AC-5b: re-running with a new password updates the stored credential hash", async () => {
const OLD_PASSWORD = "old-password-abc";
const NEW_PASSWORD = "new-password-xyz";
process.env.SEED_UAT_CUSTOMER_PASSWORD = NEW_PASSWORD;
const preExistingUsers: UserRow[] = [
{ id: "pre-existing-user", email: "uat-customer@groombook.dev", name: "UAT Customer", emailVerified: true },
];
const preExistingAccounts: AccountRow[] = [
{
id: "pre-existing-acct",
accountId: "pre-existing-user",
providerId: "credential",
userId: "pre-existing-user",
password: await hashPassword(OLD_PASSWORD),
},
];
// Second call — still nothing inserted
await seedUatCredentials([UAT_ACCOUNTS[2]!], {
users: preExistingUsers,
accounts: preExistingAccounts,
staff: [],
});
// No new records inserted
expect(insertedUsers).toHaveLength(0);
expect(insertedAccounts).toHaveLength(0);
// Password WAS updated to the new env value
expect(updatedAccounts).toHaveLength(1);
expect(updatedAccounts[0]!.id).toBe("pre-existing-acct");
// New hash is valid Better-Auth format (salt:key, each hex)
const newHashParts = updatedAccounts[0]!.password.split(":");
expect(Buffer.from(newHashParts[0]!, "hex")).toHaveLength(16);
expect(Buffer.from(newHashParts[1]!, "hex")).toHaveLength(64);
});
// ── AC-8: existing account password IS updated (not frozen at first-seed) ──
it("AC-8: re-seeding with a changed password env var updates the stored hash", async () => {
const ORIGINAL_PASSWORD = "original-password";
const ROTATED_PASSWORD = "rotated-password-456";
process.env.SEED_UAT_CUSTOMER_PASSWORD = ROTATED_PASSWORD;
const preExistingUsers: UserRow[] = [
{ id: "pre-existing-user", email: "uat-customer@groombook.dev", name: "UAT Customer", emailVerified: true },
];
// Account was created with the original password on first seed
const originalHash = await hashPassword(ORIGINAL_PASSWORD);
const preExistingAccounts: AccountRow[] = [
{
id: "pre-existing-acct",
accountId: "pre-existing-user",
providerId: "credential",
userId: "pre-existing-user",
password: originalHash,
},
];
// Re-seed with the rotated password env var
await seedUatCredentials([UAT_ACCOUNTS[2]!], {
users: preExistingUsers,
accounts: preExistingAccounts,
staff: [],
});
// No new user or account created
expect(insertedUsers).toHaveLength(0);
expect(insertedAccounts).toHaveLength(0);
// The pre-existing account's password WAS updated (not frozen at first-seed).
// hashPassword uses a random salt so we verify by format + that it is a new,
// different valid hash from the original.
const updatedAcct = preExistingAccounts[0]!;
expect(updatedAcct.password).toBeDefined();
expect(updatedAcct.password).toMatch(/^[a-f0-9]{32}:[a-f0-9]{128}$/);
expect(updatedAcct.password).not.toBe(originalHash); // it actually changed
});
// ── AC-6: missing env var skips with warning ────────────────────────────────
+1 -9
View File
@@ -594,15 +594,7 @@ async function seedKnownUsers() {
.limit(1);
if (existingAccount) {
// Re-hash and update the password so that re-seeding rotates credentials
// when the env var changes (e.g. after a password rotation). Previously
// this branch skipped entirely, freezing the hash at first-seed.
const { hashPassword } = await import("better-auth/crypto");
const passwordHash = await hashPassword(password);
await db.update(schema.account)
.set({ password: passwordHash })
.where(eq(schema.account.id, existingAccount.id));
console.log(`✓ Updated credential account password for '${acct.email}'`);
console.log(`✓ Credential account for '${acct.email}' already exists — skipping`);
} else {
// Use Better-Auth's own hashPassword to guarantee parameter/encoding match.
// better-auth/crypto uses: N=16384, r=16, p=1, dkLen=64, salt as 16-byte random
+1 -37
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, impersonationSessions, or, pets, appointments, staff, services, sql } from "../db/index.js";
import { and, desc, eq, exists, getDb, gte, groomingVisitLogs, or, pets, appointments, staff, services, sql } from "../db/index.js";
import type { AppEnv } from "../middleware/rbac.js";
import {
getPresignedUploadUrl,
@@ -307,38 +307,10 @@ 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();
@@ -349,15 +321,7 @@ 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);
}
@@ -1,9 +0,0 @@
-- Migration: 0036_add_missing_coat_type_values.sql
-- Adds missing values to coat_type enum that seed.ts requires but which were
-- omitted from the 0031_buffer_rules.sql CREATE TYPE statement (migration drift).
-- 0031 created: 'smooth', 'double', 'wire', 'curly', 'long', 'hairless'
-- Missing (from schema.ts coatTypeEnum): 'short', 'medium', 'silky'
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';
@@ -1,19 +0,0 @@
-- Migration: 0037_add_extra_large_to_pet_size_category.sql
-- GRO-1979: Adds the 'extra_large' value to the pet_size_category enum.
--
-- 0031_buffer_rules.sql created pet_size_category with values
-- ('small', 'medium', 'large', 'xlarge'), but seed.ts and the drizzle
-- schema (PetSizeCategory type) both use 'extra_large' — a mismatch that
-- caused the UAT seed job to fail with:
-- invalid input value for enum pet_size_category: "extra_large"
--
-- 0035/0036 (GRO-1971) registered 'short'/'medium'/'silky' in coat_type.
-- This migration is the pet_size_category counterpart: register
-- 'extra_large' so seed.ts can write the value the schema declares.
--
-- Postgres restriction: ALTER TYPE ADD VALUE cannot run inside a
-- transaction block. The drizzle migrate runner does not wrap
-- individual statements in an explicit transaction, so this applies
-- as a single auto-commit DDL.
ALTER TYPE "pet_size_category" ADD VALUE IF NOT EXISTS 'extra_large';
@@ -1,4 +0,0 @@
-- 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';
+3 -17
View File
@@ -248,25 +248,11 @@
"breakpoints": true
},
{
"idx": 36,
"idx": 35,
"version": "7",
"when": 1751480000000,
"tag": "0036_add_missing_coat_type_values",
"breakpoints": true
},
{
"idx": 37,
"version": "7",
"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",
"tag": "0035_add_missing_coat_type_values",
"breakpoints": true
}
]
}
}
+14 -20
View File
@@ -1106,17 +1106,14 @@ async function seed() {
temperamentScore: randInt(1, 5),
temperamentFlags: pickN(temperamentFlagPool, randInt(1, 3)),
medicalAlerts: (() => {
// TestCooper always has a behavioral alert; TestRocky always has a skin alert.
// All other UAT test pets follow the 30% random distribution.
// Deterministic alerts on 2 of 507 pets (~0.4%) do not meaningfully shift
// the overall distribution from the 25-35% target band.
if (uc.petName === "TestCooper") {
return pickN(medicalAlertPool.filter((a) => a.type === "behavioral"), 1).map((a) => ({ ...a, id: uuid() }));
}
if (uc.petName === "TestRocky") {
return pickN(medicalAlertPool.filter((a) => a.type === "skin"), 1).map((a) => ({ ...a, id: uuid() }));
}
// ~30% of pets get alerts; TestCooper/TestRocky get deterministic types
if (rand() < 0.3) {
if (uc.petName === "TestCooper") {
return pickN(medicalAlertPool.filter((a) => a.type === "behavioral"), 1).map((a) => ({ ...a, id: uuid() }));
}
if (uc.petName === "TestRocky") {
return pickN(medicalAlertPool.filter((a) => a.type === "skin"), 1).map((a) => ({ ...a, id: uuid() }));
}
const count = rand() < 0.7 ? 1 : 2;
return pickN(medicalAlertPool, count).map((a) => ({ ...a, id: uuid() }));
}
@@ -1139,17 +1136,14 @@ async function seed() {
temperamentScore: randInt(1, 5),
temperamentFlags: pickN(temperamentFlagPool, randInt(1, 3)),
medicalAlerts: (() => {
// TestCooper always has a behavioral alert; TestRocky always has a skin alert.
// All other UAT test pets follow the 30% random distribution.
// Deterministic alerts on 2 of 507 pets (~0.4%) do not meaningfully shift
// the overall distribution from the 25-35% target band.
if (uc.petName === "TestCooper") {
return pickN(medicalAlertPool.filter((a) => a.type === "behavioral"), 1).map((a) => ({ ...a, id: uuid() }));
}
if (uc.petName === "TestRocky") {
return pickN(medicalAlertPool.filter((a) => a.type === "skin"), 1).map((a) => ({ ...a, id: uuid() }));
}
// ~30% of pets get alerts; TestCooper/TestRocky get deterministic types
if (rand() < 0.3) {
if (uc.petName === "TestCooper") {
return pickN(medicalAlertPool.filter((a) => a.type === "behavioral"), 1).map((a) => ({ ...a, id: uuid() }));
}
if (uc.petName === "TestRocky") {
return pickN(medicalAlertPool.filter((a) => a.type === "skin"), 1).map((a) => ({ ...a, id: uuid() }));
}
const count = rand() < 0.7 ? 1 : 2;
return pickN(medicalAlertPool, count).map((a) => ({ ...a, id: uuid() }));
}
-569
View File
@@ -1,569 +0,0 @@
/**
* 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);
});
});
+2 -75
View File
@@ -7,7 +7,6 @@ import {
eq,
exists,
getDb,
impersonationSessions,
or,
pets,
appointments,
@@ -24,23 +23,6 @@ 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),
@@ -127,73 +109,18 @@ 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");
if (!staffRow) {
return c.json({ error: "Unauthorized" }, 401);
}
const isGroomer = staffRow.role === "groomer";
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);
// 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) {
if (isGroomer) {
const [linkage] = await db
.select({ id: appointments.id })
.from(appointments)