feat(studio): warn before CREATE TABLE without RLS in SQL editor#45008
Merged
Conversation
Detects CREATE TABLE statements in the SQL editor that don't have a matching ALTER TABLE ... ENABLE ROW LEVEL SECURITY in the same submission and prompts the user before running. Also extends sqlEventParser to support quoted identifiers containing spaces.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 8 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDetects CREATE TABLE statements missing RLS, shows a warning dialog listing affected tables, and offers running the query as-is or rewriting the SQL to append ALTER TABLE ... ENABLE ROW LEVEL SECURITY before executing. (Includes editor integration, parser/util updates, modal UI changes, tests, and an e2e test.) Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Editor as SQL Editor
participant Parser as RLS Utils / Parser
participant Modal as Warning Dialog
participant Executor as Query Executor
User->>Editor: Submit query
Editor->>Parser: getCreateTablesMissingRLS(sql)
Parser-->>Editor: missing-RLS tables
alt missing-RLS exists
Editor->>Modal: open with potentialIssues
Modal->>User: show missing tables & actions
alt User clicks "Run and enable RLS"
User->>Modal: confirm with RLS
Modal->>Editor: onConfirmWithRLS()
Editor->>Parser: appendEnableRLSStatements(sql, tables)
Parser-->>Editor: rewrittenSQL
Editor->>Executor: executeQuery(force=true, rewrittenSQL)
else User clicks "Run without RLS"
User->>Modal: confirm without RLS
Modal->>Editor: onConfirm()
Editor->>Executor: executeQuery(force=true, originalSQL)
else User cancels
User->>Modal: Close dialog
Modal->>Editor: onCancel()
end
else no missing-RLS
Editor->>Executor: executeQuery(force=false, originalSQL)
end
Executor-->>User: Return results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts`:
- Around line 102-119: The quote() helper in appendEnableRLSStatements
incorrectly uses a case-insensitive regex, so mixed-case identifiers like
MyTable (originally quoted) pass as unquoted and are emitted without quotes;
remove the /i flag (use /^[a-z_][a-z0-9_]*$/) or otherwise ensure only
all-lowercase unquoted identifiers pass, and re-quote any identifier that is not
strictly lowercase-identifier-safe before building target; update
appendEnableRLSStatements (and its quote function) accordingly and add a unit
test for CREATE TABLE "MyTable" to cover the regression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 67d46420-2c24-4592-b546-d38b94ed34de
📒 Files selected for processing (6)
apps/studio/components/interfaces/SQLEditor/RunQueryWarningModal.tsxapps/studio/components/interfaces/SQLEditor/SQLEditor.tsxapps/studio/components/interfaces/SQLEditor/SQLEditor.types.tsapps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.tsapps/studio/components/interfaces/SQLEditor/SQLEditor.utils.tsapps/studio/lib/sql-event-parser.ts
Contributor
🎭 Playwright Test Results
Details
Flaky testsFeatures › realtime-inspector.spec.ts › Realtime Inspector › Broadcast Messages › broadcast modal validates JSON payload Skipped testsFeatures › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled |
Reads the project's event triggers and suppresses the warning for CREATE TABLE statements in the public schema when the ensure_rls trigger (or any trigger using the rls_auto_enable function) is active. Tables in other schemas still trigger the warning since the trigger only enables RLS on public-schema tables.
The simple-identifier check in appendEnableRLSStatements used a case-insensitive regex, so mixed-case names like "MyTable" (whose quotes get stripped by sqlEventParser.cleanIdentifier) were emitted unquoted. Postgres then folded them to lowercase and the appended ALTER would target a table that doesn't exist. Drops the /i flag and adds regression tests for "MyTable" and "MySchema"."MyTable".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts (1)
133-134:⚠️ Potential issue | 🔴 CriticalFix quoting for mixed-case quoted identifiers before appending ALTER.
This unresolved issue still emits
MyTableunquoted for an originally quoted"MyTable", causing PostgreSQL to targetmytableinstead. Remove the case-insensitive match, or normalize unquoted identifiers in the parser before this point so only lowercase-safe names are emitted unquoted.🔧 Proposed fix
const quote = (identifier: string) => - /^[a-z_][a-z0-9_]*$/i.test(identifier) ? identifier : `"${identifier.replace(/"/g, '""')}"` + /^[a-z_][a-z0-9_]*$/.test(identifier) ? identifier : `"${identifier.replace(/"/g, '""')}"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts` around lines 133 - 134, The quote helper incorrectly treats mixed-case identifiers like "MyTable" as safe to emit unquoted because the regex uses the case-insensitive flag; update the logic in the quote function so it only leaves identifiers unquoted when they match the lowercase-safe SQL identifier pattern (remove the /i flag) or ensure identifiers are normalized to lowercase before calling quote; specifically modify the quote(identifier: string) implementation (and any call sites that append ALTER) so that mixed-case or originally-quoted names are always quoted via identifier.replace(/"/g,'""') wrapped in double-quotes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts`:
- Around line 143-146: The appended statement terminator can be swallowed when
the submitted SQL ends with a trailing line comment; update the logic that
builds the final string (using variables trimmed, separator, additions) so that
if trimmed does not already end with ';' but ends with a line comment (match
/--.*$/), you insert a standalone semicolon on its own line before adding the
comment block and additions; otherwise keep the existing behavior (use '\n\n' if
already ';' or ';\n\n' otherwise). This ensures the ';' is not inside a trailing
'--' comment and that subsequent ALTER/ENABLE RLS statements are parsed as
separate statements.
- Around line 114-123: The current logic lowercases schema/table keys when
building rlsEnabled and when checking for TableCreated events, which collapses
quoted PostgreSQL identifiers; update the keying to use a parser-normalized
identity that preserves quoted identifier case and only folds unquoted
identifiers (e.g., add a helper normalizeIdentifier(schema, tableName) and use
it when building the rlsEnabled Set and when checking .has and mapping results).
Locate the code that constructs rlsEnabled (filtering
TABLE_EVENT_ACTIONS.TableRLSEnabled) and the subsequent .filter/.map for
TABLE_EVENT_ACTIONS.TableCreated and replace the raw `${e.schema ??
''}.${e.tableName}`.toLowerCase() usage with calls to the new normalize function
so quoted names remain case-sensitive while unquoted names are folded.
---
Duplicate comments:
In `@apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts`:
- Around line 133-134: The quote helper incorrectly treats mixed-case
identifiers like "MyTable" as safe to emit unquoted because the regex uses the
case-insensitive flag; update the logic in the quote function so it only leaves
identifiers unquoted when they match the lowercase-safe SQL identifier pattern
(remove the /i flag) or ensure identifiers are normalized to lowercase before
calling quote; specifically modify the quote(identifier: string) implementation
(and any call sites that append ALTER) so that mixed-case or originally-quoted
names are always quoted via identifier.replace(/"/g,'""') wrapped in
double-quotes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f90ba822-3d81-4aac-9cf9-c65bb57e5d19
📒 Files selected for processing (3)
apps/studio/components/interfaces/SQLEditor/SQLEditor.tsxapps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.tsapps/studio/components/interfaces/SQLEditor/SQLEditor.utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts
Two fixes from PR review: - getCreateTablesMissingRLS no longer lowercases match keys, so quoted identifiers like "MyTable" and "mytable" (distinct tables in Postgres) don't collide and silently suppress the warning. - appendEnableRLSStatements puts the appended ';' on its own line when the SQL ends with a trailing line comment, so the terminator isn't swallowed and the following ALTER TABLE is parsed as a new statement.
ivasilov
approved these changes
Apr 18, 2026
Single Playwright test that types a CREATE TABLE statement, asserts the warning modal appears with the RLS bullet, clicks "Run and enable RLS", and verifies the table exists with relrowsecurity = true.
Contributor
Braintrust eval report
|
alaister
added a commit
that referenced
this pull request
Apr 20, 2026
…45050) Fixes a false positive in the CREATE-TABLE-without-RLS warning modal added in #45008. The warning was firing on `CREATE FUNCTION` statements because the `SELECT..INTO` detector was matching plpgsql variable assignments inside `$$…$$` function bodies. Reported example that triggered the modal with no table actually being created: ```sql create or replace function schema_checks() returns jsonb language plpgsql as $$ declare ret jsonb; begin select jsonb_build_object('value', 'ok') into ret; return ret; end; $$; ``` **Changed:** - `SQLEventParser.match()` now strips the body of `$tag$…$tag$` blocks before running detectors. Tags are kept as markers; content is blanked out so function bodies, DO blocks, and dollar-quoted string literals are never scanned as DDL. - Updated a pre-existing parser test that asserted the buggy behaviour (it expected `CREATE TABLE fake` inside a `$$…$$` string literal to be detected — `$$…$$` is a string literal in Postgres, not DDL). **Added:** - Regression tests in `SQLEditor.utils.test.ts` covering: the exact reported function, DO blocks with `select into`, `create table` text inside a function body, mixed top-level `CREATE TABLE` + function with `INTO` assignments, and custom `$body$…$body$` tags. - Parser-level regression test in `sql-event-parser.test.ts`. ## To test - In the SQL editor, paste the function from the Slack report and run it — the RLS warning modal should not appear. - Run `create table foo (id int8 primary key);` on its own — modal still appears as before. - Run `create table foo (id int8); create or replace function bar() returns int language plpgsql as $$ declare v int; begin select 1 into v; return v; end; $$;` — modal should flag only `foo`, not `v`. - Run an existing destructive query (`drop table x`) — unaffected, modal still works. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Parser no longer treats DDL/DML-like text inside PL/pgSQL functions, DO blocks, or dollar-quoted bodies (including nested/custom tags) as top-level CREATE TABLE/SELECT INTO, preventing false detections and UI warnings. * **Tests** * Added unit and e2e regression tests covering dollar-quoted blocks, nested dollar tags, DO blocks, SELECT INTO inside functions, and positive controls with a real top-level CREATE TABLE. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a pre-execution warning in the SQL editor when a
CREATE TABLEstatement is run without enabling Row Level Security on the new table. Responds to the press call-out around SQL editor security.Added:
executeQuerythat detectsCREATE TABLEstatements without a matchingALTER TABLE ... ENABLE ROW LEVEL SECURITYin the same submitted SQL.ALTER TABLE [schema.]<table> ENABLE ROW LEVEL SECURITY;for each detected table before running.Changed:
RunQueryWarningModalnow rendersDialogdirectly (instead ofConfirmationModal) so it can show three buttons: Cancel / Run without RLS / Run and enable RLS.sqlEventParsertable-name regex now supports quoted identifiers containing spaces (e.g."My Table") and escaped quotes (e.g."user""table").The check runs against the SQL that's actually submitted, so partial-selection works correctly — selecting only the
CREATE TABLEportion will trigger the warning even if there's a matchingENABLE RLSlower in the editor.To test
create table foo (id int8 primary key);→ modal should appear with the RLS warning bullet and three buttons.create table foo (id int8); alter table foo enable row level security;→ no modal (RLS already enabled in same submission).create table public.bar (id int8); create table baz (id int8); alter table baz enable rls;→ modal flags onlypublic.bar.create tableportion of a snippet that also enables RLS lower down and run the selection → modal should still fire.drop table x) → modal still works as before with two buttons (Cancel / Run this query).Summary by CodeRabbit
New Features
Tests