From e7a85767314e7b0fe0b35fe80b9c1af38f48dff6 Mon Sep 17 00:00:00 2001
From: Ashesh Vashi <ashesh.vashi@enterprisedb.com>
Date: Fri, 24 Jul 2026 16:11:57 +0530
Subject: [PATCH] Fix MASTER_PASSWORD_HOOK tokenisation on Windows

posix=False in shlex.split() never strips quote characters, so any
Windows hook path quoted to handle spaces (or any quoted argument)
came out with literal quotes still attached, and unquoted spaced
paths were split into multiple argv elements either way.

Use a shlex.shlex instance with posix=True (correct quote-stripping)
and escape='' (so backslashes in Windows paths are not treated as
escape characters). Document the quoting requirement in config.py
and add regression tests for quoted/unquoted spaced paths.
---
 web/config.py                                 |  4 +-
 web/pgadmin/utils/master_password.py          | 15 +++++---
 .../utils/tests/test_master_password_hook.py  | 37 +++++++++++++++++++
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/web/config.py b/web/config.py
index 50d4828af..19cdf286a 100644
--- a/web/config.py
+++ b/web/config.py
@@ -641,7 +641,9 @@ USE_OS_SECRET_STORAGE = True
 # shell, so shell features (pipes, redirection, environment-variable
 # expansion, globbing) in this value are not interpreted; put any such
 # logic inside the hook script itself. On Windows, a batch file must be
-# invoked via an executable wrapper rather than directly.
+# invoked via an executable wrapper rather than directly. If the path to
+# the script/program contains spaces, quote it, e.g. -
+# MASTER_PASSWORD_HOOK = '"<PATH WITH SPACES>/passwdgen_script.exe" %u'
 ##########################################################################
 MASTER_PASSWORD_HOOK = None
 
diff --git a/web/pgadmin/utils/master_password.py b/web/pgadmin/utils/master_password.py
index 4176b56d0..40bcda8cb 100644
--- a/web/pgadmin/utils/master_password.py
+++ b/web/pgadmin/utils/master_password.py
@@ -192,7 +192,6 @@ def get_master_password_from_master_hook():
 
     :return: Output of the command, or None on failure.
     """
-    import os
     import shlex
     import subprocess
 
@@ -204,10 +203,16 @@ def get_master_password_from_master_hook():
         # untrusted username into the individual arguments. Substituting
         # before tokenising would let metacharacters in the username alter
         # the command, so the order here is security-critical.
-        args = [
-            token.replace('%u', current_user.username)
-            for token in shlex.split(cmd, posix=(os.name != 'nt'))
-        ]
+        #
+        # posix=True strips quotes correctly on both platforms (unlike
+        # posix=False, which never strips quotes at all and breaks any
+        # quoted path or argument on Windows). escape='' disables backslash
+        # escaping so Windows paths (e.g. C:\Program Files\hook.exe) are not
+        # mangled.
+        lex = shlex.shlex(cmd, posix=True)
+        lex.whitespace_split = True
+        lex.escape = ''
+        args = [token.replace('%u', current_user.username) for token in lex]
         if not args:
             return None
 
diff --git a/web/pgadmin/utils/tests/test_master_password_hook.py b/web/pgadmin/utils/tests/test_master_password_hook.py
index fc395e7a2..ede195d93 100644
--- a/web/pgadmin/utils/tests/test_master_password_hook.py
+++ b/web/pgadmin/utils/tests/test_master_password_hook.py
@@ -87,6 +87,43 @@ class TestMasterPasswordHookConfinesUsername(BaseTestGenerator):
         self.assertEqual(output, 'secret-key')
 
 
+class TestMasterPasswordHookQuotedPath(BaseTestGenerator):
+    """A quoted hook program path (needed when the path contains spaces,
+    e.g. 'C:\\Program Files\\hook.exe' on Windows) must be tokenised as a
+    single argv element with the quotes stripped, and any backslashes in
+    the path must survive untouched."""
+
+    scenarios = [
+        ('quoted path with spaces, posix-style',
+         dict(cmd='"/opt/my secrets/get-secret" %u',
+              expected_argv0='/opt/my secrets/get-secret')),
+        ('quoted windows path with backslashes and spaces',
+         dict(cmd='"C:\\Program Files\\hook.exe" %u',
+              expected_argv0='C:\\Program Files\\hook.exe')),
+        ('unquoted windows path, no spaces',
+         dict(cmd='C:\\hook.exe %u', expected_argv0='C:\\hook.exe')),
+    ]
+
+    def runTest(self):
+        captured = {}
+
+        def fake_popen(args, *a, **kw):
+            captured['args'] = args
+            return _FakeProc()
+
+        fake_user = MagicMock()
+        fake_user.username = 'alice'
+
+        with patch.object(mp, 'current_user', fake_user), \
+                patch.object(mp.config, 'MASTER_PASSWORD_HOOK', self.cmd), \
+                patch('subprocess.Popen', side_effect=fake_popen):
+            mp.get_master_password_from_master_hook()
+
+        self.assertEqual(captured['args'][0], self.expected_argv0)
+        self.assertEqual(captured['args'][1], 'alice')
+        self.assertEqual(len(captured['args']), 2)
+
+
 class TestMasterPasswordHookNoShellExecution(BaseTestGenerator):
     """End-to-end PoC regression (POSIX only): a username containing shell
     syntax must not cause command execution. Mirrors the reported proof of