Skip to content

fix rls trigger#41776

Merged
SaxonF merged 1 commit into
masterfrom
fix/rls-trigger
Jan 8, 2026
Merged

fix rls trigger#41776
SaxonF merged 1 commit into
masterfrom
fix/rls-trigger

Conversation

@SaxonF

@SaxonF SaxonF commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

Fixes trigger generated as part of policy banner to use correct template

Summary by CodeRabbit

  • Bug Fixes
    • Refined automatic RLS enablement to ensure proper event trigger setup for new tables.

✏️ Tip: You can customize this high-level summary in your review settings.

@SaxonF SaxonF requested a review from a team as a code owner January 8, 2026 00:29
@vercel

vercel Bot commented Jan 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
studio-self-hosted Ready Ready Preview, Comment Jan 8, 2026 0:34am
studio-staging Ready Ready Preview, Comment Jan 8, 2026 0:34am
6 Skipped Deployments
Project Deployment Review Updated (UTC)
cms Ignored Ignored Jan 8, 2026 0:34am
studio Ignored Ignored Jan 8, 2026 0:34am
design-system Skipped Skipped Jan 8, 2026 0:34am
docs Skipped Skipped Jan 8, 2026 0:34am
ui-library Skipped Skipped Jan 8, 2026 0:34am
zone-www-dot-com Skipped Skipped Jan 8, 2026 0:34am

@supabase

supabase Bot commented Jan 8, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project xguihxuzqibwxjnimxev because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

A new PostgreSQL event trigger SQL constant (AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL) is introduced for auto-enabling Row-Level Security on new tables. References to the default event trigger SQL are updated to use this new constant in the event trigger template and banner component.

Changes

Cohort / File(s) Summary
Event Trigger Constants
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
Introduced new exported constant AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL containing PostgreSQL trigger definition for auto-enabling RLS. Updated EVENT_TRIGGER_TEMPLATES[0].content to reference this new constant instead of DEFAULT_EVENT_TRIGGER_SQL.
RLS Event Trigger Banner
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
Replaced DEFAULT_EVENT_TRIGGER_SQL with AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL in event trigger creation path. Replaced EMPTY_ARR with plain [] for default event triggers data. Updated imports accordingly.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is minimal and does not follow the required template structure with sections for CONTRIBUTING.md confirmation, type of change, current/new behavior, and additional context. Fill out all required template sections including confirmation of reading CONTRIBUTING.md, change type, current behavior with issue links, new behavior with screenshots if applicable, and additional context.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix rls trigger' directly references the main change (fixing RLS trigger behavior), though it lacks specificity about the nature of the fix or the affected component.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)

16-51: LGTM! Consider simplifying the schema condition.

The SQL logic correctly implements auto-enabling RLS on new tables. Security best practices are followed with SECURITY DEFINER and search_path = pg_catalog.

However, line 32 contains redundant conditions. Since you're checking cmd.schema_name IN ('public'), the subsequent checks for system schemas are unnecessary:

♻️ Optional simplification
-     IF cmd.schema_name IS NOT NULL AND cmd.schema_name IN ('public') AND cmd.schema_name NOT IN ('pg_catalog','information_schema') AND cmd.schema_name NOT LIKE 'pg_toast%' AND cmd.schema_name NOT LIKE 'pg_temp%' THEN
+     IF cmd.schema_name IS NOT NULL AND cmd.schema_name IN ('public') THEN

If you plan to expand the enforced schema list beyond 'public', consider keeping the defensive checks but add a comment explaining the future-proofing intent.

apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (1)

37-37: Minor optimization: Consider using a constant empty array.

Changing from EMPTY_ARR to [] creates a new array instance on each render when data is undefined. While the loading guard at line 72 mitigates most practical impact, this removes a minor optimization.

♻️ Optional: Restore referential equality

If you want to maintain the optimization:

+const EMPTY_ARR: never[] = []
+
 export const BannerRlsEventTrigger = () => {
   // ...
-  const { data: eventTriggers = [], isLoading: isLoadingEventTriggers } =
+  const { data: eventTriggers = EMPTY_ARR, isLoading: isLoadingEventTriggers } =

This prevents the useMemo at line 43 from recalculating unnecessarily when the component renders before data loads.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a07512 and 785d63f.

📒 Files selected for processing (2)
  • apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
  • apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/studio/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)

apps/studio/**/*.{ts,tsx}: Assign complex conditions to descriptive variables instead of using multiple conditions in a single expression
Use consistent naming conventions for boolean variables: is prefix for state/identity, has prefix for possession, can prefix for capability/permission, should prefix for conditional behavior
Derive boolean state from existing state instead of storing it separately
Use early returns for guard clauses instead of deeply nested conditionals
Extract complex logic into custom hooks when logic becomes reusable or complex
Return objects from custom hooks instead of arrays for better extensibility and clearer API
Use discriminated unions for complex state management instead of multiple independent state fields
Avoid type casting (e.g., as any, as Type); instead validate values at runtime using zod schemas

Files:

  • apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
  • apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
