Skip to content

Quote remote release-tool command arguments#402

Open
e-q wants to merge 2 commits into
python:mainfrom
e-q:quote-remote-release-commands
Open

Quote remote release-tool command arguments#402
e-q wants to merge 2 commits into
python:mainfrom
e-q:quote-remote-release-commands

Conversation

@e-q

@e-q e-q commented Jun 2, 2026

Copy link
Copy Markdown

Summary

  • Quote environment values and remote paths before building release upload command strings.
  • Preserve intended glob expansion for staged release artifact copies.
  • Quote URL-derived Windows upload paths before passing them to SSH/SCP helpers.
  • Add mocked regression tests for the generated SSH and upload command strings.

Closes #397 (release tooling remote command arguments)
Closes #401 (Windows upload URL-derived remote paths)

Testing

  • Focused tox regression tests for generated remote command quoting: passed.
  • tox -q -e mypy: passed.
  • Ruff lint and whitespace checks on touched files: passed.

@python-cla-bot

python-cla-bot Bot commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@sethmlarson

sethmlarson commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

@e-q Overall comment: Instead of introducing shlex.quote() everywhere is there value instead in changing functions to accept a list of parameters instead of a str and calling shlex.join before sending the command? Then we don't have to rely on remembering to call shlex.quote() everywhere.

@e-q

e-q commented Jun 3, 2026

Copy link
Copy Markdown
Author

Gotcha, I refactored this so the SSH helpers now accept command argument lists and call shlex.join() at the execution boundary instead of requiring each call site to remember shlex.quote().

For the two places that intentionally need remote shell glob expansion, the updated version uses static sh -c snippets and passes the dynamic paths as positional arguments. That keeps the wildcard behavior explicit while still joining the dynamic values centrally.

@hugovk

hugovk commented Jun 12, 2026

Copy link
Copy Markdown
Member

This now has a conflict after the merge of #406.

e-q added 2 commits June 12, 2026 19:27
@e-q
Quote remote release tool commands
2753604
@e-q
Use list-based remote command builders
7c5dbae 
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.
@e-q e-q force-pushed the quote-remote-release-commands branch from be6b586 to 7c5dbae Compare June 12, 2026 19:35
@e-q

e-q commented Jun 12, 2026

Copy link
Copy Markdown
Author

Rebased onto current main and resolved the conflict from #406 by preserving its -maxdepth 1 / current-user permission narrowing in the list-based command form.
Local tests, mypy, and lint pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Windows upload helper should quote URL-derived remote paths Bug: release tooling should quote remote command arguments

3 participants

@e-q @sethmlarson @hugovk