sales-trainer/docs/engineering-log/2026-08-13-sprint-1-security-hotfix.md

2026-08-13 — Sprint 1 security hotfix

Scope

Executed the first gated remediation sprint from .hermes/plans/2026-08-13_094159-sales-trainer-remediation-all-sprints.md. Work stayed on isolated temporary test data; no production .env, credentials, commit, push, or deployment action was performed.

Changes

• Added standard pytest discovery and isolated Flask fixtures under backend/tests/.
• Bound /api/auth/setup to the authenticated user and required must_setup plus terms consent.
• Enforced tenant scope on admin user provisioning and updates.
• Added request-level checks for active user, active/existing organization, JWT org claim consistency, and setup-safe endpoint allowlist.
• Removed known bootstrap password and implicit production JWT fallback; production now requires explicit JWT_SECRET and first-boot BOOTSTRAP_ADMIN_PASSWORD.
• Unified role-aware group/persona serializers so analyze, GET, list, update, and variant responses follow the same redaction policy.
• Kept trainee-created persona variants out of the shared corporate group by storing them in an owner-private group, returning the target group_id, and routing the trainee UI to that private group.
• Redacted trainee group input down to high-level context only; uploaded filenames and parsed source text are not returned by the group view.
• Preserved the platform-admin contract on group listing: super-admins can index groups across tenants, while tenant admins remain organization-scoped.
• Updated deterministic legacy scripts to inject synthetic test bootstrap configuration rather than relying on a known default credential.
• Follow-up remediation replaced admin persona blacklist stripping with an explicit allowlist, sanitized client/persisted exception messages, added request/per-file upload limits and cleanup, blocked super-admin creation/promotion and protected-field mutation through admin APIs, required strict JWT tenant claims and boolean terms consent, denied orphan-tenant login, and routed private trainee-persona responses through the revealable serializer.

Verification

• backend/.venv/bin/python -m pytest backend/tests -q → 51 passed, including threaded and forked-worker atomic setup regressions plus concurrent email-reservation coverage across setup, profile, and invited-user creation.
• backend/.venv/bin/python -m pytest backend/tests/test_auth_security.py -q → 12 passed, including literal terms-consent boundary and no-partial-update failure-path checks.
• backend/.venv/bin/python -m pytest backend/tests/test_group_redaction.py -q → 8 passed, including source-data redaction and cross-user private-variant isolation.
• backend/.venv/bin/python -m pytest backend/tests/test_sprint1_review_findings.py -q → 11 passed.
• backend/.venv/bin/python -m compileall -q backend/app backend/tests backend/scripts → passed.
• All 12 backend/scripts/test_*.py suites → 12/12 passed.
• cd frontend && npm run build → passed; 1,772 modules transformed.
• git diff --check → passed.
• Added the reviewer-requested race regressions for first-time setup, protected cross-user email uniqueness with a collection-level lock covering setup, profile, and invited-user creation, and changed the transition to a conditional per-record locked update with an OS file lock for forked workers; all focused concurrency tests and the full suite now pass.

Full evidence: docs/test-evidence/2026-08-13-sprint-1-security.md.

Remaining gate / deployment warning

Sprint 1 code-side tests pass, including the private-variant, source-data-redaction, upload-boundary, generic-error, strict-tenant-claim, protected-super-admin, and create-user/setup email-race regressions. The earlier focused reviewer verdict found the create-user email race and predates this follow-up remediation; a fresh independent reviewer rerun is pending. Docker is unavailable locally, real-provider QA is not run, and live credential/JWT rotation has not been performed. Do not restore public/untrusted access until both the fresh reviewer acceptance and the operator restricted-deployment checklist are complete: set a strong bootstrap password, rotate JWT_SECRET after patch deployment, inspect audit data, and run a fresh authenticated smoke test.

Sprint 2 is not started. Known Sprint 2 targets remain one-shot identity/concurrency, persona reply parsing, final-judge debrief, and admin Preview Mode.

Follow-up reviewer remediation — 2026-08-13

The fresh reviewer identified blocking classes: raw exception disclosure in setup/session/persona routes, a parser-import upload-cleanup hole, open-ended group envelopes, unsafe implicit runtime defaults, and bool("false") active-state coercion. A RED/GREEN cycle added regression tests before each fix.

