public inbox for [email protected]  
help / color / mirror / Atom feed
From: Chao Li <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: Jim Jones <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Subject: Re: Avoid leaking system path from pg_available_extensions
Date: Tue, 26 May 2026 15:14:25 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>



> On May 22, 2026, at 23:40, Matheus Alcantara <[email protected]> wrote:
> 
> On 22/05/26 04:25, Jim Jones wrote:
>> On 21/05/2026 17:12, Matheus Alcantara wrote:
>>> I've reproduced the issue and the fix looks correct to me.
>> same here, +1
> 
> Thank you for also testing.
> 
>> I was wondering if creating a constant for it would be, stylistically
>> speaking, a cleaner solution. For instance:
>> #define EXTENSION_SYSTEM_MACRO  "$system"
>> I realize that it's used only inside get_extension_control_directories()
>> but since it is even mentioned in the docs, I guess it wouldn't be a bad
>> idea.
> 
> I'm not against it but I don't think that it's necessary since as you mention, only get_extension_control_directories() use.
> 
> --
> Matheus Alcantara
> EDB: https://www.enterprisedb.com

In theory, I’m not against the idea either. In practice, there are many hard-coded strings in the source tree, and I’m not sure where the right place would be to define this macro.

Since this string is only used in get_extension_control_directories(), and now it is used three times, I defined it at the beginning of the function and undefined it at the end. Let’s see if there are any objections to that.

Please see the attached v2.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Attachments:

  [application/octet-stream] v2-0001-Avoid-leaking-system-path-from-pg_available_exten.patch (3.6K, 2-v2-0001-Avoid-leaking-system-path-from-pg_available_exten.patch)
  download | inline diff:
From eaf271a14944df7df87601827f5f5e4909e38c33 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 20 May 2026 08:49:15 +0800
Subject: [PATCH v2] Avoid leaking system path from pg_available_extensions

The documentation says that when extension_control_path is set to an
empty string, the default '$system' path is still assumed.  However,
get_extension_control_directories() added the system extension directory
with a NULL macro in that case.  As a result, pg_available_extensions
could expose the expanded system directory path instead of reporting
'$system' as the location.

Record the implicitly-added system directory with the '$system' macro, so
pg_available_extensions reports the documented symbolic location and does
not leak the actual system path.

Update the extension_control_path TAP test to check the reported location
directly.

Author: Chao Li <[email protected]>
Reviewed-by: Lu Feng <[email protected]>
Reviewed-by: Matheus Alcantara <[email protected]>
Reviewed-by: Jim Jones <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/commands/extension.c                          | 8 +++++---
 .../test_extensions/t/001_extension_control_path.pl       | 6 +++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index a330b5fd6ce..d073585c421 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -513,6 +513,7 @@ is_extension_script_filename(const char *filename)
 static List *
 get_extension_control_directories(void)
 {
+#define EXTENSION_SYSTEM_MACRO  "$system"
 	char		sharepath[MAXPGPATH];
 	char	   *system_dir;
 	char	   *ecp;
@@ -526,7 +527,7 @@ get_extension_control_directories(void)
 	{
 		ExtensionLocation *location = palloc_object(ExtensionLocation);
 
-		location->macro = NULL;
+		location->macro = pstrdup(EXTENSION_SYSTEM_MACRO);
 		location->loc = system_dir;
 		paths = lappend(paths, location);
 	}
@@ -556,10 +557,10 @@ get_extension_control_directories(void)
 			 * Substitute the path macro if needed or append "extension"
 			 * suffix if it is a custom extension control path.
 			 */
-			if (strcmp(piece, "$system") == 0)
+			if (strcmp(piece, EXTENSION_SYSTEM_MACRO) == 0)
 			{
 				location->macro = pstrdup(piece);
-				mangled = substitute_path_macro(piece, "$system", system_dir);
+				mangled = substitute_path_macro(piece, EXTENSION_SYSTEM_MACRO, system_dir);
 			}
 			else
 			{
@@ -582,6 +583,7 @@ get_extension_control_directories(void)
 	}
 
 	return paths;
+#undef EXTENSION_SYSTEM_MACRO
 }
 
 /*
diff --git a/src/test/modules/test_extensions/t/001_extension_control_path.pl b/src/test/modules/test_extensions/t/001_extension_control_path.pl
index c1cec0dc622..4a013a7da4b 100644
--- a/src/test/modules/test_extensions/t/001_extension_control_path.pl
+++ b/src/test/modules/test_extensions/t/001_extension_control_path.pl
@@ -109,10 +109,10 @@ is($ret, "t",
 	"\$system extension is shown correctly in pg_available_extensions");
 
 $ret = $node->safe_psql('postgres',
-	"set extension_control_path = ''; select count(*) > 0 as ok from pg_available_extensions where name = 'plpgsql'"
+	"set extension_control_path = ''; select location from pg_available_extensions where name = 'plpgsql'"
 );
-is($ret, "t",
-	"\$system extension is shown correctly in pg_available_extensions with empty extension_control_path"
+is($ret, "\$system",
+	"\$system location is shown correctly in pg_available_extensions with empty extension_control_path"
 );
 
 # Test with an extension that does not exists
-- 
2.50.1 (Apple Git-155)



reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Avoid leaking system path from pg_available_extensions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox