- 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.
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:
- Rotate both immediately. Treat them as compromised.
- Move all secrets to a real secret store (Doppler / Vault / Coolify
env / a 1Password CLI hook). Do not rely on
.env.localfor production. - Add a
pre-commit+ CI grep that fails on the literal patternsPB_ADMIN_PASSWORD=,AGENT_API_KEY=,ANTHROPIC_API_KEY=,sk-,Bearer\s+\w+in any committed file. - Make
AGENT_API_KEYat 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/mcproute, 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-Originto 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/settingsroute (src/app/api/v1/ai/settings/route.ts:43-60) does NOT include the secret in the response — but it does includeapi_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_keyat rest using a key derived fromprocess.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.dbto 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 innext.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-srcto match your actual scripts (you havenext/scriptanddangerouslySetInnerHTMLin two components, so'unsafe-inline'is currently needed; replace with nonces when you can). -
Move
allowedDevOriginsout of the production config. Gate it onprocess.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/**/claimandattachments/uploadsrc/app/api/v1/invoicing/**src/app/api/counter/orders/[id]/attachments/route.tssrc/app/api/invoicing/delivery-note/route.tssrc/app/api/invoicing/customers/route.tssrc/app/api/invoicing/delivery-note-pdf/route.tssrc/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'sorder_attachementsfield 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 theALLOWED_COLLECTIONSwhitelist, 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 againstALLOWED_FIELDSbut nothing else.
Fix:
- Apply a single auth boundary, ideally in middleware. The
current
src/middleware.ts:8-20is a no-op pass-through. The comment correctly explains why (the cookie-validating code usesnode: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:
- Skips only
/api/v1/health,/api/tv/login,/api/counter/orders/[id]/attachments(PWA kiosk). - For everything else under
/api/v1/*and/api/mcp, require a validAGENT_API_KEYor aassertBrowserSessionpass. - For
/api/invoicing/*,/api/counter/*,/api/tv/*— per-route auth as appropriate.
- Skips only
- Long-term: switch to a per-route
_authwrapper 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
localStorageinto anhttpOnly; SameSite=Lax; Securecookie set by a server endpoint. The browser can keep using PB's SDK by hitting that cookie. The currentassertBrowserSessionalready does server-side validation — the gap is the storage location. - Replace the hand-rolled email sanitizer with DOMPurify
(
isomorphic-dompurifyfor 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 perAuthorizationtoken (so a leaked key is at least throttled). - Consider adopting
@upstash/ratelimitornext-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.tssrc/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():- Enforce a per-route cap (e.g. 10 MB for orders, 8 MB for
counter photos) using
file.sizeBEFORE handing to PB. - Validate
file.typeagainst an allow-list per field (order_attachements:image/*,application/pdf; counter:image/*only; repairs.photos:image/*only; repairs.quote/PO/DN:application/pdfonly). - Validate the filename is not a path traversal attempt
(no
.., no leading/, no null bytes) and has an appropriate extension.
- Enforce a per-route cap (e.g. 10 MB for orders, 8 MB for
counter photos) using
- 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 currentisViewableheuristic 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
DeprecationorSunsetheader 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
pinowith a redact list (req.headers.authorization,pb.authStore.token,*.email,*.password). Wire it into the route-handler error catch insrc/app/api/v1/*. - For the Next.js server side, set up
pino-prettyin 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
assertBrowserSessionandassertAgentAuthcalls so the path / actor / status code are recorded. - Centralise env access in
src/lib/server/env.tsusingzodort3-envso typos fail at boot. - Replace
console.log("[send-po] ...", pdfErr)and similar withlogger.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.xline (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 auditto CI. - Prefer
@react-pdf/renderer(already a dep) for any generated PDFs. Only usepdfjs-distfor parsing untrusted PDFs, and:- Strip active content (JS, embedded files) before parse.
- Run parsing in a worker (already done via Next.js's node runtime) so a memory-corruption bug stays in the worker.
- Cap input size (see 2.4) so an attacker can't OOM the worker.
2.11 dev-identity cookie is the de-facto auth for many flows
File: src/lib/dev-identity.ts:46-67
getServerIdentity reads identity in this order:
X-User-Emailrequest header (any caller can set this)wfdash_dev_emailcookie (the UI switcher)DEV_USER_EMAILenv varDEFAULT_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-Emailheader override. Make the cookie the only override. Validate the cookie against a known set of dev emails (or againstpb.collection("users").authRefresh). -
Audit every call site:
grep -rn "getServerIdentity\|DEV_IDENTITY" src/app/apiand ensure each is
assertBrowserSession-gated before trusting the identity. -
Set a clear
DEADLINE(e.g. 30 days) for removing theDEFAULT_DEV_EMAILfallback 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, andnpm audit --audit-level=highon 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)—pdfErrincludes 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.
3.8 TV session cookie is not secure in dev
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
.dockerignoreexcluding everything exceptsrc/,public/,package.json,package-lock.json,next.config.ts,postcss.config.mjs,tsconfig.json, andnext-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
escapeFilterValueinsrc/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. getServerIdentitycookie 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).assertAgentAuthtiming — 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. xlsxandmammoth— 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.