From f3d7d63f6d813c0cd16c4ae6b51a0a9961592710 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 18 Sep 2026 10:48:08 -0700 Subject: [PATCH v1] WIP: Add a check_hook for output_plugin_libraries When adding output_plugin_libraries in 226e49cbed, I omitted explicit syntax validation. [ALTER SYSTEM] SET does this implicitly due to the parser grammar, but a DBA could still accidentally put a bad value into postgresql.conf, and a superuser could do the same with connection options. Logical replication would then fail later on. This is an annoying papercut for a security option to have, so reject these cases immediately in a check_hook. TODO: find a place to put the test that's backpatchable all the way down? Reported-by: Fujii Masao Backpatch-through: 14 --- src/include/utils/guc_hooks.h | 2 ++ src/backend/replication/logical/logical.c | 34 +++++++++++++++++++++++ src/backend/utils/misc/guc_parameters.dat | 1 + src/bin/pg_upgrade/t/003_logical_slots.pl | 13 +++++++++ 4 files changed, 50 insertions(+) diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 06453a18c03..8e3c067f760 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -91,6 +91,8 @@ extern bool check_multixact_member_buffers(int *newval, void **extra, extern bool check_multixact_offset_buffers(int *newval, void **extra, GucSource source); extern bool check_notify_buffers(int *newval, void **extra, GucSource source); +extern bool check_output_plugin_libraries(char **newval, void **extra, + GucSource source); extern bool check_primary_slot_name(char **newval, void **extra, GucSource source); extern bool check_random_seed(double *newval, void **extra, GucSource source); diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 98e5f1dd8f9..626d68b178e 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -44,6 +44,7 @@ #include "storage/procarray.h" #include "utils/builtins.h" #include "utils/guc.h" +#include "utils/guc_hooks.h" #include "utils/injection_point.h" #include "utils/inval.h" #include "utils/memutils.h" @@ -109,6 +110,39 @@ static void update_progress_txn_cb_wrapper(ReorderBuffer *cache, static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugin); +/* check_hook: validate new output_plugin_libraries value */ +bool +check_output_plugin_libraries(char **newval, void **extra, GucSource source) +{ + char *copy; + List *components; + bool ok = true; + + copy = guc_strdup(LOG, *newval); + if (!copy) + return false; + + /* + * XXX SplitGUCList won't respect guc_malloc requirements, but this is + * consistent with other check_hook implementations... + */ + if (!SplitGUCList(copy, ',', &components)) + { + GUC_check_errdetail("List syntax is invalid."); + ok = false; + } + + /* + * Like with related GUC_LIST_QUOTE variables, we check only syntax here + * and not the existence of the plugins themselves. + */ + + list_free(components); + guc_free(copy); + + return ok; +} + /* * Make sure the current settings & environment are capable of doing logical * decoding. diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index c57441f7d98..61c591314af 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -2333,6 +2333,7 @@ flags => 'GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY', variable => 'output_plugin_libraries_string', boot_val => '"pgoutput, test_decoding"', + check_hook => 'check_output_plugin_libraries', }, { name => 'parallel_leader_participation', type => 'bool', context => 'PGC_USERSET', group => 'RESOURCES_WORKER_PROCESSES', diff --git a/src/bin/pg_upgrade/t/003_logical_slots.pl b/src/bin/pg_upgrade/t/003_logical_slots.pl index 01ab82402ae..c628d51dfe6 100644 --- a/src/bin/pg_upgrade/t/003_logical_slots.pl +++ b/src/bin/pg_upgrade/t/003_logical_slots.pl @@ -66,6 +66,19 @@ $oldpub->safe_psql( SELECT pg_create_logical_replication_slot('test_slot2', 'test_decoding'); SELECT pg_create_logical_replication_slot('test_slot3', 'test_decoding'); ]); + +# check.c assumes the list syntax of output_plugin_libraries is validated by the +# server, so take a moment to confirm that now. (This is difficult to test via +# regression suite, because our SET grammar won't accept the bad syntax.) +$oldpub->connect_fails( + "options='-c output_plugin_libraries=pgoutput,'", + "server validates output_plugin_libraries syntax", + expected_stderr => qr[ + \Qinvalid value for parameter "output_plugin_libraries"\E + .* + \QList syntax is invalid.\E + ]sx); + $oldpub->stop(); # 2. Set 'max_replication_slots' to be less than the number of slots (2) -- 2.34.1