Implemented fixes:

• Parser import and all post-save parsing/persistence now share a finally cleanup boundary; multi-file, import-failure, missing-input, request-limit, save-failure, and persistence-failure paths remove every saved file.
• Auth/setup/profile/session/persona public errors use fixed messages; logs retain only exception types.
• Group envelope and tenant-admin input serialization are explicit allowlists; legacy error text remains normalized and trainee/admin source payloads are omitted.
• Runtime defaults are production/secure (APP_ENV=production, FLASK_DEBUG=false); .env no longer overrides explicitly supplied process environment; placeholder JWT and bootstrap values are rejected; malformed stored active state fails closed.
• Admin invites now pass the requested email into create_user(), preserving the global email invariant; admin active and new_org fields require literal JSON booleans. Setup uses a dedicated typed error for its 409 response rather than inspecting exception text.

Verification after the follow-up:

• Full backend pytest before the final reviewer follow-up: 65 passed.
• Focused security suites before the final reviewer follow-up: auth 12, group redaction 9, Sprint 1 findings 23, request guards 9, bootstrap config 8.
• Backend scripts: 12/12 passed; compileall, AST parse, diff check, and production added-line security scan passed with 0 blocking matches.
• Frontend build: 1,772 modules transformed; passed.

The final fresh independent reviewer gate is still pending. No commit, push, deploy, credential rotation, JWT rotation, or public-access change occurred.

Final reviewer follow-up — 2026-08-13

The first fresh reviewer rerun identified two blocking service-boundary gaps: setup eligibility was not proven strict at every layer, and complete_setup() did not receive/validate terms consent itself. The follow-up used a RED/GREEN cycle:

• Added accepted_terms and accepted_terms_at as required keyword arguments to UserStore.complete_setup() and enforced accepted_terms is True before any mutation.
• Added a create_user() boundary check rejecting non-boolean must_setup values.
• Added a dedicated set_email() versus complete_setup() concurrency regression test.
• Updated all direct setup callers to pass explicit literal consent and synchronized the test evidence.

Latest verification:

• Full backend pytest: 77 passed.
• Focused suites: auth 17, group redaction 9, Sprint 1 findings 24, request guards 12, bootstrap config 8.
• Auth security suite repeated 5/5 times for concurrency stability.
• Backend scripts 12/12 passed; frontend production build passed with 1,772 modules transformed.
• compileall, git diff --check, and direct complete_setup() call-contract scan passed.
• Static added-line scan passed with zero matches for hardcoded secrets, shell injection, eval/exec, unsafe pickle deserialization, or SQL string formatting.
• Login response serialization now uses user.get("must_setup") is True, removing the last reviewed truthiness coercion from the setup flag.
• assert_tenant(None) now fails closed instead of treating a missing object tenant as org-default; a dedicated request-guard regression covers this boundary.

The independent reviewer verdict for this latest state remains pending. No commit, push, deploy, credential rotation, JWT rotation, or public-access change occurred.

Final independent reviewer gate — 2026-08-13

The fresh independent read-only reviewer completed a schema-valid final review of the current uncommitted Sprint 1 packet and returned passed=true with empty security_concerns and logic_errors. The reviewer found no blocking security or logic issue in the reviewed packet.

The reviewer recorded only non-blocking follow-up suggestions: move private-group scan/create to a collection-level atomic primitive in Sprint 2, and keep Docker, real-provider, browser/mobile, deployment, and bootstrap/JWT rotation checks as separate operational gates.

Therefore the Sprint 1 code/reviewer gate is closed. The live-operation gate remains pending: no commit, push, deploy, credential rotation, JWT rotation, or public-access change occurred. Before restoring untrusted access, an operator must perform restricted deployment, set/rotate the bootstrap credential and JWT_SECRET, inspect audit data, and run a fresh authenticated smoke test.

Superseded current-state note — 2026-08-14

The reviewer result described in the preceding historical section belonged to an earlier packet/state. It does not approve the current uncommitted tree after the subsequent password-policy, atomic-mutation, malformed-input, user/session-serialization, debrief-allowlist, and tenant-fallback fixes. The current exact-tree reviewer gate is pending; see docs/engineering-log/2026-08-14-sprint-1-current-tree.md and the current evidence section.