apps/studio/**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)

apps/studio/**/*.tsx: Components should ideally be under 200-300 lines; break down large components with multiple distinct UI sections, complex conditional rendering, or multiple unrelated useState hooks
Extract repeated JSX patterns into reusable components instead of copying similar JSX blocks
Use consistent loading/error/success pattern: handle loading state first with early returns, then error state, then empty state, then render success state
Keep state as local as possible and only lift up when needed
Group related state using objects or reducers instead of multiple useState calls, preferring react-hook-form for form state management
Name event handlers consistently: use on prefix for prop callbacks and handle prefix for internal handlers
Avoid inline arrow functions for expensive operations; use useCallback to maintain stable function references
Use appropriate conditional rendering patterns: && for simple show/hide, ternary for binary choice, early returns or extracted components for multiple conditions
Avoid nested ternaries in JSX; use separate conditions or early returns instead
Use useMemo for expensive computations when the computation is genuinely expensive and the value is passed to memoized children
Define prop interfaces explicitly for React components with proper typing of props and callback handlers

Files:

  • apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-12T05:20:17.409Z
Learnt from: joshenlim
Repo: supabase/supabase PR: 41258
File: apps/studio/pages/project/[ref]/storage/vectors/buckets/[bucketId].tsx:9-9
Timestamp: 2025-12-12T05:20:17.409Z
Learning: In apps/studio/**/*.{ts,tsx}, use named imports for DefaultLayout: import { DefaultLayout } from 'components/layouts/DefaultLayout' instead of default import. This is the new practice being adopted across the studio app.

Applied to files:

  • apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
  • apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
📚 Learning: 2026-01-02T05:58:55.226Z
Learnt from: meGauravJain
Repo: supabase/supabase PR: 41658
File: apps/studio/components/interfaces/Connect/content/prisma/content.tsx:70-85
Timestamp: 2026-01-02T05:58:55.226Z
Learning: In Prisma configuration or related connection setup code, prefer the environment variable name DIRECT_URL for direct/non-pooled database connections instead of DATABASE_URL_UNPOOLED, per Prisma documentation. Update code that reads database connection strings to use DIRECT_URL where applicable, and document why this is preferred (explicit, documented behavior). Ensure your environment provides DIRECT_URL when using direct connections and fallback handling if needed.

Applied to files:

  • apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
  • apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
🧬 Code graph analysis (1)
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (1)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)
  • AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL (16-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: typecheck
  • GitHub Check: test (1)
  • GitHub Check: test
🔇 Additional comments (3)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)

57-57: Correct fix! The template now uses the appropriate SQL.

This change correctly replaces the generic placeholder template with the specific RLS auto-enable SQL, aligning with the PR objective.

apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (2)

7-7: Correct import for the RLS auto-enable SQL.

The import change aligns with the new constant defined in EventTriggers.constants.ts.


68-68: Correctly uses the RLS auto-enable SQL template.

This change fixes the core issue where the banner was creating a generic event trigger instead of the specific RLS auto-enable trigger.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 68.038%. remained the same
when pulling 785d63f on fix/rls-trigger
into 1a07512 on master.

@github-actions

github-actions Bot commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

🎭 Playwright Test Results

passed  66 passed
flaky  1 flaky
skipped  4 skipped

Details

stats  71 tests across 12 suites
duration  9 minutes, 22 seconds
commit  785d63f

Flaky tests

Features › table-editor.spec.ts › table editor › add enums and show enums on table

Skipped tests

Features › sql-editor.spec.ts › SQL Editor › snippet favourite works as expected
Features › sql-editor.spec.ts › SQL Editor › share with team works as expected
Features › sql-editor.spec.ts › SQL Editor › folders works as expected
Features › sql-editor.spec.ts › SQL Editor › other SQL snippets actions work as expected

@joshenlim joshenlim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verified changes on staging preview!

@SaxonF SaxonF merged commit 54a9683 into master Jan 8, 2026
26 checks passed
@SaxonF SaxonF deleted the fix/rls-trigger branch January 8, 2026 03:52
@github-actions

github-actions Bot commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

Braintrust eval report

Assistant (master-1767844677)

Score Average Improvements Regressions
Completeness 100% (+0pp) - -
Conciseness 0% (+0pp) - -
Goal Completion 90% (+10pp) 1 🟢 -
Tool Usage 60% (+0pp) - -
SQL Validity 100% (+0pp) - -
Time_to_first_token 0.22tok (+0.03tok) - 5 🔴
Llm_calls 8.4 (+1.2) 4 🟢 -
Tool_calls 4.6 (+1.8) 5 🟢 -
Errors 0 (+0) - -
Llm_errors 0 (+0) - -
Tool_errors 0 (+0) - -
Prompt_tokens 65358.4tok (+22217.6tok) 1 🟢 4 🔴
Prompt_cached_tokens 42547.2tok (+13209.6tok) 3 🟢 2 🔴
Prompt_cache_creation_tokens 0tok (+0tok) - -
Completion_tokens 6513.2tok (+2198tok) 1 🟢 4 🔴
Completion_reasoning_tokens 4736tok (+1689.6tok) 1 🟢 4 🔴
Completion_accepted_prediction_tokens 0tok (+0tok) - -
Completion_rejected_prediction_tokens 0tok (+0tok) - -
Completion_audio_tokens 0tok (+0tok) - -
Total_tokens 71871.6tok (+24415.6tok) - 5 🔴
Estimated_cost 0.02$ (+0.01$) 1 🟢 4 🔴
Duration 57.34s (+20.5s) 1 🟢 4 🔴
Llm_duration 113.94s (+40.91s) 1 🟢 4 🔴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants