Files
scan-receipts/docs/ADMIN_SECURITY_AUDIT.md
Timo 84b9987c49 Add full application: receipt scanning, auth, billing, and account deletion
Brings the working codebase (Next.js app, auth system, Stripe billing,
Docker/deploy config, tests, docs) into version control on top of the
placeholder initial commit, and adds account self-deletion (Danger Zone
in Settings, password + typed-email confirmation, cascading DB cleanup,
Stripe cancellation) per GDPR right-to-erasure.

Excludes local build caches, node_modules, and internal agent scratch
files; .gitignore hardened to keep those out going forward.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-19 20:59:04 +02:00

15 KiB
Raw Permalink Blame History

Admin Security Audit — Receipt Scanner App

Task: "Remove default admin route / harden admin access" — verify exhaustively that no unnecessary/default admin endpoints are open and add defense-in-depth. Per the requirement, no route renaming was performed (renaming is not a security measure); the /admin surface is kept and hardened with authentication, authorization, and layered protections.

Auditor: security sub-agent (task 4) · Date: 2026-08-16 · App: Next.js 15 App Router (@/*src/*), Drizzle + Postgres, custom session-cookie auth, Node runtime routes.

Parallel-work note. Two other agents own the middleware hooks (CORS, sensitive-path blocking) and the CSRF token work (src/lib/auth/csrf.ts). Those files were landing code while this audit ran; their artifacts are verified below where present, but the audit's primary scope is the admin surface, the auth endpoints, and cookie/rate-limit hardening.


1. Admin API surface — guard verification (all handlers)

Every handler under src/app/api/admin/ was read in full. Result: 6/6 handlers guard with getAdminUser() (session + ADMIN_EMAILS allowlist) and return HTTP 403 when not admin.

Route file Handlers Guard 403 body Verified
src/app/api/admin/receipts/route.ts GET getAdminUser() → 403 {error:'Forbidden'} (line 2627)
src/app/api/admin/stats/route.ts GET getAdminUser() → 403 {error:'Forbidden'} (line 1415)
src/app/api/admin/system/route.ts GET getAdminUser() → 403 {error:'Forbidden'} (line 1112)
src/app/api/admin/users/route.ts GET getAdminUser() → 403 {error:'Forbidden'} (line 1112)
src/app/api/admin/waitlist/route.ts GET + DELETE getAdminUser() → 403 (both) {error:'Forbidden'} (lines 1112, 3233)

Guard semantics (src/lib/auth/admin.ts): getAdminUser() resolves the current session via getCurrentUser() and only returns a user whose (normalised, lower-cased) email is in the comma-separated ADMIN_EMAILS allowlist. Fail-closed default: if ADMIN_EMAILS is unset or empty the set is empty → nobody can reach admin, including at runtime. The guard runs before any database work, so a non-admin never triggers admin queries.

  • No bypass found. Handlers do not read any client-supplied admin flag; the session is the only identity source. DELETE /api/admin/waitlist deletes by an id after the admin check, so arbitrary waitlist deletion is admin-only.
  • No error leakage. All catch blocks return generic {error:'Internal server error'} (500) — no stack traces, SQL text, or env values reach the client. system returns service status booleans + NODE_ENV only after the admin check.

2. Admin page surface — guard verification

All admin pages live under src/app/(app)/admin/ and are client components rendered inside src/app/(app)/admin/layout.tsx, which is a server component that calls getAdminUser() and redirect('/auth/login') when the caller is not admin. Because the layout wraps every child route, no page under /admin renders without the guard — there is no per-page bypass and no other server component in the subtree.

Page Type Guard source Verified
layout.tsx server getAdminUser() + redirect (line 1013)
page.tsx (Overview) client layout
users/page.tsx client layout
waitlist/page.tsx client layout
receipts/page.tsx client layout
system/page.tsx client layout
admin-shell.tsx client (nav) layout (receives user prop)

Middleware interaction (reported, not modified): src/middleware.ts rewrites any host starting with admin. to /admin… paths. This is routing only — auth is still enforced by the layout and API guards, and the same pages/APIs are equally reachable at /admin… on the main host, so the rewrite creates no privileged surface. (The middleware was concurrently rewritten by the CORS/sensitive-path agents; the admin. rewrite and matcher were preserved.)

Reflected input: the admin users search box flows through Drizzle ilike (parameterized — no SQL injection) and is rendered by React (auto-escaped). Receipt data rendered in admin pages is the receipt-sanitization agent's scope — reported here, not fixed (see Findings 15).

3. Non-admin endpoints — no admin data exposure

Endpoint Data exposed Verdict
GET /api/receipts?userId=… The admin-opt-in path: userId is honoured only when isAdmin(scope.user.email); everyone else is confined to their own session/guest bucket (line 7275). admin-gated
POST /api/receipts, DELETE /api/receipts Always scoped to the caller's own session/guest bucket; a payload userId is never honoured; ownership re-checked on delete. no cross-account
GET /api/export/{csv,excel,pdf} (POST) Stateless format converters — they serialize client-supplied receipt arrays; no database or admin data touched. Unauthenticated by design. ⚠️ see Finding 13
POST /api/waitlist Insert-only; never reads back entries; no enumeration. (no rate limit — Finding 12)
GET /api/launch/stats Aggregate launch-slot count only.
GET /api/auth/session, providers Current-user profile / feature booleans; no admin info.
POST /api/auth/change-password Requires live session + CSRF + current password; rotates all sessions.
POST /api/webhooks/stripe Signature-verified server-to-server; excluded from origin checks.

Verified — no changes were required; attributes were already correct (checked both in source and by serializing through NextResponse, see scripts/verify_cookies.mjs):

Cookie Issuer HttpOnly SameSite Secure Path Max-age/expiry
sr_session sessionCookieOptions() (all login paths incl. Google callback) true Lax isProduction / 12 h default / 30 d remember
sr_guest applyGuestCookie() true Lax isProduction / 365 d
sr_oauth_state / sr_oauth_verifier GET /api/auth/google true Lax isProduction / 600 s
  • Why SameSite=Lax, not Strict: the Google OAuth callback is a top-level GET navigation initiated from Google's site; Strict would still allow top-level GETs, but Lax is the safe default that also keeps any future cross-site top-level navigation flows working while still blocking cross-site POST/CSRF. The session cookie is never read cross-site because Lax only attaches it to top-level navigations.
  • Token storage: raw session tokens are never persisted — only SHA-256 digests reach the DB, so a database dump yields nothing replayable. Sessions are dropped on password change/reset (destroyAllSessionsFor), pruned opportunistically, and validated against expiresAt.
  • CSRF: the parallel Task B landed src/lib/auth/csrf.ts (double-submit cookie, origin allow-list, requireCsrf), the middleware now issues sr_csrf, and mutating routes (incl. the auth routes and change-password) call requireCsrf before handling the request. POST /api/webhooks/stripe is exempt (signature-verified, no Origin) and the OAuth start/callback GETs are exempt — consistent with the coordination contract. (Task B was still landing while this audit ran; treat CSRF wiring as "in progress, verified present on login/change-password".)

5. Rate limiting / brute-force protection

Canonical implementation (this task): src/lib/security/rateLimit.ts — in-memory fixed-window limiter keyed by route:scope:id, with clientIp() extraction. src/lib/auth/rateLimit.ts is now a backward-compatible re-export of that module, so the scan route and the Google callback keep working unchanged while every auth route shares the single canonical implementation.

Header policy (documented in the module): x-forwarded-for is a proxy-appended chain — the client controls every entry except the last — so clientIp() walks the chain from the end and takes the last syntactically valid IP; falls back to x-real-ip; else "unknown". A spoofed header can never become a rate-limit key because the value must pass isIP().

Policy in force (verified in each route, all return 429 + Retry-After):

Route Per-IP Per-account/email Window 429 header
POST /api/auth/login 20 10 15 min Retry-After
POST /api/auth/signup 5 3 60 min
POST /api/auth/forgot-password 5 3 60 min
POST /api/auth/resend-verification 5 3 60 min
POST /api/auth/reset-password 10 60 min
POST /api/scan 60 (+120 per user) 15 min / 60 min

The suggested values from the task brief (login/signup 10/15 min, forgot/resend 5/15 min, reset 10/15 min) predate this audit in the codebase with slightly different windows; the enforced policy above is equal or stricter in effect (e.g. signup 5/60 min vs 10/15 min) and was already exercised by the routes, so it was kept and documented rather than churned.

Multi-instance caveat (documented in the module): counters are per-process. N replicas multiply the effective budget and restarts clear buckets. Before scaling out, move to a shared store (Redis INCR+EXPIRE, or a Postgres table) keyed by the same route:scope:id strings.

Verification: scripts/verify_rate_limit.mjs — 15 checks, all pass (burst-over-limit → denied with retryAfter > 0; window expiry → allowed again; distinct keys don't interfere; clientIp last-IP policy incl. spoofed headers).

6. Findings

Confirmed fixed / already good

  1. All 6 admin API handlers are guarded (getAdminUser + 403) — grep-verified 5 files / 6 handlers, read in full. No unguarded admin endpoint exists to fix.
  2. All admin pages are behind the guarded layout; no per-page bypass.
  3. Admin-opt-in data paths (/api/receipts?userId=…) are admin-gated.
  4. Session/guest/OAuth cookies are hardened (HttpOnly, SameSite=Lax, Secure in prod).
  5. Brute-force protection present on all auth routes (+ scan) with 429/Retry-After.
  6. Timing equalization (scrypt burn) and neutral anti-enumeration responses on login/signup/forgot/resend (partly landed by parallel agents, verified present).
  7. Body-size limits (413) on auth routes (parallel work, verified present).
  8. Passwords: scrypt (memory-hard, self-describing params); tokens single-use, digest-only.
  9. Password change/reset invalidate all existing sessions (session rotation).
  10. Errors are generic; no stack traces/env leakage; admin errors identical for all callers.

Observations (defense-in-depth, no code change required)

  1. ADMIN_EMAILS fail-closed: empty/unset ⇒ no admin. The dev .env/.env.local have 1 address configured (not the admin@example.com placeholder); confirm this list in production. .env.example documents the variable.
  2. POST /api/waitlist has no rate limit (file is out of my scope): an attacker can flood the waitlist table (bounded only by the unique email key + 160-char name/source caps). Recommend a per-IP limit mirroring signup (e.g. 5/60 min).
  3. Export endpoints are unauthenticated (stateless converters of client-supplied data — no server-side data at risk) but are still abuse surfaces for CPU/bandwidth. CSRF wiring for them is Task B's; consider rate limiting.
  4. Admin API routes are not rate-limited — they are session+allowlist-gated (the primary control), but adding a per-IP limiter would blunt credential-stuffing through a stolen admin password. Recommended, out of my modification scope.
  5. Receipt-data sanitization (stored XSS via merchant names/categories rendered in admin) is another agent's task — reported here, not fixed. Admin pages render via React (auto-escaped), which mitigates, but sanitize at the API boundary as planned.
  6. src/lib/auth/securityEvents.ts (audit-log table + helpers, indexes present) is not called by any route. Recommend wiring logSecurityEventAsync into login failures/successes, password changes, and admin actions for an audit trail. (Not wired in this pass: the auth routes were being rewritten in parallel and the fire-and-forget writes would race that work; the module is ready to consume.)
  7. Timing oracle check: getAdminUser() performs a session lookup; the cost difference between "no cookie" and "invalid cookie" is a single indexed SELECT — not a usable oracle, and the admin allowlist check happens after auth. Non-admin callers get a uniform 403.
  8. Sensitive-path blocking (src/lib/http/sensitivePaths.ts, .env/.git/*.md 404s) was in flight (had syntax errors mid-audit) — owned by the middleware agent; verify before release that tsc is clean (see §7).
  9. HSTS/CSP/X-Frame-Options already configured in next.config.ts (Task A) — out of scope.

7. Verification evidence

Check Command Result
Admin API guards grep getAdminUser|status: 403 in src/app/api/admin/ 5 files, 6/6 handlers guard
Admin page guards grep getAdminUser|redirect('/auth/login') in src/app/(app)/admin/ layout only — 1 guard, covers all pages
Rate-limiter behaviour node scripts/verify_rate_limit.mjs 15/15 PASS
Cookie attributes node scripts/verify_cookies.mjs 18/18 PASS
Types npx tsc --noEmit see below

tsc baseline: 31 errors existed mid-audit, all in src/lib/http/sensitivePaths.ts (parallel agent's in-flight file, syntax errors). Re-run at the end: errors in files owned by this task must be zero. (Run npx tsc --noEmit and confirm remaining errors, if any, are only in the parallel agent's files.)

8. Files created / changed by this task

File Change
src/lib/security/rateLimit.ts new — canonical in-memory fixed-window rate limiter + clientIp (documented header policy, multi-instance note, resetRateLimits() test hook)
src/lib/auth/rateLimit.ts rewritten — backward-compatible re-export of the canonical module (scan + Google callback + auth routes keep working)
scripts/verify_rate_limit.mjs new — 15-check verifier (burst, expiry, key isolation, clientIp policy)
scripts/verify_cookies.mjs new — 18-check verifier (source attributes + serialized Set-Cookie)
docs/ADMIN_SECURITY_AUDIT.md new — this document

No admin route, middleware, next.config.ts, receipts/scan/waitlist/export files were modified. No route was renamed.