Skip to content

fix(rls): fail fast anon REST scans #1595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
riderx wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(rls): fail fast anon REST scans #1595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44276c6
fix(rls): fail fast anon REST scans
riderx Feb 7, 2026
3733201
fix(rls): harden fail-fast policies
riderx Feb 7, 2026
a3cf893
Merge branch 'main' into riderx/rest-timeout-audit
riderx Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
287 changes: 287 additions & 0 deletions supabase/migrations/20260206123000_fail_fast_postgrest_rls.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,287 @@
-- Fail fast for unauthenticated PostgREST queries and avoid per-row API key resolution in RLS.
--
-- Goal:
-- - Unauthenticated anon requests (no capgkey header, auth.uid() is NULL) must not scan large tables.
-- - Policies should be index-friendly for common predicates (e.g. app_id IN allowed_app_ids()).
--
-- Context:
-- PostgREST requests with the public anon key can hit RLS policies. If the policy evaluates expensive
-- identity/auth logic per row (e.g. get_identity_org_appid(owner_org, app_id)), an unfiltered query can
-- trigger statement_timeouts and cause cascading failures.

-- 1) Statement-scoped guard: true only when the request is authenticated OR carries a valid Capgo API key.
CREATE OR REPLACE FUNCTION "public"."has_auth_or_valid_apikey"("keymode" "public"."key_mode"[])
RETURNS boolean
LANGUAGE "plpgsql" STABLE SECURITY DEFINER
SET "search_path" TO ''
AS $$
DECLARE
v_user_id uuid;
v_api_key_text text;
v_api_key record;
BEGIN
SELECT auth.uid() INTO v_user_id;
IF v_user_id IS NOT NULL THEN
RETURN true;
END IF;

SELECT public.get_apikey_header() INTO v_api_key_text;
IF v_api_key_text IS NULL THEN
RETURN false;
END IF;

SELECT * FROM public.find_apikey_by_value(v_api_key_text) INTO v_api_key;
IF v_api_key.id IS NULL THEN
RETURN false;
END IF;

IF NOT (v_api_key.mode = ANY(keymode)) THEN
RETURN false;
END IF;

IF public.is_apikey_expired(v_api_key.expires_at) THEN
RETURN false;
END IF;

RETURN true;
END;
$$;

-- NOTE: GRANT/REVOKE must specify only argument types (not parameter names).
GRANT EXECUTE ON FUNCTION "public"."has_auth_or_valid_apikey"("public"."key_mode"[]) TO "anon";
GRANT EXECUTE ON FUNCTION "public"."has_auth_or_valid_apikey"("public"."key_mode"[]) TO "authenticated";

-- 2) Compute readable app_ids once per statement, then let policies use a simple index predicate:
-- app_id = ANY(allowed_read_apps()).
CREATE OR REPLACE FUNCTION "public"."allowed_read_apps"()
RETURNS text[]
LANGUAGE "plpgsql" STABLE SECURITY DEFINER
SET "search_path" TO ''
AS $$
DECLARE
v_user_id uuid;
v_api_key_text text;
v_api_key public.apikeys%ROWTYPE;
v_allowed text[] := '{}'::text[];
v_app record;
v_use_rbac boolean;
v_perm text := public.rbac_permission_for_legacy(
'read'::public.user_min_right,
public.rbac_scope_app()
);
v_enforcing_2fa boolean;
BEGIN
SELECT auth.uid() INTO v_user_id;

-- IMPORTANT: If the request is already authenticated, ignore any provided API key.
-- This preserves the historical behavior where a stray `capgkey` header does not
-- reduce the user's access compared to a normal authenticated request.
IF v_user_id IS NULL THEN
SELECT public.get_apikey_header() INTO v_api_key_text;
IF v_api_key_text IS NOT NULL THEN
SELECT * FROM public.find_apikey_by_value(v_api_key_text) INTO v_api_key;
IF v_api_key.id IS NOT NULL
AND v_api_key.mode = ANY('{read,upload,write,all}'::public.key_mode[])
AND NOT public.is_apikey_expired(v_api_key.expires_at)
THEN
v_user_id := v_api_key.user_id;
ELSE
-- Treat invalid/mismatched/expired keys as absent (fail closed).
v_api_key := NULL;
END IF;
END IF;
END IF;

