Repository navigation
fix(rls): close cross-section gaps found by pgTAP - #40
Merged
Merged
Conversation
- student_session_attendance: drop the CR branch that let any CR read every section's register logs (own-section reads stay via FOR ALL). - fn_sync_session_attendance: SECURITY DEFINER so CR register marks sync into attendance_records; rejects students outside the session's section. - section_teachers: composite FK (subject_id, section_id) -> subjects so no writer, including join_section_as_teacher, can link a foreign subject. - counsellor_notes: writes need a counsellor link to the student's section; the column-lock trigger keys on "caller is the note's student", not role. - submissions: student and CR branches pin rows to the assignment's section. - announcement_comments: trigger rejects identity-column changes and non-author content edits; drop the always-true WITH CHECK terms. Every todo() for these gaps is removed, so each check must now pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the cross-section gaps that #39 recorded as pgTAP
todo()tests. Every one of thosetodo()lines is removed, so each check now has to pass outright.RLS proposals (confirm by merging)
RLS POLICY PROPOSED - require confirm before apply
Table: student_session_attendance
Policy name: "Teachers and CRs read all section attendance logs" → "Teachers read section attendance logs"
SQL: see block 1 in
20261011140000_fix_cross_section_rls_gaps.sqlPlain English: Teachers read logs for sessions in sections they teach. CRs read only their own section, through the existing FOR ALL policy. A CR can no longer read another section's register.
Test: attendance_rls "CR of section B cannot read section A session logs"
RLS POLICY PROPOSED - require confirm before apply
Table: attendance_records (via trigger fn_sync_session_attendance)
Policy name: n/a (trigger now SECURITY DEFINER, search_path pinned, EXECUTE revoked)
SQL: block 2
Plain English: When a CR or teacher marks the register, the student's totals actually update now (before this, CR marks failed). Marking a student who isn't in the session's section is rejected.
Test: "CR marks attendance in own section session", "CR cannot mark a student of another section in own session", "classmate attendance changed only through the CR register (+1 present)"
RLS POLICY PROPOSED - require confirm before apply
Table: section_teachers
Policy name: n/a (constraint
section_teachers_subject_in_section_fkey)SQL: block 3
Plain English: A teacher link can only name a subject from the same section. This holds for CR inserts, client inserts and
join_section_as_teacher(invite, p_subject_id), which never checked it.Test: "CR cannot link a subject from another section to their section" (expects 23503)
RLS POLICY PROPOSED - require confirm before apply
Table: counsellor_notes
Policy name: "Counsellors manage own remarks" (WITH CHECK tightened); trigger check_student_counsellor_notes_update
SQL: block 4
Plain English: Only a teacher flagged as counsellor for the student's section can write or repoint a remark. The note's own student (including a CR) can only change their response.
Test: counsellor_notes_rls's three cross-section checks
RLS POLICY PROPOSED - require confirm before apply
Table: submissions
Policy name: "authenticated_insert_submissions", "authenticated_update_submissions"
SQL: block 5
Plain English: A student can't move their submission onto another section's assignment. A CR can't create or edit submission rows for students outside their section.
Test: coursework_rls's two cross-section checks
RLS POLICY PROPOSED - require confirm before apply
Table: announcement_comments
Policy name: "authenticated_update_comments" (tautologies removed); trigger handle_announcement_comment_update
SQL: block 6
Plain English: Nobody can change a comment's id, announcement, author or created_at, so the 15-minute edit window can't be stretched. Only the author can change the text. CRs can still verify.
Test: "author cannot move created_at forward", "CR cannot rewrite a student's comment"
Evidence
todoon main (1bf8508) and failed as expected.After: see Database Policy Tests on this PR. No
todo()is left insupabase/tests/.Merge Danger
Door: two-way
Every change is a policy, a function body or a constraint that can be dropped. No data is rewritten.
Blast Radius: register-sync, section_teachers
attendance_recordstotals. Students may see their numbers move the first time a CR marks the register.NOT VALID, so existing bad links stay but no new ones can be made. Find existing ones with:select st.* from section_teachers st join subjects s on s.id = st.subject_id where s.section_id <> st.section_id;section_teacherscounsellor flag is removed can no longer edit their old remarks (they can still read and delete them).Not covered here:
src/hooks/useProfileQueries.ts:205andTeacherDashboardPage.tsx:2070insert intosection_teachersas a teacher, but only CRs have a write policy. Those inserts probably fail today. That's a separate fix.🤖 Generated with Claude Code