[verified] harden Sales Trainer and add PostgreSQL foundation
This commit is contained in:
@@ -7,8 +7,9 @@
|
||||
- Remote: `https://git.moreminimore.com/kunthawat/sales-trainer.git` (GITEA_TOKEN via credential
|
||||
helper; never committed).
|
||||
- **Live deploy:** `https://moreminimoreapps-saletrainer.ahkhwd.easypanel.host` — EasyPanel,
|
||||
auto-redeploys from Gitea on push to `main` via webhook (≈3 min). Dockerfile ships prebuilt
|
||||
`frontend/dist/` (no npm in image). LLM vars set in EasyPanel env.
|
||||
auto-redeploys from Gitea on push to `main` via webhook (≈3 min). Dockerfile builds the
|
||||
frontend in a Node builder stage and serves it from the Python runtime image. LLM vars set in
|
||||
EasyPanel env.
|
||||
|
||||
## What this is
|
||||
Corporate multi-user **sales-training simulator**: admins create persona groups from a
|
||||
@@ -22,8 +23,15 @@ filesystem JSON storage (no SQL). i18n TH/EN. No self-registration (admin provis
|
||||
- **admin** — manages groups/users, sees personas with **secret fields stripped** (IP protection).
|
||||
- **user** (trainee) — trains against personas, own board.
|
||||
|
||||
## Current state — COMPLETE core + hardened
|
||||
All backend + frontend built. **11 test suites green** (mock LLM):
|
||||
## Current state — local code/security gate passed; production-operation gate pending
|
||||
The current uncommitted remediation is verified on isolated temporary data: **319 backend tests passed** from a clean `requirements.lock.txt` environment, including **166 focused auth/isolation/export/upload regressions**; **4 frontend unit tests** and **12 Playwright fixture journeys** passed across desktop, 320×568, and 500×768; the production frontend build completed with **1,781 modules** and `npm audit` found **0 vulnerabilities**. Compile, AST, diff, dependency, and added-line security checks passed. The checked-in lock is reproducible. The existing local `backend/.venv` has version drift and `pip check` reports the pre-existing `alibabacloud-tea-openapi 0.4.4` versus `cryptography 50.0.0` conflict; `uv pip sync --dry-run` was inspected but not applied. The final fresh exact-current scoped review returned clean five-key verdicts for auth/storage/rate-limit, tenant/group/session isolation, and analytics/export/parser/upload boundaries. JSON stores remain runtime-authoritative; no production operation has been performed.
|
||||
|
||||
### Newly verified local PostgreSQL evidence — 2026-08-15
|
||||
|
||||
- A temporary local PostgreSQL database completed `alembic upgrade head`, the 7-table tenant/uniqueness runtime probe, ORM-vs-migration parity, offline PostgreSQL DDL checks, the JSON importer dry-run/apply/idempotency/conflict-rollback probe, and `alembic downgrade base`; zero application tables remained and all temporary databases/fixtures/backups were cleaned up.
|
||||
- This closes the **local PostgreSQL schema/runtime and fixture-importer probes** only. Real JSON snapshot parity/rollback, runtime repository cutover, Redis persistence, Docker image/runtime smoke, real-provider QA, authenticated production smoke, and production approval remain open.
|
||||
|
||||
**Do not restore public/untrusted access.** An async reviewer packet created before the latest auth-lock state reported a password `auth_version` race; the current tree now serializes `change_password()` and `update_user_fields(password=...)` through the same per-user record lock, and the deterministic cross-path regression passes. A surgical independent review of this auth path returned valid `passed=true` with empty blocking arrays. Legacy-security reviewers `deleg_ffba9adf`, `deleg_03d06f1f`, and strict retry `deleg_0cc095f8` all timed out after 600 seconds without JSON and are no verdicts. Final one-call retry `deleg_e61f99ff` returned complete five-key JSON with `passed=false` because the extraction output exceeded the capture window; it is a limitation/no-approval verdict. The latest full-scope reviewer `deleg_93ef64c5` timed out after 600 seconds without a complete five-key JSON verdict; it is no approval. The replacement batch `deleg_df17f04e` was stopped because it began before the final service-boundary hardening and is not approval. Fresh exact-current review batches `deleg_e835e807` and `deleg_4ec7eb5a` completed for auth/storage/rate-limit, tenant/group/ownership/session isolation, and analytics/export/parser/upload boundaries. All three scoped verdicts returned complete five-key JSON with `passed=true`, `security_concerns=[]`, and `logic_errors=[]`. S4.3 reviewer `deleg_c2e728d8` returned valid `passed=false` after finding the offline Alembic dialect bypass; the shared validator and regression test fixed it, and fresh post-remediation reviewer `deleg_40e8edf9` returned valid `passed=true` with empty blocking arrays. Combined with the earlier dependency/schema review `deleg_e672880a`, the S4.2/S4.3 code-schema review scope is closed. Only complete five-key verdicts with `passed=true`, `security_concerns=[]`, and `logic_errors=[]` close review gates. Docker is unavailable locally. Local fixture-based browser/mobile E2E passes, but real-provider QA, production-authenticated browser QA, and production operations are not run. The restricted deployment checklist (bootstrap credential change, JWT secret rotation, audit inspection, fresh authenticated smoke) remains pending and requires explicit operator approval.
|
||||
|
||||
```bash
|
||||
cd backend
|
||||
@@ -39,7 +47,7 @@ uv run python scripts/test_user_journey.py # idea-flow end-to-end
|
||||
uv run python scripts/test_variant.py # clone-persona-from-persona
|
||||
uv run python scripts/test_resume_decision.py # resume + per-turn LLM decision
|
||||
|
||||
# run backend (serves SPA from frontend/dist)
|
||||
# run backend (serves frontend/dist only when a local build exists; Docker builds it reproducibly)
|
||||
cd backend && uv run python run.py # Flask :5001
|
||||
```
|
||||
|
||||
@@ -55,11 +63,14 @@ cd backend && uv run python run.py # Flask :5001
|
||||
- **Persona variant:** `POST /api/groups/<gid>/personas/<pid>/variant` — new persona (new id) that
|
||||
**locks** pains/objections/levers/tolerance/special/recontact/goal/budget/difficulty/tier/product
|
||||
**but varies** identity (name/profession/age/location/background/personality/opener). Lets a
|
||||
trainee re-practice the same challenge (one-shot is per-persona). UI button on finished personas.
|
||||
trainee re-practice the same challenge (one-shot is per-persona). Trainee-created variants go to
|
||||
the trainee's owner-private group; admin-created variants extend the shared admin pool. UI button
|
||||
on finished personas routes trainees to the private group.
|
||||
- **Auto 15 personas** on create; no "เพิ่มเติม" button (TARGET=15, retry up to 3× + accept ≥ 8 so
|
||||
real LLM under-count doesn't 500).
|
||||
- **IP protection:** `SECRET_PERSONA_FIELDS` (pains, objections, negotiation_levers, opener,
|
||||
tolerance, rootCause, resolutionConditions) stripped for `admin`; full only for `super_admin`.
|
||||
- **IP protection:** `admin` persona responses use an explicit allowlist and omit secret/process
|
||||
fields (pains, objections, negotiation_levers, opener, tolerance, rootCause, resolutionConditions)
|
||||
plus any future unapproved fields; full canonical data is only for `super_admin`.
|
||||
- **SaaS Phase 1–3 done:** tenant isolation (`g.org_id` + `assert_tenant`), login/chat rate-limit,
|
||||
audit log (`data/audit/audit.jsonl`), org plan/seats/active model + `PATCH /api/admin/orgs/<id>`,
|
||||
ToS consent on setup, org-scoped signed expiring CSV export (5-min HMAC).
|
||||
@@ -67,9 +78,8 @@ cd backend && uv run python run.py # Flask :5001
|
||||
persona detail rendered as readable form/cards (pain = line-by-line, not `[object Object]`).
|
||||
|
||||
## Credentials / data (testing)
|
||||
- Bootstrap super-admin `admin` / `1234` → first login forces email + new password + ToS consent.
|
||||
- Live test users: `testadmin` / `1234` (admin), `testuser` / `1234` (user).
|
||||
- A test group "CRM ระบบจัดการลูกค้า" exists on live (user keeps it; will delete it themselves).
|
||||
- Production bootstrap super-admin is `admin`; its initial password must come from `BOOTSTRAP_ADMIN_PASSWORD` and is never printed or hard-coded. First login forces email + new password + ToS consent.
|
||||
- Do not record or repeat live credentials in this handoff. Existing live test accounts/data require operator review after restricted deployment and secret rotation.
|
||||
- **LLM key is a placeholder on local `.env`** (`replace_me`). Real analyze/chat needs a real
|
||||
`LLM_API_KEY` (+ `LLM_PROVIDER`/`LLM_MODEL`/`LLM_BASE_URL`) in EasyPanel env then redeploy.
|
||||
|
||||
@@ -79,7 +89,8 @@ cd backend && uv run python run.py # Flask :5001
|
||||
- **Tooling guard crash:** commands whose first token is `./.venv/bin/python` trip a lifecycle guard
|
||||
→ always use `uv run python`. Prefix `PYTHONPATH=` when needed.
|
||||
- **Frontend build:** `cd frontend && npm run build` (works even with allowScripts restrictions).
|
||||
Commit `frontend/dist/` with `git add -f` (it's gitignored otherwise); the Dockerfile needs it.
|
||||
`frontend/dist/` is generated and ignored; Docker builds it in the image, and CI builds to a
|
||||
temporary output directory so clean clones never depend on stale hashed bundles.
|
||||
- **Deploy pattern:** commit + push → webhook auto-deploys in ~3 min. Cannot run Docker locally
|
||||
(no Docker on this Mac) → test with nginx/`python http.server` or `uv run python run.py`.
|
||||
- **No remote push without asking** unless it's the established auto-deploy cadence.
|
||||
@@ -93,6 +104,16 @@ cd backend && uv run python run.py # Flask :5001
|
||||
Responsive CSS (640px, single-column, `flex-wrap`, `.btn-back`) is present + deployed.
|
||||
|
||||
## Next actions / backlog (also docs/FUTURE_WORK.md)
|
||||
- **Sprint 1 live-operation gate:** deploy behind restricted access, set/verify `BOOTSTRAP_ADMIN_PASSWORD`, rotate `JWT_SECRET` after the patch, inspect audit data, and run a fresh authenticated smoke. Do not open public access before this checklist.
|
||||
- **Sprint 2:** implementation and local regression gate complete; retain current tests as the contract.
|
||||
- **P1:** run real-provider QA and browser/mobile E2E; local contract tests use deterministic fakes.
|
||||
- **P1:** verify final-judge coaching quality with representative provider outputs; correctness and idempotency are locally covered.
|
||||
- **P1:** remove any stale admin chat entry points in live UI after restricted authenticated smoke.
|
||||
- **P1/P2 ops:** add PostgreSQL repository adapters, browser E2E, and real-provider QA before production rollout. Upload limits, cleanup, fail-closed JWT/bootstrap configuration, Gunicorn, Docker healthcheck, and `.dockerignore` are implemented and tested locally.
|
||||
- **S4.2:** schema reviewer findings (nullable audit actor link, CWD-relative Alembic paths, ORM/migration defaults drift, unsupported partial-index dialect, and offline batch rendering) plus dependency reproducibility were remediated locally; combined exact-current evidence from `deleg_e672880a` and post-remediation `deleg_40e8edf9` closes the code/schema/dependency review scope. Temporary-local PostgreSQL execution, ORM/migration parity, offline DDL, and schema rollback now pass; Docker build, importer data parity/rollback, repository cutover, and production-safe rate-limit/audit storage remain blocked.
|
||||
- **S4.3:** org/users, groups/personas, and sessions/messages repository contracts and SQLAlchemy adapters are local-only; cross-tenant user lookup and offline-dialect remediations are locally verified and independently approved by `deleg_40e8edf9`. PostgreSQL schema parity passes on temporary local databases, while runtime repository cutover remains blocked.
|
||||
- **S4.4:** JSON importer dry-run, backup, idempotency, conflict rejection, cross-tenant validation, temporary-local PostgreSQL apply, and transaction rollback all pass; real target snapshot parity, retained-backup rollback rehearsal, audit migration, and apply approval remain blocked.
|
||||
- **2026-08-15 legacy-security remediation:** the latest valid reviewer finding about legacy `ratelimit.json` migration was remediated with fail-closed marker/digest validation, structured keys, duplicate-preserving import, and idempotent import metadata. Current local evidence is 319 backend tests, 166 focused regressions, and 4 frontend unit tests. Three fresh exact-current scoped reviewers returned clean five-key verdicts; see `docs/engineering-log/2026-08-15-final-security-gate.md`.
|
||||
- Real `/legal` page (setup links to it), billing/payments, per-tenant storage volume, compressed
|
||||
persona recipe, export-token polish.
|
||||
- Mobile visual polish per user feedback on real device.
|
||||
@@ -100,6 +121,18 @@ cd backend && uv run python run.py # Flask :5001
|
||||
|
||||
## Related docs
|
||||
- `docs/PLAN.md`, `docs/SAAS_PLAN.md`, `docs/FUTURE_WORK.md`.
|
||||
- `docs/engineering-log.md` + `docs/engineering-log/2026-08-09-idea-flow-qa-deploy.md` (this session:
|
||||
- `docs/engineering-log/2026-08-13-idea-implementation-audit.md` — latest evidence-based audit, runtime probes, and prioritized fixes.
|
||||
- `docs/engineering-log.md` — status index.
|
||||
- `docs/engineering-log/2026-08-15-final-security-gate.md` — final local verification and clean exact-current scoped review gate; production-operation limits remain.
|
||||
- `docs/engineering-log/2026-08-15-postgresql-runtime-gate.md` — temporary-local PostgreSQL schema/runtime, parity, offline DDL, and migration rollback evidence.
|
||||
- `docs/engineering-log/2026-08-15-postgresql-import-gate.md` — temporary-local PostgreSQL importer dry-run/apply/idempotency/conflict-rollback evidence.
|
||||
- `docs/test-evidence/2026-08-15-postgresql-runtime.md` — PostgreSQL automated gate evidence and remaining operational boundaries.
|
||||
- `docs/test-evidence/2026-08-15-postgresql-import.md` — importer evidence separated from real target apply and audit migration.
|
||||
- `docs/engineering-log/2026-08-09-idea-flow-qa-deploy.md` (this session:
|
||||
idea-flow UX, 2-scenario + recontact trait, live QA + auto-deploy, per-turn LLM judge, persona
|
||||
variant, 15-persona auto-gen).
|
||||
- `docs/engineering-log/2026-08-14-final-review.md` — prior five-finding remediation and review history; current exact-tree approval remains pending.
|
||||
- `docs/engineering-log/2026-08-15-auth-version-review-reconciliation.md` — current auth-version lock-path verification and pending fresh reviewer gate.
|
||||
- `docs/engineering-log/2026-08-15-s4-2-schema-foundation.md` — test-first SQLAlchemy/Alembic schema foundation, tenant constraints, and generated-dist cleanup.
|
||||
- `docs/engineering-log/2026-08-15-s4-3-org-users-repositories.md` — tenant-scoped repository contracts and adapters, with no runtime cutover.
|
||||
- `docs/engineering-log/2026-08-15-s4-3-offline-dialect-remediation.md` — offline Alembic dialect finding, test-first fix, verification, and pending review.
|
||||
|
||||
@@ -379,8 +379,9 @@ i18n: en + th (mirrors MiroFish pattern). Role-based navigation (admin vs traine
|
||||
- **File safety**: upload allowed types + size caps; parse text server-side; strip anything
|
||||
executable; keep raw uploads out of any served path.
|
||||
- **LLM route discipline**: the chat + persona gen + judge are the only LLM-touching callers.
|
||||
- **Deploy**: single `Dockerfile` (python:3.11 + Node 18, build Vue → serve static via
|
||||
Flask or nginx) + `docker-compose.yml` with `.env`, per the user's EasyPanel pattern.
|
||||
- **Deploy**: single source-built `Dockerfile` (Node 22 frontend builder + Python 3.11 runtime,
|
||||
serving the generated SPA through Flask) + `docker-compose.yml` with `.env`, per the user's
|
||||
EasyPanel pattern. Generated `frontend/dist/` remains ignored; clean clones build it in Docker/CI.
|
||||
Local tests via `http.server` / Flask dev (no Docker on local Mac).
|
||||
|
||||
---
|
||||
|
||||
@@ -24,6 +24,12 @@ Informed by MiroFish (CrowdSight engine) + the hermes-brain-and-tools CrowdSight
|
||||
| M5 Trainee loop (board, weak-areas, gen-persona) | complete | 2026-08-07 | `test_e2e.py` | — |
|
||||
| M6 Frontend (Vue SPA) + static serving fix | complete | 2026-08-07 | build + live HTTP 200 | — |
|
||||
| M7 Docker/deploy/docs | complete | 2026-08-07 | Dockerfile/compose/README | live-key E2E |
|
||||
| Idea-to-implementation audit | deployment blocked | 2026-08-13 | `docs/engineering-log/2026-08-13-idea-implementation-audit.md` | complete Sprint 1 live-operation checklist |
|
||||
| Sprint 1 security hotfix | local verification + exact-tree reviewer passed; live-operation gate pending | 2026-08-14 | `docs/test-evidence/2026-08-13-sprint-1-security.md`, `docs/engineering-log/2026-08-14-sprint-1-current-tree.md` | operator-approved restricted deploy/rotation/smoke; then reassess Sprint 2 start |
|
||||
| Sprint 2–4 final code gate | S4.2/S4.3 code-schema review passed; legacy-security remediation and exact-current scoped review passed; live-operation gate pending | 2026-08-15 | `docs/engineering-log/2026-08-15-final-security-gate.md` | operator-approved restricted deploy + authenticated smoke only; production readiness remains pending |
|
||||
| S4.2 relational schema foundation | code/schema/dependency review passed after offline-dialect remediation; temporary-local PostgreSQL runtime/parity/schema rollback probe passed; target runtime cutover blocked | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-2-schema-foundation.md`, `docs/engineering-log/2026-08-15-postgresql-runtime-gate.md`, `docs/test-evidence/2026-08-15-postgresql-runtime.md`, `backend/requirements.lock.txt` | run importer target parity/rollback and keep runtime cutover behind the production gate |
|
||||
| S4.3 org/users + groups/personas + sessions/messages repositories | exact-current independent review passed; runtime cutover intentionally not wired | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-3-org-users-repositories.md`, `docs/engineering-log/2026-08-15-s4-3-offline-dialect-remediation.md` | address non-blocking hardening suggestions opportunistically; then importer/parity gate |
|
||||
| S4.4 JSON importer | local SQLite and temporary-local PostgreSQL dry-run/apply/idempotency/conflict-rollback gates passed; target apply blocked | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-4-json-import.md`, `docs/engineering-log/2026-08-15-postgresql-import-gate.md`, `docs/test-evidence/2026-08-15-postgresql-import.md` | target snapshot checksum/count comparison, retained backup, and operator-approved rollback rehearsal |
|
||||
|
||||
## Guardrails
|
||||
- No self-registration; admin provisions users. (Verified: register => 404.)
|
||||
@@ -35,8 +41,27 @@ Informed by MiroFish (CrowdSight engine) + the hermes-brain-and-tools CrowdSight
|
||||
- `2026-08-07-build-out.md` — M0–M7 build-out, decisions, verification, current state.
|
||||
- `2026-08-07-security-ux.md` — security hardening (path traversal, IDOR, XSS) + UX/UI polish.
|
||||
- `2026-08-07-auth-gitea.md` — username login + first-time admin setup + Gitea push.
|
||||
- `2026-08-07-docker-final.md` — Docker build fix: ship prebuilt frontend/dist, no npm in image (resolves repeated `vite: not found`).
|
||||
- `2026-08-07-docker-final.md` — historical prebuilt-`frontend/dist` fix; superseded on 2026-08-15 by the source-built Docker/CI contract in the current Dockerfile.
|
||||
- `2026-08-07-login-after-setup.md` — can't-login-after-setup = deployment data non-persistence, not login logic (verified).
|
||||
- `2026-08-07-login-email-fix.md` — REAL fix: verify() resolves username OR email; "wrong password" after logout→login was email-login not resolving a user.
|
||||
- `2026-08-07-ui-tablayout.md` — 3-tab UI per spec: admin dashboard + date filter, personal dashboard, training w/ difficulty, settings profile.
|
||||
- `2026-08-09-idea-flow-qa-deploy.md` — idea-flow UX cleanups, 2-scenario model + recontact-as-trait, live QA on EasyPanel + auto-deploy, per-turn LLM judge for win/loss, persona variant, 15-persona auto-gen (this session).
|
||||
- `2026-08-13-idea-implementation-audit.md` — source-to-runtime audit; core flow verified, but independent probes reproduced arbitrary account takeover, two tenant bypasses, and additional product/runtime gaps.
|
||||
- `2026-08-13-sprint-1-security-hotfix.md` — S1.1–S1.6 implementation, verification, and deployment warning.
|
||||
- `2026-08-14-sprint-1-current-tree.md` — latest response-boundary and organization-update race fixes, exact verification evidence, and pending reviewer/live gates.
|
||||
- `2026-08-14-final-review.md` — final exact-tree reviewer approval, regression evidence, and explicit operational limits.
|
||||
- `2026-08-15-auth-version-review-reconciliation.md` — stale async auth-version finding, current lock-path verification, and pending fresh exact-tree review.
|
||||
- `2026-08-15-s4-2-schema-foundation.md` — test-first SQLAlchemy/Alembic schema foundation, tenant constraints, dependency lock, and generated-dist cleanup.
|
||||
- `2026-08-15-s4-3-org-users-repositories.md` — org/user repository contracts and SQLAlchemy adapters, with no runtime cutover.
|
||||
- `2026-08-15-s4-3-offline-dialect-remediation.md` — exact-current reviewer finding, test-first offline Alembic guard remediation, and fresh-review status.
|
||||
- `2026-08-15-legacy-security-review.md` — timed-out independent reviews and fail-closed gate handling.
|
||||
- `2026-08-15-legacy-security-review-retries.md` — repeated bounded retry timeouts and final pending reviewer.
|
||||
- `2026-08-15-legacy-security-remediation-verification.md` — historical 247-test verification, timeout handling, and prior scoped-review gate.
|
||||
- `2026-08-15-legacy-rate-limit-migration.md` — current legacy rate-limit migration remediation, 280/4 local evidence, and fresh pending review gates.
|
||||
- `2026-08-15-final-security-gate.md` — 319/166/4 local verification, dependency/tooling blocker, and three clean exact-current scoped reviewer verdicts; production-operation gate remains pending.
|
||||
- `2026-08-15-postgresql-runtime-gate.md` — temporary-local PostgreSQL upgrade, tenant/uniqueness probe, ORM/migration parity, offline DDL, and schema downgrade evidence; target cutover remains pending.
|
||||
- `2026-08-15-postgresql-import-gate.md` — temporary-local PostgreSQL importer dry-run, apply, idempotency, conflict rejection, transaction rollback, and cleanup evidence; target apply remains pending.
|
||||
- `docs/test-evidence/2026-08-15-legacy-security-remediation.md` — test-first remediation, local verification, and independent-review boundary.
|
||||
- `docs/test-evidence/2026-08-15-postgresql-runtime.md` — automated PostgreSQL gate evidence separated from Docker, Redis, importer, and production blockers.
|
||||
- `docs/test-evidence/2026-08-15-postgresql-import.md` — temporary-local PostgreSQL importer evidence and target-operation boundary.
|
||||
- `2026-08-15-s4-4-json-import.md` — fail-closed dry-run/apply importer, idempotency, backup, and parity blockers.
|
||||
|
||||
@@ -8,19 +8,19 @@ first-time admin setup (set email + change password), pushed the repo to Gitea.
|
||||
1. **User id = username** (was email). Email is now a separate optional field with uniqueness.
|
||||
- `UserStore.create_user(username, email=None, ...)`; login via username; JWT `sub` = username.
|
||||
- Admin user-creation uses `username` (email fallback kept for compatibility).
|
||||
2. **Default admin**: `admin` / `1234`, with `must_setup=True`.
|
||||
2. **Default admin**: `admin` / `[REDACTED]`, with `must_setup=True`.
|
||||
3. **Forced first-time setup**: after login with the default creds, the frontend router guards
|
||||
and sends the user to `/setup` — set email + new password (+ confirm), then `must_setup` clears.
|
||||
- Backend: new `POST /api/auth/setup` (`complete_setup` sets email + password, clears flag).
|
||||
- Login response now includes `must_setup`; `me` includes it too.
|
||||
- Frontend: `Login.vue` uses username, `Setup.vue` (new), router guard, `auth.finishSetup`.
|
||||
4. **Docs**: README + HANDOFF + build-out log updated to `admin`/`1234` + setup flow.
|
||||
4. **Docs**: README + HANDOFF + build-out log updated to the historical bootstrap + setup flow.
|
||||
|
||||
## Verification
|
||||
- Added `test_setup.py` (admin/1234 → must_setup → set email+password → old pw invalid, new pw
|
||||
- Added `test_setup.py` (bootstrap → must_setup → set email+password → old pw invalid, new pw
|
||||
works, admin can use app). ALL PASS.
|
||||
- Adapted m0/m1/routes/security/e2e to username creds. **All 6 suites PASS.**
|
||||
- Frontend `npm run build` ok. Live server: `admin`/`1234` login returns `must_setup=true`,
|
||||
- Frontend `npm run build` ok. Live server: bootstrap login returns `must_setup=true`,
|
||||
SPA served at `http://localhost:5001`.
|
||||
|
||||
## Push
|
||||
|
||||
@@ -54,7 +54,7 @@ sessions→debrief(latent reveal+coaching)→one-shot→board→weak-areas→gen
|
||||
|
||||
## Current state / runtime
|
||||
- Backend runs via `cd backend && uv run python run.py`; frontend dev via `cd frontend && npm run dev` (proxies /api -> :5001).
|
||||
- Default super-admin: `admin` / `1234` (bootstrap; forces email + password change on first login).
|
||||
- Default super-admin: `admin` / `[REDACTED]` (historical bootstrap; forces email + password change on first login).
|
||||
- Deploy files: root `Dockerfile`, `docker-compose.yml`, `.env.example`; repo-root `frontend/dist` build.
|
||||
|
||||
## Risks / remaining
|
||||
|
||||
@@ -28,7 +28,7 @@ Single-stage, no node/npm/vite anywhere in commands:
|
||||
- `frontend/dist/` (28 files) tracked and pushed on `origin/main`.
|
||||
- Backend `_register_frontend` path (`Path(__file__).parent.parent.parent/frontend/dist`)
|
||||
matches the Docker `COPY` target `/app/frontend/dist`.
|
||||
- Local server: `GET /` 200 (serves the new tabbed SPA), health ok, `admin`/`1234` login 200.
|
||||
- Local server: `GET /` 200 (serves the new tabbed SPA), health ok, bootstrap login 200.
|
||||
|
||||
## Convoying workflow for UI changes
|
||||
1. `cd frontend && npm run build`
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
# 2026-08-07 — Bug: can't log in after first-run setup (password wrong) — root cause + fix
|
||||
|
||||
## Symptom
|
||||
After first login (admin/1234) → change password in the forced setup screen → logout → logging
|
||||
After first login (admin/[REDACTED]) → change password in the forced setup screen → logout → logging
|
||||
back in with the new password reports "wrong password".
|
||||
|
||||
## Investigation (verified via reproduction)
|
||||
@@ -16,7 +16,7 @@ back in with the new password reports "wrong password".
|
||||
On EasyPanel / any deploy that doesn't mount a volume, the container's `DATA_DIR`
|
||||
(`/app/backend/data`) is **ephemeral — wiped on every container recreate/restart**. So after
|
||||
setup, when the container is recreated:
|
||||
- the changed admin password is lost (record resets to `1234` / `must_setup=true`),
|
||||
- the changed admin password is lost (record resets to `[REDACTED]` / `must_setup=true`),
|
||||
- logging back in with the new password fails ("รหัสผ่านผิด").
|
||||
|
||||
## Fix
|
||||
@@ -29,7 +29,7 @@ setup, when the container is recreated:
|
||||
- **EasyPanel**: add a persistent volume mount → `/app/backend/data` (or set `DATA_DIR` env to a
|
||||
mounted path). After that, data survives redeploys.
|
||||
- If they already lost data (old password gone): delete/reset the container data so a fresh
|
||||
`admin`/`1234` is bootstrapped, then redo setup and NEITHER redeploy without a volume.
|
||||
`admin`/`[REDACTED]` is bootstrapped, then redo setup and NEITHER redeploy without a volume.
|
||||
|
||||
## Note
|
||||
Local dev: `backend/data/` is gitignored and lives on disk, so this only bites container deploys.
|
||||
|
||||
@@ -31,7 +31,7 @@ delivered it to the live deployment.
|
||||
|
||||
## 3. Live QA + the bugs it surfaced
|
||||
Deployed to `moreminimoreapps-saletrainer.ahkhwd.easypanel.host` (auto-deploy via Gitea webhook on
|
||||
push to main). Verified via live API (deterministic) + Chrome via computer-use (login testadmin/1234,
|
||||
push to main). Verified via live API (deterministic) + Chrome via computer-use (login test accounts,
|
||||
testuser/1234).
|
||||
- **Bug (blocker): analyze 500 when LLM returns < 15 personas.** Real deepseek occasionally returns
|
||||
14. Fixed `persona_generator.generate`: **retry up to 3×**, then **accept short (≥ 8)** instead of
|
||||
@@ -72,7 +72,7 @@ test_saas_tenant, test_user_journey, test_variant, test_resume_decision` — **1
|
||||
## Live state
|
||||
- Live serves `index-rfeHzF-F.js` = latest build (verified). Auto-deploy on push (Gitea webhook).
|
||||
- Real LLM (deepseek) requires `LLM_API_KEY` set in EasyPanel env — local `.env` has placeholder.
|
||||
- testadmin=admin, testuser=user (both pass `1234`); bootstrap super-admin also exists.
|
||||
- testadmin=admin, testuser=user (historical test credentials redacted); bootstrap super-admin also exists.
|
||||
- A test group "CRM ระบบจัดการลูกค้า" exists on live (user keeps it; will delete themselves).
|
||||
|
||||
## Open / future (see docs/FUTURE_WORK.md)
|
||||
|
||||
136
docs/engineering-log/2026-08-13-idea-implementation-audit.md
Normal file
136
docs/engineering-log/2026-08-13-idea-implementation-audit.md
Normal file
@@ -0,0 +1,136 @@
|
||||
# 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.
|
||||
136
docs/engineering-log/2026-08-13-sprint-1-security-hotfix.md
Normal file
136
docs/engineering-log/2026-08-13-sprint-1-security-hotfix.md
Normal file
@@ -0,0 +1,136 @@
|
||||
# 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.
|
||||
46
docs/engineering-log/2026-08-14-final-review.md
Normal file
46
docs/engineering-log/2026-08-14-final-review.md
Normal file
@@ -0,0 +1,46 @@
|
||||
# 2026-08-14 — Final exact-tree review gate
|
||||
|
||||
## Scope
|
||||
Closed the post-remediation code-review loop for Sprint 2–4 implementation. No commit, push, deploy, credential rotation, JWT rotation, or public-access change was performed.
|
||||
|
||||
## Remediation verified
|
||||
The final independent review specifically checked and found no blocker in:
|
||||
|
||||
- trainee access to draft/failed/analyzing groups by guessed ID;
|
||||
- hardest-persona ranking (loss count descending, then average score ascending);
|
||||
- atomic group-analysis publication of `sales_kit`, `personas`, `report`, and `status=ready` under the group record lock;
|
||||
- group deletion versus session mutation/start, including per-session record/process locks;
|
||||
- organization active/seat invariants shared by user provisioning and org updates;
|
||||
- first-run bootstrap serialization across workers;
|
||||
- current-user/org/role-bound export authorization plus session `org_id` and user-membership filtering;
|
||||
- nested JSON record-lock reentrancy, tenant-bound session scope, and mutation paths.
|
||||
|
||||
Five regression tests were added in `backend/tests/test_final_review_regressions.py` for the concrete review findings.
|
||||
|
||||
## Verification evidence
|
||||
- Canonical backend suite: **152 passed** (`backend/.venv/bin/python -m pytest -q backend/tests`).
|
||||
- Final-review regression subset: **5 passed** (`backend/.venv/bin/python -m pytest -q backend/tests/test_final_review_regressions.py`).
|
||||
- Executable backend scripts: **12/12 passed**.
|
||||
- Frontend production build: **1,778 modules transformed**; temporary artifact check: **2 asset references / 0 missing**.
|
||||
- `compileall`, `git diff --check`, and CI YAML parsing: passed.
|
||||
- Added-line static security scan: no hardcoded-secret assignment, shell injection, dangerous `eval/exec`, pickle deserialization, or formatted SQL execution patterns detected.
|
||||
|
||||
## Independent reviewer verdict
|
||||
A fresh bounded reviewer inspected the exact current tree and the nine relevant implementation/test files without modifying them:
|
||||
|
||||
```json
|
||||
{
|
||||
"passed": true,
|
||||
"security_concerns": [],
|
||||
"logic_errors": [],
|
||||
"suggestions": [
|
||||
"The direct pytest invocation for tests/test_final_review_regressions.py reported no tests collected under the current configuration; rerun through the repository's intended test command.",
|
||||
"Consider adding an explicit interleaving test for group deletion versus concurrent session mutation/start to complement the lock-presence regression."
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
The first suggestion was verified with the repository-root command above and passed 5 tests. The second is non-blocking: lock-presence and existing concurrency tests pass, while a true multi-process interleaving test remains useful future hardening.
|
||||
|
||||
## Remaining gates
|
||||
Docker is unavailable on this Mac, so image build/runtime was not exercised locally; CI contains the clean multi-stage build and `/ready` smoke path. Real-provider QA, browser/mobile E2E, PostgreSQL/Redis validation, production authenticated smoke, restricted deploy, bootstrap-password rotation, and JWT rotation remain operational gates requiring explicit operator approval where applicable.
|
||||
68
docs/engineering-log/2026-08-14-sprint-1-current-tree.md
Normal file
68
docs/engineering-log/2026-08-14-sprint-1-current-tree.md
Normal file
@@ -0,0 +1,68 @@
|
||||
# 2026-08-14 — Sprint 1 current-tree security verification
|
||||
|
||||
## Status
|
||||
|
||||
Sprint 1 code changes remain uncommitted. Local verification is complete and passing, and the exact-current-tree independent reviewer gate is closed. The separate live-operation gate is pending. No commit, push, deploy, credential/JWT rotation, or public-access change was performed.
|
||||
|
||||
## Latest remediation
|
||||
|
||||
After the earlier password-policy finding, the current tree also closes these response and tenant-boundary classes:
|
||||
|
||||
- `UserStore` and HTTP/admin password paths share the 12-character minimum; weak/non-string credentials fail before mutation.
|
||||
- User/admin serialization uses a closed allowlist; `password_hash` and future internal user fields are omitted from listing and mutation responses.
|
||||
- Authenticated group/private-persona mutations reject missing tenant identity instead of falling back to `org-default`.
|
||||
- Admin organization `plan`, `seats`, `active`, and `new_org` inputs use strict types.
|
||||
- Provider-controlled final judge output is constrained to a closed debrief envelope.
|
||||
- Chat session start/resume/send/finish/list/get responses use a closed session serializer and omit `user_id`, hidden `internal` judge state, provider metadata, and unknown message fields.
|
||||
- Group serializers fail closed for malformed stored input/persona shapes; legacy group errors remain fixed public codes.
|
||||
- A bounded exact-tree review (`deleg_fccee30d`) identified that organization admin PATCH updates used a read-modify-write path without cross-process locking; `JsonStore.update()` now uses the same per-record process lock as conditional updates.
|
||||
- `test_org_updates_preserve_concurrent_fields_across_processes` reproduces the stale-read race and verifies that concurrent `active` and `seats` changes both persist.
|
||||
|
||||
The deferred private-group filesystem scan/create collection race remains a Sprint 2/S2.1 task and is not represented as fixed.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
Executed from the repository root:
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest backend/tests -q
|
||||
97 passed in 23.50s
|
||||
|
||||
This is the post-remediation exact current-tree run after the organization
|
||||
admin update race fix. The earlier completed background run, `proc_ba80f06ca949`,
|
||||
returned `91 passed in 928.85s (0:15:28)` and is retained as historical
|
||||
supporting evidence.
|
||||
|
||||
Another earlier background run, `proc_efcee6c93382`, returned `93 passed in
|
||||
23.11s` before the current collection state and is also historical supporting
|
||||
evidence.
|
||||
|
||||
backend/scripts/test_*.py
|
||||
12/12 passed
|
||||
|
||||
cd frontend && npm run build
|
||||
passed; 1,772 modules transformed
|
||||
|
||||
backend/.venv/bin/python -m compileall -q backend/app backend/tests backend/scripts
|
||||
AST parse + git diff --check
|
||||
passed
|
||||
|
||||
Added-line security scan:
|
||||
hardcoded_secret=0
|
||||
shell_injection=0
|
||||
eval_exec=0
|
||||
pickle=0
|
||||
sql_format=0
|
||||
```
|
||||
|
||||
Additional focused checks for the latest response-boundary changes passed, including user-list redaction, provider-debrief allowlisting, session-internal redaction, malformed group serialization, strict organization field types, and tenant fallback removal.
|
||||
|
||||
## Reviewer and operational gates
|
||||
|
||||
The exact-tree read-only reviewer `deleg_6e386e71` timed out after 600.19 seconds and returned no schema-valid JSON. The earlier two-workstream batch, `deleg_da73aca4`, also timed out in both workstreams. The bounded exact-tree reviewer `deleg_fccee30d` returned schema-valid `passed=false` and found one concrete cross-process organization-update race: `JsonStore.update()` lacked a process lock. That finding was remediated in `backend/app/storage/store.py` and covered by the new regression test above. The fresh post-remediation exact-tree reviewer `deleg_7bcf0dfd` returned the required schema-valid verdict `passed=true`, with `security_concerns=[]` and `logic_errors=[]`. This closes the Sprint 1 code/reviewer gate. Prior reviewer timeouts, packet/scoped verdicts, and delegation completion status are not approval.
|
||||
|
||||
Docker is unavailable locally. Real-provider LLM QA, browser/mobile QA, restricted deployment, bootstrap credential change, `JWT_SECRET` rotation, audit inspection, and authenticated smoke have not been run. Public/untrusted access must remain disabled.
|
||||
|
||||
## Next action
|
||||
|
||||
The next gate is the separate operator-approved restricted deployment and credential/JWT rotation checklist. Do not restore public/untrusted access or start Sprint 2 until that live-operation gate is explicitly completed.
|
||||
23
docs/engineering-log/2026-08-14-sprint-2-4-verification.md
Normal file
23
docs/engineering-log/2026-08-14-sprint-2-4-verification.md
Normal file
@@ -0,0 +1,23 @@
|
||||
# 2026-08-14 — Sprint 2–4 current-tree verification
|
||||
|
||||
## Scope
|
||||
Completed the remaining local implementation packets for persona/session correctness, trainee weak-area loops, private personas, report redaction, and production runtime foundations. No commit, push, deploy, credential rotation, JWT rotation, or public-access change was performed.
|
||||
|
||||
## Implemented
|
||||
- Canonical session identity, resume/finish idempotency, per-turn/final judge invariants, preview isolation, and private-group race protection.
|
||||
- Natural-text persona reply normalization with bounded visible output and internal-state redaction.
|
||||
- Deterministic auto-15 persona cardinality contract (5 per tier), no append generation, initiation/channel contracts, and lineage-safe variants.
|
||||
- Weak-area analysis and private persona generation UI/API; human-readable redacted group report.
|
||||
- Production foundations: `/ready`, Gunicorn WSGI entrypoint, Docker healthcheck, explicit `CORS_ORIGINS`, `.dockerignore`, and `.gitea/workflows/ci.yml`.
|
||||
- Reviewer remediation: added re-entrant per-record transaction locks for session/group mutations, serialized chat send/finish and group analysis, bound export links to the current user/org/role, fixed `SessionDetail` API wiring and group-summary channel serialization, made session initialization atomic, and serialized variant appends.
|
||||
- Production artifact flow: Docker now builds the Vue SPA inside a Node builder stage; CI builds to a temporary artifact and verifies all referenced assets, so clean clones do not depend on stale checked-in hashed bundles.
|
||||
|
||||
## Verification evidence
|
||||
- Backend pytest: **152 passed** (`backend/tests`; the repository-wide default also collects executable scripts, so the canonical suite is explicit), including **5 final-review regression tests**.
|
||||
- Executable backend scripts: **12 passed**.
|
||||
- Frontend production build: **1,778 modules transformed**, success; asset reference check: **2 references, 0 missing** (built in a temporary output directory).
|
||||
- Python compileall and `git diff --check`: passed.
|
||||
- Earlier exact-current-tree reviewer before the final remediation failed on stale export scope, non-reproducible checked-in dist, session initialization race, unlocked variant append, draft-group exposure, reversed hardest-persona ranking, ready-state publication ordering, cascade deletion locking, and org/bootstrap races. Those findings were remediated and covered by regression tests. A fresh bounded exact-current-tree reviewer then passed with empty security and logic blocker arrays; see `docs/engineering-log/2026-08-14-final-review.md`.
|
||||
|
||||
## Explicit limits
|
||||
Docker is unavailable on this Mac, so image build/runtime was not exercised locally; CI now builds the multi-stage image and smoke-tests `/ready`. Real-provider QA, browser E2E, mobile visual QA, PostgreSQL migration, and production authenticated smoke remain operational/future gates. The Sprint 1 live gate remains blocked pending explicit approval for restricted deploy and secret rotations.
|
||||
@@ -0,0 +1,34 @@
|
||||
# 2026-08-15 — Auth-version review reconciliation
|
||||
|
||||
## Incident / review result
|
||||
|
||||
An asynchronous reviewer packet returned `passed=false` with one claimed blocker: a concurrent self-service password change and admin password reset could both compute the same next `auth_version`. That verdict was produced from an earlier tree state and was not treated as approval for the current tree.
|
||||
|
||||
## Current root-cause check
|
||||
|
||||
The current `UserStore.update_user_fields()` path acquires `users.collection_lock()` and then `users.record_lock(username)` before reading the current user or computing the next `auth_version`. `change_password()` acquires the same per-user record lock before reading the current hash/version and writing the replacement hash. `complete_setup()` uses the same collection-lock → record-lock order. The only production password-bearing mutations found are in `backend/app/auth/users.py`.
|
||||
|
||||
The existing deterministic regression `test_concurrent_admin_reset_cannot_reuse_stale_auth_version` specifically gates a self-service `change_password()` against the `set_password()` / `update_user_fields(password=...)` path and asserts the version advances twice. It passed on the current tree.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- `./.venv/bin/python -m pytest tests/test_password_change.py -q` — **9 passed**.
|
||||
- `./.venv/bin/python -m pytest -q` — **176 passed**.
|
||||
- Executable backend scripts — **12/12 passed**.
|
||||
- `./.venv/bin/python -m compileall -q app tests` — passed.
|
||||
- `git diff --check` — passed.
|
||||
- Current source scan found no production password mutation outside `backend/app/auth/users.py`.
|
||||
|
||||
## Review gate
|
||||
|
||||
The surgical independent review of the current password-session path returned a complete five-key verdict with `passed=true`, `security_concerns=[]`, and `logic_errors=[]`. It independently confirmed that `change_password()`, `update_user_fields(password=...)`, `set_password()`, and `complete_setup()` serialize `auth_version` reads/increments under the shared per-user record lock.
|
||||
|
||||
The **full exact-tree review remains pending**: several broader reviewer runs were interrupted before returning the required JSON schema. Their partial outputs are not approval and contain no valid blocking finding.
|
||||
|
||||
## Operational limits
|
||||
|
||||
No commit, push, deploy, public-access change, credential rotation, or authenticated production smoke was performed. Docker, PostgreSQL, Redis/persistent audit verification, real-provider QA, and production browser/mobile verification remain pending.
|
||||
|
||||
## Next action
|
||||
|
||||
Obtain a complete full exact-tree reviewer verdict. If it passes, keep the live-operation gate closed until explicit operator approval for restricted deployment, credential/JWT rotation, audit inspection, and authenticated smoke.
|
||||
53
docs/engineering-log/2026-08-15-final-security-gate.md
Normal file
53
docs/engineering-log/2026-08-15-final-security-gate.md
Normal file
@@ -0,0 +1,53 @@
|
||||
# Final security gate verification — 2026-08-15
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: local code/security gate passed; production-operation gate remains pending
|
||||
|
||||
## Scope
|
||||
|
||||
This entry closes the post-remediation exact-current review for authentication, storage, durable rate limiting, tenant/group/session isolation, analytics/export authorization, bounded parsing, and upload handling. JSON stores remain runtime-authoritative. SQLAlchemy/Alembic repositories and importer remain local-only; no runtime cutover was performed.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- `cd backend && .venv/bin/python -m pytest -q --tb=short` — **319 passed in 53.15s**.
|
||||
- Focused auth/isolation/export/upload suite — **166 passed in 24.48s**.
|
||||
- `cd frontend && npm run test:unit` — **4 passed**.
|
||||
- `compileall` and AST parsing — passed.
|
||||
- `git diff --check` — passed.
|
||||
- Added-line static security scan — no hardcoded secrets, shell execution, dynamic `eval`/`exec`, pickle loads, or formatted SQL query patterns detected.
|
||||
- An earlier shared-environment `pip check` was blocked because `alibabacloud-tea-openapi 0.4.4` required `cryptography<47.0.0` while `cryptography 50.0.0` was installed; the exact-current isolated lock-file check below supersedes that result for repository reproducibility, not for the stale local venv.
|
||||
- `ruff`, `mypy`, `eslint`, and `tsc` are unavailable in this environment.
|
||||
|
||||
## Exact-current re-verification
|
||||
|
||||
- Clean isolated environment from `backend/requirements.lock.txt`: **319 passed in 57.62s** and `pip check` reported **no broken requirements**.
|
||||
- Existing `backend/.venv`: version-drifted; `packaging` and `pygments` are present, but `pip check` reports the pre-existing `alibabacloud-tea-openapi 0.4.4` requirement conflict with installed `cryptography 50.0.0`. This is a local environment issue, not a source or lock-file regression.
|
||||
- Frontend unit suite: **4 passed**.
|
||||
- Playwright fixture journeys using installed Google Chrome: **12 passed in 9.6s** across desktop 1440×900, mobile 320×568, and mobile 500×768.
|
||||
- Frontend production build: **1,781 modules transformed**; `npm audit --audit-level=high`: **0 vulnerabilities**.
|
||||
- `compileall`, AST parse (**38 files, 0 errors**), and `git diff --check`: passed.
|
||||
- Temporary local PostgreSQL: Alembic upgrade, 7-table tenant/uniqueness runtime probe, ORM-vs-migration parity, offline PostgreSQL DDL, and downgrade to base all passed; temporary databases were cleaned up.
|
||||
- Temporary local PostgreSQL importer: dry-run, first apply, idempotent second apply, metadata-only output, conflict rejection with transaction rollback, and downgrade cleanup all passed against disposable fixtures.
|
||||
- Docker remains unavailable locally, so image build and container `/ready` smoke remain unverified.
|
||||
|
||||
These are local verification results, not production approval.
|
||||
|
||||
## Fresh independent review gate
|
||||
|
||||
Three fresh exact-current read-only scoped reviewers returned complete five-key JSON verdicts. Each had `passed=true`, `security_concerns=[]`, and `logic_errors=[]`:
|
||||
|
||||
- `deleg_e835e807` — auth, storage, JWT identity binding, malformed records, durable rate-limit locking/migration.
|
||||
- `deleg_4ec7eb5a` task 0 — tenant, group ownership/private groups, session/chat scope, persona redaction, weak-area evidence.
|
||||
- `deleg_4ec7eb5a` task 1 — analytics/export/report authorization, signed-link redemption, bounded scans/rows/bytes, parser and upload limits/cleanup.
|
||||
|
||||
Reviewer suggestions are non-blocking hardening only: preserve regression coverage, consider rejecting invalid date filters instead of treating them as unbounded, add explicit mode checks in the shared session authorization helper, and document the intended global super-admin export scope.
|
||||
|
||||
Earlier timed-out, interrupted, stale, or incomplete reviewer runs were not counted as approval.
|
||||
|
||||
## Operational boundary
|
||||
|
||||
No commit, push, deploy, public-access restoration, production migration, credential rotation, or authenticated production smoke occurred. Docker image/runtime, Redis persistence, real target PostgreSQL snapshot parity/retained-backup rollback, runtime repository cutover, real-provider QA, production-authenticated browser/mobile QA, and production verification remain pending. The temporary-local PostgreSQL schema and importer gates passed, but production readiness is therefore **not approved**.
|
||||
|
||||
## Next action
|
||||
|
||||
Keep the application behind the restricted-operation gate. A future operator-approved deployment must still perform bootstrap credential setup, JWT secret rotation, audit inspection, authenticated smoke, and production persistence/parity checks before public access or runtime repository cutover.
|
||||
@@ -0,0 +1,33 @@
|
||||
# 2026-08-15 — Legacy rate-limit migration remediation
|
||||
|
||||
## Context
|
||||
|
||||
The exact-current independent review `deleg_4357d521` returned a valid five-key JSON verdict with one blocking finding: the new directory-backed rate limiter did not read or migrate the legacy `DATA_DIR/ratelimit.json` state. A deployment could therefore silently reset persisted brute-force counters.
|
||||
|
||||
## Remediation
|
||||
|
||||
- Added a lock-protected, idempotent migration from `ratelimit.json` into the directory-backed `JsonStore`.
|
||||
- Preserved the legacy file and write a migration sidecar only after all records are migrated successfully.
|
||||
- Merged existing records instead of overwriting newer counters.
|
||||
- Rejected malformed legacy state and invalid timestamps through the existing fail-closed rate-limit path.
|
||||
- Added a regression for legacy counters so an existing active attempt blocks instead of resetting.
|
||||
- Routed direct `by_email()` lookups through canonical email validation.
|
||||
- Added regression coverage for deactivate/reactivate auth-version revocation.
|
||||
|
||||
## Verification
|
||||
|
||||
- Targeted legacy/auth/rate-limit/password suites: 125 passed.
|
||||
- Backend full suite: 280 passed.
|
||||
- Frontend unit suite: 4 passed.
|
||||
- `compileall`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Added-line security scan: no hardcoded secrets, shell-command injection, dynamic-code, unsafe-deserialization, or formatted-SQL matches.
|
||||
- No commit, push, deploy, production mutation, credential access, or credential rotation.
|
||||
|
||||
## Independent gate status
|
||||
|
||||
The remediation invalidated the earlier review snapshot. Three fresh exact-current, read-only review scopes were dispatched after the final source change. They remain pending; no reviewer status or incomplete output is treated as approval. The gate closes only when each scope returns valid JSON with exactly the required five keys, `passed=true`, and empty `security_concerns` and `logic_errors`.
|
||||
|
||||
## Remaining operational limits
|
||||
|
||||
PostgreSQL runtime/parity, Docker build/smoke, Redis persistence, real-provider LLM QA, browser/mobile E2E, authenticated production smoke, deployment, public access, and credential/JWT rotation remain unperformed and require separate approval.
|
||||
@@ -0,0 +1,44 @@
|
||||
# Legacy security remediation verification — 2026-08-15
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: in progress; local verification complete, final exact-current independent gate pending
|
||||
|
||||
## Scope
|
||||
|
||||
This entry records the post-remediation verification after the additional exact-current findings covering falsey ownership markers, whitespace/path-like tenant IDs, malformed persona collections, non-finite JWT/debrief values, rate-limit initialization, analytics/export tenant/private scope, bounded PDF extraction, aggregate upload budgets, and upload cleanup.
|
||||
|
||||
## Implementation state
|
||||
|
||||
The current working tree contains minimal root-cause fixes with regression coverage. JSON stores remain runtime-authoritative. SQLAlchemy/Alembic repositories remain local-only and are not wired into runtime storage.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- `backend/.venv/bin/python -m pytest -q backend/tests` — **247 passed in 46.55s**.
|
||||
- `backend/.venv/bin/python -m pytest -q backend/tests/test_legacy_security_regressions.py backend/tests/test_upload_security.py backend/tests/test_final_review_regressions.py backend/tests/test_weak_area_analysis.py` — **54 passed in 5.23s**.
|
||||
- `backend/.venv/bin/python -m compileall -q backend/app backend/tests` — passed.
|
||||
- AST parse of backend application and test files — **70 files passed**.
|
||||
- `git diff --check` — passed.
|
||||
- Added-line security scans — no hardcoded secrets, shell execution, dynamic `eval`/`exec`, pickle loads, or formatted SQL query patterns detected.
|
||||
- `ruff` and `mypy` were unavailable in the local environment.
|
||||
|
||||
These are local remediation evidence only; they are not production approval.
|
||||
|
||||
## Independent review status
|
||||
|
||||
The full-scope exact-current reviewer `deleg_93ef64c5` timed out after 600 seconds without a complete five-key JSON verdict. It is no approval and no findings are inferred from the timeout.
|
||||
|
||||
A replacement batch `deleg_df17f04e` was stopped because it began before the final service-boundary hardening. The final code-freeze batch `deleg_6885e5b9` is running:
|
||||
|
||||
- authentication, storage, and rate-limit boundaries;
|
||||
- tenant, group, ownership, persona, and `/me` boundaries;
|
||||
- trainee analysis, analytics/export, parser, and upload boundaries.
|
||||
|
||||
The legacy-security gate remains blocked until every required slice returns valid JSON with exactly the five required keys, `passed=true`, and empty `security_concerns` and `logic_errors` arrays.
|
||||
|
||||
## Operational boundary
|
||||
|
||||
No commit, push, deploy, public-access restoration, production migration, credential rotation, or authenticated production smoke occurred. Docker, PostgreSQL runtime/parity, real-provider QA, browser/mobile QA, and production verification remain pending.
|
||||
|
||||
## Next action
|
||||
|
||||
Validate all three `deleg_6885e5b9` payloads when they return. If any reviewer times out, truncates, emits incomplete JSON, or reports a blocking finding, keep the gate blocked and remediate only verified findings before rerunning the affected scope.
|
||||
@@ -0,0 +1,38 @@
|
||||
# Legacy security review — repeated bounded retries
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: blocked; no independent approval yet
|
||||
|
||||
## What changed
|
||||
|
||||
The legacy-security review gate remains fail-closed after three independent reviewer attempts timed out without a complete verdict:
|
||||
|
||||
- `deleg_ffba9adf` — timeout after 600.11 seconds; no JSON
|
||||
- `deleg_03d06f1f` — timeout after 600.10 seconds; no JSON
|
||||
- `deleg_0cc095f8` — timeout after 600.09 seconds; no JSON
|
||||
|
||||
All three are **no verdict**, not approval. No reviewer was allowed to edit files, run production operations, use network access, or rotate credentials.
|
||||
|
||||
A final one-call bounded reviewer, `deleg_e61f99ff`, completed the extraction but returned `passed=false` because 93,176 characters of requested function bodies were omitted by the tool capture window. This is a complete five-key payload, but it is explicitly a limitation/no-approval verdict.
|
||||
|
||||
Three fresh scoped exact-current reviewers are now running in parallel under `deleg_a274603c`:
|
||||
|
||||
- tenant/group/persona redaction and trainee analytics
|
||||
- auth fields/tokens, JsonStore locking/corruption, and fail-closed rate limits
|
||||
- analytics/export tokens, CSV formula injection, upload preflight, and API/IDOR boundaries
|
||||
|
||||
Each must return exactly the five required keys. A scoped review counts only for its own area; the overall gate remains blocked until every area has a valid `passed=true` verdict with empty blocking arrays.
|
||||
|
||||
## Local evidence
|
||||
|
||||
- Backend regression evidence remains `209 passed` from the current working tree.
|
||||
- Documentation diff checks were rerun after recording the retry state.
|
||||
- Local tests are not a substitute for the required independent verdict.
|
||||
|
||||
## Operational boundary
|
||||
|
||||
No commit, push, deploy, public access restoration, production migration, credential rotation, or live authenticated smoke occurred. JSON stores remain runtime-authoritative.
|
||||
|
||||
## Next action
|
||||
|
||||
Validate `deleg_e61f99ff` when it returns. If it times out or returns incomplete/unparseable JSON, keep the legacy-security gate blocked and do not infer findings or approval.
|
||||
32
docs/engineering-log/2026-08-15-legacy-security-review.md
Normal file
32
docs/engineering-log/2026-08-15-legacy-security-review.md
Normal file
@@ -0,0 +1,32 @@
|
||||
# Legacy security review gate — repeated timeout and final bounded retry
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: blocked pending a valid exact-current independent verdict; no approval claimed
|
||||
|
||||
## Scope
|
||||
|
||||
The reviewer was asked to inspect only the current remediation files and directly relevant tests covering group/tenant redaction, trainee analytics, malformed-auth fail-closed behavior, cross-process JSON-store locking, rate-limit fail-closed behavior, CSV formula-injection protection, and upload-size preflight.
|
||||
|
||||
## Review result
|
||||
|
||||
- Reviewer `deleg_ffba9adf` ran for 600.11 seconds and returned status `timeout` with no JSON payload.
|
||||
- Under the review gate, timeout is **no verdict**, not approval. No blocking finding can be inferred from the incomplete run, and no approval can be inferred either.
|
||||
- Replacement reviewer `deleg_03d06f1f` also ran for 600.10 seconds and returned status `timeout` with no JSON payload; it is likewise **no verdict**.
|
||||
|
||||
## Action taken
|
||||
|
||||
- Dispatched bounded retry `deleg_03d06f1f` against the exact current tree; it timed out without JSON.
|
||||
- Retry instructions prohibit edits, deploys, credential operations, web/network access, and full-suite execution; the reviewer must return the complete five-key JSON or explicitly fail if it cannot complete the scope.
|
||||
- Dispatched stricter retry `deleg_0cc095f8` with an eight-call file-read budget and no terminal/test/network access; it also timed out after 600.09 seconds without JSON.
|
||||
- Dispatched final one-call retry `deleg_e61f99ff`; extraction completed, but the output exceeded the capture window, so the reviewer returned `passed=false` with a limitation and did not provide an approval verdict.
|
||||
- Dispatched three fresh scoped exact-current reviewers in parallel (`deleg_a274603c`, three leaf reviews) to avoid another oversized extraction: tenant/group/analytics, auth/store/rate-limit, and export/upload/API boundaries.
|
||||
|
||||
## Local evidence and limits
|
||||
|
||||
- The local backend regression suite is now green at `209 passed`; the earlier `208 passed` count was the pre-offline-guard baseline.
|
||||
- This is local evidence only. No production data, credentials, deployment, or live operation was touched.
|
||||
- A valid independent verdict is still required before closing the legacy-security review gate.
|
||||
|
||||
## Exact next action
|
||||
|
||||
Validate `deleg_e61f99ff` only if it returns exactly `passed`, `security_concerns`, `logic_errors`, `suggestions`, and `summary`. Treat it as approval only when `passed=true` and both blocking arrays are empty. The three timeout results remain no verdicts.
|
||||
22
docs/engineering-log/2026-08-15-postgresql-import-gate.md
Normal file
22
docs/engineering-log/2026-08-15-postgresql-import-gate.md
Normal file
@@ -0,0 +1,22 @@
|
||||
# PostgreSQL Importer Gate — 2026-08-15
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: temporary-local PostgreSQL importer gate passed; target-data cutover remains pending
|
||||
|
||||
## Scope
|
||||
|
||||
This entry records S4.4 importer execution against the migrated schema in a randomly named temporary local PostgreSQL database. The source was a disposable fixture based on `backend/tests/test_json_import.py`; it contained no production data. No production database, JSON store, credential, deployment, or runtime cutover was touched.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- CLI dry-run — **passed**; metadata-only report returned expected counts: 1 organization, 1 user, 2 groups, 2 personas, 1 session, and 2 messages. No password hash or connection string appeared in serialized output.
|
||||
- First CLI apply — **passed**; created 1 organization, 1 user, 2 groups, 2 personas, 1 session, and 2 messages. A fresh source backup directory was created before the target transaction.
|
||||
- Second CLI apply — **passed**; created 0 rows and reported all existing rows as `unchanged`.
|
||||
- Target row-count verification — **passed**; PostgreSQL contained exactly the expected fixture counts.
|
||||
- Conflict/transaction rollback — **passed**; after adding new org/user/group rows and changing an already-imported group, the importer returned its generic rejection code, preserved the original group, and rolled back the new rows that were queued earlier in the same transaction.
|
||||
- Alembic downgrade to base — **passed**; zero application tables remained.
|
||||
- Temporary database, fixture, and backup directories were cleaned up after the probe.
|
||||
|
||||
## Boundaries
|
||||
|
||||
This closes the **local fixture-to-PostgreSQL importer behavior** gate only. It does not approve applying a real JSON snapshot, audit migration, Redis persistence, runtime repository cutover, production rollback, deployment, or public access. A real target operation still needs an independently retained backup, target checksum/count comparison, operator approval, and a rollback rehearsal against the actual target environment.
|
||||
45
docs/engineering-log/2026-08-15-postgresql-runtime-gate.md
Normal file
45
docs/engineering-log/2026-08-15-postgresql-runtime-gate.md
Normal file
@@ -0,0 +1,45 @@
|
||||
# PostgreSQL Runtime Gate — 2026-08-15
|
||||
|
||||
Date: 2026-08-15 22:35 +0700
|
||||
Status: local PostgreSQL schema/runtime gate passed; production target cutover remains pending
|
||||
|
||||
## Scope
|
||||
|
||||
This entry records the safe local S4.2 runtime verification performed after the code/schema review. It used randomly named temporary PostgreSQL databases on the local service only. No production database, JSON store, credential, deployment, or runtime cutover was touched.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- `pg_isready -h 127.0.0.1 -p 5432` — **accepting connections**.
|
||||
- `alembic upgrade head` against a temporary local PostgreSQL database — **passed** for migrations `0001_initial_schema` and `0002_audit_actor_tenant_check`.
|
||||
- Runtime PostgreSQL probe — **passed**:
|
||||
- all 7 application tables present;
|
||||
- non-preview partial unique index preserved;
|
||||
- audit actor tenant check constraint present;
|
||||
- valid tenant fixture inserted;
|
||||
- cross-tenant group-owner FK rejected;
|
||||
- cross-tenant audit-actor FK rejected;
|
||||
- partial audit actor attribution rejected;
|
||||
- duplicate non-preview attempt rejected;
|
||||
- duplicate preview attempt accepted.
|
||||
- ORM-vs-Alembic PostgreSQL parity probe — **passed**: 7 application tables, columns, indexes, unique constraints, foreign keys, and check constraints matched.
|
||||
- Offline PostgreSQL Alembic SQL render — **passed**: generated DDL contained `organizations`, `sessions`, the non-preview predicate, the audit check constraint, and transactional `COMMIT`.
|
||||
- `alembic downgrade base` — **passed**; remaining application tables in the temporary migration database: `0`.
|
||||
- Cleanup — both temporary databases were dropped by the shell cleanup trap.
|
||||
|
||||
The bundled generic SQLite parity helper was also run. Its non-zero diagnostic result consisted of the expected `alembic_version` bookkeeping-table difference plus a MySQL partial-index warning. The project’s supported-dialect guard rejects MySQL before migration/SQL rendering; the target PostgreSQL parity probe above is the relevant runtime evidence.
|
||||
|
||||
## Environment boundary
|
||||
|
||||
- `backend/.venv` remains version-drifted and `pip check` reports the pre-existing conflict: `alibabacloud-tea-openapi 0.4.4` requires `cryptography<47.0.0`, while `cryptography 50.0.0` is installed.
|
||||
- `uv pip sync --dry-run --python backend/.venv/bin/python backend/requirements.lock.txt` proposed package alignment but was not applied; no existing venv was mutated.
|
||||
- The isolated `requirements.lock.txt` environment remains the reproducible verification environment.
|
||||
- Docker and `redis-cli` are unavailable locally. PostgreSQL schema verification does not close the Docker, Redis persistence, or production-operation gates.
|
||||
|
||||
## Remaining gates
|
||||
|
||||
- Run JSON importer parity against a real target PostgreSQL snapshot, including backup and data rollback rehearsal.
|
||||
- Verify runtime repository wiring and transaction behavior before any cutover; JSON stores remain authoritative.
|
||||
- Verify Redis-backed/persistent audit and rate-limit behavior.
|
||||
- Run Docker build, container `/ready` smoke, real-provider QA, authenticated production smoke, and operator-approved deployment steps separately.
|
||||
|
||||
No commit, push, deploy, credential rotation, public-access change, production migration, or production data mutation occurred.
|
||||
86
docs/engineering-log/2026-08-15-s4-2-schema-foundation.md
Normal file
86
docs/engineering-log/2026-08-15-s4-2-schema-foundation.md
Normal file
@@ -0,0 +1,86 @@
|
||||
# S4.2 — Relational schema foundation
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: implementation + local verification complete; dependency lock wired; offline-dialect remediation verified; code/schema/dependency review passed; PostgreSQL runtime/cutover blocked
|
||||
|
||||
## Scope actually changed
|
||||
|
||||
- `backend/requirements.txt` — pinned direct runtime/test dependencies, including SQLAlchemy, Alembic, and psycopg PostgreSQL driver.
|
||||
- `backend/requirements.lock.txt` — uv-generated transitive lock with hashes; Docker and backend CI install this file with `--require-hashes`.
|
||||
- `backend/app/config.py` — optional `DATABASE_URL` configuration.
|
||||
- `backend/app/db.py` — engine/session helpers with SQLite foreign-key enforcement.
|
||||
- `backend/app/models/entities.py` and `backend/app/models/__init__.py` — tenant-safe relational entities for organizations, users, groups, personas, sessions, messages, and audit events.
|
||||
- `backend/alembic.ini`, `backend/migrations/env.py`, and `backend/migrations/versions/0001_initial_schema.py` / `0002_audit_actor_tenant_check.py` — portable migration wiring and audit tenant hardening.
|
||||
- `backend/tests/test_db_schema.py` — schema, tenant-FK, one-shot uniqueness, and migration up/down contract tests.
|
||||
- Generated `frontend/dist/` artifacts removed from the worktree; the repository policy is source-only frontend plus reproducible Docker/CI builds.
|
||||
|
||||
## Tests written first
|
||||
|
||||
- `test_schema_contains_core_tables_and_tenant_keys`: failed before implementation because `app.db`/Alembic were unavailable; passes after implementation.
|
||||
- `test_constraints_preserve_tenant_links_and_attempt_uniqueness`: initially exposed SQLite FK fixture configuration and ORM insert ordering; after fixing the fixture it passed, then a RED regression was added for cross-org audit actors.
|
||||
- Cross-org audit actor regression: failed before the composite actor/org FK; passes after the model and migration were updated together.
|
||||
- Partial-null audit actor regression: failed before `ck_audit_actor_requires_org`; passes after the check constraint was added to ORM metadata and migration `0002`.
|
||||
- Raw SQL default regression: failed against `Base.metadata.create_all` before model `server_default` values were aligned with Alembic; passes after alignment.
|
||||
- `test_alembic_migration_up_and_down`: failed before Alembic was installed; passes after the migration foundation was added.
|
||||
- Root-CWD migration regression: failed with CWD-relative `script_location`; passes after using `%(here)s` paths in `alembic.ini`.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- `cd backend && ./.venv/bin/python -m pytest tests/test_db_schema.py -q` → `8 passed` after the offline unsupported-dialect regression was added.
|
||||
- `cd /Users/kunthawat/Gitea/Sales Trainer && backend/.venv/bin/python -m pytest -q` → `209 passed` after the offline guard remediation.
|
||||
- Clean Python 3.11 temporary venv installed `backend/requirements.lock.txt` with `pip --require-hashes`; full backend suite → `195 passed in 38.95s`.
|
||||
- uv regenerated the lock from pinned requirements: `42 packages`, `810 lines`; normalized package-entry comparison was identical.
|
||||
- Dockerfile and `.gitea/workflows/ci.yml` now install the hash-locked requirements file.
|
||||
- `cd backend && for script in scripts/test_*.py; do ./.venv/bin/python "$script"; done` → `12 executable scripts passed`.
|
||||
- Temporary SQLite `alembic upgrade head` + `alembic check` + `alembic downgrade base` → passed after the JSON-parity and audit actor FK changes.
|
||||
- Alembic upgrade/check/downgrade → passed from both repository-root and `backend/` CWDs, including migration `0002`.
|
||||
- Bundled schema parity probe found only the expected `alembic_version` bookkeeping table difference; MySQL partial-index weakening is now prevented on both online and offline Alembic paths by the shared dialect guard and regression tests.
|
||||
- `python -m compileall -q app migrations tests` → passed.
|
||||
- `git diff --check` → passed.
|
||||
- Added-line static security scan → `added_line_findings=[]`.
|
||||
- `ruff` and `mypy` were not installed in the local environment; both were skipped and not represented as passing.
|
||||
- Full-stack contract audit → API contract clean; no committed/generated `frontend/dist` remains, so the Docker/CI build path is unambiguous.
|
||||
|
||||
## Security/data notes
|
||||
|
||||
- Secrets exposed: no; scans returned no private keys, token literals, shell execution, or unsafe deserialization patterns in added lines.
|
||||
- Production data touched: no; all schema tests use temporary SQLite databases.
|
||||
- Migration/rollback: schema migration up/down verified on temporary SQLite. PostgreSQL execution, importer, parity checks, rollback rehearsal, and runtime repository cutover remain unverified.
|
||||
- `pip check` still reports a pre-existing shared-venv conflict: `alibabacloud-tea-openapi 0.4.4` requires `cryptography<47`, while the environment has `cryptography 50.0.0`. This is not caused by the S4.2 requirements and was not changed.
|
||||
|
||||
## Review findings resolved locally
|
||||
|
||||
- **Nullable audit tenant link:** fixed with `ck_audit_actor_requires_org` in ORM metadata and migration `0002`.
|
||||
- **CWD-relative Alembic paths:** fixed with `%(here)s/migrations` and `%(here)s` `prepend_sys_path`; config URL now delegates to `database_url()`.
|
||||
- **ORM/migration default drift:** model columns now carry matching server defaults while retaining ORM defaults.
|
||||
- **Dependency resolution drift:** direct requirements are pinned and all transitive artifacts are hash-locked; Docker/CI no longer resolve an unbounded requirements file.
|
||||
- **Unsupported partial-index dialect:** shared `validate_database_url()` is used by `create_db_engine()` and Alembic offline URL resolution, so MySQL is rejected before connection or SQL rendering and cannot silently weaken non-preview attempt uniqueness.
|
||||
|
||||
## Remaining blockers
|
||||
|
||||
- The earlier reviewer `deleg_e672880a` returned valid five-key JSON with `passed=true` for the schema/dependency packet; its final closure was held after the later exact-current offline-path finding from `deleg_c2e728d8`.
|
||||
- The offline-path blocker is remediated and verified locally; fresh exact-current reviewer `deleg_40e8edf9` returned valid five-key JSON with `passed=true`, empty blocking arrays, and five non-blocking suggestions.
|
||||
- Docker is unavailable on this Mac; no image build or container smoke was claimed.
|
||||
- PostgreSQL service, JSON importer, repository wiring, Redis/DB rate-limit store, real-provider QA, and production operation remain pending.
|
||||
- Non-blocking reviewer hardening suggestions remain: immutable CI action/base-image pinning, isolated CI-only credentials, preserving audit attribution semantics on actor deletion, and broader upgraded-database parity assertions.
|
||||
|
||||
## Exact next action
|
||||
|
||||
1. Continue S4.3 one aggregate at a time; do not delete JSON stores or wire production cutover until importer/parity/rollback gates pass.
|
||||
2. Resolve the remaining legacy security and S4.3 independent-review gates against the exact current tree.
|
||||
3. Keep PostgreSQL, Docker, Redis, real-provider QA, and restricted live-operation gates explicitly separate from local approval.
|
||||
|
||||
## Independent review verdict
|
||||
|
||||
- `deleg_e672880a` — valid schema/dependency review; `passed=true`; `security_concerns=[]`; `logic_errors=[]`; combined with the post-remediation review below.
|
||||
- `deleg_c2e728d8` — valid exact-current review; `passed=false` for the offline unsupported-dialect path; remediation is recorded in `2026-08-15-s4-3-offline-dialect-remediation.md`.
|
||||
- `deleg_40e8edf9` — fresh exact-current post-remediation review; `passed=true`; `security_concerns=[]`; `logic_errors=[]`.
|
||||
- Non-blocking suggestions: add explicit before-commit and immutable-field regression assertions, document global username/email uniqueness intent, map duplicate-key errors, and run live PostgreSQL smoke before cutover.
|
||||
- This closes the code/schema/dependency review scope only. It is not PostgreSQL, Docker, production cutover, or deployment approval.
|
||||
|
||||
## Files explicitly not to touch for this packet
|
||||
|
||||
- `.env`
|
||||
- credential helpers or production configuration
|
||||
- unrelated product/runtime routes
|
||||
- JSON stores and production data
|
||||
@@ -0,0 +1,36 @@
|
||||
# S4.3 remediation — offline Alembic dialect guard
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: local remediation complete; fresh exact-current independent review passed; production gates remain blocked
|
||||
|
||||
## Finding
|
||||
|
||||
Fresh reviewer `deleg_c2e728d8` returned valid JSON with `passed=false` and one blocking logic error: `backend/migrations/env.py:25-32` configured Alembic offline mode without using the supported-dialect guard in `backend/app/db.py`. A MySQL offline render therefore succeeded, and the generated SQL omitted the partial-index predicate for non-preview session uniqueness.
|
||||
|
||||
This finding supersedes the earlier S4.2 packet approval for final-gate purposes. It was a schema/migration-path issue, not a production runtime event; no MySQL or PostgreSQL production database was touched.
|
||||
|
||||
## Test-first remediation
|
||||
|
||||
- Added `test_alembic_offline_path_rejects_unsupported_dialects` in `backend/tests/test_db_schema.py`.
|
||||
- RED evidence: the test failed because `command.upgrade(..., sql=True)` did not raise and rendered MySQL SQL.
|
||||
- Added `validate_database_url()` in `backend/app/db.py` as the shared supported-dialect validator.
|
||||
- `create_db_engine()` and Alembic `_database_url()` now use the same validator, so online and offline paths reject unsupported dialects before engine creation or SQL rendering.
|
||||
|
||||
## Verification
|
||||
|
||||
- Offline regression: `1 passed`.
|
||||
- Schema suite: `8 passed`.
|
||||
- Full backend suite: `209 passed in 44.37s`.
|
||||
- `compileall` and `git diff --check`: passed.
|
||||
- Changed-scope security scan: `findings=[]`.
|
||||
- `ruff`/`mypy`: unavailable in the local environment; not represented as passing.
|
||||
|
||||
## Review state
|
||||
|
||||
- `deleg_c2e728d8`: valid fail; blocker remediated locally.
|
||||
- `deleg_40e8edf9`: fresh exact-current post-remediation review passed with empty blocking arrays and five non-blocking suggestions.
|
||||
- `deleg_e672880a`: valid prior S4.2 schema/dependency review; combined with `deleg_40e8edf9` to close the code/schema review scope after remediation.
|
||||
|
||||
## Operational limits
|
||||
|
||||
Docker, PostgreSQL runtime/parity, rollback rehearsal, JSON-to-SQL cutover, Redis persistence, real-provider QA, authenticated production smoke, credential/JWT rotation, deployment, and public access remain unverified or explicitly blocked. No commit, push, deploy, credential rotation, or production operation was performed.
|
||||
@@ -0,0 +1,45 @@
|
||||
# S4.3 — Organization/user repository packet
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: implementation and offline-dialect remediation complete locally; exact-current independent review passed; runtime cutover intentionally not started
|
||||
|
||||
## Scope
|
||||
|
||||
- Added `backend/app/repositories/contracts.py` with `OrganizationRepository` and `UserRepository` Protocols.
|
||||
- Added `backend/app/repositories/sqlalchemy.py` with SQLAlchemy adapters for organization/user CRUD and tenant-scoped user listing.
|
||||
- Added `backend/app/repositories/groups.py` with tenant-scoped group/persona CRUD, org checks before persona creation, and controlled group updates.
|
||||
- Added `backend/app/repositories/sessions.py` with tenant-scoped session/message reads and mutations; message sequence is explicit and database-unique.
|
||||
- Added repository errors for missing entities and immutable/unknown update fields.
|
||||
- Added `backend/tests/test_repositories.py` covering organization active filtering, updates, user auth-field preservation, tenant scope, duplicate-email rejection, and transaction ownership.
|
||||
- Added `backend/tests/test_group_repositories.py` covering group/persona visibility, cross-org access denial, controlled updates, and cross-org persona-creation rejection.
|
||||
- Added `backend/tests/test_session_repositories.py` covering session/message scope, one-shot uniqueness, message sequence uniqueness, and cross-org mutation denial.
|
||||
|
||||
Repositories flush but do not commit. The caller owns the transaction boundary so future services can atomically persist multiple aggregates. JSON stores remain authoritative; no route/factory cutover was made.
|
||||
|
||||
## TDD evidence
|
||||
|
||||
- RED: repository contract test collection failed with `ModuleNotFoundError: app.repositories` before implementation.
|
||||
- First GREEN attempt exposed SQLite timezone metadata normalization; the test now compares the instant after normalizing SQLite’s naive representation while PostgreSQL remains timezone-aware.
|
||||
- Focused repository tests: `8 passed`.
|
||||
- Full backend suite after the cross-tenant lookup and offline-dialect remediations: `209 passed`.
|
||||
- Offline Alembic MySQL regression: RED before the shared validator; GREEN after `test_alembic_offline_path_rejects_unsupported_dialects` was added.
|
||||
|
||||
## Verification
|
||||
|
||||
- `python -m compileall -q app migrations tests`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Repository added-line security scan: `repository_added_line_findings=[]`.
|
||||
- No production database, JSON store, credentials, deploy, or live service was touched.
|
||||
|
||||
## Remaining gates
|
||||
|
||||
- Fresh exact-current reviewer `deleg_40e8edf9` returned valid five-key JSON with `passed=true`, empty blocking arrays, and five non-blocking suggestions after `deleg_c2e728d8` found and the local tree remediated an offline unsupported-dialect path. The prior valid reviewer `deleg_cf56b7d3` correctly failed on the now-remediated unscoped `UserRepository` lookup.
|
||||
- PostgreSQL execution and repository wiring are not verified.
|
||||
- JSON importer, parity/count/hash comparison, rollback rehearsal, and runtime cutover remain blocked by later S4 packets.
|
||||
- Do not delete JSON stores or switch production dependencies until those gates pass.
|
||||
|
||||
## Independent review verdict
|
||||
|
||||
- `deleg_40e8edf9` — valid exact-current review; `passed=true`; `security_concerns=[]`; `logic_errors=[]`.
|
||||
- Verified: keyword-only tenant scoping, non-disclosing cross-tenant reads, immutable auth-field handling, flush-only transaction ownership, tenant/uniqueness constraints, ORM/Alembic parity, SQLite/PostgreSQL partial-index rendering, and online/offline MySQL rejection before SQL rendering.
|
||||
- Non-blocking suggestions: add explicit before-commit and immutable-field regression assertions, document global username/email uniqueness intent, map duplicate-key errors, and run live PostgreSQL smoke before cutover.
|
||||
37
docs/engineering-log/2026-08-15-s4-4-json-import.md
Normal file
37
docs/engineering-log/2026-08-15-s4-4-json-import.md
Normal file
@@ -0,0 +1,37 @@
|
||||
# S4.4 — JSON-to-relational importer
|
||||
|
||||
Date: 2026-08-15
|
||||
Status: local importer complete; production apply/cutover blocked and not performed
|
||||
|
||||
## Scope
|
||||
|
||||
- Added `backend/scripts/migrate_json_to_postgres.py`.
|
||||
- Added `backend/tests/test_json_import.py`.
|
||||
- Added `docs/runbooks/json-to-postgres.md`.
|
||||
|
||||
The importer covers orgs, users, groups/personas, sessions/messages, and legacy `my_personas` converted into deterministic owner-private groups. A non-empty `audit/audit.jsonl` is rejected rather than silently dropped; audit migration remains part of the S4.6 PostgreSQL audit-store gate.
|
||||
|
||||
## Safety behavior
|
||||
|
||||
- Dry-run is the default.
|
||||
- `--apply` requires an explicit target database URL and a new backup directory.
|
||||
- Full source graph is validated before target writes.
|
||||
- Existing identical rows are unchanged; conflicting target rows abort without overwrite.
|
||||
- Source JSON remains untouched; backup is created before the target transaction.
|
||||
- Cross-tenant references, duplicate IDs/uniqueness keys, malformed scalars, invalid timestamps, oversized files, and symlinks fail closed.
|
||||
- Output is metadata-only: mode, source checksum, counts, and backup status. Password hashes and payloads are never printed.
|
||||
|
||||
## TDD / verification
|
||||
|
||||
- RED: importer test collection failed before the script existed (`ModuleNotFoundError`).
|
||||
- GREEN: importer tests `6 passed`.
|
||||
- Direct CLI smoke: `python scripts/migrate_json_to_postgres.py --help` passed after adding standalone backend-path bootstrap.
|
||||
- Covered: dry-run no-write, apply counts, deterministic private group conversion, second-run idempotency, payload redaction, backup creation, cross-tenant rejection, non-empty audit rejection, malformed scalar rejection, and target-conflict no-overwrite.
|
||||
- Full backend suite after the packet: `193 passed`.
|
||||
- Compileall, diff-check, and added-line security scan passed (`added_line_findings=[]`).
|
||||
|
||||
## Explicit blockers
|
||||
|
||||
- PostgreSQL service and real target migration are unavailable/unverified locally.
|
||||
- Audit/rate-limit PostgreSQL/Redis storage is not implemented.
|
||||
- Importer apply is not production approval; no live data, credentials, deploy, or cutover was touched.
|
||||
45
docs/engineering-log/2026-08-15-sprint-3-4-verification.md
Normal file
45
docs/engineering-log/2026-08-15-sprint-3-4-verification.md
Normal file
@@ -0,0 +1,45 @@
|
||||
# 2026-08-15 — Sprint 3 closure and Sprint 4 foundation
|
||||
|
||||
## Scope
|
||||
|
||||
Continued the remediation plan by closing the remaining Sprint 3 code packets, installing the Sprint 4 frontend QA toolchain, hardening uploads/parsers, and wiring the new checks into Gitea CI. No commit, push, deploy, public-access change, credential rotation, or JWT rotation was performed.
|
||||
|
||||
## Sprint status
|
||||
|
||||
| Sprint | Status | Evidence / limitation |
|
||||
|---|---|---|
|
||||
| Sprint 1 | Code complete; live gate pending | Restricted deploy, bootstrap/JWT rotation, audit inspection, and authenticated smoke still require explicit operator approval. |
|
||||
| Sprint 2 | Complete locally | Existing simulation/session correctness suite remains green. |
|
||||
| Sprint 3 | Complete at code + deterministic fixture-journey level | S3.5 report UI, S3.6 one-time export, S3.7 password change, S3.8 i18n/error UX, and S3.9 fixture journey/responsive checks are implemented. Real-provider and production-authenticated QA are not represented by fixture tests. |
|
||||
| Sprint 4 | In progress | S4.1 frontend unit/E2E toolchain, S4.7 upload/parser hardening, and S4.9 CI integration are complete locally/configured. PostgreSQL, Redis/audit persistence, Docker runtime, real-provider QA, and final production gate remain pending. |
|
||||
|
||||
## Implemented in this session
|
||||
|
||||
- Added `GroupReport.vue`, route wiring, API client method, and GroupEdit report entry point.
|
||||
- Implemented one-time signed analytics export consumption with atomic JSON-store state, expiry/tamper checks, current DB-backed actor validation, and Bearer compatibility.
|
||||
- Added separate current-user password-change endpoint with current-password verification, minimum-length validation, target-user rejection, and rate limiting; Settings now calls it instead of first-time setup.
|
||||
- Completed affected-flow i18n/error handling, replaced colored scenario emojis with Lucide line icons, and removed raw backend error presentation from views.
|
||||
- Added Vitest router guard tests and Playwright auth/training/preview/report/export/weak-area/private-persona fixture journeys.
|
||||
- Added bounded parser behavior: `charset-normalizer`, text-byte limit, PDF page limit, extracted-character limit, unsupported-extension rejection, controlled malformed-PDF errors, and no raw parser exception details in API responses.
|
||||
- Added frontend unit/E2E/audit steps to `.gitea/workflows/ci.yml`; upgraded Vite/plugin-vue/Vitest/Playwright dependencies and confirmed `npm audit` clean.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- Backend full suite: **176 passed**.
|
||||
- Upload/parser + existing upload regression: **37 passed**.
|
||||
- Executable backend scripts: **12 passed** (negative-path scripts emit expected error logs but exit successfully).
|
||||
- Frontend unit: **4 passed**.
|
||||
- Frontend Playwright fixture journey: **12 passed** across desktop 1440×900, mobile 320×568, and mobile 500×768; includes no-horizontal-scroll assertions, auth redirect, admin training/report/export, preview label, one-time manual finish, trainee board/detail/weak-area/private-persona flow.
|
||||
- Frontend production build: **1,781 modules transformed**, successful.
|
||||
- `npm audit --audit-level=high`: **0 vulnerabilities**.
|
||||
- Python compileall, `git diff --check`, and CI YAML parse: passed.
|
||||
- Added-line/static scan including current untracked source/config files: no obvious hardcoded-secret, shell-injection, eval/exec, pickle, or formatted-SQL patterns.
|
||||
- Fresh `dist` served on an isolated local port; served index referenced the current build and GroupReport/i18n artifacts. Browser proxy visual capture returned a 500 in this environment, so the rendered screenshot path is not treated as a separate visual approval; Playwright supplied the actual viewport assertions.
|
||||
|
||||
## Explicit limitations / next actions
|
||||
|
||||
- Fresh independent exact-current-tree reviewer is pending; only a complete JSON verdict with `passed=true`, empty `security_concerns`, and empty `logic_errors` closes the review gate.
|
||||
- Local Playwright used an already-installed Chromium executable because the pinned browser revision was not present; CI is configured to run `npx playwright install --with-deps chromium`. The portable no-override command remains environment-dependent until that install completes locally.
|
||||
- Docker is unavailable on this Mac; image build/runtime and `/ready` container smoke remain CI/remote gates.
|
||||
- PostgreSQL/Alembic/repository migration, Redis or persistent audit/rate-limit store, real-provider QA, and production authenticated smoke remain pending.
|
||||
- Do not run S1.7 live operations without explicit approval for restricted deploy, bootstrap password rotation, JWT rotation, audit inspection, and authenticated smoke.
|
||||
64
docs/runbooks/json-to-postgres.md
Normal file
64
docs/runbooks/json-to-postgres.md
Normal file
@@ -0,0 +1,64 @@
|
||||
# JSON → PostgreSQL Import Runbook
|
||||
|
||||
Status: local importer implemented and verified against a temporary local PostgreSQL database; production cutover is **not** approved or performed.
|
||||
|
||||
## What this importer covers
|
||||
|
||||
`backend/scripts/migrate_json_to_postgres.py` imports the core JSON collections:
|
||||
|
||||
- `orgs/` → `organizations`
|
||||
- `users/` → `users`
|
||||
- `groups/` + embedded personas → `groups` + `personas`
|
||||
- `sessions/` + embedded messages → `sessions` + `messages`
|
||||
- `my_personas/` → deterministic owner-private groups + `personas`
|
||||
|
||||
A non-empty `audit/audit.jsonl` is rejected. Do not bypass that rejection: audit migration belongs to the S4.6 PostgreSQL audit-store gate and must remain fail-closed.
|
||||
|
||||
## Safety contract
|
||||
|
||||
1. Default mode is dry-run. It validates the full reference graph and prints metadata-only counts/checksum.
|
||||
2. `--apply` requires both an explicit target database URL and a new backup directory.
|
||||
3. The backup is created before the target transaction begins. `.env`, credential helpers, and unrelated files are not copied.
|
||||
4. Source JSON is never modified.
|
||||
5. Existing identical rows are counted as `unchanged`; conflicting rows abort the transaction rather than overwrite data.
|
||||
6. Cross-tenant references, malformed scalar values, unsafe IDs, oversized JSON files, symlinked source files, invalid timestamps, and duplicate keys fail closed.
|
||||
7. CLI output contains only mode, checksum, counts, and backup status. Password hashes, latent persona fields, JSON payloads, and connection strings are never printed.
|
||||
|
||||
## Dry-run
|
||||
|
||||
Run from `backend/` against a read-only copy of the source data. Do not paste a real connection string into documentation or chat.
|
||||
|
||||
```bash
|
||||
cd backend
|
||||
./.venv/bin/python scripts/migrate_json_to_postgres.py \
|
||||
--source /path/to/data-copy
|
||||
```
|
||||
|
||||
Expected output is a single JSON metadata object with `mode: "dry_run"`, a source checksum, and per-table `would_create` counts. A dry-run does not require a target database and does not write anything.
|
||||
|
||||
## Apply (operator-gated)
|
||||
|
||||
This is an operational action. Obtain explicit deployment/cutover approval first, provision PostgreSQL, apply Alembic migrations, and take an independently retained backup. Supply the target URL through the shell environment; do not record it in logs or chat.
|
||||
|
||||
```bash
|
||||
cd backend
|
||||
export DATABASE_URL='[REDACTED]'
|
||||
./.venv/bin/python scripts/migrate_json_to_postgres.py \
|
||||
--source /path/to/data-copy \
|
||||
--database-url "$DATABASE_URL" \
|
||||
--apply \
|
||||
--backup-dir /path/to/retained-backup
|
||||
```
|
||||
|
||||
Run the exact command a second time with a **new** backup directory. The second report must show zero `created` rows and only `unchanged` rows. If the source changes or a target row differs, the importer rejects the run; investigate and do not force an overwrite.
|
||||
|
||||
## Verification before any cutover
|
||||
|
||||
- Run the importer tests: `./.venv/bin/python -m pytest tests/test_json_import.py -q`.
|
||||
- Run the full backend suite.
|
||||
- Compare source checksum, per-table counts, and sampled entity hashes against PostgreSQL.
|
||||
- Verify tenant ownership for every imported group, persona, session, and message.
|
||||
- Verify signed-export and audit behavior against PostgreSQL/Redis implementations.
|
||||
- Perform rollback rehearsal while the JSON backup remains read-only.
|
||||
|
||||
The importer is not a production approval, a PostgreSQL availability check, or a rollback rehearsal.
|
||||
181
docs/test-evidence/2026-08-13-sprint-1-security.md
Normal file
181
docs/test-evidence/2026-08-13-sprint-1-security.md
Normal file
@@ -0,0 +1,181 @@
|
||||
# Sprint 1 Security Hotfix Evidence — 2026-08-13
|
||||
|
||||
## Scope
|
||||
|
||||
Implemented and verified the Sprint 1 security packets against isolated temporary data only:
|
||||
|
||||
- S1.1 pytest security harness
|
||||
- S1.2 identity-bound first-time setup
|
||||
- S1.3 tenant-scoped admin provisioning and updates
|
||||
- S1.4 request-level setup/user/organization guards
|
||||
- S1.5 fail-closed bootstrap and JWT configuration
|
||||
- S1.6 role-aware group/persona response redaction
|
||||
- Trainee-created persona variants are stored in an owner-private group
|
||||
- Organization admin PATCH read-modify-write updates use cross-process record locking
|
||||
- Reviewer follow-up: fail-closed persona allowlist, generic internal errors, upload limits/cleanup,
|
||||
protected super-admin mutations, strict JWT tenant claims, orphan-tenant login denial, and strict
|
||||
boolean terms consent
|
||||
|
||||
No production data, `.env` contents, credentials, deployment settings, commit, or push were touched.
|
||||
|
||||
## Security behavior verified
|
||||
|
||||
| Control | Evidence |
|
||||
|---|---|
|
||||
| Setup cannot target another account and the one-time transition is atomic | `backend/app/api/auth_routes.py:56-91`, `backend/app/auth/users.py:181-229`, `backend/app/storage/store.py:112-173`, `backend/tests/test_auth_security.py` |
|
||||
| Setup requires `must_setup` and accepted terms | `backend/app/api/auth_routes.py:66-81`, `backend/tests/test_auth_security.py` |
|
||||
| Cross-tenant provisioning/update denied | `backend/app/api/admin_routes.py:97-163`, `backend/tests/test_admin_tenant_isolation.py` |
|
||||
| Non-super-admin cannot change role/active state | `backend/app/api/admin_routes.py:165-179`, `backend/tests/test_admin_tenant_isolation.py` |
|
||||
| Super-admin group index spans tenants while tenant admins remain scoped | `backend/app/api/group_routes.py:182-207`, `backend/tests/test_admin_tenant_isolation.py` |
|
||||
| Existing tokens re-check user/org state | `backend/app/api/helpers.py:44-72`, `backend/tests/test_request_auth_guards.py` |
|
||||
| Missing/inactive org and token org mismatch denied | `backend/app/api/helpers.py:55-67`, `backend/tests/test_request_auth_guards.py` |
|
||||
| Production bootstrap has no known password fallback | `backend/app/config.py:49-81`, `backend/app/factory.py:13-32`, `backend/tests/test_bootstrap_config.py` |
|
||||
| Analyze/get/list/persona/variant responses use one policy | `backend/app/api/group_routes.py:25-63,239-385`, `backend/tests/test_group_redaction.py` |
|
||||
| Trainee group view hides uploaded filenames and parsed source text | `backend/app/api/group_routes.py:51-70`, `backend/tests/test_group_redaction.py` |
|
||||
| Trainee variant cannot mutate the shared group or appear to another trainee | `backend/app/api/group_routes.py:373-402`, `backend/app/services/groups.py:74-102`, `backend/tests/test_group_redaction.py` |
|
||||
|
||||
## Verification results
|
||||
|
||||
Executed from repository root unless noted:
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest backend/tests -q
|
||||
77 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_auth_security.py -q
|
||||
17 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_auth_security.py::test_complete_setup_reserves_email_across_concurrent_users -q
|
||||
1 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_auth_security.py::test_complete_setup_is_atomic_under_concurrency -q
|
||||
1 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_auth_security.py::test_complete_setup_is_atomic_across_worker_processes -q
|
||||
1 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_group_redaction.py -q
|
||||
9 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_sprint1_review_findings.py -q
|
||||
24 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_request_auth_guards.py -q
|
||||
12 passed
|
||||
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_bootstrap_config.py -q
|
||||
8 passed
|
||||
|
||||
backend/.venv/bin/python -m compileall -q backend/app backend/tests backend/scripts
|
||||
exit 0
|
||||
|
||||
backend/scripts/test_*.py
|
||||
12/12 passed:
|
||||
test_e2e.py
|
||||
test_ip_protection.py
|
||||
test_m0.py
|
||||
test_m1.py
|
||||
test_resume_decision.py
|
||||
test_routes.py
|
||||
test_saas_tenant.py
|
||||
test_scenario.py
|
||||
test_security.py
|
||||
test_setup.py
|
||||
test_user_journey.py
|
||||
test_variant.py
|
||||
|
||||
cd frontend && npm run build
|
||||
vite transformed 1,772 modules; build passed
|
||||
|
||||
git diff --check
|
||||
passed
|
||||
```
|
||||
|
||||
Additional reviewer-remediation checks:
|
||||
|
||||
- Admin persona responses use an explicit allowlist; unknown future persona fields are omitted.
|
||||
- Group envelopes and tenant-admin input views use explicit allowlists; unknown top-level fields,
|
||||
uploaded filenames, and parsed source payloads are omitted from non-super-admin responses.
|
||||
- Legacy raw `group.error` values are normalized at the API boundary; new failures persist only
|
||||
sanitized error codes and return generic client messages.
|
||||
- Flask `MAX_CONTENT_LENGTH` and per-file byte checks enforce upload bounds; partial uploads are
|
||||
removed on save, parse, parser-import, request-data, missing-input, multi-file, and
|
||||
group-persistence failures.
|
||||
- Auth, setup, session-start, persona-update, and profile error paths return fixed public messages;
|
||||
exception logs contain only error types. Active user/org state is literal-boolean fail-closed,
|
||||
and invited-user email uniqueness is enforced by the same locked create path.
|
||||
- Login responses also expose `must_setup` only when the stored value is literal `True`; no
|
||||
truthiness coercion remains on the reviewed auth path.
|
||||
- Super-admin creation/promotion and role/active mutation paths are blocked through the admin API.
|
||||
- JWT requests require an exact `org_id`; orphaned/inactive organizations and malformed active
|
||||
values cannot log in or pass request guards; placeholder/default bootstrap values are rejected;
|
||||
terms acceptance requires JSON boolean `true`.
|
||||
- Trainee private-persona endpoints use the same revealable serializer as shared-group views.
|
||||
- First-time setup uses a collection lock plus a locked conditional update; threaded and forked-worker
|
||||
concurrent attempts produce exactly one winner, concurrent setup of different accounts cannot
|
||||
reserve the same email address, and invite creation cannot race setup for the same email in either
|
||||
threads or separate worker processes. Service-level setup requires literal `accepted_terms is True`,
|
||||
user creation rejects non-boolean `must_setup`, and the `set_email`/setup race has a dedicated
|
||||
regression test. Tenant guards reject missing object tenant IDs instead of defaulting to
|
||||
`org-default`.
|
||||
|
||||
## Explicit limitations
|
||||
|
||||
- Docker verification was not run because Docker is not installed in the local environment.
|
||||
- Real-provider LLM QA was not run; no live provider credential was used.
|
||||
- Live bootstrap credential/JWT rotation was not performed automatically. Before restoring untrusted/public access, an operator must deploy behind restricted access, set/rotate credentials and `JWT_SECRET`, inspect audit data, and run a fresh authenticated smoke test.
|
||||
- Sprint 2 product-correctness defects remain out of scope: session identity/concurrency, persona reply parsing, final-judge debrief, and admin Preview Mode. The private-group lookup still uses the current JSON store's process-local per-file locks; collection-level scan/create atomicity remains an explicit Sprint 2 task.
|
||||
|
||||
## Current verification — 2026-08-14
|
||||
|
||||
The exact current uncommitted tree was re-run after the latest response-boundary, tenant-fallback, session/debrief serializer, and organization-update race fixes:
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest backend/tests -q
|
||||
97 passed in 23.50s
|
||||
|
||||
This is the post-remediation exact current-tree result. Earlier background run
|
||||
(completed before the latest current-tree rerun):
|
||||
proc_ba80f06ca949 — 91 passed in 928.85s (0:15:28)
|
||||
|
||||
Another earlier background run:
|
||||
proc_efcee6c93382 — 93 passed in 23.11s
|
||||
|
||||
The 91- and 93-test results are retained as supporting historical evidence.
|
||||
The 97-test run above is the authoritative post-remediation exact current-tree
|
||||
result.
|
||||
|
||||
backend/scripts/test_*.py
|
||||
12/12 passed
|
||||
|
||||
cd frontend && npm run build
|
||||
passed; 1,772 modules transformed
|
||||
|
||||
backend/.venv/bin/python -m compileall -q backend/app backend/tests backend/scripts
|
||||
AST parse + git diff --check
|
||||
passed
|
||||
|
||||
added-line security scan
|
||||
hardcoded_secret=0
|
||||
shell_injection=0
|
||||
eval_exec=0
|
||||
pickle=0
|
||||
sql_format=0
|
||||
```
|
||||
|
||||
Additional current-tree regression coverage includes:
|
||||
|
||||
- user/admin listings use a closed user allowlist; `password_hash` and unknown internal fields are omitted;
|
||||
- provider-controlled final judge output is constrained to a closed debrief allowlist;
|
||||
- trainee session/list/resume/get responses omit `user_id`, `internal` judge state, provider metadata, and unknown message fields;
|
||||
- authenticated group/private-persona mutations reject missing tenant identity instead of falling back to `org-default`;
|
||||
- malformed group input/persona shapes serialize safely and admin organization `plan`/`seats` fields reject coercion;
|
||||
- `test_org_updates_preserve_concurrent_fields_across_processes` verifies that concurrent organization `active` and `seats` updates do not lose fields across worker processes; `JsonStore.update()` holds the per-record process lock during read-modify-write.
|
||||
|
||||
## Independent review
|
||||
|
||||
The earlier schema-valid packet/scoped approvals and reviewer timeouts are not approvals of this exact current tree. The exact-tree read-only reviewer (`deleg_6e386e71`) timed out after 600.19 seconds without schema-valid JSON. The later two-workstream batch (`deleg_da73aca4`) also timed out in both tasks. The bounded exact-tree reviewer (`deleg_fccee30d`) returned schema-valid `passed=false` with one concrete finding: organization admin updates lacked cross-process locking in `JsonStore.update()`. That finding was fixed and the 97-test run above was executed afterward. The fresh post-remediation exact-tree reviewer (`deleg_7bcf0dfd`) returned the required schema-valid verdict `passed=true`, with `security_concerns=[]` and `logic_errors=[]`. The Sprint 1 code/reviewer gate is therefore **closed**. Timeout, partial transcript, delegation status, or local test success must not be treated as approval.
|
||||
|
||||
## Deployment decision
|
||||
|
||||
**Do not restore public/untrusted access.** The exact-tree code/reviewer gate is closed, but the separate live-operation gate remains pending. Docker verification, real-provider LLM QA, browser/mobile QA, restricted deployment, bootstrap credential change, `JWT_SECRET` rotation, audit inspection, and authenticated smoke have not been completed. The private-group filesystem scan/create collection race remains explicitly deferred to Sprint 2/S2.1.
|
||||
88
docs/test-evidence/2026-08-15-legacy-security-remediation.md
Normal file
88
docs/test-evidence/2026-08-15-legacy-security-remediation.md
Normal file
@@ -0,0 +1,88 @@
|
||||
# Legacy Security Remediation Evidence — 2026-08-15
|
||||
|
||||
## Scope
|
||||
|
||||
Test-first remediation of the exact-current legacy-security findings from scoped review `deleg_a274603c`:
|
||||
|
||||
- tenant/group visibility, private-owner authorization, tenant-admin persona mutation, weak-area scope, and group mutation races;
|
||||
- JWT required claims, canonical identity, empty-tenant filtering, JSON-store corruption semantics, and fail-closed rate limiting;
|
||||
- signed export-token malformed state, CSV formula injection, malformed debrief data, and bounded PDF extraction.
|
||||
|
||||
No commit, push, deploy, credential access/rotation, public-access change, or production operation occurred. JSON stores remain runtime-authoritative.
|
||||
|
||||
## TDD evidence
|
||||
|
||||
A new regression module was written before remediation and run against the current tree. The expected RED run produced 18 failures covering the reported behaviors. After the minimal fixes, the same targeted set passed:
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest -q backend/tests/test_legacy_security_regressions.py backend/tests/test_weak_area_analysis.py
|
||||
18 passed
|
||||
```
|
||||
|
||||
## Implemented controls
|
||||
|
||||
- Non-super-admin group indexes require a non-empty tenant and exclude explicit private-owner records; malformed owner markers fail closed.
|
||||
- Private-group reuse requires a valid owner/tenant and `status=ready`; stale records return a controlled conflict through the API.
|
||||
- Persona updates use an explicit tenant-admin editable allowlist and hold the group mutation lock across authorization/read/write.
|
||||
- Variant generation holds the source-group mutation lock through generation and append; report-build failures persist `status=failed`.
|
||||
- Weak-area analysis requires explicit `user_id` and `org_id` scope and ignores sessions outside both boundaries.
|
||||
- JWT decoding requires `sub`, `org_id`, `role`, `auth_version`, `iat`, and `exp`; stored `id`/`username` identity consistency is required.
|
||||
- JSON stores accept and return only object records; corruption propagates as `StoreError` instead of being silently omitted. Rate limiting remains fail-closed.
|
||||
- Export-token reads/consumption handle malformed records as invalid links; CSV formula detection scans leading whitespace/control characters; malformed debrief values produce safe zero scores.
|
||||
- PDF extraction is performed in clipped page bands with a raw file-size preflight and a cumulative extracted-character cap.
|
||||
|
||||
## Verification results
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest -q backend/tests
|
||||
247 passed in 46.55s
|
||||
|
||||
backend/.venv/bin/python -m compileall -q backend/app backend/tests
|
||||
passed
|
||||
|
||||
AST parse of backend/app and backend/tests
|
||||
70 files passed
|
||||
|
||||
git diff --check
|
||||
passed
|
||||
|
||||
Added-line security scan
|
||||
hardcoded_secret=[]
|
||||
shell_command=[]
|
||||
dynamic_code=[]
|
||||
pickle_load=[]
|
||||
sql_format=[]
|
||||
```
|
||||
|
||||
Targeted post-remediation security/parser suites also passed:
|
||||
|
||||
```text
|
||||
backend/.venv/bin/python -m pytest -q backend/tests/test_legacy_security_regressions.py backend/tests/test_upload_security.py backend/tests/test_final_review_regressions.py backend/tests/test_weak_area_analysis.py
|
||||
54 passed in 5.23s
|
||||
```
|
||||
|
||||
A direct PyMuPDF probe with a multi-page text fixture rejected extraction at the configured character cap. `ruff` and `mypy` were unavailable in the verification environment.
|
||||
|
||||
## Independent review status
|
||||
|
||||
The full-scope exact-current review `deleg_93ef64c5` timed out after 600 seconds without a complete five-key JSON verdict and is not approval. Replacement batch `deleg_df17f04e` was stopped because it began before final service-boundary hardening. Final code-freeze batch `deleg_6885e5b9` is in progress. This evidence is local verification only and does not close the legacy-security gate until every required scope in final batch `deleg_6885e5b9` returns valid JSON with `passed=true`, empty `security_concerns`, and empty `logic_errors`.
|
||||
|
||||
## Latest post-review remediation evidence (2026-08-15)
|
||||
|
||||
The valid exact-current review `deleg_4357d521` identified a blocking migration gap: directory-backed rate limiting did not preserve legacy `DATA_DIR/ratelimit.json` counters. The current tree now migrates that state under a collection lock, preserves the legacy file, fails closed on malformed migration data, and has a regression proving an existing counter still blocks.
|
||||
|
||||
Additional current-tree evidence:
|
||||
|
||||
```text
|
||||
Targeted legacy/auth/rate-limit/password suites: 125 passed
|
||||
Backend full suite: 280 passed
|
||||
Frontend unit suite: 4 passed
|
||||
compileall: passed
|
||||
git diff --check: passed
|
||||
```
|
||||
|
||||
Three fresh exact-current read-only reviewer scopes were dispatched after the final source change. Their outputs are pending; no incomplete status is approval.
|
||||
|
||||
## Remaining limits
|
||||
|
||||
Docker, PostgreSQL runtime/parity, real-provider LLM QA, browser/mobile QA, authenticated production smoke, deployment, public-access restoration, and credential/JWT rotation remain unperformed and require separate operator approval.
|
||||
17
docs/test-evidence/2026-08-15-postgresql-import.md
Normal file
17
docs/test-evidence/2026-08-15-postgresql-import.md
Normal file
@@ -0,0 +1,17 @@
|
||||
# PostgreSQL Importer Test Evidence — 2026-08-15
|
||||
|
||||
| Check | Status | Evidence |
|
||||
|---|---|---|
|
||||
| Dry-run | ✅ passed | Expected metadata-only counts: 1 org, 1 user, 2 groups, 2 personas, 1 session, 2 messages |
|
||||
| First apply | ✅ passed | Created all expected fixture rows; fresh backup directory created |
|
||||
| Second apply | ✅ passed | 0 created; all existing rows reported `unchanged` |
|
||||
| Target counts | ✅ passed | PostgreSQL contained exactly 1/1/2/2/1/2 rows |
|
||||
| Conflict rejection | ✅ passed | Changed existing group returned importer rejection code; no overwrite |
|
||||
| Transaction rollback | ✅ passed | Rows queued before the conflict were absent after rejection |
|
||||
| Output redaction | ✅ passed | Password-hash marker and database URL absent from serialized report |
|
||||
| Schema downgrade cleanup | ✅ passed | 0 application tables remained after `downgrade base` |
|
||||
| Real snapshot apply | 🚧 pending | Requires operator-approved target operation and retained backup |
|
||||
| Audit migration | 🚧 pending | Non-empty JSON audit input remains fail-closed until S4.6 |
|
||||
| Runtime repository cutover | 🚧 pending | JSON stores remain authoritative |
|
||||
|
||||
All rows above used disposable local fixtures and a temporary local PostgreSQL database. No production data or credentials were used.
|
||||
22
docs/test-evidence/2026-08-15-postgresql-runtime.md
Normal file
22
docs/test-evidence/2026-08-15-postgresql-runtime.md
Normal file
@@ -0,0 +1,22 @@
|
||||
# PostgreSQL Runtime Test Evidence — 2026-08-15
|
||||
|
||||
## Gate classification
|
||||
|
||||
| Check | Status | Evidence |
|
||||
|---|---|---|
|
||||
| Local PostgreSQL service | ✅ passed | `pg_isready -h 127.0.0.1 -p 5432` accepted connections |
|
||||
| Alembic upgrade on PostgreSQL | ✅ passed | Temporary database, migrations `0001` and `0002` |
|
||||
| Tenant/schema runtime invariants | ✅ passed | 7-table runtime probe; cross-tenant and uniqueness cases exercised |
|
||||
| ORM/migration parity | ✅ passed | Separate temporary model and migration PostgreSQL databases matched |
|
||||
| Offline PostgreSQL DDL | ✅ passed | Required tables, partial predicate, audit check, and `COMMIT` present |
|
||||
| Alembic downgrade to base | ✅ passed | 0 application tables remained in temporary migration database |
|
||||
| Temporary database cleanup | ✅ passed | Cleanup trap dropped both temporary databases |
|
||||
| Docker image/runtime smoke | 🚧 blocked | Docker unavailable on this Mac |
|
||||
| Redis persistence/audit gate | 🚧 blocked | `redis-cli` unavailable and production persistence not exercised |
|
||||
| JSON importer data rollback rehearsal | 🚧 pending | Schema rollback is not importer/data rollback |
|
||||
| Runtime repository cutover | 🚧 pending | JSON stores remain authoritative |
|
||||
| Production authenticated smoke/deploy | 🚧 blocked | Requires explicit operator approval and production access |
|
||||
|
||||
## Important boundary
|
||||
|
||||
These are local temporary-database results. They prove the current PostgreSQL schema and migration path, not production readiness, target-database parity, importer safety against real data, Redis persistence, deployment health, or authorization of a cutover.
|
||||
Reference in New Issue
Block a user