-- No auth and no usable API key.
IF v_user_id IS NULL AND v_api_key.id IS NULL THEN
RETURN v_allowed;
END IF;

-- Candidate apps come from:
-- - legacy org_users bindings (org-wide or app-wide, but not channel bindings)
-- - RBAC org/app bindings (user principal or apikey principal)
FOR v_app IN
SELECT DISTINCT a.app_id, a.owner_org
FROM public.apps a
WHERE
-- Legacy org membership / app access.
EXISTS (
SELECT 1
FROM public.org_users ou
WHERE ou.user_id = v_user_id
AND ou.org_id = a.owner_org
AND ou.channel_id IS NULL
AND (ou.app_id IS NULL OR ou.app_id = a.app_id)
)
OR
-- RBAC: org-level bindings (implies possible access across apps via inheritance).
EXISTS (
SELECT 1
FROM public.role_bindings rb
WHERE rb.scope_type = public.rbac_scope_org()
AND rb.org_id = a.owner_org
AND (
(rb.principal_type = public.rbac_principal_user() AND rb.principal_id = v_user_id)
OR
(v_api_key.rbac_id IS NOT NULL AND rb.principal_type = public.rbac_principal_apikey() AND rb.principal_id = v_api_key.rbac_id)
)
)
OR
-- RBAC: app-level bindings (apps.id is the RBAC scope identifier).
EXISTS (
SELECT 1
FROM public.role_bindings rb
WHERE rb.scope_type = public.rbac_scope_app()
AND rb.app_id = a.id
AND (
(rb.principal_type = public.rbac_principal_user() AND rb.principal_id = v_user_id)
OR
(v_api_key.rbac_id IS NOT NULL AND rb.principal_type = public.rbac_principal_apikey() AND rb.principal_id = v_api_key.rbac_id)
)
)
LOOP
-- Enforce API key scoping (if present).
IF v_api_key.id IS NOT NULL
AND COALESCE(array_length(v_api_key.limited_to_orgs, 1), 0) > 0
AND NOT (v_app.owner_org = ANY(v_api_key.limited_to_orgs))
THEN
Comment on lines +143 to +147
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip API-key scoping when user is authenticated

In allowed_read_apps, API key scoping is applied whenever a valid capgkey header is present, even if auth.uid() is already set (the function always loads the key and then applies the limited_to_orgs/apps filters). This is a behavior change from the previous get_identity_org_appid-based policies, which ignored API keys once a user session was present. As a result, an authenticated user who sends a limited API key header (e.g., a client that always includes capgkey) can lose access to apps they otherwise can read. Consider only applying API key scoping when the request is unauthenticated or when you explicitly want key-scoped access to override user rights.

Useful? React with 👍 / 👎.

CONTINUE;
END IF;

IF v_api_key.id IS NOT NULL
AND v_api_key.limited_to_apps IS DISTINCT FROM '{}'
AND NOT (v_app.app_id = ANY(v_api_key.limited_to_apps))
THEN
CONTINUE;
END IF;

v_use_rbac := public.rbac_is_enabled_for_org(v_app.owner_org);

IF NOT v_use_rbac THEN
-- Legacy rights (includes org 2FA + password policy checks).
IF public.check_min_rights_legacy(
'read'::public.user_min_right,
v_user_id,
v_app.owner_org,
v_app.app_id,
NULL::bigint
) THEN
v_allowed := array_append(v_allowed, v_app.app_id);
END IF;
ELSE
-- Mirror check_min_rights() org gating for RBAC orgs (2FA + password policy).
SELECT o.enforcing_2fa INTO v_enforcing_2fa
FROM public.orgs o
WHERE o.id = v_app.owner_org;

