mirror of
https://github.com/supabase/supabase.git
synced 2026-05-06 22:18:00 +08:00
[FE-3134] fix(studio): handle ALTER TABLE IF EXISTS in RLS detection (#45493)
The SQL Editor was warning about missing RLS even when the same query enabled it, if the user wrote `ALTER TABLE IF EXISTS ...`. The parser regex didn't recognise `IF EXISTS` and was capturing `IF` as the table name, so the RLS event never matched the `CREATE TABLE`. **Changed:** - `ALTER TABLE` regex in `sql-event-parser.ts` now accepts the optional `IF EXISTS` and `ONLY` modifiers, matching Postgres's `ALTER TABLE [ IF EXISTS ] [ ONLY ] name` grammar. **Added:** - Unit tests for `IF EXISTS`, `ONLY`, and both combined. - Regression test in `SQLEditor.utils.test.ts` using the customer's exact SQL. ## To test 1. Open the SQL Editor and paste: ```sql CREATE TABLE IF NOT EXISTS public."Conversations" (id int8 primary key); ALTER TABLE IF EXISTS public."Conversations" ENABLE ROW LEVEL SECURITY; ``` 2. Hit Run – the "table will not have RLS" warning should **not** appear. 3. Sanity check: a `CREATE TABLE` without any matching `ENABLE ROW LEVEL SECURITY` still triggers the warning. Addresses [FE-3134](https://linear.app/supabase/issue/FE-3134/sql-editor-warns-about-missing-rls-policy-incorrectly). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added comprehensive test coverage for Row Level Security detection across different SQL syntax patterns and clause combinations * **Bug Fixes** * Enhanced Row Level Security detection capabilities in the SQL editor by extending support for additional ALTER TABLE statement syntax variations, improving the accuracy and completeness of security configuration recognition <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
This commit is contained in:
@@ -414,6 +414,15 @@ describe('SQLEditor.utils:getCreateTablesMissingRLS', () => {
|
||||
expect(getCreateTablesMissingRLS(sql)).toEqual([])
|
||||
})
|
||||
|
||||
it('does not flag when ALTER TABLE IF EXISTS enables RLS', () => {
|
||||
const sql = stripIndent`
|
||||
CREATE TABLE IF NOT EXISTS public."Conversations" (id int8 primary key);
|
||||
ALTER TABLE IF EXISTS public."Conversations" ENABLE ROW LEVEL SECURITY;
|
||||
GRANT ALL ON TABLE public."Conversations" TO postgres, anon, authenticated, service_role;
|
||||
`
|
||||
expect(getCreateTablesMissingRLS(sql)).toEqual([])
|
||||
})
|
||||
|
||||
it('flags CREATE TEMP TABLE', () => {
|
||||
const result = getCreateTablesMissingRLS('create temp table foo (id int8 primary key);')
|
||||
expect(result).toHaveLength(1)
|
||||
|
||||
@@ -288,6 +288,42 @@ describe('SQL Event Parser', () => {
|
||||
const results = sqlEventParser.getTableEvents('ALTER TABLE users DISABLE ROW LEVEL SECURITY')
|
||||
expect(results).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('detects ALTER TABLE IF EXISTS ENABLE ROW LEVEL SECURITY', () => {
|
||||
const results = sqlEventParser.getTableEvents(
|
||||
'ALTER TABLE IF EXISTS public."Conversations" ENABLE ROW LEVEL SECURITY'
|
||||
)
|
||||
expect(results).toHaveLength(1)
|
||||
expect(results[0]).toEqual({
|
||||
type: TABLE_EVENT_ACTIONS.TableRLSEnabled,
|
||||
schema: 'public',
|
||||
tableName: 'Conversations',
|
||||
})
|
||||
})
|
||||
|
||||
it('detects ALTER TABLE ONLY ENABLE ROW LEVEL SECURITY', () => {
|
||||
const results = sqlEventParser.getTableEvents(
|
||||
'ALTER TABLE ONLY public.users ENABLE ROW LEVEL SECURITY'
|
||||
)
|
||||
expect(results).toHaveLength(1)
|
||||
expect(results[0]).toEqual({
|
||||
type: TABLE_EVENT_ACTIONS.TableRLSEnabled,
|
||||
schema: 'public',
|
||||
tableName: 'users',
|
||||
})
|
||||
})
|
||||
|
||||
it('detects ALTER TABLE IF EXISTS ONLY ENABLE ROW LEVEL SECURITY', () => {
|
||||
const results = sqlEventParser.getTableEvents(
|
||||
'ALTER TABLE IF EXISTS ONLY public.users ENABLE ROW LEVEL SECURITY'
|
||||
)
|
||||
expect(results).toHaveLength(1)
|
||||
expect(results[0]).toEqual({
|
||||
type: TABLE_EVENT_ACTIONS.TableRLSEnabled,
|
||||
schema: 'public',
|
||||
tableName: 'users',
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('ReDoS protection', () => {
|
||||
|
||||
@@ -39,8 +39,8 @@ export class SQLEventParser {
|
||||
{
|
||||
type: TABLE_EVENT_ACTIONS.TableRLSEnabled,
|
||||
patterns: [
|
||||
/ALTER\s+TABLE\s+(?<schema>(?:"[^"]+"|[\w]+)\.)?(?<table>(?:"(?:[^"]|"")+"|`(?:[^`]|``)+`|[\w]+)).*?ENABLE\s+ROW\s+LEVEL\s+SECURITY/i,
|
||||
/ALTER\s+TABLE\s+(?<schema>(?:"[^"]+"|[\w]+)\.)?(?<table>(?:"(?:[^"]|"")+"|`(?:[^`]|``)+`|[\w]+)).*?ENABLE\s+RLS/i,
|
||||
/ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?(?<schema>(?:"[^"]+"|[\w]+)\.)?(?<table>(?:"(?:[^"]|"")+"|`(?:[^`]|``)+`|[\w]+)).*?ENABLE\s+ROW\s+LEVEL\s+SECURITY/i,
|
||||
/ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?(?<schema>(?:"[^"]+"|[\w]+)\.)?(?<table>(?:"(?:[^"]|"")+"|`(?:[^`]|``)+`|[\w]+)).*?ENABLE\s+RLS/i,
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user