WFDASH-WEBAPP/docs/audit/security-perf-prod.md
Dave 0efbae8dc5 feat(quotes): enhance search functionality and user experience
- Updated the search input handling to improve responsiveness by integrating a debounced search mechanism, reducing latency from 550 ms to approximately 300 ms.
- Added keyboard shortcut support for focusing the search box, enhancing accessibility and usability for frequent users.
- Improved the search placeholder to reflect all searchable fields, providing clearer guidance to operators.
- Introduced a clear-search button to streamline user interactions, allowing quick resets of the search input.
- Enhanced the display of search results with a live result count badge, providing immediate feedback on search outcomes.

These changes collectively enhance the search experience for operators, making it more intuitive and efficient.
2026-08-28 20:13:44 +02:00

35 KiB

Security, Performance & Production-Readiness Audit

Scope: WFDASH Next.js app + PocketBase backend. Date: 2026-08-28 Author: tom (audit only — no code changes shipped)

This document catalogues the issues that stood out while reading the codebase end-to-end. It is intentionally a "report", not a fix-it-up PR. Each finding has a severity (critical / major / minor), a file:line reference where it lives, and a recommended fix.

Conventions used in the file refs:

  • src/... — relative to repo root.
  • Line numbers reference the current state of the file at audit time.

0. Headline summary

Severity Count What it is
Critical 4 Hardcoded production-looking secrets, missing security headers, very weak AGENT_API_KEY default, plaintext ai_settings.api_key storage
Major 11 Many /api/v1/* routes have no auth, no rate limiting, no request-size limits, unauthenticated file proxy, public CORS * on MCP, localStorage PB token, no observability, dependency risks, console noise leaking PII, no CSP, no backup plan for ai_settings
Minor 9 N+1 patterns, untyped env reads, missing no-store on sensitive endpoints, no CI, file type / extension trust, race conditions on shared rows, no HTTP secure cookie enforcement in dev, etc.

The good news: there is no dangerouslySetInnerHTML of user-controlled content without a custom sanitizer (src/components/quote-email-card.tsx:195-237), no eval/new Function anywhere, the file-proxy has a hard whitelist of collections, the TV PIN handler uses a constant-time-ish check with a 250 ms flattening delay (src/app/api/tv/login/route.ts:34-46), and the auth helper re-validates the PB token against PB on every call instead of trusting the JWT.

The bad news is mostly the auth surface, secret handling, and the "we trust the dev identity cookie" model. Those are blocking for production.


1. Critical findings

1.1 Hardcoded production-looking credentials in repo root

File: .env:5 (PB_ADMIN_PASSWORD=<redacted — see .env, ~10-digit numeric>) File: .env:7 (AGENT_API_KEY=<redacted — see .env, 6-digit numeric>)

/Volumes/EXT/DEVELOPMENT-FORGEJO/WFDASH-WEBAPP/.env is checked into the working tree on disk and contains what look like real values for the production deployment (literal values redacted here to avoid permanently leaking them into git history when this doc is committed — see the actual .env file for the current values):

PB_ADMIN_PASSWORD=<redacted — 10-digit numeric string>
AGENT_API_KEY=<redacted — 6-digit numeric string>

.gitignore correctly excludes .env* (.gitignore:28), so the file is not in git history — but a .env.local and .env.local.bak sitting next to it (ls -la output) suggest the same values have been copied into per-developer files for a while. Whoever runs git status today won't see them, but a copy-paste into a screenshot, a tar/backup, a CI artifact, or a sibling repo will leak them. The length (6 digits for AGENT_API_KEY, 10 numeric for PB_ADMIN_PASSWORD) makes both trivially brute-forceable against a public deployment.

Fix:

  1. Rotate both immediately. Treat them as compromised.
  2. Move all secrets to a real secret store (Doppler / Vault / Coolify env / a 1Password CLI hook). Do not rely on .env.local for production.
  3. Add a pre-commit + CI grep that fails on the literal patterns PB_ADMIN_PASSWORD=, AGENT_API_KEY=, ANTHROPIC_API_KEY=, sk-, Bearer\s+\w+ in any committed file.
  4. Make AGENT_API_KEY at least 32 bytes of CSPRNG output (openssl rand -hex 32).

1.2 AGENT_API_KEY is a single bearer shared with the browser bundle

Files:

  • src/lib/server/agent-auth.ts:4-19 (the check)
  • src/app/api/mcp/route.ts (the route)
  • next.config.ts:16 (the rest)

assertAgentAuth accepts Authorization: Bearer <AGENT_API_KEY> and that single token is used by:

  • The MCP /api/mcp route, which calls the REST /api/v1/* routes internally.
  • The agent CLIs that drive the same routes from outside.

The MCP route is exposed with Access-Control-Allow-Origin: * (src/app/api/mcp/route.ts:78-87 — response.headers.set("Access-Control-Allow-Origin", origin ?? "*")). That means any origin on the public internet can call /api/mcp with that bearer token, and the operator's n8n workflow broker the same key. Combined with finding 1.1 (AGENT_API_KEY is a 6-digit numeric string — see redacted value), this is the single most damaging exposure in the system.

Fix:

  • Restrict Access-Control-Allow-Origin to an explicit list (e.g. the Hermes domain and the agent's own origin). Reject any other origin with 403.
  • Issue per-integration keys (n8n key, Hermes key, agent CLI key) and track them separately. Revoke the shared one.
  • Move MCP and /api/v1/* agent routes behind a different auth path (mTLS, signed JWT with short TTL, or at minimum an HMAC request signature that includes timestamp + nonce).

1.3 Plaintext api_key in ai_settings PB row, with no at-rest encryption

File: src/lib/server/ai-settings-service.ts:36-39 (the comment that explicitly says "plaintext by design") File: src/lib/server/ai-settings-service.ts:132-133, 235-236 (where the key is read/written)

ai_settings.api_key is stored as a plain string field on the PB ai_settings row. The operator explicitly chose visibility over encryption-at-rest in August 2026 (per the file's own comment), but:

  • The PB admin JWT and the in-app UI both read the row through admin PB (getAdminPocketBase), which means the key is in PB's on-disk SQLite. Anyone with shell access to the PB container (pocketbase/data/data.db) gets every operator's LLM API key.
  • The /api/v1/ai/settings route (src/app/api/v1/ai/settings/route.ts:43-60) does NOT include the secret in the response — but it does include api_key_set: boolean, so any bearer holder can probe the row to confirm whether a key is configured.
  • The write path accepts the key in a JSON body over HTTPS only if the operator has Coolify terminating TLS. In dev (HTTP) it goes plaintext.

Fix:

  • Encrypt api_key at rest using a key derived from process.env.AI_SETTINGS_ENCRYPTION_KEY (32-byte CSPRNG, rotated quarterly). Decrypt on read inside the service.
  • Move the key to a sidecar (1Password CLI, Vault Agent, Coolify secret). The PB row should hold a reference (vault path / secret id), not the secret itself.
  • Audit PB's data.db to confirm no prior keys were committed to backup snapshots.

1.4 Zero security headers and no CSP

File: next.config.ts (no headers() block at all)

next.config.ts:1-46 configures output: "standalone", serverExternalPackages, allowedDevOrigins (which includes "*"!), and redirects. It does not set any Content-Security-Policy, X-Frame-Options, Strict-Transport-Security, Referrer-Policy, or X-Content-Type-Options. The only "origin policy" in the file is allowedDevOrigins: [..., "*"], which is the most permissive possible value and is shipped in the production build because there is no env-gated branch.

Fix:

  • Add a headers() block in next.config.ts:

    async headers() {
      return [{
        source: "/:path*",
        headers: [
          { key: "Content-Security-Policy", value: "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; connect-src 'self' <POCKETBASE_URL> <HERMES_URL> <N8N_URL>; frame-ancestors 'none'; base-uri 'self'; form-action 'self';" },
          { key: "Strict-Transport-Security", value: "max-age=63072000; includeSubDomains; preload" },
          { key: "X-Content-Type-Options", value: "nosniff" },
          { key: "X-Frame-Options", value: "DENY" },
          { key: "Referrer-Policy", value: "strict-origin-when-cross-origin" },
          { key: "Permissions-Policy", value: "geolocation=(), microphone=(), camera=()" },
        ],
      }];
    }
    

    Tweak script-src to match your actual scripts (you have next/script and dangerouslySetInnerHTML in two components, so 'unsafe-inline' is currently needed; replace with nonces when you can).

  • Move allowedDevOrigins out of the production config. Gate it on process.env.NODE_ENV === "development" and remove the *.

  • Turn on HSTS at the Coolify/proxy level too (the header is already standard for Coolify + Traefik).


2. Major findings

2.1 Wide attack surface: 102 of 180 API routes have no auth

Files: src/app/api/v1/orders/[id]/attachments/upload/route.ts, src/app/api/v1/agent/scan/route.ts, src/app/api/v1/files/[collection]/[recordId]/[filename]/route.ts, plus every route in the following directories (sample listing):

  • src/app/api/v1/stock-purchases/**/attachments/**
  • src/app/api/v1/purchase-orders/**/attachments/**
  • src/app/api/v1/quotes/**/attachments/**
  • src/app/api/v1/quotes/[id]/pdf/** (PDF render endpoints)
  • src/app/api/v1/repairs/** (entire resource — all 6 routes unauthenticated)
  • src/app/api/v1/buyer-tasks/** (PO generation, mark-ready)
  • src/app/api/v1/collection-confirm/** (confirm-stock, invoice-upload)
  • src/app/api/v1/sprint/items/**/claim and attachments/upload
  • src/app/api/v1/invoicing/**
  • src/app/api/counter/orders/[id]/attachments/route.ts
  • src/app/api/invoicing/delivery-note/route.ts
  • src/app/api/invoicing/customers/route.ts
  • src/app/api/invoicing/delivery-note-pdf/route.ts
  • src/app/api/tv/login/route.ts (PIN-gated but no rate limiting, see 2.4)

A spot count via find src/app/api -name "route.ts" -exec grep -L "assertBrowserSession\|assertAgentAuth" {} \; | wc -l shows 102 of 180 routes have no assertAgentAuth or assertBrowserSession call. They rely on the route being called from the in-app UI, which in turn relies on PocketBase's collection rules being tight.

Concrete consequences:

  • POST /api/v1/orders/[id]/attachments/upload — anyone with the URL can append arbitrary files to any order's order_attachements field on PB. PB will accept up to 10 files per order (PB schema is the only gate).
  • GET /api/v1/agent/scan — the entire PO triage report (which orders are stuck, which suppliers owe responses) is world-readable to anyone who can reach the app.
  • GET /api/v1/files/[collection]/[recordId]/[filename] — gated by the ALLOWED_COLLECTIONS whitelist, but the whitelist is a reasonable superset of every collection that holds operator-uploaded documents. Anyone can read any operator's PDFs by guessing or scraping the URL shape (/api/v1/files/orders/<id>/<pdf>).
  • POST /api/v1/repairs/[id]/files/[field] — no auth, no rate-limit, no size check beyond what PB enforces. The field is validated against ALLOWED_FIELDS but nothing else.

Fix:

  • Apply a single auth boundary, ideally in middleware. The current src/middleware.ts:8-20 is a no-op pass-through. The comment correctly explains why (the cookie-validating code uses node:crypto, which Edge runtime can't bundle), but the alternative isn't "do nothing" — it's "validate auth in each route handler" and that work hasn't been done for these 102 routes.
  • Pragmatic stopgap: add a global API middleware (Node runtime) that:
    1. Skips only /api/v1/health, /api/tv/login, /api/counter/orders/[id]/attachments (PWA kiosk).
    2. For everything else under /api/v1/* and /api/mcp, require a valid AGENT_API_KEY or a assertBrowserSession pass.
    3. For /api/invoicing/*, /api/counter/*, /api/tv/* — per-route auth as appropriate.
  • Long-term: switch to a per-route _auth wrapper that the middleware can compose.

2.2 PocketBase auth token kept in localStorage

File: src/lib/pocketbase.ts:18-46

The PB SDK's authStore is persisted to localStorage under pocketbase_auth with the raw JWT and the user record. Any XSS on the app would let an attacker read this and impersonate the operator with full PB privileges (the token is the user's PB JWT, so it can do whatever the user can do per PB's collection rules).

The app has at least two dangerouslySetInnerHTML callsites that render either operator-controlled data (the email body) or theme-bootstrap script. The email body sanitizer (src/components/quote-email-card.tsx:195-237) is a hand-rolled regex sanitizer — it blocks javascript:, strips event handlers, and limits tags. But the comment at line 177 explicitly says "This is NOT a general-purpose sanitizer (use DOMPurify for that)." That is a future footgun if anyone adds a new tags-allowing bypass (e.g. <svg>-with-<use> is currently allowed because <svg> is not in the allow-list, but a small regression in the regex could let a <style> through if the stripper changes).

Fix:

  • Move PB tokens out of localStorage into an httpOnly; SameSite=Lax; Secure cookie set by a server endpoint. The browser can keep using PB's SDK by hitting that cookie. The current assertBrowserSession already does server-side validation — the gap is the storage location.
  • Replace the hand-rolled email sanitizer with DOMPurify (isomorphic-dompurify for SSR). Add it to the dep list and drop ~70 lines of custom regex code.
  • Add a strict CSP (see 1.4) so even if XSS happens, it can't call out.

2.3 No rate limiting anywhere

Files: every /api/**/route.ts, especially:

  • src/app/api/tv/login/route.ts (6-digit PIN brute-force window)
  • src/app/api/v1/quotes/[id]/from-email/route.ts (PB write loop)
  • src/app/api/mcp/route.ts (admin PB on every tool)
  • src/app/api/v1/orders/[id]/attachments/upload/route.ts (multipart upload)

