Skip to content

Commit a8f1670

Browse files
ralyodioclaude
ralyodio
and
claude
authored
fix(security): revoke anon RPC access, bind RPCs to auth.uid(), version RLS drift (#244)
* docs: archive the Noir0x63 security audit and its verification Adds the unsolicited third-party report received 2026-08-12 verbatim, plus the maintainer verification of it. The verification matters: the report's two Criticals are not equivalent. QRY-02 (anon EXECUTE on delete_encrypted_data_only, no auth.uid() bind) is confirmed live in prod and the blast radius is wider than reported -- 48 SECURITY DEFINER functions are anon-executable, not the 8 named. QRY-01 is real repo/prod drift but is NOT live exposure; prod is already patched, the policies were just never versioned. Documentation only. No migrations, no code, no prod changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): revoke anon RPC access, bind RPCs to auth.uid(), version RLS drift Steps 1-3 of the remediation for the 2026-08 audit. See docs/audits/2026-08-14-noir0x63-verification.md. 20260814000000 - revoke anon/PUBLIC EXECUTE on six SECURITY DEFINER functions and drop the dead is_otp_valid_extended. Revoking from PUBLIC alone is not enough: production ACLs carry an explicit anon=X grant, so anon is named in each REVOKE. delete_encrypted_data_only, fn_create_message_recipients and sync_user_with_auth are locked to service_role, which is what all three of their callers already use. 20260814000100 - bind get_inactive_participants, get_user_call_history and get_user_active_calls to the session rather than to their id argument. Bodies are taken from the live definitions, which had drifted from this repo; only the guard is new. 20260814000200 - drop the stale USING (true) SELECT policies on users and messages and create the two policies production actually runs, so a fresh deploy or db reset cannot resurrect the anon-readable ones. Not applied to production. No CI step applies migrations in this repo. Step 4 (narrowing users_select_authenticated away from phone_number / backup_pin_hash / salt, and rehashing PINs) is deliberately not included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0b19891 commit a8f1670

3 files changed

Lines changed: 293 additions & 0 deletions

‎supabase/migrations/20260814000000_revoke_anon_execute_on_security_definer_rpcs.sql‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
-- Step 1 of the 2026-08 security remediation.
2+
-- Context: docs/audits/2026-08-14-noir0x63-verification.md (QRY-02).
3+
--
4+
-- Every one of these functions is SECURITY DEFINER and was reachable by the
5+
-- `anon` role, i.e. by anyone holding the public anon key. Earlier migrations
6+
-- did `GRANT EXECUTE ... TO authenticated` but never revoked the default
7+
-- PUBLIC grant.
8+
--
9+
-- NOTE: revoking from PUBLIC alone is NOT sufficient here. Production ACLs show
10+
-- an *explicit* `anon=X/postgres` grant alongside the `=X/postgres` (PUBLIC)
11+
-- entry, so `anon` must be named explicitly in each REVOKE.
12+
--
13+
-- `service_role` holds its own explicit grant (`service_role=X/postgres`) and is
14+
-- unaffected by these revokes; it is re-granted below anyway for idempotence.
15+
16+
-- --------------------------------------------------------------------------
17+
-- Only ever invoked with the service-role client, so lock them to service_role.
18+
-- delete_encrypted_data_only -> src/app/api/user/nuclear-delete/route.js
19+
-- fn_create_message_recipients-> src/app/api/messages/send/route.js,
20+
-- src/app/api/chat/messages/route.js,
21+
-- src/lib/websocket/handlers/messages.js
22+
-- Ownership for the delete path is already enforced in the API route, which
23+
-- authenticates with getUser(), derives the internal id from auth_user_id, and
24+
-- requires an explicit DELETE_ALL_MY_DATA confirmation.
25+
-- --------------------------------------------------------------------------
26+
REVOKE EXECUTE ON FUNCTION public.delete_encrypted_data_only(uuid, uuid)
27+
FROM PUBLIC, anon, authenticated;
28+
GRANT EXECUTE ON FUNCTION public.delete_encrypted_data_only(uuid, uuid)
29+
TO service_role;
30+
31+
REVOKE EXECUTE ON FUNCTION public.fn_create_message_recipients(uuid, jsonb)
32+
FROM PUBLIC, anon, authenticated;
33+
GRANT EXECUTE ON FUNCTION public.fn_create_message_recipients(uuid, jsonb)
34+
TO service_role;
35+
36+
-- --------------------------------------------------------------------------
37+
-- No application callers anywhere in src/.
38+
-- --------------------------------------------------------------------------
39+
REVOKE EXECUTE ON FUNCTION public.sync_user_with_auth(text, uuid, text, text)
40+
FROM PUBLIC, anon, authenticated;
41+
GRANT EXECUTE ON FUNCTION public.sync_user_with_auth(text, uuid, text, text)
42+
TO service_role;
43+
44+
-- --------------------------------------------------------------------------
45+
-- These must remain callable by `authenticated`.
46+
-- get_inactive_participants is called from the websocket send path, whose
47+
-- client is the anon key plus an `Authorization: Bearer <access_token>`
48+
-- header, so PostgREST resolves it to the `authenticated` role.
49+
-- The two voice-call functions have no callers today but are kept available
50+
-- to `authenticated` for the client that is expected to use them.
51+
-- Per-row ownership is enforced in the next migration (20260814000100).
52+
-- --------------------------------------------------------------------------
53+
REVOKE EXECUTE ON FUNCTION public.get_inactive_participants(uuid)
54+
FROM PUBLIC, anon;
55+
GRANT EXECUTE ON FUNCTION public.get_inactive_participants(uuid)
56+
TO authenticated, service_role;
57+
58+
REVOKE EXECUTE ON FUNCTION public.get_user_call_history(uuid, integer)
59+
FROM PUBLIC, anon;
60+
GRANT EXECUTE ON FUNCTION public.get_user_call_history(uuid, integer)
61+
TO authenticated, service_role;
62+
63+
REVOKE EXECUTE ON FUNCTION public.get_user_active_calls(uuid)
64+
FROM PUBLIC, anon;
65+
GRANT EXECUTE ON FUNCTION public.get_user_active_calls(uuid)
66+
TO authenticated, service_role;
67+
68+
-- --------------------------------------------------------------------------
69+
-- Dead code. The body is literally `RETURN TRUE;` and there are zero callers
70+
-- in src/. It was a placeholder that never got implemented (QRY-07).
71+
-- --------------------------------------------------------------------------
72+
DROP FUNCTION IF EXISTS public.is_otp_valid_extended(text, text, integer);

‎supabase/migrations/20260814000100_bind_security_definer_rpcs_to_auth_uid.sql‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
-- Step 2 of the 2026-08 security remediation.
2+
-- Context: docs/audits/2026-08-14-noir0x63-verification.md (QRY-03, QRY-04, NEW-02).
3+
--
4+
-- These SECURITY DEFINER functions took a user/conversation id as a parameter
5+
-- and trusted it. Any caller who could execute them could read any other user's
6+
-- call history, watch their active calls in real time, or dump the phone
7+
-- numbers of an arbitrary conversation's inactive participants.
8+
--
9+
-- The guard added below binds the request to the session instead of to the
10+
-- argument. `auth.uid() IS NOT NULL` lets service_role (which has no JWT, and
11+
-- therefore a NULL auth.uid()) continue to call these; `anon` is revoked in
12+
-- 20260814000000, so the only other caller is `authenticated`, which always
13+
-- has a non-NULL auth.uid().
14+
--
15+
-- Bodies below are reproduced from the *live* production definitions, which had
16+
-- drifted from this repo (they carry `SET search_path TO ''` and schema-
17+
-- qualified references that the committed migrations do not). Only the guard is
18+
-- new; the queries are unchanged.
19+
20+
-- --------------------------------------------------------------------------
21+
CREATE OR REPLACE FUNCTION public.get_inactive_participants(conversation_uuid uuid)
22+
RETURNS TABLE(user_id uuid, phone_number text, display_name text, username text)
23+
LANGUAGE plpgsql
24+
SECURITY DEFINER
25+
SET search_path TO ''
26+
AS $function$
27+
BEGIN
28+
-- Caller must be an active participant of the conversation being queried.
29+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
30+
SELECT 1
31+
FROM public.conversation_participants cp
32+
JOIN public.users u ON u.id = cp.user_id
33+
WHERE cp.conversation_id = conversation_uuid
34+
AND u.auth_user_id = auth.uid()
35+
AND cp.left_at IS NULL
36+
) THEN
37+
RAISE EXCEPTION 'Not authorized for this conversation';
38+
END IF;
39+
40+
RETURN QUERY
41+
SELECT
42+
u.id,
43+
u.phone_number,
44+
u.display_name,
45+
u.username
46+
FROM public.users u
47+
INNER JOIN public.conversation_participants cp ON u.id = cp.user_id
48+
WHERE cp.conversation_id = conversation_uuid
49+
AND cp.left_at IS NULL
50+
AND public.is_user_inactive(u.id) = TRUE
51+
AND u.phone_number IS NOT NULL;
52+
END;
53+
$function$;
54+
55+
-- --------------------------------------------------------------------------
56+
CREATE OR REPLACE FUNCTION public.get_user_active_calls(p_user_id uuid)
57+
RETURNS TABLE(id text, call_type text, status text, started_at timestamp with time zone, is_caller boolean, other_user_id uuid, other_user_username text, other_user_display_name text, other_user_avatar_url text)
58+
LANGUAGE plpgsql
59+
SECURITY DEFINER
60+
SET search_path TO ''
61+
AS $function$
62+
BEGIN
63+
-- Caller may only read their own calls.
64+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
65+
SELECT 1 FROM public.users u
66+
WHERE u.id = p_user_id AND u.auth_user_id = auth.uid()
67+
) THEN
68+
RAISE EXCEPTION 'Not authorized';
69+
END IF;
70+
71+
RETURN QUERY
72+
SELECT
73+
vc.id,
74+
vc.call_type,
75+
vc.status,
76+
vc.started_at,
77+
(vc.caller_id = p_user_id) as is_caller,
78+
CASE
79+
WHEN vc.caller_id = p_user_id THEN vc.recipient_id
80+
ELSE vc.caller_id
81+
END as other_user_id,
82+
CASE
83+
WHEN vc.caller_id = p_user_id THEN recipient_user.username
84+
ELSE caller_user.username
85+
END as other_user_username,
86+
CASE
87+
WHEN vc.caller_id = p_user_id THEN recipient_user.display_name
88+
ELSE caller_user.display_name
89+
END as other_user_display_name,
90+
CASE
91+
WHEN vc.caller_id = p_user_id THEN recipient_user.avatar_url
92+
ELSE caller_user.avatar_url
93+
END as other_user_avatar_url
94+
FROM public.voice_calls vc
95+
LEFT JOIN public.users caller_user ON vc.caller_id = caller_user.id
96+
LEFT JOIN public.users recipient_user ON vc.recipient_id = recipient_user.id
97+
WHERE (vc.caller_id = p_user_id OR vc.recipient_id = p_user_id)
98+
AND vc.status IN ('ringing', 'connected')
99+
ORDER BY vc.started_at DESC;
100+
END;
101+
$function$;
102+
103+
-- --------------------------------------------------------------------------
104+
CREATE OR REPLACE FUNCTION public.get_user_call_history(p_user_id uuid, p_limit integer DEFAULT 50)
105+
RETURNS TABLE(id text, call_type text, status text, started_at timestamp with time zone, connected_at timestamp with time zone, ended_at timestamp with time zone, duration_seconds integer, is_caller boolean, other_user_id uuid, other_user_username text, other_user_display_name text, other_user_avatar_url text)
106+
LANGUAGE plpgsql
107+
SECURITY DEFINER
108+
SET search_path TO ''
109+
AS $function$
110+
BEGIN
111+
-- Caller may only read their own call history.
112+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
113+
SELECT 1 FROM public.users u
114+
WHERE u.id = p_user_id AND u.auth_user_id = auth.uid()
115+
) THEN
116+
RAISE EXCEPTION 'Not authorized';
117+
END IF;
118+
119+
RETURN QUERY
120+
SELECT
121+
vc.id,
122+
vc.call_type,
123+
vc.status,
124+
vc.started_at,
125+
vc.connected_at,
126+
vc.ended_at,
127+
vc.duration_seconds,
128+
(vc.caller_id = p_user_id) as is_caller,
129+
CASE
130+
WHEN vc.caller_id = p_user_id THEN vc.recipient_id
131+
ELSE vc.caller_id
132+
END as other_user_id,
133+
CASE
134+
WHEN vc.caller_id = p_user_id THEN recipient_user.username
135+
ELSE caller_user.username
136+
END as other_user_username,
137+
CASE
138+
WHEN vc.caller_id = p_user_id THEN recipient_user.display_name
139+
ELSE caller_user.display_name
140+
END as other_user_display_name,
141+
CASE
142+
WHEN vc.caller_id = p_user_id THEN recipient_user.avatar_url
143+
ELSE caller_user.avatar_url
144+
END as other_user_avatar_url
145+
FROM public.voice_calls vc
146+
LEFT JOIN public.users caller_user ON vc.caller_id = caller_user.id
147+
LEFT JOIN public.users recipient_user ON vc.recipient_id = recipient_user.id
148+
WHERE vc.caller_id = p_user_id OR vc.recipient_id = p_user_id
149+
ORDER BY vc.started_at DESC
150+
LIMIT p_limit;
151+
END;
152+
$function$;
153+
154+
-- CREATE OR REPLACE preserves the existing ACL, so these are redundant when
155+
-- 20260814000000 has already run. They are re-asserted anyway so this migration
156+
-- is self-sufficient if the two are ever applied out of order, and because
157+
-- REVOKE/GRANT are idempotent.
158+
REVOKE EXECUTE ON FUNCTION public.get_inactive_participants(uuid) FROM PUBLIC, anon;
159+
GRANT EXECUTE ON FUNCTION public.get_inactive_participants(uuid) TO authenticated, service_role;
160+
161+
REVOKE EXECUTE ON FUNCTION public.get_user_active_calls(uuid) FROM PUBLIC, anon;
162+
GRANT EXECUTE ON FUNCTION public.get_user_active_calls(uuid) TO authenticated, service_role;
163+
164+
REVOKE EXECUTE ON FUNCTION public.get_user_call_history(uuid, integer) FROM PUBLIC, anon;
165+
GRANT EXECUTE ON FUNCTION public.get_user_call_history(uuid, integer) TO authenticated, service_role;

‎supabase/migrations/20260814000200_version_production_rls_policies.sql‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
-- Step 3 of the 2026-08 security remediation.
2+
-- Context: docs/audits/2026-08-14-noir0x63-verification.md (QRY-01).
3+
--
4+
-- The July 2026 RLS fix was applied directly to production and never versioned.
5+
-- Production currently has `users_select_authenticated` and
6+
-- `messages_select_participant`; neither exists in any migration in this repo.
7+
-- Meanwhile the committed migrations still create anon-readable
8+
-- `USING (true)` SELECT policies on `users` and `messages`, so any fresh
9+
-- deploy, branch database, or `supabase db reset` reintroduces the original
10+
-- data leak.
11+
--
12+
-- This migration closes that gap in both directions: it drops the stale
13+
-- permissive policies by name, and creates the production policies. It is
14+
-- written to be a no-op against production and a real fix everywhere else.
15+
16+
-- --------------------------------------------------------------------------
17+
-- Drop the permissive SELECT policies created by earlier migrations.
18+
-- Each of these is `USING (true)` with no role restriction, i.e. readable by
19+
-- `anon`. Named individually rather than dropped wholesale so that this stays
20+
-- reviewable and does not silently remove a policy added later.
21+
-- --------------------------------------------------------------------------
22+
DROP POLICY IF EXISTS "Anyone can read user unique identifiers" ON public.users;
23+
DROP POLICY IF EXISTS "Anyone can view user profiles" ON public.users;
24+
DROP POLICY IF EXISTS "Public profiles are viewable by everyone" ON public.users;
25+
DROP POLICY IF EXISTS "Users can read basic user info for search" ON public.users;
26+
27+
DROP POLICY IF EXISTS "Users can read all messages" ON public.messages;
28+
29+
-- --------------------------------------------------------------------------
30+
-- Recreate the policies that production actually runs.
31+
-- --------------------------------------------------------------------------
32+
33+
-- WARNING: `USING (true)` here is scoped to the `authenticated` role, not to
34+
-- `anon` -- but it still lets any single logged-in account read every row of
35+
-- `users`, which includes `phone_number`, `backup_pin_hash` and `salt`.
36+
-- That is Step 4 of the remediation (narrow this to non-sensitive columns and
37+
-- rehash PINs with Argon2id + the per-user salt) and is deliberately NOT
38+
-- changed here, so that this migration reproduces production exactly.
39+
-- See §2 of the verification document.
40+
DROP POLICY IF EXISTS users_select_authenticated ON public.users;
41+
CREATE POLICY users_select_authenticated ON public.users
42+
FOR SELECT TO authenticated
43+
USING (true);
44+
45+
DROP POLICY IF EXISTS messages_select_participant ON public.messages;
46+
CREATE POLICY messages_select_participant ON public.messages
47+
FOR SELECT TO authenticated
48+
USING (
49+
EXISTS (
50+
SELECT 1
51+
FROM public.conversation_participants cp
52+
JOIN public.users u ON u.id = cp.user_id
53+
WHERE cp.conversation_id = messages.conversation_id
54+
AND u.auth_user_id = auth.uid()
55+
)
56+
);

0 commit comments

Comments
 (0)