diff --git a/run_release.py b/run_release.py index b7cfe0e3..acceeaf5 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 join_remote_command(command: list[object]) -> str: + return shlex.join(str(argument) for argument in command) + + 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(join_remote_command(["rm", "-rf", destination])) with contextlib.suppress(OSError): ftp_client.mkdir(str(destination)) @@ -803,35 +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: - execute_command(f"mkdir -p {destination}") - execute_command(f"cp {source_glob} {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 {destination} -maxdepth 0 ! -group downloads " - f"-exec chgrp downloads {{}} +" + [ + "find", + destination, + "-maxdepth", + "0", + "!", + "-group", + "downloads", + "-exec", + "chgrp", + "downloads", + "{}", + "+", + ] ) execute_command( - f"find {destination} -maxdepth 0 ! -perm 775 -exec chmod 775 {{}} +" + [ + "find", + destination, + "-maxdepth", + "0", + "!", + "-perm", + "775", + "-exec", + "chmod", + "775", + "{}", + "+", + ] ) execute_command( - f"find {destination} -type f ! -perm 664 -exec chmod 664 {{}} +" + [ + "find", + destination, + "-type", + "f", + "!", + "-perm", + "664", + "-exec", + "chmod", + "664", + "{}", + "+", + ] ) - copy_and_set_permissions(f"{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"{source}/docs/*", + f"{source}/docs", f"/srv/www.python.org/ftp/python/doc/{release_tag}", ) @@ -863,20 +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" - 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 {{}} \\;") + execute_command(["mkdir", "-p", destination]) + execute_command(["unzip", f"{source}/docs/{docs_filename}.zip", "-d", destination]) + execute_command( + [ + "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( + ["find", destination, "-type", "f", "-exec", "chmod", "664", "{}", ";"] + ) @functools.cache @@ -1088,12 +1142,20 @@ 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_remote_command( + [ + "env", + f"AUTH_INFO={auth_info}", + f"SIGSTORE_IDENTITY_TOKEN={identity_token}", + "python3", + "add_to_pydotorg.py", + 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..d34f7a92 100644 --- a/tests/test_run_release.py +++ b/tests/test_run_release.py @@ -248,3 +248,199 @@ 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 == [ + "env '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", + '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 '{}' +", + "mkdir -p /srv/www.python.org/ftp/python/doc/3.15.0rc1", + '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 '{}' +", + "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", + '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 '{}' ';'", + ] 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..445dea2b 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,18 @@ class RunError(Exception): pass +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): if single_cmd: args = args[0] @@ -85,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 @@ -101,8 +134,15 @@ 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, + remote_upload_path(dest), + ) + call_ssh_commands( + ["chgrp", "downloads", dest], + ["chmod", "g-x,o+r", dest], + ) def download_ssh(source, dest): @@ -110,7 +150,20 @@ 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), + remote_upload_path(source), + dest, + ) + + +def prepare_upload_dir(dest): + destdir = dest.rpartition("/")[0] + call_ssh_commands( + ["mkdir", destdir], + ["chgrp", "downloads", destdir], + ["chmod", "a+rx", destdir], + ) def url2path(url): @@ -296,7 +349,7 @@ 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", INDEX_PATH]) or "0") except ValueError: pass @@ -356,8 +409,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 +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", INDEX_PATH) or "0") + mtime = int(call_ssh(["stat", "-c", "%Y", INDEX_PATH]) or "0") except ValueError: mtime = 0 if mtime > INDEX_MTIME: