Skip to content

Commit c8ce9ca

Browse files
fix(security): drop the out-of-scope course_id predicate on practice_attempts (#543)
`course_id` on the record-practice MCP tool is an optional, model-supplied label ("Related course, if any" — mcp-server/src/tools/practice.ts:810), not a value the server derives from anything it verified. Gating the INSERT on it turned a mislabelled drill into a hard 42501, with no MCP test harness to catch the regression. It also bought little. The cross-tenant attack this policy exists to stop — driving another school's shared Elo anchors and appearing in its league standings — is closed entirely by the tenant_users membership check; `course_id` only scopes `item_ratings` rows within the caller's own tenant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SbrtmdXw6ZwH2UeTCuT2v
1 parent 8c63d08 commit c8ce9ca

1 file changed

Lines changed: 12 additions & 12 deletions

File tree

supabase/migrations/20260726110000_write_side_entitlement_gates.sql

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,17 @@ CREATE POLICY practice_attempts_students_insert_own_rows
168168
AND tu.tenant_id = practice_attempts.tenant_id
169169
AND tu.status = 'active'
170170
)
171-
-- Topic-only drills carry no course. When one is named, it must be a
172-
-- course the caller can reach — the same gate that let them read the
173-
-- exercise the drill was generated from.
174-
AND (
175-
course_id IS NULL
176-
OR (SELECT public.is_tenant_staff())
177-
OR public.has_course_access((SELECT auth.uid()), course_id::integer)
178-
)
179171
);
180172

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+
181182
REVOKE UPDATE, DELETE ON public.practice_attempts FROM authenticated;
182183
REVOKE INSERT, UPDATE, DELETE ON public.practice_attempts FROM anon;
183184

@@ -208,10 +209,9 @@ ALTER TABLE public.practice_attempts
208209

209210
COMMENT ON TABLE public.practice_attempts IS
210211
'AI-tutor drill history. Append-only for `authenticated` (#543): INSERT '
211-
'requires active membership of the named tenant and access to the named '
212-
'course, and score/count columns are range-checked, because two SECURITY '
213-
'DEFINER triggers write tenant-shared Elo anchors and gamification rows '
214-
'from this row.';
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.';
215215

216216
-- ---------------------------------------------------------------------------
217217
-- 5. lesson_checkpoints SELECT — the last `enrollments` join in the feature.

0 commit comments

Comments
 (0)