From f54a13fc8ad4376c2c74c217020317c659c00e7c Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:02:39 +0000 Subject: [PATCH 1/6] feat(api): add DB-touching /api/readyz so schema loss cannot hide behind /health (GRO-2678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Register GET /api/readyz after /api/health in src/index.ts (before authMiddleware) - Runs `getDb().select({id: staff.id}).from(staff).limit(1)` inside try/catch - Success → 200 {"status":"ready"}; failure → 503 {"status":"degraded","check":"db"} - Raw driver error is console.error'd but never leaked to the response body - /health and /api/health are unchanged (K8s probes remain DB-less per CTO decision) - New unit tests: mock getDb to resolve → assert 200/ready; throw → assert 503/degraded - Updated UAT_PLAYBOOK.md §4.0 with TC-API-0.2 and TC-API-0.3 Co-Authored-By: Paperclip --- UAT_PLAYBOOK.md | 3 ++ src/__tests__/readyz.test.ts | 87 ++++++++++++++++++++++++++++++++++++ src/index.ts | 11 +++++ 3 files changed, 101 insertions(+) create mode 100644 src/__tests__/readyz.test.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 5f0bda8..a0a5b5f 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -65,8 +65,11 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the | # | Scenario | Steps | Expected | |---|----------|-------|----------| | TC-API-0.1 | Unauthenticated health check | GET /api/health | 200 OK, `{"status":"ok"}` | +| TC-API-0.2 | DB-touching readiness check — healthy (GRO-2678) | GET /api/readyz | 200 OK, `{"status":"ready"}` | +| TC-API-0.3 | DB-touching readiness check — response body safe | GET /api/readyz and inspect body | Body contains only `status` and (on error) `check` fields — no raw SQL, driver messages, or stack traces | > **Note (GRO-1544):** Health endpoint registered on `api` basePath before auth middleware at `/api/health`. The old path `/health` was incorrect (routed to web pod via HTTPRoute `/*` rule). +> **Note (GRO-2678):** `/api/readyz` is a separate DB-touching endpoint for monitoring. It is intentionally NOT used for K8s liveness/readiness probes — those remain DB-less to avoid pod cycling on transient DB blips. ### 4.1 Authentication diff --git a/src/__tests__/readyz.test.ts b/src/__tests__/readyz.test.ts new file mode 100644 index 0000000..f2f5fde --- /dev/null +++ b/src/__tests__/readyz.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +// ─── Mock db module ─────────────────────────────────────────────────────────── + +let selectImpl: () => Promise; + +vi.mock("@groombook/db", () => { + const staff = new Proxy( + { _name: "staff" }, + { + get(_target, prop) { + if (prop === "_name") return "staff"; + return { table: "staff", column: prop }; + }, + } + ); + + return { + getDb: () => ({ + select: (_fields: unknown) => ({ + from: (_table: unknown) => ({ + limit: (_n: number) => selectImpl(), + }), + }), + }), + staff, + }; +}); + +// ─── Build test app ─────────────────────────────────────────────────────────── + +async function makeApp() { + // Import after mocks are in place + const { getDb, staff } = await import("@groombook/db"); + + const app = new Hono(); + app.get("/api/readyz", async (c) => { + try { + await getDb().select({ id: staff.id }).from(staff).limit(1); + return c.json({ status: "ready" }, 200); + } catch (err) { + console.error("[readyz] DB check failed:", err); + return c.json({ status: "degraded", check: "db" }, 503); + } + }); + return app; +} + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +describe("GET /api/readyz", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("returns 200 {status:'ready'} when DB query succeeds", async () => { + selectImpl = () => Promise.resolve([{ id: "staff-1" }]); + + const app = await makeApp(); + const res = await app.request("/api/readyz", { method: "GET" }); + const body = (await res.json()) as Record; + + expect(res.status).toBe(200); + expect(body.status).toBe("ready"); + }); + + it("returns 503 {status:'degraded',check:'db'} when DB query throws", async () => { + selectImpl = () => Promise.reject(new Error("42P01: relation staff does not exist")); + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const app = await makeApp(); + const res = await app.request("/api/readyz", { method: "GET" }); + const body = (await res.json()) as Record; + + expect(res.status).toBe(503); + expect(body.status).toBe("degraded"); + expect(body.check).toBe("db"); + + // Raw SQL / driver error must NOT appear in the response body + expect(JSON.stringify(body)).not.toContain("42P01"); + expect(JSON.stringify(body)).not.toContain("relation"); + + consoleSpy.mockRestore(); + }); +}); diff --git a/src/index.ts b/src/index.ts index e08a2ff..6da003b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -65,6 +65,17 @@ app.use( app.get("/health", (c) => c.json({ status: "ok" })); // /api/health: used by Gateway HTTPRoute (/api/* → API pod) app.get("/api/health", (c) => c.json({ status: "ok" })); +// /api/readyz: DB-touching deep health check consumed by monitoring (not K8s probes) +// Distinct from /health so a dropped schema triggers an alert without cycling pods (GRO-2678) +app.get("/api/readyz", async (c) => { + try { + await getDb().select({ id: staff.id }).from(staff).limit(1); + return c.json({ status: "ready" }, 200); + } catch (err) { + console.error("[readyz] DB check failed:", err); + return c.json({ status: "degraded", check: "db" }, 503); + } +}); // Public booking routes — no auth required, must be registered before auth middleware app.route("/api/book", bookRouter); -- 2.52.0 From 6148ae64395413588203eaed9af7fc2ff607c633 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:10:10 +0000 Subject: [PATCH 2/6] ci: retrigger after seed-image registry push flake Co-Authored-By: Paperclip -- 2.52.0 From 7679fada0a170ee36b93e1760f4541ecaa54b74f Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:21:56 +0000 Subject: [PATCH 3/6] feat(api): add DB-touching /health/ready readiness probe (GRO-2678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Register GET /health/ready before the /api/* auth middleware so it is public and reachable by K8s readinessProbe on port 3000 without auth. On success → 200 {"status":"ready"}; on any DB/schema failure → 503 {"status":"degraded"}. Logs the pg error code; never leaks SQL in body. A dropped schema (42P01) surfaces as non-200, closing the /health mask that allowed the GRO-2678 incident to go undetected for ~43h. Add health-ready.test.ts covering the 200 success path, 503 on schema drop (42P01), and 503 on connection error; all assert no SQL leakage. Co-Authored-By: Paperclip --- src/__tests__/health-ready.test.ts | 102 +++++++++++++++++++++++++++++ src/index.ts | 11 ++++ 2 files changed, 113 insertions(+) create mode 100644 src/__tests__/health-ready.test.ts diff --git a/src/__tests__/health-ready.test.ts b/src/__tests__/health-ready.test.ts new file mode 100644 index 0000000..6a8ea88 --- /dev/null +++ b/src/__tests__/health-ready.test.ts @@ -0,0 +1,102 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +// ─── Mock db module ─────────────────────────────────────────────────────────── + +let selectImpl: () => Promise; + +vi.mock("@groombook/db", () => { + const staff = new Proxy( + { _name: "staff" }, + { + get(_target, prop) { + if (prop === "_name") return "staff"; + return { table: "staff", column: prop }; + }, + } + ); + + return { + getDb: () => ({ + select: (_fields: unknown) => ({ + from: (_table: unknown) => ({ + limit: (_n: number) => selectImpl(), + }), + }), + }), + staff, + }; +}); + +// ─── Build test app ─────────────────────────────────────────────────────────── + +async function makeApp() { + const { getDb, staff } = await import("@groombook/db"); + + const app = new Hono(); + app.get("/health/ready", async (c) => { + try { + await getDb().select({ id: staff.id }).from(staff).limit(1); + return c.json({ status: "ready" }, 200); + } catch (err) { + const pgCode = (err as Record).code ?? "unknown"; + console.error("[health/ready] DB check failed:", pgCode); + return c.json({ status: "degraded" }, 503); + } + }); + return app; +} + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +describe("GET /health/ready", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("returns 200 {status:'ready'} when DB query succeeds", async () => { + selectImpl = () => Promise.resolve([{ id: "staff-1" }]); + + const app = await makeApp(); + const res = await app.request("/health/ready", { method: "GET" }); + const body = (await res.json()) as Record; + + expect(res.status).toBe(200); + expect(body.status).toBe("ready"); + }); + + it("returns 503 {status:'degraded'} when DB query throws (schema dropped)", async () => { + const schemaErr = Object.assign(new Error("relation \"staff\" does not exist"), { code: "42P01" }); + selectImpl = () => Promise.reject(schemaErr); + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const app = await makeApp(); + const res = await app.request("/health/ready", { method: "GET" }); + const body = (await res.json()) as Record; + + expect(res.status).toBe(503); + expect(body.status).toBe("degraded"); + + // Must not leak SQL error details in the response body + expect(JSON.stringify(body)).not.toContain("42P01"); + expect(JSON.stringify(body)).not.toContain("relation"); + + consoleSpy.mockRestore(); + }); + + it("returns 503 {status:'degraded'} on any DB connection error", async () => { + selectImpl = () => Promise.reject(new Error("ECONNREFUSED")); + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const app = await makeApp(); + const res = await app.request("/health/ready", { method: "GET" }); + const body = (await res.json()) as Record; + + expect(res.status).toBe(503); + expect(body.status).toBe("degraded"); + + consoleSpy.mockRestore(); + }); +}); diff --git a/src/index.ts b/src/index.ts index 6da003b..99a66fa 100644 --- a/src/index.ts +++ b/src/index.ts @@ -65,6 +65,17 @@ app.use( app.get("/health", (c) => c.json({ status: "ok" })); // /api/health: used by Gateway HTTPRoute (/api/* → API pod) app.get("/api/health", (c) => c.json({ status: "ok" })); +// /health/ready: DB-touching readiness probe — K8s removes pod from endpoints when schema is dropped (GRO-2689) +app.get("/health/ready", async (c) => { + try { + await getDb().select({ id: staff.id }).from(staff).limit(1); + return c.json({ status: "ready" }, 200); + } catch (err) { + const pgCode = (err as Record).code ?? "unknown"; + console.error("[health/ready] DB check failed:", pgCode); + return c.json({ status: "degraded" }, 503); + } +}); // /api/readyz: DB-touching deep health check consumed by monitoring (not K8s probes) // Distinct from /health so a dropped schema triggers an alert without cycling pods (GRO-2678) app.get("/api/readyz", async (c) => { -- 2.52.0 From d8f6981be1f05772895593cdd436e6c0d4cdfd9a Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:48:10 +0000 Subject: [PATCH 4/6] =?UTF-8?q?revert(api):=20remove=20/health/ready=20?= =?UTF-8?q?=E2=80=94=20superseded=20by=20/api/readyz=20(GRO-2689)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTO architectural ruling (PR #235 review): K8s readiness/liveness probes must remain DB-less to prevent transient DB blips from cycling pods. The /api/readyz endpoint (GRO-2687) is the canonical DB-health signal for the monitoring layer and satisfies the GRO-2678 detection-gap requirement. Removes: - GET /health/ready route from src/index.ts - src/__tests__/health-ready.test.ts Refs GRO-2689, GRO-2687, GRO-2678 --- src/__tests__/health-ready.test.ts | 102 ----------------------------- src/index.ts | 11 ---- 2 files changed, 113 deletions(-) delete mode 100644 src/__tests__/health-ready.test.ts diff --git a/src/__tests__/health-ready.test.ts b/src/__tests__/health-ready.test.ts deleted file mode 100644 index 6a8ea88..0000000 --- a/src/__tests__/health-ready.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { Hono } from "hono"; - -// ─── Mock db module ─────────────────────────────────────────────────────────── - -let selectImpl: () => Promise; - -vi.mock("@groombook/db", () => { - const staff = new Proxy( - { _name: "staff" }, - { - get(_target, prop) { - if (prop === "_name") return "staff"; - return { table: "staff", column: prop }; - }, - } - ); - - return { - getDb: () => ({ - select: (_fields: unknown) => ({ - from: (_table: unknown) => ({ - limit: (_n: number) => selectImpl(), - }), - }), - }), - staff, - }; -}); - -// ─── Build test app ─────────────────────────────────────────────────────────── - -async function makeApp() { - const { getDb, staff } = await import("@groombook/db"); - - const app = new Hono(); - app.get("/health/ready", async (c) => { - try { - await getDb().select({ id: staff.id }).from(staff).limit(1); - return c.json({ status: "ready" }, 200); - } catch (err) { - const pgCode = (err as Record).code ?? "unknown"; - console.error("[health/ready] DB check failed:", pgCode); - return c.json({ status: "degraded" }, 503); - } - }); - return app; -} - -// ─── Tests ──────────────────────────────────────────────────────────────────── - -describe("GET /health/ready", () => { - beforeEach(() => { - vi.restoreAllMocks(); - }); - - it("returns 200 {status:'ready'} when DB query succeeds", async () => { - selectImpl = () => Promise.resolve([{ id: "staff-1" }]); - - const app = await makeApp(); - const res = await app.request("/health/ready", { method: "GET" }); - const body = (await res.json()) as Record; - - expect(res.status).toBe(200); - expect(body.status).toBe("ready"); - }); - - it("returns 503 {status:'degraded'} when DB query throws (schema dropped)", async () => { - const schemaErr = Object.assign(new Error("relation \"staff\" does not exist"), { code: "42P01" }); - selectImpl = () => Promise.reject(schemaErr); - - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - - const app = await makeApp(); - const res = await app.request("/health/ready", { method: "GET" }); - const body = (await res.json()) as Record; - - expect(res.status).toBe(503); - expect(body.status).toBe("degraded"); - - // Must not leak SQL error details in the response body - expect(JSON.stringify(body)).not.toContain("42P01"); - expect(JSON.stringify(body)).not.toContain("relation"); - - consoleSpy.mockRestore(); - }); - - it("returns 503 {status:'degraded'} on any DB connection error", async () => { - selectImpl = () => Promise.reject(new Error("ECONNREFUSED")); - - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - - const app = await makeApp(); - const res = await app.request("/health/ready", { method: "GET" }); - const body = (await res.json()) as Record; - - expect(res.status).toBe(503); - expect(body.status).toBe("degraded"); - - consoleSpy.mockRestore(); - }); -}); diff --git a/src/index.ts b/src/index.ts index 99a66fa..6da003b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -65,17 +65,6 @@ app.use( app.get("/health", (c) => c.json({ status: "ok" })); // /api/health: used by Gateway HTTPRoute (/api/* → API pod) app.get("/api/health", (c) => c.json({ status: "ok" })); -// /health/ready: DB-touching readiness probe — K8s removes pod from endpoints when schema is dropped (GRO-2689) -app.get("/health/ready", async (c) => { - try { - await getDb().select({ id: staff.id }).from(staff).limit(1); - return c.json({ status: "ready" }, 200); - } catch (err) { - const pgCode = (err as Record).code ?? "unknown"; - console.error("[health/ready] DB check failed:", pgCode); - return c.json({ status: "degraded" }, 503); - } -}); // /api/readyz: DB-touching deep health check consumed by monitoring (not K8s probes) // Distinct from /health so a dropped schema triggers an alert without cycling pods (GRO-2678) app.get("/api/readyz", async (c) => { -- 2.52.0 From 8f5a069e77e0779dbb6c44380139494bdf0fe615 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:52:37 +0000 Subject: [PATCH 5/6] fix(api): add /api/readyz to authMiddleware bypass list (GRO-2687) /api/readyz was returning 401 Unauthorized in UAT (GRO-2692 Shedward regression). The authMiddleware for /api/* did not whitelist /api/readyz, causing every unauthenticated probe request to be rejected. Add /api/readyz to the bypass condition alongside /api/auth/* and /api/health. Add readyz-auth-bypass.test.ts confirming the route passes through the middleware without auth and that non-whitelisted paths still block (503 when auth not configured). Closes GRO-2692 regression (UAT fail). --- src/__tests__/readyz-auth-bypass.test.ts | 44 ++++++++++++++++++++++++ src/middleware/auth.ts | 2 +- 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/readyz-auth-bypass.test.ts diff --git a/src/__tests__/readyz-auth-bypass.test.ts b/src/__tests__/readyz-auth-bypass.test.ts new file mode 100644 index 0000000..744ca60 --- /dev/null +++ b/src/__tests__/readyz-auth-bypass.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, vi } from "vitest"; +import { Hono } from "hono"; + +// Mock auth lib so getAuth() throws — non-bypass paths hit the 503 "not configured" guard. +vi.mock("../lib/auth.js", () => ({ + getAuth: () => { + throw new Error("auth not configured"); + }, + initAuth: vi.fn(), + getActiveProviders: vi.fn(() => []), +})); + +describe("authMiddleware bypass: /api/readyz", () => { + it("serves /api/readyz without auth (bypass before auth check)", async () => { + // Ensure AUTH_DISABLED is not set so the bypass is exercised, not AUTH_DISABLED shortcut. + const prev = process.env.AUTH_DISABLED; + delete process.env.AUTH_DISABLED; + + const { authMiddleware } = await import("../middleware/auth.js"); + const app = new Hono(); + app.use("/api/*", authMiddleware); + app.get("/api/readyz", (c) => c.json({ status: "ok" }, 200)); + + const res = await app.request("/api/readyz", { method: "GET" }); + expect(res.status).toBe(200); + + if (prev !== undefined) process.env.AUTH_DISABLED = prev; + }); + + it("blocks non-whitelisted /api/* paths when auth is not configured", async () => { + const prev = process.env.AUTH_DISABLED; + delete process.env.AUTH_DISABLED; + + const { authMiddleware } = await import("../middleware/auth.js"); + const app = new Hono(); + app.use("/api/*", authMiddleware); + app.get("/api/staff", (c) => c.json({ ok: true }, 200)); + + const res = await app.request("/api/staff", { method: "GET" }); + expect(res.status).toBe(503); + + if (prev !== undefined) process.env.AUTH_DISABLED = prev; + }); +}); diff --git a/src/middleware/auth.ts b/src/middleware/auth.ts index 830350f..329ba82 100644 --- a/src/middleware/auth.ts +++ b/src/middleware/auth.ts @@ -23,7 +23,7 @@ if (process.env.AUTH_DISABLED === "true") { } export const authMiddleware: MiddlewareHandler = async (c, next) => { - if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health") { + if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health" || c.req.path === "/api/readyz") { await next(); return; } -- 2.52.0 From 7dfa1ad8308c114e162614e3c06df8fb27be9ab4 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 9 Aug 2026 09:53:19 +0000 Subject: [PATCH 6/6] fix(auth): add /api/readyz to auth middleware bypass list (GRO-2692) /api/readyz is a public monitoring endpoint like /api/health. app.basePath("/api") + api.use("*", authMiddleware) applies to all /api/* paths regardless of route registration order (GRO-2692 UAT failure: Hono basePath middleware bypass). Co-Authored-By: Paperclip --- src/middleware/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middleware/auth.ts b/src/middleware/auth.ts index 830350f..329ba82 100644 --- a/src/middleware/auth.ts +++ b/src/middleware/auth.ts @@ -23,7 +23,7 @@ if (process.env.AUTH_DISABLED === "true") { } export const authMiddleware: MiddlewareHandler = async (c, next) => { - if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health") { + if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health" || c.req.path === "/api/readyz") { await next(); return; } -- 2.52.0