From 1d6646d1e03efdd247a0ee29fd406b51915f1995 Mon Sep 17 00:00:00 2001 From: "eric.quintero@trailofbits.com" Date: Tue, 2 Jun 2026 16:26:20 +0000 Subject: [PATCH 1/2] Quote remote release tool commands --- run_release.py | 56 +++++--- tests/test_run_release.py | 193 ++++++++++++++++++++++++++++ tests/test_windows_merge_upload.py | 100 ++++++++++++++ windows-release/merge-and-upload.py | 39 +++++- 4 files changed, 363 insertions(+), 25 deletions(-) create mode 100644 tests/test_windows_merge_upload.py diff --git a/run_release.py b/run_release.py index b7cfe0e3..fbe3da47 100755 --- a/run_release.py +++ b/run_release.py @@ -45,6 +45,11 @@ DOWNLOADS_SERVER = "downloads.nyc1.psf.io" DOCS_SERVER = "docs.nyc1.psf.io" + +def quote_remote_shell(value: object) -> str: + return shlex.quote(str(value)) + + WHATS_NEW_TEMPLATE = """ ***************************** What's new in Python {version} @@ -755,7 +760,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 {destination}") + client.exec_command(f"rm -rf {quote_remote_shell(destination)}") with contextlib.suppress(OSError): ftp_client.mkdir(str(destination)) @@ -810,28 +815,29 @@ def execute_command(command: str) -> None: raise ReleaseException(channel.recv_stderr(1000)) def copy_and_set_permissions(source_glob: str, destination: str) -> None: - execute_command(f"mkdir -p {destination}") - execute_command(f"cp {source_glob} {destination}") + quoted_destination = quote_remote_shell(destination) + execute_command(f"mkdir -p {quoted_destination}") + execute_command(f"cp {source_glob} {quoted_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 {destination} -maxdepth 0 ! -group downloads " + f"find {quoted_destination} -maxdepth 0 ! -group downloads " f"-exec chgrp downloads {{}} +" ) execute_command( - f"find {destination} -maxdepth 0 ! -perm 775 -exec chmod 775 {{}} +" + f"find {quoted_destination} -maxdepth 0 ! -perm 775 -exec chmod 775 {{}} +" ) execute_command( - f"find {destination} -type f ! -perm 664 -exec chmod 664 {{}} +" + f"find {quoted_destination} -type f ! -perm 664 -exec chmod 664 {{}} +" ) - copy_and_set_permissions(f"{source}/downloads/*", destination) + copy_and_set_permissions(f"{quote_remote_shell(source)}/downloads/*", destination) # Docs release_tag = db["release"] if release_tag.is_final or release_tag.is_release_candidate: copy_and_set_permissions( - f"{source}/docs/*", + f"{quote_remote_shell(source)}/docs/*", f"/srv/www.python.org/ftp/python/doc/{release_tag}", ) @@ -870,13 +876,20 @@ def execute_command(command: str) -> None: raise ReleaseException(channel.recv_stderr(1000)) docs_filename = f"python-{release_tag}-docs-html" - execute_command(f"mkdir -p {destination}") - execute_command(f"unzip {source}/docs/{docs_filename}.zip -d {destination}") - execute_command(f"mv /{destination}/{docs_filename}/* {destination}") - execute_command(f"rm -rf /{destination}/{docs_filename}") - execute_command(f"chgrp -R docs {destination}") - execute_command(f"chmod -R 775 {destination}") - execute_command(f"find {destination} -type f -exec chmod 664 {{}} \\;") + quoted_destination = quote_remote_shell(destination) + execute_command(f"mkdir -p {quoted_destination}") + execute_command( + f"unzip {quote_remote_shell(f'{source}/docs/{docs_filename}.zip')} " + f"-d {quoted_destination}" + ) + execute_command( + f"mv {quote_remote_shell(f'/{destination}/{docs_filename}')}/* " + f"{quoted_destination}" + ) + 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 @@ -1088,12 +1101,19 @@ def run_add_to_python_dot_org(db: ReleaseShelf) -> None: # Do the interactive flow to get an identity for Sigstore issuer = sigstore.oidc.Issuer(sigstore.oidc.DEFAULT_OAUTH_ISSUER_URL) - identity_token = issuer.identity_token() + identity_token = str(issuer.identity_token()) print("Adding files to python.org...") - stdin, stdout, stderr = client.exec_command( - f"AUTH_INFO={auth_info} SIGSTORE_IDENTITY_TOKEN={identity_token} python3 add_to_pydotorg.py {db['release']}" + command = " ".join( + [ + f"AUTH_INFO={quote_remote_shell(auth_info)}", + f"SIGSTORE_IDENTITY_TOKEN={quote_remote_shell(identity_token)}", + "python3", + "add_to_pydotorg.py", + quote_remote_shell(db["release"]), + ] ) + stdin, stdout, stderr = client.exec_command(command) stderr_text = stderr.read().decode() if stderr_text: raise paramiko.SSHException(f"Failed to execute the command: {stderr_text}") diff --git a/tests/test_run_release.py b/tests/test_run_release.py index 82830a85..d9387c3f 100644 --- a/tests/test_run_release.py +++ b/tests/test_run_release.py @@ -248,3 +248,196 @@ def test_update_whatsnew_toctree(tmp_path: Path) -> None: # Assert new_contents = toctree__file.read_text() assert " 3.15.rst\n 3.14.rst\n" in new_contents + + +def test_run_add_to_python_dot_org_quotes_remote_environment(monkeypatch) -> None: + commands = [] + + class FakeSFTPClient: + def put(self, source: str, destination: str) -> None: + pass + + def close(self) -> None: + pass + + class FakeSSHClient: + def load_system_host_keys(self) -> None: + pass + + def set_missing_host_key_policy(self, policy) -> None: + pass + + def connect(self, *args, **kwargs) -> None: + pass + + def get_transport(self): + return object() + + def exec_command(self, command: str): + commands.append(command) + return None, io.BytesIO(b"ok"), io.BytesIO() + + class FakeIssuer: + def __init__(self, issuer_url: str) -> None: + self.issuer_url = issuer_url + + def identity_token(self) -> str: + return "token; touch /tmp/pwned" + + monkeypatch.setattr(run_release.paramiko, "SSHClient", FakeSSHClient) + monkeypatch.setattr( + run_release.MySFTPClient, + "from_transport", + staticmethod(lambda transport: FakeSFTPClient()), + ) + monkeypatch.setattr(run_release.sigstore.oidc, "Issuer", FakeIssuer) + + db = { + "auth_info": "user:key; echo pwned", + "release": Tag("3.15.0a1"), + "ssh_key": None, + "ssh_user": "release-manager", + } + + 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' " + "python3 add_to_pydotorg.py 3.15.0a1" + ] + + +def test_upload_files_to_server_quotes_remote_cleanup_path( + monkeypatch, tmp_path: Path +) -> None: + commands = [] + + class FakeSFTPClient: + def mkdir(self, path: str) -> None: + pass + + def put_dir(self, source: Path, target: str, progress) -> None: + pass + + def close(self) -> None: + pass + + class FakeSSHClient: + def load_system_host_keys(self) -> None: + pass + + def set_missing_host_key_policy(self, policy) -> None: + pass + + def connect(self, *args, **kwargs) -> None: + pass + + def get_transport(self): + return object() + + def exec_command(self, command: str) -> None: + commands.append(command) + + @contextlib.contextmanager + def fake_alive_bar(total: int): + yield lambda *args, **kwargs: None + + release = Tag("3.15.0a1") + artifacts_path = tmp_path / str(release) + (artifacts_path / "downloads").mkdir(parents=True) + + monkeypatch.setattr(run_release.paramiko, "SSHClient", FakeSSHClient) + monkeypatch.setattr( + run_release.MySFTPClient, + "from_transport", + staticmethod(lambda transport: FakeSFTPClient()), + ) + monkeypatch.setattr(run_release, "alive_bar", fake_alive_bar) + + db = { + "git_repo": tmp_path, + "release": release, + "ssh_key": None, + "ssh_user": "release-manager; touch /tmp/pwned #", + } + + run_release.upload_files_to_server( + cast(ReleaseShelf, db), run_release.DOWNLOADS_SERVER + ) + + assert commands == [ + "rm -rf '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0a1'" + ] + + +def test_release_file_placement_quotes_remote_paths(monkeypatch) -> None: + commands = [] + + class FakeChannel: + def exec_command(self, command: str) -> None: + commands.append(command) + + def recv_exit_status(self) -> int: + return 0 + + def recv_stderr(self, size: int) -> bytes: + return b"" + + class FakeTransport: + def open_session(self) -> FakeChannel: + return FakeChannel() + + class FakeSSHClient: + def load_system_host_keys(self) -> None: + pass + + def set_missing_host_key_policy(self, policy) -> None: + pass + + def connect(self, *args, **kwargs) -> None: + pass + + def get_transport(self) -> FakeTransport: + return FakeTransport() + + monkeypatch.setattr(run_release.paramiko, "SSHClient", FakeSSHClient) + + db = { + "release": Tag("3.15.0rc1"), + "ssh_key": None, + "ssh_user": "release-manager; touch /tmp/pwned #", + } + + run_release.place_files_in_download_folder(cast(ReleaseShelf, db)) + run_release.unpack_docs_in_the_docs_server(cast(ReleaseShelf, db)) + + 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/* " + "/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 {} +", + "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/* " + "/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 {} +", + "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/* " + "/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 {} \\;", + ] diff --git a/tests/test_windows_merge_upload.py b/tests/test_windows_merge_upload.py new file mode 100644 index 00000000..c89e4a06 --- /dev/null +++ b/tests/test_windows_merge_upload.py @@ -0,0 +1,100 @@ +import importlib.util +from pathlib import Path +from typing import Any + +import pytest + + +def load_merge_upload_module(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Any: + for name in ( + "INDEX_FILE", + "LOCAL_INDEX", + "MAKECAT", + "MANIFEST_FILE", + "NO_UPLOAD", + "PLINK", + "PSCP", + "SIGN_COMMAND", + "UPLOAD_HOST", + "UPLOAD_HOST_KEY", + "UPLOAD_KEYFILE", + "UPLOAD_PATH_PREFIX", + "UPLOAD_URL_PREFIX", + "UPLOAD_USER", + ): + monkeypatch.delenv(name, raising=False) + + monkeypatch.chdir(tmp_path) + script = Path(__file__).parents[1] / "windows-release" / "merge-and-upload.py" + spec = importlib.util.spec_from_file_location("merge_and_upload_for_test", script) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + + with pytest.raises(SystemExit) as exc_info: + spec.loader.exec_module(module) + + assert exc_info.value.code == 1 + return module + + +def test_remote_upload_commands_quote_url_derived_paths( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + module = load_merge_upload_module(monkeypatch, tmp_path) + calls: list[tuple[tuple[object, ...], bool]] = [] + + def fake_run(*args: object, single_cmd: bool = False) -> str: + calls.append((args, single_cmd)) + return "" + + module._run = fake_run + module.PLINK = "plink.exe" + module.PSCP = "pscp.exe" + module.UPLOAD_HOST = "downloads.example.org" + module.UPLOAD_USER = "release-manager" + + dest = module.url2path( + "https://www.python.org/ftp/python/3.14.0;touch PTP/python-3.14.0-amd64.exe" + ) + + module.prepare_upload_dir(dest) + module.upload_ssh("python-3.14.0-amd64.exe", dest) + + assert calls == [ + ( + ( + "plink.exe", + "-batch", + "release-manager@downloads.example.org", + "mkdir '/srv/www.python.org/ftp/python/3.14.0;touch PTP' && " + "chgrp downloads '/srv/www.python.org/ftp/python/3.14.0;touch PTP' && " + "chmod a+rx '/srv/www.python.org/ftp/python/3.14.0;touch PTP'", + ), + False, + ), + ( + ( + "pscp.exe", + "-batch", + "python-3.14.0-amd64.exe", + "release-manager@downloads.example.org:" + "'/srv/www.python.org/ftp/python/3.14.0;touch PTP/" + "python-3.14.0-amd64.exe'", + ), + False, + ), + ( + ( + "plink.exe", + "-batch", + "release-manager@downloads.example.org", + "chgrp downloads " + "'/srv/www.python.org/ftp/python/3.14.0;touch PTP/" + "python-3.14.0-amd64.exe' && chmod g-x,o+r " + "'/srv/www.python.org/ftp/python/3.14.0;touch PTP/" + "python-3.14.0-amd64.exe'", + ), + False, + ), + ] diff --git a/windows-release/merge-and-upload.py b/windows-release/merge-and-upload.py index 5e00a6d1..65152d16 100644 --- a/windows-release/merge-and-upload.py +++ b/windows-release/merge-and-upload.py @@ -2,6 +2,7 @@ import json import os import re +import shlex import subprocess import sys from pathlib import Path @@ -67,6 +68,10 @@ class RunError(Exception): pass +def quote_remote_shell(value): + return shlex.quote(str(value)) + + def _run(*args, single_cmd=False): if single_cmd: args = args[0] @@ -101,8 +106,13 @@ def upload_ssh(source, dest): if not UPLOAD_HOST or NO_UPLOAD or LOCAL_INDEX: print("Skipping upload of", source, "because UPLOAD_HOST is missing") return - _run(*_std_args(PSCP), source, f"{UPLOAD_USER}@{UPLOAD_HOST}:{dest}") - call_ssh(f"chgrp downloads {dest} && chmod g-x,o+r {dest}") + _run( + *_std_args(PSCP), + source, + f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(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): @@ -110,7 +120,21 @@ def download_ssh(source, dest): print("Skipping download of", source, "because UPLOAD_HOST is missing") return Path(dest).parent.mkdir(exist_ok=True, parents=True) - _run(*_std_args(PSCP), f"{UPLOAD_USER}@{UPLOAD_HOST}:{source}", dest) + _run( + *_std_args(PSCP), + f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(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}" + ) def url2path(url): @@ -296,7 +320,9 @@ def find_missing_from_index(url, installs): INDEX_PATH = url2path(INDEX_URL) try: - INDEX_MTIME = int(call_ssh("stat", "-c", "%Y", INDEX_PATH) or "0") + INDEX_MTIME = int( + call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0" + ) except ValueError: pass @@ -356,8 +382,7 @@ def find_missing_from_index(url, installs): # Upload last to ensure we've got a valid index first for i, src, dest, sbom, sbom_dest in UPLOADS: print("Uploading", src, "to", dest) - destdir = dest.rpartition("/")[0] - call_ssh(f"mkdir {destdir} && chgrp downloads {destdir} && chmod a+rx {destdir}") + prepare_upload_dir(dest) upload_ssh(src, dest) if sbom and sbom_dest: upload_ssh(sbom, sbom_dest) @@ -366,7 +391,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", INDEX_PATH) or "0") + mtime = int(call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0") except ValueError: mtime = 0 if mtime > INDEX_MTIME: From be6b5869b110eb7e8b7426ecf5e3aac135078244 Mon Sep 17 00:00:00 2001 From: "eric.quintero@trailofbits.com" Date: Wed, 3 Jun 2026 21:32:11 +0000 Subject: [PATCH 2/2] 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. --- run_release.py | 104 +++++++++++++++++++--------- tests/test_run_release.py | 39 ++++++----- windows-release/merge-and-upload.py | 63 ++++++++++++----- 3 files changed, 139 insertions(+), 67 deletions(-) diff --git a/run_release.py b/run_release.py index fbe3da47..acceeaf5 100755 --- a/run_release.py +++ b/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 = """ @@ -760,7 +760,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)) @@ -808,36 +808,73 @@ 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} -type f ! -perm 664 -exec chmod 664 {{}} +" + [ + "find", + destination, + "-type", + "f", + "!", + "-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}", ) @@ -869,27 +906,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 @@ -1104,13 +1145,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) diff --git a/tests/test_run_release.py b/tests/test_run_release.py index d9387c3f..d34f7a92 100644 --- a/tests/test_run_release.py +++ b/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,33 @@ 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 -type f '!' -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 -type f '!' -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 '{}' ';'", ] diff --git a/windows-release/merge-and-upload.py b/windows-release/merge-and-upload.py index 65152d16..445dea2b 100644 --- a/windows-release/merge-and-upload.py +++ b/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: