sales-trainer/docs/engineering-log/2026-08-13-idea-implementation-audit.md

2026-08-13 — Idea-to-implementation audit

Scope

Compared IDEA.md, README.md, docs/PLAN.md, and docs/FUTURE_WORK.md against the Vue frontend, Flask routes/services, JSON storage, deployment files, and all executable test scripts. No production data or credentials were inspected.

Architecture traced

• Admin setup: GroupBuilder.vue → POST /api/groups → document parser/GroupStore → POST /api/groups/<gid>/analyze → Analyzer → PersonaGenerator → report → JSON group storage.
• Training: Training.vue → Personas.vue → Chat.vue → chat start/send routes → Simulator.persona_reply + Simulator.evaluate_turn → SessionStore → debrief/board/analytics.
• Trainee loop backend: /api/me/weak-areas + /api/me/personas/generate → weak-area analyzer/own-persona generator → private group. API clients exist, but no Vue view invokes them.
• Persistence: one JSON file per entity, atomic file replacement and in-process per-path locks.

Verification performed

• All 12 executable backend scripts passed: M0, M1, setup, security, scenario, E2E, IP protection, SaaS tenant, user journey, routes, resume/decision, variant.
• python -m compileall app scripts passed.
• npm run build passed (1,772 modules; production bundle generated).
• Flask smoke test passed: /health 200, SPA / 200, unauthenticated /api/groups 401.
• Docker image was not built because Docker is not installed on this Mac.
• Real-provider LLM behavior was not exercised because no live key was used.

Findings

P0 — Any authenticated user can take over an arbitrary account

auth_routes.py:56-72 accepts a client-supplied username in /api/auth/setup and passes it to complete_setup() instead of deriving the target exclusively from current_user(). It also does not require the caller's must_setup flag. A fresh isolated-data runtime probe confirmed that an ordinary user can target admin, replace its email/password, and then log in as that super-admin (setup 200; takeover login 200). This is an immediate deployment blocker.

P0 — Tenant admins can reset credentials across tenants

admin_routes.py:141-177 loads the requested user but never verifies that a non-super-admin actor and target share the same org_id. Password and email mutation therefore cross tenant boundaries. A runtime probe confirmed an admin in org-two could reset a user in org-default and then log in as that victim (update 200; hijacked login 200).

P0 — Tenant admins can provision users into another tenant

admin_routes.py:87-126 trusts the request's org_id for existing organizations. The super-admin guard applies only when new_org=true. A runtime probe confirmed an org-two admin could create a user directly inside org-default (create 201; new-user login 200), bypassing tenant membership and seat ownership boundaries.

P0 — Admin secret fields leak from analyze response

group_routes.py:233-243 returns the full group, sales_kit, and full personas after analysis, without applying the admin redaction used by the GET routes at group_routes.py:253-265. A runtime probe as a non-super admin confirmed pains and tolerance are present in the analyze response, while the subsequent list endpoint strips them. This defeats the documented IP boundary during the normal group-creation flow.

P1 — Persona replies render as raw JSON with compliant real LLMs

simulator.py:43-48 instructs the role-play model to return a JSON object, but simulator.py:142-149 now treats the entire response as plain text instead of parsing reply. A deterministic probe returned the full {"reply":...,"decision":...,"mood":...} blob as the customer message. The mock also returns JSON (scripts/mock_llm.py:113-120), but tests only assert that reply is truthy, so this escaped detection.

P1 — Normal auto-finish bypasses the promised final judge debrief

Every turn is judged by evaluate_turn; when it says buy/walk, chat_routes.py:301-318 builds _build_abbrev_debrief instead of calling Simulator.judge. That abbreviated report uses fixed scores (60/25) and generic coaching (chat_routes.py:62-118). The detailed final judge exists only in the separate finish endpoint (chat_routes.py:325-367), but the current Chat.vue has no finish action. Therefore the normal flow does not deliver the README promise of a separately judged, message-specific scored/coached debrief.

P1 — Admin UI offers chat actions that backend rejects

Admin-facing persona screens link to the trainee chat (GroupEdit.vue:40-44, Personas.vue:42-45), while chat start/send/finish routes require role user (chat_routes.py:137-140, 208-211, 325-328). Admins following the visible action receive 403.

P1 — Claimed training loop is backend-only

README lines 32-33 promise weak-area analysis and user-generated personas. Backend routes are implemented (me_routes.py:51-59, 83-122) and API client methods exist (frontend/src/api/index.js:58-60), but no Vue view calls them. MyBoard.vue:1-55 only shows counts and sessions. Trainees cannot use the promised weak-area lock/manual persona generation through the product UI.

P1 — Users cannot manually finish a stalled conversation or revisit a completed debrief

The API client and backend expose manual finish (frontend/src/api/index.js:55; chat_routes.py:325-367), but Chat.vue never calls it and only transitions to done when a send response reports finished (Chat.vue:155-165). If the per-turn judge never returns buy/walk, the user has no end-session control. After leaving a completed chat, MyBoard.vue:22-28 provides no session-detail link and the API client has no finished-session detail method, so transcripts and coaching cannot be revisited from the UI.

P1 — “15 personas” is not an enforced invariant

persona_generator.py:35-53 retries, then accepts any non-empty list; a runtime probe with an LLM returning one persona produced a ready result containing one persona. The normalization can also underfill tiers (persona_generator.py:55-90). This conflicts with the central 15-persona/5-per-tier promise. Current E2E passes only because the mock always supplies 15.

P0 — One-shot identity collides across groups and is raceable

SessionStore.create keys the business rule by (user_id, persona_id) but omits group_id (sessions.py:33-40), while independently generated groups reuse IDs such as persona-01 (persona_generator.py:67). A runtime probe confirmed finishing persona-01 in one group blocks persona-01 in another unrelated group. Active-session discovery has the same missing scope (sessions.py:69-75). The check and random-file creation are also separate operations: a synchronized 8-thread probe created eight sessions for one user/persona. The implementation therefore both over-blocks legitimate training and under-blocks concurrent duplicates.

P1 — Bootstrap credentials are privileged before mandatory setup

factory.py:13-31 creates a known bootstrap super-admin and marks it must_setup, but require_auth never enforces that flag (helpers.py:44-63). The independent runtime probe confirmed the pre-setup bootstrap account could perform a privileged group creation. A fresh Internet-facing deployment can therefore be claimed before the operator changes credentials.

P1 — Tenant deactivation does not revoke existing tokens

Organization activity is checked at login (auth/users.py:172-186) but not in require_auth (helpers.py:44-63). An isolated runtime probe confirmed a token issued before tenant deactivation continued to access /api/auth/me with 200 afterward. With the default 24-hour JWT lifetime, disabling an organization does not promptly disable its sessions.

P1 — Production container runs Flask development server

Docker executes python run.py (Dockerfile:29-39), and run.py:13-20 uses app.run. Flask itself warns this is not a production server. It may work for a small internal pilot but is not production-grade serving, graceful lifecycle, or concurrency management.

P1 — Trainee-created variants mutated the shared corporate group (resolved in Sprint 1 follow-up)

The original audit found that the variant route appended directly to the shared group's personas. The follow-up now calls get_or_create_private_group() for trainee actors, stores the variant there, returns the target group ID, and routes the UI to that group (backend/app/api/group_routes.py:373-402, backend/app/services/groups.py:74-102, backend/tests/test_group_redaction.py:180-242). Admin-created variants still extend the shared admin pool by design. Collection-level scan/create atomicity remains deferred to Sprint 2.

P2 — Upload size configuration is unused

Config.UPLOAD_MAX_MB is defined (config.py:62) but never applied to Flask MAX_CONTENT_LENGTH or route-level size validation. File type and path checks exist, but upload memory/disk consumption is effectively unbounded at the application layer.

P2 — Product copy and dead append flow contradict the decided UX

The intended design says auto-create 15 with no add-more flow, but GroupBuilder.vue:12 and GroupEdit.vue:21-23 still tell admins to click an absent “create more” button. GroupEdit.vue:83-92 retains an unreachable append function, and the backend still supports ?append=true (group_routes.py:183-190).

