Skip to content

Commit be6b586

Browse files
e-qe-q
committed
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.
1 parent 1d6646d commit be6b586

3 files changed

Lines changed: 139 additions & 67 deletions

File tree

‎run_release.py‎

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@
4646
DOCS_SERVER = "docs.nyc1.psf.io"
4747

4848

49-
def quote_remote_shell(value: object) -> str:
50-
return shlex.quote(str(value))
49+
def join_remote_command(command: list[object]) -> str:
50+
return shlex.join(str(argument) for argument in command)
5151

5252

5353
WHATS_NEW_TEMPLATE = """
@@ -760,7 +760,7 @@ def upload_files_to_server(db: ReleaseShelf, server: str) -> None:
760760
ftp_client = MySFTPClient.from_transport(transport)
761761
assert ftp_client is not None, f"SFTP client to {server} is None"
762762

763-
client.exec_command(f"rm -rf {quote_remote_shell(destination)}")
763+
client.exec_command(join_remote_command(["rm", "-rf", destination]))
764764

765765
with contextlib.suppress(OSError):
766766
ftp_client.mkdir(str(destination))
@@ -808,36 +808,73 @@ def place_files_in_download_folder(db: ReleaseShelf) -> None:
808808
source = f"/home/psf-users/{db['ssh_user']}/{db['release']}"
809809
destination = f"/srv/www.python.org/ftp/python/{db['release'].normalized()}"
810810

811-
def execute_command(command: str) -> None:
811+
def execute_command(command: list[object]) -> None:
812812
channel = transport.open_session()
813-
channel.exec_command(command)
813+
channel.exec_command(join_remote_command(command))
814814
if channel.recv_exit_status() != 0:
815815
raise ReleaseException(channel.recv_stderr(1000))
816816

817-
def copy_and_set_permissions(source_glob: str, destination: str) -> None:
818-
quoted_destination = quote_remote_shell(destination)
819-
execute_command(f"mkdir -p {quoted_destination}")
820-
execute_command(f"cp {source_glob} {quoted_destination}")
817+
def copy_and_set_permissions(source: str, destination: str) -> None:
818+
execute_command(["mkdir", "-p", destination])
819+
execute_command(["sh", "-c", 'cp "$1"/* "$2"', "sh", source, destination])
821820
# Skip chgrp/chmod if already correct: another RM may have created
822821
# the directory, and only the owner can change group or permissions.
823822
execute_command(
824-
f"find {quoted_destination} -maxdepth 0 ! -group downloads "
825-
f"-exec chgrp downloads {{}} +"
823+
[
824+
"find",
825+
destination,
826+
"-maxdepth",
827+
"0",
828+
"!",
829+
"-group",
830+
"downloads",
831+
"-exec",
832+
"chgrp",
833+
"downloads",
834+
"{}",
835+
"+",
836+
]
826837
)
827838
execute_command(
828-
f"find {quoted_destination} -maxdepth 0 ! -perm 775 -exec chmod 775 {{}} +"
839+
[
840+
"find",
841+
destination,
842+
"-maxdepth",
843+
"0",
844+
"!",
845+
"-perm",
846+
"775",
847+
"-exec",
848+
"chmod",
849+
"775",
850+
"{}",
851+
"+",
852+
]
829853
)
830854
execute_command(
831-
f"find {quoted_destination} -type f ! -perm 664 -exec chmod 664 {{}} +"
855+
[
856+
"find",
857+
destination,
858+
"-type",
859+
"f",
860+
"!",
861+
"-perm",
862+
"664",
863+
"-exec",
864+
"chmod",
865+
"664",
866+
"{}",
867+
"+",
868+
]
832869
)
833870

834-
copy_and_set_permissions(f"{quote_remote_shell(source)}/downloads/*", destination)
871+
copy_and_set_permissions(f"{source}/downloads", destination)
835872

836873
# Docs
837874
release_tag = db["release"]
838875
if release_tag.is_final or release_tag.is_release_candidate:
839876
copy_and_set_permissions(
840-
f"{quote_remote_shell(source)}/docs/*",
877+
f"{source}/docs",
841878
f"/srv/www.python.org/ftp/python/doc/{release_tag}",
842879
)
843880

@@ -869,27 +906,31 @@ def unpack_docs_in_the_docs_server(db: ReleaseShelf) -> None:
869906
source = f"/home/psf-users/{db['ssh_user']}/{db['release']}"
870907
destination = f"/srv/docs.python.org/release/{release_tag}"
871908

872-
def execute_command(command: str) -> None:
909+
def execute_command(command: list[object]) -> None:
873910
channel = transport.open_session()
874-
channel.exec_command(command)
911+
channel.exec_command(join_remote_command(command))
875912
if channel.recv_exit_status() != 0:
876913
raise ReleaseException(channel.recv_stderr(1000))
877914

878915
docs_filename = f"python-{release_tag}-docs-html"
879-
quoted_destination = quote_remote_shell(destination)
880-
execute_command(f"mkdir -p {quoted_destination}")
916+
execute_command(["mkdir", "-p", destination])
917+
execute_command(["unzip", f"{source}/docs/{docs_filename}.zip", "-d", destination])
881918
execute_command(
882-
f"unzip {quote_remote_shell(f'{source}/docs/{docs_filename}.zip')} "
883-
f"-d {quoted_destination}"
919+
[
920+
"sh",
921+
"-c",
922+
'mv "$1"/* "$2"',
923+
"sh",
924+
f"/{destination}/{docs_filename}",
925+
destination,
926+
]
884927
)
928+
execute_command(["rm", "-rf", f"/{destination}/{docs_filename}"])
929+
execute_command(["chgrp", "-R", "docs", destination])
930+
execute_command(["chmod", "-R", "775", destination])
885931
execute_command(
886-
f"mv {quote_remote_shell(f'/{destination}/{docs_filename}')}/* "
887-
f"{quoted_destination}"
932+
["find", destination, "-type", "f", "-exec", "chmod", "664", "{}", ";"]
888933
)
889-
execute_command(f"rm -rf {quote_remote_shell(f'/{destination}/{docs_filename}')}")
890-
execute_command(f"chgrp -R docs {quoted_destination}")
891-
execute_command(f"chmod -R 775 {quoted_destination}")
892-
execute_command(f"find {quoted_destination} -type f -exec chmod 664 {{}} \\;")
893934

894935

895936
@functools.cache
@@ -1104,13 +1145,14 @@ def run_add_to_python_dot_org(db: ReleaseShelf) -> None:
11041145
identity_token = str(issuer.identity_token())
11051146

11061147
print("Adding files to python.org...")
1107-
command = " ".join(
1148+
command = join_remote_command(
11081149
[
1109-
f"AUTH_INFO={quote_remote_shell(auth_info)}",
1110-
f"SIGSTORE_IDENTITY_TOKEN={quote_remote_shell(identity_token)}",
1150+
"env",
1151+
f"AUTH_INFO={auth_info}",
1152+
f"SIGSTORE_IDENTITY_TOKEN={identity_token}",
11111153
"python3",
11121154
"add_to_pydotorg.py",
1113-
quote_remote_shell(db["release"]),
1155+
db["release"],
11141156
]
11151157
)
11161158
stdin, stdout, stderr = client.exec_command(command)

‎tests/test_run_release.py‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,8 @@ def identity_token(self) -> str:
302302
run_release.run_add_to_python_dot_org(cast(ReleaseShelf, db))
303303

304304
assert commands == [
305-
"AUTH_INFO='user:key; echo pwned' "
306-
"SIGSTORE_IDENTITY_TOKEN='token; touch /tmp/pwned' "
305+
"env 'AUTH_INFO=user:key; echo pwned' "
306+
"'SIGSTORE_IDENTITY_TOKEN=token; touch /tmp/pwned' "
307307
"python3 add_to_pydotorg.py 3.15.0a1"
308308
]
309309

@@ -414,30 +414,33 @@ def get_transport(self) -> FakeTransport:
414414

415415
assert commands == [
416416
"mkdir -p /srv/www.python.org/ftp/python/3.15.0",
417-
"cp '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1'/downloads/* "
417+
'sh -c \'cp "$1"/* "$2"\' sh '
418+
"'/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/downloads' "
418419
"/srv/www.python.org/ftp/python/3.15.0",
419-
"find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 ! -group downloads "
420-
"-exec chgrp downloads {} +",
421-
"find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 ! -perm 775 "
422-
"-exec chmod 775 {} +",
423-
"find /srv/www.python.org/ftp/python/3.15.0 -type f ! -perm 664 "
424-
"-exec chmod 664 {} +",
420+
"find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 '!' "
421+
"-group downloads -exec chgrp downloads '{}' +",
422+
"find /srv/www.python.org/ftp/python/3.15.0 -maxdepth 0 '!' -perm 775 "
423+
"-exec chmod 775 '{}' +",
424+
"find /srv/www.python.org/ftp/python/3.15.0 -type f '!' -perm 664 "
425+
"-exec chmod 664 '{}' +",
425426
"mkdir -p /srv/www.python.org/ftp/python/doc/3.15.0rc1",
426-
"cp '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1'/docs/* "
427+
'sh -c \'cp "$1"/* "$2"\' sh '
428+
"'/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/docs' "
427429
"/srv/www.python.org/ftp/python/doc/3.15.0rc1",
428-
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 ! -group downloads "
429-
"-exec chgrp downloads {} +",
430-
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 ! -perm 775 "
431-
"-exec chmod 775 {} +",
432-
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -type f ! -perm 664 "
433-
"-exec chmod 664 {} +",
430+
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 '!' "
431+
"-group downloads -exec chgrp downloads '{}' +",
432+
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -maxdepth 0 '!' "
433+
"-perm 775 -exec chmod 775 '{}' +",
434+
"find /srv/www.python.org/ftp/python/doc/3.15.0rc1 -type f '!' -perm 664 "
435+
"-exec chmod 664 '{}' +",
434436
"mkdir -p /srv/docs.python.org/release/3.15.0rc1",
435437
"unzip '/home/psf-users/release-manager; touch /tmp/pwned #/3.15.0rc1/docs/"
436438
"python-3.15.0rc1-docs-html.zip' -d /srv/docs.python.org/release/3.15.0rc1",
437-
"mv //srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html/* "
439+
'sh -c \'mv "$1"/* "$2"\' sh '
440+
"//srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html "
438441
"/srv/docs.python.org/release/3.15.0rc1",
439442
"rm -rf //srv/docs.python.org/release/3.15.0rc1/python-3.15.0rc1-docs-html",
440443
"chgrp -R docs /srv/docs.python.org/release/3.15.0rc1",
441444
"chmod -R 775 /srv/docs.python.org/release/3.15.0rc1",
442-
"find /srv/docs.python.org/release/3.15.0rc1 -type f -exec chmod 664 {} \\;",
445+
"find /srv/docs.python.org/release/3.15.0rc1 -type f -exec chmod 664 '{}' ';'",
443446
]

‎windows-release/merge-and-upload.py‎

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,16 @@ class RunError(Exception):
6868
pass
6969

7070

71-
def quote_remote_shell(value):
72-
return shlex.quote(str(value))
71+
def join_remote_command(command):
72+
return shlex.join(str(argument) for argument in command)
73+
74+
75+
def join_remote_commands(*commands):
76+
return " && ".join(join_remote_command(command) for command in commands)
77+
78+
79+
def remote_upload_path(path):
80+
return f"{UPLOAD_USER}@{UPLOAD_HOST}:{join_remote_command([path])}"
7381

7482

7583
def _run(*args, single_cmd=False):
@@ -90,12 +98,32 @@ def _run(*args, single_cmd=False):
9098
return out
9199

92100

93-
def call_ssh(*args, allow_fail=True):
101+
def call_ssh(command, allow_fail=True):
94102
if not UPLOAD_HOST or NO_UPLOAD or LOCAL_INDEX:
95-
print("Skipping", args, "because UPLOAD_HOST is missing")
103+
print("Skipping", command, "because UPLOAD_HOST is missing")
96104
return ""
97105
try:
98-
return _run(*_std_args(PLINK), f"{UPLOAD_USER}@{UPLOAD_HOST}", *args)
106+
return _run(
107+
*_std_args(PLINK),
108+
f"{UPLOAD_USER}@{UPLOAD_HOST}",
109+
join_remote_command(command),
110+
)
111+
except RunError as ex:
112+
if not allow_fail:
113+
raise
114+
return ex.args[1]
115+
116+
117+
def call_ssh_commands(*commands, allow_fail=True):
118+
if not UPLOAD_HOST or NO_UPLOAD or LOCAL_INDEX:
119+
print("Skipping", commands, "because UPLOAD_HOST is missing")
120+
return ""
121+
try:
122+
return _run(
123+
*_std_args(PLINK),
124+
f"{UPLOAD_USER}@{UPLOAD_HOST}",
125+
join_remote_commands(*commands),
126+
)
99127
except RunError as ex:
100128
if not allow_fail:
101129
raise
@@ -109,10 +137,12 @@ def upload_ssh(source, dest):
109137
_run(
110138
*_std_args(PSCP),
111139
source,
112-
f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(dest)}",
140+
remote_upload_path(dest),
141+
)
142+
call_ssh_commands(
143+
["chgrp", "downloads", dest],
144+
["chmod", "g-x,o+r", dest],
113145
)
114-
quoted_dest = quote_remote_shell(dest)
115-
call_ssh(f"chgrp downloads {quoted_dest} && chmod g-x,o+r {quoted_dest}")
116146

117147

118148
def download_ssh(source, dest):
@@ -122,18 +152,17 @@ def download_ssh(source, dest):
122152
Path(dest).parent.mkdir(exist_ok=True, parents=True)
123153
_run(
124154
*_std_args(PSCP),
125-
f"{UPLOAD_USER}@{UPLOAD_HOST}:{quote_remote_shell(source)}",
155+
remote_upload_path(source),
126156
dest,
127157
)
128158

129159

130160
def prepare_upload_dir(dest):
131161
destdir = dest.rpartition("/")[0]
132-
quoted_destdir = quote_remote_shell(destdir)
133-
call_ssh(
134-
f"mkdir {quoted_destdir} && "
135-
f"chgrp downloads {quoted_destdir} && "
136-
f"chmod a+rx {quoted_destdir}"
162+
call_ssh_commands(
163+
["mkdir", destdir],
164+
["chgrp", "downloads", destdir],
165+
["chmod", "a+rx", destdir],
137166
)
138167

139168

@@ -320,9 +349,7 @@ def find_missing_from_index(url, installs):
320349
INDEX_PATH = url2path(INDEX_URL)
321350

322351
try:
323-
INDEX_MTIME = int(
324-
call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0"
325-
)
352+
INDEX_MTIME = int(call_ssh(["stat", "-c", "%Y", INDEX_PATH]) or "0")
326353
except ValueError:
327354
pass
328355

@@ -391,7 +418,7 @@ def find_missing_from_index(url, installs):
391418
# Check that nobody else has published while we were uploading
392419
if INDEX_FILE and INDEX_MTIME:
393420
try:
394-
mtime = int(call_ssh("stat", "-c", "%Y", quote_remote_shell(INDEX_PATH)) or "0")
421+
mtime = int(call_ssh(["stat", "-c", "%Y", INDEX_PATH]) or "0")
395422
except ValueError:
396423
mtime = 0
397424
if mtime > INDEX_MTIME:

0 commit comments

Comments
 (0)