Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 337c0e2733 |
@@ -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 ✓"
|
||||
|
||||
+1
-13
@@ -3,12 +3,8 @@ 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 / GRO-1981 / GRO-1985.
|
||||
# Jobs. GRO-1983 / GRO-1889 / GRO-1909.
|
||||
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
|
||||
@@ -30,8 +26,6 @@ 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
|
||||
|
||||
@@ -52,18 +46,12 @@ 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"]
|
||||
|
||||
@@ -125,9 +125,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.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)
|
||||
|
||||
|
||||
@@ -1,285 +0,0 @@
|
||||
/**
|
||||
* GET /pets/:id/profile-summary tests
|
||||
*
|
||||
* GRO-2014 regression coverage:
|
||||
* - 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.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
import type { AppEnv, StaffRow } from "../middleware/rbac.js";
|
||||
|
||||
// ─── Fixtures ────────────────────────────────────────────────────────────────
|
||||
|
||||
const MANAGER: StaffRow = {
|
||||
id: "00000000-0000-0000-0000-0000000000aa",
|
||||
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: "00000000-0000-0000-0000-0000000000bb",
|
||||
oidcSub: "oidc-groomer-sub",
|
||||
role: "groomer",
|
||||
isSuperUser: false,
|
||||
name: "Groomer Gary",
|
||||
email: "groomer@example.com",
|
||||
};
|
||||
|
||||
const PET_UUID = "11111111-1111-1111-1111-111111111111";
|
||||
const CLIENT_UUID = "22222222-2222-2222-2222-222222222222";
|
||||
const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001";
|
||||
|
||||
const PET_ROW = {
|
||||
id: PET_UUID,
|
||||
clientId: CLIENT_UUID,
|
||||
name: "Biscuit",
|
||||
species: "dog",
|
||||
breed: "Beagle",
|
||||
coatType: "short",
|
||||
petSizeCategory: "medium",
|
||||
weightKg: "12.50",
|
||||
dateOfBirth: new Date("2020-01-01"),
|
||||
};
|
||||
|
||||
// ─── Mutable DB state ─────────────────────────────────────────────────────────
|
||||
|
||||
interface DbState {
|
||||
petRow: typeof PET_ROW | null;
|
||||
linkageRow: { id: string } | null;
|
||||
recentHistory: Array<Record<string, unknown>>;
|
||||
visitCount: number;
|
||||
upcoming: Record<string, unknown> | null;
|
||||
throwOnPetSelect: boolean;
|
||||
}
|
||||
|
||||
let dbState: DbState;
|
||||
|
||||
function resetDb() {
|
||||
dbState = {
|
||||
petRow: { ...PET_ROW },
|
||||
linkageRow: { id: "appt-link" },
|
||||
recentHistory: [],
|
||||
visitCount: 0,
|
||||
upcoming: null,
|
||||
throwOnPetSelect: false,
|
||||
};
|
||||
}
|
||||
|
||||
// ─── @groombook/db mock ──────────────────────────────────────────────────────
|
||||
//
|
||||
// Each select chain needs to know which table it's targeting and which columns
|
||||
// it's projecting so we can return the right mocked rows. We thread that state
|
||||
// through a per-call object whose chain methods all return `this`. The chain
|
||||
// is also `then`-able so any `await` position resolves to the rows.
|
||||
|
||||
vi.mock("@groombook/db", () => {
|
||||
const namedTable = (name: string) =>
|
||||
new Proxy(
|
||||
{ _name: name },
|
||||
{
|
||||
get(_t, p) {
|
||||
if (p === "_name") return name;
|
||||
return { table: name, column: p };
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
const pets = namedTable("pets");
|
||||
const appointments = namedTable("appointments");
|
||||
const services = namedTable("services");
|
||||
const staff = namedTable("staff");
|
||||
|
||||
// The full chain interface is intentionally loose — only `then` is exposed
|
||||
// with a typed signature so vitest's await resolves to the right shape.
|
||||
interface ChainLike {
|
||||
from: (table: { _name: string }) => ChainLike;
|
||||
where: (...args: unknown[]) => ChainLike;
|
||||
innerJoin: (...args: unknown[]) => ChainLike;
|
||||
leftJoin: (...args: unknown[]) => ChainLike;
|
||||
orderBy: (...args: unknown[]) => ChainLike;
|
||||
limit: (...args: unknown[]) => ChainLike;
|
||||
then: <T = unknown[]>(
|
||||
onfulfilled?: ((value: unknown[]) => T | PromiseLike<T>) | null
|
||||
) => Promise<T>;
|
||||
}
|
||||
|
||||
function buildSelect(projection?: Record<string, unknown>): ChainLike {
|
||||
let targetTable = "";
|
||||
|
||||
const resolveRows = (): unknown[] => {
|
||||
if (targetTable === "pets") {
|
||||
if (dbState.throwOnPetSelect) {
|
||||
throw new Error("simulated postgres uuid cast failure");
|
||||
}
|
||||
return dbState.petRow ? [dbState.petRow] : [];
|
||||
}
|
||||
if (targetTable === "appointments") {
|
||||
const keys = projection ? Object.keys(projection) : [];
|
||||
if (projection && keys.length === 1 && keys[0] === "id") {
|
||||
return dbState.linkageRow ? [dbState.linkageRow] : [];
|
||||
}
|
||||
if (projection && keys.includes("count")) {
|
||||
return [{ count: dbState.visitCount }];
|
||||
}
|
||||
if (projection && keys.includes("confirmationStatus")) {
|
||||
return dbState.upcoming ? [dbState.upcoming] : [];
|
||||
}
|
||||
return dbState.recentHistory;
|
||||
}
|
||||
return [];
|
||||
};
|
||||
|
||||
const chain: ChainLike = {
|
||||
from(table) {
|
||||
targetTable = table._name;
|
||||
return chain;
|
||||
},
|
||||
where() {
|
||||
return chain;
|
||||
},
|
||||
innerJoin() {
|
||||
return chain;
|
||||
},
|
||||
leftJoin() {
|
||||
return chain;
|
||||
},
|
||||
orderBy() {
|
||||
return chain;
|
||||
},
|
||||
limit() {
|
||||
return chain;
|
||||
},
|
||||
then(onfulfilled) {
|
||||
return Promise.resolve(resolveRows()).then(onfulfilled ?? undefined);
|
||||
},
|
||||
};
|
||||
|
||||
return chain;
|
||||
}
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
select: (projection?: Record<string, unknown>) => buildSelect(projection),
|
||||
}),
|
||||
pets,
|
||||
appointments,
|
||||
services,
|
||||
staff,
|
||||
and: vi.fn(() => ({ _op: "and" })),
|
||||
or: vi.fn(() => ({ _op: "or" })),
|
||||
eq: vi.fn(() => ({ _op: "eq" })),
|
||||
desc: vi.fn((arg: unknown) => arg),
|
||||
exists: vi.fn((arg: unknown) => arg),
|
||||
sql: Object.assign(
|
||||
() => ({ _op: "sql" }),
|
||||
{ [Symbol.toPrimitive]: () => "sql" }
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../lib/s3.js", () => ({
|
||||
getPresignedUploadUrl: vi.fn().mockResolvedValue("https://example.com/put"),
|
||||
getPresignedGetUrl: vi.fn().mockResolvedValue("https://example.com/get"),
|
||||
deleteObject: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
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("staff", staffRow);
|
||||
await next();
|
||||
});
|
||||
app.route("/pets", petsRouter);
|
||||
return app;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetDb();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
// ─── Tests ───────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("GET /pets/: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 () => {
|
||||
dbState.petRow = null;
|
||||
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 () => {
|
||||
dbState.linkageRow = null;
|
||||
const app = buildApp(GROOMER);
|
||||
const res = await app.request(`/pets/${PET_UUID}/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 () => {
|
||||
const app = buildApp(MANAGER);
|
||||
const res = await app.request(`/pets/${PET_UUID}/profile-summary`);
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
expect(body.id).toBe(PET_UUID);
|
||||
expect(body.name).toBe("Biscuit");
|
||||
expect(body.visitCount).toBe(0);
|
||||
expect(body.upcomingAppointment).toBeNull();
|
||||
expect(body.recentGroomingHistory).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns 200 with summary for a groomer with linkage", async () => {
|
||||
const app = buildApp(GROOMER);
|
||||
const res = await app.request(`/pets/${PET_UUID}/profile-summary`);
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
expect(body.id).toBe(PET_UUID);
|
||||
});
|
||||
|
||||
it("returns a JSON envelope (not empty body) when a downstream query throws", async () => {
|
||||
dbState.throwOnPetSelect = true;
|
||||
const app = buildApp(MANAGER);
|
||||
const res = await app.request(`/pets/${PET_UUID}/profile-summary`);
|
||||
expect(res.status).toBe(500);
|
||||
const body = (await res.json()) as { error: string };
|
||||
expect(body.error).toBe("Internal Server Error");
|
||||
});
|
||||
});
|
||||
+1
-34
@@ -23,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),
|
||||
@@ -129,24 +112,8 @@ petsRouter.get("/:id", async (c) => {
|
||||
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));
|
||||
|
||||
Reference in New Issue
Block a user