Skip to content

fix(policies): fall back to public when roles is an empty array - #1107

Open
rezimeshvelashvili wants to merge 1 commit into
supabase:masterfrom
rezimeshvelashvili:fix/policies-empty-roles-default-public
Open

fix(policies): fall back to public when roles is an empty array#1107
rezimeshvelashvili wants to merge 1 commit into
supabase:masterfrom
rezimeshvelashvili:fix/policies-empty-roles-default-public

Conversation

@rezimeshvelashvili

Copy link
Copy Markdown

Problem

policies.update() builds the TO clause with roles.map(ident).join(','), which returns an empty string for []. The generated statement becomes:

ALTER POLICY "test policy" ON public.memes TO ;

and Postgres rejects it with syntax error at or near ";".

An empty array is the natural way for a client to express "all roles", and it is what policies.create() already defaults to (roles = ['public']). The update path only handled undefined ("leave roles alone") and never considered [].

Why this hasn't been hit recently

Studio applies the same fallback client-side before calling the API, in apps/studio/components/interfaces/Database/Policies/PolicyEditorPanel/index.tsx:

const updatedRoles = roles.length === 0 ? ['public'] : roles.split(', ')

so the dashboard never sends []. The original dashboard report (supabase/supabase#7740) was resolved by that client-side change, which is why the symptom disappeared while the server-side cause stayed open as #361.

The bug is still reachable for anyone calling the REST API directly or using the published @supabase/postgres-meta package. If this lands, Studio's workaround becomes redundant.

Fix

Treat [] as ['public'], matching create():

roles meaning SQL emitted
undefined leave roles unchanged (nothing)
[] all roles ... TO public;
['anon'] those roles ... TO anon;

Test

test/lib/policies.ts creates a policy with ['postgres'], updates it with [], and expects ['public']. It fails on master with the syntax error above and passes with the change. Full suite: 200/200.

One note on the test: it casts the payload because update() types name as required, even though the implementation branches on name === undefined and the route passes request.body as any. Passing the current name isn't a workaround either, since renaming a policy to its own name errors with policy ... already exists. I left the type alone to keep this PR to a single change, but happy to follow up with name?: string if you'd like it.

Fixes #361

`policies.update()` built the role list with `roles.map(ident).join(',')`,
so an empty array produced `ALTER POLICY ... TO ;` and Postgres rejected
it with `syntax error at or near ";"`.

An empty array is the natural way to express "all roles", and it is what
`policies.create()` already defaults to. Studio sidesteps the bug by
substituting `['public']` client-side before calling the API, so it only
surfaces for direct REST and library consumers.

Fixes supabase#361
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.

policies: update: roles set to [] doesn't default to public

1 participant