Skip to content

ENG-1426 Bug in permission functions #778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maparent wants to merge 5 commits into
base: main
Choose a base branch
from
+109 −9
Open

ENG-1426 Bug in permission functions #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
10e2212
eng-1426 bug in public.upsert_account_in_space, still using editor
maparent Feb 13, 2026
c2e851e
Do not change value when null update params
maparent Feb 14, 2026
eca7a8d
typing
maparent Feb 14, 2026
ba78794
use greatest, not aggregate max
maparent Feb 14, 2026
0c8847b
least, not greatest!
maparent Feb 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/roam/src/utils/syncDgNodesToSupabase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ const getAllUsers = async (): Promise<AccountLocalInput[]> => {
email: null,
email_trusted: null,
space_editor: null,
permissions: "editor",
}));
};

Expand Down
9 changes: 8 additions & 1 deletion packages/database/src/dbTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1542,10 +1542,10 @@ export type Database = {
create_account_in_space: {
Args: {
account_local_id_: string
editor_?: boolean
email_?: string
email_trusted?: boolean
name_: string
permissions_?: Database["public"]["Enums"]["SpaceAccessPermissions"]
space_id_: number
}
Returns: number
Expand Down Expand Up @@ -1660,6 +1660,10 @@ export type Database = {
text_content: string
}[]
}
my_permissions_in_space: {
Args: { space_id_: number }
Returns: Database["public"]["Enums"]["SpaceAccessPermissions"]
}
my_space_ids: {
Args: {
access_level?: Database["public"]["Enums"]["SpaceAccessPermissions"]
Expand Down Expand Up @@ -1798,6 +1802,9 @@ export type Database = {
email: string | null
email_trusted: boolean | null
space_editor: boolean | null
permissions:
| Database["public"]["Enums"]["SpaceAccessPermissions"]
| null
}
concept_local_input: {
epistemic_status: Database["public"]["Enums"]["EpistemicStatus"] | null
Expand Down
77 changes: 77 additions & 0 deletions packages/database/supabase/migrations/20260213155959_upsert_user_permissions.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
ALTER TYPE public.account_local_input ADD ATTRIBUTE permissions public."SpaceAccessPermissions";

CREATE OR REPLACE FUNCTION public.my_permissions_in_space(
space_id_ BIGINT
) RETURNS public."SpaceAccessPermissions"
SET search_path = ''
LANGUAGE sql
AS $$
SELECT permissions FROM public."SpaceAccess" WHERE space_id=space_id_ AND account_uid = auth.uid();
$$;

CREATE OR REPLACE FUNCTION public.upsert_account_in_space(
space_id_ BIGINT,
local_account public.account_local_input
) RETURNS BIGINT
SECURITY DEFINER
SET search_path = ''
LANGUAGE plpgsql
AS $$
DECLARE
platform_ public."Platform";
account_id_ BIGINT;
user_uid UUID;
permissions_ public."SpaceAccessPermissions";
BEGIN
SELECT platform INTO STRICT platform_ FROM public."Space" WHERE id = space_id_;
INSERT INTO public."PlatformAccount" AS pa (
account_local_id, name, platform
) VALUES (
local_account.account_local_id, local_account.name, platform_
) ON CONFLICT (account_local_id, platform) DO UPDATE SET
name = COALESCE(NULLIF(TRIM(EXCLUDED.name), ''), pa.name)
RETURNING id, dg_account INTO STRICT account_id_, user_uid;
IF user_uid IS NOT NULL THEN
-- is any permission specified in the input?
permissions_ := COALESCE(
local_account.permissions,
CASE WHEN local_account.space_editor IS true THEN 'editor' -- legacy
WHEN local_account.space_editor IS false THEN 'reader' END);
INSERT INTO public."SpaceAccess" as sa (space_id, account_uid, permissions)
VALUES (space_id_, user_uid, least(my_permissions_in_space(space_id_), COALESCE(permissions_, 'editor')))
ON CONFLICT (space_id, account_uid)
DO UPDATE SET permissions = CASE
WHEN permissions_ IS NULL THEN permissions
ELSE least(my_permissions_in_space(space_id_), permissions_)
END;
END IF;
INSERT INTO public."LocalAccess" (space_id, account_id) values (space_id_, account_id_)
ON CONFLICT (space_id, account_id)
DO NOTHING;
IF local_account.email IS NOT NULL THEN
-- TODO: how to distinguish basic untrusted from platform placeholder email?
INSERT INTO public."AgentIdentifier" as ai (account_id, value, identifier_type, trusted) VALUES (account_id_, local_account.email, 'email', COALESCE(local_account.email_trusted, false))
ON CONFLICT (value, identifier_type, account_id)
DO UPDATE SET trusted = COALESCE(local_account.email_trusted, ai.trusted, false);
END IF;
RETURN account_id_;
END;
$$;


DROP FUNCTION create_account_in_space;

CREATE OR REPLACE FUNCTION create_account_in_space(
space_id_ BIGINT,
account_local_id_ varchar,
name_ varchar,
email_ varchar = null,
email_trusted boolean = true,
permissions_ public."SpaceAccessPermissions" = 'editor'
) RETURNS BIGINT
SECURITY DEFINER
SET search_path = ''
LANGUAGE sql
AS $$
SELECT public.upsert_account_in_space(space_id_, ROW(name_, account_local_id_ ,email_, email_trusted, null, permissions_)::public.account_local_input);
$$;
31 changes: 23 additions & 8 deletions packages/database/supabase/schemas/account.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,19 @@ CREATE TYPE public.account_local_input AS (
-- local values
email VARCHAR,
email_trusted BOOLEAN,
space_editor BOOLEAN
space_editor BOOLEAN,
permissions public."SpaceAccessPermissions"
);

CREATE OR REPLACE FUNCTION public.my_permissions_in_space(
space_id_ BIGINT
) RETURNS public."SpaceAccessPermissions"
SET search_path = ''
LANGUAGE sql
AS $$
SELECT permissions FROM public."SpaceAccess" WHERE space_id=space_id_ AND account_uid = auth.uid();
$$;

CREATE OR REPLACE FUNCTION public.upsert_account_in_space(
space_id_ BIGINT,
local_account public.account_local_input
Expand All @@ -195,6 +205,7 @@ DECLARE
platform_ public."Platform";
account_id_ BIGINT;
user_uid UUID;
permissions_ public."SpaceAccessPermissions";
BEGIN
SELECT platform INTO STRICT platform_ FROM public."Space" WHERE id = space_id_;
INSERT INTO public."PlatformAccount" AS pa (
Expand All @@ -205,14 +216,18 @@ BEGIN
name = COALESCE(NULLIF(TRIM(EXCLUDED.name), ''), pa.name)
RETURNING id, dg_account INTO STRICT account_id_, user_uid;
IF user_uid IS NOT NULL THEN
-- is any permission specified in the input?
permissions_ := COALESCE(
local_account.permissions,
CASE WHEN local_account.space_editor IS true THEN 'editor' -- legacy
WHEN local_account.space_editor IS false THEN 'reader' END);
INSERT INTO public."SpaceAccess" as sa (space_id, account_uid, permissions)
VALUES (space_id_, user_uid,
CASE WHEN COALESCE(local_account.space_editor, true) THEN 'editor'
ELSE 'reader' END)
VALUES (space_id_, user_uid, least(my_permissions_in_space(space_id_), COALESCE(permissions_, 'editor')))
ON CONFLICT (space_id, account_uid)
DO UPDATE SET permissions = CASE
WHEN COALESCE(local_account.space_editor, sa.editor, true) THEN 'editor'
ELSE 'reader' END;
WHEN permissions_ IS NULL THEN permissions
ELSE least(my_permissions_in_space(space_id_), permissions_)
END;
Comment on lines 224 to +230
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Feb 14, 2026

Choose a reason for hiding this comment

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

🔴 least() with NULL from my_permissions_in_space causes NOT NULL constraint violation on INSERT

When my_permissions_in_space(space_id_) returns NULL (which happens when the calling user accesses the space through a group rather than having a direct SpaceAccess entry), least(NULL, COALESCE(permissions_, 'editor')) evaluates to NULL in PostgreSQL. This NULL is then used as the permissions value in the INSERT into SpaceAccess, which has a NOT NULL constraint on that column, causing the INSERT to fail with a constraint violation error.

Root Cause and Impact

The my_permissions_in_space function at packages/database/supabase/schemas/account.sql:192-193 only checks for direct user access:

SELECT permissions FROM public."SpaceAccess" WHERE space_id=space_id_ AND account_uid = auth.uid();

But other permission-checking functions like in_space and my_space_ids use my_user_accounts() which includes group memberships. When a user accesses a space through a group, my_permissions_in_space returns NULL.

In PostgreSQL, least(NULL, 'editor') returns NULL. The INSERT at line 225 becomes:

INSERT INTO public."SpaceAccess" ... VALUES (space_id_, user_uid, NULL)

which violates the NOT NULL constraint on permissions (packages/database/supabase/schemas/account.sql:108).

The same issue affects the ON CONFLICT UPDATE path at line 229:

ELSE least(my_permissions_in_space(space_id_), permissions_)

which would also set permissions to NULL.

Impact: Any user who accesses a space through a group membership and triggers upsert_account_in_space (e.g., during Roam sync) will get a database error, preventing account upsert operations from completing.

Prompt for agents 
The my_permissions_in_space function at packages/database/supabase/schemas/account.sql:187-194 only checks for direct user access (account_uid = auth.uid()) but does not account for group-based access. This causes it to return NULL for group members, and least(NULL, ...) returns NULL in PostgreSQL, violating the NOT NULL constraint on SpaceAccess.permissions.

Two possible fixes:

1. (Preferred) Update my_permissions_in_space to also check group memberships, similar to how in_space and my_space_ids use my_user_accounts():

   SELECT MAX(permissions) FROM public."SpaceAccess" 
   WHERE space_id=space_id_ AND account_uid IN (SELECT * FROM public.my_user_accounts());

2. (Alternative) Wrap the least() calls in upsert_account_in_space with COALESCE to handle NULL:
   - Line 225: COALESCE(least(my_permissions_in_space(space_id_), ...), COALESCE(permissions_, 'editor'))
   - Line 229: COALESCE(least(my_permissions_in_space(space_id_), permissions_), permissions_)

The same fix needs to be applied in the migration file at packages/database/supabase/migrations/20260213155959_upsert_user_permissions.sql lines 41 and 45.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

END IF;
INSERT INTO public."LocalAccess" (space_id, account_id) values (space_id_, account_id_)
ON CONFLICT (space_id, account_id)
Expand Down Expand Up @@ -257,13 +272,13 @@ CREATE OR REPLACE FUNCTION public.create_account_in_space(
name_ varchar,
email_ varchar = null,
email_trusted boolean = true,
editor_ boolean = true
permissions_ public."SpaceAccessPermissions" = 'editor'
) RETURNS BIGINT
SECURITY DEFINER
SET search_path = ''
LANGUAGE sql
AS $$
SELECT public.upsert_account_in_space(space_id_, ROW(name_, account_local_id_ ,email_, email_trusted, editor_)::public.account_local_input);
SELECT public.upsert_account_in_space(space_id_, ROW(name_, account_local_id_ ,email_, email_trusted, null, permissions_)::public.account_local_input);
$$;


Expand Down
Loading