|
| 1 | +-- Issue #543 (EPIC #540 §1.3) — a client may not assert its own grade, |
| 2 | +-- completion, mastery or tenant. |
| 3 | +-- |
| 4 | +-- #532 (20260725160000) stamped the rule on `enrollments`: "Never gate content, |
| 5 | +-- APIs or RLS on the presence of an enrollment row." Four write-side policies |
| 6 | +-- were never brought along. Each constrains WHO the row belongs to and nothing |
| 7 | +-- about whether the writer earned it: |
| 8 | +-- |
| 9 | +-- lesson_completions WITH CHECK (auth.uid() = user_id) |
| 10 | +-- exam_submissions WITH CHECK (auth.uid() = student_id AND tenant_id = get_tenant_id()) |
| 11 | +-- lesson_checkpoint_attempts pins identity + checkpoint consistency + access, |
| 12 | +-- but leaves score/passed/completed/evaluator_type free |
| 13 | +-- practice_attempts FOR ALL USING/WITH CHECK (user_id = auth.uid()) — no tenant predicate |
| 14 | +-- |
| 15 | +-- Plus one read-side straggler: `lesson_checkpoints` SELECT still authorizes by |
| 16 | +-- joining `enrollments`, which no revocation path ever removes. |
| 17 | +-- |
| 18 | +-- SHAPE. The three predicates below mirror the #509 content read policies |
| 19 | +-- (20260724150000): tenant match AND (staff OR course author OR |
| 20 | +-- has_course_access). Teachers and admins hold no entitlement for the courses |
| 21 | +-- they author, so the staff/author branches are what keep authoring and course |
| 22 | +-- preview working; students only ever match the has_course_access branch. |
| 23 | +-- |
| 24 | +-- `has_course_access(uuid, integer)` takes integer. `lessons.course_id` and |
| 25 | +-- `practice_attempts.course_id` are bigint, hence the ::integer casts; |
| 26 | +-- `exams.course_id` is already integer. |
| 27 | + |
| 28 | +-- --------------------------------------------------------------------------- |
| 29 | +-- 1. lesson_completions — completion is the certificate's eligibility source. |
| 30 | +-- |
| 31 | +-- `app/api/certificates/issue/route.ts` counts these rows to decide whether to |
| 32 | +-- mint the school's credential, so a self-issuable completion is a |
| 33 | +-- self-issuable certificate. The table has no `tenant_id` (see CLAUDE.md), so |
| 34 | +-- the course is reached through `lessons`. |
| 35 | +-- |
| 36 | +-- Preview lessons (#426) are deliberately NOT a branch here: a prospective |
| 37 | +-- buyer may read a preview lesson, but recording progress against a course |
| 38 | +-- they have not bought is exactly what this policy exists to stop. |
| 39 | +-- --------------------------------------------------------------------------- |
| 40 | + |
| 41 | +DROP POLICY IF EXISTS "Students can mark lessons complete" ON public.lesson_completions; |
| 42 | + |
| 43 | +CREATE POLICY "Students can mark lessons complete" |
| 44 | + ON public.lesson_completions FOR INSERT TO authenticated |
| 45 | + WITH CHECK ( |
| 46 | + (SELECT auth.uid()) = user_id |
| 47 | + AND EXISTS ( |
| 48 | + SELECT 1 FROM public.lessons l |
| 49 | + WHERE l.id = lesson_completions.lesson_id |
| 50 | + AND ( |
| 51 | + (SELECT public.is_tenant_staff()) |
| 52 | + OR EXISTS ( |
| 53 | + SELECT 1 FROM public.courses c |
| 54 | + WHERE c.course_id = l.course_id |
| 55 | + AND c.author_id = (SELECT auth.uid()) |
| 56 | + ) |
| 57 | + OR public.has_course_access((SELECT auth.uid()), l.course_id::integer) |
| 58 | + ) |
| 59 | + ) |
| 60 | + ); |
| 61 | + |
| 62 | +COMMENT ON TABLE public.lesson_completions IS |
| 63 | + 'Progress record, and the eligibility source /api/certificates/issue counts. ' |
| 64 | + 'INSERT requires course access (#543) — a completion is not self-issuable.'; |
| 65 | + |
| 66 | +-- --------------------------------------------------------------------------- |
| 67 | +-- 2. exam_submissions — submitting enqueues AI grading on the school's budget. |
| 68 | +-- |
| 69 | +-- Without an access predicate, a student with no entitlement (or one past the |
| 70 | +-- tenant's `access_cutoff_at`, which lives inside has_course_access) can POST |
| 71 | +-- submissions that `app/actions/exam-grading.ts` then grades against the |
| 72 | +-- school's plan allowance. |
| 73 | +-- --------------------------------------------------------------------------- |
| 74 | + |
| 75 | +DROP POLICY IF EXISTS "Students can create own exam submissions" ON public.exam_submissions; |
| 76 | + |
| 77 | +CREATE POLICY "Students can create own exam submissions" |
| 78 | + ON public.exam_submissions FOR INSERT TO authenticated |
| 79 | + WITH CHECK ( |
| 80 | + (SELECT auth.uid()) = student_id |
| 81 | + AND tenant_id = (SELECT public.get_tenant_id()) |
| 82 | + AND EXISTS ( |
| 83 | + SELECT 1 FROM public.exams e |
| 84 | + WHERE e.exam_id = exam_submissions.exam_id |
| 85 | + AND e.tenant_id = exam_submissions.tenant_id |
| 86 | + AND ( |
| 87 | + (SELECT public.is_tenant_staff()) |
| 88 | + OR EXISTS ( |
| 89 | + SELECT 1 FROM public.courses c |
| 90 | + WHERE c.course_id = e.course_id |
| 91 | + AND c.author_id = (SELECT auth.uid()) |
| 92 | + ) |
| 93 | + OR public.has_course_access((SELECT auth.uid()), e.course_id) |
| 94 | + ) |
| 95 | + ) |
| 96 | + ); |
| 97 | + |
| 98 | +-- --------------------------------------------------------------------------- |
| 99 | +-- 3. lesson_checkpoint_attempts — server-write-only, per the #538 pattern. |
| 100 | +-- |
| 101 | +-- #532 gated WHO may insert (own user, enabled checkpoint, consistent |
| 102 | +-- checkpoint/lesson/exercise/tenant, has_course_access) and that part holds. |
| 103 | +-- What no policy could express is that `score`, `passed`, `completed`, |
| 104 | +-- `evaluation`, `response`, `evaluator_type` and `attempt_number` must be the |
| 105 | +-- server grader's output rather than the caller's claim: |
| 106 | +-- |
| 107 | +-- * `{passed: true, score: 100, completed: true}` satisfies any is_required |
| 108 | +-- checkpoint without answering — lib/checkpoints/load.ts reads |
| 109 | +-- `latestAttempt.passed` off the newest row and required checkpoints gate |
| 110 | +-- lesson progression. |
| 111 | +-- * rows tagged `evaluator_type = 'ai'` are what countAiAttempts() bills |
| 112 | +-- against the per-student AND per-tenant monthly AI allowance, so one |
| 113 | +-- student could exhaust AI evaluation for every classmate in the school. |
| 114 | +-- |
| 115 | +-- The only legitimate writer is app/api/lesson-checkpoints/[checkpointId]/ |
| 116 | +-- attempt/route.ts, which already holds an admin client and already carries |
| 117 | +-- its own resolveCourseAccessState() gate (#535). Its insert moves to that |
| 118 | +-- client in this change, so the student INSERT grant has no remaining user. |
| 119 | +-- Reads are untouched: students still read their own rows, staff the tenant's. |
| 120 | +-- --------------------------------------------------------------------------- |
| 121 | + |
| 122 | +DROP POLICY IF EXISTS lesson_checkpoint_attempts_students_insert_own_rows |
| 123 | + ON public.lesson_checkpoint_attempts; |
| 124 | + |
| 125 | +REVOKE INSERT, UPDATE, DELETE ON public.lesson_checkpoint_attempts FROM authenticated; |
| 126 | +REVOKE INSERT, UPDATE, DELETE ON public.lesson_checkpoint_attempts FROM anon; |
| 127 | + |
| 128 | +COMMENT ON TABLE public.lesson_checkpoint_attempts IS |
| 129 | + 'Server-write-only (#543). Every column but the identity ones is a grading ' |
| 130 | + 'output; the attempt API route writes them with the service-role client after ' |
| 131 | + 'grading. `authenticated` holds SELECT only — a user-scoped INSERT failing ' |
| 132 | + 'with "permission denied for table lesson_checkpoint_attempts" is by design.'; |
| 133 | + |
| 134 | +-- --------------------------------------------------------------------------- |
| 135 | +-- 4. practice_attempts — two SECURITY DEFINER triggers fire on a row the |
| 136 | +-- caller fully controls. |
| 137 | +-- |
| 138 | +-- handle_practice_attempt_elo → elo_apply_match(new.tenant_id, …) writes the |
| 139 | +-- TENANT-SHARED `item_ratings` anchors and `student_topic_ratings`. |
| 140 | +-- handle_practice_attempt_xp → award_xp(…, new.tenant_id) upserts a |
| 141 | +-- `gamification_profiles` row for (user_id, new.tenant_id) unconditionally, |
| 142 | +-- which is the eligibility source rollover_leagues() scans — so a |
| 143 | +-- non-member appears in another school's league standings. |
| 144 | +-- |
| 145 | +-- 20260716120000_create_elo_ratings.sql justified the trigger design with "a |
| 146 | +-- student session cannot write shared rating rows from application code". That |
| 147 | +-- only holds if the row's tenant is the caller's own, which nothing checked. |
| 148 | +-- |
| 149 | +-- The blanket FOR ALL policy is split so graded history is append-only: no |
| 150 | +-- UPDATE or DELETE policy, and the grants go with it. |
| 151 | +-- --------------------------------------------------------------------------- |
| 152 | + |
| 153 | +DROP POLICY IF EXISTS students_own_practice_attempts ON public.practice_attempts; |
| 154 | + |
| 155 | +CREATE POLICY practice_attempts_students_read_own_rows |
| 156 | + ON public.practice_attempts FOR SELECT TO authenticated |
| 157 | + USING (user_id = (SELECT auth.uid())); |
| 158 | + |
| 159 | +CREATE POLICY practice_attempts_students_insert_own_rows |
| 160 | + ON public.practice_attempts FOR INSERT TO authenticated |
| 161 | + WITH CHECK ( |
| 162 | + user_id = (SELECT auth.uid()) |
| 163 | + -- Membership of the tenant being written to, from `tenant_users` rather |
| 164 | + -- than the client-settable x-tenant-id header. |
| 165 | + AND EXISTS ( |
| 166 | + SELECT 1 FROM public.tenant_users tu |
| 167 | + WHERE tu.user_id = (SELECT auth.uid()) |
| 168 | + AND tu.tenant_id = practice_attempts.tenant_id |
| 169 | + AND tu.status = 'active' |
| 170 | + ) |
| 171 | + ); |
| 172 | + |
| 173 | +-- Deliberately NOT gated on `course_id`. It is an optional, model-supplied |
| 174 | +-- label on the MCP tool ("Related course, if any" — |
| 175 | +-- mcp-server/src/tools/practice.ts), not a value derived from anything the |
| 176 | +-- server verified, so an access predicate on it would turn a mislabelled |
| 177 | +-- drill into a hard write failure. It also guards little: the cross-tenant |
| 178 | +-- attack this policy exists to stop is closed by the membership check above, |
| 179 | +-- and `course_id` only scopes `item_ratings` anchors WITHIN the caller's own |
| 180 | +-- tenant. |
| 181 | + |
| 182 | +REVOKE UPDATE, DELETE ON public.practice_attempts FROM authenticated; |
| 183 | +REVOKE INSERT, UPDATE, DELETE ON public.practice_attempts FROM anon; |
| 184 | + |
| 185 | +-- Self-reported figures still decide XP (#349), the #393 mastery gate and the |
| 186 | +-- #396 Elo update, so bound them to what a score can actually be. |
| 187 | +ALTER TABLE public.practice_attempts |
| 188 | + DROP CONSTRAINT IF EXISTS practice_attempts_score_range; |
| 189 | +ALTER TABLE public.practice_attempts |
| 190 | + ADD CONSTRAINT practice_attempts_score_range |
| 191 | + CHECK (score >= 0 AND score <= 100); |
| 192 | + |
| 193 | +ALTER TABLE public.practice_attempts |
| 194 | + DROP CONSTRAINT IF EXISTS practice_attempts_counts_coherent; |
| 195 | +ALTER TABLE public.practice_attempts |
| 196 | + ADD CONSTRAINT practice_attempts_counts_coherent |
| 197 | + CHECK ( |
| 198 | + total_questions > 0 |
| 199 | + AND correct_count >= 0 |
| 200 | + AND correct_count <= total_questions |
| 201 | + ); |
| 202 | + |
| 203 | +-- `mode` was pinned at 20260715120000; `source` never was. |
| 204 | +ALTER TABLE public.practice_attempts |
| 205 | + DROP CONSTRAINT IF EXISTS practice_attempts_source_known; |
| 206 | +ALTER TABLE public.practice_attempts |
| 207 | + ADD CONSTRAINT practice_attempts_source_known |
| 208 | + CHECK (source IN ('mcp-tutor')); |
| 209 | + |
| 210 | +COMMENT ON TABLE public.practice_attempts IS |
| 211 | + 'AI-tutor drill history. Append-only for `authenticated` (#543): INSERT ' |
| 212 | + 'requires active membership of the named tenant, and score/count columns ' |
| 213 | + 'are range-checked, because two SECURITY DEFINER triggers write ' |
| 214 | + 'tenant-shared Elo anchors and gamification rows from this row.'; |
| 215 | + |
| 216 | +-- --------------------------------------------------------------------------- |
| 217 | +-- 5. lesson_checkpoints SELECT — the last `enrollments` join in the feature. |
| 218 | +-- |
| 219 | +-- A refunded student keeps their `enrollments` row (lib/payments/ |
| 220 | +-- webhook-dispatch.ts revokes entitlements, never enrollments), so they kept |
| 221 | +-- reading every enabled checkpoint on the course. Same shape as before — |
| 222 | +-- is_enabled, tenant match, lesson in the same tenant — with the enrollment |
| 223 | +-- join replaced by the access gate the rest of the feature uses. |
| 224 | +-- Staff are already covered by lesson_checkpoints_teachers_manage_tenant. |
| 225 | +-- --------------------------------------------------------------------------- |
| 226 | + |
| 227 | +DROP POLICY IF EXISTS lesson_checkpoints_students_can_read_enrolled_lessons |
| 228 | + ON public.lesson_checkpoints; |
| 229 | + |
| 230 | +CREATE POLICY lesson_checkpoints_students_can_read_entitled_lessons |
| 231 | + ON public.lesson_checkpoints FOR SELECT TO authenticated |
| 232 | + USING ( |
| 233 | + is_enabled |
| 234 | + AND EXISTS ( |
| 235 | + SELECT 1 FROM public.lessons l |
| 236 | + WHERE l.id = lesson_checkpoints.lesson_id |
| 237 | + AND l.tenant_id = lesson_checkpoints.tenant_id |
| 238 | + AND public.has_course_access((SELECT auth.uid()), l.course_id::integer) |
| 239 | + ) |
| 240 | + ); |
0 commit comments