Skip to content

Commit 93dd093

Browse files
guillermoscriptclaude
guillermoscript
and
claude
committed
fix(security): gate exam questions, options and lesson comments through their parent (#542)
`exam_questions` and `question_options` were created `USING (true)` by 20260313152048 and left behind when #509 gated their parent `exams`, so any authenticated user of any tenant — holding no entitlement, past their school's `access_cutoff_at` — could read every exam's `correct_answer`, `grading_rubric`, `ai_grading_criteria` and `is_correct` straight over PostgREST. `lesson_comments` was the same shape with lower stakes. These tables have no `tenant_id` of their own, so the new predicates route through the FK to the parent that does, mirroring the #509 `exams` gate (tenant + staff/author/entitlement) plus the pre-existing super-admin policy via a shared `can_read_exam()` helper. Also tightened, beyond the four SELECT policies the issue lists: the sibling `FOR ALL` staff policies on both exam tables. They matched on `get_tenant_role() IN ('teacher','admin')` with no row predicate at all, and permissive policies OR together — so a teacher of tenant A kept reading and writing every other tenant's exam questions through that policy, and the SELECT fix alone would have bought nothing for staff accounts. And converged the #509 authenticated preview branch with the anon one by requiring the parent course to be published: a teacher drafting a course who marked a lesson preview + published was exposing that body to every signed-in member of the tenant while anonymous visitors correctly could not see it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xwf7RJcxHNRVXb3ufFuT8g
1 parent 146a8cf commit 93dd093

2 files changed

Lines changed: 656 additions & 0 deletions

File tree

‎supabase/migrations/20260726100000_rls_exam_child_tables_tenant_gate.sql‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
-- Issue #542 (EPIC #540 §1.2) — gate the exam child tables and lesson comments
2+
-- through their parent, and stop the #509 preview branch overshooting.
3+
--
4+
-- `20260313152048_rls_exam_child_tables.sql` created the SELECT policies on
5+
-- `exam_questions` and `question_options` as `USING (true)`. #509 replaced the
6+
-- **parent** `exams` policy with a tenant + staff/author + `has_course_access`
7+
-- gate and stopped there, so the answer key (`correct_answer`, `grading_rubric`,
8+
-- `ai_grading_criteria`, `expected_keywords`, `question_options.is_correct`) was
9+
-- readable over PostgREST by any authenticated user of any tenant, holding no
10+
-- entitlement, past their school's `access_cutoff_at`. `lesson_comments` is the
11+
-- same shape with lower stakes.
12+
--
13+
-- These tables have NO `tenant_id` of their own (see CLAUDE.md — adding one to a
14+
-- query errors it outright, which is what broke teacher grading in #282), so the
15+
-- predicate routes through the FK to the parent that does.
16+
--
17+
-- ALSO tightened, beyond the four SELECT policies the issue lists: the sibling
18+
-- `FOR ALL` staff policies on both exam tables. They read
19+
-- `get_tenant_role() IN ('teacher','admin')` with **no row predicate at all**, and
20+
-- permissive policies OR together — so without this a teacher of tenant A keeps
21+
-- reading (and writing) every exam question in every other tenant through that
22+
-- policy, and the SELECT fix below buys nothing for staff accounts.
23+
--
24+
-- Every policy is DROP + CREATE, never supplemented: permissive policies OR
25+
-- together, so an added policy could only ever widen access.
26+
27+
-- Shared predicate for the two exam child tables. Mirrors the #509 `exams`
28+
-- SELECT policy ("Students can view tenant exams") plus the pre-existing
29+
-- "Super admins can view all exams" policy, so a row is readable exactly when
30+
-- its parent `exams` row is.
31+
--
32+
-- SECURITY INVOKER on purpose: `get_tenant_id()`, `auth.uid()` and
33+
-- `is_tenant_staff()` must resolve to the caller, and RLS on `exams` still
34+
-- applies inside it, which makes the check belt-and-braces rather than a
35+
-- second, drift-prone copy of the parent gate.
36+
CREATE OR REPLACE FUNCTION public.can_read_exam(_exam_id integer)
37+
RETURNS boolean
38+
LANGUAGE sql
39+
STABLE
40+
SECURITY INVOKER
41+
SET search_path TO 'public'
42+
AS $function$
43+
SELECT EXISTS (
44+
SELECT 1 FROM exams e
45+
WHERE e.exam_id = _exam_id
46+
AND (
47+
(SELECT is_super_admin())
48+
OR (
49+
e.tenant_id = (SELECT get_tenant_id())
50+
AND (
51+
-- Staff branch. Teachers and admins hold no entitlement for the
52+
-- courses they author, and the exam editor, the teacher grading
53+
-- screens and the AI grading path all read these tables through an
54+
-- RLS-bound user client — drop this and grading breaks rather than
55+
-- fails safe.
56+
(SELECT is_tenant_staff())
57+
OR EXISTS (
58+
SELECT 1 FROM courses c
59+
WHERE c.course_id = e.course_id
60+
AND c.author_id = (SELECT auth.uid())
61+
)
62+
OR has_course_access((SELECT auth.uid()), e.course_id)
63+
)
64+
)
65+
)
66+
);
67+
$function$;
68+
69+
COMMENT ON FUNCTION public.can_read_exam(integer) IS
70+
'True when the caller may read the given exam under the #509 gate (tenant + staff/author/entitlement, or super admin). Used by the exam_questions and question_options SELECT policies, which have no tenant_id of their own (#542).';
71+
72+
-- Functions in `public` carry a default PUBLIC execute grant, which would put
73+
-- this on the REST surface for `anon` too. Neither exam child table is readable
74+
-- by anon, so anon never needs it.
75+
REVOKE EXECUTE ON FUNCTION public.can_read_exam(integer) FROM PUBLIC, anon;
76+
GRANT EXECUTE ON FUNCTION public.can_read_exam(integer) TO authenticated;
77+
78+
-- exam_questions ---------------------------------------------------------
79+
DROP POLICY IF EXISTS "Authenticated users can view exam questions" ON public.exam_questions;
80+
CREATE POLICY "Users can view questions of readable exams"
81+
ON public.exam_questions FOR SELECT TO authenticated
82+
USING (public.can_read_exam(exam_questions.exam_id));
83+
84+
-- Staff CRUD, scoped to the tenant the caller is acting in. `get_tenant_role()`
85+
-- is kept (rather than swapped for `is_tenant_staff()`) to leave the write gate
86+
-- itself untouched; the change here is purely the added row predicate.
87+
DROP POLICY IF EXISTS "Teachers and admins can manage exam questions" ON public.exam_questions;
88+
CREATE POLICY "Teachers and admins can manage exam questions"
89+
ON public.exam_questions FOR ALL TO authenticated
90+
USING (
91+
(SELECT public.get_tenant_role()) IN ('teacher', 'admin')
92+
AND EXISTS (
93+
SELECT 1 FROM public.exams e
94+
WHERE e.exam_id = exam_questions.exam_id
95+
AND e.tenant_id = (SELECT public.get_tenant_id())
96+
)
97+
)
98+
WITH CHECK (
99+
(SELECT public.get_tenant_role()) IN ('teacher', 'admin')
100+
AND EXISTS (
101+
SELECT 1 FROM public.exams e
102+
WHERE e.exam_id = exam_questions.exam_id
103+
AND e.tenant_id = (SELECT public.get_tenant_id())
104+
)
105+
);
106+
107+
-- question_options -------------------------------------------------------
108+
-- Two hops: question_options -> exam_questions -> exams.
109+
DROP POLICY IF EXISTS "Authenticated users can view question options" ON public.question_options;
110+
CREATE POLICY "Users can view options of readable exams"
111+
ON public.question_options FOR SELECT TO authenticated
112+
USING (
113+
EXISTS (
114+
SELECT 1 FROM public.exam_questions q
115+
WHERE q.question_id = question_options.question_id
116+
AND public.can_read_exam(q.exam_id)
117+
)
118+
);
119+
120+
DROP POLICY IF EXISTS "Teachers and admins can manage question options" ON public.question_options;
121+
CREATE POLICY "Teachers and admins can manage question options"
122+
ON public.question_options FOR ALL TO authenticated
123+
USING (
124+
(SELECT public.get_tenant_role()) IN ('teacher', 'admin')
125+
AND EXISTS (
126+
SELECT 1 FROM public.exam_questions q
127+
JOIN public.exams e ON e.exam_id = q.exam_id
128+
WHERE q.question_id = question_options.question_id
129+
AND e.tenant_id = (SELECT public.get_tenant_id())
130+
)
131+
)
132+
WITH CHECK (
133+
(SELECT public.get_tenant_role()) IN ('teacher', 'admin')
134+
AND EXISTS (
135+
SELECT 1 FROM public.exam_questions q
136+
JOIN public.exams e ON e.exam_id = q.exam_id
137+
WHERE q.question_id = question_options.question_id
138+
AND e.tenant_id = (SELECT public.get_tenant_id())
139+
)
140+
);
141+
142+
-- lessons ----------------------------------------------------------------
143+
-- #509 gave authenticated users a preview branch of
144+
-- `(is_preview AND status = 'published')`, justified as "a logged-in
145+
-- prospective buyer must not see less than an anonymous one". The anon policy
146+
-- it mirrors carries one more condition: the parent course must be published
147+
-- too. So a teacher drafting a course who marks a lesson preview + published
148+
-- exposed that lesson body to every signed-in member of the tenant while
149+
-- anonymous visitors correctly could not see it. Converge the two.
150+
--
151+
-- Only the preview branch changes; the staff, author and entitlement branches
152+
-- are carried over verbatim from 20260724150000.
153+
DROP POLICY IF EXISTS "Users can view tenant lessons" ON public.lessons;
154+
CREATE POLICY "Users can view tenant lessons"
155+
ON public.lessons FOR SELECT TO authenticated
156+
USING (
157+
tenant_id = (SELECT public.get_tenant_id())
158+
AND (
159+
(SELECT public.is_tenant_staff())
160+
OR (
161+
is_preview = true
162+
AND status = 'published'
163+
AND EXISTS (
164+
SELECT 1 FROM public.courses c
165+
WHERE c.course_id = lessons.course_id
166+
AND c.status = 'published'
167+
AND c.tenant_id = lessons.tenant_id
168+
)
169+
)
170+
OR EXISTS (
171+
SELECT 1 FROM public.courses c
172+
WHERE c.course_id = lessons.course_id
173+
AND c.author_id = (SELECT auth.uid())
174+
)
175+
OR public.has_course_access((SELECT auth.uid()), lessons.course_id::integer)
176+
)
177+
);
178+
179+
-- lesson_comments --------------------------------------------------------
180+
-- No tenant_id here either; mirror the lessons gate above through the FK,
181+
-- plus the pre-existing "Super admins can view all lessons" policy, so a
182+
-- thread is readable exactly when the lesson it hangs off is.
183+
DROP POLICY IF EXISTS "Authenticated users can view lesson comments" ON public.lesson_comments;
184+
CREATE POLICY "Users can view comments on readable lessons"
185+
ON public.lesson_comments FOR SELECT TO authenticated
186+
USING (
187+
EXISTS (
188+
SELECT 1 FROM public.lessons l
189+
WHERE l.id = lesson_comments.lesson_id
190+
AND (
191+
(SELECT public.is_super_admin())
192+
OR (
193+
l.tenant_id = (SELECT public.get_tenant_id())
194+
AND (
195+
(SELECT public.is_tenant_staff())
196+
OR (
197+
l.is_preview = true
198+
AND l.status = 'published'
199+
AND EXISTS (
200+
SELECT 1 FROM public.courses c
201+
WHERE c.course_id = l.course_id
202+
AND c.status = 'published'
203+
AND c.tenant_id = l.tenant_id
204+
)
205+
)
206+
OR EXISTS (
207+
SELECT 1 FROM public.courses c
208+
WHERE c.course_id = l.course_id
209+
AND c.author_id = (SELECT auth.uid())
210+
)
211+
OR public.has_course_access((SELECT auth.uid()), l.course_id::integer)
212+
)
213+
)
214+
)
215+
)
216+
);
217+
218+
-- Supporting indexes. Each new predicate looks the child rows up by their FK,
219+
-- and none of these three FKs was indexed.
220+
CREATE INDEX IF NOT EXISTS idx_exam_questions_exam_id
221+
ON public.exam_questions (exam_id);
222+
CREATE INDEX IF NOT EXISTS idx_question_options_question_id
223+
ON public.question_options (question_id);
224+
CREATE INDEX IF NOT EXISTS idx_lesson_comments_lesson_id
225+
ON public.lesson_comments (lesson_id);

0 commit comments

Comments
 (0)