Use list-based remote command builders

Refactor SSH command helpers to accept command argument lists and join them at the execution boundary with shlex.join().

Keep the two intentional wildcard operations as static sh -c snippets with dynamic paths passed as positional arguments, so glob expansion remains explicit without quoting values at each call site.

Validation: tox -q -e py313 -- -k 'run_add_to_python_dot_org_quotes_remote_environment or upload_files_to_server_quotes_remote_cleanup_path or release_file_placement_quotes_remote_paths or remote_upload_commands_quote_url_derived_paths'; tox -q -e py313 -- tests/test_run_release.py tests/test_windows_merge_upload.py; tox -q -e mypy; tox -q -e lint.

Author: e-q

run_release.py

@@ -46,8 +46,8 @@
 DOCS_SERVER = "docs.nyc1.psf.io"
 
 
-def quote_remote_shell(value: object) -> str:
-    return shlex.quote(str(value))
+def join_remote_command(command: list[object]) -> str:
+    return shlex.join(str(argument) for argument in command)
 
 
 WHATS_NEW_TEMPLATE = """
@@ -741,7 +741,7 @@ def upload_files_to_server(db: ReleaseShelf, server: str) -> None:
     ftp_client = MySFTPClient.from_transport(transport)
     assert ftp_client is not None, f"SFTP client to {server} is None"
 
-    client.exec_command(f"rm -rf {quote_remote_shell(destination)}")
+    client.exec_command(join_remote_command(["rm", "-rf", destination]))
 
     with contextlib.suppress(OSError):
         ftp_client.mkdir(str(destination))
@@ -789,37 +789,77 @@ def place_files_in_download_folder(db: ReleaseShelf) -> None:
     source = f"/home/psf-users/{db['ssh_user']}/{db['release']}"
     destination = f"/srv/www.python.org/ftp/python/{db['release'].normalized()}"
 
-    def execute_command(command: str) -> None:
+    def execute_command(command: list[object]) -> None:
         channel = transport.open_session()
-        channel.exec_command(command)
+        channel.exec_command(join_remote_command(command))
         if channel.recv_exit_status() != 0:
             raise ReleaseException(channel.recv_stderr(1000))
 
-    def copy_and_set_permissions(source_glob: str, destination: str) -> None:
-        quoted_destination = quote_remote_shell(destination)
-        execute_command(f"mkdir -p {quoted_destination}")
-        execute_command(f"cp {source_glob} {quoted_destination}")
+    def copy_and_set_permissions(source: str, destination: str) -> None:
+        execute_command(["mkdir", "-p", destination])
+        execute_command(["sh", "-c", 'cp "$1"/* "$2"', "sh", source, destination])
         # Skip chgrp/chmod if already correct: another RM may have created
         # the directory, and only the owner can change group or permissions.
         execute_command(
-            f"find {quoted_destination} -maxdepth 0 ! -group downloads "
-            f"-exec chgrp downloads {{}} +"
+            [
+                "find",
+                destination,
+                "-maxdepth",
+                "0",
+                "!",
+                "-group",
+                "downloads",
+                "-exec",
+                "chgrp",
+                "downloads",
+                "{}",
+                "+",
+            ]
         )
         execute_command(
-            f"find {quoted_destination} -maxdepth 0 ! -perm 775 -exec chmod 775 {{}} +"
+            [
+                "find",
+                destination,
+                "-maxdepth",
+                "0",
+                "!",
+                "-perm",
+                "775",
+                "-exec",
+                "chmod",
+                "775",
+                "{}",
+                "+",
+            ]
         )
         execute_command(
-            f"find {quoted_destination} -maxdepth 1 -type f -user $USER "
-            f"! -perm 664 -exec chmod 664 {{}} +"
+            [
+                "find",
+                destination,
+                "-maxdepth",
+                "1",
+                "-type",
+                "f",
+                "-user",
+                db["ssh_user"],
+                "!",
+                "-perm",
+                "664",
+                "-exec",
+                "chmod",
+                "664",
+                "{}",
+                "+",
+            ]
         )
 
-    copy_and_set_permissions(f"{quote_remote_shell(source)}/downloads/*", destination)
+    copy_and_set_permissions(f"{source}/downloads", destination)
 
     # Docs
     release_tag = db["release"]
     if release_tag.is_final or release_tag.is_release_candidate:
         copy_and_set_permissions(
-            f"{quote_remote_shell(source)}/docs/*",
+            f"{source}/docs",
             f"/srv/www.python.org/ftp/python/doc/{release_tag}",
         )
 
@@ -851,27 +891,31 @@ def unpack_docs_in_the_docs_server(db: ReleaseShelf) -> None:
     source = f"/home/psf-users/{db['ssh_user']}/{db['release']}"
     destination = f"/srv/docs.python.org/release/{release_tag}"
 
-    def execute_command(command: str) -> None:
+    def execute_command(command: list[object]) -> None:
         channel = transport.open_session()
-        channel.exec_command(command)
+        channel.exec_command(join_remote_command(command))
         if channel.recv_exit_status() != 0:
             raise ReleaseException(channel.recv_stderr(1000))
 
     docs_filename = f"python-{release_tag}-docs-html"
-    quoted_destination = quote_remote_shell(destination)
-    execute_command(f"mkdir -p {quoted_destination}")
+    execute_command(["mkdir", "-p", destination])
+    execute_command(["unzip", f"{source}/docs/{docs_filename}.zip", "-d", destination])
     execute_command(
-        f"unzip {quote_remote_shell(f'{source}/docs/{docs_filename}.zip')} "
-        f"-d {quoted_destination}"
+        [
+            "sh",
+            "-c",
+            'mv "$1"/* "$2"',
+            "sh",
+            f"/{destination}/{docs_filename}",
+            destination,
+        ]
     )
+    execute_command(["rm", "-rf", f"/{destination}/{docs_filename}"])
+    execute_command(["chgrp", "-R", "docs", destination])
+    execute_command(["chmod", "-R", "775", destination])
     execute_command(
-        f"mv {quote_remote_shell(f'/{destination}/{docs_filename}')}/* "
-        f"{quoted_destination}"
+        ["find", destination, "-type", "f", "-exec", "chmod", "664", "{}", ";"]
     )
-    execute_command(f"rm -rf {quote_remote_shell(f'/{destination}/{docs_filename}')}")
-    execute_command(f"chgrp -R docs {quoted_destination}")
-    execute_command(f"chmod -R 775 {quoted_destination}")
-    execute_command(f"find {quoted_destination} -type f -exec chmod 664 {{}} \\;")
 
 
 @functools.cache
@@ -1089,13 +1133,14 @@ def run_add_to_python_dot_org(db: ReleaseShelf) -> None:
     identity_token = str(issuer.identity_token())
 
     print("Adding files to python.org...")
-    command = " ".join(
+    command = join_remote_command(
         [
-            f"AUTH_INFO={quote_remote_shell(auth_info)}",
-            f"SIGSTORE_IDENTITY_TOKEN={quote_remote_shell(identity_token)}",
+            "env",
+            f"AUTH_INFO={auth_info}",
+            f"SIGSTORE_IDENTITY_TOKEN={identity_token}",
             "python3",
             "add_to_pydotorg.py",
-            quote_remote_shell(db["release"]),
+            db["release"],
         ]
     )
     stdin, stdout, stderr = client.exec_command(command)

tests/test_run_release.py

@@ -302,8 +302,8 @@ def identity_token(self) -> str:
     run_release.run_add_to_python_dot_org(cast(ReleaseShelf, db))
 
     assert commands == [
-        "AUTH_INFO='user:key; echo pwned' "
-        "SIGSTORE_IDENTITY_TOKEN='token; touch /tmp/pwned' "
+        "env 'AUTH_INFO=user:key; echo pwned' "
+        "'SIGSTORE_IDENTITY_TOKEN=token; touch /tmp/pwned' "
         "python3 add_to_pydotorg.py 3.15.0a1"
     ]
 
@@ -414,30 +414,34 @@ def get_transport(self) -> FakeTransport:
 
     assert commands == [
         "mkdir -p /srv/www.python.org/ftp/python/3.15.0",
-        "cp '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1'/downloads/* "
+        'sh -c \'cp "$1"/* "$2"\' sh '
+        "'/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/downloads' "
         "/srv/www.python.org/ftp/python/3.15.0",
-        "find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 ! -group downloads "
-        "-exec chgrp downloads {} +",
-        "find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 ! -perm 775 "
-        "-exec chmod 775 {} +",
-        "find /srv/www.python.org/ftp/python/3.15.0 -type f ! -perm 664 "
-        "-exec chmod 664 {} +",
+        "find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 '!' "
+        "-group downloads -exec chgrp downloads '{}' +",
+        "find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 '!' -perm 775 "
+        "-exec chmod 775 '{}' +",
+        "find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 1 -type f -user "
+        "'release-manager; touch /tmp/pwned #' '!' -perm 664 -exec chmod 664 '{}' +",
         "mkdir -p /srv/www.python.org/ftp/python/doc/3.15.0rc1",
-        "cp '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1'/docs/* "
+        'sh -c \'cp "$1"/* "$2"\' sh '
+        "'/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/docs' "
         "/srv/www.python.org/ftp/python/doc/3.15.0rc1",
-        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 ! -group downloads "
-        "-exec chgrp downloads {} +",
-        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 ! -perm 775 "
-        "-exec chmod 775 {} +",
-        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -type f ! -perm 664 "
-        "-exec chmod 664 {} +",
+        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 '!' "
+        "-group downloads -exec chgrp downloads '{}' +",
+        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 '!' "
+        "-perm 775 -exec chmod 775 '{}' +",
+        "find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 1 -type f "
+        "-user 'release-manager; touch /tmp/pwned #' '!' -perm 664 "
+        "-exec chmod 664 '{}' +",
         "mkdir -p /srv/docs.python.org/release/3.15.0rc1",
         "unzip '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/docs/"
         "python-3.15.0rc1-docs-html.zip' -d /srv/docs.python.org/release/3.15.0rc1",
-        "mv //srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html/* "
+        'sh -c \'mv "$1"/* "$2"\' sh '
+        "//srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html "
         "/srv/docs.python.org/release/3.15.0rc1",
         "rm -rf //srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html",
         "chgrp -R docs /srv/docs.python.org/release/3.15.0rc1",
         "chmod -R 775 /srv/docs.python.org/release/3.15.0rc1",
-        "find /srv/docs.python.org/release/3.15.0rc1 -type f -exec chmod 664 {} \\;",
+        "find /srv/docs.python.org/release/3.15.0rc1 -type f -exec chmod 664 '{}' ';'",
     ]

windows-release/merge-and-upload.py

@@ -68,8 +68,16 @@ class RunError(Exception):
     pass
 
 
-def quote_remote_shell(value):
-    return shlex.quote(str(value))
+def join_remote_command(command):
+    return shlex.join(str(argument) for argument in command)
+
+
+def join_remote_commands(*commands):
+    return " && ".join(join_remote_command(command) for command in commands)
+
+
+def remote_upload_path(path):
+    return f"{UPLOAD_USER}@{UPLOAD_HOST}:{join_remote_command([path])}"
 
 
 def _run(*args, single_cmd=False):
@@ -90,12 +98,32 @@ def _run(*args, single_cmd=False):
         return out
 
 
-def call_ssh(*args, allow_fail=True):
+def call_ssh(command, allow_fail=True):
     if not UPLOAD_HOST or NO_UPLOAD or LOCAL_INDEX:
-        print("Skipping", args, "because UPLOAD_HOST is missing")
+        print("Skipping", command, "because UPLOAD_HOST is missing")
         return ""
     try:
-        return _run(*_std_args(PLINK), f"{UPLOAD_USER}@{UPLOAD_HOST}", *args)
+        return _run(
+            *_std_args(PLINK),
+            f"{UPLOAD_USER}@{UPLOAD_HOST}",
+            join_remote_command(command),
+        )
+    except RunError as ex:
+        if not allow_fail:
+            raise
+        return ex.args[1]
+
+
+def call_ssh_commands(*commands, allow_fail=True):
+    if not UPLOAD_HOST or NO_UPLOAD or LOCAL_INDEX:
+        print("Skipping", commands, "because UPLOAD_HOST is missing")
+        return ""
+    try:
+        return _run(
+            *_std_args(PLINK),
+            f"{UPLOAD_USER}@{UPLOAD_HOST}",
+            join_remote_commands(*commands),
+        )
     except RunError as ex:
         if not allow_fail:
             raise
@@ -109,10 +137,12 @@ def upload_ssh(source, dest):
     _run(
         *_std_args(PSCP),
         source,
-        f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(dest)}",
+        remote_upload_path(dest),
+    )
+    call_ssh_commands(
+        ["chgrp", "downloads", dest],
+        ["chmod", "g-x,o+r", dest],
     )
-    quoted_dest = quote_remote_shell(dest)
-    call_ssh(f"chgrp downloads {quoted_dest} && chmod g-x,o+r {quoted_dest}")
 
 
 def download_ssh(source, dest):
@@ -122,18 +152,17 @@ def download_ssh(source, dest):
     Path(dest).parent.mkdir(exist_ok=True, parents=True)
     _run(
         *_std_args(PSCP),
-        f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(source)}",
+        remote_upload_path(source),
         dest,
     )
 
 
 def prepare_upload_dir(dest):
     destdir = dest.rpartition("/")[0]
-    quoted_destdir = quote_remote_shell(destdir)
-    call_ssh(
-        f"mkdir {quoted_destdir} && "
-        f"chgrp downloads {quoted_destdir} && "
-        f"chmod a+rx {quoted_destdir}"
+    call_ssh_commands(
+        ["mkdir", destdir],
+        ["chgrp", "downloads", destdir],
+        ["chmod", "a+rx", destdir],
     )
 
 
@@ -320,9 +349,7 @@ def find_missing_from_index(url, installs):
     INDEX_PATH = url2path(INDEX_URL)
 
     try:
-        INDEX_MTIME = int(
-            call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0"
-        )
+        INDEX_MTIME = int(call_ssh(["stat", "-c", "%Y", INDEX_PATH]) or "0")
     except ValueError:
         pass
 
@@ -391,7 +418,7 @@ def find_missing_from_index(url, installs):
 # Check that nobody else has published while we were uploading
 if INDEX_FILE and INDEX_MTIME:
     try:
-        mtime = int(call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0")
+        mtime = int(call_ssh(["stat", "-c", "%Y", INDEX_PATH]) or "0")
     except ValueError:
         mtime = 0
     if mtime > INDEX_MTIME: