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> (raw)
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%40ma...
[2] https://www.postgresql.org/message-id/21292.1358698487%40sss.pgh.pa.us
--
Jonathan Gonzalez V.
EDB https://enterprisedb.comAttachments:
[text/x-diff] 0001-replace-calls-to-system-with-PostgreSQL-own-implemen.patch (13.7K, ../87zeyt93ns.fsf@abdiel.eu/2-0001-replace-calls-to-system-with-PostgreSQL-own-implemen.patch)
download | inline diff:
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.cindex 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.cindex 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/Makefileindex 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.cindex 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.buildindex 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.cnew file mode 100644index 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.hnew file mode 100644index 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);++#endifdiff --git a/src/include/miscadmin.h b/src/include/miscadmin.hindex 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.hindex 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
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: pgsql-hackers@postgresql.org
Cc: jonathan@abdiel.eu, pgsql-hackers@lists.postgresql.org
Subject: Re: Introduce psystem() to replace system()
In-Reply-To: <87zeyt93ns.fsf@abdiel.eu>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox