refactor: validate audit and analytics query strings with zod

packages/control-plane/src/routes/analytics.test.ts

@@ -8,7 +8,7 @@ import {
   TEST_SERVICE_SECRETS,
 } from "../router.test-support";
 import type { Env } from "../types";
-import { analyticsRoutes } from "./analytics";
+import { analyticsRoutes, DEFAULT_ANALYTICS_DAYS } from "./analytics";
 
 const FIXED_NOW = 1_700_000_000_000;
 
@@ -114,7 +114,7 @@ describe("analytics route handlers", () => {
       const response = await callRoute("GET", "/analytics/summary");
       expect(response.status).toBe(200);
       expect(mockStore.getSummary).toHaveBeenCalledWith({
-        startAt: FIXED_NOW - 30 * 24 * 60 * 60 * 1000,
+        startAt: FIXED_NOW - DEFAULT_ANALYTICS_DAYS * 24 * 60 * 60 * 1000,
         endAt: FIXED_NOW,
         spawnSources: HUMAN_SPAWN_SOURCES,
       });
@@ -178,6 +178,57 @@ describe("analytics route handlers", () => {
     });
   });
 
+  describe("query strings", () => {
+    it.each(["7", "14", "30", "90"])("accepts days=%s", async (days) => {
+      mockStore.getSummary.mockResolvedValue({ ok: true });
+
+      const response = await callRoute("GET", `/analytics/summary?days=${days}`);
+
+      expect(response.status).toBe(200);
+      expect(mockStore.getSummary).toHaveBeenCalledWith(
+        expect.objectContaining({
+          startAt: FIXED_NOW - Number(days) * 24 * 60 * 60 * 1000,
+          endAt: FIXED_NOW,
+        })
+      );
+    });
+
+    it.each(["", "0", "8", "abc", "1e1"])("rejects days=%s", async (days) => {
+      const response = await callRoute("GET", `/analytics/summary?days=${days}`);
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({
+        error: "days must be one of: 7, 14, 30, 90",
+      });
+      expect(mockStore.getSummary).not.toHaveBeenCalled();
+    });
+
+    it("rejects a repeated days key", async () => {
+      const response = await callRoute("GET", "/analytics/summary?days=7&days=14");
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({ error: "Invalid days" });
+    });
+
+    it("rejects an empty or repeated by key", async () => {
+      const empty = await callRoute("GET", "/analytics/breakdown?days=30&by=");
+      expect(empty.status).toBe(400);
+      await expect(empty.json()).resolves.toEqual({ error: "by must be one of: user, repo" });
+
+      const repeated = await callRoute("GET", "/analytics/breakdown?days=30&by=user&by=repo");
+      expect(repeated.status).toBe(400);
+      await expect(repeated.json()).resolves.toEqual({ error: "Invalid by" });
+    });
+
+    it("reports days before by when both are invalid", async () => {
+      const response = await callRoute("GET", "/analytics/breakdown?days=1&by=nope");
+
+      await expect(response.json()).resolves.toEqual({
+        error: "days must be one of: 7, 14, 30, 90",
+      });
+    });
+  });
+
   it("denies a request without analytics permission before touching a store", async () => {
     mocks.authenticate.mockImplementation(async () => ({
       reason: "Unauthorized",

packages/control-plane/src/routes/analytics.ts

@@ -1,7 +1,6 @@
 import {
   ANALYTICS_BREAKDOWN_BY,
   ANALYTICS_DAYS,
-  type AnalyticsBreakdownBy,
   type AnalyticsDays,
 } from "@open-inspect/shared/types/analytics";
 import { type AnalyticsFilters, AnalyticsStore, HUMAN_SPAWN_SOURCES } from "../db/analytics-store";
@@ -11,29 +10,35 @@ import {
   PullRequestAnalyticsStore,
 } from "../db/pull-request-analytics-store";
 import { Hono } from "hono";
+import { z } from "zod";
 import { admit } from "../routing/admit";
 import type { ControlPlaneHonoEnv } from "../routing/hono-env";
+import { parseQuery } from "./query";
 import {
   type RequestContext,
   SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE,
-  error,
   json,
   requirePermission,
 } from "./shared";
 
-function parseDaysParam(value: string | null): AnalyticsDays | null {
-  if (value === null) return 30;
-
-  const parsed = Number(value);
-  return ANALYTICS_DAYS.includes(parsed as AnalyticsDays) ? (parsed as AnalyticsDays) : null;
-}
+export const DEFAULT_ANALYTICS_DAYS: AnalyticsDays = 30;
+
+/** The reporting window; absent, the default. The value is read the way `Number()` reads it. */
+const daysQuery = z.object({
+  days: z
+    .string()
+    .optional()
+    .transform((raw) => (raw === undefined ? DEFAULT_ANALYTICS_DAYS : Number(raw)))
+    .pipe(
+      z.literal(ANALYTICS_DAYS, { error: `days must be one of: ${ANALYTICS_DAYS.join(", ")}` })
+    ),
+});
 
-function parseBreakdownBy(value: string | null): AnalyticsBreakdownBy | null {
-  if (!value) return null;
-  return ANALYTICS_BREAKDOWN_BY.includes(value as AnalyticsBreakdownBy)
-    ? (value as AnalyticsBreakdownBy)
-    : null;
-}
+const breakdownQuery = daysQuery.extend({
+  by: z.enum(ANALYTICS_BREAKDOWN_BY, {
+    error: `by must be one of: ${ANALYTICS_BREAKDOWN_BY.join(", ")}`,
+  }),
+});
 
 function getFilters(days: AnalyticsDays): AnalyticsFilters {
   const endAt = Date.now();
@@ -52,11 +57,9 @@ function getPullRequestFilters(days: AnalyticsDays): PullRequestAnalyticsFilters
 }
 
 async function handleDashboard(request: Request, ctx: RequestContext): Promise<Response> {
-  const url = new URL(request.url);
-  const days = parseDaysParam(url.searchParams.get("days"));
-  if (!days) {
-    return error(`days must be one of: ${ANALYTICS_DAYS.join(", ")}`, 400);
-  }
+  const query = parseQuery(request, daysQuery);
+  if (query instanceof Response) return query;
+  const { days } = query;
 
   const generatedAt = Date.now();
   const store = new AnalyticsDashboardStore(ctx.db);
@@ -70,50 +73,36 @@ async function handleDashboard(request: Request, ctx: RequestContext): Promise<R
 }
 
 async function handleSummary(request: Request, ctx: RequestContext): Promise<Response> {
-  const url = new URL(request.url);
-  const days = parseDaysParam(url.searchParams.get("days"));
-  if (!days) {
-    return error(`days must be one of: ${ANALYTICS_DAYS.join(", ")}`, 400);
-  }
+  const query = parseQuery(request, daysQuery);
+  if (query instanceof Response) return query;
+  const { days } = query;
 
   const store = new AnalyticsStore(ctx.db);
   return json(await store.getSummary(getFilters(days)));
 }
 
 async function handleTimeseries(request: Request, ctx: RequestContext): Promise<Response> {
-  const url = new URL(request.url);
-  const days = parseDaysParam(url.searchParams.get("days"));
-  if (!days) {
-    return error(`days must be one of: ${ANALYTICS_DAYS.join(", ")}`, 400);
-  }
+  const query = parseQuery(request, daysQuery);
+  if (query instanceof Response) return query;
+  const { days } = query;
 
   const store = new AnalyticsStore(ctx.db);
   return json(await store.getTimeseries(getFilters(days)));
 }
 
 async function handleBreakdown(request: Request, ctx: RequestContext): Promise<Response> {
-  const url = new URL(request.url);
-  const days = parseDaysParam(url.searchParams.get("days"));
-  if (!days) {
-    return error(`days must be one of: ${ANALYTICS_DAYS.join(", ")}`, 400);
-  }
-
-  const byParam = url.searchParams.get("by");
-  const by = parseBreakdownBy(byParam);
-  if (!by) {
-    return error(`by must be one of: ${ANALYTICS_BREAKDOWN_BY.join(", ")}`, 400);
-  }
+  const query = parseQuery(request, breakdownQuery);
+  if (query instanceof Response) return query;
+  const { days, by } = query;
 
   const store = new AnalyticsStore(ctx.db);
   return json(await store.getBreakdown(getFilters(days), by));
 }
 
 async function handlePullRequests(request: Request, ctx: RequestContext): Promise<Response> {
-  const url = new URL(request.url);
-  const days = parseDaysParam(url.searchParams.get("days"));
-  if (!days) {
-    return error(`days must be one of: ${ANALYTICS_DAYS.join(", ")}`, 400);
-  }
+  const query = parseQuery(request, daysQuery);
+  if (query instanceof Response) return query;
+  const { days } = query;
 
   const store = new PullRequestAnalyticsStore(ctx.db);
   return json(await store.get(getPullRequestFilters(days)));

packages/control-plane/src/routes/audit-events.test.ts

@@ -0,0 +1,122 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type * as AuthenticateModule from "../auth/authenticate";
+import { encodeAuditEventCursor } from "../db/audit-event-cursor";
+import {
+  createTestRequestHandler,
+  ownerAuthorizationDatabase,
+  TEST_BACKGROUND_TASK_CONTEXT,
+  TEST_SERVICE_SECRETS,
+} from "../router.test-support";
+import type { Env } from "../types";
+import { auditEventRoutes, DEFAULT_AUDIT_EVENT_LIMIT } from "./audit-events";
+
+const mockStore = { list: vi.fn() };
+const mocks = vi.hoisted(() => ({ authenticate: vi.fn() }));
+
+vi.mock("../auth/authenticate", async (importOriginal) => ({
+  ...(await importOriginal<typeof AuthenticateModule>()),
+  authenticate: mocks.authenticate,
+}));
+
+vi.mock("../db/audit-event-store", async (importOriginal) => {
+  const actual = (await importOriginal()) as Record<string, unknown>;
+  return {
+    ...actual,
+    AuditEventStore: vi.fn().mockImplementation(function () {
+      return mockStore;
+    }),
+  };
+});
+
+const handleRequest = createTestRequestHandler([auditEventRoutes]);
+const env = { ...TEST_SERVICE_SECRETS, DB: ownerAuthorizationDatabase() } as unknown as Env;
+
+function list(query = ""): Promise<Response> {
+  return handleRequest(
+    new Request(`https://test.local/audit-events${query}`),
+    env,
+    TEST_BACKGROUND_TASK_CONTEXT
+  );
+}
+
+describe("audit events route", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.authenticate.mockImplementation(async (request: Request) => ({
+      principal: { kind: "user", userId: "user-1" },
+      request,
+    }));
+    mockStore.list.mockResolvedValue({ rows: [], hasMore: false, nextCursor: null });
+  });
+
+  it("defaults the limit with no cursor", async () => {
+    const response = await list();
+
+    expect(response.status).toBe(200);
+    expect(mockStore.list).toHaveBeenCalledWith({ limit: DEFAULT_AUDIT_EVENT_LIMIT, cursor: null });
+    await expect(response.json()).resolves.toEqual({
+      events: [],
+      hasMore: false,
+      nextCursor: null,
+    });
+  });
+
+  it("accepts the maximum limit", async () => {
+    expect((await list("?limit=100")).status).toBe(200);
+    expect(mockStore.list).toHaveBeenCalledWith({ limit: 100, cursor: null });
+  });
+
+  it("round-trips a cursor through the store and the next page", async () => {
+    const cursor = { occurredAt: 1_700_000_000_000, id: "event-1" };
+    const next = { occurredAt: 1_699_999_999_000, id: "event-2" };
+    mockStore.list.mockResolvedValue({ rows: [], hasMore: true, nextCursor: next });
+
+    const response = await list(`?limit=1&cursor=${encodeAuditEventCursor(cursor)}`);
+
+    expect(response.status).toBe(200);
+    expect(mockStore.list).toHaveBeenCalledWith({ limit: 1, cursor });
+    await expect(response.json()).resolves.toMatchObject({
+      hasMore: true,
+      nextCursor: encodeAuditEventCursor(next),
+    });
+  });
+
+  it.each(["0", "101", "1.5", "1e2", "+5", "-5", "abc", "", "01"])(
+    "rejects limit=%s before reading the store",
+    async (limit) => {
+      const response = await list(`?limit=${limit}`);
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({ error: "Invalid limit" });
+      expect(mockStore.list).not.toHaveBeenCalled();
+    }
+  );
+
+  it.each(["", "not-a-cursor", "100:", ":event-1"])(
+    "rejects cursor=%s before reading the store",
+    async (cursor) => {
+      const response = await list(`?cursor=${cursor}`);
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({ error: "Invalid cursor" });
+      expect(mockStore.list).not.toHaveBeenCalled();
+    }
+  );
+
+  it.each([
+    ["limit=1&limit=2", "Invalid limit"],
+    ["cursor=one&cursor=two", "Invalid cursor"],
+  ])("rejects a repeated key (%s)", async (query, message) => {
+    const response = await list(`?${query}`);
+
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({ error: message });
+    expect(mockStore.list).not.toHaveBeenCalled();
+  });
+
+  it("reports the limit before the cursor when both are invalid", async () => {
+    const response = await list("?limit=0&cursor=bad");
+
+    await expect(response.json()).resolves.toEqual({ error: "Invalid limit" });
+  });
+});