Step 5.6: validate requests and responses from the route catalog
CI / check (push) Successful in 1m1s
Details
CI / check (push) Successful in 1m1s
Details
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
768c893130
commit
7b26d57c4d
|
|
@ -244,3 +244,11 @@ Implementation decisions not spelled out in `memory-bank/implementation-plan.md`
|
|||
- **Errors:** an `ApiError(status, code, message)` thrown anywhere becomes `{ error: { code, message } }`. Unknown routes return `404 not-found`, and unexpected exceptions are logged and return `500 internal-error`.
|
||||
- **`GET /api/state` is implemented now without lazy resolution.** It reads ships, pilots (level and skill points derived with sim), bays, all locations with their discovered flag, tutorial state, in-flight dispatches and unacknowledged summaries. Step 6.1 adds resolution, and Step 6.2 its fuller tests.
|
||||
- **`buildApp` takes an optional `database`.** Tests pass the harness's connection; otherwise the app opens its own and closes it on shutdown.
|
||||
|
||||
## Step 5.6
|
||||
|
||||
- **The catalog is wired in with a small `registerRoute` helper**, not `fastify-type-provider-zod`. The catalog merges path parameters with the body into one request schema, which the type provider's separate params/body/query slots do not model.
|
||||
- **Request validation runs before the session check,** so a malformed request always gets `400 validation-error`, even without a session.
|
||||
- **Requests with no fields reject unexpected ones** (`emptyRequestSchema` is strict), including query parameters.
|
||||
- **Responses are validated everywhere except production.** A violation is logged and returned as `500 internal-error`, which makes tests fail loudly. Production skips the check to save work.
|
||||
- **Fastify's own 4xx errors** (malformed JSON, wrong content type) are reported as `400 validation-error` in the documented format.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { randomInt } from "node:crypto";
|
||||
import { content } from "@deadhead/content";
|
||||
import { routes, type PlayerState, type ServerTime } from "@deadhead/schema";
|
||||
import { routes } from "@deadhead/schema";
|
||||
import { realToGame } from "@deadhead/sim";
|
||||
import { fromNodeHeaders } from "better-auth/node";
|
||||
import Fastify, { type FastifyInstance } from "fastify";
|
||||
|
|
@ -11,6 +11,7 @@ import { createDatabase, type Database } from "./db/client";
|
|||
import { createPlayer } from "./game/new-player";
|
||||
import { loadPlayerState } from "./game/player-state";
|
||||
import { ApiError, registerErrorHandler } from "./http/errors";
|
||||
import { registerRoute, type RouteOptions } from "./http/route";
|
||||
import { requirePlayerId } from "./http/session";
|
||||
|
||||
export interface AppDeps {
|
||||
|
|
@ -33,15 +34,20 @@ export function buildApp(config: ServerConfig, deps: AppDeps = {}): FastifyInsta
|
|||
if (!deps.database) app.addHook("onClose", () => database.close());
|
||||
registerErrorHandler(app);
|
||||
|
||||
const options: RouteOptions = {
|
||||
requirePlayer: (request) => requirePlayerId(request, auth, db),
|
||||
validateResponses: config.env !== "production",
|
||||
};
|
||||
|
||||
// Under /api so it is reachable through the same-domain proxy (decision 0.5.20).
|
||||
app.get("/api/health", async () => ({ status: "ok" }));
|
||||
|
||||
app.get(routes.getServerTime.path, async (): Promise<ServerTime> => {
|
||||
registerRoute(app, routes.getServerTime, options, async () => {
|
||||
const realTime = clock();
|
||||
return { realTime, gameTime: realToGame(realTime, content.config) };
|
||||
});
|
||||
|
||||
app.post(routes.createGuest.path, async (request, reply) => {
|
||||
registerRoute(app, routes.createGuest, options, async (_input, { request, reply }) => {
|
||||
const signIn = await auth.api.signInAnonymous({
|
||||
headers: fromNodeHeaders(request.headers),
|
||||
returnHeaders: true,
|
||||
|
|
@ -59,10 +65,9 @@ export function buildApp(config: ServerConfig, deps: AppDeps = {}): FastifyInsta
|
|||
return { playerId, state: await loadPlayerState(db, playerId) };
|
||||
});
|
||||
|
||||
app.get(routes.getPlayerState.path, async (request): Promise<PlayerState> => {
|
||||
const playerId = await requirePlayerId(request, auth, db);
|
||||
return loadPlayerState(db, playerId);
|
||||
});
|
||||
registerRoute(app, routes.getPlayerState, options, async (_input, { playerId }) =>
|
||||
loadPlayerState(db, playerId),
|
||||
);
|
||||
|
||||
return app;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,11 @@ export function registerErrorHandler(app: FastifyInstance): void {
|
|||
if (error instanceof ApiError) {
|
||||
return reply.status(error.status).send(errorBody(error.code, error.message));
|
||||
}
|
||||
// Fastify's own 4xx errors, e.g. a body that is not valid JSON.
|
||||
const status = (error as { statusCode?: number }).statusCode;
|
||||
if (status !== undefined && status >= 400 && status < 500) {
|
||||
return reply.status(400).send(errorBody("validation-error", (error as Error).message));
|
||||
}
|
||||
request.log.error(error);
|
||||
return reply.status(500).send(errorBody("internal-error", "Something went wrong."));
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
import type { RouteDef } from "@deadhead/schema";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import type { z } from "zod";
|
||||
import { ApiError } from "./errors";
|
||||
|
||||
export interface RouteContext<Auth extends boolean> {
|
||||
/** The signed-in player; only present on routes that require a session. */
|
||||
playerId: Auth extends true ? string : null;
|
||||
request: FastifyRequest;
|
||||
reply: FastifyReply;
|
||||
}
|
||||
|
||||
export interface RouteOptions {
|
||||
/** Resolves the session to a player id, or throws `401 unauthorized`. */
|
||||
requirePlayer: (request: FastifyRequest) => Promise<string>;
|
||||
/** Check responses against their schema (everywhere except production). */
|
||||
validateResponses: boolean;
|
||||
}
|
||||
|
||||
function describeIssues(error: z.ZodError): string {
|
||||
return error.issues
|
||||
.map((issue) => (issue.path.length ? `${issue.path.join(".")}: ` : "") + issue.message)
|
||||
.join("; ");
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers one catalog route. The request (path parameters, query and body
|
||||
* merged) is validated before anything else, then the session is checked if the
|
||||
* route needs one, and the response is checked against its schema.
|
||||
*/
|
||||
export function registerRoute<R extends RouteDef>(
|
||||
app: FastifyInstance,
|
||||
route: R,
|
||||
options: RouteOptions,
|
||||
handler: (
|
||||
input: z.output<R["request"]>,
|
||||
context: RouteContext<R["auth"]>,
|
||||
) => Promise<z.input<R["response"]>>,
|
||||
): void {
|
||||
app.route({
|
||||
method: route.method,
|
||||
url: route.path,
|
||||
handler: async (request, reply) => {
|
||||
const raw = {
|
||||
...(request.query as object),
|
||||
...(typeof request.body === "object" && request.body !== null ? request.body : {}),
|
||||
...(request.params as object),
|
||||
};
|
||||
const parsed = route.request.safeParse(raw);
|
||||
if (!parsed.success) {
|
||||
throw new ApiError(400, "validation-error", describeIssues(parsed.error));
|
||||
}
|
||||
|
||||
const playerId = route.auth ? await options.requirePlayer(request) : null;
|
||||
const result = await handler(parsed.data as z.output<R["request"]>, {
|
||||
playerId: playerId as RouteContext<R["auth"]>["playerId"],
|
||||
request,
|
||||
reply,
|
||||
});
|
||||
|
||||
if (options.validateResponses) {
|
||||
const checked = route.response.safeParse(result);
|
||||
if (!checked.success) {
|
||||
throw new Error(
|
||||
`Response for ${route.method} ${route.path} violates its schema: ${describeIssues(checked.error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
@ -0,0 +1,84 @@
|
|||
import { errorResponseSchema, routes } from "@deadhead/schema";
|
||||
import Fastify from "fastify";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { buildApp } from "../src/app";
|
||||
import { registerErrorHandler } from "../src/http/errors";
|
||||
import { registerRoute } from "../src/http/route";
|
||||
import { useTestDatabase } from "./db";
|
||||
import { testConfig } from "./helpers";
|
||||
|
||||
const database = useTestDatabase();
|
||||
|
||||
describe("request validation from the route catalog", () => {
|
||||
let app: ReturnType<typeof buildApp>;
|
||||
beforeEach(() => {
|
||||
app = buildApp(testConfig(), { database: database() });
|
||||
});
|
||||
afterEach(() => app.close());
|
||||
|
||||
const expectValidationError = (response: { statusCode: number; json(): unknown }) => {
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(errorResponseSchema.parse(response.json()).error.code).toBe("validation-error");
|
||||
};
|
||||
|
||||
it("rejects unexpected fields on a route that takes none", async () => {
|
||||
expectValidationError(
|
||||
await app.inject({ method: "POST", url: "/api/guest", payload: { surprise: true } }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a body that is not valid JSON", async () => {
|
||||
expectValidationError(
|
||||
await app.inject({
|
||||
method: "POST",
|
||||
url: "/api/guest",
|
||||
headers: { "content-type": "application/json" },
|
||||
payload: "{not json",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects unexpected query parameters on an authenticated route before checking the session", async () => {
|
||||
expectValidationError(await app.inject({ method: "GET", url: "/api/state?evil=1" }));
|
||||
});
|
||||
|
||||
it("still answers valid requests", async () => {
|
||||
expect((await app.inject({ method: "GET", url: "/api/time" })).statusCode).toBe(200);
|
||||
});
|
||||
|
||||
it("returns not-found in the error format for unknown endpoints", async () => {
|
||||
const response = await app.inject({ method: "GET", url: "/api/nope" });
|
||||
expect(response.statusCode).toBe(404);
|
||||
expect(errorResponseSchema.parse(response.json()).error.code).toBe("not-found");
|
||||
});
|
||||
});
|
||||
|
||||
describe("response validation", () => {
|
||||
const build = (validateResponses: boolean) => {
|
||||
const app = Fastify({ logger: false });
|
||||
registerErrorHandler(app);
|
||||
// A handler that breaks the server-time response schema.
|
||||
registerRoute(
|
||||
app,
|
||||
routes.getServerTime,
|
||||
{ requirePlayer: async () => "unused", validateResponses },
|
||||
async () => ({ realTime: "not a number" }) as never,
|
||||
);
|
||||
return app;
|
||||
};
|
||||
|
||||
it("fails a response that violates its schema in test mode", async () => {
|
||||
const app = build(true);
|
||||
const response = await app.inject({ method: "GET", url: "/api/time" });
|
||||
expect(response.statusCode).toBe(500);
|
||||
expect(errorResponseSchema.parse(response.json()).error.code).toBe("internal-error");
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it("does not check responses when validation is off (production)", async () => {
|
||||
const app = build(false);
|
||||
const response = await app.inject({ method: "GET", url: "/api/time" });
|
||||
expect(response.statusCode).toBe(200);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
|
@ -365,3 +365,14 @@ export const accountRelations = relations(account, ({ one }) => ({
|
|||
## Auth and sessions
|
||||
|
||||
Better Auth (`apps/server/src/auth.ts`) with the anonymous plugin, mounted only through its server API (no `/api/auth/*` HTTP handler yet). `POST /api/guest` calls `signInAnonymous`, forwards its session cookie, and creates the player (`createPlayer`) linked to the new user. Authenticated routes call `requirePlayerId`, which resolves the cookie to a session, then to `players.user_id`; failures are `401 unauthorized`.
|
||||
|
||||
## Request pipeline (`apps/server/src/http`)
|
||||
|
||||
Every catalog endpoint is registered with `registerRoute(app, routes.<name>, options, handler)`:
|
||||
|
||||
1. Merge path parameters, query and body, and validate them against the route's `request` schema → `400 validation-error` on failure (also for malformed JSON).
|
||||
2. If `route.auth`, resolve the session to a player id → `401 unauthorized` on failure.
|
||||
3. Run the handler with the typed input and `{ playerId, request, reply }`.
|
||||
4. Outside production, validate the result against the route's `response` schema; a mismatch is logged and returned as `500 internal-error`.
|
||||
|
||||
Handlers throw `ApiError(status, code, message)` for expected failures. Unknown paths return `404 not-found`.
|
||||
|
|
|
|||
|
|
@ -50,4 +50,4 @@
|
|||
- [x] **Step 5.3 — Database schema and migrations.** Drizzle schema (`apps/server/src/db/schema.ts`, copied into `architecture.md`), first migration `drizzle/0000_init.sql`, `db:generate` / `db:migrate` scripts. Migration test runs against an emptied test database twice. CI now has a Postgres 18 service; server tests run one file at a time.
|
||||
- [x] **Step 5.4 — Test database harness.** `apps/server/test/db.ts`: `useTestDatabase()` (migrate once, truncate all tables before each test) and `seedPlayer(db, "tutorial" | "owner")`. New-player creation lives in `src/game/new-player.ts` (`createPlayer`), shared with guest sign-up.
|
||||
- [x] **Step 5.5 — Guest accounts.** Better Auth with the anonymous plugin (`src/auth.ts`, tables in `src/db/auth-schema.ts`, migration `0001_auth`). `POST /api/guest` creates a guest session and a tutorial player; `GET /api/state` returns player state for the session or `401 unauthorized`. Error handler sends the documented error format.
|
||||
- [ ] Step 5.6 — Request validation from the route catalog
|
||||
- [x] **Step 5.6 — Request validation from the route catalog.** `registerRoute` (`src/http/route.ts`) validates merged params/query/body before auth, checks sessions for `auth` routes, and checks responses outside production. Malformed JSON and unexpected fields return `400 validation-error`.
|
||||
|
|
|
|||
Loading…
Reference in New Issue