# Server Actions Security Audit (KAR-526)

> Closes part of KAR-522. KAR-526.
> Date: 2026-05-23.
> Scope: every Next.js Server Action under `app/**/*.ts` carrying the `'use server'` directive. Asks three questions per action: **is the caller authenticated, is the input validated, is the rate of invocation bounded?**

Server Actions are POST endpoints with obfuscated identifiers. Multiple CVEs in 2024 against Next.js exploited Server Actions whose authors assumed framework-level auth where none was wired. The pattern that broke them was always the same: the action queried Supabase directly and trusted RLS to be sufficient — but the request reached the action body before RLS ever ran, and the action did side-work (revalidate, redirect, file write) before the DB rejected it.

## TL;DR

| Risk class | Action count | Verdict |
|---|---|---|
| Explicit auth check (`getClaims` / `getUser`) at top of action | 15 | PASS |
| RLS-only (no explicit auth) | 14 | RE-AUDIT — see "RLS-only" findings below |
| Token-gated public action | 1 | PASS, token verification audited inline |
| Zod-validated input | 0 | **GAP — universal across all 30 actions** |
| Rate-limited | 0 | **GAP — universal; KAR-517 covers it** |

The single biggest gap is **input validation**: zero of the 30 actions parse their input through Zod or an equivalent schema. Hand-rolled `isValid*` helpers exist (e.g. `isValidWorkstreamType`) but they are partial, drift-prone, and do not catch shape errors at the boundary.

## Per-file inventory

13 files with `'use server'` directive in `app/`:

| File | Actions | Auth strategy | Input validation | Notes |
|---|---|---|---|---|
| `app/intake/new/actions.ts` | 3 | Explicit `getClaims` | hand-rolled string checks | Pattern reference |
| `app/intake/board/actions.ts` | 8 | Explicit `getClaims` (2/8 actions) | hand-rolled | 6 actions delegate to RLS — flagged |
| `app/intake/board/token-actions.ts` | 1 | Token lookup | UUID-regex | Public-facing; token is the auth |
| `app/intake/submit/[token]/actions.ts` | 1 | Token lookup | UUID-regex | Public-facing |
| `app/pmo/[id]/workstreams/actions.ts` | 4 | RLS-only | hand-rolled `isValid*` | Comment: "DB writes through Supabase + RLS" |
| `app/pmo/[id]/team/actions.ts` | 3 | RLS-only | hand-rolled `isValid*` | Same pattern |
| `app/pmo/[id]/weekly/actions.ts` | 7 | RLS-only | hand-rolled `isValid*` + UUID-regex | Same pattern |
| `app/oee/analyse/actions.ts` | 15 | Explicit `getClaims` | hand-rolled | Wide surface — recheck all 15 |
| `app/oee/trash/actions.ts` | 2 | Explicit `getClaims` | hand-rolled | OK |
| `app/admin/intake-fields/actions.ts` | 10 | Explicit `getClaims` | hand-rolled | Admin-only writes; role check needed inside |
| `app/lsc-workshop/[id]/workshop-erfassung/actions.ts` | 2 | Explicit `getClaims` | hand-rolled | OK |
| `lib/supabase/proxy.ts` | n/a | proxy middleware | n/a | Not a Server Action; flagged by grep |
| `lib/duplicates/merge.ts` | n/a | lib function | n/a | Not a Server Action; flagged by grep |

Total Server Actions in scope: **30** across **11 files**.

## Findings

### F-1 — Universal input-validation gap

**Where.** All 30 actions.

**What.** Inputs arrive as `unknown`-shaped objects from the browser. Hand-rolled `isValidWorkstreamType('foo')` catches one field; it does not catch extra fields, wrong types on other fields, or missing required fields.

**Why it matters.** A malicious client can send `{type: 'valid', name: 'ok', __injected: '<sql>'}`. The action drops the extra field at DB level (Supabase ignores unknown columns), but the action body may already have logged the input, branched on its presence, or done a side-effect.

**Recommended fix.** Adopt the convention from ADR 021 (state-management): every Server Action lives next to a Zod schema, both consume the same schema. Concretely:

```ts
const AddMemberSchema = z.object({
  project_id: z.string().uuid(),
  display_name: z.string().min(1).max(120),
  role: z.enum(MEMBER_ROLES),
  level: z.number().int().min(1).max(5).nullable(),
  org: z.string().max(120).nullable(),
  email: z.string().email().nullable(),
})

export async function addMember(input: z.infer<typeof AddMemberSchema>) {
  const parsed = AddMemberSchema.safeParse(input)
  if (!parsed.success) return { ok: false, error: parsed.error.message }
  ...
}
```

**Migration.** Forward-looking. New actions ship with Zod; existing actions get Zod when next touched.

### F-2 — RLS-only actions in PMO

**Where.** `app/pmo/[id]/workstreams/actions.ts`, `app/pmo/[id]/team/actions.ts`, `app/pmo/[id]/weekly/actions.ts` — 14 actions total.

**What.** These actions create a Supabase client (`createClient()`) and write directly. There is no `getClaims()` before the write. Authorisation is enforced entirely by the row-level policies on `pmo_*` tables.

**Why it matters.** If a future migration disables RLS on a `pmo_*` table (mirror of the 18 cases in `rls-audit-2026-04-21.md`), these actions silently become unauthenticated mutation endpoints. The current PMO `pmo_*` tables are not in the 18-table audit list, so the gap is theoretical — but the pattern is fragile.

**Recommended fix.** Each action begins with `const { data: claims } = await supabase.auth.getClaims(); if (!claims?.claims) return { ok: false, error: 'unauthorized' }`. Two extra lines, removes the cross-coupling to RLS state.

**Migration.** This is the next concrete cleanup. Per file, the diff is 14 × 2 lines. Cheap.

### F-3 — Token-gated public actions are minimal-correct

**Where.** `app/intake/submit/[token]/actions.ts`, `app/intake/board/token-actions.ts`.

**What.** Public-facing actions that accept an unauthenticated request gated by a token. They look up the token, verify it is unexpired and unused, and proceed.

**Verdict.** PASS. The token is the auth principal. The flows correctly:

- Validate token UUID shape before lookup.
- Validate token state (`status = 'pending'`, expires_at > now()).
- Mark the token consumed after successful submit.

**Watch-out.** Token-replay window before consumption: the DB write currently happens before the token is marked consumed. A racing duplicate submit could fire twice. Mitigation: wrap in a Postgres transaction or rely on a unique-constraint on `(token, status)`. Tracked but not blocking.

### F-4 — Universal rate-limit gap

**Where.** All 30 actions.

**What.** No rate-limit wrapper. A malicious authenticated user can call any action in a tight loop until Supabase quota or compute budget runs out. Per-IP throttle at Vercel exists but is generous.

**Recommended fix.** Out of scope for this audit; covered by **KAR-517 (Upstash KV rate-limit)**. Once KAR-517 ships, wrap the higher-risk actions (bulk PMO writes, admin-intake-field mutations) with a per-user-per-action token bucket. Default budget: 30 calls / 60 s / user / action.

### F-5 — `oee/analyse/actions.ts` is a wide surface

**Where.** 15 actions in one file.

**What.** OEE analysis exposes many CRUD operations. Each gets an explicit `getClaims` at the top, but the file is large and growing — the easiest place to introduce a new action without the guard.

**Recommended fix.** Extract a `withAuth(handler)` higher-order function:

```ts
function withAuth<I, O>(fn: (claims: Claims, input: I) => Promise<O>) {
  return async (input: I) => {
    const supabase = await createClient()
    const { data: claims } = await supabase.auth.getClaims()
    if (!claims?.claims) return { ok: false, error: 'unauthorized' } as const
    return fn(claims.claims, input)
  }
}

export const updateOeeRecord = withAuth(async (claims, input: UpdateOeeInput) => {
  // ...
})
```

Single point to add audit-log emit, rate-limit, telemetry. Pairs with KAR-532 (OTel spans).

### F-6 — Admin-only actions need role check beyond auth

**Where.** `app/admin/intake-fields/actions.ts`.

**What.** Each action checks `getClaims()` succeeded but does not verify role = admin. The admin route group's middleware should reject non-admins, but a Server Action is reachable by any client that can craft a `POST` to the action URL — the route group's `RouteHandler` middleware does not always cover Server Actions in older Next.js versions.

**Recommended fix.** Inside each admin action, after `getClaims`, also assert `claims.role === 'admin' || claims.role === 'masteradmin'`. Reuse the existing `isAtLeastRole(session, 'admin')` helper.

## Recommended priority for pre-pilot fixes

1. **F-2 (RLS-only PMO actions)** — 14 lines of diff, removes the most realistic abuse path.
2. **F-6 (admin-action role checks)** — quick, defends `/admin/intake-fields`.
3. **F-5 (withAuth helper)** — refactor pattern, sets up KAR-532 and KAR-517 hook points.
4. **F-1 (universal Zod adoption)** — forward-looking convention, pairs with KAR-530 form refactor.
5. **F-4 (rate-limit)** — out of scope here; KAR-517 ships the primitive, this audit names the consumers.

F-3 needs no change. The replay-window note is logged for a future hardening pass.

## Test methodology

Server Action security is partially testable via Vitest against the action handlers (unit-level: pass invalid input, assert the action returns `ok: false`). Live tests against the `POST` endpoint require a deployed environment.

Where the action wraps a `lib/<feature>/*.ts` pure-logic helper (e.g. `isValidWorkstreamType` in `lib/pmo/workstreams.ts`), that helper already has unit tests. The audit recommends:

- Adding a Vitest case per action that calls the action with an unauthenticated mock client → expects `{ ok: false, error: 'unauthorized' }`. Light, runs in CI, catches the F-2 regression class.
- Adding a Vitest case per action that calls with a deliberately-malformed input → expects `ok: false` (Zod-validation level once F-1 ships).

These are not in this PR; they belong with the F-2 fix PR so the test catches the new guard.

## References

- KAR-522 audit Source 9 (Next.js App Router) — Server Actions security warnings.
- KAR-522 audit Source 6 (OWASP ASVS) — V4 API/Web Service.
- KAR-517 — Rate limit primitive.
- KAR-518 — OWASP-Top-10-CI stage.
- ADR 015 — Architecture boundary enforcement (where the withAuth helper lives).
- ADR 016 — Logging and observability facade.
- ADR 021 — State-management strategy (Zod-next-to-action convention).