feat: add lint for detecting sensitive columns exposed via API #141
Conversation
This commit introduces a new lint that identifies tables exposed via the Supabase Data APIs containing columns with potentially sensitive data (e.g., passwords, SSNs, credit card numbers) without Row Level Security (RLS) enabled. It includes a detailed SQL view and documentation outlining the rationale, patterns detected, and recommended resolutions for mitigating security risks.
This commit introduces a new lint that identifies tables exposed via the Supabase Data APIs containing columns with potentially sensitive data (e.g., passwords, SSNs, credit card numbers) without Row Level Security (RLS) enabled. It includes a detailed SQL view and documentation outlining the rationale, patterns detected, and recommended resolutions for mitigating security risks.
|
this PR was based on https://github.com/bscript/superbase-exposure-check/tree/main work |
olirice
left a comment
olirice
left a comment
There was a problem hiding this comment.
Left a few change requests. Overall looks great. Thanks for the contribution and nice work on the test cases
| lower(a.attname) like '%' || sp.pattern || '%' | ||
| or lower(a.attname) = sp.pattern | ||
| -- Also check for common variations with underscores/hyphens removed | ||
| or replace(replace(lower(a.attname), '_', ''), '-', '') like '%' || replace(sp.pattern, '_', '') || '%' |
There was a problem hiding this comment.
wildcards are going to result in many false positives. Lets stick to
replace(lower(a.attname), '-', '_') = sp.pattern
since that will catch - and _ variants only
| @@ -0,0 +1,124 @@ | |||
| create view lint."0024_permissive_rls_policy" as | |||
|
|
|||
There was a problem hiding this comment.
lets rename this to "rls_policy_always_true" since permissive has a specific meaning in RLS
lints/0024_permissive_rls_policy.sql
Outdated
| case when ( | ||
| -- Literal true | ||
| lower(trim(coalesce(qual, ''))) = 'true' | ||
| -- 1=1 or similar tautologies | ||
| or lower(trim(coalesce(qual, ''))) ~ '^[\s\(]*1\s*=\s*1[\s\)]*$' | ||
| or lower(trim(coalesce(qual, ''))) ~ '^[\s\(]*''[^'']*''\s*=\s*''[^'']*''[\s\)]*$' | ||
| -- Empty or null qual on permissive policy means allow all for SELECT | ||
| or (qual is null and is_permissive and command in ('SELECT', 'ALL')) | ||
| ) then true else false end as has_permissive_using, | ||
| -- Check for always-true WITH CHECK clause patterns | ||
| case when ( | ||
| lower(trim(coalesce(with_check, ''))) = 'true' | ||
| or lower(trim(coalesce(with_check, ''))) ~ '^[\s\(]*1\s*=\s*1[\s\)]*$' | ||
| or lower(trim(coalesce(with_check, ''))) ~ '^[\s\(]*''[^'']*''\s*=\s*''[^'']*''[\s\)]*$' |
There was a problem hiding this comment.
the two most common variants are (true) and (1=1). Lets check for those specifically and get rid of the regex
| pa.polroles as role_oids, | ||
| (select array_agg(r::regrole::text) from unnest(pa.polroles) as x(r)) as roles, | ||
| case pa.polcmd | ||
| when 'r' then 'SELECT' |
There was a problem hiding this comment.
We should continue to allow (true) for SELECT. Its often intentional and even documented in a few places. In contrast, UPDATE/DELETE should never be wide open
| 'license_key', 'activation_key', | ||
| -- Biometric Data | ||
| 'fingerprint', 'biometric', 'facial_recognition' | ||
| ]) as pattern |
There was a problem hiding this comment.
pass, diagnosis, fingerprint, biometric, and facial_recognition don't seem high enough signal-to-noise compared with the other. lets remove those
|
@olirice thanks for the feedback this is fixed |
3 participants
Summary
Adds two new security lints inspired by external security scanning tools to detect common Supabase misconfigurations that can lead to data exposure.
New Lints
0023_sensitive_columns_exposed(ERROR)Detects tables exposed via the API that contain columns with potentially sensitive data (PII, credentials, financial info) without RLS protection.
Sensitive patterns detected:
password,secret,api_key,token,jwt,otp,2fa_secretssn,driver_license,passport_number,national_id,tax_idcredit_card,cvv,bank_account,iban,routing_numberhealth_record,diagnosis,patient_idssh_key,pgp_key,certificatefingerprint,biometric0024_permissive_rls_policy(WARN)Detects RLS policies that use overly permissive expressions, which effectively bypass row-level security while giving a false sense of protection.
Patterns detected:
USING (true)- allows reading all rowsWITH CHECK (true)- allows writing any rowUSING (1=1)or similar tautologiesKey features:
trueare less dangerous)anon,authenticated, or public rolesWhy This Matters
These are common security misconfigurations:
USING (true)policies during development and forget to update themBoth scenarios expose data while potentially giving a false sense of security.
Changes
lints/0023_sensitive_columns_exposed.sqllints/0024_permissive_rls_policy.sqlbin/installcheckandsplinter.sqlTest Results