Skip to content

Lock the write side: grades, progress and practice rows are self-issuable #543

Description

@guillermoscript

Part of EPIC #540. §1.3.

What's wrong

Four write-side policies constrain who the row belongs to but not whether the writer earned it. All four verified live in pg_policies.

lesson_completions — completion is self-issuable, and certificates count it

INSERT WITH CHECK: (SELECT auth.uid()) = user_id

That is the entire policy (supabase/migrations/20260131170137_fix_lesson_completions_rls.sql:13-16). No course gate, no tenant, no check that the lesson exists or that the caller can reach it. Any authenticated user can insert a completion for any lesson_id.

That matters because app/api/certificates/issue/route.ts — student self-serve branch, :16-85 — checks auth, tenant ownership of the course and duplicates, but never calls hasCourseAccess / resolveCourseAccessState. Eligibility falls through to a raw lesson_completions count at :176-186. So the school's credential can be minted by someone who never held access.

exam_submissions — no access gate on submit

INSERT WITH CHECK: ((SELECT auth.uid()) = student_id) AND (tenant_id = (SELECT get_tenant_id()))

(supabase/migrations/20260313151603_rls_exams_exam_submissions.sql:46-48.) No has_course_access. A student with no entitlement, or one past their school's access_cutoff_at, can POST submissions that then enter the AI grading pipeline (app/actions/exam-grading.ts) on the school's plan budget. Paired with the sibling issue in this epic (§1.2) they can read the questions first.

lesson_checkpoint_attempts — the grade is client-supplied

The policy pins user_id, checkpoint/lesson/exercise consistency, c.is_enabled and has_course_access (that is #532/#535's fix and it holds). But score, passed, completed, evaluation, response, evaluator_type and attempt_number are all unconstrained, and authenticated holds the INSERT grant.

app/api/lesson-checkpoints/[checkpointId]/attempt/route.ts:285-299 grades server-side and then inserts with the caller's own RLS client — the code comment says "so DB policies are the last word", but the policies say nothing about the graded columns. So {passed: true, score: 100, completed: true} satisfies any is_required checkpoint without answering; lib/checkpoints/load.ts:73-96 derives latestAttempt.passed straight from the newest row, and required checkpoints gate lesson progression.

Second effect: countAiAttempts (:403-418) counts rows with evaluator_type = 'ai' to enforce the per-student and per-tenant monthly AI-eval allowance (checkpoint_ai_evals_per_month). Inserting rows tagged 'ai' lets one student exhaust AI evaluation for every classmate in the school.

practice_attempts — no tenant predicate at all

FOR ALL, USING/WITH CHECK: user_id = (SELECT auth.uid())

(supabase/migrations/20260710120000_create_practice_attempts.sql:35-38; authenticated holds INSERT/UPDATE/DELETE.) Nothing constrains tenant_id, course_id, topic, score or mode — only an FK check applies to tenant_id.

Two SECURITY DEFINER triggers fire on that fully caller-controlled row:

  • handle_practice_attempt_elo (20260716120000_create_elo_ratings.sql:245-256) → elo_apply_match(new.tenant_id, …) writes the tenant-shared item_ratings anchors and student_topic_ratings.
  • handle_practice_attempt_xp (20260710120000:56-78) → award_xp(…, new.tenant_id), which upserts a gamification_profiles row for (user_id, new.tenant_id) unconditionally.

So: insert naming a foreign tenant_id and you drive another school's adaptive-difficulty anchors, and you create a gamification_profiles row there — which is exactly the eligibility source rollover_leagues scans (20260717120000:329-337), so a non-member shows up in that school's league standings with their name and avatar. In your own tenant, self-reported score: 100 makes the #393 mastery gate and the #396 rating self-certifying, and farms XP to the 10/day cap.

20260716120000_create_elo_ratings.sql:13-17 states the security rationale — "the MCP server runs every query with the caller's RLS-scoped token … so a student session cannot write shared rating rows from application code". The trigger fires on a row the student fully controls, so the stated rationale does not hold. Decision-doc/code drift.

The rule this violates

supabase/migrations/20260725160000_entitlement_gated_enrollment_inserts.sql:61-64 already stamps it on enrollments:

Never gate content, APIs or RLS on the presence of an enrollment row.

#532 applied that to one table. The generalisation — never let a client assert its own grade, completion, mastery or tenant — was never applied to the other four.

Bonus, same class: lesson_checkpoints SELECT still authorizes by joining enrollments (20260714221024_create_lesson_checkpoints.sql:67-79, unchanged by #532). A refunded student keeps their enrollments row — webhook-dispatch.ts revokes entitlements, never enrollments — and keeps reading every enabled checkpoint on the course. It is the one place in the checkpoint feature where has_course_access is not consulted.

Scope

In scope

  • lesson_completions INSERT: add a course-access predicate. Route through lessons (which has no tenant_id — see CLAUDE.md) to reach has_course_access.
  • exam_submissions INSERT: add has_course_access.
  • lesson_checkpoint_attempts: make it server-write-only — mirror the Solana settlement columns are caller-controlled at INSERT — a pending row can under-quote what it owes (#528 follow-up) #538 transactions pattern. The route already holds an admin client; switch the insert to it and revoke the student INSERT grant/policy. Leave the read policies alone.
  • practice_attempts: split the blanket FOR ALL into separate SELECT and INSERT policies; require an active tenant_users row for (auth.uid(), tenant_id); clamp score with a CHECK; pin mode/source to the known values. Do not leave UPDATE/DELETE open on graded history.
  • lesson_checkpoints SELECT: replace the enrollments join with has_course_access(auth.uid(), l.course_id::integer), keeping is_enabled and the tenant match.
  • app/api/certificates/issue/route.ts: call resolveCourseAccessState at the top of the student branch.

Out of scope

Watch out

  • has_course_access is (uuid, integer) — cast ::int from SQL.
  • lesson_completions is written from several client components; tightening it needs the full student-journey E2E green.
  • The MCP write path sets tenant_id from the session (mcp-server/src/tools/practice.ts:923-942), so it keeps working. Any other writer setting a foreign tenant starts failing — that is the point.
  • useEnrollment calls the self_enroll_subscription_course SECDEF RPC; the only direct enrollments.insert() calls are in scripts/ and tests/.

Acceptance criteria

  • A Playwright spec proves each of the four writes is rejected for a caller who has not earned it: completion for an inaccessible lesson, exam submission without entitlement, checkpoint attempt with passed: true written directly, practice_attempts row naming a foreign tenant.
  • A checkpoint attempt's score/passed in the database always equals what the server grader computed — assert by submitting a wrong answer and reading the row back.
  • /api/certificates/issue refuses a caller without course access even when lesson_completions rows exist.
  • A refunded student (entitlement revoked, enrollments row intact) gets zero rows from lesson_checkpoints. Extend the existing fixtures at tests/playwright/checkpoint-access.spec.ts:140,177.
  • Legitimate paths unchanged: student completes a lesson, submits an exam, passes a checkpoint, records practice via MCP; teacher grading works.
  • npm run test:unit green (370/370 baseline at 146a8cfa).

Effort: M · Risk: MED — four write paths used by live UI. No users exist, so prefer the correct end state over a compatible one; just keep the E2E green.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Sub-taskdb-migrationRequires a Supabase migrationsecuritySecurity vulnerability or hardeningseverity:highHigh severity security finding

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions