From 3f22ef76007256bce35784a7d0b09db1377805b6 Mon Sep 17 00:00:00 2001 From: Keysat Date: Sat, 13 Jun 2026 00:03:47 -0500 Subject: [PATCH] =?UTF-8?q?v1.1.0:9=20=E2=80=94=20P2=20hardening:=20input-?= =?UTF-8?q?validation=20400s,=20auth=20rate-limit,=20XFF=20anti-spoof,=20n?= =?UTF-8?q?on-root=20container?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2 batch from the 2026-06-13 full-eval (EVALUATION.md / ROADMAP.md), reviewed by the reviewer agent. App-code + packaging only; no schema or data change, existing /data untouched. Input validation: malformed JSON bodies, invalid date, and out-of-range or non-numeric pagination on /api/workouts now return 400 instead of 500. New lib/http.ts readJsonBody maps a bad body to a ZodError across the 11 CRUD routes whose catch maps ZodError to 400; me/import and admin/signups guard request.json() in an explicit try/catch. Rate limiting: POST /api/auth now shares the UI login server action's per-IP 10-per-15min cap and returns 429 + Retry-After. clientIpFromHeaders reads the rightmost (trusted-proxy-appended) X-Forwarded-For entry instead of the spoofable leftmost. Container: drops root. The entrypoint prepares /data as root, chowns it to nextjs, then exec su-exec nextjs:nodejs node server.js (su-exec added to the runner image). The container drop needs live sideload verification. --- AGENTS.md | 15 +- ROADMAP.md | 9 +- proof-of-work/app/api/auth/route.ts | 17 ++- proof-of-work/app/api/exercises/[id]/route.ts | 3 +- proof-of-work/app/api/exercises/route.ts | 3 +- .../app/api/import/exercises/seed/route.ts | 3 +- proof-of-work/app/api/me/import/route.ts | 10 +- proof-of-work/app/api/preferences/route.ts | 3 +- proof-of-work/app/api/programs/[id]/route.ts | 3 +- proof-of-work/app/api/programs/route.ts | 3 +- proof-of-work/app/api/workouts/[id]/route.ts | 3 +- .../app/api/workouts/[id]/sets/route.ts | 3 +- .../app/api/workouts/import/save/route.ts | 3 +- proof-of-work/app/api/workouts/route.ts | 29 +++- proof-of-work/lib/http.ts | 26 ++++ proof-of-work/lib/rateLimit.ts | 34 ++++- proof-of-work/tests/rateLimit.test.ts | 16 +- proof-of-work/tests/routes-me-import.test.ts | 15 ++ proof-of-work/tests/routes-validation.test.ts | 141 ++++++++++++++++++ start9/0.4/Dockerfile | 4 +- start9/0.4/docker_entrypoint.sh | 17 ++- start9/0.4/startos/versions/index.ts | 7 +- start9/0.4/startos/versions/v1.1.0.9.ts | 39 +++++ 23 files changed, 365 insertions(+), 41 deletions(-) create mode 100644 proof-of-work/lib/http.ts create mode 100644 proof-of-work/tests/routes-validation.test.ts create mode 100644 start9/0.4/startos/versions/v1.1.0.9.ts diff --git a/AGENTS.md b/AGENTS.md index 9c89d02..4ddae93 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,6 +72,8 @@ Canonical publish path for this project: `~/.proof-of-work/publish.sh` (builds, - **Commit subject** = `vX.Y.Z:N — short summary`, imperative, body explains the *why*. - **Git remote is self-hosted** (a private Start9 registry + a FileBrowser artifact host), NOT GitHub. The actual registry/file-host URLs are constants in `~/.proof-of-work/{publish,unpublish}.sh`; FileBrowser creds live in `~/.keysat/filebrowser.env` (outside the repo, gitignored). Default branch is `master`. - **Authorization tiers**: whole-instance routes (`settings/{export,import}-db`) are **admin-only** (`!user.isAdmin → 403`); per-user data routes scope by `user.id`. Custom-URL AI providers (Ollama, OpenAI-compatible — anything with `requiresBaseUrl`) are **admin-only** (SSRF surface); fixed-URL cloud providers (claude/openai/gemini) stay per-user. Gate the server route AND hide the control in the UI. +- **Malformed JSON body must return 400, not 500.** Routes whose catch maps `instanceof z.ZodError → 400` parse via `readJsonBody(request)` (`lib/http.ts` — throws a `ZodError` on bad JSON, so the existing branch handles it with no catch change). `safeParse`-style routes (`me/import`, `admin/signups`) wrap `request.json()` in an explicit `try/catch → 400`. (AI/admin routes using `.catch(() => ({}))` are a third, pre-existing pattern — unify if you touch them.) +- **The container runs the Node server as non-root.** `docker_entrypoint.sh` runs as root only to prep `/data` (seed, ALTERs, library reconcile), then `chown -R nextjs:nodejs "$DATA_DIR"` and `exec su-exec nextjs:nodejs node /app/server.js` (su-exec added in the Dockerfile runner stage). Any new entrypoint step that needs root must run *before* that final line. - Tests live in `proof-of-work/tests/`; mock server-action deps with `vi.hoisted()` + `vi.mock`. - **Before editing the AI subsystem (`proof-of-work/lib/ai/**` or the generate/generations routes), read `docs/guides/ai-subsystem.md`** — provider abstraction, SSE/lenient-JSON, pricing/model menus, and the background-runner architecture live there. @@ -96,18 +98,19 @@ Canonical publish path for this project: `~/.proof-of-work/publish.sh` (builds, ## Current state -Latest version is **1.1.0:8** — **built and sideloaded** to the StartOS server (2026-06-13). Registry is empty and **publishing is parked** (sideload-only via `make install`). +Latest version is **1.1.0:9** (P2 hardening batch, on `master`). **1.1.0:8** was the last build+sideload confirmed booting; **:9 is being built + sideloaded this session** — the container privilege-drop is only *verified* once that boot succeeds and the app writes `/data` as uid 1001. Registry empty, **publishing parked** (sideload-only via `make install`). Working: workout logging, programs (manual + AI), multi-user, curated library, full AI subsystem (5 providers, multi-config, background generation, history detail, cost/duration, Ollama auto-detect, infinite-scroll exercise history). -Done this session (2026-06-13 full-eval security batch, on `master`): **P0** whole-instance DB export/import now admin-only (+UI +test); **P1** SSRF guard (`lib/ai/safeUrl.ts`, allows private-LAN by design) + custom-URL AI providers made admin-only + dead legacy `ai/config` route removed; **P1** dev quick-start fixed (`npm run create-admin`, README, `.env.example`); **P1** `export-db` 0-byte dev bug (`resolveDatabasePath` now matches Prisma). Full report in `EVALUATION.md`. Tests **197 pass**, build green, tsc clean. Secrets decision: no at-rest encryption (can't protect users from the operator — structural; threat model stands). +Done this session (P2 batch from `EVALUATION.md`, reviewed by the reviewer agent): malformed bodies / invalid `date` / out-of-range pagination now **400 not 500** (new `lib/http.ts readJsonBody` across 11 CRUD routes; explicit guard on `me/import` + `admin/signups`); **`POST /api/auth` rate-limited** (shares the UI `login:${ip}` 10/15min bucket; 429+Retry-After); rate-limiter **XFF anti-spoof** (rightmost entry); **container drops root** via su-exec. Tests **209 pass**, build + tsc + lint clean. -In progress: none. +In progress: **build + sideload of 1.1.0:9** (`make x86` → `make install` from `start9/0.4/`), then verify it boots + writes `/data` as non-root. Next steps (priority order): -1. **Next.js 14→15 major bump** (the remaining P1 — CVEs) as its own tested change. Then the P2/P3 hardening backlog → see `ROADMAP.md` → Security & hardening. -2. Tiered AI prompt formatting (`ROADMAP.md` → AI quality) once the security queue is clear. +1. **Next.js 14→15 major bump** (the remaining P1 — CVEs) as its own tested change — planned next; the login server action already uses async `cookies()/headers()`, easing the migration. +2. **P3 hardening batch** (`ROADMAP.md` → Security & hardening): login timing oracle, CSP `unsafe-eval`, `/api/health` info disclosure, rate-limit map leak, `exerciseId` ownership on workout PATCH/sets POST, 30-day sessions, text max-length. Also unify the 3rd JSON-parse pattern in `programs/[id]/days/[dayId]/start`. +3. Tiered AI prompt formatting (`ROADMAP.md` → AI quality). -Open/parked: `publish.sh` Step-3 registry-register silent no-op on 1.1.0:6/:7 (parked with publishing). Community-registry submission has 4 blockers (see `ROADMAP.md` → Packaging). +Open/parked: rate-limit per-IP correctness depends on the StartOS proxy forwarding real client IPs (unverified on the box). `publish.sh` Step-3 registry no-op (parked w/ publishing). Community-registry 4 blockers (`ROADMAP.md` → Packaging). Git remote: `origin` → self-hosted Gitea at `ssh://git@immense-voyage.local:59916/grant/proof-of-work.git`; `master` tracks it. (`~/.proof-of-work/{publish,unpublish}.sh` registry/FileBrowser hosts are separate from this code remote.) diff --git a/ROADMAP.md b/ROADMAP.md index 3045778..120e934 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -10,11 +10,10 @@ Longer-term backlog. Near-term state + next steps live in `AGENTS.md` → Curren ## Security & hardening (from 2026-06-13 full-eval; full detail + file:line in `EVALUATION.md`) - **Next.js 14→15 major bump** (CVEs: RSC DoS, WS-upgrade SSRF, App Router XSS). Own tested change — breaking App Router/caching semantics, needs its own build + sideload verification. -- Input-validation 500s → should be 400: invalid `date`, malformed JSON body, negative pagination `offset` on `/api/workouts` (+ `import/exercises/seed`). One shared `try{json}→400` + Zod guard fixes the set. -- `POST /api/auth` has no rate limiting (the UI server-action is capped; the raw API isn't) → brute-forceable. -- Rate limiter trusts the spoofable leftmost `X-Forwarded-For` (`lib/rateLimit.ts`) — verify whether the StartOS proxy overwrites XFF on the live box. -- Container runs as **root** — add `USER nextjs` to `start9/0.4/Dockerfile`. -- P3 hardening batch: login timing oracle (dummy bcrypt on unknown email), CSP `unsafe-eval` vs comment, `/api/health` info disclosure, rate-limit map leak, `exerciseId` ownership unchecked on workout PATCH/sets POST, 30-day sessions, no text max-length. +- **Still open — verify on the box:** whether the StartOS proxy forwards real client IPs to the app. The rate limiter now keys on the rightmost (trusted-proxy) `X-Forwarded-For` entry; if the proxy instead makes every client look like one IP, the per-IP cap collapses to a single global bucket. Confirm with live headers. +- P3 hardening batch: login timing oracle (dummy bcrypt on unknown email), CSP `unsafe-eval` vs comment, `/api/health` info disclosure, rate-limit map leak, `exerciseId` ownership unchecked on workout PATCH/sets POST, 30-day sessions, no text max-length. Also unify the 3rd JSON-parse pattern in `programs/[id]/days/[dayId]/start` (`try{json}catch{→{}}`). + +Done in 1.1.0:9 (P2 batch): input-validation 500s → 400 (`lib/http.ts readJsonBody` + explicit guards); `POST /api/auth` rate-limited; XFF anti-spoof; container drops root via su-exec. ## Packaging / distribution diff --git a/proof-of-work/app/api/auth/route.ts b/proof-of-work/app/api/auth/route.ts index 8200f24..c5b9c77 100644 --- a/proof-of-work/app/api/auth/route.ts +++ b/proof-of-work/app/api/auth/route.ts @@ -2,6 +2,8 @@ import { NextRequest, NextResponse } from 'next/server'; import { z } from 'zod'; import { verifyPassword, createSession } from '@/lib/auth'; import { prisma } from '@/lib/prisma'; +import { readJsonBody } from '@/lib/http'; +import { rateLimit, clientIpFromHeaders } from '@/lib/rateLimit'; const loginSchema = z.object({ email: z.string().email(), @@ -10,7 +12,20 @@ const loginSchema = z.object({ export async function POST(request: NextRequest) { try { - const body = await request.json(); + // Per-IP cap, sharing the `login:${ip}` bucket with the UI login + // server action (app/auth/login/actions.ts): 10 attempts / 15 min. + // Without this the raw API endpoint is an uncapped credential-stuffing + // surface that bypasses the server-action's limiter. + const ip = clientIpFromHeaders(request.headers); + const limited = rateLimit(`login:${ip}`, { limit: 10, windowMs: 15 * 60_000 }); + if (!limited.ok) { + return NextResponse.json( + { error: `Too many login attempts. Try again in ${limited.retryAfterSec}s.` }, + { status: 429, headers: { 'Retry-After': String(limited.retryAfterSec) } } + ); + } + + const body = await readJsonBody(request); const { email, password } = loginSchema.parse(body); // Look up user by email diff --git a/proof-of-work/app/api/exercises/[id]/route.ts b/proof-of-work/app/api/exercises/[id]/route.ts index e1df2e8..3805ca4 100644 --- a/proof-of-work/app/api/exercises/[id]/route.ts +++ b/proof-of-work/app/api/exercises/[id]/route.ts @@ -1,5 +1,6 @@ import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; @@ -118,7 +119,7 @@ export async function PATCH( return NextResponse.json({ error: "Exercise not found" }, { status: 404 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = updateExerciseSchema.parse(body); const data: any = {}; diff --git a/proof-of-work/app/api/exercises/route.ts b/proof-of-work/app/api/exercises/route.ts index bc6aeb4..16fc5dd 100644 --- a/proof-of-work/app/api/exercises/route.ts +++ b/proof-of-work/app/api/exercises/route.ts @@ -1,6 +1,7 @@ import { getCurrentUser } from "@/lib/auth"; import { getExercises, createExercise } from "@/lib/db/exercises"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; @@ -60,7 +61,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = CreateExerciseSchema.parse(body); const existing = await prisma.exercise.findUnique({ diff --git a/proof-of-work/app/api/import/exercises/seed/route.ts b/proof-of-work/app/api/import/exercises/seed/route.ts index 00cd0f9..6f89a91 100644 --- a/proof-of-work/app/api/import/exercises/seed/route.ts +++ b/proof-of-work/app/api/import/exercises/seed/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; const SeedExerciseSchema = z.object({ name: z.string().min(1), @@ -26,7 +27,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - const body = await request.json(); + const body = await readJsonBody(request); const parsed = SeedPayloadSchema.parse(body); const existingExercises = await prisma.exercise.findMany({ diff --git a/proof-of-work/app/api/me/import/route.ts b/proof-of-work/app/api/me/import/route.ts index bd5ff73..6bad297 100644 --- a/proof-of-work/app/api/me/import/route.ts +++ b/proof-of-work/app/api/me/import/route.ts @@ -92,7 +92,15 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); } - const body = await request.json(); + // This route uses safeParse (not an `instanceof z.ZodError` catch), so a + // malformed body would otherwise reach the generic catch as a 500. Guard + // it explicitly — matches the pattern in app/api/admin/signups/route.ts. + let body: any; + try { + body = await request.json(); + } catch { + return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 }); + } const parsed = requestBody.safeParse(body); if (!parsed.success) { return NextResponse.json( diff --git a/proof-of-work/app/api/preferences/route.ts b/proof-of-work/app/api/preferences/route.ts index 6ec25b3..f537d2f 100644 --- a/proof-of-work/app/api/preferences/route.ts +++ b/proof-of-work/app/api/preferences/route.ts @@ -1,5 +1,6 @@ import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; @@ -61,7 +62,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = PreferencesSchema.parse(body); let preferences = await prisma.userPreferences.findUnique({ diff --git a/proof-of-work/app/api/programs/[id]/route.ts b/proof-of-work/app/api/programs/[id]/route.ts index b33475b..a97f012 100644 --- a/proof-of-work/app/api/programs/[id]/route.ts +++ b/proof-of-work/app/api/programs/[id]/route.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { Prisma } from "@prisma/client"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { getProgramById } from "@/lib/db/programs"; /** @@ -80,7 +81,7 @@ export async function PATCH( return NextResponse.json({ error: "Program not found" }, { status: 404 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = patchSchema.parse(body); // If replacing the tree, verify exercise ownership. diff --git a/proof-of-work/app/api/programs/route.ts b/proof-of-work/app/api/programs/route.ts index 6b1783e..51eba1a 100644 --- a/proof-of-work/app/api/programs/route.ts +++ b/proof-of-work/app/api/programs/route.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { Prisma } from "@prisma/client"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { getPrograms } from "@/lib/db/programs"; /** @@ -61,7 +62,7 @@ export async function POST(request: NextRequest) { const user = await getCurrentUser(); if (!user) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); - const body = await request.json(); + const body = await readJsonBody(request); const validated = createProgramSchema.parse(body); // Verify any referenced exerciseIds belong to this user. diff --git a/proof-of-work/app/api/workouts/[id]/route.ts b/proof-of-work/app/api/workouts/[id]/route.ts index e466c54..4aa9db4 100644 --- a/proof-of-work/app/api/workouts/[id]/route.ts +++ b/proof-of-work/app/api/workouts/[id]/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { z } from "zod"; // GET: Get workout by ID @@ -95,7 +96,7 @@ export async function PATCH( return NextResponse.json({ error: "Unauthorized" }, { status: 403 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = updateWorkoutSchema.parse(body); const workoutData: Record = {}; diff --git a/proof-of-work/app/api/workouts/[id]/sets/route.ts b/proof-of-work/app/api/workouts/[id]/sets/route.ts index 0704a6a..2bd845c 100644 --- a/proof-of-work/app/api/workouts/[id]/sets/route.ts +++ b/proof-of-work/app/api/workouts/[id]/sets/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; import { z } from "zod"; const addSetsSchema = z.object({ @@ -46,7 +47,7 @@ export async function POST( return NextResponse.json({ error: "Unauthorized" }, { status: 403 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = addSetsSchema.parse(body); // Delete existing sets for this exercise in this workout (replace mode) diff --git a/proof-of-work/app/api/workouts/import/save/route.ts b/proof-of-work/app/api/workouts/import/save/route.ts index da90a2d..cb60e20 100644 --- a/proof-of-work/app/api/workouts/import/save/route.ts +++ b/proof-of-work/app/api/workouts/import/save/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; const setSchema = z.object({ reps: z.number().int().positive().optional(), @@ -40,7 +41,7 @@ export async function POST(request: Request) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = saveImportSchema.parse(body); // Load all user exercises for matching diff --git a/proof-of-work/app/api/workouts/route.ts b/proof-of-work/app/api/workouts/route.ts index 12a056d..bec9513 100644 --- a/proof-of-work/app/api/workouts/route.ts +++ b/proof-of-work/app/api/workouts/route.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { Prisma } from "@prisma/client"; import { getCurrentUser } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; +import { readJsonBody } from "@/lib/http"; // Schema now supports creating empty workouts (just date) or with sets const createWorkoutSchema = z.object({ @@ -11,7 +12,10 @@ const createWorkoutSchema = z.object({ durationMinutes: z.number().int().positive().optional(), difficulty: z.number().int().min(1).max(10).optional(), caloriesBurned: z.number().int().positive().optional(), - date: z.string().optional(), // ISO date string or date-only string + date: z + .string() + .refine((s) => !Number.isNaN(Date.parse(s)), { message: "Invalid date" }) + .optional(), // ISO date string or date-only string sets: z .array( z.object({ @@ -45,8 +49,25 @@ export async function GET(request: NextRequest) { const query = searchParams.get("q"); const dateFrom = searchParams.get("dateFrom"); const dateTo = searchParams.get("dateTo"); - const limit = Math.min(parseInt(searchParams.get("limit") || "50"), 100); - const offset = parseInt(searchParams.get("offset") || "0"); + // Validate pagination up front: a negative offset or non-numeric value + // would otherwise reach Prisma's `skip`/`take` and throw a generic 500. + const pagination = z + .object({ + limit: z.coerce.number().int().min(1).max(100).default(50), + offset: z.coerce.number().int().min(0).default(0), + }) + .safeParse({ + limit: searchParams.get("limit") || undefined, + offset: searchParams.get("offset") || undefined, + }); + + if (!pagination.success) { + return NextResponse.json( + { error: "Invalid pagination parameters", details: pagination.error.errors }, + { status: 400 } + ); + } + const { limit, offset } = pagination.data; const where: Prisma.WorkoutWhereInput = { userId: user.id, @@ -116,7 +137,7 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - const body = await request.json(); + const body = await readJsonBody(request); const validated = createWorkoutSchema.parse(body); const workoutDate = validated.date ? new Date(validated.date) : new Date(); diff --git a/proof-of-work/lib/http.ts b/proof-of-work/lib/http.ts new file mode 100644 index 0000000..19f2ac3 --- /dev/null +++ b/proof-of-work/lib/http.ts @@ -0,0 +1,26 @@ +import { z } from "zod"; + +/** + * Parse a JSON request body, turning a malformed or empty body into a + * `ZodError` rather than letting the raw `SyntaxError` from `request.json()` + * fall through a route's generic `catch` and become an HTTP 500. + * + * Why a `ZodError` specifically: every body-parsing route already maps + * `instanceof z.ZodError` to a 400. Throwing one here means a malformed body + * returns 400 across all of them with no per-route catch changes — the call + * site swaps `request.json()` for `readJsonBody(request)` and nothing else + * moves. (It is a genuine `z.ZodError`, so the `instanceof` checks hold.) + */ +export async function readJsonBody(request: Request): Promise { + try { + return await request.json(); + } catch { + throw new z.ZodError([ + { + code: z.ZodIssueCode.custom, + path: [], + message: "Request body must be valid JSON", + }, + ]); + } +} diff --git a/proof-of-work/lib/rateLimit.ts b/proof-of-work/lib/rateLimit.ts index 2667019..56a8fda 100644 --- a/proof-of-work/lib/rateLimit.ts +++ b/proof-of-work/lib/rateLimit.ts @@ -44,19 +44,37 @@ export function rateLimit( } /** - * Best-effort client IP extraction. In a StartOS deployment the Next.js - * server sits behind a single proxy hop, so the leftmost - * `x-forwarded-for` entry is the originating client. If headers are - * absent (direct access in dev), fall back to the literal "unknown" key - * so the limiter still applies as a global rate cap. + * Best-effort client IP extraction for rate-limit keys. + * + * `X-Forwarded-For` is a client-appendable, comma-separated list: each proxy + * APPENDS the address it observed. A direct client can therefore forge any + * number of leftmost entries — using `xff.split(',')[0]` (the leftmost) lets + * an attacker rotate a fake IP per request and defeat the limiter entirely. + * + * In a StartOS deployment the Next.js server sits behind exactly one trusted + * proxy hop, so the RIGHTMOST entry is the address that proxy actually saw — + * the only value the client cannot spoof. We key off that. (If the proxy + * overwrites rather than appends XFF, the list has a single entry and + * rightmost == leftmost, so this is also correct in that case.) If XFF is + * absent (direct access in dev), fall back to `x-real-ip`, then to the + * literal "unknown" key so the limiter still applies as a global cap. + * + * Assumes a single trusted hop; if the deployment ever grows additional + * trusted proxies, count that many entries in from the right instead. */ export function clientIpFromHeaders(headers: Headers): string { const xff = headers.get('x-forwarded-for'); if (xff) { - const first = xff.split(',')[0]?.trim(); - if (first) return first; + const parts = xff + .split(',') + .map((p) => p.trim()) + .filter(Boolean); + if (parts.length > 0) return parts[parts.length - 1]; } const real = headers.get('x-real-ip'); - if (real) return real; + if (real) { + const trimmed = real.trim(); + if (trimmed) return trimmed; + } return 'unknown'; } diff --git a/proof-of-work/tests/rateLimit.test.ts b/proof-of-work/tests/rateLimit.test.ts index d243c77..825fdf4 100644 --- a/proof-of-work/tests/rateLimit.test.ts +++ b/proof-of-work/tests/rateLimit.test.ts @@ -40,9 +40,21 @@ describe('rateLimit', () => { }); describe('clientIpFromHeaders', () => { - it('uses the leftmost x-forwarded-for entry', () => { + it('uses the rightmost (single trusted proxy hop) x-forwarded-for entry', () => { const h = new Headers({ 'x-forwarded-for': '1.2.3.4, 5.6.7.8' }); - expect(clientIpFromHeaders(h)).toBe('1.2.3.4'); + expect(clientIpFromHeaders(h)).toBe('5.6.7.8'); + }); + + it('ignores a client-spoofed leftmost entry', () => { + // Attacker sends `X-Forwarded-For: evil`; the trusted proxy appends the + // real client address, which must win. + const h = new Headers({ 'x-forwarded-for': 'evil.spoofed, 203.0.113.9' }); + expect(clientIpFromHeaders(h)).toBe('203.0.113.9'); + }); + + it('handles a single-entry x-forwarded-for', () => { + const h = new Headers({ 'x-forwarded-for': '203.0.113.9' }); + expect(clientIpFromHeaders(h)).toBe('203.0.113.9'); }); it('falls back to x-real-ip', () => { diff --git a/proof-of-work/tests/routes-me-import.test.ts b/proof-of-work/tests/routes-me-import.test.ts index d1cd506..be7fa24 100644 --- a/proof-of-work/tests/routes-me-import.test.ts +++ b/proof-of-work/tests/routes-me-import.test.ts @@ -21,6 +21,14 @@ function jsonReq(body: unknown): NextRequest { } as ConstructorParameters[1]); } +function rawReq(rawBody: string): NextRequest { + return new NextRequest('http://x/api/me/import', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBody, + } as ConstructorParameters[1]); +} + async function makeUser(opts: { email: string }) { return prisma.user.create({ data: { email: opts.email, passwordHash: 'fake', isAdmin: false }, @@ -95,6 +103,13 @@ describe('POST /api/me/import', () => { expect(res.status).toBe(400); }); + it('returns 400 (not 500) on a malformed JSON body', async () => { + const u = await makeUser({ email: 'a@x' }); + getCurrentUserMock.mockResolvedValue(u); + const res = await importPost(rawReq('{ not valid json')); + expect(res.status).toBe(400); + }); + it('merge mode imports exercises and workouts attributed to the actor', async () => { const u = await makeUser({ email: 'a@x' }); getCurrentUserMock.mockResolvedValue(u); diff --git a/proof-of-work/tests/routes-validation.test.ts b/proof-of-work/tests/routes-validation.test.ts new file mode 100644 index 0000000..d892e40 --- /dev/null +++ b/proof-of-work/tests/routes-validation.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +const { getCurrentUserMock } = vi.hoisted(() => ({ + getCurrentUserMock: vi.fn(), +})); +vi.mock('@/lib/auth', async (orig) => { + const actual = (await orig()) as Record; + return { ...actual, getCurrentUser: getCurrentUserMock }; +}); +vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })); + +import { NextRequest } from 'next/server'; +import { prisma } from '@/lib/prisma'; +import { POST as postWorkout, GET as getWorkouts } from '@/app/api/workouts/route'; +import { POST as postAuth } from '@/app/api/auth/route'; + +function jsonReq(url: string, body?: unknown, init?: RequestInit): NextRequest { + const opts: RequestInit = { + method: body !== undefined ? 'POST' : 'GET', + headers: { 'content-type': 'application/json' }, + ...init, + }; + if (body !== undefined) { + opts.body = JSON.stringify(body); + } + return new NextRequest(url, opts as ConstructorParameters[1]); +} + +/** A request carrying a raw (possibly malformed) string body. */ +function rawReq(url: string, rawBody: string, init?: RequestInit): NextRequest { + return new NextRequest(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: rawBody, + ...init, + } as ConstructorParameters[1]); +} + +async function makeUser(email: string) { + return prisma.user.create({ + data: { email, passwordHash: 'fake', isAdmin: false }, + }); +} + +beforeEach(async () => { + await prisma.session.deleteMany(); + await prisma.exercise.deleteMany(); + await prisma.workout.deleteMany(); + await prisma.user.deleteMany(); + await prisma.instanceSettings.deleteMany(); + getCurrentUserMock.mockReset(); +}); + +describe('malformed request bodies → 400 (not 500)', () => { + it('POST /api/workouts returns 400 on a body that is not valid JSON', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await postWorkout(rawReq('http://x/api/workouts', '{ not valid json')); + expect(res.status).toBe(400); + }); + + it('POST /api/workouts returns 400 on an empty body', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await postWorkout(rawReq('http://x/api/workouts', '')); + expect(res.status).toBe(400); + }); +}); + +describe('POST /api/workouts date validation', () => { + it('returns 400 on an unparseable date string', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await postWorkout( + jsonReq('http://x/api/workouts', { name: 'X', date: 'not-a-date' }), + ); + expect(res.status).toBe(400); + }); + + it('accepts a valid date-only string', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await postWorkout( + jsonReq('http://x/api/workouts', { name: 'X', date: '2026-06-01' }), + ); + expect(res.status).toBe(201); + }); +}); + +describe('GET /api/workouts pagination validation', () => { + it('returns 400 on a negative offset (was a Prisma 500)', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await getWorkouts(jsonReq('http://x/api/workouts?offset=-5')); + expect(res.status).toBe(400); + }); + + it('returns 400 on a non-numeric limit', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await getWorkouts(jsonReq('http://x/api/workouts?limit=abc')); + expect(res.status).toBe(400); + }); + + it('returns 400 on limit=0 (new min-1 contract)', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await getWorkouts(jsonReq('http://x/api/workouts?limit=0')); + expect(res.status).toBe(400); + }); + + it('still serves valid pagination parameters', async () => { + const alice = await makeUser('a@x'); + getCurrentUserMock.mockResolvedValue(alice); + const res = await getWorkouts(jsonReq('http://x/api/workouts?limit=10&offset=0')); + expect(res.status).toBe(200); + }); +}); + +describe('POST /api/auth rate limiting', () => { + it('returns 429 + Retry-After after 10 attempts from one IP', async () => { + // Unique key per run so the process-global limiter bucket is clean. + const ip = `validation-test-${Math.random()}`; + const attempt = () => + postAuth( + new NextRequest('http://x/api/auth', { + method: 'POST', + headers: { 'content-type': 'application/json', 'x-forwarded-for': ip }, + body: JSON.stringify({ email: 'nobody@example.com', password: 'x' }), + } as ConstructorParameters[1]), + ); + + for (let i = 0; i < 10; i++) { + const res = await attempt(); + expect(res.status).toBe(401); // no such user + } + const blocked = await attempt(); + expect(blocked.status).toBe(429); + expect(blocked.headers.get('retry-after')).toBeTruthy(); + }); +}); diff --git a/start9/0.4/Dockerfile b/start9/0.4/Dockerfile index 3cfc81f..b40b1f9 100644 --- a/start9/0.4/Dockerfile +++ b/start9/0.4/Dockerfile @@ -54,7 +54,9 @@ FROM node:20-alpine AS runner WORKDIR /app -RUN apk add --no-cache dumb-init curl openssl sqlite \ +# su-exec: drop from root to the unprivileged `nextjs` user at the end of +# the entrypoint, after the root-only /data preparation is done. +RUN apk add --no-cache dumb-init curl openssl sqlite su-exec \ && addgroup -S nodejs -g 1001 \ && adduser -S nextjs -u 1001 -G nodejs diff --git a/start9/0.4/docker_entrypoint.sh b/start9/0.4/docker_entrypoint.sh index d6975dc..70a04fa 100755 --- a/start9/0.4/docker_entrypoint.sh +++ b/start9/0.4/docker_entrypoint.sh @@ -330,12 +330,23 @@ if [ -f "$TEMPLATES_JSON_PATH" ] && [ -f "$TEMPLATES_SCRIPT" ] && [ -f "$DB_PATH fi # ----------------------------------------------------------------------------- -# Step 4 — launch the app. +# Step 4 — launch the app as the unprivileged `nextjs` user. +# +# Everything above runs as root because the entrypoint has to prepare /data +# — a StartOS-mounted volume whose runtime ownership we don't control at +# build time — by creating the DB, running the ALTERs and reconciling the +# library. Now that the data layer is ready we hand /data to `nextjs` and +# drop privileges via su-exec, so the long-lived, remote-facing Node server +# never runs as root (shrinks the blast radius of any RCE in the app). # ----------------------------------------------------------------------------- export DATABASE_URL="file:$DB_PATH" export NODE_ENV="${NODE_ENV:-production}" export HOSTNAME="${HOSTNAME:-0.0.0.0}" export PORT="${PORT:-3000}" -log "launching Next.js on :${PORT} with DATABASE_URL=file:${DB_PATH}" -exec node /app/server.js +# Make every file the root-run setup just created in /data writable by the +# app user. Guarded so a chown hiccup logs rather than aborts boot. +chown -R nextjs:nodejs "$DATA_DIR" 2>/dev/null || log "WARN: could not chown $DATA_DIR; continuing" + +log "launching Next.js on :${PORT} as nextjs with DATABASE_URL=file:${DB_PATH}" +exec su-exec nextjs:nodejs node /app/server.js diff --git a/start9/0.4/startos/versions/index.ts b/start9/0.4/startos/versions/index.ts index 2271ce7..7a64155 100644 --- a/start9/0.4/startos/versions/index.ts +++ b/start9/0.4/startos/versions/index.ts @@ -14,6 +14,7 @@ import { v_1_1_0_5 } from './v1.1.0.5' import { v_1_1_0_6 } from './v1.1.0.6' import { v_1_1_0_7 } from './v1.1.0.7' import { v_1_1_0_8 } from './v1.1.0.8' +import { v_1_1_0_9 } from './v1.1.0.9' /** * Version graph for the `proof-of-work` package. @@ -52,9 +53,12 @@ import { v_1_1_0_8 } from './v1.1.0.8' * v1.1.0:8 — Multi-user authz hardening: whole-instance DB export/import * admin-only; custom-URL AI providers (Ollama / OpenAI-compatible) * admin-only + SSRF guard; dead legacy /api/ai/config removed. + * v1.1.0:9 — P2 hardening: malformed-body/invalid-date/bad-pagination -> + * 400 (not 500); POST /api/auth rate-limited; rate-limiter XFF + * anti-spoof (rightmost entry); container drops root via su-exec. */ export const versionGraph = VersionGraph.of({ - current: v_1_1_0_8, + current: v_1_1_0_9, other: [ v_1_0_0_1, v_1_0_0_2, @@ -70,5 +74,6 @@ export const versionGraph = VersionGraph.of({ v_1_1_0_5, v_1_1_0_6, v_1_1_0_7, + v_1_1_0_8, ], }) diff --git a/start9/0.4/startos/versions/v1.1.0.9.ts b/start9/0.4/startos/versions/v1.1.0.9.ts new file mode 100644 index 0000000..65958a0 --- /dev/null +++ b/start9/0.4/startos/versions/v1.1.0.9.ts @@ -0,0 +1,39 @@ +import { IMPOSSIBLE, VersionInfo } from '@start9labs/start-sdk' + +/** + * v1.1.0:9 — P2 hardening batch (2026-06-13, follows the :8 security batch). + * + * Input-validation, rate-limiting and container hardening from the full-eval + * P2 queue (see EVALUATION.md / ROADMAP.md): + * + * - Malformed request bodies now return 400 instead of 500. New + * lib/http.ts `readJsonBody` maps a bad JSON body to a ZodError across the + * 11 body-parsing CRUD routes (which already map ZodError -> 400); + * /api/me/import and /api/admin/signups guard it explicitly (safeParse + * style). Invalid `date` and out-of-range/non-numeric pagination on + * /api/workouts are likewise 400, not a Prisma 500. + * - POST /api/auth (the raw login API) is now rate-limited with the same + * per-IP 10/15min cap as the UI login server action, sharing the + * `login:${ip}` bucket — previously an uncapped credential-stuffing + * surface. Returns 429 + Retry-After. + * - The rate limiter's client-IP detection now reads the rightmost + * (trusted-proxy-appended) X-Forwarded-For entry instead of the spoofable + * leftmost one, so a forged XFF can't rotate the limiter key. + * - The container drops root: the entrypoint still prepares /data as root, + * then chowns it to `nextjs` and `exec su-exec`s the Node server as the + * unprivileged uid 1001 — shrinking the blast radius of any app RCE. + * + * App-code + packaging only — no schema, no API contract change for existing + * data, no data migration. Existing /data survives untouched. + */ +export const v_1_1_0_9 = VersionInfo.of({ + version: '1.1.0:9', + releaseNotes: { + en_US: + 'Hardening. Bad or malformed requests now return clean validation errors instead of server errors. The login API is rate-limited (matching the web form) and the limiter can no longer be fooled by spoofed forwarding headers. The app container now runs as an unprivileged user instead of root, reducing the impact of any future vulnerability. No schema or data changes — your existing data is untouched.', + }, + migrations: { + up: async () => {}, + down: IMPOSSIBLE, + }, +})