53 lines
3.6 KiB
Markdown
53 lines
3.6 KiB
Markdown
# 2026-08-07 — Security hardening + UX/UI polish
|
||
|
||
## Summary
|
||
Ran the `requesting-code-review` (security) + `ui-ux-pro-max` (design) + `frontend-dev-verification`
|
||
pipelines against the built Sales Trainer app. Fixed real security vulnerabilities and applied
|
||
accessibility/touch/visual polish. Committed as `[verified]`.
|
||
|
||
## Security audit — found & fixed
|
||
1. **Path traversal on file upload** (HIGH): `create_group` used the raw client `file.filename` in
|
||
`dest = upload_dir / f"{user}__{file.filename}"` → an attacker-provided name like `../../evil.txt`
|
||
could escape the upload directory. FIXED: strip to basename (`Path(name).name`), reject names
|
||
containing `/` or `\`, and add a `resolve().relative_to(upload_dir)` containment check.
|
||
2. **IDOR — org/owner scoping missing** (HIGH): `list_personas`, `get_persona`, `update_persona`,
|
||
and the chat routes did not verify a group belonged to the caller's org; personal groups
|
||
(`owner_user_id`) were readable by any same-org user. FIXED: centralized `_authorize_group` /
|
||
`_get_owned_group` (org scope, super_admin bypass, owner-only for personal groups) applied to all
|
||
group + chat routes; `list_groups` and `win_lose_board` now hide other users' personal groups.
|
||
3. **XSS hygiene** (MEDIUM): `Chat.vue` used `v-html="taskText"`. FIXED: switched to text
|
||
interpolation; removed server HTML in the opener-task string.
|
||
4. Confirmed no hardcoded secrets, no eval/exec, no shell injection, no self-registration (register→404).
|
||
|
||
## UX/UI (ui-ux-pro-max applied)
|
||
- Global `style.css`: visible focus rings (a11y), 44px min touch targets, button/card hover +
|
||
active-press transitions (150–300ms), input focus glow, `prefers-reduced-motion` support,
|
||
skeleton loaders, empty-state block, back-link button, spinner, responsive mobile margins.
|
||
- Login: password show/hide toggle, autocomplete attrs, spinner, disabled-when-empty.
|
||
- Personas/GroupEdit/Dashboard: `.lift` card hover, back-navigation links, proper empty states
|
||
(skeleton loaders on dashboard), spinner on analyze button.
|
||
- Chat: back link, spinner on finish button, disabled-after-debrief.
|
||
|
||
## Verification
|
||
- `test_security.py` ADDED: traversal filename rejected (no file escapes), cross-user personal-group
|
||
access → 403, personal group hidden from other users' listing. ALL PASS.
|
||
- Full suite re-run: test_m0 / test_m1 / test_routes / test_security / test_e2e ALL PASS.
|
||
- Frontend `npm run build` ok (11 chunks). Served-page verification via curl: `GET /` 200,
|
||
register 404, login 200; new CSS classes (`btn-back`, `card.lift`, `empty-state`,
|
||
`focus-visible`, `prefers-reduced-motion`, `skeleton`) present in served bundle; no `v-html` in
|
||
any built JS chunk.
|
||
- Browser visual check was BLOCKED by an environment issue: Hermes browser proxy resolves to
|
||
`camo.moreminimore.com/tabs` (HTTP 500) and cannot reach localhost. Rendered verification done
|
||
via served-HTML/DIST inspection instead.
|
||
|
||
## Independent reviewer follow-up (requesting-code-review)
|
||
Reviewer verdict: `passed: true` — zero blocking security/logic issues. It raised 2 non-blocking
|
||
(pre-existing) suggestions, both applied in `ce6076f`:
|
||
- `get_group`: now masks `sales_kit` + `report` for trainees (they contain latent pain analysis).
|
||
- `get_persona`: now gates `status==ready` for trainees (parity with `list_personas`).
|
||
- Extended `test_security.py` with latent-leak + non-ready-gate assertions; all suites still pass.
|
||
|
||
## Next
|
||
- Set a real `LLM_API_KEY` (and provider) and run a live smoke test of analyze→personas→chat→judge.
|
||
- Push to Gitea remote (repo currently local, no remote yet); validate Docker build on EasyPanel.
|