From 45bb7c30ceaa2cd5c8627ea18fb5d2dd689a625c Mon Sep 17 00:00:00 2001 From: Miranda Limonczenko Date: Mon, 17 Aug 2026 16:40:09 -0700 Subject: [PATCH 1/4] docs(database): fix RLS guide copy and two SQL examples (#49015) Stacked on #49011. Base is `docs/rls-revision`, so review that one first. ## Problem An audit of the Row Level Security guide against `apps/docs/CONTRIBUTING.md` and `WORD_LIST.md` turned up 4 lint warnings and 3 things that are wrong rather than just untidy. - Two SQL examples contradict the guide's own advice. The own-profile `SELECT` policy has no `TO` clause. The `security definer` example has no `set search_path`. - `## Bypassing Row Level Security` says Service Keys bypass RLS, then a note says Supabase adheres to the signed-in user's policy anyway. The condition that separates the two is never stated. - `#using-functions` is linked twice from the RBAC guide and has never existed on the RLS page. ## Solution Copy and correctness only. No section moves, no heading renames. - Replace the italic emphasis on `never` with bold. CONTRIBUTING permits **bold** for a term the reader must not miss, not italics for general emphasis. The matching fix for `must` lives in #49011, which rewrites that line anyway. - Drop marketing language from the opener, the Supabase intro, and the policies and performance leads. Removes the idiom "get the hang of them" and the filler `just`. - Replace `we` with second person in two places. - Scope the own-profile `SELECT` example with `to authenticated`. - Pin `search_path = ''` on the `security definer` example, schema-qualify its body to match, and state the requirement in prose. - State when a Service Key actually bypasses RLS. - Repoint the two RBAC links to `#use-security-definer-functions` and `#helper-functions`. `supa-mdx-lint` on the RLS guide goes from 4 warnings to 0. ## Manual testing 1. Open the [Row Level Security guide](https://docs-git-docs-rls-copy-fixes-supabase.vercel.app/docs/guides/database/postgres/row-level-security) on the preview. The own-profile SELECT example shows `to authenticated`, and the security definer example shows `set search_path = ''`. 2. Open the [RBAC guide](https://docs-git-docs-rls-copy-fixes-supabase.vercel.app/docs/guides/api/custom-claims-and-role-based-access-control-rbac) and select the "RLS helper functions" link near the end. It lands on the Helper functions section instead of the top of the page. 3. From `apps/docs`, run `pnpm lint:mdx`. The RLS guide reports no warnings. ## Summary by CodeRabbit - **Documentation** - Updated access-control guidance with clearer links for security-definer functions and RLS helper functions. - Clarified that exposed tables require Row Level Security (RLS), while table grants and row policies provide separate controls. - Added least-privilege and grant-revocation examples, plus explanations for authorization errors. - Expanded testing guidance for CRUD policies, identity switching, and denied operations. - Improved recommendations for service keys, policy performance, indexing, and secure function configuration. --------- Co-authored-by: Claude Opus 5 Co-authored-by: Cursor --- ...ims-and-role-based-access-control-rbac.mdx | 4 +- .../database/postgres/row-level-security.mdx | 170 ++++++++++++++++-- 2 files changed, 157 insertions(+), 17 deletions(-) diff --git a/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx b/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx index cea9bc511670d..d1cf1d7e794e8 100644 --- a/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx +++ b/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx @@ -181,7 +181,7 @@ $$ language plpgsql stable security definer set search_path = ''; -You can read more about using functions in RLS policies in the [RLS guide](/docs/guides/database/postgres/row-level-security#using-functions). +You can read more about using functions in RLS policies in the [RLS guide](/docs/guides/database/postgres/row-level-security#use-security-definer-functions). @@ -219,5 +219,5 @@ You now have a robust system in place to manage user roles and permissions withi - [Auth Hooks](/docs/guides/auth/auth-hooks) - [Row Level Security](/docs/guides/database/postgres/row-level-security) -- [RLS Functions](/docs/guides/database/postgres/row-level-security#using-functions) +- [RLS helper functions](/docs/guides/database/postgres/row-level-security#helper-functions) - [Next.js Slack Clone Example](https://github.com/supabase/supabase/tree/master/examples/slack-clone/nextjs-slack-clone) diff --git a/apps/docs/content/guides/database/postgres/row-level-security.mdx b/apps/docs/content/guides/database/postgres/row-level-security.mdx index 460e2aa761191..7943aebcc910f 100644 --- a/apps/docs/content/guides/database/postgres/row-level-security.mdx +++ b/apps/docs/content/guides/database/postgres/row-level-security.mdx @@ -5,7 +5,7 @@ description: 'Secure your data using Postgres Row Level Security.' subtitle: 'Secure your data using Postgres Row Level Security.' --- -When you need granular authorization rules, nothing beats Postgres's [Row Level Security (RLS)](https://www.postgresql.org/docs/current/ddl-rowsecurity.html). +Postgres [Row Level Security (RLS)](https://www.postgresql.org/docs/current/ddl-rowsecurity.html) gives you granular authorization rules that run inside the database. ## Row Level Security in Supabase @@ -31,15 +31,15 @@ Policies alone don't do this. See [Grants and policies](#grants-and-policies). -RLS is incredibly powerful and flexible, allowing you to write complex SQL rules that fit your unique business needs. RLS can be combined with [Supabase Auth](/docs/guides/auth) for end-to-end user security from the browser to the database. +You write RLS rules in SQL, so a rule can express whatever access logic your app needs. Combine RLS with [Supabase Auth](/docs/guides/auth) for end-to-end user security from the browser to the database. RLS is a Postgres primitive and can provide "[defense in depth]()" to protect your data from malicious actors even when accessed through third-party tooling. ## Policies -[Policies](https://www.postgresql.org/docs/current/sql-createpolicy.html) are Postgres's rule engine. Policies are easy to understand once you get the hang of them. Each policy is attached to a table, and the policy is executed every time a table is accessed. +[Policies](https://www.postgresql.org/docs/current/sql-createpolicy.html) are Postgres's rule engine. Each policy is attached to a table, and the policy is executed every time a table is accessed. -You can just think of them as adding a `WHERE` clause to every query. For example a policy like this ... +Think of a policy as adding a `WHERE` clause to every query. For example, a policy like this: ```sql create policy "Individuals can view their own todos." @@ -47,7 +47,7 @@ on todos for select using ( (select auth.uid()) = user_id ); ``` -.. would translate to this whenever a user tries to select from the todos table: +That policy translates to this whenever a user tries to select from the todos table: ```sql select * @@ -217,7 +217,7 @@ Using the `anon` Postgres role is different from an [anonymous user](/docs/guide Policies are SQL logic that you attach to a Postgres table. You can attach as many policies as you want to each table. -Supabase provides some [helpers](#helper-functions) that simplify RLS if you're using Supabase Auth. We'll use these helpers to illustrate some basic policies: +Supabase provides some [helpers](#helper-functions) that simplify RLS if you're using Supabase Auth. The examples below use these helpers. ### SELECT policies @@ -247,15 +247,16 @@ Alternatively, if you only wanted users to be able to see their own profiles: ```sql create policy "User can see their own profile only." -on profiles -for select using ( (select auth.uid()) = user_id ); +on profiles for select +to authenticated +using ( (select auth.uid()) = user_id ); ``` ### INSERT policies You can specify insert policies with the `with check` clause. The `with check` expression ensures that any new row data adheres to the policy constraints. -Say you have a table called `profiles` in the public schema and you only want users to create a profile for themselves. In that case, we want to check their User ID matches the value that they are trying to insert: +Say you have a table called `profiles` in the public schema and you only want users to create a profile for themselves. In that case, check that their user ID matches the value they are trying to insert: ```sql -- 1. Create table @@ -408,7 +409,7 @@ Supabase provides special "Service" keys, which can be used to bypass RLS. These -Supabase will adhere to the RLS policy of the signed-in user, even if the client library is initialized with a Service Key. +A Service Key bypasses RLS only when the request carries no user access token. If the request carries one, it runs under the RLS policies of that signed-in user, even when the client library was initialized with a Service Key. @@ -418,7 +419,143 @@ You can also create new [Postgres Roles](/docs/guides/database/postgres/roles) w alter role "role_name" with bypassrls; ``` -This can be useful for system-level access. You should _never_ share login credentials for any Postgres Role with this privilege. +This can be useful for system-level access. **Never** share login credentials for any Postgres Role with this privilege. + +## Test your policies + +We recommend writing tests for every policy, in the same change that sets the grants and creates the policies. Tests are a fundamental part of a secure setup, and they give you a repeatable way to prove a policy behaves the way you intended. + +A wrong policy fails quietly. Too permissive, and a query returns rows it shouldn't. Too strict, and it returns nothing and raises no error. Neither case surfaces as an error, so tests are how you find out. + +Supabase runs database tests with [pgTAP](/docs/guides/database/extensions/pgtap) through the CLI. Test files are `.sql` files under `supabase/tests/`. + +### Anatomy of a policy test + +Each case sets an identity, runs one statement as that identity, and asserts the outcome. Three things decide whether the assertion means anything. + +**Identity.** Switch role and identity between cases with `set local role` and `set local request.jwt.claim.sub`, so each assertion runs as the user it describes. Without the switch, every case runs as the same role and proves nothing about access. + +**Denials.** A denied request doesn't always raise an error, so match the assertion to the way the denial happens: + +- A missing grant raises `42501`. Assert it with `throws_ok`. +- A `with check` violation raises `42501`. Assert it with `throws_ok`. +- A `using` clause that filters the target row out raises nothing. The update or delete matches zero rows instead. Assert that no row changed. + +**Allowed writes.** The absence of an error doesn't prove that anything changed. Add `returning` to the statement so one assertion covers both directions. An allowed write returns the changed row, and a write the policy filters out returns nothing. + +### Write and run the tests + +1. Create the tests directory and a test file: + + ```bash + mkdir -p supabase/tests + touch supabase/tests/profiles_rls.test.sql + ``` + +2. Write the tests. Cover `select`, `insert`, `update`, and `delete` twice each, once for a request the policy allows and once for a request it denies. Cover `anon` as well as `authenticated`. + +3. Run the suite: + + ```bash + supabase test db + ``` + +This example tests a `profiles` table where `authenticated` holds every privilege, `anon` holds none, and each user reads and writes only their own row: + +```sql supabase/tests/profiles_rls.test.sql +begin; +select plan(11); + +-- Seed two users. The rows come later, through the policies under test. +insert into auth.users (id, email) +values + ('11111111-1111-1111-1111-111111111111', 'owner@example.com'), + ('22222222-2222-2222-2222-222222222222', 'other@example.com'); + +-- Signed-out visitors hold no grant, so the request stops before any policy runs. +set local role anon; +select throws_ok( + $$select * from profiles$$, + '42501', + null, + 'anon cannot read profiles' +); +select throws_ok( + $$insert into profiles (id, user_id) + values (gen_random_uuid(), '11111111-1111-1111-1111-111111111111')$$, + '42501', + null, + 'anon cannot insert a profile' +); + +-- The owner reads and writes their own row. +set local role authenticated; +set local request.jwt.claim.sub = '11111111-1111-1111-1111-111111111111'; +select results_eq( + $$insert into profiles (id, user_id, avatar_url) + values ( + gen_random_uuid(), + '11111111-1111-1111-1111-111111111111', + 'owner.png' + ) + returning avatar_url$$, + array['owner.png'], + 'the owner creates their own profile' +); +select results_eq( + $$select avatar_url from profiles$$, + array['owner.png'], + 'the owner reads their own profile' +); +select results_eq( + $$update profiles set avatar_url = 'updated.png' returning avatar_url$$, + array['updated.png'], + 'the owner updates their own profile' +); + +-- The with check clause rejects the row, which raises. +select throws_ok( + $$insert into profiles (id, user_id) + values (gen_random_uuid(), '22222222-2222-2222-2222-222222222222')$$, + '42501', + null, + 'the owner cannot create a profile for someone else' +); + +-- A signed-in stranger holds the grant, so the policy is what stops them. The +-- using clause filters the row out, so these match nothing and raise nothing. +set local request.jwt.claim.sub = '22222222-2222-2222-2222-222222222222'; +select is_empty( + $$select * from profiles$$, + 'another user reads no profiles' +); +select is_empty( + $$update profiles set avatar_url = 'stolen.png' returning avatar_url$$, + 'another user updates no profiles' +); +select is_empty( + $$delete from profiles returning id$$, + 'another user deletes no profiles' +); + +-- The row is still there, still holding the owner's value. +set local request.jwt.claim.sub = '11111111-1111-1111-1111-111111111111'; +select results_eq( + $$select avatar_url from profiles$$, + array['updated.png'], + 'the other user changed nothing' +); +select results_eq( + $$delete from profiles returning avatar_url$$, + array['updated.png'], + 'the owner deletes their own profile' +); + +select * from finish(); +rollback; +``` + +For CLI setup and more pgTAP helpers, see [Testing your database](/docs/guides/database/testing). ## Test your policies @@ -558,9 +695,9 @@ For CLI setup and more pgTAP helpers, see [Testing your database](/docs/guides/d ## RLS performance recommendations -Every authorization system has an impact on performance. While row level security is powerful, the performance impact is important to keep in mind. This is especially true for queries that scan every row in a table - like many `select` operations, including those using limit, offset, and ordering. +Every authorization system has an impact on performance. Postgres evaluates a policy expression against each candidate row, so the cost scales with the rows a query scans. This matters most for queries that scan every row in a table, like many `select` operations, including those using limit, offset, and ordering. -Based on a series of [tests](https://github.com/GaryAustin1/RLS-Performance), we have a few recommendations for RLS: +Based on a series of [tests](https://github.com/GaryAustin1/RLS-Performance), these are the recommendations for RLS: ### Add indexes @@ -688,10 +825,11 @@ create function private.has_good_role() returns boolean language plpgsql security definer -- will run as the creator +set search_path = '' -- every name inside must be schema-qualified as $$ begin return exists ( - select 1 from roles_table + select 1 from public.roles_table where (select auth.uid()) = user_id and role = 'good_role' ); end; @@ -704,9 +842,11 @@ to authenticated using ( (select private.has_good_role()) ); ``` +Set `search_path = ''` on every `security definer` function and schema-qualify the names inside it. Without a pinned `search_path`, a caller can point an unqualified name at their own object and run it with the function owner's privileges. + -Security-definer functions should never be created in a schema in the "Exposed schemas" inside your [API settings](/dashboard/project/_/settings/api)`. +A `security definer` function in an exposed schema is callable over the Data API with the creator's privileges. Never create one in a schema listed under "Exposed schemas" in your [API settings](/dashboard/project/_/settings/api). From fba373314819b9e793c17fae2f86dca34070b943 Mon Sep 17 00:00:00 2001 From: Danny White <3104761+dnywh@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:40:40 +1000 Subject: [PATCH 2/4] test(studio): allow scoped token creation in CI (#49163) ## What kind of change does this PR introduce? Test reliability fix. ## What is the current behavior? The two longest scoped access token creation tests can exceed Vitest's default five-second timeout when they run under the full Studio CI shard, despite passing locally. ## What is the new behavior? The project-scoped and organisation-scoped token creation tests each use a targeted ten-second timeout. The global timeout and production code remain unchanged. ## Summary by CodeRabbit * **Tests** * Increased test timeouts for project- and organization-scoped token creation scenarios to improve test reliability. --- .../Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx index dce13235404ed..a1c401f7170b7 100644 --- a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx +++ b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx @@ -204,7 +204,7 @@ describe('NewScopedTokenSheet', () => { fireEvent.click(await screen.findByRole('button', { name: 'Done' })) // Dialog has been closed await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) - }) + }, 10_000) // Organization scope tests test('requires an organization when scope is Organization', async () => { @@ -254,7 +254,7 @@ describe('NewScopedTokenSheet', () => { fireEvent.click(await screen.findByRole('button', { name: 'Done' })) // Dialog has been closed await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) - }) + }, 10_000) test('opens the experimental API dialog from the dropdown', async () => { renderSheet() From cfde341c3144b45325b8c0676af9d186b31ab658 Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Tue, 18 Aug 2026 10:44:32 +0800 Subject: [PATCH 3/4] QueryTab: Scope row limit to each tab instead of global (#49138) ## Context Previous PR [here](https://github.com/supabase/supabase/pull/49098) introduced row limits to the Explorer -> Query Tab, but the setting was global (e.g selected row limit value would be the same despite switching query tabs) Changes here shifts the scope of row limit into the query draft so that the value is tied to each individual query tab instead Also added a logic as CodeRabbit suggested [here](https://github.com/supabase/supabase/pull/49138#discussion_r3795525802) - to default invalid row limit values to 100 if the persisted data is mutated incorrectly ## Summary by CodeRabbit ## Summary by CodeRabbit * **New Features** * Query tabs now retain row-limit settings independently for each database draft. * Row-limit preferences are restored when reopening Studio, with older drafts defaulting to 100 rows. * **Bug Fixes** * Changing the row limit now persists immediately and no longer affects other query drafts. * Invalid saved row limits are safely normalized to a supported value. --- .../interfaces/Explorer/QueryTab.tsx | 5 +- apps/studio/state/explorer-query.test.ts | 91 +++++++++++++++++++ apps/studio/state/explorer-query.ts | 34 ++++++- 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/apps/studio/components/interfaces/Explorer/QueryTab.tsx b/apps/studio/components/interfaces/Explorer/QueryTab.tsx index ee94abf4ea894..0002706862432 100644 --- a/apps/studio/components/interfaces/Explorer/QueryTab.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryTab.tsx @@ -19,7 +19,6 @@ export const QueryTab = () => { const querySnap = useExplorerQueryStateSnapshot() const roleImpersonationState = useLocalRoleImpersonationState() - const [rowLimit, setRowLimit] = useState(100) const [restoredQueryKey, setRestoredQueryKey] = useState() const stateDraft = id ? querySnap.drafts[id] : undefined @@ -84,7 +83,7 @@ export const QueryTab = () => { : { ...toQuerySourceBinding(draft), uncheckedSql: draft.uncheckedSql, - rowLimit, + rowLimit: draft.rowLimit, } return ( @@ -103,7 +102,7 @@ export const QueryTab = () => { onSqlChange={(sql) => explorerQueryState.updateDraft({ id, sql })} onSourceChange={(source) => explorerQueryState.updateDraft({ id, source })} onResultChange={handleResultChange} - onRowLimitChange={setRowLimit} + onRowLimitChange={(rowLimit) => explorerQueryState.updateDraft({ id, rowLimit })} /> ) } diff --git a/apps/studio/state/explorer-query.test.ts b/apps/studio/state/explorer-query.test.ts index e23f7f9fbdb56..5a8c954e391fa 100644 --- a/apps/studio/state/explorer-query.test.ts +++ b/apps/studio/state/explorer-query.test.ts @@ -202,6 +202,97 @@ describe('explorer query drafts', () => { expect(persisted[`query-${MAX_PERSISTED_EXPLORER_QUERY_DRAFTS}`]).toBeDefined() }) + it('persists and restores a per-draft row limit independently of other drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.createDraft({ id: 'query-2', projectRef: 'project-a' }) + state.updateDraft({ id: 'query-1', rowLimit: 500 }) + + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 500 }) + expect(state.drafts['query-2']).toMatchObject({ rowLimit: 100 }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.restoreDraft({ id: 'query-2', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ rowLimit: 500 }) + expect(restored.drafts['query-2']).toMatchObject({ rowLimit: 100 }) + }) + + it('defaults the row limit for drafts persisted before row limits existed', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Legacy query', sql: 'select 1', updatedAt: 1 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('accepts every row limit the row limit menu can produce', () => { + const storage = createMemoryStorage() + + for (const rowLimit of [-1, 100, 500, 1000]) { + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit }) + } + }) + + it('normalizes a fractional persisted row limit to the default', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 100.5 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('normalizes an out-of-range persisted row limit to the default', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 999999 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('normalizes a non-numeric persisted row limit to the default without dropping the draft', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 'unlimited' }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100, uncheckedSql: 'select 1' }) + }) + it('removes the persisted draft and its session result when its tab closes', () => { const storage = createMemoryStorage() const state = createExplorerQueryState(storage) diff --git a/apps/studio/state/explorer-query.ts b/apps/studio/state/explorer-query.ts index ae1f1972c6177..5e6f6cb6898e4 100644 --- a/apps/studio/state/explorer-query.ts +++ b/apps/studio/state/explorer-query.ts @@ -3,7 +3,9 @@ import { LOCAL_STORAGE_KEYS, safeLocalStorage } from 'common' import { proxy, ref, snapshot, useSnapshot } from 'valtio' import { z } from 'zod' +import { DEFAULT_CELL_ROW_LIMIT } from '@/components/interfaces/Explorer/QueryCell/QueryCell.utils' import { type QueryResult } from '@/components/interfaces/Explorer/types' +import { ROWS_PER_PAGE_OPTIONS } from '@/components/interfaces/SQLEditor/SQLEditor.constants' import { type DatabaseSourceParameters, type LogsSourceParameters, @@ -33,6 +35,7 @@ export type DatabaseQueryDraft = ExplorerQueryDraftBase & DatabaseSourceParameters & { _tag: 'database' uncheckedSql: UntrustedSqlFragment + rowLimit: number } export type LogsQueryDraft = ExplorerQueryDraftBase & @@ -57,6 +60,7 @@ type PersistedExplorerQueryDraft = { source: QuerySourceBinding sql: string updatedAt: number + rowLimit?: number } type PersistedExplorerQueryDrafts = Record @@ -72,8 +76,23 @@ const persistedDraftSchema = z.object({ sql: z.string(), updatedAt: z.number(), source: z.unknown().optional(), + rowLimit: z.unknown().optional(), }) +const VALID_ROW_LIMITS = ROWS_PER_PAGE_OPTIONS.map((option) => option.value) + +/** + * Falls back to the default whenever a persisted row limit isn't one of the values the row + * limit menu can actually produce — e.g. a fractional or out-of-range number from corrupted + * or hand-edited storage. `undefined` (never persisted) passes through unchanged; `toDraft` + * applies the default for that case. + */ +const rowLimitSchema = z + .number() + .refine((value) => VALID_ROW_LIMITS.includes(value)) + .catch(DEFAULT_CELL_ROW_LIMIT) + .optional() + /** * Rebuilds a draft from its persisted form, branding the SQL for the backend the binding * names. The single place a stored string re-enters the type system as untrusted SQL, which @@ -104,6 +123,7 @@ const toDraft = ({ _tag: 'database', database_identifier: persisted.source.database_identifier, uncheckedSql: untrustedSql(persisted.sql), + rowLimit: persisted.rowLimit ?? DEFAULT_CELL_ROW_LIMIT, } } @@ -125,6 +145,8 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { ? parsedSource.data : createDefaultSourceBinding('database') + const rowLimit = rowLimitSchema.parse(draft.data.rowLimit) + return [ [ id, @@ -133,6 +155,7 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { source, sql: draft.data.sql, updatedAt: draft.data.updatedAt, + rowLimit, }, ], ] @@ -172,6 +195,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: toQuerySourceBinding(draft), sql: draft.uncheckedSql, updatedAt: draft.updatedAt, + rowLimit: draft._tag === 'database' ? draft.rowLimit : undefined, } writePersistedDrafts(storage, draft.projectRef, persisted) } @@ -186,12 +210,14 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage name = 'Untitled query', sql = '', source = createDefaultSourceBinding('database'), + rowLimit = DEFAULT_CELL_ROW_LIMIT, }: { id: string projectRef: string name?: string sql?: string source?: QuerySourceBinding + rowLimit?: number }) => { const draft = toDraft({ id, @@ -201,6 +227,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: querySourceBindingSchema.parse(source), sql, updatedAt: Date.now(), + rowLimit, }, }) @@ -237,11 +264,13 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage name, source, sql, + rowLimit, }: { id: string name?: string source?: QuerySourceBinding sql?: string + rowLimit?: number }) => { const draft = state.drafts[id] if (!draft) return @@ -249,6 +278,8 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage const nextSource = source === undefined ? undefined : querySourceBindingSchema.parse(source) if (nextSource !== undefined && nextSource._tag !== draft._tag) delete state.results[id] + const currentRowLimit = draft._tag === 'database' ? draft.rowLimit : undefined + state.drafts[id] = toDraft({ id, projectRef: draft.projectRef, @@ -257,6 +288,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: nextSource ?? toQuerySourceBinding(draft), sql: sql ?? draft.uncheckedSql, updatedAt: Date.now(), + rowLimit: rowLimit ?? currentRowLimit, }, }) @@ -273,7 +305,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage const pending = pendingPersistence.get(id) if (pending) clearTimeout(pending.timeout) - if (name !== undefined || source !== undefined) persist() + if (name !== undefined || source !== undefined || rowLimit !== undefined) persist() else { const timeout = setTimeout(persist, EXPLORER_QUERY_PERSIST_DELAY) pendingPersistence.set(id, { timeout, persist }) From bfab3090f52841d31b028f061237811b6f93edba Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Tue, 18 Aug 2026 11:27:09 +0800 Subject: [PATCH 4/4] QueryTab: Scope role impersonation to each tab instead of global (#49139) ## Context Previous PR [here](https://github.com/supabase/supabase/pull/49101) introduced role impersonation to the Explorer -> Query Tab, but the setting was global (e.g selected role would be the same despite switching query tabs) Changes here shifts the scope of the role impersonation into the query draft so that the value is tied to each individual query tab instead ## Summary by CodeRabbit * **New Features** * Query drafts now remember selected impersonated roles when switching between drafts or returning later. * Added support for clearing saved impersonated roles. * Impersonation state remains isolated across query tabs. * **Bug Fixes** * Prevented impersonation settings from carrying over between unrelated drafts. * Invalid saved role data is safely ignored during restoration. * Logs drafts no longer persist or update impersonated roles. --- .../interfaces/Explorer/QueryTab.test.tsx | 40 +++++++- .../interfaces/Explorer/QueryTab.tsx | 15 ++- apps/studio/lib/role-impersonation.ts | 53 +++++++++++ apps/studio/state/explorer-query.test.ts | 64 +++++++++++++ apps/studio/state/explorer-query.ts | 26 +++++ .../studio/state/role-impersonation-state.tsx | 95 +++++++++++++++++-- 6 files changed, 282 insertions(+), 11 deletions(-) diff --git a/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx b/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx index 61c303017ada8..21da1d7857472 100644 --- a/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx @@ -32,7 +32,19 @@ vi.mock('@/components/ui/CodeEditor/CodeEditor', () => ({ ), })) -vi.mock('./ExplorerQuerySourceMenu', () => ({ ExplorerQuerySourceMenu: () => null })) +vi.mock('./ExplorerQuerySourceMenu', () => ({ + ExplorerQuerySourceMenu: ({ + roleImpersonationState, + }: { + roleImpersonationState?: { role?: { type: string; role?: string } } + }) => ( +
+ {roleImpersonationState?.role?.type === 'postgrest' + ? roleImpersonationState.role.role + : 'none'} +
+ ), +})) const renderQueryTab = () => customRender( @@ -168,4 +180,30 @@ describe('QueryTab execution', () => { new Date(bodies[0].iso_timestamp_start).getTime() ).toBe(2 * 60 * 60 * 1000) }) + + it('isolates the impersonated role selection per query tab', async () => { + createDraft({ _tag: 'database' }) + explorerQueryState.removeDraft({ id: 'query-test-2', projectRef: 'default' }) + explorerQueryState.createDraft({ id: 'query-test-2', projectRef: 'default', sql: 'select 2' }) + explorerQueryState.setRole({ + id: 'query-test', + role: { type: 'postgrest', role: 'service_role' }, + }) + + const { rerender } = renderQueryTab() + expect(await screen.findByTestId('impersonated-role')).toHaveTextContent('service_role') + + testContext.params = { ref: 'default', id: 'query-test-2' } + rerender( + + + + ) + + expect(await screen.findByTestId('impersonated-role')).toHaveTextContent('none') + + await act(async () => { + explorerQueryState.removeDraft({ id: 'query-test-2', projectRef: 'default' }) + }) + }) }) diff --git a/apps/studio/components/interfaces/Explorer/QueryTab.tsx b/apps/studio/components/interfaces/Explorer/QueryTab.tsx index 0002706862432..306b03b41712b 100644 --- a/apps/studio/components/interfaces/Explorer/QueryTab.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryTab.tsx @@ -1,14 +1,14 @@ import { useParams } from 'common' import { Loader2, SquareCode } from 'lucide-react' import { useRouter } from 'next/router' -import { useContext, useEffect, useState } from 'react' +import { useCallback, useContext, useEffect, useState } from 'react' import { Button } from 'ui' import { QueryEditor, type ExplorerQueryModel } from './QueryEditor' import { type QueryResult } from './types' import { toQuerySourceBinding } from '@/data/query-sources/query-source-registry' import { explorerQueryState, useExplorerQueryStateSnapshot } from '@/state/explorer-query' -import { useLocalRoleImpersonationState } from '@/state/role-impersonation-state' +import { useControlledRoleImpersonationState } from '@/state/role-impersonation-state' import { createTabId, TabsStateContext } from '@/state/tabs' /** Query-tab lifecycle adapter around the shared QueryEditor. */ @@ -17,7 +17,6 @@ export const QueryTab = () => { const router = useRouter() const tabs = useContext(TabsStateContext) const querySnap = useExplorerQueryStateSnapshot() - const roleImpersonationState = useLocalRoleImpersonationState() const [restoredQueryKey, setRestoredQueryKey] = useState() @@ -26,6 +25,16 @@ export const QueryTab = () => { const result = draft && id ? querySnap.results[id] : undefined const queryKey = id && ref ? `${ref}:${id}` : undefined + const roleImpersonationState = useControlledRoleImpersonationState( + draft?._tag === 'database' ? draft.role : undefined, + useCallback( + (role) => { + if (id) explorerQueryState.setRole({ id, role }) + }, + [id] + ) + ) + useEffect(() => { if (!id || !ref) return diff --git a/apps/studio/lib/role-impersonation.ts b/apps/studio/lib/role-impersonation.ts index 0f6353f12b70e..96fd694deaf29 100644 --- a/apps/studio/lib/role-impersonation.ts +++ b/apps/studio/lib/role-impersonation.ts @@ -1,4 +1,5 @@ import { getImpersonationSQL, type SafeSqlFragment } from '@supabase/pg-meta' +import { z } from 'zod' import { uuidv4 } from './helpers' import type { User } from '@/data/auth/users-infinite-query' @@ -40,6 +41,58 @@ type CustomImpersonationRole = { export type ImpersonationRole = PostgrestImpersonationRole | CustomImpersonationRole +/** + * The impersonated `user` is the same generated `User` shape already persisted verbatim to + * localStorage elsewhere (see `USER_IMPERSONATION_SELECTOR_PREVIOUS_SEARCHES`) — trusted + * as-is rather than re-validated field-by-field, since it only ever round-trips our own + * writes and its shape tracks a generated API type this schema shouldn't have to mirror. + */ +const impersonatedUserSchema = z + .record(z.string(), z.unknown()) + .transform((value) => value as unknown as User) + +const aalSchema = z.enum(['aal1', 'aal2']) + +const postgrestImpersonationRoleSchema = z.union([ + z.object({ type: z.literal('postgrest'), role: z.literal('anon') }).strict(), + z.object({ type: z.literal('postgrest'), role: z.literal('service_role') }).strict(), + z + .object({ + type: z.literal('postgrest'), + role: z.literal('authenticated'), + userType: z.literal('native'), + user: impersonatedUserSchema.optional(), + aal: aalSchema.optional(), + }) + .strict(), + z + .object({ + type: z.literal('postgrest'), + role: z.literal('authenticated'), + userType: z.literal('external'), + externalAuth: z + .object({ + sub: z.string(), + additionalClaims: z.record(z.string(), z.unknown()).optional(), + }) + .optional(), + aal: aalSchema.optional(), + }) + .strict(), +]) + +const customImpersonationRoleSchema = z + .object({ type: z.literal('custom'), role: z.string() }) + .strict() + +/** Parses to `ImpersonationRole` — verified at the `role` field assignment in `toDraft` + * (`state/explorer-query.ts`), since annotating the schema type directly here would also + * constrain its *input* type, which is narrower than `ImpersonationRole` pre-transform. */ +export const impersonationRoleSchema = z.union([ + postgrestImpersonationRoleSchema, + customImpersonationRoleSchema, +]) + export function getExp1HourFromNow() { return Math.floor((Date.now() + 60 * 60 * 1000) / 1000) } diff --git a/apps/studio/state/explorer-query.test.ts b/apps/studio/state/explorer-query.test.ts index 5a8c954e391fa..d3268a2606f42 100644 --- a/apps/studio/state/explorer-query.test.ts +++ b/apps/studio/state/explorer-query.test.ts @@ -293,6 +293,70 @@ describe('explorer query drafts', () => { expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100, uncheckedSql: 'select 1' }) }) + it('persists and restores a per-draft impersonated role independently of other drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.createDraft({ id: 'query-2', projectRef: 'project-a' }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'anon' } }) + + expect(state.drafts['query-1']).toMatchObject({ role: { type: 'postgrest', role: 'anon' } }) + expect(state.drafts['query-2']).toMatchObject({ role: undefined }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.restoreDraft({ id: 'query-2', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ + role: { type: 'postgrest', role: 'anon' }, + }) + expect(restored.drafts['query-2']).toMatchObject({ role: undefined }) + }) + + it('clears a persisted role when set back to undefined', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'service_role' } }) + state.setRole({ id: 'query-1', role: undefined }) + + expect(state.drafts['query-1']).toMatchObject({ role: undefined }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ role: undefined }) + }) + + it('drops a malformed persisted role rather than failing to restore the draft', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { + name: 'Query with bad role', + sql: 'select 1', + updatedAt: 1, + role: { type: 'postgrest', role: 'not-a-real-role' }, + }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ role: undefined }) + }) + + it('ignores role updates for logs drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a', source: LOGS_SOURCE }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'anon' } }) + + expect(state.drafts['query-1']).not.toHaveProperty('role') + }) + it('removes the persisted draft and its session result when its tab closes', () => { const storage = createMemoryStorage() const state = createExplorerQueryState(storage) diff --git a/apps/studio/state/explorer-query.ts b/apps/studio/state/explorer-query.ts index 5e6f6cb6898e4..eb15cd9f3bfea 100644 --- a/apps/studio/state/explorer-query.ts +++ b/apps/studio/state/explorer-query.ts @@ -17,6 +17,7 @@ import { toQuerySourceBinding, type QuerySourceBinding, } from '@/data/query-sources/query-source-registry' +import { impersonationRoleSchema, type ImpersonationRole } from '@/lib/role-impersonation' type ExplorerQueryDraftBase = { id: string @@ -36,6 +37,7 @@ export type DatabaseQueryDraft = ExplorerQueryDraftBase & _tag: 'database' uncheckedSql: UntrustedSqlFragment rowLimit: number + role?: ImpersonationRole } export type LogsQueryDraft = ExplorerQueryDraftBase & @@ -61,6 +63,7 @@ type PersistedExplorerQueryDraft = { sql: string updatedAt: number rowLimit?: number + role?: ImpersonationRole } type PersistedExplorerQueryDrafts = Record @@ -77,6 +80,7 @@ const persistedDraftSchema = z.object({ updatedAt: z.number(), source: z.unknown().optional(), rowLimit: z.unknown().optional(), + role: z.unknown().optional(), }) const VALID_ROW_LIMITS = ROWS_PER_PAGE_OPTIONS.map((option) => option.value) @@ -124,6 +128,7 @@ const toDraft = ({ database_identifier: persisted.source.database_identifier, uncheckedSql: untrustedSql(persisted.sql), rowLimit: persisted.rowLimit ?? DEFAULT_CELL_ROW_LIMIT, + role: persisted.role, } } @@ -145,6 +150,8 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { ? parsedSource.data : createDefaultSourceBinding('database') + const parsedRole = impersonationRoleSchema.safeParse(draft.data.role) + const role = parsedRole.success ? parsedRole.data : undefined const rowLimit = rowLimitSchema.parse(draft.data.rowLimit) return [ @@ -155,6 +162,7 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { source, sql: draft.data.sql, updatedAt: draft.data.updatedAt, + role, rowLimit, }, ], @@ -196,6 +204,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage sql: draft.uncheckedSql, updatedAt: draft.updatedAt, rowLimit: draft._tag === 'database' ? draft.rowLimit : undefined, + role: draft._tag === 'database' ? draft.role : undefined, } writePersistedDrafts(storage, draft.projectRef, persisted) } @@ -279,6 +288,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage if (nextSource !== undefined && nextSource._tag !== draft._tag) delete state.results[id] const currentRowLimit = draft._tag === 'database' ? draft.rowLimit : undefined + const currentRole = draft._tag === 'database' ? draft.role : undefined state.drafts[id] = toDraft({ id, @@ -289,6 +299,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage sql: sql ?? draft.uncheckedSql, updatedAt: Date.now(), rowLimit: rowLimit ?? currentRowLimit, + role: currentRole, }, }) @@ -337,6 +348,21 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage setResult: ({ id, result }: { id: string; result: ExplorerQueryResult }) => { state.results[id] = ref(result) }, + + /** + * Separate from `updateDraft` because `undefined` is a meaningful value here (clearing + * the impersonated role), whereas `updateDraft`'s optional fields all use `undefined` + * to mean "leave unchanged." Logs drafts have no impersonation concept, so this is a + * no-op for them. + */ + setRole: ({ id, role }: { id: string; role: ImpersonationRole | undefined }) => { + const draft = state.drafts[id] + if (!draft || draft._tag !== 'database') return + + const updatedDraft: DatabaseQueryDraft = { ...draft, role, updatedAt: Date.now() } + state.drafts[id] = updatedDraft + persistDraft(updatedDraft) + }, }) return state diff --git a/apps/studio/state/role-impersonation-state.tsx b/apps/studio/state/role-impersonation-state.tsx index e1ba37494ffdb..4142c61d6966f 100644 --- a/apps/studio/state/role-impersonation-state.tsx +++ b/apps/studio/state/role-impersonation-state.tsx @@ -6,11 +6,15 @@ import { useCallback, useContext, useEffect, + useRef, useState, } from 'react' import { proxy, snapshot, subscribe, useSnapshot } from 'valtio' -import { CustomAccessTokenHookDetails } from '../hooks/misc/useCustomAccessTokenHookDetails' +import { + CustomAccessTokenHookDetails, + useCustomAccessTokenHookDetails, +} from '../hooks/misc/useCustomAccessTokenHookDetails' import { executeSql } from '@/data/sql/execute-sql-mutation' import { useLatest } from '@/hooks/misc/useLatest' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' @@ -100,12 +104,6 @@ export function createRoleImpersonationState( export type RoleImpersonationState = ReturnType -/** - * The subset of `RoleImpersonationState` a role-picker UI needs: the current selection, its - * resolved claims, and the setter. Satisfied by both the shared project-wide context (via - * `useRoleImpersonationStateSnapshot`) and `useLocalRoleImpersonationState`, so role-picking - * components can work against either without knowing which one they got. - */ export type RoleImpersonationController = Pick< RoleImpersonationState, 'role' | 'claims' | 'setRole' @@ -141,6 +139,9 @@ export function useRoleImpersonationStateSnapshot(options?: Parameters void +): RoleImpersonationController { + const { data: project } = useSelectedProjectQuery() + const customizeAccessToken = useCustomizeAccessToken(project?.ref, project?.connectionString) + + const projectRef = project?.ref ?? '' + const customizeAccessTokenRef = useLatest(customizeAccessToken) + const customAccessTokenHookDetails = useCustomAccessTokenHookDetails(project?.ref) + const customAccessTokenHookDetailsRef = useLatest(customAccessTokenHookDetails) + const onRoleChangeRef = useLatest(onRoleChange) + + const [claims, setClaims] = useState(undefined) + + // Guards against re-resolving claims for a role change that `setRole` below just resolved + // itself — without it, every selection would re-run the (possibly RPC-backed) resolution + // twice: once eagerly in `setRole`, once again here once `role` updates on the next render. + const skipNextResolveRef = useRef(false) + + useEffect(() => { + if (skipNextResolveRef.current) { + skipNextResolveRef.current = false + return + } + + let cancelled = false + + resolveRoleClaims( + projectRef, + role, + customAccessTokenHookDetailsRef.current, + customizeAccessTokenRef.current + ).then((nextClaims) => { + if (!cancelled) setClaims(nextClaims) + }) + + return () => { + cancelled = true + } + // Resolves only when the controlled role identity changes (e.g. switching tabs) + }, [customAccessTokenHookDetailsRef, customizeAccessTokenRef, projectRef, role]) + + const setRole = useCallback( + async ( + nextRole: ImpersonationRole | undefined, + customAccessTokenHookDetails?: CustomAccessTokenHookDetails + ) => { + // Captured before the await: if the controlling tab changes while this resolution is + // in flight, `onRoleChangeRef.current` will point at a different tab's callback by the + // time we get here. Comparing against the captured reference lets us detect that and + // discard the result instead of writing this role/claims into the wrong tab. + const onRoleChangeAtCallTime = onRoleChangeRef.current + + const nextClaims = await resolveRoleClaims( + projectRef, + nextRole, + customAccessTokenHookDetails ?? customAccessTokenHookDetailsRef.current, + customizeAccessTokenRef.current + ) + + if (onRoleChangeRef.current !== onRoleChangeAtCallTime) return + + skipNextResolveRef.current = true + onRoleChangeAtCallTime(nextRole) + setClaims(nextClaims) + }, + [projectRef, customizeAccessTokenRef, customAccessTokenHookDetailsRef, onRoleChangeRef] + ) + + return { role, claims, setRole } +} + export function useGetImpersonatedRoleState() { const roleImpersonationState = useContext(RoleImpersonationStateContext)