Refactor Study Desk application structure
This commit is contained in:
parent
faaccf8a7e
commit
089439ed90
145 changed files with 8087 additions and 3412 deletions
67
audit-results/CONFIRMED_FINDINGS.md
Normal file
67
audit-results/CONFIRMED_FINDINGS.md
Normal file
|
|
@ -0,0 +1,67 @@
|
|||
# Confirmed Findings
|
||||
|
||||
Severity: Critical / High / Medium / Low / Observation. Confidence: Confirmed / High-confidence inference / Unverified risk.
|
||||
Full structured detail for every finding (files, functions, scenario, expected/actual, root cause, impact, evidence, fix direction, complexity, regression test, runtime-confirmation flag) is in `FINDINGS.json` (validated). Source IDs from specialist auditors are in parentheses.
|
||||
|
||||
| ID | Severity | Confidence | Title | Area |
|
||||
|---|---|---|---|---|
|
||||
| FIN-01 (DBAUD-01, GRP-01) | **Critical** | Confirmed | Schema/migration drift: `MaterialGroup` table + `groupId` columns never migrated — fresh deployments' databases are incompatible with the app (verified: `no such column: Deck.groupId`, `no such table: MaterialGroup`) | DB/migrations |
|
||||
| FIN-02 (AUTH-01, OPS-03) | **Critical** | High-confidence inference | Hardcoded fallback `SESSION_SECRET` (`"dev-session-secret-change-in-production-must-be-32-chars"`) in `src/lib/auth.ts:15` + `src/proxy.ts:8-9`; empty env var in shipped compose → forgeable session cookie → full auth bypass | Auth/deploy |
|
||||
| FIN-03 (QUIZ-01, ADV-01) | **High** | Confirmed | In-viewer "Retake Missed" never sets `retakeIds` prop → persisted as a full attempt with a wrong, lower score; pollutes SEQUENTIAL progress | Quiz |
|
||||
| FIN-04 (CARD-01, IMPT-01, QUIZ-06) | **High** | Confirmed | Stale card ids in saved progress never filtered on resume (`filterAndClampOrder` dead code) → deleting a card mid-session bricks the flashcard study session | Flashcards |
|
||||
| FIN-05 (OPS-01) | **High** | Confirmed | Port mismatch: container listens on 3726, compose publishes 3000:3000 → production deployment unreachable | Deploy |
|
||||
| FIN-06 (OPS-06) | **High** | High-confidence inference | No `.dockerignore`: Windows-host builds inject win32 native modules (better-sqlite3, argon2) into the Linux image | Deploy |
|
||||
| FIN-07 (OPS-08) | **High** | Confirmed | `docker-compose.override.yml` auto-merges on plain `docker compose up` → production command silently runs dev mode | Deploy |
|
||||
| FIN-08 (AUTH-04, ADV-09) | Medium | High-confidence inference | First-login provisioning takeover; `ADMIN_PASSWORD_HASH` env is dead config | Auth |
|
||||
| FIN-09 (AUTH-02) | Medium | High-confidence inference | Proxy auth bypass for any path containing `.` (latent today; would expose any future non-UUID-id route) | Auth |
|
||||
| FIN-10 (AUTH-03) | Medium | High-confidence inference | Password-reset abuse: token overwrite DoS, spoofable `x-forwarded-for` rate limits, unthrottled `complete` | Auth |
|
||||
| FIN-11 (QUIZ-02) | Medium | High-confidence inference | Attempt route stores unvalidated `answersJson`; duplicate option ids inflate SATA credit; malformed shapes → 500 | Quiz |
|
||||
| FIN-12 (QUIZ-03, FE-04) | Medium | Confirmed | Quiz Finish double-submit creates duplicate attempts; failure path silent | Quiz/FE |
|
||||
| FIN-13 (QUIZ-04) | Medium | High-confidence inference | Progress PATCH can land after Finish's DELETE → stale "Continue" row for a completed quiz | Quiz |
|
||||
| FIN-14 (CARD-02, FE-05, QUIZ-09) | Medium | High-confidence inference | Fire-and-forget, unsequenced progress autosaves: out-of-order PATCHes regress resume state; multi-tab last-write-wins | Flashcards/Quiz |
|
||||
| FIN-15 (CARD-03, ADV-07) | Medium | Confirmed | Completed flashcard session never persisted as complete → resume re-shows the last graded card | Flashcards |
|
||||
| FIN-16 (CARD-04) | Medium | Confirmed | SRS review endpoint enforces membership only — no due-date or `newCardsPerDay` enforcement server-side | SRS |
|
||||
| FIN-17 (CARD-05, ADV-02) | Medium | Confirmed | Unbounded DB scans: whole `StudyActivity` table per fetch; all SRS card states per set; Navbar polls every 60 s | Perf/DB |
|
||||
| FIN-18 (FE-01, GRP-03, FE-02, GRP-05) | Medium | Confirmed | Library optimistic mutations are fire-and-forget (no `res.ok`, no rollback); cross-group reorder omits origin group from payload | FE/Groups |
|
||||
| FIN-19 (GRP-02, CARD-07, AUTH-06) | Medium | Confirmed | Reorder/create endpoints trust client `groupId`/`sortOrder` — no class/type/existence validation; cross-class group membership can leak content into another class's share link | Groups |
|
||||
| FIN-20 (FE-03) | Medium | Confirmed | Shared quiz session resets and options reshuffle on any parent re-render (unstable inline `quiz` prop) | FE |
|
||||
| FIN-21 (IMPT-02, FE-08, QUIZ-05, ADV-10) | Medium | Confirmed | `/api/progress` PATCH completely unvalidated; unguarded client `JSON.parse` of persisted progress crashes pages | Quiz/FE |
|
||||
| FIN-22 (OPS-02) | Medium | High-confidence inference | Entrypoint migrate-deploy chain fragile: crash loop on conflicting DB state; runner prisma CLI install unverified/unpinned | Deploy |
|
||||
| FIN-23 (OPS-05) | Medium | Confirmed | Non-reproducible installs: `npm install` (not `ci`); unpinned `prisma@^7.8.0` re-install re-resolves the whole tree in the runner | Deploy |
|
||||
| FIN-24 (OPS-07) | Medium | Confirmed | No healthcheck, no backup, no documented recovery | Deploy |
|
||||
| FIN-25 (OPS-10, TEST-10) | Medium | Confirmed | CI builds/pushes without tests, lint, prisma validate, or container smoke test — deployment-breaking defects ship green | CI |
|
||||
| FIN-26 (QUIZ-07) | Low | Confirmed | SATA scoring divides by `correctIds.length` with no zero-guard → NaN | Quiz |
|
||||
| FIN-27 (QUIZ-08) | Low | Confirmed | Historical review/category breakdown recompute scores from current content, not stored attempt | Quiz |
|
||||
| FIN-28 (IMPT-03) | Low | Confirmed | Card add/edit endpoints bypass Zod: empty strings storable; CreateTab silently drops incomplete cards | Imports |
|
||||
| FIN-29 (IMPT-04) | Low | Confirmed | No payload size/string-length/array-length caps on import schemas/routes; whitespace-only names accepted | Imports |
|
||||
| FIN-30 (IMPT-05) | Low | Confirmed | SATA constraint mismatch: instructions say ≥2 correct, schema enforces ≥1 | Imports |
|
||||
| FIN-31 (IMPT-06) | Low | Confirmed | Unvalidated `name`/`groupId` override fields on import POST routes → 500; PATCH routes mask failures as 404 | Imports |
|
||||
| FIN-32 (GRP-04) | Low | Confirmed | Group deletion leaves duplicate sortOrder values in Uncategorized; no unique constraint/tie-break | Groups |
|
||||
| FIN-33 (GRP-06) | Low | Confirmed | Keyboard users cannot move items between groups (empty `handleDragOver`) | Groups/a11y |
|
||||
| FIN-34 (CARD-08) | Low | High-confidence inference | Restart deletes progress fire-and-forget; slow DELETE can remove the fresh session's progress row | Flashcards |
|
||||
| FIN-35 (CARD-09) | Low | Confirmed | Concurrent first review of same new card → P2002 → generic 500 instead of 409 | SRS |
|
||||
| FIN-36 (CARD-10) | Low | High-confidence inference | "Previous card" during the 350 ms grade animation races the pending timeout | Flashcards |
|
||||
| FIN-37 (CARD-11) | Low | Confirmed | SRS set page never refreshes on focus → stale membership after deck deletion in another tab | SRS/FE |
|
||||
| FIN-38 (CARD-06) | Low | Confirmed | Day boundary hardcoded to Arizona (UTC-7): "today"/new-card limit/streak roll over at 07:00 UTC | SRS |
|
||||
| FIN-39 (AUTH-05) | Low | Confirmed | Proxy `destroy()` cookie-clear lost on redirect; layout check ignores `sessionGeneration` | Auth |
|
||||
| FIN-40 (AUTH-08) | Low | Confirmed | Session cookie Secure flag off in shipped compose (plain HTTP, no TLS story) | Auth/deploy |
|
||||
| FIN-41 (DBAUD-02, AUTH-07) | Low | Confirmed | `/api/share` accepts arbitrary `targetType` → repeatable junk all-NULL `ShareLink` rows | Auth/DB |
|
||||
| FIN-42 (DBAUD-03) | Low | High-confidence inference | `dev.db` committed in git history (8 commits), remains in blobs on the LAN remote | Repo hygiene |
|
||||
| FIN-43 (OPS-09) | Low | Confirmed | `.gitignore` gaps (`data/`, `study.db*`); scratch files `out.css`/`temp.css`/`test.css` tracked | Repo hygiene |
|
||||
| FIN-44 (FE-07, ADV-04) | Low | Confirmed | Dashboard `fetchClasses` no `.catch`/`res.ok` → unhandled rejection, misleading empty state (or render crash) | FE |
|
||||
| FIN-45 (FE-09) | Low | Confirmed | Effect fetches without `.catch` in GenerateTab and ShareMenu | FE |
|
||||
| FIN-46 (FE-10) | Low | High-confidence inference | Cross-class navigation fetch race renders wrong-class data; module cache never invalidated | FE |
|
||||
| FIN-47 (ADV-03) | Low | Confirmed | Logout has no error handling — failed logout strands the user | FE |
|
||||
| FIN-48 (ADV-05) | Low | Confirmed | `slugify` can yield an empty slug (unreachable class); renames never update the URL slug | Classes |
|
||||
| FIN-49 (ADV-08) | Low | High-confidence inference | Shared-viewer localStorage keys item-scoped, not token-scoped → session bleed across tokens | Sharing/FE |
|
||||
| FIN-50 (GRP-07) | Observation | Confirmed | Group `sortOrder` uses inverted desc convention with unvalidated PATCH — latent trap | Groups |
|
||||
| FIN-51 (FE-11) | Observation | Confirmed | Collapsed-groups state read in effect → one-frame expand flash | FE |
|
||||
| FIN-52 (AUTH-10) | Observation | Confirmed | Shared quiz links ship the full answer key to anonymous viewers — by design, worth a warning | Sharing |
|
||||
| FIN-53 (TEST-01) | Low | Confirmed | False-confidence test: "distinct valid state for every rating" never asserts distinctness | Tests |
|
||||
|
||||
## Notes on consolidation
|
||||
|
||||
- Duplicate findings across specialist reports were merged under one root cause (see `sourceIds`).
|
||||
- FE-06 ("all /api/* routes unauthenticated") was **refuted** by the adversarial review — `src/proxy.ts` IS the compiled middleware and enforces session checks; the residual truth is FIN-09 (dot-bypass). See REJECTED_FINDINGS.md.
|
||||
- OPS-02's fresh-volume crash-loop framing was weakened by the adversarial review (on a fresh volume `migrate deploy` succeeds; the app then fails per FIN-01) — reframed as FIN-22.
|
||||
- FIN-18 severity downgraded from the frontend auditor's High to Medium after adversarial review (no visible break for a single user; silent divergence self-heals on reload).
|
||||
38
audit-results/COVERAGE_MAP.md
Normal file
38
audit-results/COVERAGE_MAP.md
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
# Coverage Map
|
||||
|
||||
Mapping of repository areas → audit status → specialist report reference. Statuses: done / done+verified (parent-verified with runtime evidence) / partial.
|
||||
|
||||
| Area | Files | Auditor | Status | Report ref |
|
||||
|---|---|---|---|---|
|
||||
| Prisma schema/migrations/transactions/cascades | prisma/schema.prisma, prisma/migrations/*, prisma.config.ts, src/lib/db.ts, services | DB specialist (task-1) | done+verified (drift reproduced) | sa_20260806_054030_000000000_5ed877563a0d |
|
||||
| Quiz scoring/attempts/history/retakes/progress | src/lib/scoring.ts, quizService, progressService, quizzes/*, api/quizzes/**, api/progress | Quiz specialist (task-2) | done+verified (server scoring path re-checked) | sa_20260806_054030_000000000_e75a8ce575f9 |
|
||||
| Flashcards/SRS/progress/deletion/ordering | flashcards/*, spaced-repetition/*, cardService, deckService, spacedRepetitionService, api/cards|decks|spaced-repetition-sets|progress | Flashcard specialist (task-3) | done | sa_20260806_054030_000000000_a222804c133c |
|
||||
| Auth/protected routes/API authz/sharing | src/proxy.ts, lib/auth.ts, authService, shareService, shareMetadata, api/auth/**, api/share, shared/*, all api routes | Auth specialist (task-4) | done+verified (proxy + fallback secret read directly) | sa_20260806_054030_000000000_aab0c59dbad3 |
|
||||
| Imports/exports/generation/Zod/malformed input | lib/validation/*, jsonRepair, components/import/*, api/decks|quizzes|material-groups|settings | Import specialist (task-5) | done | sa_20260806_054030_000000000_5e72c2713989 |
|
||||
| Material groups/library/drag-drop/deletion/orphans | api/material-groups/**, reorder routes, library pages, classService | Groups specialist (task-6) | done | sa_20260806_054030_000000000_9ae1f3dfa282 |
|
||||
| Frontend state/refresh/races/localStorage/boundaries | all components + pages + shared viewers | Frontend specialist (task-7) | done | sa_20260806_054030_000000000_0e17fd155702 |
|
||||
| Test quality/coverage gaps | *.test.ts, vitest.config.ts, package.json, CI workflow | Test specialist (task-8) | done | sa_20260806_054030_000000000_cd3652deb3f7 |
|
||||
| Docker/startup/env/scripts/production/recovery | Dockerfile, docker-entrypoint.sh, compose files, package.json, prisma.config.ts, next.config.ts, .gitignore, CI | Ops specialist (task-9) | done | sa_20260806_054030_000000000_f49cdaacaabf |
|
||||
| Adversarial review (challenge Critical/High + missed issues) | (all of the above + lightly-covered files) | Adversarial specialist (task-10) | done — 9/10 challenged findings confirmed/weakened, FE-06 refuted, 10 new issues added | sa_20260806_055608_000000000_c550c742d5dc |
|
||||
|
||||
## Parent-level verification performed (see VERIFICATION_LOG.md and audit-results/tmp/)
|
||||
|
||||
- `npx prisma validate` — PASS
|
||||
- `npm test` — PASS (30 tests: 20 arcade + 10 spacedRepetition)
|
||||
- `npm run lint` — 28 baseline problems (9 errors / 19 warnings), all pre-existing
|
||||
- `npm run build` — PASS (route table incl. "ƒ Proxy (Middleware)")
|
||||
- `prisma migrate deploy` on fresh temp DB — PASS (creates DB **missing** MaterialGroup/groupId → FIN-01 drift confirmed)
|
||||
- Temp DB introspection (`audit-results/tmp/inspect-db.cjs`) — `MaterialGroup` table and `groupId` columns absent
|
||||
- Prisma-shaped SQL against temp DB (`audit-results/tmp/reproduce-drift.cjs`) — `no such column: Deck.groupId`, `no such table: MaterialGroup`, `no such column: ShareLink.groupId`
|
||||
- `FINDINGS.json` validity — PASS (53 findings; validator `audit-results/tmp/validate-findings.cjs`)
|
||||
|
||||
## Coverage gaps (areas NOT fully audited)
|
||||
|
||||
1. **Arcade feature internals** — intentionally out of scope (may be removed). Only checked for shared-DB/build/deploy impact (Arcade models/indexes verified in migrations).
|
||||
2. **Real browser interaction** — no browser available in the audit environment: drag-drop, resume flows, share pages, keyboard a11y, theme behavior were code-verified only; findings needing runtime confirmation are flagged in UNVERIFIED_RISKS.md.
|
||||
3. **Container build/run** — docker CLI unavailable: OPS-02/FIN-22, FIN-06, FIN-24 need a container run to observe exact failure modes.
|
||||
4. **git history deep-dive** — limited to drift commit (7af0935), dev.db commits, and migration commits; other commits not diffed line-by-line.
|
||||
5. **Committed dev.db blob contents** — could not be fully enumerated (no sqlite3 CLI; blob grep only; no admin_password hash found in checked blobs).
|
||||
6. **Live dev.db** — none exists in the workspace; runtime behavior on a populated database (perf findings FIN-17) not measured.
|
||||
7. **Network/remote behavior** — the Forgejo remote and CI execution were not reachable; CI findings are static analysis of `.forgejo/workflows/build.yml`.
|
||||
8. **External services** — none (app calls no LLM/third-party APIs).
|
||||
69
audit-results/EXECUTIVE_SUMMARY.md
Normal file
69
audit-results/EXECUTIVE_SUMMARY.md
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
# Study Desk — Overnight Read-Only Audit: Executive Summary
|
||||
|
||||
Status: **COMPLETE** (2026-08-06)
|
||||
|
||||
## Scope and method
|
||||
|
||||
Comprehensive read-only audit of the Study repository (Next.js 16 / React 19 / Prisma 7 / SQLite, self-hosted single-user study app). Ten specialist subagents audited: (1) Prisma schema/migrations/transactions/cascades/drift, (2) quiz scoring/attempts/retakes/progress, (3) flashcards/SRS/progress/ordering, (4) auth/protected routes/API authorization/sharing, (5) imports/exports/generation/Zod validation, (6) material groups/library/drag-drop/deletion, (7) frontend state/races/localStorage/boundaries, (8) test quality, (9) Docker/env/production/recovery, (10) adversarial review challenging every Critical/High finding and hunting missed issues. The parent agent independently re-verified all Critical/High claims (code tracing, git history, temp-DB migration chain, DB introspection, Prisma-shaped SQL probes). The Arcade feature set was excluded per scope unless it affects the main app, shared dependencies, database integrity, build, or deployment.
|
||||
|
||||
## Constraints honored
|
||||
|
||||
- **No source code, tests, migrations, config, package files, lockfiles, docs, or databases modified.** Final `git status --short`: only untracked `.reasonix/` and `audit-results/`; `git diff --check` clean; `src/generated/prisma/` byte-identical before/after all prisma commands (sha256).
|
||||
- All reports and diagnostic artifacts live under `audit-results/` (reports + `tmp/`: `audit-migration.db`, `inspect-db.cjs`, `reproduce-drift.cjs`, `validate-findings.cjs`, `generated-before.sha256`).
|
||||
- Existing databases: none found in the workspace; the migration-chain test used a brand-new temp DB.
|
||||
|
||||
## Baseline verification
|
||||
|
||||
| Check | Result |
|
||||
|---|---|
|
||||
| `npm test` | ✅ PASS — 30 tests / 6 files (20 arcade out of scope, 10 spacedRepetition) |
|
||||
| `npm run lint` | ⚠️ 28 problems (9 errors, 19 warnings) — all pre-existing baseline |
|
||||
| `npm run build` | ✅ PASS — full route table incl. "ƒ Proxy (Middleware)" |
|
||||
| `npx prisma validate` | ✅ PASS |
|
||||
| Migration chain on fresh temp DB | ✅ PASS — 4/4 migrations apply, but the resulting DB is **missing** `MaterialGroup` + `groupId` columns (drift, see below) |
|
||||
|
||||
## Findings (53 total: 2 Critical, 5 High, 18 Medium, 25 Low, 3 Observation)
|
||||
|
||||
Full detail in `FINDINGS.json` (validated) and `CONFIRMED_FINDINGS.md`.
|
||||
|
||||
### Critical
|
||||
|
||||
1. **FIN-01 — Schema/migration drift (Confirmed, runtime-verified).** `prisma/schema.prisma` defines `MaterialGroup` and `Deck/QuizSet/ShareLink.groupId` (added by commit 7af0935, which also rewrote a committed `dev.db` via `prisma db push`) but **no migration creates them**. Every DB built by `prisma migrate deploy` — the `predev` hook and the Docker entrypoint — lacks these objects; the generated client then fails every Deck/QuizSet/ShareLink/MaterialGroup query (`no such column: Deck.groupId`, `no such table: MaterialGroup` — reproduced against a temp DB). **Any fresh install/deploy is non-functional.** The app only works on the developer's db-push-synced local database.
|
||||
2. **FIN-02 — Hardcoded session-secret fallback (High-confidence inference).** `src/lib/auth.ts:15` and `src/proxy.ts:8-9` fall back to the public constant `"dev-session-secret-change-in-production-must-be-32-chars"` whenever `SESSION_SECRET` is unset — and the shipped `docker-compose.yml` passes `${SESSION_SECRET}`, which is empty by default. Anyone with the source can forge an iron-session cookie (`{isAuthenticated:true, sessionGeneration:1}`) and fully bypass authentication on such deployments.
|
||||
|
||||
### High
|
||||
|
||||
3. **FIN-03 — "Retake Missed" persists as a full attempt with a wrong lower score** (in-viewer retake never sets the `retakeIds` prop; server scores all unanswered questions as 0) and pollutes SEQUENTIAL progress.
|
||||
4. **FIN-04 — Deleting a card mid-session bricks the flashcard study session** (`filterAndClampOrder` is dead code; resume never filters stale ids; no skip UI).
|
||||
5. **FIN-05 — Production compose is unreachable**: container listens on 3726 (`ENV PORT=3726`), compose publishes 3000:3000.
|
||||
6. **FIN-06 — No `.dockerignore`**: Windows-host builds inject win32 native modules (better-sqlite3, argon2) into the Linux image.
|
||||
7. **FIN-07 — `docker-compose.override.yml` auto-merges**: plain `docker compose up` silently runs dev mode in production.
|
||||
|
||||
### Medium (selected)
|
||||
|
||||
First-login provisioning takeover + dead `ADMIN_PASSWORD_HASH` (FIN-08); proxy auth bypass for dot-containing paths (FIN-09); password-reset abuse (FIN-10); unvalidated attempt `answersJson` (FIN-11); finish double-submit duplicates attempts (FIN-12); progress PATCH/DELETE race (FIN-13); unsequenced progress autosaves (FIN-14); completed session resumes on last card (FIN-15); SRS review endpoint lacks due/new-card-limit enforcement (FIN-16); unbounded DB scans (FIN-17); library optimistic mutations without rollback (FIN-18); unvalidated `groupId`/`sortOrder` incl. cross-class group membership (FIN-19); shared-quiz reshuffle on re-render (FIN-20); unvalidated `/api/progress` + unguarded `JSON.parse` crashes (FIN-21); entrypoint migrate fragility (FIN-22); non-reproducible installs (FIN-23); no healthcheck/backup (FIN-24); CI ships without tests/lint/prisma/smoke gates (FIN-25).
|
||||
|
||||
### Low / Observation
|
||||
|
||||
25 Low (e.g. SATA div-by-zero NaN, historical score recompute, card-edit endpoints without Zod, no import size caps, sortOrder duplicates after group deletion, keyboard cross-group moves impossible, restart DELETE race, P2002 → 500, git history containing `dev.db`, `.gitignore` gaps, dashboard fetch crash, empty slug from `slugify`, shared-viewer localStorage key collisions) and 3 Observations (group sortOrder desc convention, collapse flash, shared quizzes ship the answer key by design).
|
||||
|
||||
## Verification highlights
|
||||
|
||||
- FIN-01 disproved attempts: checked whether the client avoids selecting `groupId` (it doesn't — generated `Deck.ts` includes it in every payload), whether pages swallow errors (they don't), and whether any migration creates the objects (grep = 0 hits). Confirmed via live probes instead: see `audit-results/tmp/reproduce-drift.cjs` output in VERIFICATION_LOG.md.
|
||||
- Adversarial review **refuted** the frontend auditor's "no auth on API routes" claim (proxy is compiled middleware — verified in `.next` artifacts), **weakened** the entrypoint crash-loop framing (fresh-volume `migrate deploy` succeeds), and downgraded the optimistic-reorder severity. It independently confirmed all other Critical/High findings and added 10 missed issues, merged into the final list.
|
||||
|
||||
## Repository areas NOT fully audited
|
||||
|
||||
1. Arcade feature internals (out of scope; shared-DB/build/deploy impact only — verified in migrations).
|
||||
2. Real browser interaction (no browser available): drag-drop, resume flows, share pages, keyboard a11y, themes — code-verified; flagged for runtime confirmation in UNVERIFIED_RISKS.md.
|
||||
3. Container build/run (docker CLI unavailable): FIN-06, FIN-22, FIN-24 need a container run.
|
||||
4. Deep git-history diff of every commit; full content audit of the committed `dev.db` blobs (no sqlite3 CLI; no credentials found in the checked blobs).
|
||||
5. Runtime performance on a populated database (FIN-17 not measured).
|
||||
6. CI/remote execution (Forgejo pipeline analyzed statically).
|
||||
|
||||
## Recommended priority (when fixes are authorized)
|
||||
|
||||
1. **FIN-01** — generate and commit the missing migration (`prisma migrate dev --name add_material_groups`); add a CI drift gate. Everything else depends on a working fresh deployment.
|
||||
2. **FIN-02 / FIN-05 / FIN-06 / FIN-07** — harden the deployment path: require `SESSION_SECRET`, fix the port mapping, add `.dockerignore`, rename the dev override, and smoke-test the image in CI.
|
||||
3. **FIN-03 / FIN-04** — the two most user-visible application bugs (wrong retake scores; stuck flashcard sessions).
|
||||
4. Then the Medium cluster (validation, races, SRS enforcement, progress handling) and finally Low items + TEST_GAPS.md.
|
||||
1024
audit-results/FINDINGS.json
Normal file
1024
audit-results/FINDINGS.json
Normal file
File diff suppressed because it is too large
Load diff
15
audit-results/REJECTED_FINDINGS.md
Normal file
15
audit-results/REJECTED_FINDINGS.md
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
# Rejected Findings
|
||||
|
||||
Findings that were investigated and disproven, or removed as duplicate/stylistic/insignificant, with the reason.
|
||||
|
||||
| ID | Original claim | Rejection reason |
|
||||
|---|---|---|
|
||||
| FE-06 | "All `/api/*` routes are unauthenticated (no middleware, no per-route checks) — curl without a session cookie returns data" | **Refuted by adversarial review + parent verification.** `src/proxy.ts` IS the compiled Next.js 16 middleware (verified in `.next/server/functions-config-manifest.json`, `middleware.js`, and the compiled chunk; build output shows "ƒ Proxy (Middleware)"). It decrypts the iron-session cookie and enforces `isAuthenticated` + `sessionGeneration` on every path except `/login`, `/api/auth`, `/shared`, `/_next*`, `/favicon*`, and paths containing `.`. The auditor searched for `middleware.ts` (the Next 15 name) and missed `proxy.ts`. The residual truth is the dot-bypass (FIN-09). The claim that "no frontend code handles 401s because there is no auth" is also wrong — there is no 401 because the proxy redirects to /login. |
|
||||
| QUIZ-06 (standalone) | "Quiz resume with stale question ids crashes the viewer" | Merged into FIN-04. The quiz-side path is currently unreachable (no question editing/deletion exists in the app; quiz deletion cascades progress), so it is a latent variant of the same root cause (dead `filterAndClampOrder`), not a separate live defect. |
|
||||
| OPS-02 (original framing) | "Fresh-volume deployment crash-loops because `migrate deploy` fails" | **Weakened by adversarial review.** On a fresh volume `prisma migrate deploy` *succeeds* (verified against a temp DB); the fresh-volume failure is FIN-01 (schema drift → app 500s while the container runs). The crash loop requires a secondary trigger (db-push DB, tampering, lock). Reframed as FIN-22 with the crash-loop path demoted. |
|
||||
| FE-01 (original severity) | Optimistic drag-drop reorder without rollback rated High | **Severity downgraded to Medium by adversarial review.** Mechanics confirmed (fire-and-forget fetch, in-place mutation, origin group omitted from cross-group payload) but impact analysis: sortOrder gaps only, silent divergence self-heals on reload, no visible break for a single user. Merged into FIN-18. |
|
||||
| AUTH-06 (standalone) | "Cross-class group membership via reorder API exposes another class's content" | Merged into FIN-19 (same root cause: `groupId`/`sortOrder` never validated server-side). The sharing-exposure angle is recorded as impact of FIN-19. |
|
||||
| DBAUD-04 | "Relative SQLite path resolution may cause dev dual-DB split-brain" | **Disproven.** CLI (`prisma.config.ts`) and app adapter both resolve `file:./dev.db` cwd-relative; `predev` and the app run from the project root → same file. Docker uses absolute `file:/app/data/study.db`. Recorded as verified-safe in UNVERIFIED_RISKS.md. |
|
||||
| DBAUD-05/06/07, QUIZ-08-verdicts, GRP-01-verdicts (various) | "Cascades broken / transactions partial / StudyProgress duplicates via NULL tricks / id regeneration on card edit / XSS via markdown / client-server boundary violations" | **Disproven** by the respective auditors with evidence: FKs enforced by compile default; nested creates implicit-transactional; service-level guards block duplicates; PATCH edits in place (ids preserved); no `rehype-raw`; no server-module imports in client components. Recorded as verified-safe. |
|
||||
| FE-11 (as bug) | "Collapsed-groups expand flash" | Retained only as Observation (FIN-51) — cosmetic, no data impact. |
|
||||
| Various | Style preferences, harmless duplication, generic best practices, speculative micro-optimizations, Arcade-only issues | Excluded per audit scope rules. |
|
||||
340
audit-results/REMEDIATION_PLAN.md
Normal file
340
audit-results/REMEDIATION_PLAN.md
Normal file
|
|
@ -0,0 +1,340 @@
|
|||
# Study Desk audit evaluation and remediation plan
|
||||
|
||||
Date: 2026-08-06
|
||||
Scope: independent evaluation of `CONFIRMED_FINDINGS.md`, `FINDINGS.json`, the supporting audit reports, and the current repository source.
|
||||
Change policy for this evaluation: this plan is the only new file; no application source, tests, migrations, configuration, generated files, database, or existing audit artifact was changed.
|
||||
|
||||
## Executive verdict
|
||||
|
||||
The audit is credible and the central findings hold up against the current source. The most urgent problems are not cosmetic:
|
||||
|
||||
1. The committed migration chain does not create `MaterialGroup` or the three `groupId` columns, while the generated Prisma client selects those fields. A database created only with `prisma migrate deploy` is incompatible with the app.
|
||||
2. Production authentication can use a public fallback session secret, making a deployment with an empty `SESSION_SECRET` forgeable.
|
||||
3. The in-viewer quiz retake flow loses the retake scope and records the retake as a full, incorrectly scored attempt.
|
||||
4. Flashcard resume trusts stale IDs and unguarded JSON, so deletion or malformed progress can leave the study viewer blank or crash it.
|
||||
5. The production container path is presently unreliable: the port mapping is wrong without the development override, the conventional override silently changes normal Compose behavior, the build context can include host `node_modules`, and the runner installs a ranged Prisma CLI outside the lockfile.
|
||||
|
||||
I agree with most of the remaining findings as defects or resilience gaps, but not always with their severity or proposed remedy. In particular:
|
||||
|
||||
- FIN-04 needs more than wiring in the existing `filterAndClampOrder`: that helper does not preserve the logical current card when a stale ID occurs before the saved index, and it turns a completed index back into the last card.
|
||||
- FIN-05 should normally use `3000:3726`, preserving the documented host port while mapping it to the actual container port.
|
||||
- FIN-22 should not be fixed with an automatic migration retry loop. Schema conflicts should fail fast with actionable diagnostics and a documented recovery path.
|
||||
- FIN-27 is more important than Low because changing quiz content can rewrite the meaning of persisted history.
|
||||
- FIN-34 is only partially present: the dedicated deck page already awaits both DELETE requests, but the library start/restart path fires them without waiting.
|
||||
- FIN-38 is not currently a defect for this Arizona-based installation. It becomes a portability requirement only if the app is meant to support a configurable local day boundary.
|
||||
- FIN-42 does not justify a destructive history rewrite without evidence that the old database contained sensitive data and without coordinating every clone/remote.
|
||||
- FIN-50 is an intentional convention. Newest named groups must remain first and Uncategorized must remain last; validation should make that convention harder to corrupt.
|
||||
- FIN-52 is an architectural consequence of local grading in an anonymous read-only viewer. It merits a sharing warning, not removal of the answer key unless the product adopts server-side anonymous grading.
|
||||
|
||||
## Finding-by-finding disposition
|
||||
|
||||
Priority definitions: P0 = release/deployment blocker; P1 = next remediation batch; P2 = important hardening; P3 = worthwhile backlog; Conditional = verify a stated condition before changing behavior; No separate fix = intentional behavior or covered by another item.
|
||||
|
||||
| ID | Verdict | Worth fixing? | Priority and disposition |
|
||||
|---|---|---:|---|
|
||||
| FIN-01 | Agree | Yes | **P0.** Add the missing migration plus a safe adoption runbook for already-`db push`-synchronized databases. |
|
||||
| FIN-02 | Agree, conditional on an unset/blank production secret | Yes | **P0.** Production must fail before serving requests; development may use an explicitly development-only value. |
|
||||
| FIN-03 | Agree | Yes | **P1.** Make attempted-question scope explicit and derive partial-retake status on the server. |
|
||||
| FIN-04 | Agree; proposed helper is insufficient as written | Yes | **P1.** Normalize order, index, and results while preserving the logical current card and completed state. |
|
||||
| FIN-05 | Agree | Yes | **P0.** Map host 3000 to container 3726 and smoke-test it. |
|
||||
| FIN-06 | Agree with the mechanics; exact runtime failure is unconfirmed | Yes | **P0.** Add `.dockerignore` and prove native modules load in the Linux image. |
|
||||
| FIN-07 | Agree | Yes | **P0.** Rename the development override and document explicit dev/prod commands. |
|
||||
| FIN-08 | Partly agree: first-login setup is intentional, accidental exposure and dead config are real | Yes | **P1.** Honor a pre-provisioned admin hash or require an explicit one-time setup mode. |
|
||||
| FIN-09 | Agree; latent with current IDs | Yes | **P1.** Replace the broad dot bypass with exact public/static path rules. |
|
||||
| FIN-10 | Agree | Yes | **P1.** Prevent token overwrite, remove spoofable per-IP trust, and make reset initiation a local/admin operation. |
|
||||
| FIN-11 | Agree | Yes | **P1.** Validate the whole attempt envelope, question scope, option ownership, array uniqueness, and types. |
|
||||
| FIN-12 | Agree | Yes | **P1.** Add an in-flight guard, disable Finish, and surface failure/retry state. |
|
||||
| FIN-13 | Agree as a credible timing race | Yes | **P1.** Persist the attempt and clear full-attempt progress in one server transaction. |
|
||||
| FIN-14 | Agree as a credible timing race | Yes | **P2.** Add session identity and monotonic revisions, not only client debouncing. |
|
||||
| FIN-15 | Agree | Yes | **P1.** Persist `currentIndex === order.length` and restore the summary state. |
|
||||
| FIN-16 | Agree; impact is lower in a protected single-user app | Yes | **P2.** Accept only the queue-eligible card, including the intended learn-ahead rule. |
|
||||
| FIN-17 | Agree that the queries are unbounded; user impact is unmeasured | Conditional | **P3.** Benchmark realistic data first, then optimize queries that exceed the budget. |
|
||||
| FIN-18 | Agree | Yes | **P2.** Stop mutating shared objects, capture the origin group before mutation, await writes, and rollback/refetch on error. |
|
||||
| FIN-19 | Agree | Yes | **P1.** Enforce class/type/group membership and server-owned ordering; retain a share-page defense. |
|
||||
| FIN-20 | Agree | Yes | **P1.** Give the viewer a stable session input so unrelated parent renders cannot reset it. |
|
||||
| FIN-21 | Agree | Yes | **P1.** Validate progress writes and defensively normalize legacy/corrupt progress on reads. |
|
||||
| FIN-22 | Partly agree with the operational gap, not the retry remedy | No separate fix | Fail fast; cover CLI pinning in FIN-23 and recovery/health in FIN-24. Do not retry a conflicting migration automatically. |
|
||||
| FIN-23 | Agree | Yes | **P0.** Use `npm ci` and ship a lockfile-pinned Prisma CLI/runtime path. |
|
||||
| FIN-24 | Agree | Yes | **P1.** Add a schema-aware health check and tested backup/restore instructions. |
|
||||
| FIN-25 | Agree | Yes | **P0/P1.** Gate test, build, Prisma validation/drift, and container smoke. Stage lint until its known baseline is resolved. |
|
||||
| FIN-26 | Agree | Yes | **P1.** Return zero for an invalid zero-correct SATA question and test finite totals. |
|
||||
| FIN-27 | Agree; severity should be Medium | Yes | **P2.** Persist an immutable result/review snapshot for new attempts, with legacy fallback. |
|
||||
| FIN-28 | Agree | Yes | **P2.** Reuse a trimmed card-content schema and reject rather than silently drop incomplete Create-tab rows. |
|
||||
| FIN-29 | Agree | Yes | **P2.** Add reasonable string, array, and request-size limits with clear 400/413 errors. |
|
||||
| FIN-30 | Agree that the contract is inconsistent | Yes | **P2.** Make SATA imports require at least two correct options, matching the shipped generation instructions; preserve legacy stored data. |
|
||||
| FIN-31 | Agree | Yes | **P2.** Validate complete request envelopes and map only actual not-found errors to 404. |
|
||||
| FIN-32 | Agree | Yes | **P2.** Delete/reassign/renumber in one transaction and add deterministic item tie-breaks. |
|
||||
| FIN-33 | Agree | Yes | **P2.** Add an explicit keyboard-accessible “Move to group” action; do not depend on drag gestures. |
|
||||
| FIN-34 | Partly agree | Yes | **P2.** Fix the library path that does not await deletion; make deletion session-aware with FIN-14. |
|
||||
| FIN-35 | Agree | Yes | **P3.** Translate concurrent first-review uniqueness conflicts to 409 and refetch. |
|
||||
| FIN-36 | Agree as a credible timing race | Yes | **P2.** Disable navigation while grading and cancel/ignore stale animation callbacks. |
|
||||
| FIN-37 | Agree | Yes | **P3.** Refresh set/deck membership on focus and after local deck-change events. |
|
||||
| FIN-38 | Factually correct, but Arizona is the current intended boundary | Not now | Keep the existing behavior. Revisit only with an explicit portability requirement and a timezone setting/migration plan. |
|
||||
| FIN-39 | Agree about the lost clear-cookie response; layout impact is overstated because proxy checks generation | Yes | **P2.** Destroy the cookie on the actual redirect response and test its header. |
|
||||
| FIN-40 | Deployment-policy gap, not a bug on intentional HTTP | Yes, as documentation/config | **P1.** Document a TLS proxy profile and `SECURE_COOKIES=true`; do not enable Secure cookies on plain HTTP. |
|
||||
| FIN-41 | Agree | Yes | **P1.** Validate a strict target enum and target existence; reject impossible/null-target rows. |
|
||||
| FIN-42 | Agree that blobs exist; sensitivity is unproven | Conditional | Do not rewrite history now. Inventory the old DB offline before any public remote; rewrite only with explicit coordination if sensitive data is found. |
|
||||
| FIN-43 | Partly agree | Yes | **P3.** Ignore `data/` and production DB sidecars. Confirm ownership, then remove unreferenced CSS scratch files. Do not automatically ignore the audit plan. |
|
||||
| FIN-44 | Agree | Yes | **P2.** Add `res.ok`, error, and retry handling; never present a failed load as an empty dashboard. |
|
||||
| FIN-45 | Agree | Yes | **P3.** Add abort/error handling to settings/share fetches. |
|
||||
| FIN-46 | Agree as a credible timing race | Yes | **P2.** Use `AbortController` or a request generation tied to `classSlug`; remove or bound the module cache. |
|
||||
| FIN-47 | Agree | Yes | **P3.** Show logout failure and retry; do not falsely claim logout if the server call failed. |
|
||||
| FIN-48 | Partly agree | Yes | **P2.** Guarantee a non-empty unique slug. Keep slugs stable on rename unless redirect/history semantics are deliberately designed. |
|
||||
| FIN-49 | Agree; impact is same-browser only | Yes | **P3.** Include the share token in local session keys and migrate/ignore old keys safely. |
|
||||
| FIN-50 | Intentional newest-first convention | No separate fix | Preserve descending named-group order and final Uncategorized placement; restrict arbitrary sort-order writes under FIN-19/31. |
|
||||
| FIN-51 | Agree, cosmetic only | Not now | Defer unless it can be removed without hydration mismatch or new effect-rule violations. |
|
||||
| FIN-52 | Expected local-grading design | Warning only | **P3.** Explain in ShareMenu that recipients can inspect correct answers; do not imply answer secrecy. |
|
||||
| FIN-53 | Agree | Yes | **P1.** Replace the false-confidence assertion and add scoring/scheduling edge cases. |
|
||||
|
||||
## Remediation sequence
|
||||
|
||||
### Phase 0 — safeguards and test foundation
|
||||
|
||||
Do this before behavior or schema changes.
|
||||
|
||||
1. Create a database-backed Vitest harness that always points Prisma at a unique disposable SQLite path. It must refuse to run if the resolved path is `dev.db`, `/app/data/study.db`, or any existing non-test database.
|
||||
2. Add helpers to apply committed migrations to the disposable database and dispose of it after the test process.
|
||||
3. Record the current lint baseline separately. Do not make a failing baseline a nominally “green” CI gate, and do not mix unrelated lint cleanup into P0 fixes.
|
||||
4. Add focused pure tests first for scoring, progress normalization, import schemas, and rate limiting. These give fast feedback before route/service integration tests.
|
||||
5. Before testing any real deployment database, make and verify a restorable backup. No remediation command should run against the only copy.
|
||||
|
||||
Exit criteria:
|
||||
|
||||
- Tests demonstrably create and use only a disposable DB.
|
||||
- A deliberate attempt to point the harness at the real DB aborts.
|
||||
- Existing `npm.cmd test` remains green.
|
||||
|
||||
### Phase 1 — restore a safe, reproducible deployment path
|
||||
|
||||
Addresses FIN-01, FIN-02, FIN-05–10, FIN-23–25, FIN-39, and FIN-40.
|
||||
|
||||
#### 1A. Repair migration drift without breaking already-pushed databases
|
||||
|
||||
1. Generate a new migration; never edit the four applied migrations. It must add:
|
||||
- `MaterialGroup` with its Class cascade foreign key.
|
||||
- nullable `groupId` on Deck and QuizSet with `ON DELETE SET NULL`.
|
||||
- nullable unique `groupId` on ShareLink with `ON DELETE CASCADE`.
|
||||
- the indexes Prisma expects.
|
||||
2. Review the generated SQLite table-rebuild SQL by hand for preserved rows, foreign keys, defaults, and unique indexes.
|
||||
3. Publish a one-time preflight/adoption script that classifies a database as:
|
||||
- migration-tracked and missing the group schema: apply the migration normally;
|
||||
- already schema-pushed and exactly matching the intended DDL: after backup and exact introspection, mark the new migration applied with `prisma migrate resolve --applied`;
|
||||
- partially matching or otherwise inconsistent: stop with diagnostics and require manual recovery; never guess or auto-resolve.
|
||||
4. Keep the adoption operation explicit. The normal entrypoint should not silently mutate migration history based on table existence.
|
||||
|
||||
Verification:
|
||||
|
||||
- Fresh empty DB: `prisma migrate deploy` succeeds; `prisma migrate diff --from-migrations ... --to-schema ...` is empty; Deck, QuizSet, ShareLink, and MaterialGroup queries succeed.
|
||||
- Populated pre-group DB: seed classes/decks/quizzes/shares, apply the new migration, and prove every row/count/relationship is preserved.
|
||||
- Disposable `db push`-style DB: introspect exact equivalence, resolve the migration, run deploy again, and prove an empty schema diff.
|
||||
- Partial/conflicting DB: preflight exits nonzero without changing schema or `_prisma_migrations`.
|
||||
|
||||
#### 1B. Make session configuration unforgeable and setup explicit
|
||||
|
||||
1. Centralize session option construction so `src/lib/auth.ts` and `src/proxy.ts` cannot drift.
|
||||
2. Require a nonblank, at-least-32-character production `SESSION_SECRET` at runtime. Do not require or bake the real secret during `docker build`.
|
||||
3. Make the container entrypoint fail before migrations/server startup when the secret is absent or equals the known fallback. Make Compose use required-variable expansion.
|
||||
4. Allow a development-only fallback only when `NODE_ENV !== "production"`, clearly label it non-production, and cover both branches with tests.
|
||||
5. Replace `pathname.includes(".")` with exact public/static rules. Ensure every `/api/*` path other than the intentional auth endpoints remains protected even when the URL contains a dot or encoded dot.
|
||||
6. Construct the unauthenticated redirect first and bind `getIronSession` to that response before `destroy()`, so the returned redirect carries the clearing cookie.
|
||||
7. Honor a valid `ADMIN_PASSWORD_HASH` when the database has no configured password, or require an explicit one-time setup flag/token. The default production state must not let the first remote request select the admin password.
|
||||
8. Prefer a local console/CLI initiation for password reset. If the HTTP request endpoint remains, use a global single-user throttle, never replace an unexpired token, rate-limit verify/complete, and do not trust arbitrary `x-forwarded-for` unless a trusted proxy is explicitly configured.
|
||||
|
||||
Verification:
|
||||
|
||||
- Production startup with missing, blank, short, or known fallback secret exits nonzero before serving.
|
||||
- A cookie sealed with the old fallback does not authenticate when a real secret is configured.
|
||||
- Valid login, logout, reset completion, and session-generation invalidation still work.
|
||||
- Dot-containing protected page/API requests redirect or reject; known static assets still load.
|
||||
- A stale-generation response contains a `Set-Cookie` deletion header.
|
||||
- Pre-provisioned hash rejects a different first password and reports setup complete.
|
||||
|
||||
#### 1C. Make the image and Compose definitions deterministic
|
||||
|
||||
1. Add `.dockerignore` for `node_modules`, `.next`, VCS metadata, environment files, databases/data, coverage, temporary caches, and audit scratch artifacts while retaining source, Prisma schema/migrations, lockfile, and public assets.
|
||||
2. Change the dependency stage to `npm ci`.
|
||||
3. Remove `npm install prisma@^7.8.0` from the runner. Ship a Prisma CLI/runtime installed from `package-lock.json` at the exact repository version. If Prisma is needed at runtime, classify it as a runtime dependency and prune/copy dependencies deterministically rather than re-resolving them.
|
||||
4. Map `3000:3726` in production Compose. Keep `PORT=3726` and `EXPOSE 3726` internally unless there is a deliberate decision to standardize everything on 3000.
|
||||
5. Rename `docker-compose.override.yml` to a non-auto-merged development filename such as `docker-compose.dev.yml`. Document exact production and development invocations.
|
||||
6. Add a minimal internal health route that performs schema-aware DB checks (including a query that touches `Deck.groupId` and MaterialGroup), returning no sensitive details. The container health check must call the actual internal port.
|
||||
7. Add a lock-safe SQLite backup command using the SQLite backup API, plus a documented restore drill. A raw copy of only the main DB while WAL writes are active is not an acceptable backup procedure.
|
||||
|
||||
Verification:
|
||||
|
||||
- `docker compose -f docker-compose.yml config` shows the production command, production environment, and `3000:3726` only.
|
||||
- The explicit development Compose command shows dev mode and source mounts.
|
||||
- Build on a Windows-host context and run on Linux; requiring `better-sqlite3` and `argon2` succeeds in the final image.
|
||||
- Fresh-volume container becomes healthy and `/login` returns 200 through host port 3000.
|
||||
- A migration-drifted disposable volume stays unhealthy with an actionable log.
|
||||
- Backup a populated disposable volume, destroy the disposable container/volume, restore it, and compare row counts plus representative content.
|
||||
|
||||
#### 1D. Add CI gates in a sequence that can actually be green
|
||||
|
||||
1. Gate `npm ci`, `npm test`, Prisma validate, migration/schema drift, and `npm run build` immediately.
|
||||
2. Build the production image, start it on a fresh disposable volume with a generated CI secret, wait for health, and smoke `/login` plus setup status.
|
||||
3. Tag immutable images by commit SHA; optionally move the mutable `latest` tag only after all gates pass.
|
||||
4. Add strict lint only after the existing 9-error baseline is resolved in an explicit cleanup or after a changed-files lint gate is implemented. Never report baseline lint as passing.
|
||||
|
||||
### Phase 2 — make quiz attempts authoritative and immutable
|
||||
|
||||
Addresses FIN-03, FIN-11–13, FIN-20, FIN-26, FIN-27, and FIN-53.
|
||||
|
||||
1. Replace the client-controlled `isPartialRetake` contract with an explicit ordered `questionIds` scope. The server validates that IDs are unique and belong to the quiz, then derives whether the attempt is partial.
|
||||
2. Accept a structured answers object at the route boundary and persist only canonical server-serialized JSON. Validate that:
|
||||
- each key is in the attempted scope;
|
||||
- every selected option belongs to that question;
|
||||
- selected IDs are unique;
|
||||
- multiple-choice has at most one selection;
|
||||
- missing answers score zero rather than silently disappearing.
|
||||
3. Make the in-viewer “Retake Missed” and “Retake Full Quiz” set explicit local attempt scope. Do not infer retake state from the original prop after the viewer has transitioned internally.
|
||||
4. Add an `isFinishing` state/ref, disable the Finish button during submission, and show a retryable error without discarding answers.
|
||||
5. Move attempt creation and full-attempt SEQUENTIAL-progress deletion into one service transaction. Partial retakes must not overwrite or delete a full in-progress session.
|
||||
6. Guard zero-correct SATA scoring with zero points and deduplicate selections defensively in the pure scorer even though the route also validates them.
|
||||
7. Add an optional immutable review snapshot to QuizAttempt for new attempts: attempted question/order, prompt/category/rationale, options and correctness, selections, and per-question points. Render new history from that snapshot; keep a clearly tested legacy fallback for old rows.
|
||||
8. Stabilize QuizViewer initialization. A parent render that changes topics or another surrounding control must not regenerate option order, clear answers, or reset the index. A deliberate restart/retake must use a new session key.
|
||||
|
||||
Focused verification:
|
||||
|
||||
- Full 5-question attempt scores out of 5 and clears only its matching full progress row.
|
||||
- Two-question retake scores out of 2, is stored as partial, and leaves full progress untouched.
|
||||
- An unanswered question in the explicit scope scores zero and remains in the review snapshot.
|
||||
- Duplicate/unknown question or option IDs, wrong shapes, and client-forged partial flags return 400 without creating an attempt.
|
||||
- Delayed double-click Finish produces exactly one attempt.
|
||||
- Changing or deleting current quiz content after an attempt does not change the stored score, category breakdown, or review copy for snapshot-backed attempts.
|
||||
- Parent rerender preserves question order, option order, current index, and answers; explicit restart changes only the intended session.
|
||||
|
||||
### Phase 3 — make progress and flashcard resume race-safe
|
||||
|
||||
Addresses FIN-04, FIN-14, FIN-15, FIN-21, FIN-34, and FIN-36.
|
||||
|
||||
1. Replace or correct `filterAndClampOrder` with a normalization function that accepts saved order, live IDs, saved index, results/answers, and completion state. It must:
|
||||
- remove stale and duplicate IDs;
|
||||
- preserve the saved current ID when it still exists;
|
||||
- when it was deleted, select the next surviving saved card, otherwise the previous survivor;
|
||||
- preserve `index === oldOrder.length` as completion by returning `index === newOrder.length`;
|
||||
- filter result/answer keys to live IDs;
|
||||
- safely fall back when any JSON field is malformed.
|
||||
2. Apply normalization before any viewer state initializer calls `JSON.parse`. Treat corrupt legacy progress as recoverable, show a small restore warning, and allow a fresh start.
|
||||
3. Add Zod schemas for GET/PATCH/DELETE progress inputs, enum values, nonnegative index, content-specific JSON shape, and referenced content existence.
|
||||
4. Add `sessionId` and monotonic `revision` to progress. The server accepts only a newer revision for the same session; a stale session cannot overwrite or delete a newer session. Serialize client saves but retain server-side revision enforcement because request completion order is not guaranteed.
|
||||
5. On the last flashcard, persist `currentIndex = order.length`. Restoring that row must show the existing completion summary, not the last card.
|
||||
6. Await restart deletion before remount/navigation. Make DELETE conditional on the session being cleared so a late old request cannot delete fresh progress.
|
||||
7. Disable Previous, shuffle, and repeated grading during the 350 ms transition, and clear/cancel the timer on restart/unmount.
|
||||
|
||||
Focused verification:
|
||||
|
||||
- Stale ID before, at, and after the current index all resume on the correct logical card.
|
||||
- All saved cards deleted yields a clear empty/completed state rather than a blank viewer.
|
||||
- Completed sessions reopen on the summary; restart opens card 1.
|
||||
- Invalid JSON and invalid PATCH payloads produce a safe UI fallback or 400, never a render crash/500.
|
||||
- Deliver revisions 3, 1, and 2 in that order; the DB retains revision 3.
|
||||
- Delay an old-session DELETE until after a new-session PATCH; the new session remains.
|
||||
- Grade then immediately try Previous/restart; no index jump or stale result write occurs.
|
||||
|
||||
### Phase 4 — enforce group, ordering, and sharing invariants on the server
|
||||
|
||||
Addresses FIN-18, FIN-19, FIN-32, FIN-33, FIN-41, FIN-49, FIN-50, and FIN-52.
|
||||
|
||||
1. Move reorder logic into focused services. The route must identify the owning class and content type from the database, not trust the client.
|
||||
2. Validate every target group exists, belongs to the same class, and matches DECK/QUIZ type. Reject duplicate IDs, foreign-class IDs, type mismatches, and unknown IDs.
|
||||
3. Prefer an ordered list/group assignment contract and compute contiguous `sortOrder` values server-side. If a complete-set contract is required, verify completeness explicitly.
|
||||
4. In the library, capture origin group before any update, update state immutably, await `res.ok`, disable conflicting mutations in flight, and rollback/refetch on failure.
|
||||
5. Delete a group in one transaction: capture affected items, delete/reassign through the FK behavior, then renumber Uncategorized items deterministically. Add `createdAt`/`id` tie-breakers to reads so legacy duplicates are stable.
|
||||
6. Add a keyboard-accessible Move-to-group menu on every item. Preserve drag-and-drop for pointer users and keep drag listeners off action controls.
|
||||
7. Validate ShareLink target type and target existence. Enforce exactly one populated target in service logic; consider CHECK constraints in a later SQLite migration after compatibility testing.
|
||||
8. Retain the shared-page class/type/token checks and additionally assert each selected group item has the group’s class and expected content type.
|
||||
9. Pass the share token/session namespace to shared viewers and include it in localStorage keys. Avoid copying an old item-only session into a different token namespace without explicit user confirmation.
|
||||
10. Preserve named-group ordering as `sortOrder desc, createdAt desc`, prepend newly created groups locally, and render Uncategorized last. Remove arbitrary `sortOrder` from the ordinary group rename PATCH contract.
|
||||
11. Add a concise ShareMenu warning that a shared quiz necessarily sends answer/rationale data to the recipient’s browser for local grading.
|
||||
|
||||
Focused verification:
|
||||
|
||||
- Cross-class and cross-type group assignment returns 400/409 and changes no rows.
|
||||
- A failing reorder restores/refetches the visible order.
|
||||
- Deleting a group yields unique contiguous Uncategorized item order and does not delete decks/quizzes.
|
||||
- Newer named groups stay above older groups; Uncategorized remains last in quizzes, flashcards, and import selectors.
|
||||
- A keyboard-only user can move an item between two groups and hear/see confirmation.
|
||||
- Invalid share target types and missing IDs create zero rows.
|
||||
- A tampered group share cannot render a foreign-class item.
|
||||
- Two tokens for the same content maintain independent local sessions.
|
||||
|
||||
### Phase 5 — validation, SRS integrity, and user-visible error handling
|
||||
|
||||
Addresses FIN-16, FIN-28–31, FIN-35, FIN-37, FIN-43–48, and the measurable part of FIN-17.
|
||||
|
||||
1. Reuse shared Zod schemas for card create/edit, deck/quiz create/edit, classes, imports, and material groups. Trim before minimum checks and add documented maximums for names, descriptions, Markdown content, options, questions, and cards.
|
||||
2. Reject a Create-tab submission containing any partially filled card; identify the row(s) instead of silently dropping them.
|
||||
3. Enforce at least two correct options for imported SATA questions while leaving existing stored questions readable. Add a targeted message explaining how to repair invalid generated JSON.
|
||||
4. Narrow Prisma error mapping: P2025 becomes 404, P2002 becomes 409 where appropriate, validation is 400/422, and unexpected failures remain 500 with non-sensitive server diagnostics.
|
||||
5. In SRS review, verify the submitted card is the queue-eligible card for the current set/time/new-card allowance (including deliberate learn-ahead). Retain the existing state-version comparison. Map concurrent first-review P2002 to 409 and return/refetch current study state.
|
||||
6. Refresh SRS membership on focus and after same-tab deck changes.
|
||||
7. Add abortable, checked fetch helpers or a small consistent pattern for Dashboard, GenerateTab, ShareMenu, logout, and class library requests. Distinguish loading, empty, error, and retry states.
|
||||
8. Tie library responses to the active `classSlug`; abort or ignore stale responses. Bound or remove module-level caches that can outlive their class data.
|
||||
9. Guarantee class slug generation produces a non-empty unique slug, for example a stable `class-<short-id>` fallback. Keep existing slugs stable on rename.
|
||||
10. Add `data/`, `study.db`, and sidecars to `.gitignore`. Confirm `out.css`, `temp.css`, and `test.css` are unreferenced scratch artifacts before removing them in a separate cleanup change.
|
||||
11. For performance, first seed a disposable benchmark DB approximating expected upper use (for example 100k activity rows, 10k cards, and several SRS sets). Add date predicates/aggregation and narrower SRS queries only where the measured request/poll budget is exceeded. Preserve long-streak correctness when limiting the activity window.
|
||||
|
||||
Focused verification:
|
||||
|
||||
- Whitespace-only and oversized content is rejected with an actionable client error and no partial write.
|
||||
- SATA 1-correct import fails; 2+-correct passes; multiple-choice still requires exactly one.
|
||||
- SRS cannot review a not-due/non-selected/new-limit-exhausted card; valid due and learn-ahead reviews still work.
|
||||
- Concurrent first review produces one success and one handled conflict, not a generic 500.
|
||||
- Focus refresh removes a deleted deck from the SRS set UI.
|
||||
- Failed dashboard/settings/share/logout requests have visible, retryable states and no unhandled promise rejection.
|
||||
- Rapid A→B class navigation cannot render A data under B.
|
||||
- Creating a class named only punctuation yields a navigable, unique slug; rename does not break the old URL.
|
||||
- Performance tests record query counts and latency before/after and prove the 53-week display plus longer current streak remain correct.
|
||||
|
||||
## Cross-cutting release verification
|
||||
|
||||
Run after every phase, with focused tests first and the full suite last.
|
||||
|
||||
Automated gates:
|
||||
|
||||
1. `npm.cmd test`
|
||||
2. `npx.cmd prisma validate`
|
||||
3. migration deploy + empty migration/schema diff against a fresh disposable DB whenever schema changes
|
||||
4. `npm.cmd run build`
|
||||
5. `git diff --check`
|
||||
6. production image build, fresh-volume health wait, and HTTP smoke when Docker/deployment files change
|
||||
7. lint against changed files; full `npm.cmd run lint` only becomes blocking after the known baseline is cleared
|
||||
|
||||
Manual browser matrix for affected phases:
|
||||
|
||||
1. Authenticated routes and corresponding public/shared routes.
|
||||
2. Fresh, resumed, completed, restarted, partial-retake, and stale/deleted-content sessions.
|
||||
3. Loading, empty, server-error, malformed-persisted-data, and retry states.
|
||||
4. Light and dark themes.
|
||||
5. Narrow mobile viewport with no horizontal overflow.
|
||||
6. Keyboard-only navigation, including group moves and all modified icon controls.
|
||||
7. Throttled-network checks for Finish, autosave, restart, class navigation, reorder, and logout.
|
||||
8. Two-tab checks for progress revisions, SRS conflicts, and focus refresh.
|
||||
|
||||
Release/rollback rules:
|
||||
|
||||
- Back up and restore-test the SQLite database before the first migration-bearing release.
|
||||
- Deploy schema and code as one versioned release; do not run a newer generated Prisma client against an older database.
|
||||
- Keep the prior image and verified pre-migration backup until post-deploy smoke and representative data checks pass.
|
||||
- If migration preflight sees a partial or unknown schema state, stop. Do not run `db push`, edit an applied migration, reset the database, or auto-mark the migration applied.
|
||||
|
||||
## Explicitly deferred or rejected work
|
||||
|
||||
- **FIN-38:** no timezone-setting work without a product requirement to support a non-Arizona study day.
|
||||
- **FIN-42:** no history rewrite without an offline sensitivity review and explicit remote/clone coordination.
|
||||
- **FIN-51:** no cosmetic state-initialization change unless it avoids both hydration mismatch and the existing effect-rule class of lint failures.
|
||||
- **FIN-22 retry proposal:** no automatic migration retry/backoff for schema conflicts; fail fast with health diagnostics and the adoption/recovery runbook.
|
||||
- No changes to the audit’s already rejected hypotheses (global API auth absence, split-brain SQLite paths, broken cascades/transactions, Markdown XSS, or client/server import violations) unless new evidence appears.
|
||||
|
||||
## Recommended delivery slices
|
||||
|
||||
Keep the implementation reviewable rather than landing all remediation at once:
|
||||
|
||||
1. **Release blocker:** FIN-01/02/05/06/07/23/25 plus migration adoption, container smoke, and backup prerequisites.
|
||||
2. **Quiz correctness:** FIN-03/11/12/13/20/26/27/53.
|
||||
3. **Progress integrity:** FIN-04/14/15/21/34/36.
|
||||
4. **Group/share integrity:** FIN-18/19/32/33/41/49/50/52.
|
||||
5. **Auth recovery and deployment posture:** FIN-08/09/10/24/39/40.
|
||||
6. **Validation, SRS, UI resilience, and measured performance:** the remaining accepted items.
|
||||
|
||||
Each slice should be independently releasable, have its own focused regression tests, and finish with the cross-cutting gates above.
|
||||
20
audit-results/SUBAGENT_REPORTS.md
Normal file
20
audit-results/SUBAGENT_REPORTS.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
# Subagent Reports
|
||||
|
||||
Full specialist reports were produced by 10 read-only subagents. Each returned a structured markdown report with a findings table and per-finding detail blocks (ID, severity, confidence, files:lines, scenario, expected/actual, root cause, impact, evidence, test coverage, fix direction, complexity, regression test, runtime-confirmation flag). The parent agent re-verified the Critical/High claims directly (see VERIFICATION_LOG.md) and consolidated/deduplicated into `FINDINGS.json` (53 findings).
|
||||
|
||||
## Report index (all reports were reviewed in full by the parent)
|
||||
|
||||
| # | Auditor | Ref | Key output |
|
||||
|---|---|---|---|
|
||||
| 1 | DB / migrations / transactions / cascades / drift | sa_20260806_054030_000000000_5ed877563a0d | **DBAUD-01 Critical migration drift** (MaterialGroup/groupId never migrated; commit 7af0935 db-push artifact); DBAUD-02 junk ShareLink rows; DBAUD-03 dev.db in git history; verified-safe: cascades, transactions, path resolution, single-row AuthSecurity |
|
||||
| 2 | Quiz scoring / attempts / retakes / progress | sa_20260806_054030_000000000_e75a8ce575f9 | **QUIZ-01 High retake-scoring bug** (in-viewer retake never sets retakeIds); QUIZ-02..09 (unvalidated answersJson, double-submit, progress races, dead filter, div-by-zero, historical recompute, multi-tab) |
|
||||
| 3 | Flashcards / SRS / progress / deletion / ordering | sa_20260806_054030_000000000_a222804c133c | CARD-01..11: dead stale-id filter (stuck sessions), unsequenced saves, completed-session resume, SRS due/limit not enforced, unbounded scans, Arizona day boundary, restart race, P2002, animation race, focus staleness |
|
||||
| 4 | Auth / protected routes / API authz / sharing | sa_20260806_054030_000000000_aab0c59dbad3 | **AUTH-01 Critical hardcoded session secret**; AUTH-02 dot-bypass; AUTH-03 reset abuse; AUTH-04 first-login takeover + dead ADMIN_PASSWORD_HASH; AUTH-05..10 (cookie-clear loss, cross-class group membership, share validation, secure flag, no auth tests, answer-key exposure); verified proxy compiled as middleware |
|
||||
| 5 | Imports / exports / generation / Zod / malformed input | sa_20260806_054030_000000000_5e72c2713989 | IMPT-01..06: dead filterAndClampOrder, unvalidated /api/progress + unguarded JSON.parse, card edit endpoints without Zod, no size caps, SATA doc mismatch, override-field 500s; verified: shared schemas, atomic imports, no XSS, no id regeneration |
|
||||
| 6 | Material groups / ordering / drag-drop / deletion / orphans | sa_20260806_054030_000000000_9ae1f3dfa282 | **GRP-01 Critical drift** (independent confirmation); GRP-02..07: unvalidated groupId/sortOrder, fire-and-forget reorder, sortOrder duplicates after group deletion, keyboard a11y, desc-convention fragility; verified: FKs enforced, uncategorized mapping, cascade safety |
|
||||
| 7 | Frontend state / refresh / races / localStorage / boundaries | sa_20260806_054030_000000000_0e17fd155702 | FE-01..11: optimistic reorder without rollback, CRUD res.ok ignored, shared quiz reshuffle (unstable prop), finish double-submit, progress race, **FE-06 (later refuted — proxy exists)**, dashboard fetch gap, unguarded JSON.parse, effect fetches, cross-class race, collapse flash; verified: shared viewers read-only, localStorage clean, no boundary violations |
|
||||
| 8 | Test quality / coverage / false confidence | sa_20260806_054030_000000000_cd3652deb3f7 | TEST-01..10: false-confidence SRS test, zero tests for scoring/auth/services/routes, no DB harness, CI never runs tests; ranked missing-test gaps by impact |
|
||||
| 9 | Docker / startup / env / scripts / production / recovery | sa_20260806_054030_000000000_f49cdaacaabf | OPS-01..10: port mismatch 3726 vs 3000:3000, entrypoint fragility, session secret in prod path, dead ADMIN_PASSWORD_HASH, non-reproducible installs, no .dockerignore (win32 leak), no healthcheck/backup, override auto-merge, .gitignore gaps, CI without gates; argon2/standalone tracing verified OK |
|
||||
| 10 | Adversarial review | sa_20260806_055608_000000000_c550c742d5dc | Challenged all Critical/High findings: confirmed drift, secret, retake, port, dockerignore, override; weakened OPS-02 (fresh-volume migrate succeeds) and FE-01 severity; **refuted FE-06** (proxy active — verified in compiled .next artifacts); added ADV-01..10 (retake progress pollution, activity scan, logout, dashboard crash, slugify empty slug, unguarded parses, completed-resume, localStorage key collisions, first-login race, progress validation) |
|
||||
|
||||
All reports are preserved in full in the session; their findings are consolidated (deduplicated, source IDs preserved) in `FINDINGS.json` and `CONFIRMED_FINDINGS.md`.
|
||||
21
audit-results/TEST_GAPS.md
Normal file
21
audit-results/TEST_GAPS.md
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
# Test Gaps
|
||||
|
||||
Current suite: 6 files / 30 tests — 20 in `src/lib/arcade/*` (out of scope) + 10 in `src/lib/spacedRepetition.test.ts` (pure scheduling math only). **No test touches a service, an API route, the database, auth, scoring, imports, or progress.** CI never runs `npm test`. Gaps ranked by user-impact × regression-likelihood.
|
||||
|
||||
| ID | Area | Gap | Risk it would fail to catch | Suggested test |
|
||||
|---|---|---|---|---|
|
||||
| TEST-GAP-01 | Auth (FIN-02/08/09/10) | `authService.ts` (login, lockout, first-login provisioning, reset token, generation bump) and session config have zero tests | Session-secret fallback regression, lockout bypass, generation-bump loss (stolen sessions stay valid) | Extract + unit-test `getLockoutDuration` thresholds, `parseResetTokenRecord`; service tests with temp SQLite + fake timers: 5 wrong passwords → 423; locked-out login rejected; success resets counter; `completePasswordReset` bumps generation |
|
||||
| TEST-GAP-02 | Scoring (FIN-03/11/26/27) | `src/lib/scoring.ts` SATA partial credit has zero tests | Wrong grades silently persisted into every attempt — permanent data corruption | `scoreQuestion`: SATA 2/4 correct select both → 1; 1 correct + 1 wrong → 0; over-select → 0; MC multi-select first-wrong → 0; `scoreQuiz` with missing question keys; 0-correct SATA → 0 (not NaN) |
|
||||
| TEST-GAP-03 | Quiz attempt persistence (FIN-03/12/13) | `POST /api/quizzes/[id]/attempt` + partial-retake filter + `progressService.upsertProgress` asymmetry (DECK writes cardResultsJson, QUIZ writes answersJson) have zero tests | Retake-scoring regression (FIN-03 class), duplicate attempts, progress column clobbering | Route/service test: partial retake answering 2 of 5 → maxScore 2, score from those 2; upsert round-trip preserves the respective JSON columns |
|
||||
| TEST-GAP-04 | SRS service (FIN-16/35) | `spacedRepetitionService.ts` (reviewCard optimistic concurrency, findNextCard precedence, removeDeck transaction, addDeck validations) has zero tests — only the pure lib is tested | Double-review stale write, due/new-card precedence break, cross-class membership, P2002 → 500 | Seed class→deck→cards→set on temp DB: stale `expectedStateVersion` → 409; due before new before learn-ahead; new card with limit 0 rejected; removeDeck atomically deletes states |
|
||||
| TEST-GAP-05 | Import/repair pipeline (FIN-28/29/30) | `jsonRepair.ts`, `importSchemas.ts`, `shuffle.ts`/`filterAndClampOrder` have zero tests | AI-import regression (fences/repair), schema constraint drift (MC exactly-one), resume clamp bugs | jsonRepair fence/trailing-comma/broken-JSON cases; schema whitespace/oversize/exactly-one cases; `filterAndClampOrder` stale-id + overflow cases |
|
||||
| TEST-GAP-06 | Rate limiter (FIN-10) | `rateLimiter.ts` has zero tests | Brute-force guard silently disabled or locking the owner out | Fake timers: 10 allowed / 11th blocked with retryAfterMs; window expiry; per-key isolation; custom limits |
|
||||
| TEST-GAP-07 | Integration harness (FIN-01/21/25) | No route tests, no DB-backed tests, no setup file; `src/lib/db.ts` defaults to `file:./dev.db` (a naive service test would touch the real dev DB) | Migration drift (FIN-01) — nothing would catch a schema/migration mismatch; progress/share/attempt route validation gaps | `setupFiles` + `DATABASE_URL=file:<tmp>` + `prisma migrate deploy` in globalSetup; route tests with `NextRequest`: attempt 200/400/404, reviews 409, share unknown token, login 429 |
|
||||
| TEST-GAP-08 | CI gate (FIN-25) | `.forgejo/workflows/build.yml` runs no tests/lint/prisma validate; no coverage config; no drift check | Entire suite can regress with a green pipeline; FIN-01-class drift ships | Add `npm ci && npm test && npm run lint && npx prisma validate` job; `prisma migrate diff --from-migrations --to-schema-datamodel` gate; container smoke (curl /login on fresh volume); coverage floor |
|
||||
| TEST-GAP-09 | False confidence (FIN-53) | `spacedRepetition.test.ts:15-23` "distinct valid state for every rating" never compares ratings | `scheduleRating` collapsing all ratings to one schedule passes the suite | Assert pairwise due ordering AGAIN < HARD < GOOD < EASY and distinct due timestamps; graduated-state lapse case |
|
||||
|
||||
## Test-quality positives (verified)
|
||||
|
||||
- `spacedRepetition.test.ts` is deterministic (fixed `now`, `enable_fuzz: false`, no `Math.random`), tests the real pure module, and asserts exact FSRS intervals.
|
||||
- Arcade tests (out of scope) use seeded shuffles and meaningful assertions.
|
||||
- No test uses wall-clock time, locale, or network — no flakiness found.
|
||||
35
audit-results/UNVERIFIED_RISKS.md
Normal file
35
audit-results/UNVERIFIED_RISKS.md
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
# Unverified Risks
|
||||
|
||||
Risks that could not be fully confirmed within audit constraints (need a browser, a container run, a live deployment, or timing-dependent reproduction). All are code-verified as plausible; the missing piece is runtime confirmation.
|
||||
|
||||
| ID | Title | Why unverified | Suggested verification |
|
||||
|---|---|---|---|
|
||||
| FIN-02 | Hardcoded fallback `SESSION_SECRET` → session forgery | Code path fully traced (seal/verify semantics verified in iron-session), but no live production deployment exists here to demonstrate an actual forged-cookie login | Boot production build without `SESSION_SECRET`, forge a cookie with the fallback password (iron-webcrypto seal script), assert full access; then with a real secret assert rejection |
|
||||
| FIN-06 | No `.dockerignore` → win32 modules in Linux image | Mechanics verified (PE32+ DLL present in local node_modules; Dockerfile COPY order), exact failure mode (build error vs `ERR_DLOPEN_FAILED`) not observed | Run `docker build .` on this Windows host; inspect `better_sqlite3.node`/argon2 prebuilds in the image (must be ELF) |
|
||||
| FIN-13 | Progress PATCH-after-DELETE race | Timing-dependent; needs throttled network | DevTools throttling: submit final answer + Finish immediately; inspect `StudyProgress` after |
|
||||
| FIN-14 | Out-of-order progress PATCHes | Timing-dependent | DevTools throttling with rapid grading; reload and compare resume point/results |
|
||||
| FIN-22 | Entrypoint migrate crash loop / runner CLI install | Requires container build + run on `node:22-slim` (docker CLI unavailable in audit environment) | `docker build` + fresh-volume run (assert 200 + `_prisma_migrations`); corrupt-volume run (assert actionable failure, not silent loop) |
|
||||
| FIN-24 | Healthcheck/backup gaps | Behavior observable only in a real container run | Compose up with a corrupt volume; observe restart loop and absence of health status |
|
||||
| FIN-32 | Group-delete sortOrder duplicates | Deterministic from code; visual impact needs a browser | Delete a group in the app, inspect Uncategorized order + DB sortOrders |
|
||||
| FIN-33 | Keyboard cross-group moves impossible | Deterministic from code (empty `handleDragOver`), needs manual keyboard test | Tab to a drag handle, attempt keyboard cross-group move |
|
||||
| FIN-34 | Restart DELETE race | Timing-dependent | Throttled network: Restart, grade first card, inspect progress row |
|
||||
| FIN-36 | Previous-card animation race | Timing-dependent (350 ms window) | Grade then immediately click Previous; observe index jump |
|
||||
| FIN-37 | SRS page stale on focus | Deterministic from code; browser needed to observe | Two tabs: delete deck in one, focus the other |
|
||||
| FIN-39 | Proxy destroy cookie lost on redirect | Deterministic from code; browser needed to observe Set-Cookie | Stale-generation cookie → follow redirect → inspect response headers |
|
||||
| FIN-40 | Secure cookie flag off | Deterministic from code (flag logic); deployment-dependent | Inspect `Set-Cookie` in a production container |
|
||||
| FIN-46 | Cross-class fetch race | Timing-dependent | Throttled network + fast class switching |
|
||||
| FIN-48 | Empty slug / stale slug on rename | Deterministic from code (slugify verified); UI behavior needs browser | Create class named "!!!", try to navigate to it; rename a class, check old URL |
|
||||
| FIN-49 | Shared-viewer localStorage key collisions | Code-verified key construction; symptom needs browser | Open two share tokens for the same content, answer in one, reload the other |
|
||||
| FIN-51 | Collapsed-groups expand flash | Cosmetic; browser-only | Visit library with saved collapsed state |
|
||||
|
||||
## Items verified safe (hypotheses disproven — recorded for completeness)
|
||||
|
||||
- SQLite relative-path split-brain between Prisma CLI and app (both cwd-relative → same `dev.db`; Docker uses absolute path) — verified safe.
|
||||
- Foreign-key enforcement (better-sqlite3 compiled with `SQLITE_DEFAULT_FOREIGN_KEYS=1`) — cascades/SetNull fire; verified safe.
|
||||
- Transactionality of deck/quiz/attempt/reorder writes — nested creates + `$transaction`; verified safe.
|
||||
- `StudyProgress`/`ShareLink` NULL-uniqueness — service-level guards prevent duplicates via app paths; only direct API misuse (FIN-41/FIN-21) can create junk rows.
|
||||
- Client/server boundary violations — none found (no client import of `@/lib/db`/services/prisma).
|
||||
- XSS via Markdown — react-markdown without `rehype-raw`; no `dangerouslySetInnerHTML` on user data; verified safe.
|
||||
- Password-reset nonce/token crypto (192-bit token, SHA-256 digest, `timingSafeEqual`, expiry) — sound; abuse vectors are FIN-10.
|
||||
- SRS optimistic concurrency (`expectedStateVersion` → 409, rollback via refetch) — correct.
|
||||
- Migrations are purely additive; no NOT NULL-without-default, no DROP — safe on populated migration-tracked DBs (except FIN-01 drift).
|
||||
38
audit-results/VERIFICATION_LOG.md
Normal file
38
audit-results/VERIFICATION_LOG.md
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
# Verification Log
|
||||
|
||||
Chronological record of every command run, its outcome, and classification (baseline / environment / possible defect).
|
||||
|
||||
## Environment notes
|
||||
|
||||
- OS: Windows, shell: bash (git-bash)
|
||||
- `node_modules` was **absent** at audit start → `npm test` initially failed with `'vitest' is not recognized`. This is an environment condition, not a repo defect. `npm ci` completed successfully during the first audit session (518 packages present).
|
||||
- No existing application database (`dev.db*`) exists anywhere in the repo — no real user data to protect; the migration-chain test uses a brand-new temp DB under `audit-results/tmp`.
|
||||
- `src/generated/prisma/` (tracked) hashed BEFORE any prisma command: `audit-results/tmp/generated-before.sha256`.
|
||||
|
||||
## Entries
|
||||
|
||||
| # | Time | Command | Result | Classification |
|
||||
|---|---|---|---|---|
|
||||
| 1 | session start | `git status --short` | clean; only `?? .reasonix/` and `?? audit-results/` untracked | baseline |
|
||||
| 2 | session start | `npm test` (first run, prior session) | FAILED: `'vitest' is not recognized` (no node_modules) | environment |
|
||||
| 3 | session start | `npm ci` (background, prior session) | completed; 518 packages in node_modules | environment |
|
||||
| 4 | 22:36 | `npx prisma validate` | PASS — "The schema at prisma\schema.prisma is valid" (exit 0) | baseline |
|
||||
| 5 | 22:36 | `npm test` | PASS — 6 files, 30 tests, all green (arcade ×20, spacedRepetition ×10) | baseline |
|
||||
| 6 | 22:37 | `npm run lint` | 28 problems: 9 errors, 19 warnings (react-hooks/set-state-in-effect errors + unused-vars warnings). All pre-existing; matches AGENTS.md known baseline. | baseline |
|
||||
| 7 | 22:37 | `npm run build` (background) | PASS — route table emitted, no compile errors | baseline |
|
||||
| 8 | 22:38 | `DATABASE_URL=file:./audit-results/tmp/audit-migration.db npx prisma migrate deploy` | PASS — all 4 migrations applied to brand-new temp DB | baseline |
|
||||
| 9 | 22:38 | `sha256sum -c audit-results/tmp/generated-before.sha256` | PASS — every generated Prisma file byte-identical (all ": OK") | baseline |
|
||||
| 10 | 22:38 | `git status --short` | only untracked `.reasonix/`, `audit-results/`; no tracked modifications | baseline |
|
||||
| 11 | 22:38 | `git diff --check` | clean (no whitespace errors) | baseline |
|
||||
| 12 | 22:40 | `git log --all --oneline -- prisma/migrations/` + `git show 7af0935 --stat` | Confirms drift: commit 7af0935 changed `prisma/schema.prisma` (+31) and `dev.db` (139264→155648 bytes) with **no migration**; `dev.db` present in 8 commits of history | possible defect (FIN-01, FIN-42) |
|
||||
| 13 | 22:41 | `node audit-results/tmp/inspect-db.cjs audit-results/tmp/audit-migration.db` | Temp DB (built solely by `migrate deploy`) has **no** `MaterialGroup` table and **no** `groupId` column on Deck/QuizSet/ShareLink | possible defect (FIN-01) |
|
||||
| 14 | 22:42 | `node audit-results/tmp/reproduce-drift.cjs` | Prisma-shaped queries fail: `no such column: Deck.groupId`, `no such table: MaterialGroup`, `no such column: ShareLink.groupId` — fresh-deploy breakage proven | possible defect (FIN-01) |
|
||||
| 15 | 22:45 | `node --check audit-results/tmp/inspect-db.cjs && node --check audit-results/tmp/reproduce-drift.cjs` | diagnostic scripts syntax-valid | baseline |
|
||||
| 16 | 22:50 | `node audit-results/tmp/validate-findings.cjs` | `FINDINGS.json` VALID JSON — 53 findings: 2 Critical, 5 High, 18 Medium, 25 Low, 3 Observation; no duplicate ids | baseline |
|
||||
| 17 | 23:00 | `git status --short` + `git diff --check` (final) | Only untracked `.reasonix/`, `audit-results/`; no tracked file modified; diff clean | baseline |
|
||||
|
||||
## Classification legend
|
||||
|
||||
- **baseline** — repository behaves as expected; recorded for reference.
|
||||
- **environment** — failure caused by local machine state, not repo code.
|
||||
- **possible defect** — outcome may indicate a repo defect; investigated further in findings.
|
||||
BIN
audit-results/tmp/audit-migration.db
Normal file
BIN
audit-results/tmp/audit-migration.db
Normal file
Binary file not shown.
26
audit-results/tmp/generated-before.sha256
Normal file
26
audit-results/tmp/generated-before.sha256
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
cb737289f5b5f6cdb8b90fc23d2c89ebd41d6602ce99e88bf6dfd751beab7103 *src/generated/prisma/browser.ts
|
||||
2ea331124cc56c9c9a360b4a85f42f029992811ece7efe9db0ec1300a43f59c3 *src/generated/prisma/client.ts
|
||||
f1f6280ff65c1e8aab5ce30307c8777ce1795f81bf1a513396263fc1c9a30e5c *src/generated/prisma/commonInputTypes.ts
|
||||
8bcc37ae19ee9c55424b735ce5f3a0972b7e741958ef86b350c9a54dc2238a0d *src/generated/prisma/enums.ts
|
||||
d52628a3dc8285d9014e2fda489badce6c88604810e2975310ab6c40ec3c708d *src/generated/prisma/internal/class.ts
|
||||
91794eb9b3c395e57b96d18f2bae895149f7b70ddf0fda8b817e7aa9da17a950 *src/generated/prisma/internal/prismaNamespace.ts
|
||||
6645109792ed08507d79da319aaecc2752bffa6e8bd5a3364dccc5ae93e2f23c *src/generated/prisma/internal/prismaNamespaceBrowser.ts
|
||||
b8ffef1ad4428179847aafe66abee3ad5eb74a84fff46265532551b10ab2537d *src/generated/prisma/models.ts
|
||||
daecbb1c94a96d19da4accfdf81fe8f11fa73c543958016bc7d6060d454f3943 *src/generated/prisma/models/AnswerOption.ts
|
||||
87e18727cec17a341d32709bc0c1602c66e6c6dcc57672b9c9fbe38a6ce12801 *src/generated/prisma/models/ArcadeAttempt.ts
|
||||
0c9de62bdbefafe8b5ff5300d1d764cc73235a077f8e3b3e03e6144287610c9d *src/generated/prisma/models/ArcadePack.ts
|
||||
67b8ea01f0a866d14db7fdff3178a080e2879990129c5e08690463fb9940f48d *src/generated/prisma/models/AuthSecurity.ts
|
||||
6885465a57019e8cba28755d9c6abaa087e913d666d02d8912abf0790c41a77b *src/generated/prisma/models/Class.ts
|
||||
ea873177511c6f7354dc01ec0ad228d9f777bca2fbfb15d281d3f2ab8f817ebf *src/generated/prisma/models/Deck.ts
|
||||
1082ff29bec6e4f920c93d778032ffffa324c312b05efd6c5fa7ada1be6674f7 *src/generated/prisma/models/Flashcard.ts
|
||||
de94c2da405c08a0b690aeb1b0cdc03582c94ccddaf913f4c6d1b18d8c413b61 *src/generated/prisma/models/MaterialGroup.ts
|
||||
941998d2c098e9fe5cd0a888bac470190108238c936c1d7cfce2742ed1e40d8a *src/generated/prisma/models/Question.ts
|
||||
3d54ed93e516f4d773c200b166a84ab2c9c8b7b0d7f7ddfd61afc3ca057ee755 *src/generated/prisma/models/QuizAttempt.ts
|
||||
454f19f97a82b8f37fc5c5179e87afe6ea185a79810043471d55c0a8ca26f14a *src/generated/prisma/models/QuizSet.ts
|
||||
3b884aaaad311e8f22c1390cf24ebe69b74b52a97c213781d2a4cbb56288b8a7 *src/generated/prisma/models/Setting.ts
|
||||
1ccf32b2f56b1d35c9b7152d2e1817b4c1b7853549a1ff187e9fb7c29fa49f2a *src/generated/prisma/models/ShareLink.ts
|
||||
cba950d431c702afe67f3ac92d8397275d3aeeebbc59d357fd20dec2123e0ee2 *src/generated/prisma/models/SpacedRepetitionCardState.ts
|
||||
2ef43d1c5d7e2eb0d85080fb29703ae99c1704ba8f5200703e2bb8e7a617bf04 *src/generated/prisma/models/SpacedRepetitionSet.ts
|
||||
0bf9eab9f937ef2d621ccf0cea8e209eae9372a9b333dfe6b18267e563a6b2b0 *src/generated/prisma/models/SpacedRepetitionSetDeck.ts
|
||||
916bda8f95647f0dc3560bc6848c4a97d4c1f37fd5ed54e6736d9d7d45e08adc *src/generated/prisma/models/StudyActivity.ts
|
||||
36724157e76b49df49e7827eacbcb0ab8336917f35913777701d8a6830770500 *src/generated/prisma/models/StudyProgress.ts
|
||||
52
audit-results/tmp/inspect-db.cjs
Normal file
52
audit-results/tmp/inspect-db.cjs
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
// Read-only diagnostic: introspect the temp DB created by `prisma migrate deploy`
|
||||
// to check whether the migrated schema matches schema.prisma (drift check).
|
||||
// Usage: node audit-results/tmp/inspect-db.cjs <path-to-sqlite-db>
|
||||
const path = require("node:path");
|
||||
const Database = require("better-sqlite3");
|
||||
|
||||
const dbPath = process.argv[2];
|
||||
if (!dbPath) {
|
||||
console.error("usage: node inspect-db.cjs <db-path>");
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
const db = new Database(path.resolve(dbPath), { readonly: true });
|
||||
|
||||
const tables = db
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' ORDER BY name")
|
||||
.all()
|
||||
.map((r) => r.name);
|
||||
|
||||
console.log("=== TABLES ===");
|
||||
console.log(tables.join(", "));
|
||||
console.log("Has MaterialGroup table:", tables.includes("MaterialGroup"));
|
||||
|
||||
for (const table of ["Deck", "QuizSet", "ShareLink", "MaterialGroup"]) {
|
||||
if (tables.includes(table)) {
|
||||
const cols = db
|
||||
.prepare(`PRAGMA table_info('${table}')`)
|
||||
.all()
|
||||
.map((r) => r.name);
|
||||
console.log(`--- ${table} columns ---`);
|
||||
console.log(cols.join(", "));
|
||||
if (table === "Deck" || table === "QuizSet" || table === "ShareLink") {
|
||||
console.log(` -> has groupId:`, cols.includes("groupId"));
|
||||
}
|
||||
} else {
|
||||
console.log(`--- ${table}: MISSING TABLE ---`);
|
||||
}
|
||||
}
|
||||
|
||||
const indexes = db
|
||||
.prepare("SELECT name, tbl_name FROM sqlite_master WHERE type='index' ORDER BY name")
|
||||
.all()
|
||||
.map((r) => `${r.tbl_name}.${r.name}`);
|
||||
console.log("=== INDEXES (count) ===", indexes.length);
|
||||
|
||||
// Confirm migrations table
|
||||
const applied = db.prepare("SELECT migration_name FROM _prisma_migrations ORDER BY started_at").all();
|
||||
console.log("=== APPLIED MIGRATIONS ===");
|
||||
applied.forEach((m) => console.log(" -", m.migration_name));
|
||||
|
||||
db.close();
|
||||
console.log("DONE");
|
||||
40
audit-results/tmp/reproduce-drift.cjs
Normal file
40
audit-results/tmp/reproduce-drift.cjs
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
// Read-only diagnostic: execute the exact SQL shape the generated Prisma client
|
||||
// emits for deck.findMany() / materialGroup.findMany() against a DB built purely
|
||||
// from `prisma migrate deploy` (predev + Docker entrypoint flow).
|
||||
// Proves DBAUD-01: migrated DB is incompatible with the shipped client.
|
||||
const path = require("node:path");
|
||||
const Database = require("better-sqlite3");
|
||||
|
||||
const dbPath = path.resolve("audit-results/tmp/audit-migration.db");
|
||||
const db = new Database(dbPath, { readonly: true });
|
||||
|
||||
// 1. Shape of prisma.deck.findMany({ include: { class: true } }) — Prisma selects
|
||||
// every scalar field, including groupId (schema.prisma:33-34, generated client).
|
||||
const deckSql =
|
||||
'SELECT "Deck"."id", "Deck"."classId", "Deck"."name", "Deck"."description", "Deck"."sortOrder", "Deck"."createdAt", "Deck"."groupId" FROM "Deck"';
|
||||
try {
|
||||
db.prepare(deckSql).all();
|
||||
console.log("deck.findMany SQL: OK");
|
||||
} catch (e) {
|
||||
console.log("deck.findMany SQL: FAILED ->", e.message);
|
||||
}
|
||||
|
||||
// 2. Shape of prisma.materialGroup.findMany()
|
||||
const groupSql = 'SELECT "MaterialGroup"."id", "MaterialGroup"."classId", "MaterialGroup"."name", "MaterialGroup"."type", "MaterialGroup"."sortOrder", "MaterialGroup"."createdAt" FROM "MaterialGroup"';
|
||||
try {
|
||||
db.prepare(groupSql).all();
|
||||
console.log("materialGroup.findMany SQL: OK");
|
||||
} catch (e) {
|
||||
console.log("materialGroup.findMany SQL: FAILED ->", e.message);
|
||||
}
|
||||
|
||||
// 3. Shape of prisma.shareLink.findFirst() (share page validation)
|
||||
const shareSql = 'SELECT "ShareLink"."id", "ShareLink"."targetType", "ShareLink"."deckId", "ShareLink"."quizSetId", "ShareLink"."groupId", "ShareLink"."createdAt" FROM "ShareLink" LIMIT 1';
|
||||
try {
|
||||
db.prepare(shareSql).all();
|
||||
console.log("shareLink.findFirst SQL: OK");
|
||||
} catch (e) {
|
||||
console.log("shareLink.findFirst SQL: FAILED ->", e.message);
|
||||
}
|
||||
|
||||
db.close();
|
||||
27
audit-results/tmp/validate-findings.cjs
Normal file
27
audit-results/tmp/validate-findings.cjs
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
// Validates audit-results/FINDINGS.json parses and reports counts per severity.
|
||||
const fs = require("node:fs");
|
||||
const path = require("node:path");
|
||||
|
||||
const file = path.resolve("audit-results/FINDINGS.json");
|
||||
const raw = fs.readFileSync(file, "utf8");
|
||||
const data = JSON.parse(raw); // throws if invalid
|
||||
|
||||
const findings = data.findings;
|
||||
const bySeverity = {};
|
||||
for (const f of findings) {
|
||||
bySeverity[f.severity] = (bySeverity[f.severity] || 0) + 1;
|
||||
}
|
||||
const byConfidence = {};
|
||||
for (const f of findings) {
|
||||
byConfidence[f.confidence] = (byConfidence[f.confidence] || 0) + 1;
|
||||
}
|
||||
|
||||
const ids = findings.map((f) => f.id);
|
||||
const dupes = ids.filter((id, i) => ids.indexOf(id) !== i);
|
||||
if (dupes.length) throw new Error("duplicate ids: " + dupes.join(","));
|
||||
|
||||
console.log("FINDINGS.json: VALID JSON");
|
||||
console.log("total findings:", findings.length);
|
||||
console.log("by severity:", JSON.stringify(bySeverity));
|
||||
console.log("by confidence:", JSON.stringify(byConfidence));
|
||||
console.log("ids:", ids.join(", "));
|
||||
Loading…
Add table
Add a link
Reference in a new issue