Skip to content

feat(0024): flag SELECT using (true) when the policy name claims owner or role scope #195

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
ohad6k wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(0024): flag SELECT using (true) when the policy name claims owner or role scope #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 46 additions & 2 deletions docs/0024_permissive_rls_policy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ The lint identifies policies with these always-true patterns:
**USING Clause (controls which rows can be read):**
- `USING (true)` - explicitly allows reading all rows
- `USING (1=1)` - tautology that always evaluates to true
- `USING ('a'='a')` - string comparison tautology
- Missing USING clause on permissive SELECT policies

`SELECT` policies with `USING (true)` are not flagged, because public read access is often intentional. The exception is a `SELECT` policy whose name says it is limited to the row owner or a role (see below).

**WITH CHECK Clause (controls which rows can be written):**
- `WITH CHECK (true)` - allows writing any row
Expand All @@ -41,6 +41,44 @@ When a permissive policy with `USING (true)` exists:

This is particularly dangerous when the policy applies to `anon` or `authenticated` roles, as it exposes data to all API users.

### Policy Names That Claim Ownership

A policy name is what people read when they review RLS. Postgres only enforces the expression. A policy like this looks correct in a list of policies, but returns every row to every signed in user:

```sql
create policy "Users can view their own posts"
on public.posts
for select
to authenticated
using (true);
```

A policy name counts as claiming scope when it contains one of these (matched as whole words, case-insensitive, with `_`, `-` and other punctuation treated as spaces):

- `own` or `their`, e.g. "Users can view their own posts", "select_own_posts"
- `only owner`, `only owners`, `only the owner`, `only the owners`
- `for owner`, `for owners`, `for the owner`, `for the owners`
- `owner can`, `owners can`, `owner only`, `owners only`
- `admin` or `admins` followed by `can`, `only`, `read`, `reads`, `view`, `views`, `see`, `sees`, `manage`, `manages`, `select`, `insert`, `update` or `delete`, e.g. "Admin reads all activity", "admin_select_responses"
- `only admin`, `only admins`
- `for admin` or `for admins` at the very end of the name, e.g. "Allow authenticated read for admin" (but not "... for admin reports")
- `service role`

A bare `owner` or `admin` is not enough, because it often names the table's contents ("Allow reading admin users", "Public can view vehicle owners"). A name that contains `public`, `everyone` or `anyone` never counts as claiming scope.

These are English-name heuristics, so some names that do claim scope are not matched:

- a name that also says `public`, `everyone` or `anyone`, e.g. "Anyone can read own payments by phone", or one where `public` is part of a table name, e.g. "Users can view own public_profiles"
- camelCase names, e.g. "usersViewOwnPosts"
- names in other languages

For a policy name that claims scope, the lint:

- flags a `SELECT` policy with an always-true `USING` clause, which it would otherwise skip
- adds a note to the detail message of any flagged policy where nothing in the policy checks the caller

A policy for the `service_role` is not needed at all, because the `service_role` bypasses RLS. A policy named after it applies to the roles in its `to` clause, and to every role when `to` is left out.

### How to Resolve

**Option 1: Add proper row-level conditions**
Expand Down Expand Up @@ -132,3 +170,9 @@ In some cases, `USING (true)` may be intentional:
- Tables where access is controlled by other means (e.g., API layer)

If the policy is intentional, you can document why in a comment or consider suppressing this lint for specific tables.

If a `SELECT` policy is flagged only because of its name and the table really is public, rename the policy so the name matches what it does:

