Files
TREK/AUDIT_FINDINGS.md
Claude 5f07bdaaf1 docs: add comprehensive security and code quality audit findings
AUDIT_FINDINGS.md documents all findings across security, code quality,
best practices, dependency hygiene, documentation, and testing categories.

https://claude.ai/code/session_01SoQKcF5Rz9Y8Nzo4PzkxY8
2026-03-31 00:33:50 +00:00

20 KiB

TREK Security & Code Quality Audit

Date: 2026-03-30 Auditor: Automated comprehensive audit Scope: Full codebase — server, client, infrastructure, dependencies


Table of Contents

  1. Security
  2. Code Quality
  3. Best Practices
  4. Dependency Hygiene
  5. Documentation & DX
  6. Testing
  7. Remediation Summary

1. Security

1.1 General

# Severity File Line(s) Description Recommended Fix Status
S-1 CRITICAL server/src/middleware/auth.ts 17 JWT verify() does not pin algorithm — accepts whatever algorithm is in the token header, potentially including none. Pass { algorithms: ['HS256'] } to all jwt.verify() calls. FIXED
S-2 HIGH server/src/websocket.ts 56 Same JWT verify without algorithm pinning in WebSocket auth. Pin algorithm to HS256. FIXED
S-3 HIGH server/src/middleware/mfaPolicy.ts 54 Same JWT verify without algorithm pinning. Pin algorithm to HS256. FIXED
S-4 HIGH server/src/routes/oidc.ts 84-88 OIDC generateToken() includes excessive claims (username, email, role) in JWT payload. If the JWT is leaked, this exposes PII. Only include { id: user.id } in token, consistent with auth.ts. FIXED
S-5 HIGH client/src/api/websocket.ts 27 Auth token passed in WebSocket URL query string (?token=). Tokens in URLs appear in server logs, proxy logs, and browser history. Document as known limitation; WebSocket protocol doesn't easily support headers from browsers. Add LOW priority note to switch to message-based auth in the future. DOCUMENTED
S-6 HIGH client/vite.config.js 47-56 Service worker caches ALL /api/.* responses with NetworkFirst, including auth tokens, user data, budget, reservations. Data persists after logout. Exclude sensitive API paths from caching: /api/auth/.*, /api/admin/.*, /api/backup/.*. FIXED
S-7 HIGH client/vite.config.js 57-65 User-uploaded files (possibly passport scans, booking confirmations) cached with CacheFirst for 30 days, persisting after logout. Reduce cache lifetime; add note about clearing on logout. FIXED
S-8 MEDIUM server/src/index.ts 60 CSP allows 'unsafe-inline' for scripts, weakening XSS protection. Remove 'unsafe-inline' from scriptSrc if Vite build doesn't require it. If needed for development, only allow in non-production. FIXED
S-9 MEDIUM server/src/index.ts 64 CSP connectSrc allows http: and https: broadly, permitting connections to any origin. Restrict to known API domains (nominatim, overpass, Google APIs) or use 'self' with specific external origins. FIXED
S-10 MEDIUM server/src/index.ts 62 CSP imgSrc allows http: broadly. Restrict to https: and 'self' plus known image domains. FIXED
S-11 MEDIUM server/src/websocket.ts 84-90 No message size limit on WebSocket messages. A malicious client could send very large messages to exhaust server memory. Set maxPayload on WebSocketServer configuration. FIXED
S-12 MEDIUM server/src/websocket.ts 84 No rate limiting on WebSocket messages. A client can flood the server with join/leave messages. Add per-connection message rate limiting. FIXED
S-13 MEDIUM server/src/websocket.ts 29 No origin validation on WebSocket connections. Add origin checking against allowed origins. FIXED
S-14 MEDIUM server/src/routes/auth.ts 157-163 JWT tokens have 24h expiry with no refresh token mechanism. Long-lived tokens increase window of exposure if leaked. Document as accepted risk for self-hosted app. Consider refresh tokens in future. DOCUMENTED
S-15 MEDIUM server/src/routes/auth.ts 367-368 Password change does not invalidate existing JWT tokens. Old tokens remain valid for up to 24h. Implement token version/generation tracking, or reduce token expiry and add refresh tokens. REQUIRES MANUAL REVIEW
S-16 MEDIUM server/src/services/mfaCrypto.ts 2, 5 MFA encryption key is derived from JWT_SECRET. If JWT_SECRET is compromised, all MFA secrets are also compromised. Single point of failure. Use a separate MFA_ENCRYPTION_KEY env var, or derive using a different salt/purpose. Current implementation with :mfa:v1 salt is acceptable but tightly coupled. DOCUMENTED
S-17 MEDIUM server/src/routes/maps.ts 429 Google API key exposed in URL query string (&key=${apiKey}). Could appear in logs. Use header-based auth (X-Goog-Api-Key) consistently. Already used elsewhere in the file. FIXED
S-18 MEDIUM MCP.md 232-235 Contains publicly accessible database download link with hardcoded credentials (admin@admin.com / admin123). Remove credentials from documentation. FIXED
S-19 LOW server/src/index.ts 229 Error handler logs full error object including stack trace to console. In containerized deployments, this could leak to centralized logging. Sanitize error logging in production. FIXED
S-20 LOW server/src/routes/backup.ts 301-304 Error detail leaked in non-production environments (detail: process.env.NODE_ENV !== 'production' ? msg : undefined). Acceptable for dev, but ensure it's consistently not leaked in production. Already correct. OK

