refactor(actions): parse server-action input with zod

src/app/account/actions.ts

@@ -1,5 +1,6 @@
 "use server";
 
+import { z } from "zod";
 import { and, eq } from "drizzle-orm";
 import { revalidatePath } from "next/cache";
 import { redirect } from "next/navigation";
@@ -12,6 +13,37 @@ import { setMainCharacter, unlinkCharacter, wakeSelf } from "@/services/accounts
 import { unlinkDiscord } from "@/services/discord-link";
 import { getSessionAccount } from "@/services/session";
 
+/** `setMainAction`/`unlinkAction`'s one bound argument. Neither action reads
+ *  any FormData at all — `account/page.tsx` renders zero named controls — but
+ *  a bound server-action argument is still caller input on the wire, not
+ *  trusted state, so it is parsed rather than cast. Unreachable from the
+ *  rendered page with anything but a real character id, so a bad value throws
+ *  rather than earning notice copy — the same posture the admin actions take
+ *  on their own unreachable inputs (`access-lists/actions.ts`'s `parseId`,
+ *  `sync/actions.ts`'s `jobTypeSchema`). Both callers parse AFTER
+ *  `requireAccount()`, not before — same order those two siblings use — so a
+ *  caller who is not signed in gets `requireAccount`'s own redirect regardless
+ *  of what they sent, rather than a thrown validation error telling an
+ *  unauthenticated request whether its argument even had the right shape.
+ *
+ *  The code is spelled on every step rather than only the last:
+ *  `parseCharacterId` throws what it reads off the rejected issue, and
+ *  `z.number().int().positive({ error })` attaches the code to `positive`
+ *  alone — so a non-integer would otherwise surface zod's own wording
+ *  ("Invalid input: expected int, received number") as the thrown message. */
+const characterIdSchema = z
+  .number({ error: "invalid_character_id" })
+  .int({ error: "invalid_character_id" })
+  .positive({ error: "invalid_character_id" });
+
+function parseCharacterId(value: number): number {
+  const parsed = characterIdSchema.safeParse(value);
+  if (!parsed.success) {
+    throw new Error(parsed.error.issues[0]?.message ?? "invalid_character_id");
+  }
+  return parsed.data;
+}
+
 async function requireAccount(): Promise<string> {
   const cfg = getConfig();
   const sid = (await cookies()).get(cfg.sessionCookieName)?.value;
@@ -26,6 +58,7 @@ async function requireAccount(): Promise<string> {
 
 export async function setMainAction(characterId: number): Promise<void> {
   const accountId = await requireAccount();
+  characterId = parseCharacterId(characterId);
   const result = await getDb().transaction((dbtx) =>
     setMainCharacter(dbtx, accountId, accountId, characterId),
   );
@@ -52,6 +85,7 @@ export async function setMainAction(characterId: number): Promise<void> {
 
 export async function unlinkAction(characterId: number): Promise<void> {
   const accountId = await requireAccount();
+  characterId = parseCharacterId(characterId);
   const db = getDb();
   const cfg = getConfig();
   // members may only unlink their own characters. This is a fast, non-locking

src/app/admin/access-lists/actions.ts

@@ -1,5 +1,6 @@
 "use server";
 
+import { z } from "zod";
 import { revalidatePath } from "next/cache";
 import { redirect } from "next/navigation";
 import { getDb } from "@/db";
@@ -17,14 +18,35 @@ import { type ActionOutcome } from "@/app/_components/confirm-group";
  * worker performs every read.
  */
 
-/** A server action takes whatever the wire sends, so an id that will become a
- *  bigint column and an audit target is parsed rather than trusted.
- *  Unreachable from the rendered page, so a bad value throws rather than
- *  earning notice copy — the same posture `syncJobAction` takes on `jobType`. */
+/** An id that will become a bigint column and an audit target, parsed with
+ *  zod rather than cast — a server action takes whatever the wire sends.
+ *  `removeWatchAction`'s `accessListId` arrives as a submit button's own
+ *  name/value, not a hidden input, so a scripted POST with no submitter gives
+ *  `null`; the input type is `FormDataEntryValue | null`, never bare `string`. */
+const idSchema = z.preprocess(
+  (value) => Number(value),
+  // The `error` on the type gate is not redundant with the refine's, and both
+  // are read: `parseId` below throws the code it takes off the rejected issue,
+  // so a path left without one would surface zod's own generated wording as
+  // the thrown message. `Number()` runs first and maps a non-numeric spelling
+  // to `NaN`, which `z.number()` rejects at the gate — the refine never runs —
+  // so the most likely bad input ("12abc") rejects through the gate while a
+  // well-formed-but-out-of-range one ("-1") rejects through the refine. Both
+  // spell it the same way, so the caller gets `invalid_id` either way.
+  z
+    .number({ error: "invalid_id" })
+    .refine((n) => Number.isSafeInteger(n) && n > 0, { error: "invalid_id" }),
+);
+
+/** Unreachable from the rendered page, so a bad value throws rather than
+ *  earning notice copy — the same posture `syncJobAction` takes on `jobType`.
+ *  The code comes off the rejected issue rather than being restated here, so
+ *  `invalid_id` has one spelling (the schema's) rather than two that can
+ *  drift; same shape as `admin/accounts/actions.ts`'s `assertValid`. */
 function parseId(value: FormDataEntryValue | null): number {
-  const n = Number(value);
-  if (!Number.isSafeInteger(n) || n <= 0) throw new Error("invalid_id");
-  return n;
+  const parsed = idSchema.safeParse(value);
+  if (!parsed.success) throw new Error(parsed.error.issues[0]?.message ?? "invalid_id");
+  return parsed.data;
 }
 
 export async function designateHolderAction(formData: FormData): Promise<void> {

src/app/admin/accounts/actions.ts

@@ -1,5 +1,6 @@
 "use server";
 
+import { z } from "zod";
 import { revalidatePath } from "next/cache";
 import { redirect } from "next/navigation";
 import { getDb } from "@/db";
@@ -27,6 +28,62 @@ import {
   type AdminAccountsDoneCode,
 } from "./view";
 
+/**
+ * Every bound argument below is caller input on the wire, not trusted state —
+ * a server action's bound arguments round-trip through the client the same
+ * way a form field does — so each is parsed with zod before touching a
+ * service, mirroring `saveNoteAction`'s own `invalid_note` throw (the one
+ * FormData field this file reads directly). None of these are reachable with
+ * a bad value from the rendered page itself, so every rejection throws rather
+ * than earning notice copy — the same posture `access-lists/actions.ts`'s
+ * `parseId` and `sync/actions.ts`'s `jobTypeSchema` take on their own
+ * unreachable inputs.
+ *
+ * Every `assertValid` call below sits AFTER `requireAdminAction()`, not
+ * before — the same order `access-lists/actions.ts`'s `parseId` and
+ * `sync/actions.ts`'s `jobTypeSchema` already use. Authorization is checked
+ * before input shape: a caller who fails the guard gets the guard's own
+ * redirect regardless of what they sent, rather than a thrown validation
+ * error handing an unauthenticated request a way to distinguish a malformed
+ * argument from a well-formed one.
+ *
+ * Each schema spells its code on EVERY path it can reject through, not just
+ * the last one: `assertValid` throws the code it reads back off the issue, so
+ * a path left carrying zod's own generated wording ("Invalid input: expected
+ * number, received string") would surface that wording as the thrown message
+ * instead of this file's code. `z.number().int().positive({ error })` attaches
+ * the code to `positive` alone — hence the repetition on the two schemas that
+ * reject through more than one step.
+ */
+const accountIdSchema = z.uuid({ error: "invalid_account_id" });
+const identitySchema = z
+  .string({ error: "invalid_identity" })
+  .min(1, { error: "invalid_identity" });
+const listSearchSchema = z.string({ error: "invalid_list_search" });
+const tierSchema = z.enum(["member", "associate", "alumni"], { error: "invalid_tier" });
+const approveTierSchema = z.enum(["alumni", "associate"], { error: "invalid_tier" });
+const statusSchema = z.enum(["active", "cryo"], { error: "invalid_status" });
+const characterIdSchema = z
+  .number({ error: "invalid_character_id" })
+  .int({ error: "invalid_character_id" })
+  .positive({ error: "invalid_character_id" });
+const noteSchema = z.string({ error: "invalid_note" });
+
+/** Parses `value` against `schema`, throwing `Error(code)` — not a `ZodError`
+ *  — on rejection, matching every other unreachable-input throw in this file's
+ *  siblings. The code comes from the rejected issue's own message, which is
+ *  what each schema's `error:` option puts there, so the code has exactly one
+ *  spelling per schema rather than one on the schema and another repeated at
+ *  each call site. (`error:` is zod v4's spelling of v3's `message:`, which is
+ *  what the older schemas in `src/config.ts` and `src/lib/wanderer/client.ts`
+ *  still use; new schemas here take the v4 form.) */
+function assertValid<T>(schema: z.ZodType<T>, value: unknown): T {
+  const result = schema.safeParse(value);
+  if (!result.success)
+    throw new Error(result.error.issues[0]?.message ?? "invalid_input");
+  return result.data;
+}
+
 /**
  * `/admin/accounts?[<listSearch>&]done=<code>&name=<name>&at=<instant>`.
  *
@@ -171,6 +228,10 @@ export async function setTierAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  tier = assertValid(tierSchema, tier);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) =>
     setTierManual(tx, actor, accountId, tier),
   );
@@ -203,6 +264,10 @@ export async function approveAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  tier = assertValid(approveTierSchema, tier);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) =>
     approveAccount(tx, actor, accountId, tier),
   );
@@ -221,6 +286,9 @@ export async function returnToAutoAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) =>
     returnTierToAuto(tx, actor, accountId),
   );
@@ -246,6 +314,10 @@ export async function setStatusAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  status = assertValid(statusSchema, status);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) =>
     setAccountStatus(tx, actor, accountId, status),
   );
@@ -284,14 +356,15 @@ export async function saveNoteAction(
   formData: FormData,
 ): Promise<NoteSaveState> {
   const { accountId: actor } = await requireAdminAction();
-  const raw = formData.get("note");
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
   // FormData.get() is string | File | null. Coercing a File or a missing field
   // to "" would silently CLEAR the note (setStatusNote maps "" to null) and
   // write a status.note_changed audit entry for an edit nobody requested.
   // Reject the malformed request instead; "" itself stays valid — that is how
   // the form asks for the note to be cleared. This can only happen if the
   // form itself is tampered with, so it stays a throw rather than a race.
-  if (typeof raw !== "string") throw new Error("invalid_note");
+  const raw = assertValid(noteSchema, formData.get("note"));
 
   const result = await getDb().transaction((tx) =>
     setStatusNote(tx, actor, accountId, raw),
@@ -313,6 +386,9 @@ export async function syncAccountAction(
   identity: string,
 ): Promise<void> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   await getDb().transaction(async (tx) => {
     await logAudit(tx, { actor, action: "sync.requested", target: accountId });
     await enqueueSync(tx, { kind: "account", accountId });
@@ -327,6 +403,9 @@ export async function promoteAdminAction(
   identity: string,
 ): Promise<void> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) => promoteAdmin(tx, actor, accountId));
   if (!result.ok) {
     // promoteAdmin predates AdminMutationResult and returns `error` as
@@ -349,6 +428,9 @@ export async function demoteAdminAction(
   identity: string,
 ): Promise<void> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) => demoteAdmin(tx, actor, accountId));
   if (!result.ok && result.error === "last_admin") {
     // Surface the service's protection instead of a 500 (carry-over).
@@ -389,6 +471,9 @@ export async function unlinkDiscordAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  listSearch = assertValid(listSearchSchema, listSearch);
+  identity = assertValid(identitySchema, identity);
   const result = await getDb().transaction((tx) =>
     unlinkDiscord(tx, actor, accountId, "admin"),
   );
@@ -433,10 +518,12 @@ export async function unlinkDiscordAction(
 /**
  * Promote a character to main from the drawer's crew table.
  *
- * `characterId` is bound at render time from the row's own crew list, so it is
- * not user input in the way a form field would be — but the service still
- * verifies the character belongs to the account, which is what makes a forged
- * request land on `not_found` rather than on someone else's account.
+ * `characterId` is bound at render time from the row's own crew list, but a
+ * bound argument round-trips through the client exactly as a form field does,
+ * so it is parsed here like every other argument (see this file's docblock).
+ * The service independently verifies the character belongs to the account,
+ * which is what makes a forged request that clears the parse land on
+ * `not_found` rather than on someone else's account.
  *
  * Returns an `ActionOutcome` rather than redirecting: the control lives inside
  * the drawer, and a redirect would collapse it.
@@ -449,6 +536,9 @@ export async function setMainAction(
   _formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
+  accountId = assertValid(accountIdSchema, accountId);
+  characterId = assertValid(characterIdSchema, characterId);
+  listSearch = assertValid(listSearchSchema, listSearch);
   const result = await getDb().transaction((tx) =>
     setMainCharacterAsAdmin(tx, actor, accountId, characterId),
   );

src/app/admin/sync/actions.ts

@@ -1,9 +1,10 @@
 "use server";
 
+import { z } from "zod";
 import { revalidatePath } from "next/cache";
 import { redirect } from "next/navigation";
 import { getDb } from "@/db";
-import { isJobType } from "@/core/schedules";
+import { isJobType, type JobType } from "@/core/schedules";
 import { HEARTBEAT_STALE_AFTER_MS, evaluateFreshness } from "@/core/health";
 import { requireAdminAction } from "@/lib/admin-guard";
 import { logAudit } from "@/services/audit";
@@ -13,6 +14,13 @@ import { elapsedShort } from "@/app/_components/format-ago";
 import { type ActionOutcome } from "@/app/_components/confirm-group";
 import { queuedNotice } from "./view";
 
+/** Defers to `isJobType` (`@/core/schedules`) — the one place a job's schedule
+ *  is written down — rather than restating that job-type list here as literals
+ *  that could drift from it. `z.custom` lifts an existing type guard into a
+ *  schema; it derives no literals of its own, so there is nothing here to keep
+ *  in step. */
+const jobTypeSchema = z.custom<JobType>(isJobType, { error: "invalid_job_type" });
+
 export async function syncAllAction(): Promise<void> {
   const { accountId: actor } = await requireAdminAction();
   await getDb().transaction(async (tx) => {
@@ -69,16 +77,18 @@ export async function syncJobAction(
   formData: FormData,
 ): Promise<ActionOutcome> {
   const { accountId: actor } = await requireAdminAction();
-  const jobType = formData.get("jobType");
+  const rawJobType = formData.get("jobType");
   // A server action takes whatever the wire sends, and `jobType` becomes a
   // queue name downstream. Only the schedules table's own keys are accepted,
   // so a tampered form cannot enqueue against an arbitrary queue. The dispatch
   // side checks again; this one keeps the bad row out of the outbox and the
   // audit log entirely. Unreachable from the rendered page, so it throws
   // rather than earning notice copy.
-  if (!isJobType(jobType)) {
+  const parsed = jobTypeSchema.safeParse(rawJobType);
+  if (!parsed.success) {
     throw new Error("invalid_job_type");
   }
+  const jobType = parsed.data;
 
   const db = getDb();
   await db.transaction(async (tx) => {