Merge pull request #327 from guillermoscript/fix/exercise-eval-media-rls

fix(rls): close cross-tenant leak in exercise evaluation/media RLS

Author: guillermoscript

supabase/migrations/20260611130000_create_exercise_evaluations.sql

@@ -0,0 +1,76 @@
+-- exercise_evaluations: unified AI-evaluation record across all exercise engines
+-- (text/simulation via lib/ai/tools.ts + artifact/evaluate route, audio/video via media/analyze route).
+-- Code wrote+read this table for months but the CREATE was never shipped, so every AI
+-- exercise-evaluation insert failed silently (relation-not-found) and the student exercise
+-- history + the artifact-route rate-limit both read nothing. This migration adds the table
+-- the live code already expects. Shape derived verbatim from the insert/select call sites.
+
+create table if not exists public.exercise_evaluations (
+  id                bigint generated by default as identity primary key,
+  exercise_id       bigint      not null references public.exercises(id) on delete cascade,
+  user_id           uuid        not null references public.profiles(id) on delete cascade,
+  tenant_id         uuid        not null references public.tenants(id)   on delete cascade,
+  engine_type       text        not null,              -- 'text' | 'simulation' | 'audio' | 'video'
+  submission_id     bigint,                            -- polymorphic ref (e.g. exercise_media_submissions.id); no FK
+  submission_source text,                              -- e.g. 'exercise_media_submissions'
+  score             numeric(5,2),
+  passed            boolean     not null default false,
+  ai_result         jsonb       not null default '{}'::jsonb,
+  ai_metrics        jsonb,
+  attempt_number    integer     not null default 1,    -- auto-assigned by trigger (no caller sets it)
+  created_at        timestamptz not null default now()
+);
+
+-- Serves both hot reads: page history (exercise_id+user_id+tenant_id, order by created_at desc)
+-- and the artifact-route rate-limit count (same keys + created_at >= now()-1h).
+create index if not exists exercise_evaluations_lookup_idx
+  on public.exercise_evaluations (exercise_id, user_id, tenant_id, created_at desc);
+
+-- attempt_number is read by the student exercise page but written by no caller; auto-number it
+-- per (user, exercise) so the displayed history is meaningful.
+create or replace function public.set_exercise_evaluation_attempt_number()
+returns trigger
+language plpgsql
+as $$
+begin
+  select count(*) + 1
+    into new.attempt_number
+  from public.exercise_evaluations
+  where exercise_id = new.exercise_id
+    and user_id = new.user_id;
+  return new;
+end;
+$$;
+
+create trigger trg_exercise_evaluation_attempt
+  before insert on public.exercise_evaluations
+  for each row execute function public.set_exercise_evaluation_attempt_number();
+
+-- RLS: a student has full access to their own rows; a teacher/admin may read any row in a
+-- tenant where they hold an active role. Tenant scope is derived from authoritative
+-- `tenant_users` membership (bound to auth.uid()), NOT the `x-tenant-id` request header —
+-- the header is client-settable on a direct PostgREST call, so a header-only teacher policy
+-- would let any authenticated user read another tenant's evaluations by spoofing it.
+-- Server-side writes use the service-role admin client and bypass RLS.
+alter table public.exercise_evaluations enable row level security;
+
+-- Students: own rows only. user_id is globally unique and bound to the JWT, so this fully
+-- scopes access without any tenant header.
+create policy students_own_evaluations on public.exercise_evaluations
+  for all
+  using ( user_id = (select auth.uid()) )
+  with check ( user_id = (select auth.uid()) );
+
+-- Teachers/admins: rows in tenants where they hold an active teacher/admin membership.
+create policy teachers_view_tenant_evaluations on public.exercise_evaluations
+  for select
+  using (
+    exists (
+      select 1
+      from public.tenant_users tu
+      where tu.user_id = (select auth.uid())
+        and tu.tenant_id = exercise_evaluations.tenant_id
+        and tu.role in ('teacher', 'admin')
+        and tu.status = 'active'
+    )
+  );

supabase/migrations/20260612120000_fix_media_submissions_teacher_rls.sql

@@ -0,0 +1,38 @@
+-- Fix cross-tenant leak in exercise_media_submissions RLS.
+--
+-- The original policies (migration 20260302200000_add_voice_exercises.sql) scoped tenant via
+-- the `x-tenant-id` request header:
+--
+--   teachers_view_tenant_media_submissions  USING ( tenant_id = header('x-tenant-id') )
+--
+-- That header is client-settable on a direct PostgREST call, and the teacher policy had no
+-- user/role predicate. SELECT policies are OR'd, so ANY authenticated user could read every
+-- row of an arbitrary tenant by sending `x-tenant-id: <victim-tenant-uuid>`. The student
+-- policy was safe (bound by `user_id = auth.uid()`), but it also relied on the spoofable
+-- header for its tenant clause.
+--
+-- Fix: derive tenant scope from authoritative `tenant_users` membership (bound to auth.uid()),
+-- never the header. Server-side writes still use the service-role admin client (bypass RLS).
+
+drop policy if exists "students_own_media_submissions" on public.exercise_media_submissions;
+drop policy if exists "teachers_view_tenant_media_submissions" on public.exercise_media_submissions;
+
+-- Students: own rows only. user_id is globally unique and bound to the JWT.
+create policy "students_own_media_submissions" on public.exercise_media_submissions
+  for all
+  using ( user_id = (select auth.uid()) )
+  with check ( user_id = (select auth.uid()) );
+
+-- Teachers/admins: rows in tenants where they hold an active teacher/admin membership.
+create policy "teachers_view_tenant_media_submissions" on public.exercise_media_submissions
+  for select
+  using (
+    exists (
+      select 1
+      from public.tenant_users tu
+      where tu.user_id = (select auth.uid())
+        and tu.tenant_id = exercise_media_submissions.tenant_id
+        and tu.role in ('teacher', 'admin')
+        and tu.status = 'active'
+    )
+  );