1.2 Auth (JWT + OIDC + TOTP)

# Severity File Line(s) Description Recommended Fix Status
A-1 CRITICAL All jwt.verify calls Multiple JWT algorithm not pinned. jsonwebtoken library defaults to accepting the algorithm specified in the token header, which could include none. Add { algorithms: ['HS256'] } to every jwt.verify() call. FIXED
A-2 MEDIUM server/src/routes/auth.ts 315-318 MFA login token uses same JWT_SECRET and same jwt.sign(). Purpose field mfa_login prevents misuse but should use a shorter expiry. Currently 5m which is acceptable. OK — 5 minute expiry is reasonable. OK
A-3 MEDIUM server/src/routes/oidc.ts 113-143 OIDC redirect URI is dynamically constructed from request headers (x-forwarded-proto, x-forwarded-host). An attacker who can control these headers could redirect the callback to a malicious domain. Validate the constructed redirect URI against an allowlist, or use a configured base URL from env vars. FIXED
A-4 LOW server/src/routes/auth.ts 21 TOTP window: 1 allows codes from adjacent time periods (±30s). This is standard and acceptable. OK OK

1.3 SQLite (better-sqlite3)

# Severity File Line(s) Description Recommended Fix Status
D-1 HIGH server/src/routes/files.ts 90-91 Dynamic SQL with IN (${placeholders}) — however, placeholders are correctly generated from array length and values are parameterized. Not an injection risk. OK — pattern is safe. OK
D-2 MEDIUM server/src/routes/auth.ts 455 Dynamic SQL UPDATE users SET ${updates.join(', ')} WHERE id = ? — column names come from controlled server-side code, not user input. Parameters are properly bound. OK — column names are from a controlled set. OK
D-3 LOW server/src/db/database.ts 26-28 WAL mode and busy_timeout configured. Good. OK OK

1.4 WebSocket (ws)

# Severity File Line(s) Description Recommended Fix Status
W-1 MEDIUM server/src/websocket.ts 29 No maxPayload set on WebSocketServer. Default is 100MB which is excessive. Set maxPayload: 64 * 1024 (64KB). FIXED
W-2 MEDIUM server/src/websocket.ts 84-110 Only join and leave message types are handled; unknown types are silently ignored. This is acceptable but there is no schema validation on the message structure. Add basic type/schema validation using Zod. FIXED
W-3 LOW server/src/websocket.ts 88 JSON.parse errors are silently caught with empty catch. Log malformed messages at debug level. FIXED

1.5 Express

# Severity File Line(s) Description Recommended Fix Status
E-1 LOW server/src/index.ts 82 Body parser limit set to 100KB. Good. OK OK
E-2 LOW server/src/index.ts 14-16 Trust proxy configured conditionally. Good. OK OK
E-3 LOW server/src/index.ts 121-136 Path traversal protection on uploads endpoint. Uses path.basename and path.resolve check. Good. OK OK

1.6 PWA / Workbox

# Severity File Line(s) Description Recommended Fix Status
P-1 HIGH client/vite.config.js 47-56 API response caching includes sensitive endpoints. Exclude auth, admin, backup, and settings endpoints from caching. FIXED
P-2 MEDIUM client/vite.config.js 23, 31, 42, 54, 63 cacheableResponse: { statuses: [0, 200] } — status 0 represents opaque responses which may cache error responses silently. Remove status 0 from API and upload caches (keep for CDN/map tiles where CORS may return opaque responses). FIXED
P-3 MEDIUM client/src/store/authStore.ts 126-135 Logout does not clear service worker caches. Sensitive data persists after logout. Clear CacheStorage for api-data and user-uploads caches on logout. FIXED

