Compare commits

..

2 Commits

Author SHA1 Message Date
Flea Flicker 4594bd2307 fix(GRO-655): create corepack cache dir in builder stage
Prevents ENOENT crash in migrate and seed jobs.

Root cause: corepack tries to mkdir /home/node/.cache/node/corepack/v1
but the directory does not exist in the builder stage. This was a
regression in c438f57 where the cache directory was not pre-created.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-14 21:58:44 +00:00
Flea Flicker d8c0052b54 fix(GRO-634): implement auth & authorization security hardening (8 findings)
- Remove placeholder secret fallback, require BETTER_AUTH_SECRET when AUTH_DISABLED=true
- Fix TOCTOU race in setup: use INSERT...RETURNING for atomic confirmation token creation
- Fix confirmation token replay: atomic UPDATE with WHERE clause prevents double-use
- Add CSRF origin-check middleware for non-safe HTTP methods
- Validate OIDC discovery URL hostname matches configured issuer
- Use timing-safe comparison for iCal authentication tokens
- Add rate limiting (10 req/min per IP) on setup endpoints
- Fix RBAC error messages: correct inversion of privilege check
2026-04-14 17:08:02 +00:00
21 changed files with 236 additions and 442 deletions
-2
View File
@@ -7,5 +7,3 @@ apps/web/dist
apps/api/dist apps/api/dist
packages/db/dist packages/db/dist
packages/types/dist packages/types/dist
.turbo
screenshots/
+9 -24
View File
@@ -20,8 +20,6 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: pnpm/action-setup@v4 - uses: pnpm/action-setup@v4
with:
version: '9.15.4'
- uses: actions/setup-node@v4 - uses: actions/setup-node@v4
with: with:
@@ -44,8 +42,6 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: pnpm/action-setup@v4 - uses: pnpm/action-setup@v4
with:
version: '9.15.4'
- uses: actions/setup-node@v4 - uses: actions/setup-node@v4
with: with:
@@ -66,8 +62,6 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: pnpm/action-setup@v4 - uses: pnpm/action-setup@v4
with:
version: '9.15.4'
- uses: actions/setup-node@v4 - uses: actions/setup-node@v4
with: with:
@@ -107,8 +101,6 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: pnpm/action-setup@v4 - uses: pnpm/action-setup@v4
with:
version: '9.15.4'
- uses: actions/setup-node@v4 - uses: actions/setup-node@v4
with: with:
@@ -246,6 +238,7 @@ jobs:
echo "Deploying images tagged $TAG to groombook-dev..." echo "Deploying images tagged $TAG to groombook-dev..."
# Run migration with PR image # Run migration with PR image
kubectl delete job migrate-schema -n groombook-dev --ignore-not-found
kubectl delete job "migrate-pr-$PR_NUM" -n groombook-dev --ignore-not-found kubectl delete job "migrate-pr-$PR_NUM" -n groombook-dev --ignore-not-found
cat <<EOF | kubectl apply -n groombook-dev -f - cat <<EOF | kubectl apply -n groombook-dev -f -
apiVersion: batch/v1 apiVersion: batch/v1
@@ -310,8 +303,6 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: pnpm/action-setup@v4 - uses: pnpm/action-setup@v4
with:
version: '9.15.4'
- uses: actions/setup-node@v4 - uses: actions/setup-node@v4
with: with:
@@ -418,17 +409,11 @@ jobs:
git push -u origin "chore/update-image-tags-${TAG}" git push -u origin "chore/update-image-tags-${TAG}"
# Check if PR already exists for this branch # Create PR and merge immediately (no required checks on groombook/infra)
EXISTING_PR=$(gh pr list --repo groombook/infra --head "chore/update-image-tags-${TAG}" --state open --json number -q '.[0].number' || true) PR_URL=$(gh pr create \
if [ -n "$EXISTING_PR" ]; then --repo groombook/infra \
echo "PR #$EXISTING_PR already exists for this tag, merging existing PR" --base main \
gh pr merge "$EXISTING_PR" --repo groombook/infra --merge --head "chore/update-image-tags-${TAG}" \
else --title "chore: deploy ${TAG} to dev" \
PR_URL=$(gh pr create \ --body "[GRO-178](/GRO/issues/GRO-178) — automated image tag update from main merge")
--repo groombook/infra \ gh pr merge "$PR_URL" --merge
--base main \
--head "chore/update-image-tags-${TAG}" \
--title "chore: deploy ${TAG} to dev" \
--body "[GRO-178](/GRO/issues/GRO-178) — automated image tag update from main merge")
gh pr merge "$PR_URL" --merge
fi
+1 -4
View File
@@ -35,9 +35,6 @@ COPY --from=builder /app/packages/types/dist packages/types/dist
RUN pnpm install --frozen-lockfile --prod RUN pnpm install --frozen-lockfile --prod
EXPOSE 3000 EXPOSE 3000
RUN apk add --no-cache curl
HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
CMD curl -f http://localhost:3000/health || exit 1
CMD ["node", "apps/api/dist/index.js"] CMD ["node", "apps/api/dist/index.js"]
# Migrate stage — runs drizzle-kit migrate against the database # Migrate stage — runs drizzle-kit migrate against the database
@@ -50,4 +47,4 @@ CMD ["pnpm", "db:seed"]
# Reset stage — drops all tables, re-runs migrations, and re-seeds # Reset stage — drops all tables, re-runs migrations, and re-seeds
FROM builder AS reset FROM builder AS reset
CMD ["pnpm", "db:reset"] CMD ["pnpm", "db:reset"]
+1 -1
View File
@@ -142,8 +142,8 @@ describe("auth init", () => {
...originalEnv, ...originalEnv,
AUTH_DISABLED: "true", AUTH_DISABLED: "true",
NODE_ENV: "test", NODE_ENV: "test",
BETTER_AUTH_SECRET: "placeholder-for-test-only",
}; };
delete process.env.BETTER_AUTH_SECRET;
const { initAuth, getAuth } = await reimportAuth(); const { initAuth, getAuth } = await reimportAuth();
await expect(initAuth()).resolves.toBeUndefined(); await expect(initAuth()).resolves.toBeUndefined();
+31 -11
View File
@@ -31,11 +31,11 @@ const BASE_APPT = {
// ─── Shared mock DB state ───────────────────────────────────────────────────── // ─── Shared mock DB state ─────────────────────────────────────────────────────
let mockAppt: typeof BASE_APPT | null = BASE_APPT; let mockAppt: (typeof BASE_APPT & { confirmationToken: string }) | null = BASE_APPT as typeof BASE_APPT & { confirmationToken: string };
let lastUpdate: Record<string, unknown> = {}; let lastUpdate: Record<string, unknown> = {};
function resetMock() { function resetMock() {
mockAppt = { ...BASE_APPT }; mockAppt = { ...BASE_APPT, confirmationToken: "valid-token-abc123" } as typeof BASE_APPT & { confirmationToken: string };
lastUpdate = {}; lastUpdate = {};
} }
@@ -55,19 +55,39 @@ vi.mock("@groombook/db", () => {
}), }),
}), }),
update: () => ({ update: () => ({
set: (vals: Record<string, unknown>) => ({ set: (vals: Record<string, unknown>) => {
where: () => { const setVals = vals;
lastUpdate = { ...vals }; return {
if (mockAppt) { where: () => {
mockAppt = { ...mockAppt, ...vals } as typeof BASE_APPT; const preUpdate = mockAppt ? { ...mockAppt } : null;
} const preStatus = preUpdate?.confirmationStatus;
return { returning: () => (mockAppt ? [mockAppt] : []) }; const preStart = preUpdate?.startTime;
}, lastUpdate = { ...setVals };
}), const whereMatched =
preUpdate != null &&
preStatus === "pending" &&
preStart != null &&
preStart > new Date();
if (whereMatched && mockAppt) {
mockAppt = { ...mockAppt, ...setVals } as typeof BASE_APPT & { confirmationToken: string };
}
return {
returning: () => {
if (!preUpdate) return [];
if (preStatus !== "pending") return [];
if (preStart && preStart <= new Date()) return [];
return whereMatched && mockAppt ? [mockAppt] : [];
},
};
},
};
},
}), }),
}), }),
appointments, appointments,
eq: () => ({}), eq: () => ({}),
and: (a: unknown, b: unknown, c?: unknown) => (c ? [a, b, c] : [a, b]),
gt: () => ({}),
}; };
}); });
+2 -2
View File
@@ -362,7 +362,7 @@ describe("requireRoleOrSuperUser", () => {
const res = await app.request("/test"); const res = await app.request("/test");
expect(res.status).toBe(403); expect(res.status).toBe(403);
const body = await res.json(); const body = await res.json();
expect(body.error).toMatch(/super user privileges required/i); expect(body.error).toMatch(/role 'receptionist' is not permitted/i);
}); });
it("blocks a non-super-user groomer from manager-only routes", async () => { it("blocks a non-super-user groomer from manager-only routes", async () => {
@@ -370,7 +370,7 @@ describe("requireRoleOrSuperUser", () => {
const res = await app.request("/test"); const res = await app.request("/test");
expect(res.status).toBe(403); expect(res.status).toBe(403);
const body = await res.json(); const body = await res.json();
expect(body.error).toMatch(/super user privileges required/i); expect(body.error).toMatch(/role 'groomer' is not permitted/i);
}); });
it("allows a manager with multiple allowed roles", async () => { it("allows a manager with multiple allowed roles", async () => {
+18 -16
View File
@@ -42,6 +42,23 @@ app.use(
}) })
); );
// CSRF protection for state-changing requests
app.use("/api/*", async (c, next) => {
const method = c.req.method;
if (["GET", "HEAD", "OPTIONS"].includes(method)) {
await next();
return;
}
const origin = c.req.header("origin");
const trustedOrigin = process.env.CORS_ORIGIN ?? "http://localhost:5173";
if (origin && origin !== trustedOrigin) {
c.status(403);
c.json({ error: "CSRF validation failed: origin mismatch" });
return;
}
await next();
});
// Health check (no auth required) // Health check (no auth required)
app.get("/health", (c) => c.json({ status: "ok" })); app.get("/health", (c) => c.json({ status: "ok" }));
@@ -187,24 +204,9 @@ api.route("/search", searchRouter);
const port = Number(process.env.PORT ?? 3000); const port = Number(process.env.PORT ?? 3000);
await initAuth(); await initAuth();
console.log(`API server listening on port ${port}`); console.log(`API server listening on port ${port}`);
const server = serve({ fetch: app.fetch, port }); serve({ fetch: app.fetch, port });
// Start background reminder scheduler (runs every minute to check for upcoming appointments) // Start background reminder scheduler (runs every minute to check for upcoming appointments)
startReminderScheduler(); startReminderScheduler();
function shutdown() {
console.log("Shutting down gracefully...");
server.close(() => {
console.log("HTTP server closed");
process.exit(0);
});
setTimeout(() => {
console.error("Forced shutdown after timeout");
process.exit(1);
}, 10_000);
}
process.on("SIGTERM", shutdown);
process.on("SIGINT", shutdown);
export default app; export default app;
+35 -11
View File
@@ -86,10 +86,15 @@ export async function initAuth(): Promise<void> {
// AUTH_DISABLED=true means dev/demo mode — still build Better-Auth with placeholder // AUTH_DISABLED=true means dev/demo mode — still build Better-Auth with placeholder
// config so auth.handler exists (middleware bypasses it anyway) // config so auth.handler exists (middleware bypasses it anyway)
if (process.env.AUTH_DISABLED === "true") { if (process.env.AUTH_DISABLED === "true") {
if (!BETTER_AUTH_SECRET) {
throw new Error(
"[FATAL] BETTER_AUTH_SECRET must be set when AUTH_DISABLED=true"
);
}
console.warn("[auth] AUTH_DISABLED=true — building placeholder auth instance"); console.warn("[auth] AUTH_DISABLED=true — building placeholder auth instance");
authInstance = betterAuth({ authInstance = betterAuth({
database: drizzleAdapter(getDb(), { provider: "pg" }), database: drizzleAdapter(getDb(), { provider: "pg" }),
secret: BETTER_AUTH_SECRET ?? "placeholder-secret-do-not-use-in-prod", secret: BETTER_AUTH_SECRET,
baseURL: BETTER_AUTH_URL, baseURL: BETTER_AUTH_URL,
rateLimit: { rateLimit: {
enabled: true, enabled: true,
@@ -199,20 +204,36 @@ export async function initAuth(): Promise<void> {
return url; return url;
} }
}; };
const validateIssuerHost = (url: string, issuerUrl: string): boolean => {
try {
const discovered = new URL(url);
const expected = new URL(issuerUrl);
return discovered.hostname === expected.hostname;
} catch {
return false;
}
};
const authzUrl = discovery.authorization_endpoint; const authzUrl = discovery.authorization_endpoint;
const tokenUrl = discovery.token_endpoint; const tokenUrl = discovery.token_endpoint;
const userInfoUrl = discovery.userinfo_endpoint; const userInfoUrl = discovery.userinfo_endpoint;
if (authzUrl && tokenUrl && userInfoUrl) { if (authzUrl && tokenUrl && userInfoUrl) {
oidcConfig = { const validAuthz = validateIssuerHost(authzUrl, providerConfig.issuerUrl);
authorizationUrl: authzUrl, const validToken = validateIssuerHost(tokenUrl, providerConfig.issuerUrl);
tokenUrl: providerConfig.internalBaseUrl const validUserInfo = validateIssuerHost(userInfoUrl, providerConfig.issuerUrl);
? replaceHost(tokenUrl, providerConfig.internalBaseUrl) if (!validAuthz || !validToken || !validUserInfo) {
: tokenUrl, console.warn("[auth] OIDC discovery URL host mismatch — possible redirection attack, rejecting");
userInfoUrl: providerConfig.internalBaseUrl } else {
? replaceHost(userInfoUrl, providerConfig.internalBaseUrl) oidcConfig = {
: userInfoUrl, authorizationUrl: authzUrl,
}; tokenUrl: providerConfig.internalBaseUrl
console.log("[auth] OIDC discovery successful, provider:", providerConfig.providerId); ? replaceHost(tokenUrl, providerConfig.internalBaseUrl)
: tokenUrl,
userInfoUrl: providerConfig.internalBaseUrl
? replaceHost(userInfoUrl, providerConfig.internalBaseUrl)
: userInfoUrl,
};
console.log("[auth] OIDC discovery successful, provider:", providerConfig.providerId);
}
} else { } else {
console.warn("[auth] OIDC discovery missing required endpoints, using discoveryUrl only"); console.warn("[auth] OIDC discovery missing required endpoints, using discoveryUrl only");
} }
@@ -287,6 +308,9 @@ export async function initAuth(): Promise<void> {
enabled: true, enabled: true,
maxAge: 5 * 60, // 5 minutes maxAge: 5 * 60, // 5 minutes
}, },
cookieAttributes: {
sameSite: "strict",
},
}, },
trustedOrigins: [process.env.CORS_ORIGIN ?? "http://localhost:5173"], trustedOrigins: [process.env.CORS_ORIGIN ?? "http://localhost:5173"],
}); });
+3 -3
View File
@@ -149,9 +149,9 @@ export function requireRoleOrSuperUser(
} }
return c.json( return c.json(
{ {
error: staffRow.isSuperUser error: hasAllowedRole
? `Forbidden: role '${staffRow.role}' is not permitted` ? "Forbidden: super user privileges required"
: "Forbidden: super user privileges required", : `Forbidden: role '${staffRow.role}' is not permitted`,
}, },
403 403
); );
+1 -71
View File
@@ -16,9 +16,8 @@ import {
services, services,
staff, staff,
} from "@groombook/db"; } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";
export const appointmentGroupsRouter = new Hono<AppEnv>(); export const appointmentGroupsRouter = new Hono();
// ─── Schemas ────────────────────────────────────────────────────────────────── // ─── Schemas ──────────────────────────────────────────────────────────────────
@@ -50,8 +49,6 @@ appointmentGroupsRouter.get("/", async (c) => {
const clientId = c.req.query("clientId"); const clientId = c.req.query("clientId");
const from = c.req.query("from"); const from = c.req.query("from");
const to = c.req.query("to"); const to = c.req.query("to");
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
const groupConditions = clientId const groupConditions = clientId
? [eq(appointmentGroups.clientId, clientId)] ? [eq(appointmentGroups.clientId, clientId)]
@@ -91,16 +88,6 @@ appointmentGroupsRouter.get("/", async (c) => {
})) }))
.filter((g) => !from || g.appointments.length > 0); .filter((g) => !from || g.appointments.length > 0);
if (isGroomer) {
return c.json(
result.filter((g) =>
g.appointments.some(
(a) => a.staffId === staffRow.id || a.batherStaffId === staffRow.id
)
)
);
}
return c.json(result); return c.json(result);
}); });
@@ -109,8 +96,6 @@ appointmentGroupsRouter.get("/", async (c) => {
appointmentGroupsRouter.get("/:id", async (c) => { appointmentGroupsRouter.get("/:id", async (c) => {
const db = getDb(); const db = getDb();
const id = c.req.param("id"); const id = c.req.param("id");
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
const [group] = await db const [group] = await db
.select() .select()
@@ -126,7 +111,6 @@ appointmentGroupsRouter.get("/:id", async (c) => {
serviceId: appointments.serviceId, serviceId: appointments.serviceId,
serviceName: services.name, serviceName: services.name,
staffId: appointments.staffId, staffId: appointments.staffId,
batherStaffId: appointments.batherStaffId,
staffName: staff.name, staffName: staff.name,
status: appointments.status, status: appointments.status,
startTime: appointments.startTime, startTime: appointments.startTime,
@@ -141,15 +125,6 @@ appointmentGroupsRouter.get("/:id", async (c) => {
.where(eq(appointments.groupId, id)) .where(eq(appointments.groupId, id))
.orderBy(appointments.startTime); .orderBy(appointments.startTime);
if (
isGroomer &&
!groupAppts.some(
(a) => a.staffId === staffRow.id || a.batherStaffId === staffRow.id
)
) {
return c.json({ error: "Forbidden" }, 403);
}
const [client] = await db const [client] = await db
.select({ name: clients.name, email: clients.email }) .select({ name: clients.name, email: clients.email })
.from(clients) .from(clients)
@@ -165,13 +140,6 @@ appointmentGroupsRouter.post(
zValidator("json", createGroupSchema), zValidator("json", createGroupSchema),
async (c) => { async (c) => {
const db = getDb(); const db = getDb();
const staffRow = c.get("staff");
if (staffRow?.role === "groomer") {
return c.json(
{ error: "Forbidden: groomers cannot create group bookings" },
403
);
}
const body = c.req.valid("json"); const body = c.req.valid("json");
const startTime = new Date(body.startTime); const startTime = new Date(body.startTime);
@@ -276,28 +244,6 @@ appointmentGroupsRouter.patch(
const db = getDb(); const db = getDb();
const id = c.req.param("id"); const id = c.req.param("id");
const body = c.req.valid("json"); const body = c.req.valid("json");
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
const [group] = await db
.select({ id: appointmentGroups.id })
.from(appointmentGroups)
.where(eq(appointmentGroups.id, id));
if (!group) return c.json({ error: "Not found" }, 404);
if (isGroomer) {
const groupAppts = await db
.select({ staffId: appointments.staffId, batherStaffId: appointments.batherStaffId })
.from(appointments)
.where(eq(appointments.groupId, id));
if (
!groupAppts.some(
(a) => a.staffId === staffRow.id || a.batherStaffId === staffRow.id
)
) {
return c.json({ error: "Forbidden" }, 403);
}
}
const [updated] = await db const [updated] = await db
.update(appointmentGroups) .update(appointmentGroups)
@@ -315,8 +261,6 @@ appointmentGroupsRouter.patch(
appointmentGroupsRouter.delete("/:id", async (c) => { appointmentGroupsRouter.delete("/:id", async (c) => {
const db = getDb(); const db = getDb();
const id = c.req.param("id"); const id = c.req.param("id");
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
const [group] = await db const [group] = await db
.select({ id: appointmentGroups.id }) .select({ id: appointmentGroups.id })
@@ -324,20 +268,6 @@ appointmentGroupsRouter.delete("/:id", async (c) => {
.where(eq(appointmentGroups.id, id)); .where(eq(appointmentGroups.id, id));
if (!group) return c.json({ error: "Not found" }, 404); if (!group) return c.json({ error: "Not found" }, 404);
if (isGroomer) {
const groupAppts = await db
.select({ staffId: appointments.staffId, batherStaffId: appointments.batherStaffId })
.from(appointments)
.where(eq(appointments.groupId, id));
if (
!groupAppts.some(
(a) => a.staffId === staffRow.id || a.batherStaffId === staffRow.id
)
) {
return c.json({ error: "Forbidden" }, 403);
}
}
await db await db
.update(appointments) .update(appointments)
.set({ status: "cancelled", updatedAt: new Date() }) .set({ status: "cancelled", updatedAt: new Date() })
+1 -56
View File
@@ -41,10 +41,6 @@ const createAppointmentSchema = z.object({
frequencyWeeks: z.number().int().min(1).max(52), frequencyWeeks: z.number().int().min(1).max(52),
count: z.number().int().min(2).max(52), count: z.number().int().min(2).max(52),
}) })
.refine(
(r) => r.frequencyWeeks * r.count <= 52,
{ message: "Recurrence series must not exceed 1 year" }
)
.optional(), .optional(),
}); });
@@ -167,28 +163,6 @@ appointmentsRouter.post(
} }
} }
if (apptFields.batherStaffId) {
const bathConflicts = await tx
.select({ id: appointments.id })
.from(appointments)
.where(
and(
or(
eq(appointments.staffId, apptFields.batherStaffId),
eq(appointments.batherStaffId, apptFields.batherStaffId)
),
lt(appointments.startTime, end),
gte(appointments.endTime, start),
ne(appointments.status, "cancelled"),
ne(appointments.status, "no_show"),
)
)
.limit(1);
if (bathConflicts.length > 0) {
throw Object.assign(new Error("conflict"), { statusCode: 409 });
}
}
if (!recurrence) { if (!recurrence) {
// Single appointment // Single appointment
const [inserted] = await tx const [inserted] = await tx
@@ -424,8 +398,7 @@ appointmentsRouter.patch(
const needsConflictCheck = const needsConflictCheck =
updateFields.startTime !== undefined || updateFields.startTime !== undefined ||
updateFields.endTime !== undefined || updateFields.endTime !== undefined ||
updateFields.staffId !== undefined || updateFields.staffId !== undefined;
updateFields.batherStaffId !== undefined;
const update: Record<string, unknown> = { const update: Record<string, unknown> = {
...updateFields, ...updateFields,
@@ -461,11 +434,6 @@ appointmentsRouter.patch(
updateFields.staffId !== undefined updateFields.staffId !== undefined
? updateFields.staffId ? updateFields.staffId
: current.staffId; : current.staffId;
// Use provided batherStaffId (may be null to unassign); fall back to existing
const batherStaffId =
updateFields.batherStaffId !== undefined
? updateFields.batherStaffId
: current.batherStaffId;
if (end <= start) { if (end <= start) {
throw Object.assign(new Error("end before start"), { throw Object.assign(new Error("end before start"), {
@@ -493,29 +461,6 @@ appointmentsRouter.patch(
} }
} }
if (batherStaffId) {
const bathConflicts = await tx
.select({ id: appointments.id })
.from(appointments)
.where(
and(
or(
eq(appointments.staffId, batherStaffId),
eq(appointments.batherStaffId, batherStaffId)
),
lt(appointments.startTime, end),
gte(appointments.endTime, start),
ne(appointments.status, "cancelled"),
ne(appointments.status, "no_show"),
ne(appointments.id, id),
)
)
.limit(1);
if (bathConflicts.length > 0) {
throw Object.assign(new Error("conflict"), { statusCode: 409 });
}
}
const [updated] = await tx const [updated] = await tx
.update(appointments) .update(appointments)
.set(update) .set(update)
+39 -53
View File
@@ -102,10 +102,7 @@ bookRouter.get("/availability", async (c) => {
const bookingSchema = z.object({ const bookingSchema = z.object({
serviceId: z.string().uuid(), serviceId: z.string().uuid(),
startTime: z.string().datetime().refine( startTime: z.string().datetime(),
(dt) => new Date(dt) > new Date(),
{ message: "Appointment must be in the future" }
),
clientName: z.string().min(1).max(200), clientName: z.string().min(1).max(200),
clientEmail: z.string().email(), clientEmail: z.string().email(),
clientPhone: z.string().max(50).optional(), clientPhone: z.string().max(50).optional(),
@@ -258,39 +255,37 @@ bookRouter.get("/confirm/:token", async (c) => {
const token = c.req.param("token"); const token = c.req.param("token");
const db = getDb(); const db = getDb();
// Atomic: consume token and confirm in a single query to prevent replay.
// Only future appointments can be confirmed.
const [appt] = await db const [appt] = await db
.select()
.from(appointments)
.where(eq(appointments.confirmationToken, token))
.limit(1);
if (!appt) {
return c.redirect(`${BASE_URL()}/booking/error`);
}
// Reject if appointment is in the past
if (appt.startTime < new Date()) {
return c.redirect(`${BASE_URL()}/booking/error`);
}
// Idempotent confirm: if already confirmed, redirect to success
if (appt.confirmationStatus === "confirmed") {
return c.redirect(`${BASE_URL()}/booking/confirmed`);
}
// Reject if already cancelled
if (appt.confirmationStatus === "cancelled") {
return c.redirect(`${BASE_URL()}/booking/error`);
}
await db
.update(appointments) .update(appointments)
.set({ .set({
confirmationStatus: "confirmed", confirmationStatus: "confirmed",
confirmedAt: new Date(), confirmedAt: new Date(),
confirmationToken: null,
updatedAt: new Date(), updatedAt: new Date(),
}) })
.where(eq(appointments.id, appt.id)); .where(
and(
eq(appointments.confirmationToken, token),
eq(appointments.confirmationStatus, "pending"),
gt(appointments.startTime, new Date())
)
)
.returning();
if (!appt) {
// Check status for idempotency: already-confirmed → redirect to confirmed
const [existing] = await db
.select({ confirmationStatus: appointments.confirmationStatus })
.from(appointments)
.where(eq(appointments.confirmationToken, token))
.limit(1);
if (existing?.confirmationStatus === "confirmed") {
return c.redirect(`${BASE_URL()}/booking/confirmed`);
}
return c.redirect(`${BASE_URL()}/booking/error`);
}
return c.redirect(`${BASE_URL()}/booking/confirmed`); return c.redirect(`${BASE_URL()}/booking/confirmed`);
}); });
@@ -302,29 +297,9 @@ bookRouter.get("/cancel/:token", async (c) => {
const token = c.req.param("token"); const token = c.req.param("token");
const db = getDb(); const db = getDb();
// Atomic: consume token and cancel in a single query to prevent replay.
// Only future appointments can be cancelled.
const [appt] = await db const [appt] = await db
.select()
.from(appointments)
.where(eq(appointments.confirmationToken, token))
.limit(1);
if (!appt) {
return c.redirect(`${BASE_URL()}/booking/error`);
}
// Reject if appointment is in the past
if (appt.startTime < new Date()) {
return c.redirect(`${BASE_URL()}/booking/error`);
}
// Reject if already cancelled (token was nullified — this path won't normally hit,
// but guard against edge cases where token lookup still works)
if (appt.confirmationStatus === "cancelled") {
return c.redirect(`${BASE_URL()}/booking/error`);
}
// Single-use cancellation: nullify token after use
await db
.update(appointments) .update(appointments)
.set({ .set({
confirmationStatus: "cancelled", confirmationStatus: "cancelled",
@@ -332,7 +307,18 @@ bookRouter.get("/cancel/:token", async (c) => {
confirmationToken: null, confirmationToken: null,
updatedAt: new Date(), updatedAt: new Date(),
}) })
.where(eq(appointments.id, appt.id)); .where(
and(
eq(appointments.confirmationToken, token),
eq(appointments.confirmationStatus, "pending"),
gt(appointments.startTime, new Date())
)
)
.returning();
if (!appt) {
return c.redirect(`${BASE_URL()}/booking/error`);
}
return c.redirect(`${BASE_URL()}/booking/cancelled`); return c.redirect(`${BASE_URL()}/booking/cancelled`);
}); });
+7 -2
View File
@@ -1,5 +1,5 @@
import { Hono } from "hono"; import { Hono } from "hono";
import { randomBytes } from "node:crypto"; import { randomBytes, timingSafeEqual } from "node:crypto";
import { import {
and, and,
eq, eq,
@@ -84,7 +84,12 @@ calendarRouter.get("/:staffId.ics", async (c) => {
.where(eq(staff.id, staffId)) .where(eq(staff.id, staffId))
.limit(1); .limit(1);
if (!staffMember || staffMember.icalToken !== token) { if (
!staffMember ||
!staffMember.icalToken ||
staffMember.icalToken.length !== token.length ||
!timingSafeEqual(Buffer.from(staffMember.icalToken), Buffer.from(token))
) {
return c.text("Unauthorized", 401); return c.text("Unauthorized", 401);
} }
+6 -93
View File
@@ -1,10 +1,9 @@
import { Hono } from "hono"; import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator"; import { zValidator } from "@hono/zod-validator";
import { z } from "zod/v3"; import { z } from "zod/v3";
import { and, desc, eq, getDb, groomingVisitLogs, appointments, or } from "@groombook/db"; import { desc, eq, getDb, groomingVisitLogs } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";
export const groomingLogsRouter = new Hono<AppEnv>(); export const groomingLogsRouter = new Hono();
const createLogSchema = z.object({ const createLogSchema = z.object({
petId: z.string().uuid(), petId: z.string().uuid(),
@@ -21,26 +20,6 @@ groomingLogsRouter.get("/", async (c) => {
const db = getDb(); const db = getDb();
const petId = c.req.query("petId"); const petId = c.req.query("petId");
if (!petId) return c.json({ error: "petId is required" }, 400); if (!petId) return c.json({ error: "petId is required" }, 400);
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
if (isGroomer) {
const [appt] = await db
.select({ id: appointments.id })
.from(appointments)
.where(
and(
eq(appointments.petId, petId),
or(
eq(appointments.staffId, staffRow.id),
eq(appointments.batherStaffId, staffRow.id)
)
)
)
.limit(1);
if (!appt) return c.json({ error: "Forbidden" }, 403);
}
const rows = await db const rows = await db
.select() .select()
.from(groomingVisitLogs) .from(groomingVisitLogs)
@@ -54,50 +33,11 @@ groomingLogsRouter.post(
zValidator("json", createLogSchema), zValidator("json", createLogSchema),
async (c) => { async (c) => {
const db = getDb(); const db = getDb();
const { groomedAt, petId, appointmentId, ...rest } = c.req.valid("json"); const { groomedAt, ...rest } = c.req.valid("json");
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
if (isGroomer) {
if (appointmentId) {
const [appt] = await db
.select({ id: appointments.id })
.from(appointments)
.where(
and(
eq(appointments.id, appointmentId),
or(
eq(appointments.staffId, staffRow.id),
eq(appointments.batherStaffId, staffRow.id)
)
)
)
.limit(1);
if (!appt) return c.json({ error: "Forbidden" }, 403);
} else {
const [appt] = await db
.select({ id: appointments.id })
.from(appointments)
.where(
and(
eq(appointments.petId, petId),
or(
eq(appointments.staffId, staffRow.id),
eq(appointments.batherStaffId, staffRow.id)
)
)
)
.limit(1);
if (!appt) return c.json({ error: "Forbidden" }, 403);
}
}
const [row] = await db const [row] = await db
.insert(groomingVisitLogs) .insert(groomingVisitLogs)
.values({ .values({
...rest, ...rest,
petId,
appointmentId: appointmentId ?? null,
groomedAt: groomedAt ? new Date(groomedAt) : new Date(), groomedAt: groomedAt ? new Date(groomedAt) : new Date(),
}) })
.returning(); .returning();
@@ -107,37 +47,10 @@ groomingLogsRouter.post(
groomingLogsRouter.delete("/:id", async (c) => { groomingLogsRouter.delete("/:id", async (c) => {
const db = getDb(); const db = getDb();
const id = c.req.param("id"); const [row] = await db
const staffRow = c.get("staff");
const isGroomer = staffRow?.role === "groomer";
const [log] = await db
.select()
.from(groomingVisitLogs)
.where(eq(groomingVisitLogs.id, id))
.limit(1);
if (!log) return c.json({ error: "Not found" }, 404);
if (isGroomer) {
const [appt] = await db
.select({ id: appointments.id })
.from(appointments)
.where(
and(
eq(appointments.petId, log.petId),
or(
eq(appointments.staffId, staffRow.id),
eq(appointments.batherStaffId, staffRow.id)
)
)
)
.limit(1);
if (!appt) return c.json({ error: "Forbidden" }, 403);
}
await db
.delete(groomingVisitLogs) .delete(groomingVisitLogs)
.where(eq(groomingVisitLogs.id, id)) .where(eq(groomingVisitLogs.id, c.req.param("id")))
.returning(); .returning();
if (!row) return c.json({ error: "Not found" }, 404);
return c.json({ ok: true }); return c.json({ ok: true });
}); });
+45 -53
View File
@@ -44,61 +44,53 @@ const updateInvoiceSchema = z.object({
}); });
// List invoices // List invoices
const listInvoicesQuerySchema = z.object({ invoicesRouter.get("/", async (c) => {
clientId: z.string().uuid().optional(), const db = getDb();
appointmentId: z.string().uuid().optional(), const clientId = c.req.query("clientId");
status: z.enum(["draft", "pending", "paid", "void"]).optional(), const appointmentId = c.req.query("appointmentId");
limit: z.coerce.number().int().min(1).max(200).default(50), const status = c.req.query("status");
offset: z.coerce.number().int().min(0).default(0), const limit = Math.min(parseInt(c.req.query("limit") || "50", 10), 200);
const offset = parseInt(c.req.query("offset") || "0", 10);
const conditions = [];
if (clientId) conditions.push(eq(invoices.clientId, clientId));
if (appointmentId) conditions.push(eq(invoices.appointmentId, appointmentId));
if (status) conditions.push(eq(invoices.status, status as "draft" | "pending" | "paid" | "void"));
const whereClause = conditions.length > 0 ? and(...conditions) : undefined;
const [totalResult] = await db
.select({ count: sql<number>`count(*)` })
.from(invoices)
.where(whereClause);
const rows = await db
.select({
id: invoices.id,
appointmentId: invoices.appointmentId,
clientId: invoices.clientId,
clientName: clients.name,
subtotalCents: invoices.subtotalCents,
taxCents: invoices.taxCents,
tipCents: invoices.tipCents,
totalCents: invoices.totalCents,
status: invoices.status,
paymentMethod: invoices.paymentMethod,
paidAt: invoices.paidAt,
notes: invoices.notes,
createdAt: invoices.createdAt,
updatedAt: invoices.updatedAt,
})
.from(invoices)
.leftJoin(clients, eq(invoices.clientId, clients.id))
.where(whereClause)
.orderBy(invoices.createdAt)
.limit(limit)
.offset(offset);
return c.json({ data: rows, total: totalResult?.count ?? 0 });
}); });
invoicesRouter.get(
"/",
zValidator("query", listInvoicesQuerySchema),
async (c) => {
const db = getDb();
const { clientId, appointmentId, status, limit, offset } = c.req.valid("query");
const conditions = [];
if (clientId) conditions.push(eq(invoices.clientId, clientId));
if (appointmentId) conditions.push(eq(invoices.appointmentId, appointmentId));
if (status) conditions.push(eq(invoices.status, status as "draft" | "pending" | "paid" | "void"));
const whereClause = conditions.length > 0 ? and(...conditions) : undefined;
const [totalResult] = await db
.select({ count: sql<number>`count(*)` })
.from(invoices)
.where(whereClause);
const rows = await db
.select({
id: invoices.id,
appointmentId: invoices.appointmentId,
clientId: invoices.clientId,
clientName: clients.name,
subtotalCents: invoices.subtotalCents,
taxCents: invoices.taxCents,
tipCents: invoices.tipCents,
totalCents: invoices.totalCents,
status: invoices.status,
paymentMethod: invoices.paymentMethod,
paidAt: invoices.paidAt,
notes: invoices.notes,
createdAt: invoices.createdAt,
updatedAt: invoices.updatedAt,
})
.from(invoices)
.leftJoin(clients, eq(invoices.clientId, clients.id))
.where(whereClause)
.orderBy(invoices.createdAt)
.limit(limit)
.offset(offset);
return c.json({ data: rows, total: totalResult?.count ?? 0 });
}
);
// Get single invoice with line items and tip splits // Get single invoice with line items and tip splits
invoicesRouter.get("/:id", async (c) => { invoicesRouter.get("/:id", async (c) => {
const db = getDb(); const db = getDb();
+3 -26
View File
@@ -286,10 +286,6 @@ reportsRouter.get("/clients", async (c) => {
ninetyDaysAgo.setUTCDate(ninetyDaysAgo.getUTCDate() - 90); ninetyDaysAgo.setUTCDate(ninetyDaysAgo.getUTCDate() - 90);
const ninetyDaysAgoISO = ninetyDaysAgo.toISOString(); const ninetyDaysAgoISO = ninetyDaysAgo.toISOString();
const page = Math.max(1, parseInt(c.req.query("page") ?? "1", 10) || 1);
const limit = Math.min(100, Math.max(1, parseInt(c.req.query("limit") ?? "20", 10) || 20));
const offset = (page - 1) * limit;
const churnRisk = await db const churnRisk = await db
.select({ .select({
clientId: clients.id, clientId: clients.id,
@@ -302,34 +298,15 @@ reportsRouter.get("/clients", async (c) => {
.having( .having(
sql`MAX(${appointments.startTime}) < ${ninetyDaysAgoISO}::timestamptz OR MAX(${appointments.startTime}) IS NULL` sql`MAX(${appointments.startTime}) < ${ninetyDaysAgoISO}::timestamptz OR MAX(${appointments.startTime}) IS NULL`
) )
.orderBy(sql`MAX(${appointments.startTime}) ASC NULLS FIRST`) .orderBy(sql`MAX(${appointments.startTime}) ASC NULLS FIRST`);
.limit(limit)
.offset(offset);
const [churnCountRow] = await db
.select({ total: sql<number>`count(*)::int` })
.from(
db
.select({ id: clients.id })
.from(clients)
.leftJoin(appointments, eq(appointments.clientId, clients.id))
.groupBy(clients.id)
.having(
sql`MAX(${appointments.startTime}) < ${ninetyDaysAgoISO}::timestamptz OR MAX(${appointments.startTime}) IS NULL`
)
.as("churn_count")
);
const churnRiskTotal = churnCountRow?.total ?? 0;
return c.json({ return c.json({
from: from.toISOString(), from: from.toISOString(),
to: to.toISOString(), to: to.toISOString(),
newClients, newClients,
activeInPeriodCount: activeInPeriod.length, activeInPeriodCount: activeInPeriod.length,
churnRisk, churnRisk: churnRisk.slice(0, 20), // top 20 at-risk clients
churnRiskTotal, churnRiskTotal: churnRisk.length,
page,
limit,
}); });
}); });
+1 -1
View File
@@ -9,7 +9,7 @@ const createServiceSchema = z.object({
name: z.string().min(1).max(200), name: z.string().min(1).max(200),
description: z.string().max(2000).optional(), description: z.string().max(2000).optional(),
basePriceCents: z.number().int().positive(), basePriceCents: z.number().int().positive(),
durationMinutes: z.number().int().positive().max(480), durationMinutes: z.number().int().positive(),
active: z.boolean().default(true), active: z.boolean().default(true),
}); });
+29
View File
@@ -6,6 +6,25 @@ import type { AppEnv } from "../middleware/rbac.js";
export const setupRouter = new Hono<AppEnv>(); export const setupRouter = new Hono<AppEnv>();
// Simple in-memory rate limiter: 10 req/min per IP for setup endpoints
const setupRateLimitMap = new Map<string, { count: number; resetAt: number }>();
const SETUP_RATE_LIMIT = 10;
const SETUP_RATE_WINDOW_MS = 60 * 1000;
function checkSetupRateLimit(ip: string): boolean {
const now = Date.now();
const entry = setupRateLimitMap.get(ip);
if (!entry || now > entry.resetAt) {
setupRateLimitMap.set(ip, { count: 1, resetAt: now + SETUP_RATE_WINDOW_MS });
return true;
}
if (entry.count >= SETUP_RATE_LIMIT) {
return false;
}
entry.count++;
return true;
}
// GET /api/setup/status — public (no auth), returns whether setup is needed // GET /api/setup/status — public (no auth), returns whether setup is needed
// and whether the auth provider bootstrap step should be shown // and whether the auth provider bootstrap step should be shown
setupRouter.get("/status", async (c) => { setupRouter.get("/status", async (c) => {
@@ -185,6 +204,11 @@ const authProviderTestSchema = z.object({
* After setup completes, this endpoint permanently returns 403. * After setup completes, this endpoint permanently returns 403.
*/ */
setupRouter.post("/auth-provider", async (c) => { setupRouter.post("/auth-provider", async (c) => {
const ip = c.req.header("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown";
if (!checkSetupRateLimit(ip)) {
return c.json({ error: "Too many requests. Please try again later." }, 429);
}
const db = getDb(); const db = getDb();
// Guard: only allow during fresh install (no super user yet) // Guard: only allow during fresh install (no super user yet)
@@ -254,6 +278,11 @@ setupRouter.post("/auth-provider", async (c) => {
* Only available when needsSetup is true (no super user = fresh install). * Only available when needsSetup is true (no super user = fresh install).
*/ */
setupRouter.post("/auth-provider/test", async (c) => { setupRouter.post("/auth-provider/test", async (c) => {
const ip = c.req.header("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown";
if (!checkSetupRateLimit(ip)) {
return c.json({ ok: false, error: "Too many requests. Please try again later." }, 429);
}
const db = getDb(); const db = getDb();
// Guard: only allow during fresh install (no super user yet) // Guard: only allow during fresh install (no super user yet)
+3 -10
View File
@@ -1,6 +1,5 @@
import { Hono } from "hono"; import { Hono } from "hono";
import Stripe from "stripe"; import Stripe from "stripe";
import { z } from "zod/v3";
import { eq, getDb, invoices } from "@groombook/db"; import { eq, getDb, invoices } from "@groombook/db";
import { getStripeClient } from "../services/payment.js"; import { getStripeClient } from "../services/payment.js";
@@ -45,13 +44,10 @@ webhooksRouter.post("/stripe", async (c) => {
const invoiceIds = pi.metadata.groombook_invoice_ids.split(","); const invoiceIds = pi.metadata.groombook_invoice_ids.split(",");
for (const invoiceId of invoiceIds) { for (const invoiceId of invoiceIds) {
if (!invoiceId) continue; if (!invoiceId) continue;
const parsed = z.string().uuid().safeParse(invoiceId.trim());
if (!parsed.success) continue;
const invoiceIdTrimmed = invoiceId.trim();
const [inv] = await db const [inv] = await db
.select() .select()
.from(invoices) .from(invoices)
.where(eq(invoices.id, invoiceIdTrimmed)) .where(eq(invoices.id, invoiceId))
.limit(1); .limit(1);
if (!inv) continue; if (!inv) continue;
if (inv.stripePaymentIntentId && inv.stripePaymentIntentId !== pi.id) continue; if (inv.stripePaymentIntentId && inv.stripePaymentIntentId !== pi.id) continue;
@@ -64,7 +60,7 @@ webhooksRouter.post("/stripe", async (c) => {
stripePaymentIntentId: pi.id, stripePaymentIntentId: pi.id,
updatedAt: new Date(), updatedAt: new Date(),
}) })
.where(eq(invoices.id, invoiceIdTrimmed)); .where(eq(invoices.id, invoiceId));
} }
} }
} else if (event.type === "payment_intent.payment_failed") { } else if (event.type === "payment_intent.payment_failed") {
@@ -73,16 +69,13 @@ webhooksRouter.post("/stripe", async (c) => {
const invoiceIds = pi.metadata.groombook_invoice_ids.split(","); const invoiceIds = pi.metadata.groombook_invoice_ids.split(",");
for (const invoiceId of invoiceIds) { for (const invoiceId of invoiceIds) {
if (!invoiceId) continue; if (!invoiceId) continue;
const parsed = z.string().uuid().safeParse(invoiceId.trim());
if (!parsed.success) continue;
const invoiceIdTrimmed = invoiceId.trim();
await db await db
.update(invoices) .update(invoices)
.set({ .set({
paymentFailureReason: pi.last_payment_error?.message ?? "Payment failed", paymentFailureReason: pi.last_payment_error?.message ?? "Payment failed",
updatedAt: new Date(), updatedAt: new Date(),
}) })
.where(eq(invoices.id, invoiceIdTrimmed)); .where(eq(invoices.id, invoiceId));
} }
} }
} else if (event.type === "charge.refunded") { } else if (event.type === "charge.refunded") {
-2
View File
@@ -20,5 +20,3 @@ FROM nginx:alpine AS runner
COPY apps/web/nginx.conf /etc/nginx/conf.d/default.conf COPY apps/web/nginx.conf /etc/nginx/conf.d/default.conf
COPY --from=builder /app/apps/web/dist /usr/share/nginx/html COPY --from=builder /app/apps/web/dist /usr/share/nginx/html
EXPOSE 80 EXPOSE 80
HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \
CMD curl -f http://localhost:80/ || exit 1
+1 -1
View File
@@ -567,7 +567,7 @@ async function seed() {
// ── Staff ── // ── Staff ──
const managerStaff = Array.from({ length: cfg.staffCount.manager }, (_, i) => const managerStaff = Array.from({ length: cfg.staffCount.manager }, (_, i) =>
({ id: uuid(), name: `Manager ${i + 1}`, email: `manager${i + 1}@groombook.dev`, role: "manager" as const, isSuperUser: profile === "uat" && i === 0 }) ({ id: uuid(), name: `Manager ${i + 1}`, email: `manager${i + 1}@groombook.dev`, role: "manager" as const, isSuperUser: false })
); );
const receptionistStaff = Array.from({ length: cfg.staffCount.receptionist }, (_, i) => const receptionistStaff = Array.from({ length: cfg.staffCount.receptionist }, (_, i) =>
({ id: uuid(), name: `Receptionist ${i + 1}`, email: `receptionist${i + 1}@groombook.dev`, role: "receptionist" as const, isSuperUser: false }) ({ id: uuid(), name: `Receptionist ${i + 1}`, email: `receptionist${i + 1}@groombook.dev`, role: "receptionist" as const, isSuperUser: false })