| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
- **Immich SSRF prevention** — Added URL validation on save (block private IPs, metadata endpoints, non-HTTP protocols)
- **Immich API key isolation** — Removed `userId` query parameter from asset proxy endpoints; all Immich requests now use authenticated user's own credentials only
- **Immich asset ID validation** — Added alphanumeric pattern validation to prevent path traversal in proxied URLs
| I-1 | **CRITICAL** | `server/src/routes/immich.ts` | 38-39, 85, 199, 250, 274 | SSRF via user-controlled `immich_url`. Users can set any URL which is then used in `fetch()` calls, allowing requests to internal metadata endpoints (169.254.169.254), localhost services, etc. | Validate URL on save: require HTTP(S) protocol, block private/internal IPs. | FIXED |
| I-2 | **CRITICAL** | `server/src/routes/immich.ts` | 194-196, 244-246, 269-270 | Asset info/thumbnail/original endpoints accept `userId` query param, allowing any authenticated user to proxy requests through another user's Immich API key. This exposes other users' Immich credentials and photo libraries. | Restrict all Immich proxy endpoints to the authenticated user's own credentials only. | FIXED |
| I-3 | **MEDIUM** | `server/src/routes/immich.ts` | 199, 250, 274 | `assetId` URL parameter used directly in `fetch()` URL construction. Path traversal characters could redirect requests to unintended Immich API endpoints. | Validate assetId matches `[a-zA-Z0-9_-]+` pattern. | FIXED |
| AD-1 | **MEDIUM** | `server/src/routes/admin.ts` | 302-310 | Self-update endpoint runs `git pull` then `npm run build`. While admin-only and `npm install` uses `--ignore-scripts`, `npm run build` executes whatever is in the pulled package.json. A compromised upstream could execute arbitrary code. | Document as accepted risk for self-hosted self-update feature. Users should pin to specific versions. | DOCUMENTED |
| X-1 | **CRITICAL** | `client/src/components/Admin/GitHubPanel.tsx` | 66, 106 | `dangerouslySetInnerHTML` with `inlineFormat()` renders GitHub release notes as HTML without escaping. Malicious HTML in release notes could execute scripts. | Escape HTML entities before applying markdown-style formatting. Validate link URLs. | FIXED |
| X-2 | **LOW** | `client/src/components/Map/MapView.tsx` | divIcon | Uses `escAttr()` for HTML sanitization in divIcon strings. Properly mitigated. | OK | OK |
- **MEDIUM** — `server/src/index.ts:121-136`: Upload files (`/uploads/:type/:filename`) served without authentication. UUIDs are unguessable but this is security-through-obscurity. **REQUIRES MANUAL REVIEW** — adding auth would break shared trip image URLs.
- **MEDIUM** — `server/src/routes/oidc.ts:194`: OIDC token exchange error was logging full token response (potentially including access tokens). **FIXED** — now logs only HTTP status.
- **MEDIUM** — `server/src/services/notifications.ts:194-196`: Email body is not HTML-escaped. User-generated content (trip names, usernames) interpolated directly into HTML email template. Potential stored XSS in email clients. **DOCUMENTED** — needs HTML entity escaping.
- **LOW** — `server/src/demo/demo-seed.ts:7-9`: Hardcoded demo credentials (`demo12345`, `admin12345`). Intentional for demo mode but dangerous if DEMO_MODE accidentally left on in production. Already has startup warning.
- **LOW** — `server/src/routes/auth.ts:742`: MFA setup returns plaintext TOTP secret to client. This is standard TOTP enrollment flow — users need the secret for manual entry. Must be served over HTTPS.
- **LOW** — `server/src/routes/auth.ts:473`: Admin settings GET returns API keys in full (not masked). Only accessible to admins.
- **LOW** — `server/src/routes/auth.ts:564`: SMTP password stored as plaintext in `app_settings` table. Masked in API response but unencrypted at rest.