2. Code Quality

# Severity File Line(s) Description Recommended Fix Status
Q-1 MEDIUM client/src/store/authStore.ts 153-161 loadUser silently catches all errors and logs user out. A transient network failure logs the user out. Only logout on 401 responses, not on network errors. FIXED
Q-2 MEDIUM client/src/hooks/useRouteCalculation.ts 36 useCallback depends on entire tripStore object, defeating memoization. Select only needed properties from the store. DOCUMENTED
Q-3 MEDIUM client/src/hooks/useTripWebSocket.ts 14 collabFileSync captures stale tripStore reference from initial render. Use useTripStore.getState() instead. DOCUMENTED
Q-4 MEDIUM client/src/store/authStore.ts 38 vs 105 register function accepts 4 params but TypeScript interface only declares 3. Update interface to include optional invite_token. FIXED
Q-5 LOW client/src/store/slices/filesSlice.ts Empty catch block on file link operation (catch {}). Log error. DOCUMENTED
Q-6 LOW client/src/App.tsx 101, 108 Empty catch blocks silently swallow errors. Add minimal error logging. DOCUMENTED
Q-7 LOW client/src/App.tsx 155 RegisterPage imported but never used — /register route renders LoginPage. Remove unused import. FIXED
Q-8 LOW client/tsconfig.json 14 strict: false disables TypeScript strict mode. Enable strict mode and fix resulting type errors. REQUIRES MANUAL REVIEW
Q-9 LOW client/src/main.tsx 7 Non-null assertion on getElementById('root')!. Add null check. DOCUMENTED
Q-10 LOW server/src/routes/files.ts 278 Empty catch block on file link insert (catch {}). Log duplicate link errors. FIXED
Q-11 LOW server/src/db/database.ts 20-21 Silent catch on WAL checkpoint in initDb. Log warning on failure. DOCUMENTED

3. Best Practices

3.1 Node / Express

# Severity File Line(s) Description Recommended Fix Status
B-1 LOW server/src/index.ts 251-271 Graceful shutdown implemented with SIGTERM/SIGINT handlers. Good — closes DB, HTTP server, with 10s timeout. OK OK
B-2 LOW server/src/index.ts 87-112 Debug logging redacts sensitive fields. Good. OK OK

3.2 React / Vite

# Severity File Line(s) Description Recommended Fix Status
V-1 MEDIUM client/vite.config.js No explicit build.sourcemap: false for production. Source maps may be generated. Add build: { sourcemap: false } to Vite config. FIXED
V-2 LOW client/index.html 24 Leaflet CSS loaded from unpkg CDN without Subresource Integrity (SRI) hash. Add integrity and crossorigin attributes. FIXED

3.3 Docker

# Severity File Line(s) Description Recommended Fix Status
K-1 MEDIUM Dockerfile 2, 10 Base images use floating tags (node:22-alpine), not pinned to digest. Pin to specific digest for reproducible builds. DOCUMENTED
K-2 MEDIUM Dockerfile No HEALTHCHECK instruction. Only docker-compose has health check. Add HEALTHCHECK to Dockerfile for standalone deployments. FIXED
K-3 LOW .dockerignore Missing exclusions for chart/, docs/, .github/, docker-compose.yml, *.sqlite*. Add missing exclusions. FIXED

3.4 docker-compose.yml

# Severity File Line(s) Description Recommended Fix Status
C-1 HIGH docker-compose.yml 25 JWT_SECRET defaults to empty string if not set. App auto-generates one, but it changes on restart, invalidating all sessions. Log a prominent warning on startup if JWT_SECRET is auto-generated. FIXED
C-2 MEDIUM docker-compose.yml No resource limits defined for the app service. Add deploy.resources.limits section. DOCUMENTED

3.5 Git Hygiene

# Severity File Line(s) Description Recommended Fix Status
G-1 HIGH .gitignore 12-14 Missing *.sqlite, *.sqlite-wal, *.sqlite-shm patterns. Only *.db variants covered. Add sqlite patterns. FIXED
G-2 LOW No .env or .sqlite files found in git history. OK OK

3.6 Helm Chart

# Severity File Line(s) Description Recommended Fix Status
H-1 MEDIUM chart/templates/secret.yaml 22 randAlphaNum 32 generates a new JWT secret on every helm upgrade, invalidating all sessions. Use lookup to preserve existing secret across upgrades. FIXED
H-2 MEDIUM chart/values.yaml 3 Default image tag is latest. Use a specific version tag. DOCUMENTED
H-3 MEDIUM chart/templates/deployment.yaml No securityContext on pod or container. Runs as root by default. Add runAsNonRoot: true, runAsUser: 1000. FIXED
H-4 MEDIUM chart/templates/pvc.yaml PVC always created regardless of .Values.persistence.enabled. Add conditional check. FIXED
H-5 LOW chart/values.yaml 41 resources: {} — no default resource requests or limits. Add sensible defaults. FIXED

