From 4bc7ed5d2318e14ca52ef0d532bacce82bbbccda Mon Sep 17 00:00:00 2001 From: Stefan Date: Fri, 12 Jun 2026 13:35:03 +0200 Subject: [PATCH] docs: add full repository review (2026-06-12) Cross-cutting review across security/auth, LLM & audio pipeline, architecture & data model, code quality, tests & CI, operations & scaling, and product/license/compliance. Findings carry stable IDs, severities, effort estimates, file references, and concrete recommendations, plus a prioritized roadmap. Co-Authored-By: Claude Fable 5 --- docs/REVIEW-2026-06-12.md | 986 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 986 insertions(+) create mode 100644 docs/REVIEW-2026-06-12.md diff --git a/docs/REVIEW-2026-06-12.md b/docs/REVIEW-2026-06-12.md new file mode 100644 index 0000000..c4852d6 --- /dev/null +++ b/docs/REVIEW-2026-06-12.md @@ -0,0 +1,986 @@ +# OpenSquawk — Full Repository Review + +> Date: 2026-06-12 · Method: 7 parallel category reviews + adversarial verification of every critical/high finding (37 AI agents total, ~1.8M tokens). Severities shown are post-verification: severities adjusted by the verifier are marked inline. + +## Executive summary + +**Overall:** The codebase is in better shape than most solo-built products at this stage — clean TypeScript across app and server, sound core auth (scrypt, timing-safe JWT checks, httpOnly refresh cookies, server-side `requireAdmin` on every admin/editor route), a genuinely well-engineered client audio pipeline (speech queue with generation-counter cancellation, TTS prefetch, sample-accurate ATIS loop), and a real 80-test suite. The problems are concentrated in five themes, not spread randomly: + +1. **Unmetered money endpoints (the one critical).** `/api/atc/say` runs OpenAI TTS with auth deliberately commented out and is exempted in the auth middleware; `/api/service/tools/latency` runs a paid LLM completion per anonymous GET; nothing in the repo has rate limiting. Anyone who finds the URL can run up the OpenAI bill. Same root cause behind **LLM-01, QUAL-08, OPS-03, LLM-02** — one fix (re-enable auth, add a length cap and a simple per-user rate limit) closes all four. + +2. **Telemetry trust model (SEC-01, SEC-02).** Bridge tokens are client-chosen, 6+ chars, no expiry; FlightLab's WebSocket trusts a client-supplied `userId` and a client-asserted `instructor` role. Any user can read or spoof another user's live telemetry. Needs server-minted high-entropy tokens and session-derived identity. + +3. **The repo lies about its own architecture.** The LLM decision routing (`routeDecision()`, `/api/llm/decide`, `/api/decision-flows/runtime`) was moved to an external Python backend, but CLAUDE.md/AGENTS.md/README still describe the old flow, dead client/server code for it still ships, and the Python backend is unauthenticated and unpublished (which also undermines the open-source/self-host claim). For a project that wants AI agents as co-developers, stale CLAUDE.md is actively harmful — every agent session starts with a wrong mental model. Covers **LLM-08, ARCH-01/02, QUAL-05/07, TEST-06, OPS-10, COMP-05, LLM-05**. + +4. **Nothing enforces green.** No CI, no lint config, and the test suite is currently red on main (one stale assertion). The core state machine (1,568 lines) and all auth endpoints have zero tests. Fixing TEST-01 + adding the minimal pipeline in TEST-02 is half a day and changes the project's trajectory more than any single bug fix. Covers **QUAL-02, TEST-01..04**. + +5. **Single-process runtime + GDPR drift.** All bridge/FlightLab state lives in module-level Maps (gone on every deploy, broken at 2 instances), and the privacy notice no longer matches reality (voice audio to OpenAI Whisper undisclosed, Hotjar undisclosed, user deletion leaves data in five collections, no double opt-in). Covers **OPS-01/08, COMP-01/02/03**. + +**Suggested first week** (small efforts, big risk reduction): QUAL-08/LLM-01 (re-enable TTS auth), TEST-01 (fix red test), TEST-02 + QUAL-02 (minimal CI: typecheck + tests on push), SEC-07 (rotate/remove committed secrets), OPS-02/SEC-09 (mandatory cron secret), COMP-01 (privacy notice update — legal exposure, S effort). Then take the two M-effort security items SEC-01 and SEC-02 before promoting the product to a wider audience. + +**Cross-references:** LLM-01 = QUAL-08 = OPS-03 (one fix), OPS-02 = SEC-09, LLM-09 = ARCH-10 = OPS-07 (phantom stored URLs), LLM-11 = OPS-11 = QUAL-05 (cruft). The doc-drift theme spans LLM-08/ARCH-02/QUAL-05/QUAL-07/TEST-06/OPS-10. + +## How to use this document + +Each finding has a stable ID (e.g. `SEC-01`), severity, effort estimate (S = <1h, M = a few hours, L = a day+), file references, and a concrete recommendation. Work top-down through the priority roadmap; tick checkboxes as items land. The finding descriptions are written to be self-contained so an AI agent can be pointed at a single finding ID and implement it. + +Severity counts (confirmed findings): **1 critical · 17 high · 48 medium · 21 low**. No finding was fully refuted during verification, but 9 were downgraded in severity; verifier corrections are quoted inline where the original description overstated details. + +## Priority roadmap + +### P0 — Critical (fix before inviting more users) + +- [ ] **LLM-01** (M) Unauthenticated, unmetered TTS endpoint /api/atc/say — direct OpenAI cost abuse vector — *LLM & Audio Pipeline* + +### P1 — High + +- [ ] **ARCH-07** (L) Flow editor can corrupt live trees: node deletion leaves dangling transition targets, startState is unvalidated, multi-doc edits are non-transactional, no flow versioning — *Architecture & Data Model* +- [ ] **ARCH-08** (M) TransmissionLog grows unbounded and lacks the indexes its admin queries require; session list is a full-collection aggregation — *Architecture & Data Model* +- [ ] **COMP-01** (S) Privacy notice misstates OpenAI data flow and omits Hotjar, cookies, and product-usage tracking — *Product, License & Compliance* +- [ ] **COMP-02** (M) No double opt-in: drip cron emails unverified addresses and marketing consent is recorded without confirmation — *Product, License & Compliance* +- [ ] **COMP-03** (S) User deletion leaves personal data in five collections (incomplete GDPR Art. 17 erasure) — *Product, License & Compliance* +- [ ] **LLM-03** (M) No timeouts on any OpenAI call — a hung TTS/STT request stalls the speech queue for up to 10 minutes — *LLM & Audio Pipeline* +- [ ] **OPS-02** (M) Unauthenticated, non-idempotent cron endpoint /api/service/cron/waitlist-drip sends real emails — *Operations & Scaling* +- [ ] **OPS-03** (M) /api/atc/say is unauthenticated and there is no rate limiting anywhere — unbounded paid TTS/LLM cost exposure — *Operations & Scaling* +- [ ] **QUAL-02** (M) No ESLint/Prettier configuration and no CI pipeline — *Code Quality & Maintainability* +- [ ] **QUAL-03** (L) Monolithic page components: classroom.vue 8,614 lines, pm.vue 5,046 lines, editor 3,032 lines — *Code Quality & Maintainability* +- [ ] **QUAL-08** (S) Commented-out authentication on the OpenAI-billed TTS endpoint — *Code Quality & Maintainability* +- [ ] **SEC-01** (M) Bridge telemetry endpoints authorize on client-chosen, low-entropy tokens with no expiry/revocation (cross-user telemetry read/write IDOR) — *Security & Auth* +- [ ] **SEC-02** (M) FlightLab WebSocket is unauthenticated and trusts a client-supplied userId for telemetry subscription (cross-user live telemetry IDOR) — *Security & Auth* +- [ ] **TEST-01** (S) Test suite is red on main: known-failing radioSpeech SID test committed without updating the assertion — *Tests & CI* +- [ ] **TEST-02** (S) No CI pipeline of any kind — no automated test, typecheck, or lint gate — *Tests & CI* +- [ ] **TEST-03** (M) communicationsEngine.ts (1,568 lines) — the core live-ATC state machine — has zero tests — *Tests & CI* +- [ ] **TEST-04** (M) Auth endpoints and refresh-token rotation untested (login, refresh, register, reset-password, forgot-password) — *Tests & CI* + +### P2 — Medium + +- [ ] **ARCH-01** (M) Dual state machine (local engine mirror + Python backend) with no divergence detection; project docs describe an architecture that no longer exists — *Architecture & Data Model* +- [ ] **ARCH-02** (M) Dead code: Nuxt runtime-tree builders, LLM decision client, and a no-op 'import' service that still ships an admin endpoint — *Architecture & Data Model* +- [ ] **ARCH-03** (M) timer_next transitions are modeled, stored, and serialized but never fire — flows using them dead-end — *Architecture & Data Model* +- [ ] **ARCH-04** (S) Delayed auto-transitions fire without re-checking current state, and pending timers survive engine resets — sessions can teleport — *Architecture & Data Model* +- [ ] **ARCH-05** (S) No re-entrancy guard for overlapping pilot transmissions in /pm — *Architecture & Data Model* +- [ ] **ARCH-06** (L) No session recovery: page reload loses the live ATC session and leaks the backend session — *Architecture & Data Model* +- [ ] **ARCH-09** (L) Server-process in-memory state (bridge logs, telemetry store, WS sessions, dome-light cache) breaks on restart and multi-instance deployment; FlightLab uses one GLOBAL session for all users — *Architecture & Data Model* +- [ ] **ARCH-10** (S) /api/atc/say returns stored.url and file paths that are never written; the referenced audio route does not exist — *Architecture & Data Model* +- [ ] **ARCH-11** (S) Duplicate SimBrief proxy routes with divergent contracts, and a helper module registered as a phantom API route — *Architecture & Data Model* +- [ ] **ARCH-12** (M) Inconsistent API design: soft 200-errors vs createError, mixed German/English messages, side-effecting GET cron routes, prefix-based auth exemptions — *Architecture & Data Model* +- [ ] **ARCH-13** (L) pm.vue (5,046 lines) and classroom.vue (8,614 lines) are monoliths duplicating PTT recording, TTS playback, and speech-queue plumbing — *Architecture & Data Model* +- [ ] **ARCH-14** (M) Core state machine has zero unit tests — *Architecture & Data Model* +- [ ] **COMP-04** (M) No retention enforcement: transcripts, analytics and generated audio/metadata are stored indefinitely — *Product, License & Compliance* +- [ ] **COMP-05** (S) README and CLAUDE.md contradict the shipped product; Live ATC depends on an unpublished Python backend, undermining the open-source claim — *Product, License & Compliance* +- [ ] **COMP-06** (S) Flight telemetry forwarded by default to a hardcoded private third-party webhook — *Product, License & Compliance* +- [ ] **COMP-07** (M) Unauthenticated VATSIM proxies expose third-party personal data and are undisclosed — *Product, License & Compliance* +- [ ] **LLM-02** (S) Unauthenticated GET /api/service/tools/latency triggers a paid LLM completion per request — *LLM & Audio Pipeline* +- [ ] **LLM-04** (S) PTT race: releasing the button before async mic setup completes leaves recording stuck, then auto-submits 30 s of unintended audio to Whisper and the live session — *LLM & Audio Pipeline* +- [ ] **LLM-05** (L) Browser talks to the Python radio backend directly with zero authentication — *LLM & Audio Pipeline* +- [ ] **LLM-06** (S) preNormalized flag is sent by the client but silently ignored by /api/atc/say — text is normalized twice — *LLM & Audio Pipeline* +- [ ] **LLM-07** (S) OpenAI TTS audio is MP3 but labeled audio/wav; the requested 'format' parameter is ignored for openai/piper providers — *LLM & Audio Pipeline* +- [ ] **LLM-08** (M) Dead legacy LLM decision layer and stale core docs — /api/llm/decide, routeDecision(), /api/decision-flows/runtime do not exist — *LLM & Audio Pipeline* +- [ ] **OPS-01** (L) All bridge/FlightLab runtime state is in-process memory — breaks with 2+ instances and is wiped on every deploy — *Operations & Scaling* +- [ ] **OPS-04** (S) ffmpeg is required at runtime but not installed by the nixpacks build — *Operations & Scaling* +- [ ] **OPS-05** (M) Missing/invalid environment variables fail silently or late instead of at startup — *Operations & Scaling* +- [ ] **OPS-06** (M) No structured logging, request IDs, or error reporting; high-frequency telemetry spams stdout with console.table — *Operations & Scaling* +- [ ] **OPS-07** (M) say.post.ts returns broken stored-audio URLs and never writes the audio/meta files it reports; TTS disk cache grows unbounded — *Operations & Scaling* +- [ ] **OPS-08** (L) No graceful shutdown handling — deploys drop WebSocket sessions and in-flight requests with no recovery path — *Operations & Scaling* +- [ ] **OPS-09** (S) VATSIM proxy endpoints fetch upstream on every request with no caching — *Operations & Scaling* +- [ ] **QUAL-01** (S) Test suite fails at HEAD (stale assertion after SID-pronunciation change) — *Code Quality & Maintainability* +- [ ] **QUAL-04** (M) Radio-audio playback pipeline duplicated in three pages — *Code Quality & Maintainability* +- [ ] **QUAL-05** (S) Dead code: openaiDecision.ts calls a non-existent endpoint, unused prompt/TTS exports in normalize.ts, out.ogg artifact — *Code Quality & Maintainability* +- [ ] **QUAL-06** (S) Helper module without default export lives inside server/api and is registered as a route — *Code Quality & Maintainability* +- [ ] **QUAL-07** (S) CLAUDE.md describes an architecture that no longer exists — *Code Quality & Maintainability* +- [ ] **QUAL-09** (M) Inconsistent error handling: 200-with-error-object routes, mixed German/English messages, divergent error shapes — *Code Quality & Maintainability* +- [ ] **QUAL-10** (L) TypeScript strict mode disabled across app and server — *Code Quality & Maintainability* +- [ ] **QUAL-11** (M) Duplicated route logic: two SimBrief fetchers, repeated editor-flow boilerplate, two OpenAI client singletons — *Code Quality & Maintainability* +- [ ] **QUAL-12** (S) Config hygiene: .env.example incomplete, real-looking secrets/defaults committed — *Code Quality & Maintainability* +- [ ] **SEC-03** (M) No rate limiting on authentication and invitation endpoints (credential brute force / email bombing) — *Security & Auth* +- [ ] **SEC-04** (S) Registration leaks account existence (email enumeration) — *Security & Auth* +- [ ] **SEC-05** (S) reset-password enforces a weaker password policy than register — *Security & Auth* +- [ ] **SEC-06** (S) Invitation codes have low entropy (32 bits) enabling brute-force registration — *Security & Auth* +- [ ] **SEC-07** (S) Real-looking secret defaults committed in .env.example (manual invite password, JWT default) — *Security & Auth* +- [ ] **SEC-09** (S) Public waitlist-drip cron endpoint can be abused to send emails and burn invite codes — *Security & Auth* +- [ ] **TEST-05** (M) decisionFlowService runtime-tree building untested, and its mongoose-document coupling blocks easy testing — *Tests & CI* +- [ ] **TEST-06** (S) CLAUDE.md/AGENTS.md describe an LLM routing path (routeDecision, /api/llm/decide) that does not exist; dead client code would 404 — *Tests & CI* +- [ ] **TEST-07** (M) Brittle test seams: globalThis.defineEventHandler hack and ad-hoc mongoose monkey-patching, no shared test helpers or DB harness — *Tests & CI* +- [ ] **TEST-09** (L) Critical frontend logic locked inside 5,000-14,000-line page components with no component test infrastructure — *Tests & CI* + +### P3 — Low + +- [ ] **ARCH-15** (M) Engine semantics ride on naming conventions and hardcoded defaults (INT_ prefix, handoff regexes, fake frequencies/SIDs) — *Architecture & Data Model* +- [ ] **COMP-08** (S) Impressum and privacy notice cite superseded German statutes and the wrong supervisory authority — *Product, License & Compliance* +- [ ] **COMP-09** (S) AGPL hygiene gaps: no per-file headers, no in-app source offer, no contributor licensing policy — *Product, License & Compliance* +- [ ] **COMP-10** (M) No self-service data export or account deletion for users — *Product, License & Compliance* +- [ ] **LLM-09** (S) /api/atc/say returns phantom storage metadata — files never written, audio URL route does not exist — *LLM & Audio Pipeline* +- [ ] **LLM-10** (S) TTS disk cache grows without bound — no eviction or size cap — *LLM & Audio Pipeline* +- [ ] **LLM-11** (S) Stray out.ogg committed to repo root; fluent-ffmpeg dependency declared but never used — *LLM & Audio Pipeline* +- [ ] **LLM-12** (S) playPTTBeep leaks an AudioContext per beep — *LLM & Audio Pipeline* +- [ ] **LLM-13** (S) Page unmount does not stop in-flight ATC speech or tear down the speech queue — *LLM & Audio Pipeline* +- [ ] **LLM-14** (M) Pilot readback doubles TTS spend per transmission and is never cached — *LLM & Audio Pipeline* +- [ ] **LLM-15** (L) TTS responses are fully buffered base64 JSON — no streaming, inflated payloads, slower time-to-first-audio — *LLM & Audio Pipeline* +- [ ] **OPS-10** (S) Documentation/code drift: client calls /api/llm/decide which has no server route; CLAUDE.md describes the removed flow — *Operations & Scaling* +- [ ] **OPS-11** (S) Repo/build hygiene: stray binary artifact, hardcoded Hotjar ID with debug outside prod, unused native-dep scaffolding — *Operations & Scaling* +- [ ] **QUAL-13** (S) Stale TODO encoding feature requirements in German inside route source — *Code Quality & Maintainability* +- [ ] **QUAL-14** (S) Frontend auth-fetch wrapper bypassed in bridge/connect.vue and flightlab/takeoff.vue — *Code Quality & Maintainability* +- [ ] **SEC-08** (S) Personal webhook URL hardcoded as fallback in source and committed to .env.example — *Security & Auth* +- [ ] **SEC-10** (M) Access token stored in localStorage (XSS token theft) — *Security & Auth* +- [ ] **SEC-11** (M) Refresh tokens are stateless and not rotated or revocable individually — *Security & Auth* +- [ ] **SEC-12** (S) Public unsubscribe endpoint deletes arbitrary waitlist/subscriber records by email without verification — *Security & Auth* +- [ ] **TEST-08** (S) Pure validation logic inside /api/atc/ptt.post.ts is untested and not exported — *Tests & CI* +- [ ] **TEST-10** (S) No coverage measurement or test documentation; AGENTS.md/CLAUDE.md never mention how to run tests — *Tests & CI* + +--- + +## Security & Auth + +OpenSquawk's core JWT auth, password hashing (scrypt + timing-safe compare), and admin/editor authorization (every admin/* and editor/* route calls requireAdmin server-side) are implemented soundly. However, the bridge and FlightLab telemetry subsystems have a serious authorization model flaw: bridge tokens are client-chosen, low-entropy (min 6 chars), never expire/rotate, and every bridge read/write endpoint plus the FlightLab WebSocket authorize purely on a caller-supplied token or userId, enabling cross-user telemetry read/write (IDOR). There is also no rate limiting anywhere on authentication or invitation endpoints, register leaks account existence, reset-password enforces a weaker policy than register, and a real-looking manual-invite password plus a personal webhook URL are committed to the repo. A public cron endpoint (waitlist-drip) can be abused to send emails and burn invite codes. + +### What's done well + +- Admin and editor authorization is enforced server-side on every route: all 12 admin/* handlers and all 11 editor/* handlers call requireAdmin(event) (verified via grep), and requireAdmin bumps to a 403 when role is not admin/dev (server/utils/auth.ts:177-183). +- Password hashing uses scrypt with a per-user random 16-byte salt and constant-time comparison via timingSafeEqual (server/utils/auth.ts:70-84), and JWT signature verification also uses timingSafeEqual with an explicit alg check rejecting non-HS256 (server/utils/auth.ts:51-67). +- Refresh tokens are stored in an httpOnly, sameSite=lax cookie with secure flag in production (server/utils/auth.ts:112-120), not exposed to JS. +- Password reset tokens are stored only as sha256 hashes, are single-use (usedAt), expire after 60 minutes with a Mongo TTL index, and bump tokenVersion to invalidate existing sessions on reset (server/api/service/auth/reset-password.post.ts, server/models/PasswordResetToken.ts). +- forgot-password returns {success:true} unconditionally and only branches internally, avoiding account enumeration (server/api/service/auth/forgot-password.post.ts:21-54). +- The /api/atc/ptt audio pipeline uses execFile (not shell) with fixed ffmpeg args, validates base64 with a strict regex, whitelists the format, and caps payload at 2MB, so there is no command injection or path traversal (server/api/atc/ptt.post.ts:33-77). +- Server secrets in nuxt.config.ts runtimeConfig are kept under the top-level (server-only) config; only apiDocumentationUrl and radioBackendUrl are placed under runtimeConfig.public. + +### Findings + +#### SEC-01 · HIGH · effort M ✓ verified — Bridge telemetry endpoints authorize on client-chosen, low-entropy tokens with no expiry/revocation (cross-user telemetry read/write IDOR) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\bridge\data.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\bridge\live.get.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\bridge\me.get.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\bridge\status.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\utils\bridge.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\models\BridgeToken.ts`, `C:\Users\Stefan\Repo\OpenSquawk\app\pages\bridge\connect.vue` + +The bridge token is supplied by the client, never generated by the server. normalizeBridgeToken only checks `token.length < 6 || token.length > 256` (server/utils/bridge.ts:11) and connect.vue derives it from `route.query.token` with `hasToken = token.value.length >= 6` (connect.vue:445-455). The auth.global middleware excludes `/api/bridge/` entirely (auth.global.ts:15-17), so the only credential is the x-bridge-token header. /api/bridge/connect requires a user session and upserts {token -> user}, but data.post.ts, live.get.ts, me.get.ts and status.post.ts authorize purely by looking the token up: `BridgeToken.findOne({token}).select('user')` then `flightlabTelemetryStore.update(userId, mapped)` / `.get(userId)` (data.post.ts:139-157, live.get.ts:12-23). Consequences: (1) anyone who learns or guesses another user's 6-char token can POST /api/bridge/data to inject fake flight telemetry into that user's session, and GET /api/bridge/live or /me to read their live position/telemetry; (2) tokens have no expiry, no revocation, and no minimum entropy, so a 6-character token is brute-forceable; (3) status.post.ts and data.post.ts upsert on the token, letting an attacker pre-create arbitrary token documents. + +**Recommendation:** Generate bridge tokens server-side with high entropy (e.g. randomBytes(32)), store only a hash, and require an authenticated session at /api/bridge/connect to mint and bind a token to the user. On data/live/me/status, resolve the user from the bound token and additionally verify the requesting principal where applicable; add an expiry (TTL) and a revocation/disconnect path. Reject tokens that are not server-issued. Increase the minimum length check well beyond 6. + +**Verifier note:** data.post.ts does not upsert (it uses BridgeToken.findOne and 401s when the token is not bound to a user); only status.post.ts upserts, and that document has no user so it does not grant telemetry access. Also, a session-gated, ownership-checked revocation path already exists (disconnect.post.ts) — the real gap is no automatic TTL/expiry, not the lack of any revocation. + +#### SEC-02 · HIGH · effort M ✓ verified — FlightLab WebSocket is unauthenticated and trusts a client-supplied userId for telemetry subscription (cross-user live telemetry IDOR) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\flightlab\ws.ts` + +The WebSocket handler performs no authentication on open and never verifies the caller's identity. The `subscribe-telemetry` message takes `data.userId` directly from the client and routes that user's live telemetry to the requesting socket: `session.telemetryUserIds.add(data.userId); userSessionMap.set(data.userId, session.code)` (ws.ts:175-184), and the store subscription then broadcasts telemetry for that userId to all peers in the session (ws.ts:26-32). Any client can connect, create/join the shared GLOBAL session, and subscribe with an arbitrary userId to receive another user's live flight telemetry. Additionally, role is client-asserted: `join-session` sets `const role = data.role ?? 'participant'` (ws.ts:116-117), so a client can join as 'instructor' and then issue privileged `instructor-command` actions (pause/resume/restart/goto) — the role check at ws.ts:140 trusts that self-assigned role. + +**Recommendation:** Authenticate the WebSocket on upgrade (validate the access token) and derive the userId server-side from the authenticated session instead of accepting data.userId. Only allow a socket to subscribe to its own telemetry. Assign the instructor/participant role from server-side authorization, not from the client-provided data.role. Scope sessions per user/group rather than a single shared GLOBAL session. + +#### SEC-03 · MEDIUM (reported high, adjusted to medium after verification) · effort M ✓ verified — No rate limiting on authentication and invitation endpoints (credential brute force / email bombing) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\login.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\register.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\forgot-password.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\reset-password.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\invitations\manual.post.ts` + +None of the authentication or invitation endpoints apply any rate limiting, lockout, or CAPTCHA. login.post.ts validates email+password and returns 401 on failure with no attempt counter (login.post.ts:19-27). manual.post.ts gates invite creation behind a single shared password compared via HMAC, but with unlimited attempts (manual.post.ts:37-39), so the manual invite password is brute-forceable. forgot-password and reset-password are likewise unthrottled. There is no global rate-limit middleware (the only middlewares are auth.global.ts and bridge-cors.ts). + +**Recommendation:** Add per-IP and per-account rate limiting / exponential backoff on login, register, forgot-password, reset-password, and the manual/bootstrap invite endpoints (e.g. a shared limiter middleware or a store-backed counter). Consider account lockout after repeated failures and CAPTCHA on register/forgot-password. + +**Verifier note:** Severity is better rated medium than high. Mitigating context the finding omits: (1) reset-password tokens are 32 random bytes hashed with SHA-256, so unthrottled token guessing is computationally infeasible — that endpoint is not practically brute-forceable; (2) register requires a valid unused invitation code, bounding abuse; (3) login uses a uniform 'Invalid credentials' message (no user enumeration) and passwords have enforced strength at registration; (4) the manual invite password comparison uses HMAC + timingSafeEqual (no timing side channel), and a successful brute force only yields invitation codes for an invite-only app, not account takeover. The real exposures are unlimited login credential stuffing, forgot-password email bombing plus reset-link invalidation DoS (each request runs PasswordResetToken.deleteMany for the target user), and offline-speed online guessing of the shared manual-invite password — a classic CWE-307 missing-anti-automation finding, typically medium without an amplifier such as a weak/default password or admin-takeover path. + +#### SEC-04 · MEDIUM · effort S — Registration leaks account existence (email enumeration) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\register.post.ts` + +register.post.ts returns a distinct 409 'An account already exists for this email address' when the email is already registered (register.post.ts:43-46), before checking the invitation code. Combined with the lack of rate limiting, an attacker can enumerate which emails have accounts. The invitation-code path also returns granular errors (404 not found, 400 already used, 400 expired) at lines 48-57, which leaks invite-code state. + +**Recommendation:** Return a generic error that does not distinguish 'email already registered' from other validation failures (or require the invitation flow to complete before revealing account state), and avoid distinguishing invite-code states in responses. Pair with rate limiting. + +#### SEC-05 · MEDIUM · effort S — reset-password enforces a weaker password policy than register + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\reset-password.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\utils\validation.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\register.post.ts` + +register.post.ts calls validatePasswordStrength which requires >=10 chars, no whitespace, letters+digits, and a special character (validation.ts:12-26). reset-password.post.ts only checks `if (password.length < 8)` (reset-password.post.ts:21-23), allowing an 8-character letters-only password to be set via the reset flow — bypassing the registration policy. + +**Recommendation:** Call validatePasswordStrength(password) in reset-password.post.ts (and any other password-setting path) so the policy is consistent everywhere. + +#### SEC-06 · MEDIUM · effort S — Invitation codes have low entropy (32 bits) enabling brute-force registration + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\utils\invitations.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\register.post.ts` + +generateInvitationCode() returns `randomBytes(4).toString('hex').toUpperCase()` — only 4 bytes / 8 hex chars / 32 bits of entropy (invitations.ts:15-17). register.post.ts looks codes up directly with `InvitationCode.findOne({code})` (register.post.ts:48) and there is no rate limiting on register, so valid unused codes can be brute-forced to gain registration access to the invite-gated app. + +**Recommendation:** Increase invitation code entropy (e.g. randomBytes(16) or a longer base32 string) and rate-limit registration attempts. Optionally bind codes to a specific email. + +#### SEC-07 · MEDIUM · effort S — Real-looking secret defaults committed in .env.example (manual invite password, JWT default) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\.env.example` + +.env.example commits `MANUAL_INVITE_PASSWORD=pm.local@zghl.de` — a concrete, real-looking credential rather than a placeholder, which gates the manual invite-code generator (consumed at manual.post.ts via config.manualInvitePassword). It also ships `JWT_SECRET=changeme` / `JWT_REFRESH_SECRET=changeme`; since getSecrets() only checks that jwtSecret is truthy (auth.ts:18-19), a deployment that copies the example verbatim runs with a publicly known signing secret, allowing forged access/refresh JWTs. + +**Recommendation:** Replace committed values with obvious non-functional placeholders (e.g. CHANGE_ME) and rotate the manual invite password if it was ever used in a real environment. Add a startup assertion that JWT_SECRET is not the example value and meets a minimum length. + +#### SEC-09 · MEDIUM · effort S — Public waitlist-drip cron endpoint can be abused to send emails and burn invite codes + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\cron\waitlist-drip.get.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\middleware\auth.global.ts` + +This is a NEW aspect beyond the previously-documented cron exposure: unlike weekly-kpi-report.get.ts (which at least supports an optional KPI_CRON_SECRET), waitlist-drip.get.ts has NO secret/auth check at all and is reachable by anyone because auth.global.ts excludes all of /api/service/ (auth.global.ts:12-14). A GET to /api/service/cron/waitlist-drip sends invitation and feedback emails to waitlist entries and creates InvitationCode documents (waitlist-drip.get.ts:138-146, 124-133, 172-177). Repeated unauthenticated calls let an attacker trigger outbound email sends (deliverability/abuse, spam to waitlisted addresses) and generate invite codes. Note also that weekly-kpi-report.get.ts accepts the secret via query string (`query.secret`, line 11), which tends to be logged. + +**Recommendation:** Require a strong shared secret (header, not query string) on all /api/service/cron/* endpoints and reject when unset, or move cron triggers off the public surface. Make the secret mandatory rather than optional. + +#### SEC-08 · LOW · effort S — Personal webhook URL hardcoded as fallback in source and committed to .env.example + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\bridge\data.post.ts`, `C:\Users\Stefan\Repo\OpenSquawk\.env.example`, `C:\Users\Stefan\Repo\OpenSquawk\nuxt.config.ts` + +A personal/home-automation webhook URL is embedded directly in source as a fallback: `const DOME_LIGHT_WEBHOOK_FALLBACK_URL = 'https://home.io.faktorxmensch.com/api/webhook/lidl_stab_3modi_8492'` (data.post.ts:10), used when no env var is set (data.post.ts:152), and the same URL is duplicated as a default in nuxt.config.ts (domeLightWebhookUrl) and committed in .env.example. Bridge telemetry from any token holder is forwarded to this third-party endpoint, leaking telemetry and pinging the owner's infrastructure. + +**Recommendation:** Remove the hardcoded fallback URL; require the webhook URL to be explicitly configured and no-op when unset. Drop the personal URL from .env.example and nuxt.config.ts defaults. + +#### SEC-10 · LOW · effort M — Access token stored in localStorage (XSS token theft) + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\app\stores\auth.ts`, `C:\Users\Stefan\Repo\OpenSquawk\nuxt.config.ts` + +The JWT access token is persisted in localStorage under 'os_access_token' (auth.ts:3,35,41) and sent as a Bearer header. localStorage is readable by any JavaScript running on the origin, so a single XSS (or a compromised third-party script — the app loads Hotjar via nuxt-module-hotjar, nuxt.config.ts) can exfiltrate the 24h access token. The refresh token is correctly httpOnly, but the long-lived (ACCESS_TOKEN_TTL_SECONDS = 24h, auth.ts:10) access token in localStorage is a meaningful exposure. + +**Recommendation:** Prefer storing the access token in memory only (re-acquired via the httpOnly refresh cookie on load) rather than localStorage, and/or shorten the access token TTL. Apply a strict Content-Security-Policy to reduce XSS/third-party script risk. + +#### SEC-11 · LOW · effort M — Refresh tokens are stateless and not rotated or revocable individually + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\utils\auth.ts`, `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\auth\refresh.post.ts` + +rotateRefreshToken() verifies the refresh JWT and issues a brand-new access+refresh pair, but the previous refresh token is not invalidated — it remains valid until its 7-day exp because there is no server-side store of issued refresh tokens (auth.ts:192-217). Despite the name, no real rotation/blacklisting occurs; the only kill-switch is the global tokenVersion bump (on logout/password-reset/role-change), which revokes ALL of a user's tokens at once. A stolen refresh token therefore remains usable for up to 7 days unless the user logs out everywhere. + +**Recommendation:** Persist refresh tokens (or a jti) server-side and invalidate the old one on each refresh (true rotation with reuse detection), or shorten the refresh TTL. This also enables per-session revocation. + +#### SEC-12 · LOW · effort S — Public unsubscribe endpoint deletes arbitrary waitlist/subscriber records by email without verification + +**Files:** `C:\Users\Stefan\Repo\OpenSquawk\server\api\service\updates.delete.ts` + +updates.delete.ts is public (under the auth-excluded /api/service/ prefix) and deletes records purely by the email in the request body: `UpdateSubscriber.deleteOne({email})` and `WaitlistEntry.deleteOne({email})` (updates.delete.ts:17-20) with no signed token or ownership check. Anyone can remove any address from the waitlist/update list (data tampering / denial of a user's waitlist position). + +**Recommendation:** Require a signed, single-use unsubscribe token tied to the email (as already generated for some flows) rather than accepting a raw email, or rate-limit and log this endpoint. + +--- + +## LLM & Audio Pipeline + +The product core has migrated: LLM decision routing no longer lives in this repo (no routeDecision(), no /api/llm/decide) — pilot transcripts go from the browser straight to an external Python backend, while this repo owns STT (/api/atc/ptt via Whisper), TTS (/api/atc/say via OpenAI/Speaches/Piper), and client-side audio. The client audio architecture (speech queue with generation-counter cancellation, parallel TTS prefetch, ATIS virtual-clock loop with carrier noise) is genuinely well engineered. The biggest problems are cost control — /api/atc/say and /api/service/tools/latency are unauthenticated, unmetered OpenAI calls — plus a layer of dead legacy LLM code and stale docs that misdescribe the core flow, several contract bugs (ignored preNormalized flag, mislabeled mp3-as-wav audio, phantom stored URLs), and missing timeouts around every OpenAI call. + +### What's done well + +- Client speech pipeline interruption handling is excellent: generation counter + AbortController set cancels queued tasks, in-flight TTS fetches, and playing audio atomically on frequency change (app/pages/pm.vue:2326-2366, 2526-2560; stopCurrentSpeech called at 3476, 3818, 3912) +- Latency-conscious design: TTS generation is kicked off immediately and overlaps the artificial controller delay and any queued speech, so audible latency is max(generation, delay) rather than the sum (app/pages/pm.vue:2582-2597) +- ATIS pipeline is strong: server-side content-hashed TTS cache for atis/flightlab tags (server/api/atc/say.post.ts:44-56, 242-249), client request dedup + prefetch (app/pages/pm.vue:3600-3642), and a sample-accurate virtual-clock loop with carrier-noise bed so tuning back resumes mid-broadcast (shared/utils/atisAudioLoop.ts) +- PTT input hardening on the server: base64 regex validation, 2 MB payload cap, format whitelist, and temp-file cleanup on both success and error paths (server/api/atc/ptt.post.ts:41-67, 122-125, 152-155) +- PTT client hardening: 30 s auto-stop safety timer and chunked base64 encoding that avoids the call-stack overflow documented in the comment (app/pages/pm.vue:3152-3161, 3264-3284) +- ffmpeg is invoked via execFile with array arguments everywhere — no shell string interpolation, so no command injection (server/api/atc/ptt.post.ts:33-39, 69-77; server/utils/radio.ts:4-7) +- Classroom mode caches TTS audio client-side keyed by voice/level/rate/text and dedupes concurrent requests (app/pages/classroom.vue:4621-4658) +- Speech-server health check uses an AbortController with a 2.5 s timeout and multiple fallback URLs (server/api/classroom/speech-server-health.get.ts:19-34) +- STT matching logic is extracted to shared/utils/sttMatch with unit tests (tests/shared/sttMatch.test.ts), and the radiotelephony normalizer is tested (tests/shared/radioSpeech.test.ts) + +### Findings + +#### LLM-01 · CRITICAL · effort M ✓ verified — Unauthenticated, unmetered TTS endpoint /api/atc/say — direct OpenAI cost abuse vector + +**Files:** `server/middleware/auth.global.ts:9-11`, `server/api/atc/say.post.ts:174`, `server/api/atc/say.post.ts:181-189`, `server/api/atc/say.post.ts:346-372` + +auth.global.ts explicitly exempts the endpoint ('if (url.pathname.startsWith(\'/api/atc/say\')) { return }') and the handler's own auth is commented out ('// const user = await requireUserSession(event);'). There is no rate limit and no cap on text length — the only validation is non-empty ('const raw = (body?.text || \"\").trim()'). Any anonymous visitor can POST arbitrary text and receive OpenAI TTS audio (model tts-1 billed per character), making it a free TTS proxy and a billing DoS. Each request also creates a TransmissionLog document with no user attribution (the 'user: user._id' line is commented out) — unbounded anonymous DB writes — and with tag 'atis'/'flightlab' writes two files per unique text into the disk cache. /api/atc/ptt (Whisper, billed per audio minute) does require a session but is also completely unmetered per user. + +**Recommendation:** Remove the /api/atc/say exemption from server/middleware/auth.global.ts and re-enable requireUserSession in say.post.ts (the only legitimate callers — pm.vue, classroom.vue, classroom-introduction.vue — all send Bearer tokens via useApi already). Add a server-side text length cap (e.g. 1000 chars, 413 above it) and a simple per-user sliding-window rate limit (e.g. counter in Mongo or in-memory LRU keyed by user id, ~30 TTS calls/min, ~10 STT calls/min) applied to both say.post.ts and ptt.post.ts. Re-attach user._id to the TransmissionLog writes. + +#### LLM-03 · HIGH · effort M ✓ verified — No timeouts on any OpenAI call — a hung TTS/STT request stalls the speech queue for up to 10 minutes + +**Files:** `server/utils/openai.ts:13-23`, `server/utils/normalize.ts:8-16`, `server/api/atc/say.post.ts:272-278`, `server/api/atc/ptt.post.ts:108-114`, `app/composables/useApi.ts:30-44`, `server/api/atc/say.post.ts:147-151` + +Both OpenAI clients are constructed without 'timeout' or 'maxRetries' options (openai.ts: 'const clientOptions ... = { apiKey: openaiKey, defaultHeaders: ... }'; normalize.ts: 'new OpenAI(normalizeClientOptions)'), so the SDK default of 600 s applies. The Speaches fetch in say.post.ts has no AbortController at all. Client-side, useApi.ts sets no timeout either, and pm.vue's speech queue serializes playback behind 'await (audioPromise ?? fetchSpeechAudio(...))' — one hung TTS request blocks every subsequent ATC transmission until the user manually changes frequency (the only path that aborts). Mid-session OpenAI outages therefore manifest as the controller going silent indefinitely with no user feedback. + +**Recommendation:** Pass { timeout: 15_000, maxRetries: 1 } to both OpenAI client constructors (server/utils/openai.ts and server/utils/normalize.ts), wrap speachesTTS's fetch in an AbortController with a ~15 s timeout, and add a client-side timeout in fetchSpeechAudio (pm.vue) — e.g. AbortSignal.timeout(20_000) combined with the existing abort controller — so a stuck request skips that transmission and the queue continues. Surface a 'transmission unreadable, say again' style fallback in the UI when TTS fails. + +#### LLM-02 · MEDIUM (reported high, adjusted to medium after verification) · effort S ✓ verified — Unauthenticated GET /api/service/tools/latency triggers a paid LLM completion per request + +**Files:** `server/api/service/tools/latency.get.ts:13-28`, `server/middleware/auth.global.ts:12-14` + +The handler calls 'client.chat.completions.create({ model, ... })' with a fixed prompt on every GET, and it lives under /api/service/ which auth.global.ts exempts from authentication ('if (url.pathname.startsWith(\'/api/service/\')) { return }'). Anyone (including crawlers) can hammer it to run up the OpenAI bill. There is no caching, no rate limit, and the file header even says '// server/api/llm/latency.get.ts', suggesting it was moved under the unauthenticated /service/ prefix by accident. + +**Recommendation:** Move the route out of /api/service/ (e.g. server/api/admin/tools/latency.get.ts behind admin auth), or add requireUserSession at the top of the handler. Additionally cache the result for 60+ seconds so repeated calls don't each hit OpenAI. + +**Verifier note:** The finding is accurate but severity is overstated. The prompt is fixed (no attacker-controlled input), the model defaults to a nano-tier model, and each call is ~100 tokens, so per-request cost is fractions of a cent; meaningful billing damage would require millions of requests that OpenAI account rate limits would throttle first. The realistic impact is OpenAI quota/rate-limit exhaustion (degrading the live ATC feature for legitimate users) plus modest cost leakage — medium rather than high. The recommendation (move behind admin auth or add requireUserSession, plus caching) is sound. + +#### LLM-04 · MEDIUM (reported high, adjusted to medium after verification) · effort S ✓ verified — PTT race: releasing the button before async mic setup completes leaves recording stuck, then auto-submits 30 s of unintended audio to Whisper and the live session + +**Files:** `app/pages/pm.vue:3163-3225`, `app/pages/pm.vue:3227-3262`, `app/pages/pm.vue:562-567`, `app/pages/pm.vue:1865` + +startRecording awaits async setup (startPrerecCapture(), or navigator.mediaDevices.getUserMedia in the MediaRecorder path) before setting recording state and starting the safety timer ('mediaRecorder.value.start(); isRecording.value = true; startPttTimer()'). If the user taps and releases quickly, stopRecording runs while setup is still pending: 'prerecLiveCapture' is false and 'mediaRecorder.value && isRecording.value' is false, so it no-ops. The await then resolves and recording starts with the button already released. Nothing stops it until the PTT_MAX_DURATION_MS = 30_000 auto-stop fires, after which 30 seconds of ambient room audio is encoded, sent to /api/atc/ptt (Whisper cost), and whatever Whisper hallucinates is fed to handlePilotTransmission and the live backend session as a pilot transmission. + +**Recommendation:** Track intent with a synchronous flag: set 'pttPressed = true' at the top of startRecording and 'pttPressed = false' at the top of stopRecording. After each await in startRecording (startPrerecCapture, getUserMedia), check 'if (!pttPressed)' and bail out — stopping tracks/discarding the recorder without calling processTransmission. Apply the same guard in the prerec path before setting prerecLiveCapture = true. + +**Verifier note:** The bug is real but the default/common path is immune: prerecEnabled defaults to true (pm.vue:1875) and an immediate watch (pm.vue:4216-4222) eagerly starts prerec capture once mic permission + voice mode hold, so steady-state startRecording is fully synchronous (pm.vue:3170-3184) with no race window. The race requires prerec disabled by the user, a press while the eager init is still in flight (prerecStarting guard at pm.vue:3073 causes fall-through to the MediaRecorder path), or prerec init failure. Additionally, in the stuck prerec-WAV path a 30 s capture at the typical 48 kHz AudioContext rate (~2.88 MB) exceeds the server's 2 MB cap (ptt.post.ts:42, 63-65) and is rejected before Whisper — only the webm MediaRecorder fallback delivers 30 s of ambient audio to Whisper and the session. The pad also visibly shows a pulsing "Transmitting" state during the stuck recording. Hence medium rather than high. + +#### LLM-05 · MEDIUM (reported high, adjusted to medium after verification) · effort L ✓ verified — Browser talks to the Python radio backend directly with zero authentication + +**Files:** `app/composables/useRadioBackend.ts:39-62`, `nuxt.config.ts:60`, `app/pages/pm.vue:2714`, `shared/utils/communicationsEngine.ts:604-607` + +useRadioBackend issues bare '$fetch' calls to NUXT_PUBLIC_RADIO_BACKEND_URL ('createSession', 'transmit', 'deleteSession', 'fetchFlows') with no Authorization header, API key, or session binding — unlike every Nuxt API call which goes through useApi with a Bearer token. The backend URL is public runtime config, so anyone can read it from the bundle and create/drive unlimited sessions on the decision engine (which per AGENTS.md owns routing and readback evaluation; if it calls an LLM, that is unmetered too). The Nuxt auth/invitation gating of /pm is purely cosmetic for this path. Additionally, deleteSession is never called anywhere in pm.vue — sessions are abandoned on unmount/scenario change, leaking state on the backend. + +**Recommendation:** Either (a) proxy radio-backend calls through a Nuxt server route that enforces requireUserSession and adds a shared secret header for the Python service, or (b) mint a short-lived signed token in a Nuxt endpoint and have useRadioBackend send it so the Python backend can verify it. Also call radioBackend.deleteSession(backendSessionId) in onUnmounted and before creating a new session in startMonitoring. + +**Verifier note:** The finding is accurate except for two points. (1) The "unmetered LLM" cost-amplification is speculative: AGENTS.md:23 describes the Python backend as doing "regex routing, readback evaluation, and side effects" — not LLM calls (the legacy LLM router server/utils/openai.ts is "no longer called by /pm" per AGENTS.md:13). The concrete verifiable impact is unauthenticated session creation/driving and unbounded session leakage (resource exhaustion), not data exposure or confirmed API spend — hence medium rather than high. Would be high if the deployed Python backend invokes an LLM. (2) Minor: /pm has no auth route middleware at all — gating is only in-page client-side redirects (pm.vue:1980-1991), so "purely cosmetic" understates it slightly. The default backend URL is 127.0.0.1:8000, so in a purely local deployment exposure is nil, but any remote deployment necessarily exposes the backend unauthenticated. + +#### LLM-06 · MEDIUM · effort S — preNormalized flag is sent by the client but silently ignored by /api/atc/say — text is normalized twice + +**Files:** `app/pages/pm.vue:2533-2545`, `app/pages/pm.vue:3613-3623`, `shared/utils/communicationsEngine.ts:129-141`, `server/api/atc/say.post.ts:162-172`, `server/api/atc/say.post.ts:188` + +pm.vue sends 'preNormalized: usesNormalized' with the comment 'the server must not normalize again', and communicationsEngine.ts documents 'preNormalized texts skip the server normalizer'. But say.post.ts's readBody type has no preNormalized field and line 188 unconditionally runs 'const normalized = normalizeATC(raw)'. Every /pm and ATIS utterance is therefore expanded client-side (normalizeRadioPhrase with callsign/airport expansion) and then run through normalizeRadioPhrase again on the server. Besides wasted CPU, double expansion risks subtle TTS mangling whenever the normalizer is not idempotent (e.g. re-matching word sequences produced by the first pass), and it changes the cache key for ATIS entries versus what the client thinks it requested. + +**Recommendation:** In say.post.ts, read 'preNormalized?: boolean' from the body and skip normalizeATC when true: 'const normalized = body?.preNormalized ? raw : normalizeATC(raw)'. Add a regression test that a pre-normalized string passes through byte-identical. + +#### LLM-07 · MEDIUM · effort S — OpenAI TTS audio is MP3 but labeled audio/wav; the requested 'format' parameter is ignored for openai/piper providers + +**Files:** `server/api/atc/say.post.ts:270-282`, `server/api/atc/say.post.ts:58-63`, `server/api/atc/say.post.ts:202-205` + +The OpenAI branch calls 'normalize.audio.speech.create({ model: TTS_MODEL, voice, input: normalized, speed })' without response_format — the API default is mp3 — yet then sets 'actualMime = \"audio/wav\"' and 'outputExt' is forced to 'wav' ('const outputExt = (provider === \"openai\" || provider === \"piper\") ? \"wav\" : ext'). Clients receive MP3 bytes declared as audio/wav: Web Audio decodeAudioData sniffs and works, but the HTMLAudio fallback path builds 'data:audio/wav;base64,' (pm.vue:2444) which relies on browser sniffing leniency, and the ATIS/flightlab disk cache stores .wav files containing MP3 data. The client-requested 'format' field is silently dropped for these providers. + +**Recommendation:** Pass 'response_format: \"wav\"' (or honor the requested fmt) in the OpenAI speech.create call and derive actualMime/outputExt from what was actually requested, instead of hardcoding wav. Verify Piper genuinely returns WAV and document it next to defaultMimeForProvider. + +#### LLM-08 · MEDIUM · effort M — Dead legacy LLM decision layer and stale core docs — /api/llm/decide, routeDecision(), /api/decision-flows/runtime do not exist + +**Files:** `shared/utils/openaiDecision.ts:5-10`, `CLAUDE.md:19-21`, `shared/utils/communicationsEngine.ts:608-614`, `server/utils/normalize.ts:28-140`, `server/utils/normalize.ts:162-173`, `server/utils/radio.ts:1-8` + +shared/utils/openaiDecision.ts posts to '/api/llm/decide' — no such route exists anywhere under server/api, so any caller would 404. CLAUDE.md still documents 'POST /api/llm/decide → routeDecision()' and 'server/utils/openai.ts — LLM decision router (routeDecision())', but openai.ts contains only getOpenAIClient(); routeDecision exists nowhere in the repo. communicationsEngine.fetchRuntimeTree's no-baseUrl fallback fetches '/api/decision-flows/runtime', which also has no Nuxt route (only the Python backend serves it). normalize.ts carries an entire unused LLM prompt suite (atcSystemPrompt/atcSeedPrompt/atcReplyPrompt — referenced only by a smoke test — with atcReplyPrompt interpolating raw user text 'Pilot said: \"${userText}\"' into the prompt with no delimiting, a ready-made injection hole if ever revived) and speakATC which writes 'atc.mp3' to cwd with an invalid 'format' param. server/utils/radio.ts applyRadioEffect is never called. This dead layer is exactly where the next contributor will look for the product core and be misled. + +**Recommendation:** Delete shared/utils/openaiDecision.ts, the prompt builders + speakATC in server/utils/normalize.ts (keep normalizeATC/CALLSIGN_MAP), server/utils/radio.ts, and the related smoke test sections; update CLAUDE.md to match AGENTS.md's accurate description of the Python-backend flow; either remove the no-baseUrl fallback in fetchRuntimeTree or make it throw a descriptive error. If the prompt builders are kept for a future in-repo router, wrap user text in clear delimiters and add an instruction to ignore embedded directives. + +#### LLM-09 · LOW · effort S — /api/atc/say returns phantom storage metadata — files never written, audio URL route does not exist + +**Files:** `server/api/atc/say.post.ts:228-233`, `server/api/atc/say.post.ts:318-343`, `server/api/atc/say.post.ts:389-393` + +For non-cacheable requests the handler computes 'fileOut'/'fileJson' paths and returns 'stored: { audioPath, jsonPath, url: /api/atc/audio/${dateFolder}/${id}.${outputExt} }', but there is no writeFile for fileOut or fileJson anywhere, and no /api/atc/audio/** route exists in server/api. Consumers (and the api-docs page) are handed paths and URLs that 404 / point at nonexistent files. simulateRadioQuality's computed 'gain' (lines 65-74) is likewise returned but never applied to audio. + +**Recommendation:** Remove fileOut/fileJson/storedUrl and the 'stored' block from the response (no client uses stored.url — verified by grep), or actually persist the audio and add a server route that streams it with auth. Drop the unused gain from simulateRadioQuality or document that radio degradation is client-side only. + +#### LLM-10 · LOW · effort S — TTS disk cache grows without bound — no eviction or size cap + +**Files:** `server/api/atc/say.post.ts:20-22`, `server/api/atc/say.post.ts:288-316` + +Every unique (text, level, voice, speed, format, provider, model) tuple with tag 'atis' or 'flightlab' writes a .wav/.mp3 plus a .json meta file into .cache/flightlab-tts. ATIS content changes every 30 minutes per station and includes the full METAR text in the hash, so the cache accretes new entries indefinitely; combined with the endpoint being unauthenticated (see critical finding), an attacker can fill the disk by varying text with tag 'atis'. + +**Recommendation:** Add an eviction pass (e.g. on each cache write, delete files older than 48 h, or keep an LRU index capped at ~500 MB). A simple readdir + stat + unlink of stale files inside ensureDir's code path is sufficient. + +#### LLM-11 · LOW · effort S — Stray out.ogg committed to repo root; fluent-ffmpeg dependency declared but never used + +**Files:** `out.ogg`, `package.json:23`, `README.md:29` + +out.ogg (293 bytes, committed in fe8b762 'iwas was nich tut dazutun') sits in the repo root — leftover from manual ffmpeg testing. package.json declares 'fluent-ffmpeg: ^2.1.3' and '@types/fluent-ffmpeg', and the README states ffmpeg is 'Needed for audio processing (fluent-ffmpeg)', but no source file imports fluent-ffmpeg — both ffmpeg call sites (ptt.post.ts:70, radio.ts:5) use execFile directly. + +**Recommendation:** git rm out.ogg; remove fluent-ffmpeg and @types/fluent-ffmpeg from package.json; correct the README line to say ffmpeg is invoked directly for PTT format conversion. + +#### LLM-12 · LOW · effort S — playPTTBeep leaks an AudioContext per beep + +**Files:** `app/pages/pm.vue:3945-3965` + +Every PTT press and release creates 'new (window.AudioContext || ...)' and never calls close(). Browsers cap the number of concurrent AudioContexts (Chrome historically ~6 per tab on some platforms); a long session with many PTT cycles can exhaust the limit, at which point construction throws and is silently swallowed — beeps die, and other code paths creating contexts (pizzicato, ATIS loop) can also start failing. + +**Recommendation:** Reuse the existing module-level audioContext via ensureAudioContext() for the beep oscillator, or call 'oscillator.onended = () => audioContext.close()' after the 0.1 s beep. + +#### LLM-13 · LOW · effort S — Page unmount does not stop in-flight ATC speech or tear down the speech queue + +**Files:** `app/pages/pm.vue:4239-4243`, `app/pages/pm.vue:2336-2353` + +onUnmounted calls stopAtisLoop(), stopPrerecCapture() and cancelAirportDataRefresh() but not stopCurrentSpeech(). Queued/playing controller audio (HTMLAudio or pizzicato through the module AudioContext) keeps playing after navigating away from /pm, and pending TTS fetches complete and play into the void. The shared audioContext is also never closed. + +**Recommendation:** Add stopCurrentSpeech() (and audioContext?.close()) to the onUnmounted hook in pm.vue. + +#### LLM-14 · LOW · effort M — Pilot readback doubles TTS spend per transmission and is never cached + +**Files:** `app/pages/pm.vue:2667-2669`, `app/pages/pm.vue:2644-2652` + +When readbackEnabled is on, every pilot transmission triggers a second full TTS round-trip ('speakPilotReadback(transcript)' → /api/atc/say with voice 'verse') purely to echo the pilot's own words back through radio effects. Unlike ATIS, these are not cacheable server-side (tag 'pilot-readback' is not in isCacheableTag), so with OpenAI TTS this doubles per-utterance cost and adds a queued network round-trip before the controller reply plays. + +**Recommendation:** Either synthesize the readback locally (Web Speech API / pre-recorded voice through the existing pizzicato chain) or include 'pilot-readback' usage in the per-user rate budget and document the cost. At minimum make readbackEnabled default off for OpenAI-provider deployments. + +#### LLM-15 · LOW · effort L — TTS responses are fully buffered base64 JSON — no streaming, inflated payloads, slower time-to-first-audio + +**Files:** `server/api/atc/say.post.ts:383-388`, `app/pages/pm.vue:2441-2453` + +The server waits for complete TTS generation, then returns '{ audio: { base64: audioBuffer.toString(\"base64\") } }' — a 33% size overhead on top of WAV (the default non-Speaches format, ~10x larger than mp3 for the same content), all parsed as one JSON blob before the first sample can play. For long ATIS broadcasts this is multi-megabyte JSON. OpenAI TTS and Speaches both support chunked/streamed audio responses. + +**Recommendation:** Return binary audio (event.node.res with Content-Type, or sendStream) instead of base64 JSON, and prefer mp3/opus over wav for the wire format (decodeAudioData handles both). As a later step, stream the OpenAI TTS response through to the client and feed it via MediaSource for true progressive playback. Keep the artificial 800-2800 ms controller delay — it already masks generation latency well. + +--- + +## Architecture & Data Model + +OpenSquawk's architecture is in mid-migration: the documented Nuxt-side decision loop (/api/llm/decide, /api/decision-flows/runtime, routeDecision) no longer exists — /pm now drives a separate Python backend while keeping a local mirror of the state machine in shared/utils/communicationsEngine.ts, leaving substantial dead code and a fragile dual-state-machine sync. The engine itself has real correctness gaps (timer_next never fires, delayed auto-transitions race past state changes, no transmit re-entrancy guard, no reload recovery), the flow editor can corrupt live trees (node deletion leaves dangling transition targets, no versioning), and several MongoDB collections grow unbounded without the indexes their admin queries need. Boundaries are otherwise decent: /shared has no server/app imports, and models that matter (DecisionNode, DecisionFlow) have proper unique constraints. + +### What's done well + +- Clean /shared boundary: no file in shared/ imports from server/ or app/ (verified by grep); shared/types/decision.ts is the single type source used by both server/services/decisionFlowService.ts and the client engine. +- Strong integrity constraints where it matters: DecisionNode has a compound unique index { flow: 1, stateId: 1 } (server/models/DecisionNode.ts:201) and DecisionFlow.slug is unique (server/models/DecisionFlow.ts:31); User.email, WaitlistEntry.email, InvitationCode.code, BridgeToken.token are all unique. +- PasswordResetToken uses a Mongo TTL index (expiresAt with index: { expires: 0 }, server/models/PasswordResetToken.ts:14) so expired tokens self-clean. +- KpiReportDelivery.weekKey is unique (server/models/KpiReportDelivery.ts:14), making the weekly cron idempotent across restarts and replicas — a correct pattern for cron-style endpoints. +- server/utils/decisionSanitizer.ts is a thorough, defensive input-normalization layer for editor payloads (whitelisted transition/trigger/condition types, operator sets, fallbacks) and has dedicated unit tests (tests/server/decisionSanitizer.test.ts). +- Node rename (server/api/editor/flows/[slug]/nodes/[stateId]/rename.patch.ts:58-82) correctly rewrites all referencing transitions plus flow.startState/endStates, including a dry-run mode when the new id equals the old. +- The engine has consistent loop protection: collectAtcStatesUntilPilotTurn uses maxHops + a visited set (shared/utils/communicationsEngine.ts:984-994), and evaluateAutoTransitions/evaluateSimpleAutoFlow carry loopGuard caps (lines 1158, 1213). +- The moveTo vs moveToSilent split (shared/utils/communicationsEngine.ts:1295-1336) makes 'backend owns the next move' explicit, and pm.vue consistently uses moveToSilent when replaying backend-driven state (app/pages/pm.vue:2746-2751). +- Per-flow FlowSnapshot design (shared/utils/communicationsEngine.ts:102-112, 306-404) cleanly isolates variables/flags/log/auto-history per flow with a stack-based linear-flow resume (resumeLinearFlow). +- TransmissionLog writes are best-effort (try/catch + warn) in both /api/atc/say and /api/atc/ptt, so logging failures never break the user-facing TTS/STT path (server/api/atc/ptt.post.ts:128-148). + +### Findings + +#### ARCH-07 · HIGH · effort L ✓ verified — Flow editor can corrupt live trees: node deletion leaves dangling transition targets, startState is unvalidated, multi-doc edits are non-transactional, no flow versioning + +**Files:** `server/api/editor/flows/[slug]/nodes/[stateId]/index.delete.ts:25-29`, `server/api/editor/flows/[slug]/index.put.ts:44-46`, `server/api/editor/flows/[slug]/nodes/[stateId]/rename.patch.ts:66-82`, `server/models/DecisionFlow.ts:14`, `shared/utils/communicationsEngine.ts:1251-1256` + +DELETE node runs `DecisionNode.deleteOne({ flow, stateId })` with zero checks: no test whether other nodes reference it via transitions.target, whether it is flow.startState, or an endState — the runtime tree then contains transitions whose target resolves to nothing, and the client engine just logs '[Engine] Unknown state' and stays put (communicationsEngine.ts:1253-1255), a silent dead end. Flow PUT accepts any string for startState ('if (typeof body.startState === \'string\'...) flow.startState = ...') without verifying a node with that stateId exists. Rename updates node.save() then updateMany() then flow.save() without a Mongo transaction, so a crash mid-way leaves mixed references. There is no version/revision field on DecisionFlow (only the free-text schemaVersion), so the Python backend and connected clients have no way to detect that a flow changed under a live session; runtime trees are rebuilt from current documents on every fetch. + +**Recommendation:** In nodes/[stateId]/index.delete.ts, before deleteOne: query DecisionNode.find({ flow, 'transitions.target': stateId }) and flow.startState === stateId, and either reject with 409 listing referencing nodes or cascade-remove the transitions. In flows/[slug]/index.put.ts, validate body.startState against DecisionNode.exists({ flow, stateId }). Wrap rename's three writes in mongoose session.withTransaction(). Add a numeric `revision` field on DecisionFlow incremented by a post-save hook on flow/node mutations and include it in the runtime payload so sessions can detect staleness. Add a shared validateFlow() (unreachable nodes, dangling targets, missing start/end states) invoked by the editor save endpoints and exposed as GET /api/editor/flows/[slug]/validate. + +#### ARCH-08 · HIGH · effort M ✓ verified — TransmissionLog grows unbounded and lacks the indexes its admin queries require; session list is a full-collection aggregation + +**Files:** `server/models/TransmissionLog.ts:17-28`, `server/api/admin/logs/transmissions.get.ts:106-112`, `server/api/admin/logs/sessions.get.ts:53-77`, `server/models/LandingAnalyticsEvent.ts:16-25`, `server/models/ProductUsageSession.ts:15-24` + +Every PTT and TTS call inserts a TransmissionLog document (ptt.post.ts:134, say.post.ts:346) with no TTL and no cap. The only index is sessionId; yet /api/admin/logs/transmissions sorts `{ createdAt: -1 }` with optional channel/direction/role/createdAt filters plus case-insensitive regex over text/normalized — all unindexed, forcing collection scans that degrade linearly with growth. /api/admin/logs/sessions aggregates the ENTIRE collection on every page view: $match sessionId exists → $sort { sessionId: 1, createdAt: 1 } → $group first/last/count, plus a second full aggregation for the total count. LandingAnalyticsEvent and ProductUsageSession are written from public unauthenticated endpoints (middleware exempts /api/service/) and also grow unbounded. + +**Recommendation:** Add to TransmissionLog: schema.index({ createdAt: -1 }), schema.index({ channel: 1, createdAt: -1 }), and a compound { sessionId: 1, createdAt: 1 } (covers the sessions aggregation sort). Decide a retention policy and add TTL indexes (e.g. expireAfterSeconds 90 days) on TransmissionLog.createdAt and LandingAnalyticsEvent.createdAt, or a scheduled pruning job under server/api/service/cron. For the sessions list, maintain a small SessionSummary collection updated on insert (or $merge materialized view refreshed by cron) instead of aggregating all transmissions per request. + +#### ARCH-01 · MEDIUM (reported high, adjusted to medium after verification) · effort M ✓ verified — Dual state machine (local engine mirror + Python backend) with no divergence detection; project docs describe an architecture that no longer exists + +**Files:** `app/pages/pm.vue:2931-2942`, `app/pages/pm.vue:2746-2751`, `shared/utils/communicationsEngine.ts:984-1070`, `CLAUDE.md:18-22`, `app/composables/useRadioBackend.ts:27-64` + +pm.vue runs TWO state machines: the Python backend session (radioBackend.transmit) is authoritative, while the local communicationsEngine is replayed via moveToSilent for every auto_advanced_state. The initial ATC walk is done purely locally with a comment admitting the assumption: 'Safe because we loaded the tree from the same Python backend, so the walk is identical to what the backend will do' (pm.vue:2931-2933) — nothing verifies this, and the local collectAtcStatesUntilPilotTurn has its own transition-selection heuristics (prefer ok_next for auto states, single-eligible-transition rule) that can diverge from the backend's walker. Meanwhile CLAUDE.md still documents 'POST /api/llm/decide → routeDecision()' — routeDecision exists nowhere in the repo (grep matches only CLAUDE.md/AGENTS.md) and there is no /api/llm or /api/decision-flows route under server/api. + +**Recommendation:** Make the backend the single source of truth: have POST /api/radio/session and transmissions return the rendered initial ATC messages (the backend already returns auto_advanced_states) and delete the local collectAtcStatesUntilPilotTurn walk in pm.vue startup, replacing it with replay of backend-provided messages. Add a state-mismatch assertion after each moveToSilent (compare engine.currentStateId to response.next_state_id, log/report divergence). Rewrite CLAUDE.md/AGENTS.md sections 'Live ATC Flow' to describe the Python-backend flow (useRadioBackend.ts) instead of routeDecision/applyLLMDecision. + +**Verifier note:** The finding is factually accurate, but it understates the existing self-correction: every transmit re-syncs the local cursor (moveToSilent(response.next_state_id), pm.vue:2751), patches variables/flags from the backend (pm.vue:2760-2768), and uses backend-rendered TTS (pm.vue:2773), so divergence cannot compound across transmissions. The unguarded window is limited to the initial local-only ATC walk (locally rendered TTS, backend never informed) and inter-transmission UI state. That bounds the runtime impact, making this a medium-severity architecture/documentation-debt issue rather than high. + +#### ARCH-02 · MEDIUM · effort M — Dead code: Nuxt runtime-tree builders, LLM decision client, and a no-op 'import' service that still ships an admin endpoint + +**Files:** `server/services/decisionFlowService.ts:221-269`, `shared/utils/openaiDecision.ts:5-10`, `shared/utils/communicationsEngine.ts:601-617`, `server/services/decisionImportService.ts:9-21`, `server/api/editor/flows/import.post.ts:9-19` + +buildRuntimeDecisionTree and buildRuntimeDecisionSystem (decisionFlowService.ts:221,230) are exported but imported nowhere — no Nuxt route serves /api/decision-flows/runtime. shared/utils/openaiDecision.ts posts to '/api/llm/decide', a route that does not exist, and decideNextStateLLM is never imported. communicationsEngine.fetchRuntimeTree without baseUrl fetches '/api/decision-flows/runtime' against Nuxt (line 613), which would 404; pm.vue always passes radioBackendUrl (pm.vue:1996, 2835). decisionImportService.importATCDecisionTree just calls getFlowWithNodes and returns { importedStates: nodes.length } — it imports nothing — yet is exposed as POST /api/editor/flows/import and scripts/import-decision-tree.ts, which is actively misleading for operators. + +**Recommendation:** Delete shared/utils/openaiDecision.ts and decisionImportService.ts plus /api/editor/flows/import.post.ts (or reimplement import to actually upsert DecisionFlow/DecisionNode documents from a payload). Either delete buildRuntimeDecisionTree/buildRuntimeDecisionSystem or wire them to a real GET /api/decision-flows/runtime route so the Nuxt server can serve flows without the Python backend; remove the no-baseUrl branch of fetchRuntimeTree if the Python backend is the only source. + +#### ARCH-03 · MEDIUM (reported high, adjusted to medium after verification) · effort M ✓ verified — timer_next transitions are modeled, stored, and serialized but never fire — flows using them dead-end + +**Files:** `shared/utils/communicationsEngine.ts:455`, `server/services/decisionFlowService.ts:135-144`, `server/models/DecisionNode.ts:128-134`, `shared/types/decision.ts:230` + +The whole pipeline supports timer transitions: DecisionNode.transitions accepts type 'timer' with { afterSeconds, allowManualProceed }, toRuntimeTimers serializes them to timer_next, and the type RuntimeDecisionState declares `timer_next?: Array<{ to; after_s }>`. But the engine never schedules them: timer_next only appears in nextCandidates (line 455, as LLM candidates stripped of after_s) and in pm.vue:4004 for chain display. Neither evaluateAutoTransitions nor evaluateSimpleAutoFlow reads timer_next, and the legacy state types like 'after_s' have no setTimeout anywhere. A state whose only outgoing edges are timer transitions is a silent dead end (collectAtcStatesUntilPilotTurn finds zero eligible transitions and stops). + +**Recommendation:** Implement timer scheduling in communicationsEngine: when moveTo lands on a state with timer_next, schedule setTimeout(after_s * 1000) entries that (a) check currentStateId is still the scheduling state before firing, and (b) are tracked in a per-engine array cancelled in resetEngineFromSystem/setActiveFlow. Alternatively, if timers are intentionally backend-only now, remove timer support from the editor UI and sanitizer (decisionSanitizer.ts TRANSITION_TYPES) so authors cannot create transitions that never run, and document it in shared/types/decision.ts. + +**Verifier note:** Slight overstatement on "dead-end": timer_next targets are included in nextCandidates (communicationsEngine.ts:455) and therefore appear as LLM candidates in buildLLMContext (lines 835-843), so a pilot transmission can still route through a timer edge via routeDecision/applyLLMDecision. A timer-only state is thus a stall of automatic progression (the timer never fires after after_s seconds; allowManualProceed is dropped) rather than a permanently unreachable dead end. Additionally, no flow data checked into the repo actually uses type 'timer' transitions (flows live in MongoDB, so production usage is unverified), making this a dormant-feature/authoring-trap bug — severity medium rather than high. + +#### ARCH-04 · MEDIUM (reported high, adjusted to medium after verification) · effort S ✓ verified — Delayed auto-transitions fire without re-checking current state, and pending timers survive engine resets — sessions can teleport + +**Files:** `shared/utils/communicationsEngine.ts:1235-1244`, `shared/utils/communicationsEngine.ts:1201-1210`, `shared/utils/communicationsEngine.ts:506-571` + +In evaluateAutoTransitions the delayed branch runs `setTimeout(() => { moveTo(transition.to); evaluateAutoTransitions(loopGuard + 1) }, delay)` with no guard that the engine is still on the originating state — unlike evaluateSimpleAutoFlow, which correctly checks `if (currentStateId.value !== fromStateId) return` (lines 1205-1207). If the pilot transmits (or the backend moves the cursor) during the delay, the stale timer still executes moveTo and yanks the session to an unrelated state. Additionally resetEngineFromSystem (line 506) clears snapshots and flowStack but cancels no pending setTimeout from evaluateAutoTransitions/evaluateSimpleAutoFlow and does not reset pendingSimpleAutoTransition; after a scenario switch, a leftover timer can fire moveTo(targetId) into the freshly loaded tree whenever the target stateId also exists there (state ids like CD_REQUEST recur across flows). + +**Recommendation:** Capture `const fromStateId = currentStateId.value` (and the active flow slug) before scheduling in evaluateAutoTransitions and bail inside the callback if either changed, mirroring evaluateSimpleAutoFlow. Keep all scheduled timer handles in a Set on the engine instance; clear them (clearTimeout) and null pendingSimpleAutoTransition in resetEngineFromSystem and setActiveFlow. Add a generation counter incremented on every reset, captured by closures and compared before acting. + +**Verifier note:** The finding is accurate but overstates severity. Mitigations narrow the blast radius: (a) moveTo rejects state ids absent from the loaded tree (lines 1253-1256), so post-reset teleports require an id collision or same-flow restart; (b) the fully-unguarded path only triggers for trees that actually use auto_transitions with delayMs > 0 — supported by the editor/schema but unverifiable in MongoDB data; (c) the most frequently scheduled timer (evaluateSimpleAutoFlow, 1-2 s delays) already guards the common pilot-transmit race; (d) in /pm the Python backend owns authoritative state and backend-driven syncs use moveToSilent (no auto-evaluation), so the damage is local-mirror desync (wrong LLM candidates, wrong TTS) rather than irreversible authoritative-state corruption. Real bug, recommended fix (capture from-state + flow slug, track timer handles, generation counter on reset) is correct — but medium, not high. + +#### ARCH-05 · MEDIUM · effort S — No re-entrancy guard for overlapping pilot transmissions in /pm + +**Files:** `app/pages/pm.vue:2654-2801`, `app/pages/pm.vue:2746-2768` + +handlePilotTransmission awaits radioBackend.transmit and then mutates the local engine (moveToSilent per auto_advanced state, patchVariables, patchFlags, appendLogEntry). There is no in-flight lock: a second PTT release or text submit while the first request is pending issues a concurrent transmit against the same backend session, and the two responses interleave their moveToSilent/patch sequences in arrival order — the local cursor and variables can end up reflecting a mix of both responses (e.g. response A's auto_advanced_states replayed after response B's next_state_id). The Python backend likely also processes the second utterance against a state the pilot never heard. + +**Recommendation:** Add a `transmitInFlight = ref(false)` gate in handlePilotTransmission: either drop/queue new transmissions while one is pending (queueing matches radio semantics — you cannot transmit while the controller replies), and disable the PTT/send UI from the same flag. Apply each response atomically only if it is the latest request (request sequence number captured before await). + +#### ARCH-06 · MEDIUM · effort L — No session recovery: page reload loses the live ATC session and leaks the backend session + +**Files:** `app/pages/pm.vue:1309`, `app/pages/pm.vue:2893`, `app/pages/pm.vue:2021-2030`, `shared/utils/communicationsEngine.ts:190-229` + +backendSessionId lives only in a ref (`const backendSessionId = ref(null)`, pm.vue:1309) set at session creation (line 2893) and never persisted. The engine's entire state (FlowSnapshots, variables, flags, communicationLog) is in-component memory. pm.vue persists STORAGE_KEYS.selectedPlan, vatsimId, and prerec settings in localStorage (lines 2021-2030) but not the session id or state cursor, so an accidental reload mid-flight discards the whole training session; the Python backend session is orphaned (deleteSession is never called for it) and the user must restart from clearance delivery. + +**Recommendation:** Persist { backendSessionId, activeFlow, currentStateId, variables-hash, scenario } to localStorage on every transmit response; on mount, if a stored session exists, offer 'Resume previous session' which re-attaches by fetching session state from the Python backend (add GET /api/radio/session/:id there if missing) and replaying it via moveToSilent/patchVariables. Call radioBackend.deleteSession in onBeforeUnmount/visibilitychange teardown when the user explicitly exits. + +#### ARCH-09 · MEDIUM · effort L — Server-process in-memory state (bridge logs, telemetry store, WS sessions, dome-light cache) breaks on restart and multi-instance deployment; FlightLab uses one GLOBAL session for all users + +**Files:** `server/utils/bridgeLog.ts:13-15`, `server/utils/flightlabTelemetry.ts:10-35`, `server/api/flightlab/ws.ts:19-23`, `server/api/bridge/data.post.ts:13`, `server/api/bridge/data.post.ts:11` + +Four module-level singletons hold session state in process memory: bridgeLog's `logStore = new Map()`, flightlabTelemetryStore (latest telemetry per userId, plus listener fan-out that WS handlers subscribe to in-process), ws.ts `sessions` / `userSessionMap` Maps, and `lastDomeLightModeByToken`. With more than one Nitro instance, a bridge POST /api/bridge/data landing on instance A never reaches a WebSocket peer connected to instance B, and /api/bridge/log returns empty for requests routed elsewhere; any restart drops all of it. ws.ts additionally hardcodes `GLOBAL_SESSION_CODE = 'GLOBAL'` and getOrCreateGlobalSession, so ALL users joining FlightLab share one session object — one user's instructor 'restart' command resets every connected user. bridge/data.post.ts also hardcodes a personal home-automation webhook fallback URL ('https://home.io.faktorxmensch.com/api/webhook/lidl_stab_3modi_8492'). + +**Recommendation:** Short term: document a single-instance constraint (sticky sessions) in README/deploy config. Medium term: move telemetry pub/sub to Redis (or Mongo change streams) keyed by userId so POST and WS can live on different instances; persist bridge log entries to a capped Mongo collection instead of the Map. Replace GLOBAL session with per-user/per-code session creation (generate a join code in create-session, require it in join-session). Move the dome-light webhook URL into runtimeConfig (env var) and default it to empty/disabled. + +#### ARCH-10 · MEDIUM · effort S — /api/atc/say returns stored.url and file paths that are never written; the referenced audio route does not exist + +**Files:** `server/api/atc/say.post.ts:228-232`, `server/api/atc/say.post.ts:317-320`, `server/api/atc/say.post.ts:290-292`, `server/api/atc/say.post.ts:385-390` + +say.post.ts computes `fileOut`/`fileJson` under storage/atc// and returns `stored: { audioPath: storedAudioPath, jsonPath: storedJsonPath, url: storedUrl }` where storedUrl = '/api/atc/audio//.' for non-cacheable requests. But writeFile is only ever called for the flightlab/atis cache path (lines 290-292); fileOut and fileJson are never written, and no route exists under server/api/atc except ptt.post.ts and say.post.ts (no /api/atc/audio handler anywhere — grep finds the string only inside say.post.ts). So the response advertises files and URLs that 404/don't exist, and leaks absolute server filesystem paths (process.cwd()-based) to clients. + +**Recommendation:** Remove the `stored` block, `fileOut`, `fileJson`, `outDir()` and the unused dateFolder logic from say.post.ts since clients consume the base64 payload — or, if persisted audio is wanted, actually write the files and add a server/api/atc/audio/[...path].get.ts handler with path-traversal protection. Never return raw server paths; return only the relative URL. + +#### ARCH-11 · MEDIUM · effort S — Duplicate SimBrief proxy routes with divergent contracts, and a helper module registered as a phantom API route + +**Files:** `server/api/learn/simbrief.get.ts:1-70`, `server/api/copilot/simbrief.get.ts:1-60`, `server/api/service/tools/airportGeocode.ts:1`, `server/api/service/tools/airport-geocode.get.ts:1-10`, `server/middleware/auth.global.ts:18-20` + +Two endpoints proxy the same https://www.simbrief.com/api/xml.fetcher.php: /api/learn/simbrief (param userId, AbortController timeout, returns { data }, auth-required) and /api/copilot/simbrief (params username|userid, $fetch without timeout, returns a flattened OFP plus `raw`, and is auth-EXEMPT because auth.global.ts skips /api/copilot/). Same upstream, different params, error mapping, response shape, and auth posture. Separately, server/api/service/tools/airportGeocode.ts is a pure helper module (fetchAirportFeatures/resolveFeature, no default export) but lives inside server/api, so Nitro registers it as route /api/service/tools/airportGeocode that fails when hit — helpers must live under server/utils. File naming in the same folder also mixes kebab-case (airport-geocode.get.ts) with camelCase (airportGeocode.ts). + +**Recommendation:** Extract a single fetchSimbriefOfp(usernameOrId) into server/utils/simbrief.ts with the timeout from learn/simbrief.get.ts, and make both routes thin adapters — or delete one route and point both pages (classroom.vue:2651, copilot.vue:278) at the surviving endpoint; align auth (require session for both or document why copilot is public). Move airportGeocode.ts to server/utils/airportGeocode.ts and update the import in airport-geocode.get.ts. + +#### ARCH-12 · MEDIUM · effort M — Inconsistent API design: soft 200-errors vs createError, mixed German/English messages, side-effecting GET cron routes, prefix-based auth exemptions + +**Files:** `server/api/service/tools/airport-geocode.get.ts:52-58`, `server/api/bridge/connect.post.ts:12`, `server/api/editor/flows/[slug]/nodes/[stateId]/rename.patch.ts:24`, `server/api/service/cron/waitlist-drip.get.ts:1-20`, `server/middleware/auth.global.ts:5-24`, `server/api/auth/logout.post.ts`, `server/api/service/auth/login.post.ts` + +Error shapes diverge per route: airport-geocode returns HTTP 200 with `{ error: 'missing_airport' }` / `{ error: 'overpass_error', details }` while everything else throws createError with statusMessage; clients must implement both. statusMessages mix languages ('x-bridge-token header fehlt oder ist ungültig.' in bridge/connect.post.ts vs English elsewhere; 'Neuer Identifier ist erforderlich' in rename.patch.ts; 'Verbindung zum Training-Backend fehlgeschlagen.' in pm.vue). Auth endpoints are split across two namespaces (/api/auth/logout.post.ts, /api/auth/me.get.ts vs /api/service/auth/login.post.ts, register, refresh) with no apparent rule. Cron actions that send email and mutate WaitlistEntry are GET endpoints (service/cron/waitlist-drip.get.ts, weekly-kpi-report.get.ts) — cacheable/prefetchable verbs with side effects. Authorization is decided by an exclusion list of path prefixes in auth.global.ts (skips ALL of /api/service/, /api/bridge/, /api/copilot/, /api/atc/say), so any new route added under those prefixes is silently public. + +**Recommendation:** Adopt one error contract: always throw createError({ statusCode, statusMessage, data }) and convert airport-geocode's soft errors to 400/502. Sweep statusMessages to English (grep 'fehlt|ungültig|erforderlich|fehlgeschlagen'). Consolidate auth routes under /api/auth/. Change cron routes to POST and require the existing cron secret. Invert the middleware default: maintain an explicit allowlist of public routes rather than public prefixes, so new endpoints are private by default. + +#### ARCH-13 · MEDIUM · effort L — pm.vue (5,046 lines) and classroom.vue (8,614 lines) are monoliths duplicating PTT recording, TTS playback, and speech-queue plumbing + +**Files:** `app/pages/pm.vue:2067-3340`, `app/pages/classroom.vue:2904-3720`, `app/pages/pm.vue:2598-2652`, `app/pages/classroom.vue:4930-5070` + +CLAUDE.md's split is real — classroom.vue does not import communicationsEngine — but both pages independently implement: MediaRecorder/getUserMedia PTT capture posting base64 to /api/atc/ptt (pm.vue:3186-3330, classroom.vue:3672-3717), TTS via /api/atc/say with radio-effect post-processing and queued playback (pm.vue speakWithRadioEffects/enqueueSpeech 2598-2631, classroom.vue 4634-5010 incl. its own browser speechSynthesis fallback), signal-level handling, and audio decode/playback paths (18 vs 28 audio-API references respectively). The shared pure logic already lives in shared/utils (radioSpeech, radioEffects, sttMatch) — the duplication is the stateful browser plumbing, which is exactly what drifts (e.g. classroom has a speechSynthesis fallback and voice preloading pm lacks; pm has pre-record buffering classroom lacks). + +**Recommendation:** Extract two app-level composables: usePttRecorder() (getUserMedia + MediaRecorder + base64 encode + POST /api/atc/ptt, with the prerec option from pm.vue) and useAtcSpeech() (queue, /api/atc/say call, radio-effect playback, speechSynthesis fallback) under app/composables/. Migrate classroom.vue first (it has the superset of features), then pm.vue. This also shrinks both SFCs toward reviewable size. + +#### ARCH-14 · MEDIUM · effort M — Core state machine has zero unit tests + +**Files:** `shared/utils/communicationsEngine.ts:190-1568`, `tests/shared/radioSpeech.test.ts`, `package.json:16` + +tests/ covers decisionSanitizer, radioSpeech, sttMatch, scenario, auth, bridge handlers — but there is no test file for communicationsEngine.ts (1,568 lines), the component that owns transition selection (collectAtcStatesUntilPilotTurn's ok_next-preference and single-eligible rules), condition evaluation (evaluateConditionExpression's hand-rolled ||/&& splitting), flow snapshots/stack, and applyLLMDecision. The test runner (node:test via tsx, package.json test script) and a vue import are the only dependencies, so it is testable today; the engine's subtle rules (e.g. '"look through" auto-behavior check states' in buildLLMContext:849-873, INT_ stack-push convention at 1258) are exactly the kind of logic that regresses silently. + +**Recommendation:** Add tests/shared/communicationsEngine.test.ts with fixture RuntimeDecisionSystem objects covering: moveTo unknown-state handling, collectAtcStatesUntilPilotTurn stop conditions (pilot state, end state, ambiguous transitions, maxHops), evaluateConditionExpression operator/namespace matrix, applyLLMDecision (updates/flags/activate_flow/resume_previous), and flow-switch snapshot isolation. The fixes from the timer/race findings should land with regression tests here. + +#### ARCH-15 · LOW · effort M — Engine semantics ride on naming conventions and hardcoded defaults (INT_ prefix, handoff regexes, fake frequencies/SIDs) + +**Files:** `shared/utils/communicationsEngine.ts:1258`, `shared/utils/communicationsEngine.ts:1460-1468`, `shared/utils/communicationsEngine.ts:758-815`, `shared/utils/communicationsEngine.ts:29-38`, `shared/utils/communicationsEngine.ts:1476-1498` + +moveTo pushes the previous state onto flags.stack whenever the target stateId merely startsWith('INT_') (line 1258) — an invisible contract no editor validation enforces or documents; a flow author naming a normal state 'INT_CHECK' silently changes stack behavior. unitFromHandoff maps handoff targets via regexes (/GROUND/i, /CENTER|CTR/i). initializeFlight hardcodes Frankfurt-flavored defaults (delivery 121.900, SID list ['SULUS5S','TOBAK2E',...], genStand/genRunway/genSquawk random pools), and FLIGHT_PHASES pins frequencies that the real flow then overrides — these placeholder values can leak into TTS if real data is missing (e.g. squawk randomized into transponder readbacks). + +**Recommendation:** Replace the INT_ prefix check with an explicit node attribute (e.g. metadata.interrupt: true or autoBehavior 'push_stack') stored on DecisionNode and surfaced in the editor; keep the prefix check temporarily behind a deprecation warning. Move unit mapping into the handoff object (handoff.unit: 'GROUND') in the schema. Gate genStand/genRunway/genSquawk fallbacks behind an explicit demo flag and log when a placeholder value is used in a live session. + +--- + +## Code Quality & Maintainability + +The codebase typechecks cleanly on both app and server projects and has genuine strengths — a shared type layer used on both sides of the API, a centralized authenticated-fetch composable, an 80-test suite, and careful auth utilities. However, maintainability is dominated by six monolithic pages (classroom.vue at 8,614 lines, pm.vue at 5,046) with a TTS/radio-audio pipeline copy-pasted across three of them, there is no lint config or CI at all, and the test suite is red at HEAD. Cruft is accumulating: a dead client for a non-existent /api/llm/decide endpoint, unused prompt/TTS exports, an accidental out.ogg artifact, commented-out auth on the OpenAI-billed TTS route, and CLAUDE.md documenting an architecture (routeDecision/Node decision API) that was replaced by an external Python backend. + +### What's done well + +- Shared type layer is real, not aspirational: shared/types/decision.ts is imported by server models (server/models/DecisionFlow.ts:5, DecisionNode.ts:12), services (server/services/decisionFlowService.ts:14), sanitizer, editor API routes AND frontend (app/pages/editor/index.vue:1163, app/components/editor/DecisionNodeCanvas.vue:217), giving end-to-end type agreement on the decision-flow domain. +- app/composables/useApi.ts is a clean centralized fetch wrapper with Bearer injection, abort-aware 401 refresh-retry and logout fallback (lines 43-60), and it is used consistently by pm.vue, classroom.vue, admin and most other pages. +- Typecheck is enforced and currently green: nuxt.config.ts sets typescript.typeCheck: true, and vue-tsc --noEmit passes with zero errors on both .nuxt/tsconfig.app.json and .nuxt/tsconfig.server.json (verified in this review). +- A real test suite exists (80 tests, node:test via tsx, no heavy framework) covering security-relevant server utils (tests/server/auth.test.ts, invitations, decisionSanitizer, runtimeConfig, waitlistReferrals) and shared phraseology logic (tests/shared/radioSpeech.test.ts, sttMatch.test.ts). +- server/utils/auth.ts is a careful hand-rolled JWT implementation: HS256 with timingSafeEqual signature comparison (line 58), algorithm pinning (line 62), token versioning against User.tokenVersion (line 143), scrypt password hashing, and httpOnly/secure refresh cookies. +- Frontend logging discipline: app/pages contain essentially no stray console.log; pm.vue uses a localStorage-gated leveled logger (pmLog, lines 1217-1242) with off/on/verbose modes instead. +- Error handling in the API layer is mostly uniform: 205 createError usages across 53 files with statusCode+statusMessage, and admin/editor routes consistently gate via requireAdmin/requireUserSession helpers. +- server/utils/decisionSanitizer.ts (335 lines) centralizes validation/sanitization of editor payloads instead of scattering it across the seven editor routes that consume it. + +### Findings + +#### QUAL-02 · HIGH · effort M ✓ verified — No ESLint/Prettier configuration and no CI pipeline + +**Files:** `package.json`, `tsconfig.json` + +`git ls-files | grep -iE 'eslint|prettier|editorconfig|husky|lint'` returns nothing and there is no .github/workflows directory. package.json has no lint script and no eslint/prettier devDependencies. Consequences are visible: a failing test at HEAD, an accidental `out.ogg` artifact committed to the repo root (commit message 'iwas was nich tut dazutun'), mixed indentation/quote styles between files (e.g. 4-space server/utils/openai.ts vs 2-space server/utils/auth.ts), and commented-out auth shipping to main. + +**Recommendation:** Add @nuxt/eslint (the official Nuxt 4 ESLint module) with a flat eslint.config.mjs, add `lint` and `typecheck` scripts to package.json, and create a GitHub Actions workflow that runs `yarn install && yarn lint && vue-tsc --noEmit (via nuxt typecheck) && yarn test` on every PR. Optionally add husky+lint-staged for pre-commit. + +#### QUAL-03 · HIGH · effort L ✓ verified — Monolithic page components: classroom.vue 8,614 lines, pm.vue 5,046 lines, editor 3,032 lines + +**Files:** `app/pages/classroom.vue:1502`, `app/pages/pm.vue:1197`, `app/pages/editor/index.vue:1148`, `app/pages/copilot.vue:1`, `app/pages/admin/index.vue:989`, `app/pages/index.vue:861` + +Six pages exceed 1,900 lines. classroom.vue has a 3,567-line script block (lines 1502–5069) plus 3,538 lines of scoped CSS (5071–8609). pm.vue has a 3,047-line script (1197–4244) containing at least 8 unrelated concerns: a debug logger (1217+), ~600 lines of inline simulation scenario data ('Scenario definitions', 1714–2300), a TTS speech queue with radio effects (2321–2660), a PTT pre-recording PCM ring buffer (2992+), frequency presets (3830) and manual frequency entry (3888), plus the backend transmission flow. editor/index.vue (1,811-line script) and admin/index.vue (910-line script) follow the same pattern. These files are effectively unreviewable and any change risks unrelated breakage. + +**Recommendation:** Extract per-concern composables and components. For pm.vue: move the simulation scenario data (lines ~1716–2300) into shared/data/pmSimulationScenarios.ts; extract useAtcSpeechQueue (stopCurrentSpeech/enqueueSpeech/playAudioWithEffects), usePttRecorder (pre-rec ring buffer + MediaRecorder), and useFrequencyTuning (presets + manual entry) into app/composables/. For classroom.vue: move the scoped CSS into app/assets/css (a learn-theme.css already exists) and split the lesson runner, STT readback, and TTS playback into composables/components. For editor/index.vue: split inspector panels into components next to the existing app/components/editor/DecisionNodeCanvas.vue. + +#### QUAL-08 · HIGH · effort S ✓ verified — Commented-out authentication on the OpenAI-billed TTS endpoint + +**Files:** `server/api/atc/say.post.ts:174`, `server/api/atc/say.post.ts:347`, `server/api/atc/ptt.post.ts:128` + +say.post.ts line 174 reads `// const user = await requireUserSession(event);` and line 347 `// user: user._id,` — auth was deliberately disabled and left as commented-out code, so any anonymous client can trigger OpenAI/Speaches TTS generation (and disk writes under storage/atc) at the operator's expense. The companion Whisper endpoint ptt.post.ts uses only the optional getUserFromEvent (line 128) for logging, never requiring a session. Beyond the cost/abuse risk this is classic cruft: dead code that hides the intended behavior. + +**Recommendation:** Either restore `await requireUserSession(event)` in say.post.ts (and pass user._id to TransmissionLog.create) and add the same to ptt.post.ts, or, if anonymous access is intentional for the classroom demo, delete the commented lines and add rate limiting plus an explicit comment documenting the decision. + +**Verifier note:** The Whisper endpoint ptt.post.ts is actually protected: the global middleware server/middleware/auth.global.ts:24 requires a user session for /api/atc/ptt (it is not in the exemption list), and getUserFromEvent at line 128 merely retrieves the already-authenticated user for logging. Also, the anonymous access to /api/atc/say is not caused only by the commented-out line — there is a deliberate middleware exemption at auth.global.ts:9, so restoring requireUserSession in the handler is the only place the check can happen for this route. The exemption appears intentional for the FlightLab no-auth demo (docs/plans/2025-02-13-flightlab-takeoff-implementation.md:7), but it contradicts the public API docs which label the endpoint as protected (app/pages/api-docs.vue:1051). + +#### QUAL-01 · MEDIUM (reported high, adjusted to medium after verification) · effort S ✓ verified — Test suite fails at HEAD (stale assertion after SID-pronunciation change) + +**Files:** `tests/shared/radioSpeech.test.ts:37`, `shared/utils/radioSpeech.ts` + +Running `yarn test` on a clean checkout of main yields 80 tests / 79 pass / 1 fail. tests/shared/radioSpeech.test.ts:37 asserts `assert.equal(sid, 'Mike Alfa Romeo Uniform November seven Foxtrot')` but normalizeRadioPhrase('MARUN 7F') now returns 'Marun seven Foxtrot'. Commit d758873 ('fix(radio): speak SID names and waypoints as words') changed the behavior without updating the test. With no CI (see separate finding), the red suite went unnoticed. + +**Recommendation:** Update the expected value in tests/shared/radioSpeech.test.ts:37 to the new word-based pronunciation ('Marun seven Foxtrot') and adjust the comment above it which still claims SIDs are spelled phonetically. Then keep the suite green by wiring it into CI. + +#### QUAL-04 · MEDIUM · effort M — Radio-audio playback pipeline duplicated in three pages + +**Files:** `app/pages/pm.vue:2432`, `app/pages/classroom.vue:4613`, `app/pages/classroom-introduction.vue:569` + +ensurePizzicato is implemented three times with near-identical bodies (pm.vue 2432–2438, classroom.vue 4613–4619, classroom-introduction.vue 569+), each accompanied by its own AudioContext bootstrap (pm.vue ensureAudioContext:2412, classroom.vue ensureSpeechAudioContext:4591), webkitAudioContext fallback casts, createNoiseGenerators/getReadabilityProfile wiring, and base64-audio playback with radio effects. classroom.vue additionally re-implements /api/atc/say fetching with its own cache (buildSayCacheKey/requestSayAudio:4621+) while pm.vue calls the same endpoint at lines 2533/2610/3613. MediaRecorder-based STT capture against /api/atc/ptt is also duplicated (pm.vue:3190–3199 vs classroom.vue:3672–3677). + +**Recommendation:** Create shared composables: shared/composables/useRadioAudioPlayback.ts (AudioContext + PizzicatoLite loading + playAudioWithEffects with readability profiles) and shared/composables/useAtcTts.ts (calls /api/atc/say, handles caching and abort). The flightlab area already demonstrates the pattern with shared/composables/flightlab/useFlightLabAudio.ts — follow it. Then delete the three local copies. + +#### QUAL-05 · MEDIUM · effort S — Dead code: openaiDecision.ts calls a non-existent endpoint, unused prompt/TTS exports in normalize.ts, out.ogg artifact + +**Files:** `shared/utils/openaiDecision.ts:6`, `server/utils/normalize.ts:50`, `server/utils/normalize.ts:162`, `server/utils/normalize.ts:175`, `out.ogg`, `shared/utils/communicationsEngine.ts:29` + +(1) shared/utils/openaiDecision.ts POSTs to '/api/llm/decide' but no such route exists under server/api (the decision logic moved to the external Python radio backend), and its only export decideNextStateLLM is imported nowhere. (2) server/utils/normalize.ts exports atcSystemPrompt, atcSeedPrompt, atcReplyPrompt, CALLSIGN_MAP, LLM_MODEL and speakATC — grep finds zero usages outside the file; speakATC even writes 'atc.mp3' to cwd and uses an `(normalize as any)` cast; lines 175–204 are a 30-line commented-out usage example. (3) out.ogg in the repo root is a 293-byte JSON error blob ('{"error":true,"message":"[wav @ ...] inv...') committed by commit fe8b762 ('iwas was nich tut dazutun'). (4) communicationsEngine.ts exports FLIGHT_PHASES/COMMUNICATION_STEPS (lines 29, 40) which are referenced nowhere outside the file. + +**Recommendation:** Delete shared/utils/openaiDecision.ts and out.ogg. In server/utils/normalize.ts keep only `normalize` (the OpenAI client), TTS_MODEL and normalizeATC, deleting the prompt builders, speakATC and the example block; ideally also rename the `normalize` client export (see separate finding). Un-export FLIGHT_PHASES/COMMUNICATION_STEPS or remove them if unused internally. + +#### QUAL-06 · MEDIUM · effort S — Helper module without default export lives inside server/api and is registered as a route + +**Files:** `server/api/service/tools/airportGeocode.ts:1`, `server/api/service/tools/airport-geocode.get.ts:3`, `server/api/service/tools/taxiroute.get.ts:4` + +server/api/service/tools/airportGeocode.ts is a 466-line shared helper (Overpass fetching, feature resolution) with no `export default defineEventHandler` — grep for 'export default|defineEventHandler' in the file returns nothing. Because every file under server/api/ becomes a Nitro route, GET /api/service/tools/airportGeocode is registered with an undefined handler and will error at request time. It is imported by airport-geocode.get.ts (line 3–8) and taxiroute.get.ts (line 4). + +**Recommendation:** Move the file to server/utils/airportGeocode.ts (Nitro does not route files there) and update the two imports from './airportGeocode' to '../../../utils/airportGeocode'. Also consolidate the two diverging local parseCoordinate implementations (airport-geocode.get.ts:10-13 vs taxiroute.get.ts:9-14) into the moved module. + +#### QUAL-07 · MEDIUM · effort S — CLAUDE.md describes an architecture that no longer exists + +**Files:** `CLAUDE.md:11`, `CLAUDE.md:20`, `CLAUDE.md:29`, `AGENTS.md:13`, `app/composables/useRadioBackend.ts:39` + +CLAUDE.md states '/server/utils/openai.ts — LLM decision router (routeDecision())' and 'POST /api/llm/decide → routeDecision() selects next state', but routeDecision() does not exist anywhere in the codebase (grep finds it only in the two doc files) and there is no /api/llm/decide route. The actual /pm flow goes through app/composables/useRadioBackend.ts to an external Python backend (`${baseUrl()}/api/radio/session`, radioBackendUrl default http://127.0.0.1:8000). AGENTS.md:13 already flags openai.ts as 'Legacy... No longer called by /pm', contradicting CLAUDE.md. CLAUDE.md:29 also says 'bun run dev' while the repo pins yarn@4.9.4 via packageManager and .yarnrc.yml. Decision trees are 'fetched via /api/decision-flows/runtime' — that route only exists on the Python backend, not in server/api. + +**Recommendation:** Rewrite CLAUDE.md's 'Live ATC Flow' section to describe the Python radio-backend flow (useRadioBackend.createSession/transmit, NUXT_PUBLIC_RADIO_BACKEND_URL), remove the routeDecision/api-llm-decide references, and align the Commands section with yarn. Keep CLAUDE.md and AGENTS.md from drifting by making one include or reference the other. + +#### QUAL-09 · MEDIUM · effort M — Inconsistent error handling: 200-with-error-object routes, mixed German/English messages, divergent error shapes + +**Files:** `server/api/service/tools/taxiroute.get.ts:36`, `server/api/bridge/me.get.ts:14`, `server/api/bridge/connect.post.ts:12`, `server/utils/auth.ts:19`, `server/utils/auth.ts:180`, `server/api/atc/say.post.ts:402`, `server/api/service/cron/waitlist-drip.get.ts:185` + +Most routes correctly `throw createError({statusCode, statusMessage})`, but: taxiroute.get.ts returns plain objects like `return { error: 'missing_origin', details: ... }` with HTTP 200 (lines 36–41), so clients cannot rely on status codes; bridge/me.get.ts uses `message:` where the six sibling bridge routes use `statusMessage:` for the same 401 ('x-bridge-token header fehlt oder ist ungültig.'); user-facing error strings mix German ('Administratorrechte erforderlich' auth.ts:180, 'bitte JWT_SECRET in .env setzen' auth.ts:19, bridge messages) and English ('Authentication required' auth.ts:158); say.post.ts:402-405 wraps every failure as 500 and interpolates the raw internal error message (`TTS generation failed: ${err?.message || err}`) into the client-visible statusMessage; the cron route logs results via bare console.log (waitlist-drip.get.ts:185-189) while everything else is silent or uses console.warn. + +**Recommendation:** Adopt one convention: always throw createError with statusCode + English statusMessage; convert taxiroute's error returns to createError(400/404); change bridge/me.get.ts `message:` to `statusMessage:`; centralize the repeated bridge-401 in server/utils/bridge.ts as a `requireBridgeToken(event)` helper (the null-check + throw is copy-pasted in 7 files); stop echoing internal err.message to clients in say.post.ts/ptt.post.ts (log it server-side, return a generic message). + +#### QUAL-10 · MEDIUM · effort L — TypeScript strict mode disabled across app and server + +**Files:** `nuxt.config.ts:6`, `nuxt.config.ts:98`, `types/nodemailer.d.ts:1`, `shared/types/llm.ts:5`, `shared/utils/communicationsEngine.ts:314` + +nuxt.config.ts sets `typescript: { strict: false, ... noUncheckedIndexedAccess: false, noImplicitOverride: false, vueCompilerOptions: { strictTemplates: false } }` and mirrors the same under nitro.typescript (line 98+). vue-tsc passes on both .nuxt/tsconfig.app.json and tsconfig.server.json today, but only because strict is off. The codebase carries ~278 `any` occurrences across 60+ files: shared/types/llm.ts types the LLM boundary as `state: any` / `candidates: Array<{ id: string; state: any ... }>`, communicationsEngine.ts has 31 `as any` casts (e.g. lines 314–348 reading flags via `(baseFlags as any).in_air`), and types/nodemailer.d.ts stubs the whole nodemailer module as `any` ('declare module nodemailer { const nodemailer: any ... }'). + +**Recommendation:** Enable strict incrementally: (1) flip `typescript.strict: true` and fix the resulting errors module-by-module starting with server/ (smaller surface, passes today); (2) replace types/nodemailer.d.ts with the official @types/nodemailer devDependency; (3) give shared/types/llm.ts a real RuntimeDecisionState type for `state` (it already exists in shared/types/decision.ts); (4) in communicationsEngine.ts introduce a typed EngineFlags interface instead of `(baseFlags as any).x` casts. + +#### QUAL-11 · MEDIUM · effort M — Duplicated route logic: two SimBrief fetchers, repeated editor-flow boilerplate, two OpenAI client singletons + +**Files:** `server/api/copilot/simbrief.get.ts:1`, `server/api/learn/simbrief.get.ts:1`, `server/api/editor/flows/[slug]/index.put.ts:17`, `server/api/editor/flows/[slug]/nodes.post.ts:24`, `server/utils/openai.ts:7`, `server/utils/normalize.ts:7` + +(1) copilot/simbrief.get.ts and learn/simbrief.get.ts both call https://www.simbrief.com/api/xml.fetcher.php but with incompatible conventions: copilot accepts `username|userid` and uses a `(globalThis as any).$fetch` cast with ignoreResponseError; learn accepts `userId|userid`, builds a URL object and applies a 10s timeout. (2) Seven editor routes repeat the same prologue — `requireAdmin`, slug param trim/validate ('Missing flow identifier' appears 7 times in the statusMessage survey), `DecisionFlow.findOne({slug})`, 404 'Decision flow not found' (8 occurrences) — e.g. flows/[slug]/index.put.ts:17-26 and nodes.post.ts:24-33. (3) Two module-level OpenAI clients exist: server/utils/openai.ts (lazy ensureOpenAI) and server/utils/normalize.ts:16 (`export const normalize = new OpenAI(...)`) which is constructed eagerly at import time from getServerRuntimeConfig() called at module scope (line 7), bypassing the lazy missing-key error path and confusingly named `normalize`. + +**Recommendation:** (1) Extract a server/utils/simbrief.ts `fetchSimbriefOfp({username?, userid}, {timeoutMs})` used by both routes; standardize the query param. (2) Add `resolveFlowFromEvent(event)` to server/services/decisionFlowService.ts returning the flow or throwing the 400/404, and use it in all editor routes; move sanitizeStringArray there too. (3) Delete the eager client in normalize.ts and reuse getOpenAIClient() from server/utils/openai.ts in say.post.ts. + +#### QUAL-12 · MEDIUM · effort S — Config hygiene: .env.example incomplete, real-looking secrets/defaults committed + +**Files:** `.env.example:41`, `nuxt.config.ts:48`, `.env.example:1` + +.env.example omits keys the runtime reads: OPENAIP_API_KEY (used by runtimeConfig.openaipApiKey and airports/[icao]/frequencies.get.ts), KPI_CRON_SECRET (weekly-kpi-report.get.ts:8), NUXT_PUBLIC_RADIO_BACKEND_URL (the /pm backend!), MONGODB_URI variants and NUXT_IMAGE_PROVIDER. Conversely it commits what looks like a real credential: `MANUAL_INVITE_PASSWORD=pm.local@zghl.de` (line 41). nuxt.config.ts:48 hardcodes a personal home-automation webhook as the production default: `domeLightWebhookUrl: process.env.DOME_LIGHT_WEBHOOK_URL || 'https://home.io.faktorxmensch.com/api/webhook/lidl_stab_3modi_8492'` — an open-source AGPL project should not default to the maintainer's private endpoint. + +**Recommendation:** Add the missing keys to .env.example with placeholder values and a one-line comment each; replace MANUAL_INVITE_PASSWORD's value with 'changeme'; change the domeLightWebhookUrl default to empty string and guard the webhook call on truthiness (verify in server/api/bridge/data.post.ts). If the committed manual-invite password was ever live, rotate it. + +#### QUAL-13 · LOW · effort S — Stale TODO encoding feature requirements in German inside route source + +**Files:** `server/api/service/tools/taxiroute.get.ts:6` + +The only TODO in the codebase is a 4-line German paragraph addressed to a specific person: `// TODO @najajan-de: für die taxiroute soll gelten: wenn es um wegbeschreibungen mit dem name eines runways ...` describing runway-endpoint selection logic. Requirements buried in source comments addressed at individuals get lost and don't belong in an open-source repo's hot path. + +**Recommendation:** Move the requirement into a GitHub issue (the repo has a roadmap system) and replace the comment with a one-line `// TODO(#): runway start/end node selection` reference, in English. + +#### QUAL-14 · LOW · effort S — Frontend auth-fetch wrapper bypassed in bridge/connect.vue and flightlab/takeoff.vue + +**Files:** `app/pages/bridge/connect.vue:587`, `app/pages/bridge/connect.vue:673`, `app/pages/flightlab/takeoff.vue:1`, `app/composables/useApi.ts:18` + +useApi() centralizes the Bearer header, 401-refresh-retry and logout fallback, and most pages use it. But bridge/connect.vue hand-rolls four raw `$fetch` calls with manual `Authorization: Bearer ${accessToken.value}` headers (lines 587-590, 618-621, 673, 742) and flightlab/takeoff.vue does the same once. These call sites silently lose the automatic token-refresh-on-401 behavior, so an expired access token shows as a hard failure instead of refreshing. + +**Recommendation:** Replace the raw $fetch calls in bridge/connect.vue and flightlab/takeoff.vue with useApi().get/post so the 401-refresh path applies uniformly; useApi already supports custom headers if the bridge routes need extra ones. + +--- + +## Tests & CI + +OpenSquawk has a real but narrow test suite: 14 files / 80 tests using zero-dependency node:test via tsx (package.json:16), heavily weighted toward speech/phraseology string processing (radioSpeech, sttMatch, scenario) and small server utilities (auth utils, validation, bridge token/log, sanitizers). The suite currently FAILS on main (1 known-stale assertion committed two days ago) and there is no CI of any kind — no .github/workflows, no GitLab CI, no lint/typecheck scripts, and the nixpacks deploy config builds without running tests — so nothing enforces green. The most critical product paths are entirely untested: the 1,568-line communicationsEngine state machine, decisionFlowService tree building, all auth/registration/reset endpoints, and invitation redemption. The documented LLM routing path (routeDecision() / POST /api/llm/decide) no longer exists in the server at all, which makes the CLAUDE.md test-planning map misleading. + +### What's done well + +- A lightweight, fast test setup with zero test-framework dependencies: node:test + tsx with a dedicated tsconfig (package.json:16, tsconfig.tests.json), full run completes in ~0.6s — very low friction for adding tests. +- Excellent, realistic coverage of the STT matching layer: tests/shared/sttMatch.test.ts covers Whisper-mangled airline names ("Loftansa", "Speed bird"), digit folding, squawk collapse, false-positive guards, and full readback scenarios — this is genuinely hard logic and it is well tested. +- Phraseology/TTS normalization is well covered: tests/shared/radioSpeech.test.ts exercises full VATSIM ATIS normalization end-to-end; tests/smoke/normalize.smoke.test.ts covers server prompt builders and normalizeATC. +- Clean seam for Nuxt runtime config in tests: tsconfig.tests.json maps #imports to tests/stubs/nuxt-imports.ts, letting server/utils code (e.g. server/utils/auth.ts:17) run under plain node:test. +- Edge-case-conscious utility tests exist: tests/server/runtimeConfig.test.ts covers env parsing, caching, and invalid-value fallbacks; tests/server/decisionSanitizer.test.ts covers type coercion of editor payloads; tests/server/auth.test.ts covers token-version mismatch producing 401. +- A working precedent for handler-level testing exists (tests/server/bridgeMe.handler.test.ts tests the real server/api/bridge/me.get handler including 401 and populated/unpopulated token states), and server/api/bridge/data.post.ts:4 exports resolveDomeLightMode specifically so it can be unit tested. + +### Findings + +#### TEST-01 · HIGH · effort S ✓ verified — Test suite is red on main: known-failing radioSpeech SID test committed without updating the assertion + +**Files:** `tests/shared/radioSpeech.test.ts:37`, `shared/utils/radioSpeech.ts:311` + +Running `yarn test` exits 1: 79/80 pass, 1 fails. The test expects the old letter-by-letter SID spelling: `assert.equal(sid, 'Mike Alfa Romeo Uniform November seven Foxtrot')` but the code now (intentionally) produces 'Marun seven Foxtrot'. Commit d758873 (2026-06-10) changed sidSuffixSpeak to speak SID basenames as words and its commit message explicitly admits: "Note: tests/radioSpeech 'normalizes SID suffix and METAR data' still expects the old spelled-out SID behavior and fails until updated." A permanently red suite destroys the regression signal — any new failure is indistinguishable from the known one. + +**Recommendation:** Update tests/shared/radioSpeech.test.ts:37 to assert the new intended behavior (`assert.equal(sid, 'Marun seven Foxtrot')`) and add a second case asserting a 5-letter waypoint (e.g. SULUS) is spoken as a word, matching the design in shared/utils/radioSpeech.ts:311 (sidSuffixSpeak). Then adopt the rule that behavior changes land with their test updates in the same commit — enforced by the CI gate from the next finding. + +#### TEST-02 · HIGH (reported critical, adjusted to high after verification) · effort S ✓ verified — No CI pipeline of any kind — no automated test, typecheck, or lint gate + +**Files:** `package.json:7-17`, `nixpacks.toml:1-22` + +There is no .github/ directory, no .gitlab-ci.yml, no .circleci, and the only git hooks are git-lfs stock hooks (.git/hooks/pre-push is `git lfs pre-push`). The deploy config (nixpacks.toml) runs only `yarn install --immutable` and `yarn build` — tests are never executed anywhere automatically. package.json has no `lint` or `typecheck` script (vue-tsc is installed at package.json:46 but unused), and there is no ESLint/Prettier/Biome config in the repo. The direct consequence is visible: a knowingly-failing test has lived on main for two days, and broken code can ship to production with zero gates. + +**Recommendation:** Add .github/workflows/ci.yml triggered on push/PR to main with one job: checkout, setup-node (node-version-file: .nvmrc), `corepack enable`, `yarn install --immutable`, `yarn test`. Add a second step `yarn nuxi typecheck` (or `vue-tsc --noEmit -p tsconfig.tests.json` for server/shared only) — run it with continue-on-error: true initially if it is currently red, then promote to blocking once clean. Add `"typecheck": "nuxt typecheck"` to package.json scripts. Optionally add a deploy-blocking branch protection rule on main requiring the workflow. + +**Verifier note:** The finding is factually accurate, but 'critical' overstates it. The failing test is a stale expectation after an intentional behavior change (the shipped code works as intended; the test was knowingly left un-updated), so there is no active production defect — the issue is a missing quality gate, a process/infrastructure gap. That warrants high (broken code could ship to production with zero automated checks, and main already tolerates a red test suite), but not critical, which is typically reserved for active security holes, data loss, or production-breaking bugs. + +#### TEST-03 · HIGH · effort M ✓ verified — communicationsEngine.ts (1,568 lines) — the core live-ATC state machine — has zero tests + +**Files:** `shared/utils/communicationsEngine.ts:835`, `shared/utils/communicationsEngine.ts:887`, `shared/utils/communicationsEngine.ts:984`, `shared/utils/communicationsEngine.ts:1213`, `shared/utils/communicationsEngine.ts:427`, `shared/utils/communicationsEngine.ts:170` + +The state machine that drives the entire /pm live ATC experience is completely untested: buildLLMContext (line 835, including the subtle check_readback/monitor candidate-expansion logic at 849-873), applyLLMDecision (line 887: variable/flag merging, flow activation, off_schema/radio_check counters, resume_previous stack handling), collectAtcStatesUntilPilotTurn (line 984: the auto-advance walk with maxHops/visited loop guards), evaluateAutoTransitions (1213), setActiveFlow/flow stacking (427), and renderTpl (170). The module imports only `ref, computed, readonly, reactive` from 'vue' (line 2), so it runs fine under plain node:test with an in-memory fixture tree — no DOM or Nuxt needed. fetchRuntimeTree (line 601) is the only network-touching function and is bypassable via loadRuntimeTree/loadRuntimeSystem (lines 573-577). + +**Recommendation:** Create tests/shared/communicationsEngine.test.ts using a small hand-written RuntimeDecisionSystem fixture (3 flows, ~10 states covering pilot/atc/system roles, ok_next/bad_next/timer_next/auto_transitions). Highest-value cases: (1) loadRuntimeSystem + moveTo + variable interpolation via renderTpl; (2) applyLLMDecision applies updates/flags, moves to next_state, and radio_check suppresses the move; (3) collectAtcStatesUntilPilotTurn walks an atc→system→atc chain, stops at the pilot state, and the visited-set breaks a cycle; (4) buildLLMContext expands candidates when all are check_readback states; (5) setActiveFlow with stack push + resume_previous restores the prior flow snapshot. Use engine.loadRuntimeSystem(fixture) to avoid any fetch. + +#### TEST-04 · HIGH · effort M ✓ verified — Auth endpoints and refresh-token rotation untested (login, refresh, register, reset-password, forgot-password) + +**Files:** `server/utils/auth.ts:192-217`, `server/api/service/auth/login.post.ts:10-45`, `server/api/service/auth/refresh.post.ts:1-9`, `server/api/service/auth/register.post.ts:43-81`, `server/api/service/auth/reset-password.post.ts:25-44` + +tests/server/auth.test.ts covers only the utility layer (hashing, token claims, resolveUserFromToken, requireUserSession). Untested security-critical logic: rotateRefreshToken (auth.ts:192-217 — manual cookie-header parsing, refresh-type check, tokenVersion mismatch, cookie clearing on failure); login.post.ts (401 on unknown user/bad password, lastLoginAt update); register.post.ts invitation redemption (lines 48-57: not-found/already-used/expired code branches; lines 74-76 marking the code used — note there is no atomicity test for double-redemption); reset-password.post.ts (sha256 token-hash lookup, expiry/usedAt single-use check at line 28, tokenVersion bump at line 39 that invalidates all sessions, deletion of sibling tokens at line 44). verifyJwtToken negative paths (tampered signature, non-HS256 alg header, expired exp — auth.ts:51-68) are also untested. + +**Recommendation:** Extend tests/server/auth.test.ts with rotateRefreshToken cases (no cookie → 401, access-token-in-cookie → 401, version mismatch → 401 + cookie cleared, happy path) by passing a fake H3 event with node.req.headers.cookie and patching User.findById, following the existing pattern. Add tests/server/registerInvitation.test.ts and tests/server/resetPassword.test.ts that import the handlers with the existing `globalThis.defineEventHandler = (h) => h` shim and monkey-patched models (the bridgeMe.handler.test.ts pattern), covering: used/expired/missing invitation → 400/404, valid code creates user and stamps usedBy; reset token reuse → 400 and tokenVersion increments exactly once. + +#### TEST-05 · MEDIUM · effort M — decisionFlowService runtime-tree building untested, and its mongoose-document coupling blocks easy testing + +**Files:** `server/services/decisionFlowService.ts:119-160`, `server/services/decisionFlowService.ts:162-191`, `server/services/decisionFlowService.ts:230-269` + +The service that converts MongoDB flow/node documents into the RuntimeDecisionTree consumed by the live ATC engine has no tests: toRuntimeTransitions/toRuntimeTimers/toRuntimeAutoTransitions (ordering by `order`, ok/bad/timer/auto type filtering, lines 119-160), serializeRuntimeState (162-191), and buildRuntimeDecisionSystem's main-flow selection chain `preferredMain || fallbackMain || order[0]` (lines 260-262). A bug here silently corrupts every live session. Testability problem: serializeRuntimeState and serializeNodeDocument take DecisionNodeDocument and call `node.toObject(...)` (lines 46, 163), so they cannot be called with plain fixture objects; toRuntimeTransitions and friends are pure but module-private (not exported). + +**Recommendation:** Export toRuntimeTransitions, toRuntimeTimers, toRuntimeAutoTransitions and refactor serializeRuntimeState to accept a plain object (`const obj = typeof node.toObject === 'function' ? node.toObject({virtuals:false}) : node`), then add tests/server/decisionFlowService.test.ts with plain-object node fixtures asserting: transition type filtering (auto included in `next` when includeAuto), order sorting, timer_next afterSeconds mapping, auto_transitions trigger passthrough, and main-flow selection (isMain wins over the 'icao_atc_decision_tree' fallback over first-by-updatedAt). Patch DecisionFlow.find/DecisionNode.find statics for the buildRuntimeDecisionSystem test, as done in tests/server/subscribers.test.ts. + +#### TEST-06 · MEDIUM · effort S — CLAUDE.md/AGENTS.md describe an LLM routing path (routeDecision, /api/llm/decide) that does not exist; dead client code would 404 + +**Files:** `shared/utils/openaiDecision.ts:6`, `server/utils/openai.ts:1-30`, `CLAUDE.md` + +CLAUDE.md states 'POST /api/llm/decide → routeDecision() selects next state' and names routeDecision() in server/utils/openai.ts, but server/utils/openai.ts only exports getOpenAIClient() (30 lines, no routeDecision anywhere in the repo — grep matches only CLAUDE.md/AGENTS.md), and there is no server/api/llm/ directory at all. shared/utils/openaiDecision.ts:6 still calls `$fetch('/api/llm/decide', ...)` which would 404 — its export decideNextStateLLM has zero callers. This matters for test planning: anyone (human or agent) writing tests for the documented 'routeDecision LLM routing' critical path will look for code that is not there; the decision step has evidently moved to an external Python runtime (per docs/plans/2026-05-06-pm-python-runtime-contract.md and the `sessionId` comment in server/api/atc/ptt.post.ts:20). + +**Recommendation:** Delete shared/utils/openaiDecision.ts (unused, points at a nonexistent endpoint) and update the 'Live ATC Flow' section of CLAUDE.md/AGENTS.md to describe the actual path (/api/atc/ptt for STT, external runtime for decisions, communicationsEngine.applyLLMDecision for state application). Then the testable server-side LLM surface is just getOpenAIClient + /api/atc/ptt and /api/atc/say. + +#### TEST-07 · MEDIUM · effort M — Brittle test seams: globalThis.defineEventHandler hack and ad-hoc mongoose monkey-patching, no shared test helpers or DB harness + +**Files:** `tests/server/bridgeMe.handler.test.ts:18`, `tests/server/auth.test.ts:78-79`, `tests/server/subscribers.test.ts:9-17` + +Handler tests rely on `(globalThis as any).defineEventHandler = (handler: any) => handler` set immediately before a dynamic import (bridgeMe.handler.test.ts:18), which only works because the handler imports everything else explicitly from 'h3'; any handler using another Nitro auto-import (readBody, getQuery, etc. without explicit import) will crash at import time. Model mocking is done by reassigning statics in each test with try/finally restoration (`(User as any).findById = async ...` at auth.test.ts:79; UpdateSubscriber.findOne/create at subscribers.test.ts:13-17) — repeated boilerplate with no type safety, and a forgotten restore leaks across tests in the same file. There is no mongodb-memory-server, no fixture/factory helpers, and no shared mock-event builder (createEvent is duplicated in bridgeMe.handler.test.ts:7 and inline in auth.test.ts:82-91). + +**Recommendation:** Create tests/helpers/index.ts exporting: (1) makeEvent(headers?, body?) building the fake H3 event shape used today; (2) stubModel(Model, methods) returning a restore function (or use node:test's t.mock.method) to replace the try/finally boilerplate; (3) a registerNitroGlobals() that installs defineEventHandler/readBody shims once before handler imports. For endpoint tests that exercise real queries (register/invitation race, reset-password sibling deletion at reset-password.post.ts:44), add mongodb-memory-server as a devDependency and a tests/helpers/db.ts that connects mongoose to it in before/after hooks. + +#### TEST-09 · MEDIUM · effort L — Critical frontend logic locked inside 5,000-14,000-line page components with no component test infrastructure + +**Files:** `app/pages/pm.vue:2533-3613`, `app/pages/classroom.vue:1` + +pm.vue is 5,046 lines and classroom.vue is 8,614 lines. The live ATC orchestration in pm.vue — /api/atc/say TTS calls (2533, 2610, 3613), PTT submission (3294-3316), VATSIM flight-plan and METAR fetching (2811, 3499), frequency resolution (3445) — is embedded directly in page components, so none of it can be unit tested. The repo has no vitest/@vue/test-utils/happy-dom setup; node:test cannot mount SFCs. Recent regressions fixed in commits like 881a085 ('fix: interrupt ATC speech on frequency change, fix PTT stack overflow') are exactly the kind of logic that has no test today and will regress again. + +**Recommendation:** Do not try to test the .vue files directly. Extract the non-UI orchestration from pm.vue into plain composables under app/composables/ or shared/utils/ (e.g. useAtcSpeechQueue for the say/TTS scheduling + interruption logic, useFrequencyResolver for the airport-frequency selection) with injected fetch/api dependencies, then test those with the existing node:test runner — same approach that already works for communicationsEngine. Defer a full vitest+@vue/test-utils setup until the extracted-composable approach is exhausted. + +#### TEST-08 · LOW · effort S — Pure validation logic inside /api/atc/ptt.post.ts is untested and not exported + +**Files:** `server/api/atc/ptt.post.ts:45-67`, `server/api/atc/ptt.post.ts:80-92` + +decodeAudioPayload (lines 51-67: base64 regex validation, empty-payload 400, 2 MB limit → 413) and resolveAudioFormat (45-49: whitelist with wav fallback) are pure functions guarding the PTT upload path but are module-private and untested. The request-validation branch (lines 82-87 requiring audio/moduleId/lessonId) and the sessionId fallback logic (131-132) are likewise untested. The repo already has a precedent for exporting handler-internal pure functions for tests: server/api/bridge/data.post.ts exports resolveDomeLightMode, tested in tests/server/bridgeDataDomeLight.test.ts. + +**Recommendation:** Export decodeAudioPayload and resolveAudioFormat from server/api/atc/ptt.post.ts (same pattern as resolveDomeLightMode) and add tests/server/pttValidation.test.ts asserting: whitespace-stripped base64 accepted, invalid characters → 400, decoded-empty → 400, >2 MB → 413, unknown format → 'wav'. + +#### TEST-10 · LOW · effort S — No coverage measurement or test documentation; AGENTS.md/CLAUDE.md never mention how to run tests + +**Files:** `package.json:16`, `AGENTS.md`, `CLAUDE.md` + +The only documented commands in CLAUDE.md are `bun run dev` and the decision-tree import; neither CLAUDE.md nor AGENTS.md mentions `yarn test` (grep for 'test' in AGENTS.md returns nothing), so agents and contributors do not know a suite exists or that it must stay green. There is no coverage reporting (`node --experimental-test-coverage` is unused), so the large blind spots identified above are invisible in day-to-day work. Also note: package.json engines pins node 22.x while the local environment runs 24.x — the suite passes either way, but CI should pin via .nvmrc to keep results comparable. + +**Recommendation:** Add a `## Testing` section to CLAUDE.md and AGENTS.md: 'Run `yarn test` (node:test via tsx); the suite must pass before committing; add tests next to behavior changes.' Add a `test:coverage` script: `tsx --tsconfig tsconfig.tests.json --test --experimental-test-coverage "tests/**/*.test.ts"` and surface it in the CI workflow as an informational step. + +--- + +## Operations & Scaling + +OpenSquawk's build pipeline is reasonably reproducible (pinned Node 22.x, Yarn 4 via Corepack, --immutable installs, a well-documented nixpacks.toml), but the runtime is designed for exactly one long-lived process: bridge telemetry, FlightLab WebSocket sessions, bridge debug logs, and dome-light state all live in module-level Maps that break with 2+ instances and vanish on every deploy. Operational hygiene has significant gaps: the waitlist-drip cron endpoint is publicly callable with no secret and no idempotent claim of work, /api/atc/say is unauthenticated and proxies paid TTS with no rate limiting, ffmpeg is required by the PTT path but absent from the nixpacks package list, and there is no structured logging, request IDs, or error reporting beyond ~28 console.* calls. Environment misconfiguration mostly fails silently (Mongo URI defaults to localhost, JWT secret only throws on first auth request, OpenAI key only warns). + +### What's done well + +- nixpacks.toml is unusually well documented — it explains the exact failure mode it prevents (Yarn 1 fallback producing an empty .output) and uses corepack + `yarn install --immutable`, giving reproducible installs (nixpacks.toml:1-22) +- Toolchain is fully pinned: `"engines": {"node": "22.x"}` and `"packageManager": "yarn@4.9.4"` in package.json:36-39, matching nixPkgs nodejs_22 +- A complete .env.example exists covering Mongo, JWT, OpenAI, TTS providers, SMTP and cache dirs, and README.md documents ffmpeg and MongoDB as prerequisites (README.md:29, 49-65) +- server/api/atc/ptt.post.ts has solid resource hygiene: base64 validation, a 2 MB payload cap (line 42), and temp-file cleanup on both success (lines 122-125) and error (lines 153-154) paths +- Weekly KPI delivery inside the drip cron is idempotent via a `KpiReportDelivery.exists({ weekKey })` check backed by a unique index (server/api/service/cron/waitlist-drip.get.ts:64-67, server/models/KpiReportDelivery.ts:13) +- External fetches use AbortController timeouts and clear them in finally blocks (server/api/learn/simbrief.get.ts:17-43, server/api/classroom/speech-server-health.get.ts:19-34) +- FlightLab/ATIS TTS responses are content-hash cached on disk, avoiding repeated paid TTS for identical broadcasts (server/api/atc/say.post.ts:44-56, 242-249) +- Server runtime config is centralized with type coercion and a test-friendly cache reset (server/utils/runtimeConfig.ts:53-90) +- Admin session-log endpoints exist with pagination and per-session drill-down for debugging user sessions (server/api/admin/logs/sessions.get.ts:42-107) + +### Findings + +#### OPS-02 · HIGH · effort M ✓ verified — Unauthenticated, non-idempotent cron endpoint /api/service/cron/waitlist-drip sends real emails + +**Files:** `server/api/service/cron/waitlist-drip.get.ts:96-196`, `server/middleware/auth.global.ts:12-14`, `server/api/service/cron/weekly-kpi-report.get.ts:8-15` + +auth.global.ts exempts everything under `/api/service/` (`if (url.pathname.startsWith('/api/service/')) { return }`), and waitlist-drip.get.ts has no secret check at all — anyone who discovers the URL can trigger invitation/feedback email sends. By contrast weekly-kpi-report.get.ts checks `KPI_CRON_SECRET`, but only `if (secret)` — when the env var is unset the endpoint is fully public too. The drip loop is also racy under concurrent invocation: it does `WaitlistEntry.find(...)` then per-entry `entry.invitationSentAt = sentAt; await entry.save()` (lines 135-136) — two overlapping requests both read the same candidates before either saves, producing duplicate invitation codes and duplicate emails. Nothing in the repo schedules these endpoints (no nitro scheduledTasks, no croner usage in app code), so scheduling depends on an undocumented external pinger. + +**Recommendation:** Require a shared secret (header or query, compared with timingSafeEqual) on both cron endpoints and fail closed when the secret env var is missing. Make the drip idempotent under concurrency by atomically claiming entries: `WaitlistEntry.findOneAndUpdate({ _id, invitationSentAt: null }, { $set: { invitationSentAt: now } })` and only send mail when the claim succeeded. Document the external cron schedule (or switch to Nitro scheduledTasks with a Mongo-based lock) in README. + +#### OPS-03 · HIGH · effort M ✓ verified — /api/atc/say is unauthenticated and there is no rate limiting anywhere — unbounded paid TTS/LLM cost exposure + +**Files:** `server/middleware/auth.global.ts:9-11`, `server/api/atc/say.post.ts:160-282`, `server/api/atc/say.post.ts:174` + +auth.global.ts explicitly exempts the TTS endpoint: `if (url.pathname.startsWith('/api/atc/say')) { return }`, and inside the handler the session check is commented out: `// const user = await requireUserSession(event);` (say.post.ts:174). Every anonymous POST triggers a paid OpenAI TTS call (`normalize.audio.speech.create`, line 272) unless a local provider is configured. A ripgrep for `rateLimit|rate-limit|429` across server/** returns zero matches — no endpoint in the app has any rate limiting, including login/register and the OpenAI Whisper transcription path. The disk-backed cache only applies to `tag: flightlab|atis` requests, so arbitrary text always hits the provider. A single curl loop can run up the OpenAI bill and fill `.cache/flightlab-tts`/storage with attacker-chosen content. + +**Recommendation:** Remove the `/api/atc/say` exemption from auth.global.ts (the say endpoint already accepts an Authorization header from the app's api client) and re-enable `requireUserSession`. Add a simple per-user/per-IP token-bucket rate limit middleware (in-memory is acceptable single-instance; note the multi-instance caveat) applied to /api/atc/*, /api/service/auth/*, and the SimBrief proxies. + +**Verifier note:** Two small inaccuracies, neither material: (a) the endpoint triggers paid TTS only — no LLM call occurs in say.post.ts (the LLM router /api/llm/decide is a separate endpoint that IS behind requireUserSession), so 'TTS/LLM cost exposure' should read 'TTS cost exposure'; (b) for non-flightlab/atis requests, the `storage/atc` audio/json files referenced in the response metadata are never actually written (fileOut/fileJson are computed but no writeFile occurs at say.post.ts:288-316), so disk-fill is limited to `.cache/flightlab-tts` when the attacker sets tag=flightlab|atis — however every anonymous request does create an unbounded MongoDB TransmissionLog document (say.post.ts:346), so the resource-exhaustion point stands in modified form. + +#### OPS-01 · MEDIUM (reported critical, adjusted to medium after verification) · effort L ✓ verified — All bridge/FlightLab runtime state is in-process memory — breaks with 2+ instances and is wiped on every deploy + +**Files:** `server/utils/flightlabTelemetry.ts:10-35`, `server/api/flightlab/ws.ts:20-32`, `server/utils/bridgeLog.ts:12-15`, `server/api/bridge/data.post.ts:12`, `server/api/bridge/live.get.ts:23` + +Four independent module-level stores hold live user state: `const sessions = new Map()` and `const userSessionMap = new Map()` (ws.ts:20-23), the singleton `flightlabTelemetryStore` ("Singleton — shared across all server handlers", flightlabTelemetry.ts:35), the bridge debug ring buffer `const logStore = new Map()` (bridgeLog.ts:15), and `const lastDomeLightModeByToken = new Map(...)` (data.post.ts:12). The telemetry pipeline is POST /api/bridge/data -> in-memory store -> in-process subscription -> WebSocket peers. Behind a load balancer with 2 instances, the simulator's POST and the browser's WebSocket land on different processes and telemetry silently never arrives; /api/bridge/log returns empty for the same reason. On every deploy/restart, FlightLab session phase/history, telemetry, and bridge logs are lost and dome-light dedup state resets (causing one duplicate webhook call per token). + +**Recommendation:** Short term: document and enforce a single-instance deployment constraint (e.g. replicas=1 in the deploy config) and add it to README. Medium term: move telemetry and session state to shared infrastructure — last-telemetry per user into a Mongo collection or Redis key with TTL, and WS fan-out via Redis pub/sub (crossws supports custom adapters); persist FlightLab session state (phase, history) in Mongo keyed by session code so a restart resumes instead of resetting. + +**Verifier note:** The technical claim is fully accurate, but this is a latent scaling constraint rather than a current-production-breaking issue. The repo's deployment (nixpacks.toml, single `node .output/server/index.mjs` process) is single-instance with no evidence of load balancing or replicas, so nothing is broken today. Accurate framing: "Bridge/FlightLab runtime state is in-process only; horizontal scaling (2+ instances) would silently break telemetry delivery and bridge logs, and deploys/restarts reset live FlightLab session state and the debug log. Document the single-instance constraint now; move state to Mongo/Redis before scaling." + +#### OPS-04 · MEDIUM (reported high, adjusted to medium after verification) · effort S ✓ verified — ffmpeg is required at runtime but not installed by the nixpacks build + +**Files:** `nixpacks.toml:7-8`, `server/api/atc/ptt.post.ts:69-77`, `server/api/atc/ptt.post.ts:100-106`, `README.md:29` + +nixpacks.toml installs only Node: `[phases.setup] nixPkgs = ["nodejs_22"]`, but README.md:29 states ffmpeg "must be on the system PATH" and ptt.post.ts shells out to it (`await sh("ffmpeg", [...])`). In the nixpacks-built container the conversion fails on every non-wav PTT upload and is silently swallowed: `catch (err) { console.warn('FFmpeg conversion failed, using original audio:', err) }` — the original webm/ogg blob is then sent to Whisper with whatever the client recorded, degrading transcription reliability without any operator-visible signal beyond a per-request warn. server/utils/radio.ts (applyRadioEffect) also depends on ffmpeg, and `fluent-ffmpeg` is declared in package.json:23 but never imported anywhere. + +**Recommendation:** Add ffmpeg to the build: `nixPkgs = ["nodejs_22", "ffmpeg"]` in nixpacks.toml. Add a startup check (nitro plugin) that logs an explicit error if `ffmpeg -version` fails in production. Remove the unused `fluent-ffmpeg` and `@types/fluent-ffmpeg` dependencies, or actually use them. + +**Verifier note:** ffmpeg is indeed missing from the nixpacks build and the conversion failure is silently swallowed, but PTT does not functionally break: the default /pm path sends WAV (no ffmpeg needed), and webm/ogg fallback uploads are accepted natively by OpenAI whisper-1. The claim of degraded transcription reliability is speculative; actual breakage occurs only with non-OpenAI STT backends. The unused fluent-ffmpeg dependency and dead applyRadioEffect code are accurately reported. + +#### OPS-05 · MEDIUM · effort M — Missing/invalid environment variables fail silently or late instead of at startup + +**Files:** `nuxt.config.ts:50-56`, `server/utils/auth.ts:16-25`, `server/utils/runtimeConfig.ts:60-64`, `server/api/service/cron/weekly-kpi-report.get.ts:8`, `server/utils/notifications.ts:44-47` + +There is no startup validation of required configuration. `MONGODB_URI` silently defaults to `mongodb://127.0.0.1:27017/opensquawk` (nuxt.config.ts:55) — a production deploy with a typoed var connects to a nonexistent local Mongo and only surfaces as request-time mongoose errors. `JWT_SECRET` absence only throws on the first auth request (`throw new Error('JWT secret missing – bitte JWT_SECRET in .env setzen')`, auth.ts:18-19), so the app boots green and then 500s on login. `OPENAI_API_KEY` absence is a one-time console.warn (runtimeConfig.ts:61-64). SMTP absence downgrades all mail (including invitation codes and password resets) to a console.info fallback (notifications.ts:107-112) with `success=false` mostly ignored by callers. A production webhook URL is hardcoded as a fallback in two places (nuxt.config.ts:50, data.post.ts:10), so an unconfigured instance posts dome-light events to a third-party home-automation endpoint. + +**Recommendation:** Add a Nitro plugin (server/plugins/validate-env.ts) that runs at startup, checks JWT_SECRET, MONGODB_URI, and (when NODE_ENV=production) OPENAI_API_KEY + SMTP settings, and exits the process with a clear message when required values are missing. Remove the hardcoded DOME_LIGHT_WEBHOOK_URL fallbacks — make the feature a no-op when the env var is unset. Document all server env vars (KPI_CRON_SECRET, BRIDGE_CORS_ORIGINS, ATC_OUT_DIR, FLIGHTLAB_TTS_CACHE_DIR, NUXT_PUBLIC_RADIO_BACKEND_URL) in .env.example — several are currently undocumented there. + +#### OPS-06 · MEDIUM · effort M — No structured logging, request IDs, or error reporting; high-frequency telemetry spams stdout with console.table + +**Files:** `server/api/bridge/data.post.ts:148-149`, `server/utils/notifications.ts:110`, `server/api/atc/say.post.ts:370-372`, `server/models/TransmissionLog.ts:17-27` + +Observability is 28 ad-hoc `console.*` calls, several with ANSI escape codes that garble log aggregators (e.g. data.post.ts:148). Worst: every bridge telemetry POST — which a simulator sends multiple times per second — executes `console.table(telemetryKeys.reduce(...))` (data.post.ts:149), rendering an ASCII table per frame; this is both log-volume and CPU overhead in production. There is no pino/winston, no Sentry or any error reporter (grep across server/, nuxt.config.ts, package.json: zero matches), and no request-ID correlation. TransmissionLog is the only session debugging tool, but the ATC `say` log writes no user (`// user: user._id`, say.post.ts:347), the schema has no index on `user` or `createdAt` and no TTL, and admin session listing runs an unbounded full-collection aggregation (sessions.get.ts:53-74) that will degrade as the collection grows forever. + +**Recommendation:** Remove the console.table and per-frame console.info from data.post.ts (or gate behind DEBUG env). Introduce pino via a small server/utils/logger.ts and a request-ID middleware (crypto.randomUUID stored on event.context, included in error logs). Add `user` and `createdAt` indexes plus an optional TTL index to TransmissionLog, and populate `user` in say.post.ts now that auth is required. Add a Nitro `error` hook that ships errors to Sentry (or at minimum logs them structured with the request ID). + +#### OPS-07 · MEDIUM · effort M — say.post.ts returns broken stored-audio URLs and never writes the audio/meta files it reports; TTS disk cache grows unbounded + +**Files:** `server/api/atc/say.post.ts:228-233`, `server/api/atc/say.post.ts:318-320`, `server/api/atc/say.post.ts:288-316` + +For non-cacheable requests the handler computes `fileOut`/`fileJson` under `storage/atc//` (lines 231-233) and returns `stored: { audioPath: storedAudioPath, jsonPath: storedJsonPath, url: '/api/atc/audio/${dateFolder}/${id}.${outputExt}' }` — but there is no `writeFile(fileOut, ...)` anywhere, and no `/api/atc/audio/**` route exists in server/api (Glob for `server/**/audio*` returns nothing). Clients that follow `stored.url` get a 404, and `meta.files.audio` points to a file that was never created. Meanwhile the flightlab/ATIS cache dir (`.cache/flightlab-tts`) is written on every cache miss (lines 288-316) with no size cap, eviction, or cleanup — on a persistent volume it grows forever; on an ephemeral container it is silently lost each deploy, re-incurring TTS cost. + +**Recommendation:** Delete the dead `stored.url`/`fileOut`/`fileJson` plumbing (clients already receive the audio as base64 in the response), or implement the missing write + a `/api/atc/audio/[date]/[file]` serving route with path traversal protection. Add a cache cap to the flightlab TTS cache (e.g. LRU by mtime, prune to N MB on write) and point FLIGHTLAB_TTS_CACHE_DIR at a persistent volume in the deploy config. + +#### OPS-08 · MEDIUM · effort L — No graceful shutdown handling — deploys drop WebSocket sessions and in-flight requests with no recovery path + +**Files:** `server/api/flightlab/ws.ts:20-55`, `nuxt.config.ts:93-96` + +There are no Nitro server plugins and no SIGTERM/close hooks anywhere in server/** (no `server/plugins/` directory exists). On deploy: all FlightLab WebSocket peers are disconnected and the `sessions` Map (including `currentPhaseId`, `history`, `startedAt`) is destroyed, so an instructor-led session resets to the 'welcome' phase with empty history when clients reconnect; in-flight OpenAI TTS/Whisper requests are killed mid-call and surface as opaque 500s on the client. The /pm live ATC page holds its state machine client-side and in the separate Python radio backend (config.public.radioBackendUrl), so it survives a Nuxt deploy except that any in-flight /api/atc/say or /api/vatsim/metar call fails — pm.vue's ATIS loop does handle a null broadcast start (pm.vue:3700-3706) but a failed controller-speech TTS mid-dialogue is simply lost. + +**Recommendation:** Persist FlightLab session state to Mongo on each state change so reconnecting peers (the client already reconnects per join-session flow) resume the correct phase. Add a Nitro plugin hooking `nitroApp.hooks.hook('close', ...)` to broadcast a 'server-restarting' WS message before exit. On the client, add retry-with-backoff around scheduleControllerSpeech's /api/atc/say call so one failed TTS during a rolling deploy doesn't swallow an ATC transmission. + +#### OPS-09 · MEDIUM · effort S — VATSIM proxy endpoints fetch upstream on every request with no caching + +**Files:** `server/api/vatsim/feed.get.ts:3-6`, `server/api/vatsim/metar.get.ts:3-9` + +feed.get.ts proxies the full VATSIM datafeed — a multi-megabyte JSON document — once per incoming request: `return await fetcher('https://data.vatsim.net/v3/vatsim-data.json')` with no server-side cache or Cache-Control headers (it also uses an untyped `(globalThis as any).$fetch` cast instead of the auto-imported $fetch). VATSIM updates this feed every ~15 seconds and asks consumers not to poll faster; with N concurrent users each browser request triggers a fresh upstream download, multiplying bandwidth N-fold and risking upstream rate limiting/blocking. metar.get.ts has the same per-request pass-through. + +**Recommendation:** Wrap both handlers in Nitro's `defineCachedEventHandler` (or `cachedFunction`) with `maxAge: 15` for the feed and `maxAge: 60` per-ICAO for METAR, and set a `Cache-Control: public, max-age=15` response header. Note the default Nitro cache storage is in-memory — acceptable here since stale data is bounded, and it also self-heals in multi-instance setups. + +#### OPS-10 · LOW · effort S — Documentation/code drift: client calls /api/llm/decide which has no server route; CLAUDE.md describes the removed flow + +**Files:** `shared/utils/openaiDecision.ts:6`, `CLAUDE.md:20`, `AGENTS.md:13` + +shared/utils/openaiDecision.ts still does `await $fetch('/api/llm/decide', ...)`, but no `server/api/llm/` directory exists — any code path that invokes it 404s at runtime. CLAUDE.md:20 documents "POST /api/llm/decide → routeDecision()" as the live flow, while AGENTS.md:13 says routeDecision is "Legacy... No longer called by /pm" — the live /pm path actually depends on an external Python radio backend at `NUXT_PUBLIC_RADIO_BACKEND_URL` (default `http://127.0.0.1:8000`, nuxt.config.ts:60), a deployment dependency that nixpacks.toml does not build and README does not list as a production requirement. + +**Recommendation:** Delete shared/utils/openaiDecision.ts if unused, or restore the route. Update CLAUDE.md to match the Python-backend architecture in AGENTS.md. Document the Python radio backend (repo location, deploy method, required NUXT_PUBLIC_RADIO_BACKEND_URL) in README as a production dependency — currently a fresh deploy of this repo alone yields a /pm page pointed at 127.0.0.1:8000. + +#### OPS-11 · LOW · effort S — Repo/build hygiene: stray binary artifact, hardcoded Hotjar ID with debug outside prod, unused native-dep scaffolding + +**Files:** `out.ogg`, `nuxt.config.ts:30-34`, `.env.example:42`, `package.json:14` + +`out.ogg` (293 bytes, commit fe8b762 "iwas was nich tut dazutun") is committed at the repo root. Hotjar is configured unconditionally with a hardcoded site id (`hotjarId: 6522897`) and `debug: process.env.NODE_ENV !== 'production'`, so every non-prod build (including local dev and previews) loads Hotjar in debug mode and reports traffic into the production Hotjar site — there is no env gate to disable tracking entirely. `.env.example:42` ships a real-looking default `MANUAL_INVITE_PASSWORD=pm.local@zghl.de`, which deployers may leave in place. The `sharp:rebuild` script targets a package that is only a transitive dep of @nuxt/image/ipx. + +**Recommendation:** Remove out.ogg from the repo. Move the Hotjar ID to `NUXT_PUBLIC_HOTJAR_ID` and skip registering the module (or set a no-op id) when the var is unset, so dev/preview traffic never pollutes production analytics. Blank out MANUAL_INVITE_PASSWORD in .env.example and have the manual-invite endpoint refuse to run when the env var is unset. + +--- + +## Product, License & Compliance + +OpenSquawk has a solid compliance skeleton for a German-operated alpha: full AGPL-3.0 license text with a matching package.json field, all-permissive direct dependencies, a complete Impressum/Datenschutz/AGB/unsubscribe page set, and a genuinely consent-gated Hotjar integration. However, the privacy notice has drifted from the actual data flows (raw voice audio goes to OpenAI Whisper, Hotjar and product-usage tracking are undisclosed), email marketing lacks double opt-in, user deletion leaves identifying data in five collections, and retention is unbounded everywhere. Product documentation contradicts the code in legally relevant ways: README claims no VATSIM integration while one ships, and the "open source core / 0€ self-host" claim is not reproducible because the Live ATC routing backend (OpenSquawk-LiveATC-api Python service) is not in or referenced by this repository. + +### What's done well + +- AGPL-3.0 licensing is internally consistent: full license text in LICENSE and "license": "AGPL-3.0-only" in package.json:6; all direct dependencies (nuxt, vue, vuetify, pinia, three, openai SDK Apache-2.0, nodemailer, fluent-ffmpeg, dotenv, nuxt-module-hotjar MIT per npm registry) are permissive and AGPL-compatible — no copyleft conflicts. +- Hotjar is genuinely consent-gated, not decorative: nuxt-module-hotjar requires manual initialize() (verified against the module's docs), and app/app.vue:148-182 only calls scheduleHotjarInitialization() after analytics consent, persisting opt-out via window._hjOptOut; app/composables/useCookieConsent.ts implements versioned, 6-month consent with reject-all parity in app/components/CookieConsentBanner.vue. +- Complete German legal page set: app/pages/impressum.vue (company, register, VAT ID), app/pages/datenschutz.vue (controller, purposes with Art. 6 legal bases, rights), app/pages/agb.vue, app/pages/unsubscribe.vue, all linked from the landing footer (app/pages/index.vue:844-846). +- Consent is captured with timestamps at registration (server/api/service/auth/register.post.ts:30-31, 69-70 → User.acceptedTermsAt/acceptedPrivacyAt are required schema fields, server/models/User.ts:28-29) and waitlist signup enforces consentPrivacy/consentTerms (server/api/service/waitlist.post.ts:76-78). +- First-party analytics are data-minimal: LandingAnalyticsEvent and ProductUsageSession store no IP addresses or user agents (server/models/LandingAnalyticsEvent.ts, server/models/ProductUsageSession.ts); roadmap votes store only a SHA-256 hash of IP+UA (server/api/service/roadmap.post.ts:13-17). +- All outbound waitlist/feedback emails include unsubscribe links in both HTML and plain text (server/utils/invitations.ts:19-27, 51-56, 72-73, 100-101), and a working unsubscribe page + endpoint exist (app/pages/unsubscribe.vue, server/api/service/updates.delete.ts). +- Competitor references (SayIntentions.AI, BeyondATC, FSHud) are nominative, factual and sourced (app/pages/present.vue:163, content/news/state.md:26-29 with links to official pricing pages) — legally clean comparative positioning, no logo use or implied affiliation found. +- PTT raw audio is deleted from the server immediately after transcription (server/api/atc/ptt.post.ts:122-125), consistent with the privacy notice's 'processed temporarily only' claim for server-side handling; admin/editor API routes consistently enforce requireAdmin/auth. + +### Findings + +#### COMP-01 · HIGH · effort S ✓ verified — Privacy notice misstates OpenAI data flow and omits Hotjar, cookies, and product-usage tracking + +**Files:** `app/pages/datenschutz.vue:58`, `app/pages/datenschutz.vue:32`, `server/api/atc/ptt.post.ts:108-114`, `app/app.vue:105-116`, `app/components/CookieConsentBanner.vue:17`, `nuxt.config.ts:28-34` + +datenschutz.vue:58 claims 'External AI providers (e.g. OpenAI) receive only pseudonymised text', but ptt.post.ts:109 sends the user's raw voice recording to OpenAI Whisper ('openai.audio.transcriptions.create({ file: createReadStream(audioFileForWhisper), model: "whisper-1" ...')— voice audio is personal data transferred to a US processor and the disclosure is factually wrong. The cookie banner says 'find out more in our privacy policy' (CookieConsentBanner.vue:17), but datenschutz.vue contains zero mention of Hotjar, cookies, the consent cookie, or session recording (grep for 'hotjar' matches only app.vue, the banner, the composable and nuxt.config.ts). Per-user product usage tracking (app.vue:105-116 POSTs product, path, duration tied to the authenticated user to /api/service/analytics/product-session) is also not described. This violates GDPR Art. 13 transparency duties. + +**Recommendation:** Rewrite app/pages/datenschutz.vue: (1) correct section 5 to state that push-to-talk voice recordings are transmitted to OpenAI (US) for transcription and TTS text for speech synthesis, citing the DPA/SCCs and OpenAI API data-retention terms; (2) add a 'Cookies & Analytics' section covering the osq-cookie-consent cookie, Hotjar (provider, Hotjar Ltd/Contentsquare, data categories, consent as legal basis Art. 6(1)(a), withdrawal via banner); (3) add product-usage measurement (per-account session duration/path) with its legal basis. Add a footer link or settings entry to reopen the consent banner (resetConsent already exists in useCookieConsent.ts). + +#### COMP-02 · HIGH · effort M ✓ verified — No double opt-in: drip cron emails unverified addresses and marketing consent is recorded without confirmation + +**Files:** `server/api/service/waitlist.post.ts:166-199`, `server/api/service/cron/waitlist-drip.get.ts:101-183`, `server/models/UpdateSubscriber.ts:17-18`, `server/api/service/updates.delete.ts:9-26` + +Anyone can submit any email address to POST /api/service/waitlist (waitlist.post.ts:166 creates the entry with 'consentPrivacy: true, consentTerms: true' from an unauthenticated request). No verification email is sent. After 5 days the cron (waitlist-drip.get.ts:101-149) automatically emails an invite code to that unverified address, and after 14 more days a feedback request. wantsProductUpdates sets 'consentMarketing: true' on UpdateSubscriber server-side with no confirmation. Under German law (UWG §7(2) Nr. 2 and BGH double-opt-in case law), marketing emails to unverified addresses are an Abmahnung risk; the operator cannot prove consent. Additionally GET /api/service/cron/waitlist-drip has no authentication at all, so any visitor can trigger the email batch, and DELETE /api/service/updates.delete.ts removes any waitlist entry given only an email (no token), letting third parties unsubscribe/delete others' signups. + +**Recommendation:** Implement double opt-in: on waitlist signup send a confirmation email with a signed token (e.g. HMAC(email, JWT_SECRET)); only set consent flags and drip eligibility (a new 'confirmedAt' field on WaitlistEntry/UpdateSubscriber) after the token link is visited. Gate waitlist-drip.get.ts behind a required secret (reuse the KPI_CRON_SECRET pattern from weekly-kpi-report.get.ts:8-13, but make it mandatory). Change unsubscribe links in server/utils/invitations.ts to carry the same signed token instead of the bare email, and verify it in updates.delete.ts. + +#### COMP-03 · HIGH · effort S ✓ verified — User deletion leaves personal data in five collections (incomplete GDPR Art. 17 erasure) + +**Files:** `server/api/admin/users/[id]/index.delete.ts:31-37`, `server/models/TransmissionLog.ts:18`, `server/models/ProductUsageSession.ts:16`, `server/models/WaitlistEntry.ts:28`, `server/models/UpdateSubscriber.ts:14`, `server/models/InvitationCode.ts:20-23` + +The only deletion path, index.delete.ts:31-35, removes just 'LearnProfile.deleteOne... BridgeToken.deleteMany... PasswordResetToken.deleteMany' before 'user.deleteOne()'. Left behind: TransmissionLog documents (speech transcripts with 'user' ObjectId ref, TransmissionLog.ts:18), ProductUsageSession (user ref), WaitlistEntry and UpdateSubscriber (keyed by the user's plaintext email — fully identifying), FeedbackSubmission (email/name/discordHandle), and InvitationCode.createdBy/usedBy references. The privacy notice promises erasure rights (datenschutz.vue:68), but the implemented mechanism cannot honor them; emails remain in the waitlist/subscriber collections indefinitely after account deletion. + +**Recommendation:** In server/api/admin/users/[id]/index.delete.ts, extend the Promise.all to: TransmissionLog.deleteMany({ user: user._id }) (or $unset user to anonymize), ProductUsageSession.deleteMany({ user: user._id }), WaitlistEntry.deleteOne({ email: user.email }), UpdateSubscriber.deleteOne({ email: user.email }), FeedbackSubmission.deleteMany({ email: user.email }), and InvitationCode.updateMany({ $or: [{ usedBy: user._id }, { createdBy: user._id }] }, { $unset: { usedBy: 1, createdBy: 1 } }). Consider a shared 'eraseUserData(user)' util so a future self-service deletion endpoint reuses it. + +**Verifier note:** Mostly accurate, one nuance: once the User document is deleted, the bare ObjectId refs in InvitationCode.createdBy/usedBy (and the user refs in TransmissionLog/ProductUsageSession) no longer resolve to any identifying record, so those are dangling pseudonyms rather than directly identifying data. The unambiguous GDPR residue is the plaintext email/name data in WaitlistEntry, UpdateSubscriber, and FeedbackSubmission, plus any identifying content within retained speech transcripts. High severity remains justified by the email-keyed collections and transcript retention contradicting the published erasure promise. + +#### COMP-04 · MEDIUM · effort M — No retention enforcement: transcripts, analytics and generated audio/metadata are stored indefinitely + +**Files:** `server/models/TransmissionLog.ts:17-27`, `server/models/LandingAnalyticsEvent.ts:16-24`, `server/models/ProductUsageSession.ts:15-23`, `server/api/atc/say.post.ts:228-343`, `app/pages/datenschutz.vue:51` + +Only PasswordResetToken has a TTL index ('index: { expires: 0 }', PasswordResetToken.ts:14). TransmissionLog (user speech transcriptions), LandingAnalyticsEvent and ProductUsageSession have no TTL and no cleanup job exists under server/api/service/cron. say.post.ts additionally writes every generated ATC audio file plus a JSON metadata file containing the raw text to disk ('storage/atc//.json', say.post.ts:230-233, 322-343) with no pruning. The privacy notice itself states an open-ended retention ('Communication logs: at least 12 months ... longer if needed', datenschutz.vue:51), which conflicts with the GDPR Art. 5(1)(e) storage-limitation principle requiring a defined maximum. Note also say.post.ts:174 has authentication commented out ('// const user = await requireUserSession(event);'), so logs accumulate from anonymous callers. + +**Recommendation:** Add TTL indexes: transmissionSchema.index({ createdAt: 1 }, { expireAfterSeconds: 60*60*24*365 }) (12 months, matching the policy), and e.g. 24 months for LandingAnalyticsEvent/ProductUsageSession. Add a cron task that prunes storage/atc date folders older than N days. Change datenschutz.vue:51 from 'at least 12 months' to a fixed maximum ('12 months, then deleted or anonymized'). Re-enable authentication on say.post.ts. + +#### COMP-05 · MEDIUM · effort S — README and CLAUDE.md contradict the shipped product; Live ATC depends on an unpublished Python backend, undermining the open-source claim + +**Files:** `README.md:18-20`, `CLAUDE.md:14-21`, `AGENTS.md:8`, `app/composables/useRadioBackend.ts:27-63`, `nuxt.config.ts:60`, `app/pages/index.vue:157`, `app/pages/index.vue:769`, `server/api/vatsim/feed.get.ts:1-7` + +README.md:20 states 'VATSIM: No integration for now due to unclear licensing ... will avoid coupling until approval is granted', yet server/api/vatsim/{feed,metar,flightplans}.get.ts proxy live VATSIM data and app/pages/pm.vue collects a 'VATSIM CID' and loads member flight plans. README.md:18 calls simulator bridges 'planned ... a few more months' while server/api/bridge/* and app/pages/bridge exist. CLAUDE.md:14-21 describes 'POST /api/llm/decide → routeDecision()' and '/api/decision-flows/runtime' — neither route exists (grep finds no routeDecision in server/, no decision-flows API route); AGENTS.md:8 documents the real architecture: a 'Python backend (OpenSquawk-LiveATC-api) — owns PM session state and routing decisions' reached via NUXT_PUBLIC_RADIO_BACKEND_URL (nuxt.config.ts:60, useRadioBackend.ts). That backend is not in this repo and not referenced in README, so the marketing claims 'Open source core' (index.vue:157) and 'the core is open source and self-hosting remains 0€ / always' (index.vue:769) are not reproducible from the published source — a self-hoster following README cannot run /pm at all. + +**Recommendation:** Update README.md: document the OpenSquawk-LiveATC-api requirement (link to its repository, or publish it under AGPL/compatible license if it is not yet public — without it the 'open source alternative' positioning is misleading); correct the VATSIM and bridge sections to reflect shipped functionality and the actual licensing posture. Replace CLAUDE.md's Live ATC flow section with the accurate one from AGENTS.md (or make CLAUDE.md point to AGENTS.md). + +#### COMP-06 · MEDIUM · effort S — Flight telemetry forwarded by default to a hardcoded private third-party webhook + +**Files:** `server/api/bridge/data.post.ts:10`, `server/api/bridge/data.post.ts:151-153`, `nuxt.config.ts:50` + +Every bridge telemetry POST forwards dome-light state derived from user simulator telemetry to a hardcoded personal home-automation endpoint: 'const DOME_LIGHT_WEBHOOK_FALLBACK_URL = "https://home.io.faktorxmensch.com/api/webhook/lidl_stab_3modi_8492"' (data.post.ts:10), used as fallback even when the env var is unset, and the same URL is the runtimeConfig default (nuxt.config.ts:50: 'domeLightWebhookUrl: process.env.DOME_LIGHT_WEBHOOK_URL || "https://home.io..."'). For any third party self-hosting this AGPL project, their users' telemetry-derived events silently flow to the maintainers' private server — an undisclosed data recipient not mentioned in datenschutz.vue, and a trust problem for an open-source project. data.post.ts:148-149 also console.tables full telemetry on every request. + +**Recommendation:** Remove the hardcoded fallback URL in both files; only forward when DOME_LIGHT_WEBHOOK_URL is explicitly configured (default to disabled). Drop or debug-gate the console.table of full telemetry. If the dome-light demo is needed, move it to a documented opt-in plugin. + +#### COMP-07 · MEDIUM · effort M — Unauthenticated VATSIM proxies expose third-party personal data and are undisclosed + +**Files:** `server/api/vatsim/feed.get.ts:1-7`, `server/api/vatsim/flightplans.get.ts:14-17`, `app/pages/pm.vue:9-14`, `app/pages/datenschutz.vue:79` + +feed.get.ts proxies the complete VATSIM datafeed ('return await fetcher("https://data.vatsim.net/v3/vatsim-data.json")') with no caching, no rate limiting and no auth — the feed contains personal data of VATSIM members (names, CIDs, positions), and VATSIM's data-feed usage policy expects consumers to cache (the feed updates ~15s) rather than fan out per-client requests. flightplans.get.ts lets anyone query 'https://api.vatsim.net/v2/members//flightplans' for an arbitrary CID. The privacy notice never mentions VATSIM as a data source/recipient even though pm.vue asks users for their 'VATSIM CID' and datenschutz.vue:79 itself references a 'VATSIM ID' as an identifier. The README's own statement that VATSIM licensing is 'unclear' (README.md:20) shows the team knows approval is outstanding. + +**Recommendation:** Add a server-side cache (e.g. 15-30s in-memory/nitro storage) in feed.get.ts and require an authenticated session on both endpoints; restrict flightplans.get.ts to the requesting user's own stored CID. Add a VATSIM paragraph to datenschutz.vue (data fetched, purpose, VATSIM as source). Resolve the licensing question with VATSIM before public launch or feature-flag the integration off, matching README's stated posture. + +#### COMP-08 · LOW · effort S — Impressum and privacy notice cite superseded German statutes and the wrong supervisory authority + +**Files:** `app/pages/impressum.vue:13`, `app/pages/impressum.vue:44`, `app/pages/datenschutz.vue:72` + +impressum.vue:13 cites '§5 TMG' — the Telemediengesetz was replaced by the Digitale-Dienste-Gesetz (DDG, §5) in May 2024. impressum.vue:44 cites '§55 II RStV' — the Rundfunkstaatsvertrag was replaced by the Medienstaatsvertrag (§18 Abs. 2 MStV) in 2020. datenschutz.vue:72 points complainants to 'the Berlin Commissioner for Data Protection' although the controller (Faktor Mensch MEDIA UG) is seated in Mainz, so the competent authority is the Landesbeauftragte für den Datenschutz und die Informationsfreiheit Rheinland-Pfalz. Both legal pages are dated 2025-09-17 (agb.vue/datenschutz.vue lastUpdated) and predate several shipped data flows. + +**Recommendation:** Update impressum.vue to cite §5 DDG and §18 Abs. 2 MStV; change the supervisory-authority example in datenschutz.vue:72 to the Rheinland-Pfalz LfDI; bump the lastUpdated dates when the privacy-notice content fixes (other findings) land. + +#### COMP-09 · LOW · effort S — AGPL hygiene gaps: no per-file headers, no in-app source offer, no contributor licensing policy + +**Files:** `LICENSE`, `package.json:6`, `README.md:115-120`, `app/pages/index.vue:868` + +The repo has the full AGPL-3.0 text and 'license': 'AGPL-3.0-only', but: (1) no source file carries the GPL-recommended copyright/license header, and there is no NOTICE identifying the copyright holder (LICENSE is the unmodified FSF text with no 'Copyright (C) ' applied via the 'How to Apply These Terms' appendix); (2) the running app only links the GitHub repo from the landing page (index.vue:868 GITHUB_URL) — authenticated app views (pm, classroom) show no source offer, which downstream operators need for AGPL §13 'prominent offer' compliance; (3) there is no CONTRIBUTING.md, DCO or CLA, so for an AGPL-3.0-only project run by a commercial UG that may later want dual licensing or relicensing, inbound rights are unresolved (README.md:115-120 invites contributions without terms). + +**Recommendation:** Add a copyright line to LICENSE ('Copyright (C) 2025-2026 Faktor Mensch MEDIA UG'); add a footer component used across app pages with 'Source code (AGPL-3.0)' linking the repo; add CONTRIBUTING.md declaring inbound=outbound (AGPL-3.0-only) or a DCO sign-off requirement, and decide explicitly whether a CLA is wanted before external contributions accumulate. + +#### COMP-10 · LOW · effort M — No self-service data export or account deletion for users + +**Files:** `app/pages/datenschutz.vue:67-73`, `server/api/admin/users/[id]/index.delete.ts:8-9`, `app/pages/datenschutz.vue:79` + +datenschutz.vue promises access, erasure and portability rights (Art. 15-20), but the only deletion endpoint is admin-only ('requireAdmin', index.delete.ts:9) and there is no data-export endpoint at all; users must email info@opensquawk.de (datenschutz.vue:79). Acceptable for a small invite-only alpha, but manual fulfillment also depends on the incomplete erasure logic (see separate finding) and there is no documented internal process or deadline tracking for the one-month Art. 12(3) response window. + +**Recommendation:** Add a user-facing 'Delete my account' action (server/api/service/auth/delete-account.post.ts calling the shared eraseUserData() util recommended above, with password re-confirmation) and a simple JSON export endpoint (user doc + LearnProfile + TransmissionLogs + ProductUsageSessions for the requesting user). Until then, document the manual DSAR process internally.