The TV PIN is 6 digits, so 1 000 000 combinations. The handler has a 250 ms flattening delay (src/app/api/tv/login/route.ts:35-38), which means ~4000 guesses/min from a single IP. No X-Forwarded-For trust boundary is enforced, so an attacker spoofing source IPs can multiply that. No per-IP lockout.

assertAgentAuth does not rate-limit at all. A leaked AGENT_API_KEY (see 1.1) means a free pass to admin PB.

Fix:

  • Add a simple token-bucket rate limiter in src/middleware.ts (Edge runtime) keyed on the request IP. ~30 req/min per IP globally, ~5/min on /api/tv/login, ~10/min on agent routes.
  • For the TV PIN, lock out an IP for 60 seconds after 5 wrong attempts in a 5-minute window.
  • For the MCP and /api/v1/* agent routes, rate-limit per Authorization token (so a leaked key is at least throttled).
  • Consider adopting @upstash/ratelimit or next-safe-action's rate-limit helper to avoid hand-rolling token buckets.

2.4 File upload routes have no per-route size cap and trust PB's defaults

Files:

  • src/app/api/v1/orders/[id]/attachments/upload/route.ts:39-50 (the comment says "default 5 MB but operators usually set 25 MB")
  • src/app/api/v1/collection-confirm/invoice-upload/route.ts
  • src/app/api/counter/orders/[id]/attachments/route.ts:14-22 (only enforces a photo cap, but does it via env)
  • src/app/api/v1/repairs/[id]/files/[field]/route.ts

The order-attachment route says "PB enforces 25 MB" but never re-validates against the runtime value of PB's schema. An attacker can POST a 25 MB binary, PB will store it, the proxy will serve it back, and the operator pays the storage + bandwidth cost.

More importantly, MIME type is not validated at the route. file.type is browser-supplied and trivially spoofable in a direct curl. PB stores whatever the client claims. A PDF-claimed HTML file uploaded to order_attachements is then served back through /api/v1/files/orders/<id>/<file> with Content-Type: text/html (PB echoes the stored MIME) and rendered in the operator's preview pane. The preview pane uses <iframe> / <img> / <object> depending on type, so a successful upload turns into an XSS on the operator's next visit to the order detail page.

Fix:

  • In each upload route, after req.formData():
    1. Enforce a per-route cap (e.g. 10 MB for orders, 8 MB for counter photos) using file.size BEFORE handing to PB.
    2. Validate file.type against an allow-list per field (order_attachements: image/*, application/pdf; counter: image/* only; repairs.photos: image/* only; repairs.quote/PO/DN: application/pdf only).
    3. Validate the filename is not a path traversal attempt (no .., no leading /, no null bytes) and has an appropriate extension.
  • Set Content-Disposition: attachment; filename="..." in the proxy (src/app/api/v1/files/[collection]/[recordId]/[filename]/route.ts:152-154) for HTML and any other "executable" MIME, even if the operator originally uploaded it as PDF. The proxy's current isViewable heuristic trusts PB's stored content-type, which the attacker controls.

2.5 next.config.ts ships allowedDevOrigins: ["*"]

File: next.config.ts:18-20

allowedDevOrigins is a dev-server feature that lets the HMR client connect from a non-localhost origin. Setting it to * in a next.config.ts that's the same file used for next build and next start means the production server also allows HMR from any origin. While the HMR socket is dev-only, having * in the config is misleading and a sign that the dev/prod split is missing.

Fix:

const isDev = process.env.NODE_ENV !== "production";
const nextConfig: NextConfig = {
  output: "standalone",
  serverExternalPackages: ["pdf-parse", "pdfjs-dist", "@napi-rs/canvas"],
  ...(isDev ? { allowedDevOrigins: ["127.0.0.1", "localhost", "192.168.1.42", "192.168.1.122", "192.168.1.128"] } : {}),
  ...
};

2.6 next.config.ts redirects include the local PIN

File: next.config.ts:30-44

The /counter → /new-order and /tv → /pin redirects are fine. But the underscore-to-hyphen redirects for /api/v1/order_items etc. are a soft commitment: agents (and humans copy-pasting from PB collection names) reach the right URL, but it means the server will accept both spellings forever. The redirect is permanent: true so it 308s in perpetuity. That's a long-term naming inconsistency baked into the public API.

Fix:

  • Keep the redirects, but add an HTTP Deprecation or Sunset header on the underscore paths so callers can migrate.
  • Or, change PB collection names to use hyphens (PB allows it) and drop the redirect.

2.7 CORS on /api/mcp is * (any origin can use the agent key)

File: src/app/api/mcp/route.ts:78-87

function withCors(response: NextResponse, req: NextRequest): NextResponse {
  const origin = req.headers.get("origin");
  response.headers.set("Access-Control-Allow-Origin", origin ?? "*");
  ...
}

If the request has no Origin header (e.g. curl, a server-side agent), the response sets *. With an Origin, the response echoes the origin back. That is functionally Access-Control-Allow-Origin: * and credentialed CORS at once. The comment on line 68-77 says "Production should narrow the origin allow-list" — it has not been narrowed.

Fix:

  • Replace with an allow-list:

    const ALLOWED_ORIGINS = new Set([
      process.env.NEXT_PUBLIC_SITE_URL,
      "https://hermes.example.com",
    ]);
    const origin = req.headers.get("origin");
    if (origin && ALLOWED_ORIGINS.has(origin)) {
      response.headers.set("Access-Control-Allow-Origin", origin);
      response.headers.set("Vary", "Origin");
    }
    // No CORS headers = browser will block; CLI/server callers ignore
    

2.8 No structured logging / observability

Files: 132 console.* callsites across src/, no logger abstraction, no Sentry / OpenTelemetry / pino / datadog.

grep -rn "console\." src/ --include="*.ts" --include="*.tsx" | wc -l → 132. The counts include:

  • 13 in src/app/(app)/orders/[id]/process/page.tsx (operator PII in the body of the log lines)
  • 10 in src/app/api/v1/quotes/[id]/pdf/route.ts
  • 7 in src/app/api/v1/quotes/[id]/submit/route.ts

The messages include console.log of error bodies that contain the customer's email subject, the supplier name, and the parsed email body (src/app/(app)/orders/[id]/process/page.tsx:1396-1405). That is PII leaking into the server's stdout.

process.env is also used directly in many places without a typed loader (grep -rn "process.env" src/lib/server/ returns 30+ results), so a typo in an env var name (ORDER_ITEMS_COLLECTIONS vs ORDER_ITEMS_COLLECTION) silently falls back to a default and nobody notices.

Fix:

  • Add pino with a redact list (req.headers.authorization, pb.authStore.token, *.email, *.password). Wire it into the route-handler error catch in src/app/api/v1/*.
  • For the Next.js server side, set up pino-pretty in dev and ship JSON to stdout in prod (Coolify picks it up).
  • Add Sentry (or self-hosted GlitchTip) with the same redact config. Wrap the assertBrowserSession and assertAgentAuth calls so the path / actor / status code are recorded.
  • Centralise env access in src/lib/server/env.ts using zod or t3-env so typos fail at boot.
  • Replace console.log("[send-po] ...", pdfErr) and similar with logger.error({ err, ...context }, "send-po failed").

2.9 Lucide-react pinned to a very old 1.16.0 fork

File: package.json:73 — "lucide-react": "^1.16.0" File: package-lock.json:8499-8506 — confirmed version

The current published lucide-react package is at version 0.4xx+ (the 0.x line is the actively-maintained one by the Lucide maintainers). Version 1.16.0 is from a separate fork (the original Lucide pre-rebrand, ISC-licensed, last published 2018-ish). Peer-dep is "react": "^16.5.1 || ^17.0.0 || ^18.0.0 || ^19.0.0", so it does work with React 19, but it ships the old "Lucide" icon set (outlined style) and is no longer receiving security or icon updates.

This is a maintenance risk, not a known CVE. If a future advisory lands against the 1.x line, the project has no upgrade path short of re-mapping every icon import.

Fix:

  • Audit the icon list used (grep -rn "from \"lucide-react\"" src/).
  • Migrate to the 0.x line (lucide-react@^0.460.0) — every icon name has been preserved, but you'll get active maintenance
    • a much larger icon set.
  • If migrating is too expensive today, pin a known-good lockfile revision and add a Renovate / Dependabot rule to track upstream.

2.10 PDF rendering libs: pdfjs-dist 5.x and pdf-parse 2.4.5

File: package.json:78-79 File: next.config.ts:10-13 (the serverExternalPackages escape hatch)

pdfjs-dist 5.x has had multiple CVEs in 2024-2025 (font parser, JBIG2 decoder, arbitrary code execution on crafted PDFs). The escape hatch serverExternalPackages: ["pdf-parse", "pdfjs-dist", "@napi-rs/canvas"] is a workaround for webpack's RSC runtime, not a security control — and the comment (next.config.ts:8-13) admits the underlying lib has ESM/CJS quirks that suggest an older or untested version may be in use.

pdf-parse@2.4.5 is not the same package as the pdf-parse@1.1.1 that npm originally had; it's a separate maintainer with a recent rewrite, and there's no public security audit of 2.x yet.

Fix:

  • Pin both to the latest patch of a known-good line. Add npm audit to CI.
  • Prefer @react-pdf/renderer (already a dep) for any generated PDFs. Only use pdfjs-dist for parsing untrusted PDFs, and:
    1. Strip active content (JS, embedded files) before parse.
    2. Run parsing in a worker (already done via Next.js's node runtime) so a memory-corruption bug stays in the worker.
    3. Cap input size (see 2.4) so an attacker can't OOM the worker.

File: src/lib/dev-identity.ts:46-67

getServerIdentity reads identity in this order:

  1. X-User-Email request header (any caller can set this)
  2. wfdash_dev_email cookie (the UI switcher)
  3. DEV_USER_EMAIL env var
  4. DEFAULT_DEV_EMAIL = "david.els1@outlook.com"

The header is explicitly listed as a per-request override — but any HTTP client can set X-User-Email: <anything> and be attributed as that operator. The function is used by routes that need an actor name (audit notes, claim owners), so a malicious caller can spoof audit-trail entries by setting the header.

The comment block at the top of the file (dev-identity.ts:1-20) calls out a real migration plan: "when real PB auth lands, replace getServerIdentity with assertBrowserSession." Until then, the system is in an explicit "no auth" mode, but the routes that depend on identity don't all gate on it.

Fix:

  • Drop the X-User-Email header override. Make the cookie the only override. Validate the cookie against a known set of dev emails (or against pb.collection("users").authRefresh).

  • Audit every call site:

    grep -rn "getServerIdentity\|DEV_IDENTITY" src/app/api
    

    and ensure each is assertBrowserSession-gated before trusting the identity.

  • Set a clear DEADLINE (e.g. 30 days) for removing the DEFAULT_DEV_EMAIL fallback so this can't quietly ship to prod.


3. Minor findings

3.1 CORS preflight sets Access-Control-Max-Age: 86400

File: src/app/api/mcp/route.ts:85

CORS preflight is cached for 24h. If you tighten the CORS allow-list (see 2.7), browsers will keep allowing disallowed origins for 24h. Lower to 600s (10 min) during the transition.

3.2 No request size cap on JSON endpoints

File: all req.json() callers in src/app/api/v1/*/route.ts

