Skip to content

Commit

Permalink
fix: allow members of source table to create vectorizer
Browse files Browse the repository at this point in the history
In f68e73a we added support for cascading the source/target table drop
to the queue and other dependent objects. In adding this, we added a
security definer function which asserts that the owner of the source
table is calling the security definer function.

Due to how security definer functions work with `current_user`, the
ownership check opted to use `session_user` to check ownership. This is
problematic because in connections which use `SET ROLE`, the
`session_user` may not be the same as `current_user`.

This commit relaxes the ownership check to check whether the current
user is a member of the role that owns the source table.
  • Loading branch information
JamesGuthrie committed Nov 26, 2024
1 parent df72b08 commit 3953779
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 4 deletions.
4 changes: 2 additions & 2 deletions projects/extension/sql/idempotent/012-vectorizer-int.sql
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ begin
where v.id operator(pg_catalog.=) vectorizer_id
;

-- don't let anyone but the owner of the source table call this
select k.relowner operator(pg_catalog.=) pg_catalog.session_user()::pg_catalog.regrole
-- don't let anyone but the owner (or members of the owner's role) of the source table call this
select pg_catalog.pg_has_role(pg_catalog.session_user(), k.relowner, 'MEMBER')
into strict _is_owner
from pg_catalog.pg_class k
inner join pg_catalog.pg_namespace n on (k.relnamespace operator(pg_catalog.=) n.oid)
Expand Down
3 changes: 1 addition & 2 deletions projects/extension/sql/idempotent/013-vectorizer-api.sql
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,13 @@ begin
end if;

-- get source table name and schema name
select k.relname, n.nspname, k.relowner operator(pg_catalog.=) current_user::regrole
select k.relname, n.nspname, pg_catalog.pg_has_role(pg_catalog.current_user(), k.relowner, 'MEMBER')
into strict _source_table, _source_schema, _is_owner
from pg_catalog.pg_class k
inner join pg_catalog.pg_namespace n on (k.relnamespace operator(pg_catalog.=) n.oid)
where k.oid operator(pg_catalog.=) source
;

-- TODO: consider allowing (in)direct members of the role that owns the source table
if not _is_owner then
raise exception 'only the owner of the source table may create a vectorizer on it';
end if;
Expand Down
47 changes: 47 additions & 0 deletions projects/extension/tests/privileges/test_privileges.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,50 @@ def test_secret_privileges():
)
with pytest.raises(Exception, match="user does not have access"):
cur.execute("select ai.reveal_secret('OPENAI_API_KEY')")


def test_create_vectorizer_privileges():
# set up role "base" and role "member", which is member of base
with psycopg.connect(db_url("postgres", "postgres"), autocommit=True) as con:
with con.cursor() as cur:
cur.execute("drop database if exists vec_priv;")
cur.execute(
"""
drop role if exists member;
drop role if exists base;
create role base with login;
create role member with login;
grant base to member;
"""
)
cur.execute("create database vec_priv owner base;")
# connect as "base", create vectorizer
with psycopg.connect(db_url("base", "vec_priv")) as con:
with con.cursor() as cur:
cur.execute(
"""
create extension ai cascade;
create table blog(id bigint primary key, content text);
select ai.create_vectorizer(
'blog'
, destination=>'base_vectorizer'
, embedding=>ai.embedding_openai('text-embedding-3-small', 768)
, chunking=>ai.chunking_character_text_splitter('content', 128, 10)
, scheduling=>ai.scheduling_none()
, indexing=>ai.indexing_none()
);
"""
)
# connect as "member", create vectorizer
with psycopg.connect(db_url("member", "vec_priv")) as con:
with con.cursor() as cur:
cur.execute("""
select ai.create_vectorizer(
'blog'
, destination=>'member_vectorizer'
, embedding=>ai.embedding_openai('text-embedding-3-small', 768)
, chunking=>ai.chunking_character_text_splitter('content', 128, 10)
, scheduling=>ai.scheduling_none()
, indexing=>ai.indexing_none()
);
""")

0 comments on commit 3953779

Please sign in to comment.