fix(supabase): require team membership for team-scoped writes and pin row ownership

This commit is contained in:
Yun Chan 2026-09-28 00:54:01 +09:00
parent f517eedffc
commit 72c4ddaefb
2 changed files with 435 additions and 0 deletions

View file

@ -0,0 +1,183 @@
-- Team-scoped write checks for meetings, meeting_documents and knowledge.
--
-- Two holes in the shipped policies:
--
-- 1) Publishing into a team without being a member of it.
-- meetings_insert and knowledge_documents_insert only checked user_id.
-- meetings_update (20260411000001) and knowledge_documents_update
-- (20260410000002) had no WITH CHECK, so Postgres reused USING, which also
-- ignores team_id. Anyone who knew a team uuid (a removed member, or an
-- invitee who never joined) could INSERT a row with team_id = T, or move
-- one of their own rows into T. meetings_read then showed it to every T
-- member, and match_knowledge_chunks fed the chunks into every T member's
-- RAG answers. remove_team_member only deletes the team_members row, so
-- removal did not close this route.
--
-- Now every write that leaves a row with team_id set requires the writer
-- to be a current member of that team. A removed member keeps full
-- control of their own rows (read, delete, and unsharing by setting
-- team_id back to NULL), but can no longer write content that the team
-- sees. knowledge_chunks_insert gets the same rule, so a removed member
-- cannot keep adding chunks to a document that is still shared.
--
-- 2) Team admins taking over a member's rows.
-- The admin branch of meetings_update and meeting_documents_update passed
-- USING, and with no WITH CHECK the rewritten row passed the owner branch
-- once the admin set user_id to their own id. meetings_delete and
-- meeting_documents_delete_own are owner-only, so the admin could then
-- delete the member's meeting (cascading transcripts, memos and documents)
-- and the sync tombstone went to the admin instead of the owner.
--
-- Ownership is now immutable for JWT callers: a BEFORE UPDATE trigger
-- rejects any change of user_id, and rejects a non-owner moving the row
-- to another team (meetings) or another meeting (meeting_documents).
-- Clearing team_id stays allowed so ON DELETE SET NULL from teams keeps
-- working. The WITH CHECK clauses also pin the owner branch: an owner can
-- only attach a document to a meeting they can see, the same rule as
-- meeting_documents_insert.
--
-- Service-role and internal jobs (no auth.uid()) are not affected by the
-- triggers, and SECURITY DEFINER RPCs never write team_id or user_id on these
-- tables (the portability import forces team_id NULL).
-- ── meetings ─────────────────────────────────────────────────────────────
DROP POLICY IF EXISTS "meetings_insert" ON public.meetings;
CREATE POLICY "meetings_insert" ON public.meetings
FOR INSERT WITH CHECK (
user_id = auth.uid()
AND (team_id IS NULL OR team_id IN (SELECT public.user_team_ids(auth.uid())))
);
DROP POLICY IF EXISTS "meetings_update" ON public.meetings;
CREATE POLICY "meetings_update" ON public.meetings
FOR UPDATE
USING (
user_id = auth.uid()
OR (team_id IS NOT NULL AND team_id IN (SELECT public.user_admin_team_ids(auth.uid())))
)
WITH CHECK (
(
user_id = auth.uid()
OR (team_id IS NOT NULL AND team_id IN (SELECT public.user_admin_team_ids(auth.uid())))
)
AND (team_id IS NULL OR team_id IN (SELECT public.user_team_ids(auth.uid())))
);
CREATE OR REPLACE FUNCTION public.enforce_meeting_ownership_v1()
RETURNS trigger
LANGUAGE plpgsql
SET search_path = ''
AS $$
DECLARE
actor_id uuid := auth.uid();
BEGIN
IF actor_id IS NULL THEN
RETURN NEW;
END IF;
IF NEW.user_id IS DISTINCT FROM OLD.user_id THEN
RAISE EXCEPTION 'meeting_owner_immutable' USING ERRCODE = '42501';
END IF;
IF NEW.team_id IS DISTINCT FROM OLD.team_id
AND NEW.team_id IS NOT NULL
AND OLD.user_id IS DISTINCT FROM actor_id THEN
RAISE EXCEPTION 'meeting_team_owner_only' USING ERRCODE = '42501';
END IF;
RETURN NEW;
END;
$$;
REVOKE ALL ON FUNCTION public.enforce_meeting_ownership_v1()
FROM PUBLIC, anon, authenticated;
DROP TRIGGER IF EXISTS meetings_ownership_v1 ON public.meetings;
CREATE TRIGGER meetings_ownership_v1
BEFORE UPDATE OF user_id, team_id
ON public.meetings
FOR EACH ROW EXECUTE FUNCTION public.enforce_meeting_ownership_v1();
-- ── meeting_documents ────────────────────────────────────────────────────
DROP POLICY IF EXISTS "meeting_documents_update" ON public.meeting_documents;
CREATE POLICY "meeting_documents_update" ON public.meeting_documents
FOR UPDATE
USING (
user_id = auth.uid()
OR meeting_id IN (
SELECT id FROM public.meetings
WHERE team_id IN (SELECT public.user_admin_team_ids(auth.uid()))
)
)
WITH CHECK (
(
user_id = auth.uid()
OR meeting_id IN (
SELECT id FROM public.meetings
WHERE team_id IN (SELECT public.user_admin_team_ids(auth.uid()))
)
)
AND meeting_id IN (
SELECT id FROM public.meetings
WHERE user_id = auth.uid()
OR (team_id IS NOT NULL AND team_id IN (SELECT public.user_team_ids(auth.uid())))
)
);
CREATE OR REPLACE FUNCTION public.enforce_meeting_document_ownership_v1()
RETURNS trigger
LANGUAGE plpgsql
SET search_path = ''
AS $$
DECLARE
actor_id uuid := auth.uid();
BEGIN
IF actor_id IS NULL THEN
RETURN NEW;
END IF;
IF NEW.user_id IS DISTINCT FROM OLD.user_id THEN
RAISE EXCEPTION 'meeting_document_owner_immutable' USING ERRCODE = '42501';
END IF;
IF NEW.meeting_id IS DISTINCT FROM OLD.meeting_id
AND OLD.user_id IS DISTINCT FROM actor_id THEN
RAISE EXCEPTION 'meeting_document_meeting_owner_only' USING ERRCODE = '42501';
END IF;
RETURN NEW;
END;
$$;
REVOKE ALL ON FUNCTION public.enforce_meeting_document_ownership_v1()
FROM PUBLIC, anon, authenticated;
DROP TRIGGER IF EXISTS meeting_documents_ownership_v1 ON public.meeting_documents;
CREATE TRIGGER meeting_documents_ownership_v1
BEFORE UPDATE OF user_id, meeting_id
ON public.meeting_documents
FOR EACH ROW EXECUTE FUNCTION public.enforce_meeting_document_ownership_v1();
-- ── knowledge_documents / knowledge_chunks ───────────────────────────────
DROP POLICY IF EXISTS "knowledge_documents_insert" ON public.knowledge_documents;
CREATE POLICY "knowledge_documents_insert" ON public.knowledge_documents
FOR INSERT WITH CHECK (
user_id = auth.uid()
AND (team_id IS NULL OR team_id IN (SELECT public.user_team_ids(auth.uid())))
);
DROP POLICY IF EXISTS "knowledge_documents_update" ON public.knowledge_documents;
CREATE POLICY "knowledge_documents_update" ON public.knowledge_documents
FOR UPDATE
USING (user_id = auth.uid())
WITH CHECK (
user_id = auth.uid()
AND (team_id IS NULL OR team_id IN (SELECT public.user_team_ids(auth.uid())))
);
DROP POLICY IF EXISTS "knowledge_chunks_insert" ON public.knowledge_chunks;
CREATE POLICY "knowledge_chunks_insert" ON public.knowledge_chunks
FOR INSERT WITH CHECK (
document_id IN (
SELECT id FROM public.knowledge_documents
WHERE user_id = auth.uid()
AND (team_id IS NULL OR team_id IN (SELECT public.user_team_ids(auth.uid())))
)
);

