Project create security section updates#42021
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
SaxonF
changed the title
WalkthroughReplaces Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (browser)
participant UI as Project Creation Page
participant API as Project Creation API
participant DB as Database
User->>UI: fill & submit form (dataApi, enableRlsEventTrigger, ...)
UI->>API: POST /createProject { ..., dataApi, enableRlsEventTrigger, dataApiUseApiSchema:false, dbSql? }
alt enableRlsEventTrigger == true
API->>DB: execute AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL
DB-->>API: success
end
API-->>UI: project created response
UI-->>User: show success / next steps
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/studio/components/interfaces/ProjectCreation/SecurityOptions.tsx`:
- Around line 46-52: The external Link in SecurityOptions.tsx using
target="_blank" (the Link with href
"https://supabase.com/docs/reference/javascript/introduction" and className
"text-link") is missing a rel attribute; add rel="noopener noreferrer" to that
Link element to prevent reverse-tabnabbing and ensure safe external navigation.
- Around line 85-90: The Admonition currently reads form.getValues('dataApi'),
which does not trigger re-renders; replace that usage by subscribing to the form
field with useWatch_Shadcn_ so the warning is reactive: import and call
useWatch_Shadcn_ to watch the 'dataApi' field (e.g. const dataApi =
useWatch_Shadcn_({ control: form.control, name: 'dataApi' })), then use that
dataApi boolean in the conditional that renders the Admonition instead of
form.getValues('dataApi'); ensure the import for useWatch_Shadcn_ is added if
missing and remove the stale getValues() usage.
| <Link | ||
| href="https://supabase.com/docs/reference/javascript/introduction" | ||
| target="_blank" | ||
| className="text-link" | ||
| > | ||
| supabase-js | ||
| </Link> |
There was a problem hiding this comment.
Add rel on external target="_blank" link.
Prevents reverse‑tabnabbing when opening external docs.
🔐 Proposed fix
<Link
href="https://supabase.com/docs/reference/javascript/introduction"
target="_blank"
+ rel="noreferrer noopener"
className="text-link"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link | |
| href="https://supabase.com/docs/reference/javascript/introduction" | |
| target="_blank" | |
| className="text-link" | |
| > | |
| supabase-js | |
| </Link> | |
| <Link | |
| href="https://supabase.com/docs/reference/javascript/introduction" | |
| target="_blank" | |
| rel="noreferrer noopener" | |
| className="text-link" | |
| > | |
| supabase-js | |
| </Link> |
🤖 Prompt for AI Agents
In `@apps/studio/components/interfaces/ProjectCreation/SecurityOptions.tsx` around
lines 46 - 52, The external Link in SecurityOptions.tsx using target="_blank"
(the Link with href
"https://supabase.com/docs/reference/javascript/introduction" and className
"text-link") is missing a rel attribute; add rel="noopener noreferrer" to that
Link element to prevent reverse-tabnabbing and ensure safe external navigation.
🎭 Playwright Test Results
Details
Flaky testsFeatures › sql-editor.spec.ts › SQL Editor › should check if SQL editor is working as expected Skipped testsFeatures › sql-editor.spec.ts › SQL Editor › snippet favourite works as expected |
| postgresEngine: useOrioleDb ? availableOrioleVersion?.postgres_engine : postgresEngine, | ||
| releaseChannel: useOrioleDb ? availableOrioleVersion?.release_channel : releaseChannel, | ||
| ...(smartRegionEnabled ? { regionSelection: selectedRegion } : { dbRegion }), | ||
| ...(enableRlsEventTrigger ? { dbSql: AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL } : {}), |
There was a problem hiding this comment.
Note for future: we should only use this for testing purposes because it prevents warm project pool transfer, which increases project startup time for users
There was a problem hiding this comment.
There is a platform PR to fix this
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/studio/pages/new/`[slug].tsx:
- Around line 230-235: The current match allows undefined emails to compare
equal when profile hasn't loaded; update the logic around userPrimaryEmail and
isUserAtFreeProjectLimit so comparisons only run when userPrimaryEmail is
defined: ensure userPrimaryEmail is checked truthy before using it in
membersExceededLimit.some and/or filter members to only those with a truthy
primary_email (use member.primary_email) in isUserAtFreeProjectLimit, and make
shouldShowFreeProjectInfo depend on the presence of userPrimaryEmail as well;
reference the variables userPrimaryEmail, isUserAtFreeProjectLimit,
membersExceededLimit and profile to locate and change the comparison guard.
♻️ Duplicate comments (2)
apps/studio/components/interfaces/ProjectCreation/SecurityOptions.tsx (2)
51-56: Addrelon externaltarget="_blank"link.
Missingrel="noreferrer noopener"exposes reverse‑tabnabbing risk.🔐 Proposed fix
<Link href="https://supabase.com/docs/reference/javascript/introduction" target="_blank" + rel="noreferrer noopener" className="text-link" >
92-96: Make the Data API warning reactive to checkbox state.
getValues()won’t trigger re-renders, so the Admonition can remain stale.🔧 Proposed fix
import { Checkbox_Shadcn_, FormControl_Shadcn_, FormDescription_Shadcn_, FormField_Shadcn_, FormItem_Shadcn_, FormLabel_Shadcn_, + useWatch_Shadcn_, } from 'ui' @@ export const SecurityOptions = ({ form, layout = 'horizontal' }: SecurityOptionsProps) => { const shouldShowEnableRlsEventTrigger = usePHFlag<string>( 'projectCreationEnableRlsEventTrigger' ) + const isDataApiEnabled = useWatch_Shadcn_({ control: form.control, name: 'dataApi' }) @@ - {!form.getValues('dataApi') && ( + {!isDataApiEnabled && ( <Admonition type="warning" title="Data API will effectively be disabled"> You will not be able to query or mutate data via Supabase client libraries like supabase-js. </Admonition> )}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/studio/pages/new/`[slug].tsx:
- Around line 75-78: Remove the duplicated feature-flag computation by passing
the already-computed rlsExperimentVariant-derived boolean into the
SecurityOptions component: use the existing usePHFlag call that defines
rlsExperimentVariant and the derived shouldShowEnableRlsEventTrigger variable,
then supply shouldShowEnableRlsEventTrigger as a prop to <SecurityOptions ... />
and remove the internal recomputation inside the SecurityOptions component so it
consumes the prop instead of calling usePHFlag itself.
| const rlsExperimentVariant = usePHFlag<'control' | 'test' | false | undefined>( | ||
| 'projectCreationEnableRlsEventTrigger' | ||
| ) | ||
| const shouldShowEnableRlsEventTrigger = rlsExperimentVariant === 'test' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for shouldShowEnableRlsEventTrigger usage in the new.tsx file
rg -n 'shouldShowEnableRlsEventTrigger' apps/studio/pages/new/
# Also check the SecurityOptions component to see what props it accepts
fd -t f 'SecurityOptions' apps/studio/ --exec head -50 {}
# If SecurityOptions is exported from another file, find it
rg -n 'export.*SecurityOptions' apps/studio/Repository: supabase/supabase
Length of output: 2204
🏁 Script executed:
# Get more context from the pages/new/[slug].tsx file around line 75-78 and where SecurityOptions is used
sed -n '70,120p' apps/studio/pages/new/[slug].tsx | cat -n
# Check if shouldShowEnableRlsEventTrigger is used anywhere in pages/new/[slug].tsx
rg -A 20 -B 5 'const shouldShowEnableRlsEventTrigger' apps/studio/pages/new/[slug].tsxRepository: supabase/supabase
Length of output: 3871
🏁 Script executed:
# Search for where SecurityOptions is used in pages/new/[slug].tsx
rg -n 'SecurityOptions' apps/studio/pages/new/[slug].tsx | head -20
# Get the full context of the pages/new/[slug].tsx file around where SecurityOptions is rendered
rg -B 5 -A 10 '<SecurityOptions' apps/studio/pages/new/[slug].tsxRepository: supabase/supabase
Length of output: 845
Remove unused shouldShowEnableRlsEventTrigger variable or pass it to SecurityOptions.
The variable shouldShowEnableRlsEventTrigger is defined on line 78 but never used in this file. Currently, the SecurityOptions component computes its own shouldShowEnableRlsEventTrigger variable independently. To avoid duplicate computation of the flag, pass this variable to the SecurityOptions component as a prop.
🤖 Prompt for AI Agents
In `@apps/studio/pages/new/`[slug].tsx around lines 75 - 78, Remove the duplicated
feature-flag computation by passing the already-computed
rlsExperimentVariant-derived boolean into the SecurityOptions component: use the
existing usePHFlag call that defines rlsExperimentVariant and the derived
shouldShowEnableRlsEventTrigger variable, then supply
shouldShowEnableRlsEventTrigger as a prop to <SecurityOptions ... /> and remove
the internal recomputation inside the SecurityOptions component so it consumes
the prop instead of calling usePHFlag itself.
seanoliver
force-pushed
the
chore/project-create-security
branch
from
9d1d3a5 to
8297dab
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/studio/pages/new/[slug].tsx (1)
1-63: Prettier check failed for this file.
Please run Prettier (e.g.,prettier --write) to resolve the CI formatting error.
♻️ Duplicate comments (1)
apps/studio/components/interfaces/ProjectCreation/SecurityOptions.tsx (1)
54-60: Addrelto the externaltarget="_blank"link.
Prevents reverse‑tabnabbing.🔐 Proposed fix
<Link href="https://supabase.com/docs/reference/javascript/introduction" target="_blank" + rel="noreferrer noopener" className="text-link" >
There was a problem hiding this comment.
This LGTM! I added a few tweaks in this commit: 9d1d3a5
- Switched the feature flag to a multivariate string response instead of boolean (this allows us to measure the experiment in PostHog)
- Added the useWatch_Shadcn_ hook so the form is reactive to the box being checked
- Added an exposure event that only fires if the user is bucketed into
controlortest(meaning it won't fire for users who aren't currently bucketed or before the experiment has officially launched)
Tested that the events are logging correctly and the UI is updating appropriately in local dev. Experiment is ramped to 100% in staging so you should be able to test this on the staging site as well.
Braintrust eval report
|
<img width="1196" height="427" alt="image" src="https://github.com/user-attachments/assets/6784b5e9-99c8-4fc9-b9f5-49672ba6e768" /> This reworks the security section within our project creation form to expose two options. 1/ Enable or disable Data API which is essentially the same as we had previously, just reframed 2/ Enable auto RLS which creates an event trigger that enables RLS on all tables created via public schema. This is the same as what we do via the RLS banner in authentication pages. Note that this also removes the option to disable Data API on public schema and move to dedicated schema. The user can still do this post project creation . Assumption is this is rarely changed on project creation and adds complexity. To test: 1. Create a new project 2. Enable Data API and enable RLS setting 3. After project creation, go to triggers -> event tab -> notice enable rls trigger 4. Create project and disable Data API 5. After project creation , go to settings/api and make sure data api is disabled <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an optional automatic Row-Level Security (RLS) event trigger option in project creation (toggleable via experiment). * **Improvements** * Security panel simplified to checkbox-based controls for Data API and RLS. * Project creation form now includes the enableRlsEventTrigger flag and applies related setup when enabled. * Telemetry records RLS experiment exposure, variant, and whether the RLS trigger was enabled. * Free-project messaging updated to consider user limits. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Charis Lam <26616127+charislam@users.noreply.github.com> Co-authored-by: Sean Oliver <882952+seanoliver@users.noreply.github.com>
4 participants
This reworks the security section within our project creation form to expose two options.
1/ Enable or disable Data API which is essentially the same as we had previously, just reframed
2/ Enable auto RLS which creates an event trigger that enables RLS on all tables created via public schema. This is the same as what we do via the RLS banner in authentication pages.
Note that this also removes the option to disable Data API on public schema and move to dedicated schema. The user can still do this post project creation . Assumption is this is rarely changed on project creation and adds complexity.
To test:
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.