IF v_enforcing_2fa = true AND (v_user_id IS NULL OR NOT public.has_2fa_enabled(v_user_id)) THEN
CONTINUE;
END IF;

IF NOT public.user_meets_password_policy(v_user_id, v_app.owner_org) THEN
CONTINUE;
END IF;

-- Allow if the user or the API key principal has the required RBAC permission.
IF v_user_id IS NOT NULL
AND public.rbac_has_permission(
public.rbac_principal_user(),
v_user_id,
v_perm,
v_app.owner_org,
v_app.app_id,
NULL::bigint
)
THEN
v_allowed := array_append(v_allowed, v_app.app_id);
ELSIF v_api_key.id IS NOT NULL
AND v_api_key.rbac_id IS NOT NULL
AND public.rbac_has_permission(
public.rbac_principal_apikey(),
v_api_key.rbac_id,
v_perm,
v_app.owner_org,
v_app.app_id,
NULL::bigint
)
THEN
v_allowed := array_append(v_allowed, v_app.app_id);
END IF;
END IF;
END LOOP;

RETURN v_allowed;
END;
$$;

GRANT EXECUTE ON FUNCTION "public"."allowed_read_apps"() TO "anon";
GRANT EXECUTE ON FUNCTION "public"."allowed_read_apps"() TO "authenticated";

-- 3) Apply fail-fast + index-friendly policies on the largest affected tables.

-- audit_logs: keep org_id predicate but add a one-time guard so unauthenticated anon requests do not scan.
DROP POLICY IF EXISTS "Allow select for auth, api keys (super_admin+)" ON "public"."audit_logs";
CREATE POLICY "Allow select for auth, api keys (super_admin+)" ON "public"."audit_logs"
FOR SELECT TO "anon", "authenticated"
USING (
"org_id" = ANY("public"."audit_logs_allowed_orgs"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

-- app_versions + app_versions_meta: avoid per-row identity resolution; use allowed_read_apps().
DROP POLICY IF EXISTS "Allow for auth, api keys (read+)" ON "public"."app_versions";
CREATE POLICY "Allow for auth, api keys (read+)" ON "public"."app_versions"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

DROP POLICY IF EXISTS "Allow read for auth (read+)" ON "public"."app_versions_meta";
CREATE POLICY "Allow read for auth (read+)" ON "public"."app_versions_meta"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

-- 4) (Optional hardening) Replace common read policies to avoid per-row get_identity_org_appid() on large tables.
-- NOTE: This intentionally deviates from the "use get_identity_org_appid() in every policy" convention
-- for large tables, because per-row identity resolution is the root cause of the PostgREST DoS vector.
-- The statement-scoped helpers keep the same security checks, but compute them once per statement.
-- apps
DROP POLICY IF EXISTS "Allow for auth, api keys (read+)" ON "public"."apps";
CREATE POLICY "Allow for auth, api keys (read+)" ON "public"."apps"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

-- channels
DROP POLICY IF EXISTS "Allow select for auth, api keys (read+)" ON "public"."channels";
CREATE POLICY "Allow select for auth, api keys (read+)" ON "public"."channels"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

-- channel_devices
DROP POLICY IF EXISTS "Allow read for auth, api keys (read+)" ON "public"."channel_devices";
DROP POLICY IF EXISTS "Allow read for auth (read+)" ON "public"."channel_devices";
CREATE POLICY "Allow read for auth, api keys (read+)" ON "public"."channel_devices"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);

-- build_requests
DROP POLICY IF EXISTS "Allow org members to select build_requests" ON "public"."build_requests";
CREATE POLICY "Allow org members to select build_requests" ON "public"."build_requests"
FOR SELECT TO "anon", "authenticated"
USING (
"app_id" = ANY("public"."allowed_read_apps"())
AND public.has_auth_or_valid_apikey('{read,upload,write,all}'::public.key_mode[])
);
Comment on lines 220 to 287
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for app_id column definitions in channel_devices and build_requests tables
# Look through migrations to find table definitions
find supabase/migrations -name "*.sql" -type f | head -20