```sql
alter policy "Users can view their own posts" on public.posts rename to "Posts are viewable by everyone";
```
58 changes: 48 additions & 10 deletions lints/0024_rls_policy_always_true.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,28 @@ with policies as (
end as command,
pb.qual,
pb.with_check,
-- Normalize expressions by removing whitespace and lowercasing
replace(replace(replace(lower(coalesce(pb.qual, '')), ' ', ''), E'\n', ''), E'\t', '') as normalized_qual,
replace(replace(replace(lower(coalesce(pb.with_check, '')), ' ', ''), E'\n', ''), E'\t', '') as normalized_with_check
-- Normalize expressions by removing whitespace and lowercasing, then check for always-true forms
replace(replace(replace(lower(coalesce(pb.qual, '')), ' ', ''), E'\n', ''), E'\t', '')
in ('true', '(true)', '1=1', '(1=1)') as qual_always_true,
replace(replace(replace(lower(coalesce(pb.with_check, '')), ' ', ''), E'\n', ''), E'\t', '')
in ('true', '(true)', '1=1', '(1=1)') as with_check_always_true,
-- Policy name claims the policy is scoped to the row owner or a role. Bare words are not enough:
-- in "Allow reading admin users" or "Public can view vehicle owners" the word describes the
-- table's contents. Names that declare public, everyone or anyone access never count.
(
policy_name_words.words ~ any(array[
' (own|their) ', -- "Users can view their own posts", "select_own_posts"
' only (the )?owners? ', -- "Only owners can read", "Readable by only the owner"
' for (the )?owners? ', -- "Read access for the owner"
' owners? (can|only) ', -- "Owners can view posts", "Owner only read"
' admins? (can|only|read|reads|view|views|see|sees|manage|manages|select|insert|update|delete) ',
-- "Admins can view orders", "Admin reads all activity", "admin_select_responses"
' only admins? ', -- "Visible to only admins"
' for admins? $', -- "Allow authenticated read for admin", but not "... for admin reports"
' service role ' -- "Service role can read logs"
])
and policy_name_words.words !~ ' (public|everyone|anyone) '
) as name_implies_scope
from
pg_catalog.pg_policy pa
join pg_catalog.pg_class pc
Expand All @@ -33,6 +52,11 @@ with policies as (
on pc.relname = pb.tablename
and nsp.nspname = pb.schemaname
and pa.polname = pb.policyname
-- Lowercased policy name with every non-alphanumeric run turned into one space and a space at each end,
-- so the patterns above match whole words: "select_own_posts" matches " own ", "shown" does not
cross join lateral (
select ' ' || regexp_replace(lower(pa.polname), '[^a-z0-9]+', ' ', 'g') || ' ' as words
) as policy_name_words
where
pc.relkind = 'r' -- regular tables
and nsp.nspname not in (
Expand All @@ -43,23 +67,28 @@ permissive_patterns as (
select
p.*,
-- Check for always-true USING clause patterns
-- Note: SELECT with (true) is often intentional and documented, so we only flag UPDATE/DELETE
-- Note: SELECT with (true) is often intentional and documented, so we only flag UPDATE/DELETE,
-- and SELECT only when the policy name says it is scoped to the row owner or a role
case when (
command in ('UPDATE', 'DELETE', 'ALL')
and (
normalized_qual in ('true', '(true)', '1=1', '(1=1)')
qual_always_true
-- Empty or null qual on permissive policy means allow all
or (qual is null and is_permissive)
)
) or (
command = 'SELECT'
and name_implies_scope
and qual_always_true
) then true else false end as has_permissive_using,
-- Check for always-true WITH CHECK clause patterns
case when (
normalized_with_check in ('true', '(true)', '1=1', '(1=1)')
with_check_always_true
-- Empty with_check on INSERT means allow all (INSERT has no USING to fall back on)
or (with_check is null and is_permissive and command = 'INSERT')
-- Empty with_check on UPDATE/ALL with permissive USING means allow all writes
or (with_check is null and is_permissive and command in ('UPDATE', 'ALL')
and normalized_qual in ('true', '(true)', '1=1', '(1=1)'))
and qual_always_true)
) then true else false end as has_permissive_with_check
from
policies p
Expand All @@ -84,9 +113,9 @@ select
'WARN' as level,
'EXTERNAL' as facing,
array['SECURITY'] as categories,
'Detects RLS policies that use overly permissive expressions like \`USING (true)\` or \`WITH CHECK (true)\` for UPDATE, DELETE, or INSERT operations. SELECT policies with \`USING (true)\` are intentionally excluded as this pattern is often used deliberately for public read access.' as description,
'Detects RLS policies that use overly permissive expressions like \`USING (true)\` or \`WITH CHECK (true)\` for UPDATE, DELETE, or INSERT operations. SELECT policies with \`USING (true)\` are excluded as this pattern is often used deliberately for public read access, unless the policy name says it is limited to the row owner or a role.' as description,
format(
'Table `%s.%s` has an RLS policy `%s` for `%s` that allows unrestricted access%s. This effectively bypasses row-level security for %s.',
'Table `%s.%s` has an RLS policy `%s` for `%s` that allows unrestricted access%s. This effectively bypasses row-level security for %s.%s',
schema_name,
table_name,
policy_name,
Expand All @@ -97,7 +126,16 @@ select
when has_permissive_with_check then ' (WITH CHECK clause is always true)'
else ''
end,
array_to_string(roles, ', ')
array_to_string(roles, ', '),
-- Only when nothing in the policy checks the caller: every expression it has is always true.
-- A missing USING only counts for INSERT, which has none.
case
when name_implies_scope
and (qual_always_true or (qual is null and command = 'INSERT'))
and (with_check is null or with_check_always_true)
then ' The policy name suggests access is limited to the row owner or a specific role, but the policy does not check who the caller is.'
else ''
end
) as detail,
'https://supabase.com/docs/guides/database/database-linter?lint=0024_permissive_rls_policy' as remediation,
jsonb_build_object(
Expand Down
58 changes: 48 additions & 10 deletions splinter.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,9 +1314,28 @@ with policies as (
end as command,
pb.qual,
pb.with_check,
-- Normalize expressions by removing whitespace and lowercasing
replace(replace(replace(lower(coalesce(pb.qual, '')), ' ', ''), E'\n', ''), E'\t', '') as normalized_qual,
replace(replace(replace(lower(coalesce(pb.with_check, '')), ' ', ''), E'\n', ''), E'\t', '') as normalized_with_check
-- Normalize expressions by removing whitespace and lowercasing, then check for always-true forms
replace(replace(replace(lower(coalesce(pb.qual, '')), ' ', ''), E'\n', ''), E'\t', '')
in ('true', '(true)', '1=1', '(1=1)') as qual_always_true,
replace(replace(replace(lower(coalesce(pb.with_check, '')), ' ', ''), E'\n', ''), E'\t', '')
in ('true', '(true)', '1=1', '(1=1)') as with_check_always_true,
-- Policy name claims the policy is scoped to the row owner or a role. Bare words are not enough:
-- in "Allow reading admin users" or "Public can view vehicle owners" the word describes the
-- table's contents. Names that declare public, everyone or anyone access never count.
(
policy_name_words.words ~ any(array[
' (own|their) ', -- "Users can view their own posts", "select_own_posts"
' only (the )?owners? ', -- "Only owners can read", "Readable by only the owner"
' for (the )?owners? ', -- "Read access for the owner"
' owners? (can|only) ', -- "Owners can view posts", "Owner only read"
' admins? (can|only|read|reads|view|views|see|sees|manage|manages|select|insert|update|delete) ',
-- "Admins can view orders", "Admin reads all activity", "admin_select_responses"
' only admins? ', -- "Visible to only admins"
' for admins? $', -- "Allow authenticated read for admin", but not "... for admin reports"
' service role ' -- "Service role can read logs"
])
and policy_name_words.words !~ ' (public|everyone|anyone) '
) as name_implies_scope
from
pg_catalog.pg_policy pa
join pg_catalog.pg_class pc
Expand All @@ -1327,6 +1346,11 @@ with policies as (
on pc.relname = pb.tablename
and nsp.nspname = pb.schemaname
and pa.polname = pb.policyname
-- Lowercased policy name with every non-alphanumeric run turned into one space and a space at each end,
-- so the patterns above match whole words: "select_own_posts" matches " own ", "shown" does not
cross join lateral (
select ' ' || regexp_replace(lower(pa.polname), '[^a-z0-9]+', ' ', 'g') || ' ' as words
) as policy_name_words
where
pc.relkind = 'r' -- regular tables
and nsp.nspname not in (
Expand All @@ -1337,23 +1361,28 @@ permissive_patterns as (
select
p.*,
-- Check for always-true USING clause patterns
-- Note: SELECT with (true) is often intentional and documented, so we only flag UPDATE/DELETE
-- Note: SELECT with (true) is often intentional and documented, so we only flag UPDATE/DELETE,
-- and SELECT only when the policy name says it is scoped to the row owner or a role
case when (
command in ('UPDATE', 'DELETE', 'ALL')
and (
normalized_qual in ('true', '(true)', '1=1', '(1=1)')
qual_always_true
-- Empty or null qual on permissive policy means allow all
or (qual is null and is_permissive)
)
) or (
command = 'SELECT'
and name_implies_scope
and qual_always_true
) then true else false end as has_permissive_using,
-- Check for always-true WITH CHECK clause patterns
case when (
normalized_with_check in ('true', '(true)', '1=1', '(1=1)')
with_check_always_true
-- Empty with_check on INSERT means allow all (INSERT has no USING to fall back on)
or (with_check is null and is_permissive and command = 'INSERT')
-- Empty with_check on UPDATE/ALL with permissive USING means allow all writes
or (with_check is null and is_permissive and command in ('UPDATE', 'ALL')
and normalized_qual in ('true', '(true)', '1=1', '(1=1)'))
and qual_always_true)
) then true else false end as has_permissive_with_check
from
policies p
Expand All @@ -1378,9 +1407,9 @@ select
'WARN' as level,
'EXTERNAL' as facing,
array['SECURITY'] as categories,
'Detects RLS policies that use overly permissive expressions like \`USING (true)\` or \`WITH CHECK (true)\` for UPDATE, DELETE, or INSERT operations. SELECT policies with \`USING (true)\` are intentionally excluded as this pattern is often used deliberately for public read access.' as description,
'Detects RLS policies that use overly permissive expressions like \`USING (true)\` or \`WITH CHECK (true)\` for UPDATE, DELETE, or INSERT operations. SELECT policies with \`USING (true)\` are excluded as this pattern is often used deliberately for public read access, unless the policy name says it is limited to the row owner or a role.' as description,
format(
'Table `%s.%s` has an RLS policy `%s` for `%s` that allows unrestricted access%s. This effectively bypasses row-level security for %s.',
'Table `%s.%s` has an RLS policy `%s` for `%s` that allows unrestricted access%s. This effectively bypasses row-level security for %s.%s',
schema_name,
table_name,
policy_name,
Expand All @@ -1391,7 +1420,16 @@ select
when has_permissive_with_check then ' (WITH CHECK clause is always true)'
else ''
end,
array_to_string(roles, ', ')
array_to_string(roles, ', '),
-- Only when nothing in the policy checks the caller: every expression it has is always true.
-- A missing USING only counts for INSERT, which has none.
case
when name_implies_scope
and (qual_always_true or (qual is null and command = 'INSERT'))
and (with_check is null or with_check_always_true)
then ' The policy name suggests access is limited to the row owner or a specific role, but the policy does not check who the caller is.'
else ''
end
) as detail,
'https://supabase.com/docs/guides/database/database-linter?lint=0024_permissive_rls_policy' as remediation,
jsonb_build_object(
Expand Down
Loading