From 664911883af47e1fa1a5b1e5de240ae1e07a868d Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Wed, 16 Oct 2024 19:57:49 -0500 Subject: [PATCH 1/2] fix: permission for net tables and remove secdef --- sql/pg_net--0.11.0--0.11.1.sql | 13 +++++++ sql/pg_net.sql | 19 ++-------- test/test_privileges.py | 69 ++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 sql/pg_net--0.11.0--0.11.1.sql create mode 100644 test/test_privileges.py diff --git a/sql/pg_net--0.11.0--0.11.1.sql b/sql/pg_net--0.11.0--0.11.1.sql new file mode 100644 index 0000000..8859216 --- /dev/null +++ b/sql/pg_net--0.11.0--0.11.1.sql @@ -0,0 +1,13 @@ +alter function net.http_get(text, jsonb, jsonb, integer) security invoker; + +alter function net.http_post(text, jsonb, jsonb, jsonb, integer) security invoker; + +alter function net.http_delete ( text, jsonb, jsonb, integer) security invoker; + +alter function net._http_collect_response ( bigint, boolean) security invoker; + +alter function net.http_collect_response ( bigint, boolean) security invoker; + +grant usage on schema net to PUBLIC; +grant all on all sequences in schema net to PUBLIC; +grant all on all tables in schema net to PUBLIC; diff --git a/sql/pg_net.sql b/sql/pg_net.sql index a32f287..897ceb4 100644 --- a/sql/pg_net.sql +++ b/sql/pg_net.sql @@ -115,7 +115,6 @@ create or replace function net.http_get( volatile parallel safe language plpgsql - security definer as $$ declare request_id bigint; @@ -159,7 +158,6 @@ create or replace function net.http_post( volatile parallel safe language plpgsql - security definer as $$ declare request_id bigint; @@ -229,7 +227,6 @@ create or replace function net.http_delete( volatile parallel safe language plpgsql - security definer as $$ declare request_id bigint; @@ -290,7 +287,6 @@ create or replace function net._http_collect_response( volatile parallel safe language plpgsql - security definer as $$ declare rec net._http_response; @@ -345,7 +341,6 @@ create or replace function net.http_collect_response( volatile parallel safe language plpgsql - security definer as $$ begin raise notice 'The net.http_collect_response function is deprecated.'; @@ -353,14 +348,6 @@ begin end; $$; -create or replace function net.worker_restart() returns bool as $$ - select pg_reload_conf(); - select pg_terminate_backend(pid) - from pg_stat_activity - where backend_type ilike '%pg_net%'; -$$ -security definer -language sql; - -grant all on schema net to postgres; -grant all on all tables in schema net to postgres; +grant usage on schema net to PUBLIC; +grant all on all sequences in schema net to PUBLIC; +grant all on all tables in schema net to PUBLIC; diff --git a/test/test_privileges.py b/test/test_privileges.py new file mode 100644 index 0000000..0b1ebc5 --- /dev/null +++ b/test/test_privileges.py @@ -0,0 +1,69 @@ +import pytest +from sqlalchemy import text + +def test_net_on_postgres_role(sess): + """Check that the postgres role can use the net schema by default""" + + role = sess.execute(text("select current_user;")).fetchone() + + assert role[0] == "postgres" + + # Create a request + (request_id,) = sess.execute(text( + """ + select net.http_get( + 'http://localhost:8080/anything' + ); + """ + )).fetchone() + + # Commit so background worker can start + sess.commit() + + # Confirm that the request was retrievable + response = sess.execute( + text( + """ + select * from net._http_collect_response(:request_id, async:=false); + """ + ), + {"request_id": request_id}, + ).fetchone() + assert response[0] == "SUCCESS" + +def test_net_on_another_role(sess): + """Check that a newly created role can use the net schema""" + + sess.execute(text(""" + create role another; + """)) + + # Create a request + (request_id,) = sess.execute(text( + """ + set local role to another; + select net.http_get( + 'http://localhost:8080/anything' + ); + """ + )).fetchone() + + # Commit so background worker can start + sess.commit() + + # Confirm that the request was retrievable + response = sess.execute( + text( + """ + set local role to another; + select * from net._http_collect_response(:request_id, async:=false); + """ + ), + {"request_id": request_id}, + ).fetchone() + assert response[0] == "SUCCESS" + + sess.execute(text(""" + set local role postgres; + drop role another; + """)) From d42a943ba7a0ce14bbe55c731b45bf76be2c3998 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Thu, 17 Oct 2024 08:43:20 -0500 Subject: [PATCH 2/2] fix: worker_restart without security definer --- README.md | 2 +- sql/pg_net--0.11.0--0.11.1.sql | 5 +++++ sql/pg_net.sql | 4 ++++ src/worker.c | 41 ++++++++++++++++++++++++++++------ test/test_privileges.py | 11 +++++++++ test/test_worker_error.py | 2 ++ 6 files changed, 57 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 676b601..e0aed01 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # PG_NET *A PostgreSQL extension that enables asynchronous (non-blocking) HTTP/HTTPS requests with SQL*. -Requires libcurl >= 7.83. +Requires libcurl >= 7.83. Compatible with PostgreSQL > = 12. ![PostgreSQL version](https://img.shields.io/badge/postgresql-12+-blue.svg) [![License](https://img.shields.io/pypi/l/markdown-subtemplate.svg)](https://github.com/supabase/pg_net/blob/master/LICENSE) diff --git a/sql/pg_net--0.11.0--0.11.1.sql b/sql/pg_net--0.11.0--0.11.1.sql index 8859216..1d8c7e5 100644 --- a/sql/pg_net--0.11.0--0.11.1.sql +++ b/sql/pg_net--0.11.0--0.11.1.sql @@ -8,6 +8,11 @@ alter function net._http_collect_response ( bigint, boolean) security invoker; alter function net.http_collect_response ( bigint, boolean) security invoker; +create or replace function net.worker_restart() + returns bool + language 'c' +as 'pg_net'; + grant usage on schema net to PUBLIC; grant all on all sequences in schema net to PUBLIC; grant all on all tables in schema net to PUBLIC; diff --git a/sql/pg_net.sql b/sql/pg_net.sql index 897ceb4..f4405ab 100644 --- a/sql/pg_net.sql +++ b/sql/pg_net.sql @@ -96,6 +96,10 @@ create or replace function net._encode_url_with_params_array(url text, params_ar immutable as 'pg_net'; +create or replace function net.worker_restart() + returns bool + language 'c' +as 'pg_net'; -- Interface to make an async request -- API: Public diff --git a/src/worker.c b/src/worker.c index c320479..b52083a 100644 --- a/src/worker.c +++ b/src/worker.c @@ -30,6 +30,8 @@ #include #include +#include + #include "util.h" #include "core.h" @@ -40,17 +42,25 @@ _Static_assert(LIBCURL_VERSION_NUM >= MIN_LIBCURL_VERSION_NUM, REQUIRED_LIBCURL_ PG_MODULE_MAGIC; -static char *guc_ttl; -static int guc_batch_size; -static char* guc_database_name; -static MemoryContext CurlMemContext = NULL; +static char* guc_ttl; +static int guc_batch_size; +static char* guc_database_name; +static MemoryContext CurlMemContext = NULL; +static shmem_startup_hook_type prev_shmem_startup_hook = NULL; +static long latch_timeout = 1000; +static volatile sig_atomic_t got_sigterm = false; +static volatile sig_atomic_t got_sighup = false; +static bool* restart_worker = NULL; void _PG_init(void); PGDLLEXPORT void pg_net_worker(Datum main_arg) pg_attribute_noreturn(); -static long latch_timeout = 1000; -static volatile sig_atomic_t got_sigterm = false; -static volatile sig_atomic_t got_sighup = false; +PG_FUNCTION_INFO_V1(worker_restart); +Datum worker_restart(PG_FUNCTION_ARGS) { + bool result = DatumGetBool(DirectFunctionCall1(pg_reload_conf, (Datum) NULL)); // reload the config + *restart_worker = true; + PG_RETURN_BOOL(result && *restart_worker); // TODO is not necessary to return a bool here, but we do it to maintain backward compatibility +} static void handle_sigterm(SIGNAL_ARGS) @@ -141,6 +151,12 @@ void pg_net_worker(Datum main_arg) { ProcessConfigFile(PGC_SIGHUP); } + if (restart_worker && *restart_worker) { + *restart_worker = false; + elog(INFO, "Restarting pg_net worker"); + break; + } + delete_expired_responses(guc_ttl, guc_batch_size); consume_request_queue(lstate.curl_mhandle, guc_batch_size, CurlMemContext); @@ -206,6 +222,14 @@ void pg_net_worker(Datum main_arg) { proc_exit(EXIT_FAILURE); } +static void net_shmem_startup(void) { + if (prev_shmem_startup_hook) + prev_shmem_startup_hook(); + + restart_worker = ShmemAlloc(sizeof(bool)); + *restart_worker = false; +} + void _PG_init(void) { if (IsBinaryUpgrade) { return; @@ -226,6 +250,9 @@ void _PG_init(void) { .bgw_restart_time = 1, }); + prev_shmem_startup_hook = shmem_startup_hook; + shmem_startup_hook = net_shmem_startup; + CurlMemContext = AllocSetContextCreate(TopMemoryContext, "pg_net curl context", ALLOCSET_DEFAULT_MINSIZE, diff --git a/test/test_privileges.py b/test/test_privileges.py index 0b1ebc5..7a17638 100644 --- a/test/test_privileges.py +++ b/test/test_privileges.py @@ -63,6 +63,17 @@ def test_net_on_another_role(sess): ).fetchone() assert response[0] == "SUCCESS" + ## can use the net.worker_restart function + response = sess.execute( + text( + """ + set local role to another; + select net.worker_restart(); + """ + ) + ).fetchone() + assert response[0] == True + sess.execute(text(""" set local role postgres; drop role another; diff --git a/test/test_worker_error.py b/test/test_worker_error.py index 5791e1f..debe4a0 100644 --- a/test/test_worker_error.py +++ b/test/test_worker_error.py @@ -5,6 +5,8 @@ def test_success_when_worker_is_up(sess): """net.check_worker_is_up should not return anything when the worker is running""" + time.sleep(1) # wait if another test did a net.worker_restart() + (result,) = sess.execute(text(""" select net.check_worker_is_up(); """)).fetchone()