Repository: Cap-go/capgo

Length of output: 1251

🏁 Script executed:

#!/bin/bash
# Search for channel_devices table definition
rg -A 20 'CREATE TABLE.*channel_devices' supabase/migrations/

Repository: Cap-go/capgo

Length of output: 1646

🏁 Script executed:

#!/bin/bash
# Search for build_requests table definition
rg -A 20 'CREATE TABLE.*build_requests' supabase/migrations/

Repository: Cap-go/capgo

Length of output: 2354

🏁 Script executed:

#!/bin/bash
# Search for allowed_read_apps function definition
rg -B 5 -A 15 'allowed_read_apps' supabase/migrations/ --type sql | head -50

Repository: Cap-go/capgo

Length of output: 5113

🏁 Script executed:

#!/bin/bash
# Search for allowed_read_apps function in schema definitions
rg 'allowed_read_apps' supabase/

Repository: Cap-go/capgo

Length of output: 1834

🏁 Script executed:

#!/bin/bash
# Get the full allowed_read_apps function definition
rg -A 60 'CREATE OR REPLACE FUNCTION.*allowed_read_apps' supabase/migrations/20260206123000_fail_fast_postgrest_rls.sql

Repository: Cap-go/capgo

Length of output: 2000

🏁 Script executed:

#!/bin/bash
# Check if there's any reference to get_identity_org_appid in the migration
rg 'get_identity_org_appid' supabase/migrations/20260206123000_fail_fast_postgrest_rls.sql

Repository: Cap-go/capgo

Length of output: 275

🏁 Script executed:

#!/bin/bash
# Check what the original policies looked like before this migration
rg 'get_identity_org_appid' supabase/migrations/ --type sql | head -20

Repository: Cap-go/capgo

Length of output: 2682

🏁 Script executed:

#!/bin/bash
# Get the full allowed_read_apps function to see complete implementation
sed -n '/CREATE OR REPLACE FUNCTION.*allowed_read_apps/,/^$$;/p' supabase/migrations/20260206123000_fail_fast_postgrest_rls.sql

Repository: Cap-go/capgo

Length of output: 5093

RLS policies deviate from mandatory guidelines; get_identity_org_appid() must be used when app_id exists on the table.

The policies correctly identify that both channel_devices and build_requests have app_id columns, and they filter on app_id = ANY(allowed_read_apps()). However, this approach contradicts the mandatory coding guideline: "ALWAYS use get_identity_org_appid() when app_id exists on the table; use get_identity_org_allowed() only as an absolute last resort."

While allowed_read_apps() is a well-implemented optimization that handles auth, API keys, RBAC, and 2FA validation, it shifts from the mandated per-row identity validation pattern to a statement-scoped approach. This deviates from the explicit "ALWAYS" requirement in the guidelines, regardless of performance rationale.

Either:

  1. Refactor policies to use get_identity_org_appid(app_id) in the USING clause to comply with mandatory guidelines, or
  2. Obtain explicit exception approval if this optimization is deemed critical for the fail-fast goal.
🤖 Prompt for AI Agents 
In `@supabase/migrations/20260206123000_fail_fast_postgrest_rls.sql` around lines
217 - 281, The RLS policies on channel_devices and build_requests use
allowed_read_apps() but must follow the mandatory guideline to call
get_identity_org_appid() when an app_id column exists; update the USING clauses
in the CREATE POLICY statements for "Allow read for auth, api keys (read+)" on
"channel_devices" and "Allow org members to select build_requests" on
"build_requests" to call public.get_identity_org_appid(app_id) (or an equivalent
wrapper) instead of app_id = ANY(public.allowed_read_apps()), ensuring
auth/APikey checks via public.has_auth_or_valid_apikey() remain; if this
optimization cannot be changed, obtain explicit exception approval and document
it.

Loading