The default Next.js req.json() is bounded by the framework defaults, but no route explicitly sets a max. A 100 MB JSON body on POST /api/v1/orders/create would parse, allocate, and only then be rejected as "items must be a non-empty array".

Fix: Add an explicit content-length check or use req.json({ limit: "1mb" }) where supported.

3.3 assertBrowserSession doesn't bind to PB user id

File: src/lib/server/browser-session.ts:30-44

The function refreshes the JWT but never extracts the user id. Downstream code that needs to "is this user the one who claimed this row?" can't tell. Add a return value:

export async function assertBrowserSession(req: NextRequest): Promise<{ id: string; record: PBRecord }> { ... }

and propagate the id into getServerIdentity so the two paths converge.

3.4 PB admin auth token cached in module-level memory indefinitely

File: src/lib/server/pocketbase-admin.ts:7-15, 78-89

cachedAdminAuth lives in the Node module scope. In a serverless deploy (Vercel, AWS Lambda) this is fine (per-invocation). In the standalone Next.js server (Coolify), it lives for the process lifetime. If an operator's PB password is rotated, every running instance keeps using the old token until process restart (5-minute drift, per the ADMIN_TOKEN_REFRESH_BUFFER_MS = 60_000 buffer). The clearAdminPocketBaseAuthCache export exists but no caller uses it.

