Merge origin/dev into flea/gro-2013-deployed-tree-conflicts

# Conflicts:
#	UAT_PLAYBOOK.md
#	src/__tests__/petProfileSummary.test.ts

.gitea/workflows/ci.yml

@@ -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 ✓"

Dockerfile

@@ -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"]

UAT_PLAYBOOK.md

@@ -128,6 +128,9 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
 | 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)
 

src/__tests__/petProfileSummary.test.ts

@@ -1,23 +1,34 @@
 /**
  * Pet Profile Summary Tests
  *
- * Covers GET /api/pets/:id/profile-summary in the deployed tree
- * (root src/). The headline cases validate the GRO-2013 owner-bypass:
- * 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.
+ * Covers GET /api/pets/:id/profile-summary in the deployed tree (root src/).
  *
- * Deployed tree: src/routes/pets.ts. This test mirrors the live handler
- * (which queries the `appointments` table for visit history, not
- * `groomingVisitLogs`).
+ * 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";
 
-// ─── Mock staff fixtures ──────────────────────────────────────────────────────
+// ─── Staff fixtures ──────────────────────────────────────────────────────────
 
 const MANAGER: StaffRow = {
   id: "staff-manager-id",
@@ -38,6 +49,7 @@ const GROOMER: StaffRow = {
   id: "staff-groomer-id",
   oidcSub: "oidc-groomer-sub",
   role: "groomer",
+  isSuperUser: false,
   name: "Groomer Gary",
   email: "groomer@example.com",
 };
@@ -57,11 +69,12 @@ const CUSTOMER_STAFF: StaffRow = {
   email: "uat-customer@groombook.dev",
 };
 
-// ─── Mutable mock state ───────────────────────────────────────────────────────
+// ─── 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);
@@ -147,7 +160,7 @@ function makeSession(overrides: Record<string, unknown> = {}) {
   };
 }
 
-// ─── DB mock state ────────────────────────────────────────────────────────────
+// ─── DB mock state ──────────────────────────────────────────────────────────
 
 let petsTable: Record<string, unknown>[];
 let appointmentsTable: Record<string, unknown>[];
@@ -157,12 +170,26 @@ 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()).
-let selectQueue: Array<{ table: string; rows: unknown[] }> = [];
+//
+// 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()];
@@ -171,7 +198,7 @@ function resetMock() {
   selectQueue = [];
 }
 
-// ─── Module mocks ─────────────────────────────────────────────────────────────
+// ─── Module mocks ───────────────────────────────────────────────────────────
 
 vi.mock("@groombook/db", () => {
   function makeTable(name: string) {
@@ -201,7 +228,12 @@ vi.mock("@groombook/db", () => {
 
   function takeQueuedRows(tableName: string): unknown[] {
     const next = selectQueue.shift();
-    if (next && next.table === tableName) return next.rows;
+    if (next && next.table === tableName) {
+      if (next.throw) {
+        throw new Error(next.throw);
+      }
+      return next.rows ?? [];
+    }
     return [];
   }
 
@@ -261,35 +293,113 @@ vi.mock("../lib/s3.js", () => ({
   deleteObject: vi.fn(),
 }));
 
-// ─── Import after mocks are set up ────────────────────────────────────────────
+// ─── Import after mocks are set up ──────────────────────────────────────────
 
 const { petsRouter } = await import("../routes/pets.js");
 
-// ─── App builder ──────────────────────────────────────────────────────────────
+// ─── App builder ────────────────────────────────────────────────────────────
 
-function buildApp(staffRow: StaffRow) {
+function buildApp(staffRow: StaffRow | null) {
   const app = new Hono<AppEnv>();
   app.use("*", async (c, next) => {
-    c.set("jwtPayload", { sub: staffRow.oidcSub ?? staffRow.userId ?? "" });
-    c.set("staff", staffRow);
+    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 ───────────────────────────────────────────────────
+// ─── Reset before each test ─────────────────────────────────────────────────
 
 beforeEach(() => {
   resetMock();
   vi.clearAllMocks();
 });
 
-// ─── Tests ────────────────────────────────────────────────────────────────────
+// ─── GRO-2014 error-handling suite ──────────────────────────────────────────
 
-describe("GET /:id/profile-summary — basic access", () => {
+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 () => {
-    petsTable = [];
     enqueue("pets", []);
     const app = buildApp(MANAGER);
     const res = await app.request(`/pets/${PET_ID}/profile-summary`);
@@ -297,7 +407,6 @@ describe("GET /:id/profile-summary — basic access", () => {
   });
 
   it("returns 200 with aggregated profile for a manager", async () => {
-    // Query order: pets, recent history, visit count, upcoming
     enqueue("pets", petsTable);
     enqueue("appointments", appointmentsTable);
     enqueue("appointments", [{ count: 1 }]);
@@ -315,10 +424,9 @@ describe("GET /:id/profile-summary — basic access", () => {
   });
 
   it("returns 200 for a groomer with appointment linkage to the pet's client", async () => {
-    // Query order: pets, linkage check, recent history, visit count, upcoming
     enqueue("pets", petsTable);
     enqueue("appointments", [{ id: "appt-1" }]); // linkage found
-    enqueue("appointments", appointmentsTable); // history
+    enqueue("appointments", appointmentsTable);
     enqueue("appointments", [{ count: 1 }]);
     enqueue("appointments", []);
 
@@ -328,7 +436,6 @@ describe("GET /:id/profile-summary — basic access", () => {
   });
 
   it("returns 403 for a groomer with no appointment linkage and no bypass header", async () => {
-    // Query order: pets, linkage check (returns empty → 403)
     enqueue("pets", petsTable);
     enqueue("appointments", []); // no linkage
 
@@ -336,14 +443,8 @@ describe("GET /:id/profile-summary — basic access", () => {
     const res = await app.request(`/pets/${PET_ID}/profile-summary`);
     expect(res.status).toBe(403);
   });
-});
 
-// ─── GRO-2013 owner-bypass ────────────────────────────────────────────────────
-
-describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
   it("customer-as-groomer with valid active session for pet's client returns 200", async () => {
-    // Query order: pets, session lookup (found, active, future), recent history,
-    // visit count, upcoming
     enqueue("pets", petsTable);
     enqueue("impersonationSessions", sessionsTable); // active session found
     enqueue("appointments", appointmentsTable);
@@ -360,7 +461,6 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
   });
 
   it("customer-as-groomer with no header still gets 403 (no bypass)", async () => {
-    // Query order: pets, session lookup (header missing → returns [], 403)
     enqueue("pets", petsTable);
 
     const app = buildApp(CUSTOMER_STAFF);

src/routes/pets.ts

@@ -24,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),
@@ -142,8 +159,24 @@ async function resolveImpersonationClientId(
 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));