v8-0002-Add-allow_alter_system-GUC.patch

From ecc65307eb448fd64afded0ab88f686cfaa1c961 Mon Sep 17 00:00:00 2001
From: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Date: Fri, 8 Sep 2023 20:25:36 +0200
Subject: [PATCH v8 2/2] Add allow_alter_system GUC

Introduce the allow_alter_system GUC (by default set to on). This GUC
can be used as a guard rail to prevent accidental usage of ALTER SYSTEM
in environments where that is not the intended way to make global
configuration changes.

Author: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
---
 doc/src/sgml/config.sgml                      | 42 +++++++++++++++++++
 src/backend/utils/misc/guc.c                  |  9 ++++
 src/backend/utils/misc/guc_tables.c           | 17 ++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/utils/guc.h                       |  1 +
 src/include/utils/guc_tables.h                |  3 ++
 6 files changed, 73 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 65a6e6c4086..3916bd4c592 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10767,6 +10767,48 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-allow-alter-system" xreflabel="allow_alter_system">
+      <term><varname>allow_alter_system</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>allow_alter_system</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        When <literal>allow_alter_system</literal> is set to
+        <literal>off</literal>, an error is returned if the <command>ALTER
+        SYSTEM</command> command is used. This parameter can only be set in
+        the <filename>postgresql.conf</filename> file or on the server command
+        line. The default value is <literal>on</literal>.
+       </para>
+
+       <para>
+        Note that this setting cannot be regarded as a security feature. It
+        only disables the <literal>ALTER SYSTEM</literal> command. It does not
+        prevent a superuser from changing the configuration using other SQL
+        commands. A superuser has many ways of executing shell commands at
+        the operating system level, and can therefore modify
+        <literal>postgresql.auto.conf</literal> regardless of the value of
+        this setting. The purpose of the setting is to prevent
+        <emphasis>accidental</emphasis> modifications via <literal>ALTER
+        SYSTEM</literal> in environments where the
+        configuration of <productname>PostgreSQL</productname> is managed by
+        some outside mechanism. In such environments, <command>ALTER
+        SYSTEM</command> might appear to correctly update the configuration,
+        but then may be discarded at some point in the future when that outside
+        mechanism updates the configuration. Setting this parameter to
+        <literal>off</literal> can help to avoid such mistakes.
+       </para>
+
+       <para>
+        This parameter only controls the use of <command>ALTER SYSTEM</command>.
+        The settings stored in <filename>postgresql.auto.conf</filename> always
+        take effect, even if <literal>allow_alter_system</literal> is set to
+        <literal>off</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      </variablelist>
     </sect2>
    </sect1>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 391866145ee..df8f08ee68d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4563,6 +4563,15 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 	 */
 	name = altersysstmt->setstmt->name;
 
+	if (!AllowAlterSystem)
+	{
+
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("ALTER SYSTEM is not allowed in this environment"),
+				 errhint("Global configuration changes should be made through a configuration system outside of PostgreSQL, not by using ALTER SYSTEM.")));
+	}
+
 	switch (altersysstmt->setstmt->kind)
 	{
 		case VAR_SET_VALUE:
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index abd9029451f..92fcd5fa4d5 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -494,6 +494,7 @@ extern const struct config_enum_entry dynamic_shared_memory_options[];
 /*
  * GUC option variables that are exported from this module
  */
+bool		AllowAlterSystem = true;
 bool		log_duration = false;
 bool		Debug_print_plan = false;
 bool		Debug_print_parse = false;
@@ -1040,6 +1041,22 @@ struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
+	{
+		/*
+		 * This setting itself cannot be set by ALTER SYSTEM to avoid an
+		 * operator turning this setting off by using ALTER SYSTEM, without a
+		 * way to turn it back on.
+		 */
+		{"allow_alter_system", PGC_SIGHUP, COMPAT_OPTIONS_OTHER,
+			gettext_noop("Allows running the ALTER SYSTEM command."),
+			gettext_noop("Can be set to off for environments where global configuration "
+						 "changes should be made using a different method."),
+			GUC_DISALLOW_IN_AUTO_FILE
+		},
+		&AllowAlterSystem,
+		true,
+		NULL, NULL, NULL
+	},
 	{
 		{"bonjour", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
 			gettext_noop("Enables advertising the server via Bonjour."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2244ee52f79..adcc0257f91 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -805,6 +805,7 @@
 # - Other Platforms and Clients -
 
 #transform_null_equals = off
+#allow_alter_system = on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 3712aba09b0..8d1fe04078a 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -254,6 +254,7 @@ extern PGDLLIMPORT bool log_btree_build_stats;
 extern PGDLLIMPORT bool check_function_bodies;
 extern PGDLLIMPORT bool current_role_is_superuser;
 
+extern PGDLLIMPORT bool AllowAlterSystem;
 extern PGDLLIMPORT bool log_duration;
 extern PGDLLIMPORT int log_parameter_max_length;
 extern PGDLLIMPORT int log_parameter_max_length_on_error;
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 0c0277c4230..bfc457bc55c 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -320,4 +320,7 @@ extern char *config_enum_get_options(struct config_enum *record,
 									 const char *suffix,
 									 const char *separator);
 
+/* GUC reference to enable/disable alter system */
+extern PGDLLIMPORT bool AllowAlterSystem;
+
 #endif							/* GUC_TABLES_H */
-- 
2.34.1