If append mode is later re-exposed, it also concatenates newly generated personas without replacing IDs (group_routes.py:219-228) while generation restarts at persona-01 (persona_generator.py:55-68). This would create duplicate Vue keys, ambiguous persona lookup, and shared one-shot history.

P2 — Deployment defaults are unsafe if operators omit configuration

config.py:50 falls back to a short public development JWT secret; runtime emitted an insecure HMAC key warning. This should fail closed outside development rather than silently issue production tokens with the fallback.

P2 — Non-UTF-8 file parsing imports an undeclared dependency

file_parser.py:26 imports chardet, but requirements.txt:1-9 installs charset-normalizer instead. The reviewer probe confirmed a non-UTF-8 input reaches ModuleNotFoundError: chardet. UTF-8 inputs pass, which is why the current scripts do not expose the deployment defect.

P2 — Settings password change is wired to first-time setup and fails

Settings.vue:88-102 calls /auth/setup without accepted_terms, while that route requires the field at auth_routes.py:65-69. The same route is also semantically unsafe for repeat password changes and caused the P0 arbitrary-target takeover above. Password change needs a separate current-user endpoint with appropriate current-password/session safeguards.

P2 — Docker build context can include local secrets

The Dockerfile copies the complete backend directory (Dockerfile:26-27), and the repository has no .dockerignore. Git ignore rules do not constrain Docker build context, so a local backend/.env or other untracked development artifact can be copied into an image layer. Add a restrictive .dockerignore and verify the built image contains no local configuration or credentials.

P2 — Active sessions are displayed as losses

/api/chat/sessions returns active and finished sessions, but MyBoard.vue:27 renders every outcome other than won as lost. Since active sessions have outcome: null (sessions.py:49-53), unfinished training is falsely reported as a loss instead of an active/resume state.

Reviewer claim rejected — analytics CSV Bearer header is correct

One reviewer reported a malformed authorization header because its tool output had redacted the token scheme. Direct byte/codepoint inspection of Analytics.vue:92 confirmed the source contains the correct Bearer ${auth.token} template string, and the frontend production build succeeds. This is not a defect. The separate signed-link route remains unreachable without Bearer auth because route decorators run before query-token verification, but the currently shipped frontend uses Bearer auth directly.

Capability verdict

Core concept is substantially implemented: roles, group creation, file parsing, LLM sales-kit/persona generation, tiers, scenarios, chat/resume, per-turn decision, variants, session results, analytics, and static deployment all exist and pass mock-based tests.

However, the application must not be deployed to untrusted users in its current state. Independent review plus isolated-data reproduction confirmed arbitrary super-admin takeover and two cross-tenant administration bypasses. Beyond those security blockers, the product path has a secret-data leak, likely raw-JSON customer messages with a compliant real model, generic rather than final-judge coaching, inaccessible weak-area training UI, a broken one-shot identity invariant, and a development server in production packaging. Passing mock scripts materially overstate readiness because none cover these attack paths.

Recommended order

1. Freeze deployment and fix /auth/setup arbitrary-target takeover; add an ordinary-user-to-super-admin regression test.
2. Enforce tenant scope on admin user creation/update and add cross-org denial tests.
3. Enforce must_setup server-side, remove known bootstrap privilege exposure, and re-check organization activity on authenticated requests.
4. Fix analyze-response redaction and add a regression test.
5. Redesign session identity as (org_id, user_id, group_id, persona_id) and enforce creation atomically.
6. Align persona prompt/response parsing and make auto-finish invoke the final judge; test exact visible messages and non-generic coaching.
7. Remove admin chat affordances or deliberately support admin preview mode; wire weak-area/manual persona generation into the trainee UI.
8. Enforce exactly 15/5-per-tier or make partial generation explicit and recoverable.
9. Add a transactional database, production WSGI server, upload limit, fail-closed JWT configuration, dependency lock, and CI security gates.
10. Run real-provider, hostile-input, concurrency, and mobile browser QA after the above.