View file

@ -0,0 +1,252 @@
\set ON_ERROR_STOP on
-- Regression for 20260927000038_team_scoped_write_checks.sql.
-- 1) A non-member (or removed member) cannot publish meetings, knowledge
-- documents or knowledge chunks into a team.
-- 2) A team admin cannot take over a member's meeting or meeting document by
-- rewriting user_id, nor move it to another team or meeting.
BEGIN;
CREATE OR REPLACE FUNCTION pg_temp.assert_true(condition boolean, message text)
RETURNS void
LANGUAGE plpgsql
AS $$
BEGIN
IF condition IS NOT TRUE THEN
RAISE EXCEPTION 'assertion_failed: %', message;
END IF;
END;
$$;
CREATE OR REPLACE FUNCTION pg_temp.act_as(uid uuid)
RETURNS void
LANGUAGE sql
AS $$
SELECT set_config(
'request.jwt.claims',
json_build_object('sub', uid, 'role', 'authenticated')::text,
true
);
$$;
-- Must be rejected with 42501. expected_message, when set, pins the exact
-- trigger error so an RLS failure cannot mask a missing trigger (or the
-- reverse).
CREATE OR REPLACE FUNCTION pg_temp.assert_denied(stmt text, label text, expected_message text DEFAULT NULL)
RETURNS void
LANGUAGE plpgsql
AS $$
DECLARE
affected bigint;
BEGIN
EXECUTE stmt;
GET DIAGNOSTICS affected = ROW_COUNT;
RAISE EXCEPTION 'write_was_allowed: % (% rows)', label, affected;
EXCEPTION
WHEN insufficient_privilege THEN
IF expected_message IS NOT NULL AND SQLERRM <> expected_message THEN
RAISE EXCEPTION 'unexpected_denial: % got "%" expected "%"', label, SQLERRM, expected_message;
END IF;
END;
$$;
-- ── fixtures (as postgres) ───────────────────────────────────────────────
INSERT INTO auth.users (
id, aud, role, email, encrypted_password, email_confirmed_at,
raw_app_meta_data, raw_user_meta_data, created_at, updated_at
) VALUES
(
'38000000-0000-4000-8000-000000000001', 'authenticated', 'authenticated',
'scoped-admin@example.invalid', crypt('fixture-password', gen_salt('bf')), now(),
'{"provider":"email","providers":["email"]}'::jsonb, '{"name":"Admin"}'::jsonb, now(), now()
),
(
'38000000-0000-4000-8000-000000000002', 'authenticated', 'authenticated',
'scoped-member@example.invalid', crypt('fixture-password', gen_salt('bf')), now(),
'{"provider":"email","providers":["email"]}'::jsonb, '{"name":"Member"}'::jsonb, now(), now()
),
(
'38000000-0000-4000-8000-000000000003', 'authenticated', 'authenticated',
'scoped-removed@example.invalid', crypt('fixture-password', gen_salt('bf')), now(),
'{"provider":"email","providers":["email"]}'::jsonb, '{"name":"Removed"}'::jsonb, now(), now()
);
-- Team T (admin owns, member joined) and team T2 (admin only).
INSERT INTO public.teams (id, name, owner_id) VALUES
('38100000-0000-4000-8000-00000000000a', 'Scoped Team', '38000000-0000-4000-8000-000000000001'),
('38100000-0000-4000-8000-00000000000b', 'Other Team', '38000000-0000-4000-8000-000000000001');
INSERT INTO public.team_members (team_id, user_id, role) VALUES
('38100000-0000-4000-8000-00000000000a', '38000000-0000-4000-8000-000000000001', 'owner'),
('38100000-0000-4000-8000-00000000000a', '38000000-0000-4000-8000-000000000002', 'member'),
('38100000-0000-4000-8000-00000000000b', '38000000-0000-4000-8000-000000000001', 'owner');
-- The removed user was a member of T once; remove_team_member only deletes
-- the membership row, so rows they shared before stay shared.
INSERT INTO public.meetings (id, user_id, team_id, title) VALUES
('38200000-0000-4000-8000-000000000001', '38000000-0000-4000-8000-000000000002',
'38100000-0000-4000-8000-00000000000a', 'Member team meeting'),
('38200000-0000-4000-8000-000000000002', '38000000-0000-4000-8000-000000000003',
NULL, 'Removed personal meeting'),
('38200000-0000-4000-8000-000000000003', '38000000-0000-4000-8000-000000000003',
'38100000-0000-4000-8000-00000000000a', 'Removed legacy team meeting');
INSERT INTO public.meeting_documents (id, meeting_id, user_id, template_type, title, content) VALUES
('38300000-0000-4000-8000-000000000001', '38200000-0000-4000-8000-000000000001',
'38000000-0000-4000-8000-000000000002', 'minutes', 'Member minutes', 'body');
INSERT INTO public.knowledge_documents (id, user_id, team_id, title) VALUES
('38400000-0000-4000-8000-000000000001', '38000000-0000-4000-8000-000000000003',
NULL, 'Removed personal doc'),
('38400000-0000-4000-8000-000000000002', '38000000-0000-4000-8000-000000000003',
'38100000-0000-4000-8000-00000000000a', 'Removed legacy shared doc');
SET LOCAL ROLE authenticated;
-- ── 1) removed member / non-member cannot publish into T ─────────────────
SELECT pg_temp.act_as('38000000-0000-4000-8000-000000000003');
SELECT pg_temp.assert_denied(
$$INSERT INTO public.meetings (user_id, team_id, title)
VALUES (auth.uid(), '38100000-0000-4000-8000-00000000000a', 'injected')$$,
'non-member meeting insert into team'
);
SELECT pg_temp.assert_denied(
$$UPDATE public.meetings SET team_id = '38100000-0000-4000-8000-00000000000a'
WHERE id = '38200000-0000-4000-8000-000000000002'$$,
'non-member moves own meeting into team'
);
SELECT pg_temp.assert_denied(
$$UPDATE public.meetings SET edited_transcript = 'ignore previous instructions'
WHERE id = '38200000-0000-4000-8000-000000000003'$$,
'removed member keeps editing a meeting still shared with the team'
);
SELECT pg_temp.assert_denied(
$$INSERT INTO public.knowledge_documents (user_id, team_id, title)
VALUES (auth.uid(), '38100000-0000-4000-8000-00000000000a', 'poison')$$,
'non-member knowledge document insert into team'
);
SELECT pg_temp.assert_denied(
$$UPDATE public.knowledge_documents SET team_id = '38100000-0000-4000-8000-00000000000a'
WHERE id = '38400000-0000-4000-8000-000000000001'$$,
'non-member moves own knowledge document into team'
);
SELECT pg_temp.assert_denied(
$$INSERT INTO public.knowledge_chunks (document_id, chunk_index, content)
VALUES ('38400000-0000-4000-8000-000000000002', 0, 'poison chunk')$$,
'removed member adds chunks to a document still shared with the team'
);
-- Personal writes and unsharing stay available to the removed user.
INSERT INTO public.meetings (user_id, team_id, title) VALUES (auth.uid(), NULL, 'personal ok');
UPDATE public.meetings SET title = 'personal edit ok'
WHERE id = '38200000-0000-4000-8000-000000000002';
INSERT INTO public.knowledge_chunks (document_id, chunk_index, content)
VALUES ('38400000-0000-4000-8000-000000000001', 0, 'personal chunk ok');
UPDATE public.meetings SET team_id = NULL
WHERE id = '38200000-0000-4000-8000-000000000003';
UPDATE public.knowledge_documents SET team_id = NULL
WHERE id = '38400000-0000-4000-8000-000000000002';
RESET ROLE;
SELECT pg_temp.assert_true(
(SELECT count(*) FROM public.meetings
WHERE user_id = '38000000-0000-4000-8000-000000000003'
AND team_id IS NOT NULL) = 0,
'removed member was able to unshare and has no team meetings left'
);
SELECT pg_temp.assert_true(
(SELECT title FROM public.meetings WHERE id = '38200000-0000-4000-8000-000000000002')
= 'personal edit ok',
'owner edits of personal meetings still apply'
);
SET LOCAL ROLE authenticated;
-- A current member can still share into the team.
SELECT pg_temp.act_as('38000000-0000-4000-8000-000000000002');
INSERT INTO public.meetings (user_id, team_id, title)
VALUES (auth.uid(), '38100000-0000-4000-8000-00000000000a', 'member share ok');
INSERT INTO public.knowledge_documents (id, user_id, team_id, title)
VALUES ('38400000-0000-4000-8000-000000000003', auth.uid(),
'38100000-0000-4000-8000-00000000000a', 'member shared doc ok');
INSERT INTO public.knowledge_chunks (document_id, chunk_index, content)
VALUES ('38400000-0000-4000-8000-000000000003', 0, 'member chunk ok');
UPDATE public.meetings SET title = 'member edit ok'
WHERE id = '38200000-0000-4000-8000-000000000001';
-- A member cannot attach their document to a meeting they cannot see.
SELECT pg_temp.assert_denied(
$$UPDATE public.meeting_documents SET meeting_id = '38200000-0000-4000-8000-000000000002'
WHERE id = '38300000-0000-4000-8000-000000000001'$$,
'owner attaches document to a stranger''s private meeting'
);
-- ── 2) admin cannot take over a member's rows ────────────────────────────
SELECT pg_temp.act_as('38000000-0000-4000-8000-000000000001');
SELECT pg_temp.assert_denied(
$$UPDATE public.meetings SET user_id = auth.uid()
WHERE id = '38200000-0000-4000-8000-000000000001'$$,
'admin rewrites member meeting owner',
'meeting_owner_immutable'
);
SELECT pg_temp.assert_denied(
$$UPDATE public.meetings SET team_id = '38100000-0000-4000-8000-00000000000b'
WHERE id = '38200000-0000-4000-8000-000000000001'$$,
'admin moves member meeting to another team',
'meeting_team_owner_only'
);
SELECT pg_temp.assert_denied(
$$UPDATE public.meeting_documents SET user_id = auth.uid()
WHERE id = '38300000-0000-4000-8000-000000000001'$$,
'admin rewrites member document owner',
'meeting_document_owner_immutable'
);
-- Admin edit rights on content are unchanged.
UPDATE public.meetings SET title = 'admin edit ok'
WHERE id = '38200000-0000-4000-8000-000000000001';
UPDATE public.meeting_documents SET content = 'admin edit ok'
WHERE id = '38300000-0000-4000-8000-000000000001';
-- Owner-only delete still cannot reach the member's rows.
DELETE FROM public.meetings WHERE id = '38200000-0000-4000-8000-000000000001';
DELETE FROM public.meeting_documents WHERE id = '38300000-0000-4000-8000-000000000001';
RESET ROLE;
SELECT pg_temp.assert_true(
(SELECT user_id FROM public.meetings WHERE id = '38200000-0000-4000-8000-000000000001')
= '38000000-0000-4000-8000-000000000002'::uuid,
'member keeps ownership of the team meeting'
);
SELECT pg_temp.assert_true(
(SELECT title FROM public.meetings WHERE id = '38200000-0000-4000-8000-000000000001')
= 'admin edit ok',
'admin content edit applied'
);
SELECT pg_temp.assert_true(
(SELECT user_id FROM public.meeting_documents WHERE id = '38300000-0000-4000-8000-000000000001')
= '38000000-0000-4000-8000-000000000002'::uuid,
'member keeps ownership of the document'
);
-- ── team deletion still clears team_id through ON DELETE SET NULL ────────
-- The delete runs as postgres (the atomic team RPCs are SECURITY DEFINER) but
-- keeps the admin's JWT, so the ownership trigger sees a non-owner actor on
-- the cascaded UPDATE and must still let team_id go to NULL.
SELECT pg_temp.act_as('38000000-0000-4000-8000-000000000001');
DELETE FROM public.teams WHERE id = '38100000-0000-4000-8000-00000000000a';
SELECT pg_temp.assert_true(
(SELECT team_id FROM public.meetings WHERE id = '38200000-0000-4000-8000-000000000001') IS NULL,
'team delete cascades team_id to NULL on a member meeting'
);
SELECT pg_temp.assert_true(
(SELECT team_id FROM public.knowledge_documents WHERE id = '38400000-0000-4000-8000-000000000003') IS NULL,
'team delete cascades team_id to NULL on a member knowledge document'
);
ROLLBACK;