Skip to content

Commit f0552cd

Browse files
ralyodioclaude
andauthored
fix(security): revoke anon EXECUTE on the remaining 41 SECURITY DEFINER fns (#247)
Completes the sweep started in 20260814030009, which closed only the six functions the audit named. 41 more were still executable by anyone holding the public anon key. After this, zero SECURITY DEFINER functions in `public` are reachable by anon (verified: 47 total, 0 anon, 17 authenticated, service_role intact). Classified by tracing every .rpc() call site in src/ and identifying the client each uses. 27 have no caller at all or a provably service-role caller and are locked to service_role -- including nuclear_delete_user_data and fn_get_user_message_content, which read/destroy user data and were anon-reachable. 14 have live callers that may run as authenticated, so anon is revoked and authenticated retained. Applied as a loop over pg_proc so overloads are covered and missing names are skipped rather than erroring. Already applied to production as 20260814032259. Nine Group B functions still lack an auth.uid() binding and remain IDOR-able by authenticated users; listed in a trailing comment. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 74ba143 commit f0552cd

1 file changed

Lines changed: 125 additions & 0 deletions

File tree

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
-- Sweep of every remaining anon-executable SECURITY DEFINER function.
2+
-- Context: docs/audits/2026-08-14-noir0x63-verification.md
3+
--
4+
-- 20260814030009 closed the six functions the audit named. This closes the
5+
-- rest of the class: 41 further SECURITY DEFINER functions in `public` were
6+
-- still executable by `anon`, i.e. by anyone holding the public anon key.
7+
-- After this migration, zero SECURITY DEFINER functions in `public` are
8+
-- reachable by `anon`.
9+
--
10+
-- Classification was made by tracing every `.rpc('<name>')` call site in src/
11+
-- and identifying which Supabase client each one uses:
12+
--
13+
-- Group A - no application caller at all, or a caller that provably uses the
14+
-- service-role client. Locked to service_role.
15+
-- Group B - has a live caller that may run as `authenticated` (browser client
16+
-- with a session, or the websocket client, which is the anon key
17+
-- plus an Authorization bearer and therefore resolves to
18+
-- `authenticated`). anon revoked, authenticated retained.
19+
--
20+
-- Both groups are applied via a loop over pg_proc rather than as literal
21+
-- statements, so overloads are covered and a name that does not exist (or is
22+
-- not SECURITY DEFINER) is skipped instead of erroring.
23+
--
24+
-- NOTE: revoking from PUBLIC alone is not sufficient; these carry an explicit
25+
-- `anon=X/postgres` grant, so `anon` is named in every REVOKE.
26+
--
27+
-- Trigger functions are included. PostgreSQL checks EXECUTE on a trigger
28+
-- function at CREATE TRIGGER time, not when the trigger fires, so revoking
29+
-- here does not affect trigger execution.
30+
31+
-- --------------------------------------------------------------------------
32+
-- Group A: locked to service_role.
33+
-- --------------------------------------------------------------------------
34+
DO $$
35+
DECLARE
36+
fn text;
37+
sig text;
38+
names text[] := ARRAY[
39+
'backfill_all_message_status','calculate_message_expiration',
40+
'check_message_status_coverage','check_sms_rate_limit',
41+
'cleanup_expired_messages','cleanup_sms_audit_logs','cleanup_sms_rate_limits',
42+
'fix_canonical_auth_user_ids','fix_missing_conversation_participants',
43+
'fn_create_message_status_entries','fn_get_user_message_content',
44+
'fn_messages_ready_for_gc','get_conversations_with_archive_data',
45+
'get_conversations_with_archive_support','get_sms_stats',
46+
'get_storage_bucket_limits','get_user_archived_conversations',
47+
'get_user_call_sessions','is_user_inactive','log_otp_verification_attempt',
48+
'log_sms_event','nuclear_delete_user_data','set_user_offline',
49+
'trigger_create_message_status','update_canonical_auth_user_ids',
50+
'update_message_has_files','bump_autoblog_integration'
51+
];
52+
BEGIN
53+
FOREACH fn IN ARRAY names LOOP
54+
FOR sig IN
55+
SELECT format('public.%I(%s)', p.proname, pg_get_function_identity_arguments(p.oid))
56+
FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace
57+
WHERE n.nspname = 'public' AND p.proname = fn AND p.prosecdef
58+
LOOP
59+
EXECUTE format('REVOKE EXECUTE ON FUNCTION %s FROM PUBLIC, anon, authenticated', sig);
60+
EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO service_role', sig);
61+
END LOOP;
62+
END LOOP;
63+
END $$;
64+
65+
-- --------------------------------------------------------------------------
66+
-- Group B: anon revoked, authenticated + service_role retained.
67+
--
68+
-- Call sites:
69+
-- archive_conversation / unarchive_conversation -> api/chat/conversations,
70+
-- api/conversations/{archive,unarchive}
71+
-- create_call_session / establish_call_session -> websocket/handlers/ml-kem-voice-calls
72+
-- find_user_by_unique_identifier -> api/users/by-id/[identifier]
73+
-- fn_cleanup_expired_messages -> lib/services/message-cleanup-service
74+
-- fn_create_deliveries_for_message -> api/chat/messages
75+
-- fn_get_user_active_deliveries / fn_mark_read -> lib/realtime/deliveries
76+
-- fn_mark_message_read -> api/chat/messages
77+
-- log_sms_notification -> lib/services/sms-notification-service
78+
-- update_user_activity -> websocket/handlers/messages,
79+
-- api/messages/{send,load}
80+
-- get_user_public_key / upsert_user_public_key -> api/crypto/public-keys,
81+
-- api/keys/reset (both service-role today, but kept available to
82+
-- `authenticated` because breaking key exchange is the worst failure
83+
-- mode here; upsert_user_public_key already enforces auth.uid()
84+
-- internally, and get_user_public_key returns public keys by design)
85+
-- --------------------------------------------------------------------------
86+
DO $$
87+
DECLARE
88+
fn text;
89+
sig text;
90+
names text[] := ARRAY[
91+
'archive_conversation','unarchive_conversation',
92+
'create_call_session','establish_call_session',
93+
'find_user_by_unique_identifier','fn_cleanup_expired_messages',
94+
'fn_create_deliveries_for_message','fn_get_user_active_deliveries',
95+
'fn_mark_message_read','fn_mark_read','log_sms_notification',
96+
'update_user_activity','get_user_public_key','upsert_user_public_key'
97+
];
98+
BEGIN
99+
FOREACH fn IN ARRAY names LOOP
100+
FOR sig IN
101+
SELECT format('public.%I(%s)', p.proname, pg_get_function_identity_arguments(p.oid))
102+
FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace
103+
WHERE n.nspname = 'public' AND p.proname = fn AND p.prosecdef
104+
LOOP
105+
EXECUTE format('REVOKE EXECUTE ON FUNCTION %s FROM PUBLIC, anon', sig);
106+
EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO authenticated, service_role', sig);
107+
END LOOP;
108+
END LOOP;
109+
END $$;
110+
111+
-- --------------------------------------------------------------------------
112+
-- STILL OPEN after this migration (deliberately not fixed here):
113+
--
114+
-- Of the Group B functions, these take a user/conversation id as a parameter
115+
-- and do NOT check it against auth.uid(), so an authenticated user can still
116+
-- pass someone else's id:
117+
-- archive_conversation, unarchive_conversation, find_user_by_unique_identifier,
118+
-- fn_cleanup_expired_messages, fn_create_deliveries_for_message,
119+
-- fn_get_user_active_deliveries, fn_mark_message_read, update_user_activity,
120+
-- log_sms_notification
121+
-- Closing those needs the same auth.uid() binding applied in 20260814030037,
122+
-- which means rewriting nine bodies and re-testing each call path.
123+
-- (create_call_session, establish_call_session, fn_mark_read and
124+
-- upsert_user_public_key already bind to auth.uid().)
125+
-- --------------------------------------------------------------------------

0 commit comments

Comments
 (0)