Fix: Call clearAdminPocketBaseAuthCache from a process.on("SIGHUP") (or a /api/v1/admin/reload-auth debug-only endpoint) so a PB password rotation can take effect without a process restart.

3.5 Many getFullList calls — N+1 risk

File: src/lib/server/orders-service.ts:433, 473, 482, 621, 657, 696, 873

The service uses pb.collection(...).getFullList(...) in many places. PB paginates this internally (default 500/batch), so "full list" really means "iterate until done". For collections in the tens of thousands (order_items is approaching that), this is slow under load.

Fix: Replace with paginated reads or filter-then-page where the caller only needs a slice. Add a regression test that asserts total latency for a 5 000-order workspace.

3.6 No CI, no test gating, no pre-commit hooks

Files: package.json:73-90 has no lint:fix, no format:check, no test:ci script. No .github/workflows/, no .gitlab-ci.yml. The Dockerfile builds but doesn't run tests.

Fix:

  • Add a GitHub Actions workflow that runs npm run lint, npm run test:run, and npm audit --audit-level=high on every PR.
  • Add Husky + lint-staged to enforce formatting on commit.

3.7 console.error messages embed PII (operator names, customer emails)

File: src/app/(app)/orders/[id]/process/page.tsx:1276, 1285, 1396, 1405

