Feedback-ID: i19364b5b:Fastmail
From: "Jonathan Gonzalez V." <jonathan@abdiel.eu>
To: pgsql-hackers@lists.postgresql.org
Subject: Introduce psystem() to replace system()
Date: Mon, 10 Aug 2026 21:27:51 +0200
Message-ID: <87zeyt93ns.fsf@abdiel.eu>
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87zeyt93ns.fsf%40abdiel.eu>
Precedence: bulk

--=-=-=
Content-Type: text/plain


Hello!!

For some time I've been wondering why PostgreSQL needs system() calls,
which use a shell that can lead to many problems, and also why it
requires a shell to run a command.

I first started thinking about this when I was trying to run a full
distroless PostgreSQL container. It turns out that isn't possible since
the shell is a requirement, and distroless containers are secure exactly
_because_ there's no shell to execute any command other than the ones
that are meant to be executed.

After some research I found out that using system() has other problems,
like issues related to quoting that are really painful to solve [0][1],
and also the exit codes control[2]. Both topics have already been
discussed on the list.

But the main argument now for me is security. Not having a shell avoids
any possible PATH injection, missing quoting to escape a command, or new
lines that the shell interprets differently from what you'd expect.

After some thinking I came up with a small interface, which only purpose
is to replace system() calls in a more smooth way using execv() under
the hood. I suppose you could use execl() but I've decided to keep it
simple, leaving the opportunity to expand in the future. I already
implemented one call with `pg_ctl initdb` as an example.

There's an important topic related to using shell versus not a shell. In
some places like `archive_command` people may use `&&`, but this idea
aims to avoid this kind of behavior since it's not secure. Probably we
can implement a way to run commands in sequence, or simply tell the
users that this isn't allowed anymore, but it's possible to trigger
commands in sequence since the interface allows to manipulate the STDIN
and STDOUT.

I would like to open the discussion here if this is the right direction.
There's a lot to do and this still a work in progress, the current patch
is small and simple, but already provides building blocks in this direction.

[0] https://www.postgresql.org/message-id/7606.1153326421%40sss.pgh.pa.us
[1] https://www.postgresql.org/message-id/CA%2BTgmobBmWWCgPUd04NGoQ%3D_XvcidV%2BsE2F7KChEXfs8KBPg6w%40mail.gmail.com
[2] https://www.postgresql.org/message-id/21292.1358698487%40sss.pgh.pa.us

--
Jonathan Gonzalez V.
EDB https://enterprisedb.com

--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0001-replace-calls-to-system-with-PostgreSQL-own-implemen.patch

From 22707f3f0c5a160d9483136a198b64dc1fbb9d82 Mon Sep 17 00:00:00 2001
From: "Jonathan Gonzalez V." <jonathan@abdiel.eu>
Date: Mon, 3 Aug 2026 15:21:45 -0400
Subject: [PATCH 1/1] replace calls to system() with PostgreSQL own
 implementation

Introduce PCommand and psystem() to execute commands from argument
arrays, avoiding the shell on Unix.

Windows implementation still on a work in progress
---
 src/backend/utils/init/postinit.c |  58 ---------
 src/bin/pg_ctl/pg_ctl.c           |  30 ++---
 src/common/Makefile               |   1 +
 src/common/exec.c                 |  59 +++++++++
 src/common/meson.build            |   2 +
 src/common/pg_exec.c              | 206 ++++++++++++++++++++++++++++++
 src/include/common/pg_exec.h      |  39 ++++++
 src/include/miscadmin.h           |   1 -
 src/include/port.h                |   2 +
 9 files changed, 324 insertions(+), 74 deletions(-)
 create mode 100644 src/common/pg_exec.c
 create mode 100644 src/include/common/pg_exec.h

diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 3d8c9bdebd5..78ea7d56022 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -492,64 +492,6 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
 	ReleaseSysCache(tup);
 }
 
-
-/*
- * pg_split_opts -- split a string of options and append it to an argv array
- *
- * The caller is responsible for ensuring the argv array is large enough.  The
- * maximum possible number of arguments added by this routine is
- * (strlen(optstr) + 1) / 2.
- *
- * Because some option values can contain spaces we allow escaping using
- * backslashes, with \\ representing a literal backslash.
- */
-void
-pg_split_opts(char **argv, int *argcp, const char *optstr)
-{
-	StringInfoData s;
-
-	initStringInfo(&s);
-
-	while (*optstr)
-	{
-		bool		last_was_escape = false;
-
-		resetStringInfo(&s);
-
-		/* skip over leading space */
-		while (isspace((unsigned char) *optstr))
-			optstr++;
-
-		if (*optstr == '\0')
-			break;
-
-		/*
-		 * Parse a single option, stopping at the first space, unless it's
-		 * escaped.
-		 */
-		while (*optstr)
-		{
-			if (isspace((unsigned char) *optstr) && !last_was_escape)
-				break;
-
-			if (!last_was_escape && *optstr == '\\')
-				last_was_escape = true;
-			else
-			{
-				last_was_escape = false;
-				appendStringInfoChar(&s, *optstr);
-			}
-
-			optstr++;
-		}
-
-		/* now store the option in the next argv[] position */
-		argv[(*argcp)++] = pstrdup(s.data);
-	}
-
-	pfree(s.data);
-}
-
 /*
  * Initialize MaxBackends value from config options.
  *
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 6c604e2d962..19e884bac1f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -25,6 +25,7 @@
 #include "common/controldata_utils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/pg_exec.h"
 #include "common/string.h"
 #include "datatype/timestamp.h"
 #include "getopt_long.h"
@@ -42,6 +43,7 @@ typedef enum
 	IMMEDIATE_MODE,
 } ShutdownMode;
 
+
 typedef enum
 {
 	POSTMASTER_READY,
@@ -904,26 +906,18 @@ find_other_exec_or_die(const char *argv0, const char *target, const char *versio
 static void
 do_init(void)
 {
-	char	   *cmd;
+	PCommand   *cmd;
 
 	if (exec_path == NULL)
 		exec_path = find_other_exec_or_die(argv0, "initdb", "initdb (PostgreSQL) " PG_VERSION "\n");
 
-	if (pgdata_opt == NULL)
-		pgdata_opt = "";
-
-	if (post_opts == NULL)
-		post_opts = "";
-
-	if (!silent_mode)
-		cmd = psprintf("\"%s\" %s%s",
-					   exec_path, pgdata_opt, post_opts);
-	else
-		cmd = psprintf("\"%s\" %s%s > \"%s\"",
-					   exec_path, pgdata_opt, post_opts, DEVNULL);
+	cmd = pcommand_init(exec_path, "initdb");
+	pcommand_append_arg(cmd, pgdata_opt);
+	pcommand_append_arg(cmd, post_opts);
+	cmd->silent = silent_mode;
 
 	fflush(NULL);
-	if (system(cmd) != 0)
+	if (psystem(cmd) != 0)
 	{
 		write_stderr(_("%s: database system initialization failed\n"), progname);
 		exit(1);
@@ -2155,6 +2149,12 @@ adjust_data_dir(void)
 	else
 		my_exec_path = pg_strdup(exec_path);
 
+/*
+ * command = pcommand_init(my_exec_path, "postgres");
+ * pcommand_append_arg(command, "-C data_directory");
+ * pcommand_append_arg(command, pgdata_opt);
+ * pcommand_append_arg(command, post_opts);
+ */
 	/* it's important for -C to be the first option, see main.c */
 	cmd = psprintf("\"%s\" -C data_directory %s%s",
 				   my_exec_path,
@@ -2287,7 +2287,7 @@ main(int argc, char **argv)
 					 * We could pass PGDATA just in an environment variable
 					 * but we do -D too for clearer postmaster 'ps' display
 					 */
-					pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
+					pgdata_opt = psprintf("-D %s ", pgdata_D);
 					pg_free(pgdata_D);
 					break;
 				}
diff --git a/src/common/Makefile b/src/common/Makefile
index 1a2fbbe887f..753399f023a 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -68,6 +68,7 @@ OBJS_COMMON = \
 	md5_common.o \
 	parse_manifest.o \
 	percentrepl.o \
+	pg_exec.o \
 	pg_get_line.o \
 	pg_lzcompress.o \
 	pg_prng.o \
diff --git a/src/common/exec.c b/src/common/exec.c
index 2881aa92ca6..c5a31c17a95 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -33,6 +33,7 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <fcntl.h>
 
 #ifdef EXEC_BACKEND
 #if defined(HAVE_SYS_PERSONALITY_H)
@@ -43,6 +44,7 @@
 #endif
 
 #include "common/string.h"
+#include "lib/stringinfo.h"
 
 /* Inhibit mingw CRT's auto-globbing of command line arguments */
 #if defined(WIN32) && !defined(_MSC_VER)
@@ -711,3 +713,60 @@ GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser)
 }
 
 #endif
+
+/*
+ * pg_split_opts -- split a string of options and append it to an argv array
+ *
+ * The caller is responsible for ensuring the argv array is large enough.  The
+ * maximum possible number of arguments added by this routine is
+ * (strlen(optstr) + 1) / 2.
+ *
+ * Because some option values can contain spaces we allow escaping using
+ * backslashes, with \\ representing a literal backslash.
+ */
+void
+pg_split_opts(char **argv, int *argcp, const char *optstr)
+{
+	StringInfoData s;
+
+	initStringInfo(&s);
+
+	while (*optstr)
+	{
+		bool		last_was_escape = false;
+
+		resetStringInfo(&s);
+
+		/* skip over leading space */
+		while (isspace((unsigned char) *optstr))
+			optstr++;
+
+		if (*optstr == '\0')
+			break;
+
+		/*
+		 * Parse a single option, stopping at the first space, unless it's
+		 * escaped.
+		 */
+		while (*optstr)
+		{
+			if (isspace((unsigned char) *optstr) && !last_was_escape)
+				break;
+
+			if (!last_was_escape && *optstr == '\\')
+				last_was_escape = true;
+			else
+			{
+				last_was_escape = false;
+				appendStringInfoChar(&s, *optstr);
+			}
+
+			optstr++;
+		}
+
+		/* now store the option in the next argv[] position */
+		argv[(*argcp)++] = pstrdup(s.data);
+	}
+
+	pfree(s.data);
+}
diff --git a/src/common/meson.build b/src/common/meson.build
index 9bd55cda95b..06ebb4dd874 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -22,6 +22,7 @@ common_sources = files(
   'md5_common.c',
   'parse_manifest.c',
   'percentrepl.c',
+	'pg_exec.c',
   'pg_get_line.c',
   'pg_lzcompress.c',
   'pg_prng.c',
@@ -190,6 +191,7 @@ foreach name, opts : pgcommon_variants
       kwargs: opts + {
         'include_directories': [
           include_directories('.'),
+					include_directories('../interfaces/libpq'),
           opts.get('include_directories', []),
         ],
         'dependencies': opts['dependencies'] + [ssl],
diff --git a/src/common/pg_exec.c b/src/common/pg_exec.c
new file mode 100644
index 00000000000..bb5613216f2
--- /dev/null
+++ b/src/common/pg_exec.c
@@ -0,0 +1,206 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_exec.c
+ *              Functions fo execute and manage the input/output of commands
+ *
+ *
+ * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *        src/common/pg_exec.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include <unistd.h>
+#include <sys/wait.h>
+#include <fcntl.h>
+
+#include "c.h"
+
+#include "postgres.h"
+#include "common/pg_exec.h"
+#include "utils/palloc.h"
+
+#ifdef WIN32
+#include "fe_utils/string_utils.h"
+#endif
+
+static int
+pcommand_count_args(const char *arg)
+{
+	int			count = 0;
+	bool		in_word = false;
+
+	Assert(arg != NULL);
+
+	while (*arg)
+	{
+		if (isspace((unsigned char) *arg))
+			in_word = false;
+		else if (!in_word)
+		{
+			count++;
+			in_word = true;
+		}
+
+		arg++;
+	}
+
+	return count;
+}
+
+void
+pcommand_append_arg(PCommand * cmd, const char *arg)
+{
+	int			args_count;
+
+	if (arg == NULL)
+		return;
+
+	args_count = pcommand_count_args(arg);
+
+	if (args_count == 0)
+		return;
+
+	if (cmd->argc + args_count >= cmd->nalloc)
+	{
+		cmd->nalloc += args_count + 1;
+		cmd->argv = repalloc_array(cmd->argv, char *, cmd->nalloc);
+	}
+
+	if (args_count > 1)
+	{
+		pg_split_opts(cmd->argv, &cmd->argc, arg);
+		cmd->argv[cmd->argc] = NULL;
+	}
+	else
+	{
+
+		cmd->argv[cmd->argc++] = pstrdup(arg);
+		cmd->argv[cmd->argc] = NULL;
+	}
+}
+
+
+#ifdef WIN32
+int
+pcommand_win(PCommand * cmd)
+{
+	PQExpBufferData cmd_str;
+	int			i;
+
+	initPQExpBuffer(&cmd_str);
+
+	appendShellString(&cmd_str, cmd->path);
+
+	for (i = 1; i < cmd->argc; i++)
+	{
+		appendPQExpBufferChar(&cmd_str, ' ');
+		appendShellString(&cmd_str, cmd->argv[i]);
+	}
+
+	if (cmd->silent)
+	{
+		appendPQExpBufferStr(&cmd_str, " > ");
+		appendShellString(&cmd_str, DEVNULL);
+	}
+
+	return system(cmd_str.data);
+}
+#endif
+
+PCommand *
+pcommand_init(char *path, char *command)
+{
+	PCommand   *cmd = palloc(sizeof(PCommand));
+
+	cmd->path = path;
+	cmd->command = command;
+
+	cmd->argc = 0;
+	cmd->nalloc = 2;
+	cmd->argv = palloc_array(char *, cmd->nalloc);
+
+	pcommand_append_arg(cmd, command);
+
+	cmd->stdin_fd = -1;
+	cmd->stdout_fd = -1;
+	cmd->stderr_fd = -1;
+
+	return cmd;
+}
+
+int
+pcommand_exec(PCommand * cmd)
+{
+	if (cmd->stdin_fd >= 0 && dup2(cmd->stdin_fd, STDIN_FILENO) < 0)
+		return errno;
+	if (cmd->stdout_fd >= 0 && dup2(cmd->stdout_fd, STDOUT_FILENO) < 0)
+		return errno;
+	if (cmd->stderr_fd >= 0 && dup2(cmd->stderr_fd, STDERR_FILENO) < 0)
+		return errno;
+
+	execv(cmd->path, cmd->argv);
+	return errno;
+}
+
+
+#ifndef WIN32
+int
+pcommand_wait(PCommand * cmd)
+{
+	int			status = -1;
+
+	/* This should have some kind of timeout... do we have that somewhere else */
+	while (true)
+	{
+		if (waitpid(cmd->pid, &status, 0) < 0)
+		{
+			if (errno == EINTR)
+				continue;
+			return status;
+		}
+		else
+		{
+			return status;
+		}
+	}
+}
+#endif
+
+/*
+ * psystem() is a replacement for system(3) that avoids the use of a shell
+ * It's not a full replacement yet since it doesn't support pipes. For now it
+ * should be used only in places where is known that no pipe is requried and the
+ * commands being call are known commands.
+ * This funciton shouldn't be used as a replacement for system(3) call in places
+ * like RestoreArchivedFile() or shell_archive_file()
+ *
+ * The return value is always the return of the executed command
+ */
+int
+psystem(PCommand * cmd)
+{
+#ifdef WIN32
+	return pcommand_win(cmd);
+#else
+
+	cmd->pid = fork();
+	if (cmd->pid < 0)
+		return -1;
+
+	if (cmd->pid == 0)
+	{
+		if (cmd->silent)
+			cmd->stdout_fd = open(DEVNULL, O_WRONLY);
+
+		errno = pcommand_exec(cmd);
+		_exit(127);
+	}
+
+	return pcommand_wait(cmd);
+#endif
+}
diff --git a/src/include/common/pg_exec.h b/src/include/common/pg_exec.h
new file mode 100644
index 00000000000..d987449c615
--- /dev/null
+++ b/src/include/common/pg_exec.h
@@ -0,0 +1,39 @@
+/*-------------------------------------------------------------------------
+ * Exec commands for backend/frontend programs
+ *
+ * Copyright (c) 2018-2026, PostgreSQL Global Development Group
+ *
+ * src/include/common/pg_exec.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COMMON_PG_EXEC_H
+#define COMMON_PG_EXEC_H
+
+typedef struct PCommand
+{
+	pid_t		pid;
+	char	   *path;
+	char	   *command;
+	char	   *command_win;
+	char	  **argv;
+	int			argc;
+	int			nalloc;
+	bool		silent;
+	int			stdin_fd;
+	int			stdout_fd;
+	int			stderr_fd;
+}			PCommand;
+
+extern PCommand * pcommand_init(char *path, char *command);
+extern void pcommand_append_arg(PCommand * cmd, const char *arg);
+#ifdef WIN32
+extern int pcommand_win(PCommand *cmd);
+#endif
+#ifndef WIN32
+extern int pcommand_wait(PCommand *cmd);
+#endif
+extern int	pcommand_exec(PCommand * cmd);
+extern int	psystem(PCommand * command);
+
+#endif
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0fc59af02b9..41d4c62c531 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -512,7 +512,6 @@ extern PGDLLIMPORT ProcessingMode Mode;
 #define INIT_PG_LOAD_SESSION_LIBS		0x0001
 #define INIT_PG_OVERRIDE_ALLOW_CONNS	0x0002
 #define INIT_PG_OVERRIDE_ROLE_LOGIN		0x0004
-extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
 extern void InitializeMaxBackends(void);
 extern void InitializeFastPathLocks(void);
 extern void InitPostgres(const char *in_dbname, Oid dboid,
diff --git a/src/include/port.h b/src/include/port.h
index 172acf7d02f..82d900fcbc4 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -140,6 +140,8 @@ extern int	find_my_exec(const char *argv0, char *retpath);
 extern int	find_other_exec(const char *argv0, const char *target,
 							const char *versionstr, char *retpath);
 extern char *pipe_read_line(char *cmd);
+extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
+
 
 /* Doesn't belong here, but this is used with find_other_exec(), so... */
 #define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n"
-- 
2.53.0


--=-=-=--