4. Dependency Hygiene

4.1 npm audit

Package Severity Description Status
serialize-javascript (via vite-plugin-pwa → workbox-build → @rollup/plugin-terser) HIGH RCE via RegExp.flags / CPU exhaustion DoS Fix requires vite-plugin-pwa major version upgrade.
picomatch (via @rollup/pluginutils, tinyglobby) MODERATE ReDoS via extglob quantifiers npm audit fix available.

Server: 0 vulnerabilities.

4.2 Outdated Dependencies (Notable)

Package Current Latest Risk Status
express ^4.18.3 5.2.1 Major version — breaking changes DOCUMENTED
uuid ^9.0.0 13.0.0 Major version DOCUMENTED
dotenv ^16.4.1 17.3.1 Major version DOCUMENTED
lucide-react ^0.344.0 1.7.0 Major version DOCUMENTED
react ^18.2.0 19.2.4 Major version DOCUMENTED
zustand ^4.5.2 5.0.12 Major version DOCUMENTED

Major version upgrades require manual evaluation and testing. Not applied in this remediation pass.


5. Documentation & DX

# Severity File Description Recommended Fix Status
X-1 MEDIUM server/.env.example Missing many env vars documented in README: OIDC_*, FORCE_HTTPS, TRUST_PROXY, DEMO_MODE, TZ, ALLOWED_ORIGINS, DEBUG. Add all configurable env vars. FIXED
X-2 MEDIUM server/.env.example JWT_SECRET placeholder is your-super-secret-jwt-key-change-in-production — easily overlooked. Use CHANGEME_GENERATE_WITH_openssl_rand_hex_32. FIXED
X-3 LOW server/.env.example PORT=3001 differs from Docker default of 3000. Align to 3000. FIXED

6. Testing

# Severity Description Status
T-1 HIGH No test files found anywhere in the repository. Zero test coverage for auth flows, WebSocket handling, SQLite queries, API routes, or React components. REQUIRES MANUAL REVIEW
T-2 HIGH No test framework configured (no jest, vitest, or similar in dependencies). REQUIRES MANUAL REVIEW
T-3 MEDIUM No CI step runs tests before building Docker image. DOCUMENTED

7. Remediation Summary

Applied Fixes

  • JWT algorithm pinning — Added { algorithms: ['HS256'] } to all jwt.verify() calls (auth middleware, MFA policy, WebSocket, OIDC, auth routes)
  • OIDC token payload — Reduced to { id } only, matching auth.ts pattern
  • OIDC redirect URI validation — Validates against APP_URL env var when set
  • WebSocket hardening — Added maxPayload: 64KB, message rate limiting (30 msg/10s), origin validation, improved message validation
  • CSP tightening — Removed 'unsafe-inline' from scripts in production, restricted connectSrc and imgSrc to known domains
  • PWA cache security — Excluded sensitive API paths from caching, removed opaque response caching for API/uploads, clear caches on logout
  • Service worker cache cleanup on logout
  • Google API key — Moved from URL query string to header in maps photo endpoint
  • MCP.md credentials — Removed hardcoded demo credentials
  • .gitignore — Added *.sqlite* patterns
  • .dockerignore — Added missing exclusions
  • Dockerfile — Added HEALTHCHECK instruction
  • Helm chart — Fixed secret rotation, added securityContext, conditional PVC, resource defaults
  • Vite config — Disabled source maps in production
  • CDN integrity — Added SRI hash for Leaflet CSS
  • .env.example — Complete with all env vars
  • Various code quality fixes — Removed dead imports, fixed empty catch blocks, fixed auth store interface

Requires Manual Review

  • Password change should invalidate existing tokens (S-15)
  • TypeScript strict mode should be enabled (Q-8)
  • Test suite needs to be created from scratch (T-1, T-2)
  • Major dependency upgrades (express 5, React 19, zustand 5, etc.)
  • serialize-javascript vulnerability fix requires vite-plugin-pwa major upgrade

Accepted Risks (Documented)

  • WebSocket token in URL query string (browser limitation)
  • 24h JWT expiry without refresh tokens (acceptable for self-hosted)
  • MFA encryption key derived from JWT_SECRET (noted coupling)
  • localStorage for token storage (standard SPA pattern)