Examples (paraphrased — see actual file for exact strings):

  • console.error("[send-po] PDF fetch threw:", pdfErr) — pdfErr includes the full response body, which includes the order number + customer email.
  • console.error("[send-po] PDF generation failed:", errBody) — ditto.
  • console.log(...) in the same file dumps the customer subject line to stdout.

Fix: Adopt a logger.error({ err, orderId, userId, route }, ...) shape (see 2.8) and never log raw request/response bodies.

File: src/lib/server/tv-session.ts:80-83 File: src/app/api/tv/login/route.ts:64-66

The cookie sets secure: req.nextUrl.protocol === "https:". In dev (http://localhost), the cookie goes out as Secure: false which is correct for localhost. But there is no environment check that prevents secure: false from shipping to a production Coolify deploy that terminates TLS at a proxy and forwards plain HTTP internally — if req.nextUrl.protocol ever reads http in that scenario (it should, since Next sees the incoming protocol from X-Forwarded-Proto only when explicitly configured), the cookie will be sent over plain HTTP and an attacker on the LAN can steal it.

Fix: Force secure: process.env.NODE_ENV === "production" in the cookie options; trust the env, not the request protocol.

3.9 MCP_ERR_NETWORK (-32010) and MCP_ERR_UPSTREAM (-32011) can leak internal URLs

File: src/lib/server/mcp-upstream-fetch.ts:55-75

The error envelope includes data.url — the full upstream URL the MCP route tried to call. In a container deploy that URL is https://0.0.0.0:3000 (per the Dockerfile HOSTNAME="0.0.0.0" and the upstreamOrigin fix in the MCP route). The comment acknowledges the issue but the fix is partial. An agent that reads this error and uses the URL to call back will hit 0.0.0.0 from outside the container and fail.

Fix: Strip the URL down to origin + path in production. For local-dev, keep the full URL. Use an env flag.

3.10 No CORS on /api/v1/files/... proxy

File: src/app/api/v1/files/[collection]/[recordId]/[filename]/route.ts

The file proxy is meant to be loaded by <iframe src=...> and <a target=_blank href=...>. Both are same-origin in the operator flow, so no CORS is needed today. But if a Hermes frontend tries to embed a PDF preview in an <iframe>, the browser will block it because there's no Access-Control-Allow-Origin. Either add CORS to the proxy for the Hermes origin, or document that the proxy is same-origin-only.

3.11 tsconfig paths used in tests require real process.env setup

File: vitest.config.ts (not opened but referenced in README) File: src/lib/server/agent-auth.test.ts:6-16 (must set AGENT_API_KEY before import)

Many tests assume process.env.PB_ADMIN_EMAIL / PB_ADMIN_PASSWORD are set. If they're missing at import time, tests fail with confusing errors. Add a test-setup.ts (referenced in package.json) that asserts the env or stubs getAdminPocketBase.

3.12 output: "standalone" includes dev artifacts in the image

File: Dockerfile:1-31

The Dockerfile uses node:20-alpine (3-year-old EOL track — Node 20 is in maintenance until April 2026, EOL October 2026). The builder stage copies . into the image, including the docs/, scripts/, e2e/, screenshots, .audit-screenshots/, and PB exports. The runner stage only copies public/, .next/standalone/, .next/static/, so the extras don't ship — but the build cache is bigger than it needs to be.

Fix:

  • Add .dockerignore excluding everything except src/, public/, package.json, package-lock.json, next.config.ts, postcss.config.mjs, tsconfig.json, and next-env.d.ts.
  • Bump base image to node:22-alpine (current LTS as of 2026-08). Pin to a specific minor (e.g. node:22.11-alpine) for reproducibility.

4. Production readiness checklist (roll-up)

# Item Status Owner
1 Rotate AGENT_API_KEY to ≥32 bytes CSPRNG blocking ops
2 Rotate PB_ADMIN_PASSWORD blocking ops
3 Move secrets out of .env files into a real store blocking ops
4 Add CSP, HSTS, X-CTO, X-Frame-Options, Referrer-Policy headers blocking dev
5 Add a global API auth middleware; remove allowedDevOrigins: "*" from prod config blocking dev
6 Restrict CORS on /api/mcp to an allow-list blocking dev
7 Move PB auth token out of localStorage into httpOnly cookie blocker for prod dev
8 Replace hand-rolled email HTML sanitizer with DOMPurify major dev
9 Add per-route upload size cap + MIME validation major dev
10 Add rate limiting (TV PIN, agent routes, global) major dev
11 Encrypt ai_settings.api_key at rest major dev
12 Add structured logging + Sentry/GlitchTip; redact PII major dev
13 Add CI (lint + test + audit) major dev
14 Replace getFullList patterns with paginated reads where feasible perf dev
15 Drop X-User-Email header override in getServerIdentity security dev
16 Force Secure cookie in prod for TV session minor dev
17 Migrate lucide-react to the 0.x maintained line maintenance dev
18 Bump base Docker image to Node 22 LTS maintenance ops
19 Centralise process.env access in a typed env module quality dev
20 Add CORS to file proxy for Hermes embed depends on Hermes scope dev

5. What I deliberately did not flag

  • escapeFilterValue in src/app/(app)/invoicing/page.tsx:103 — The filter strings are built from server-resolved PB ids and the comment makes the escaping contract explicit. This is safe.
  • The hand-rolled email sanitizer in src/components/quote-email-card.tsx:195-237 — Has gaps (no DOMPurify) but is well-tested in scope. The recommendation is to swap it for DOMPurify, not to remove the current check.
  • getServerIdentity cookie path — The cookie path is /, SameSite=Lax, and 1 year max-age. For the dev-identity temporary stand-in this is acceptable; the real fix is to replace it (see 2.11).
  • assertAgentAuth timing — String comparison of fixed-length tokens is not constant-time, but the token is large enough that timing attacks are impractical. If you rotate to a 32-byte CSPRNG token, this becomes irrelevant.
  • PDF and image generation libs (jspdf, pdf-lib, @react-pdf/renderer) — These are widely used and have no known critical CVEs at the pinned versions. Re-evaluate at next major bump.
  • xlsx and mammoth — Used for operator-supplied file parsing (price lists, supplier docs). They have a long history of CVEs around embedded XXE / formula injection. They are scoped to trusted-operator input today, but if you ever expose the upload UI to a customer, the same input boundary needs the same treatment as 2.4.

End of audit. No code was changed during this task — all recommendations are documented here for the team to triage.