Skip to content

pkg/cliwrappers: shell-escape log messages#79

Merged
chmeliik merged 1 commit intomainfrom
shellquote-logs
Mar 24, 2026
Merged

pkg/cliwrappers: shell-escape log messages#79
chmeliik merged 1 commit intomainfrom
shellquote-logs

Conversation

@chmeliik
Copy link
Copy Markdown
Contributor

The commands we execute may include shell control characters (most commonly whitespace). If we log the arguments separated by space, the log messages will look confusing and will not be copy-paste-able into a terminal.

Shell-quote the arguments that need quoting to make the log messages copy-paste-able. Use a similar approach as alessio/shellescape 1 or shlex 2 from the Python standard library.

Compared to those, use a slighly nicer escape style for single quotes. Our approach:

it's => 'it'\''s'

Their approach:

it's => 'it'"'"'s'

Assisted-by: Claude

@chmeliik chmeliik requested a review from a team as a code owner March 13, 2026 12:48
@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Mar 13, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Mar 13, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@chmeliik
Copy link
Copy Markdown
Contributor Author

This is in response to Qodo's review on an older PR, where I dismissed it, saying that hermetic builds will most likely need shell-quoting anyway.

I'm happy to have found a solution that does not require executing bash -c 'ip link set lo up && buildah build ...', but that means Qodo's point about unquoted shell strings in log messages still stands. This PR addresses it.

@chmeliik chmeliik force-pushed the shellquote-logs branch 2 times, most recently from e07bbf5 to 8ff93d8 Compare March 17, 2026 09:07
The commands we execute may include shell control characters
(most commonly whitespace). If we log the arguments separated by space,
the log messages will look confusing and will not be copy-paste-able
into a terminal.

Shell-quote the arguments that need quoting to make the log messages
copy-paste-able. Use a similar approach as alessio/shellescape [1]
or shlex [2] from the Python standard library.

Compared to those, use a slighly nicer escape style for single quotes.
Our approach:

    it's => 'it'\''s'

Their approach:

    it's => 'it'"'"'s'

[1]: https://github.com/alessio/shellescape/blob/59ee74454256aaf5478a6283be4c480c13fd3152/shellescape.go#L26-L42
[2]: https://github.com/python/cpython/blob/485699216f2186c63f85fc546301e5edbe6b2f22/Lib/shlex.py#L320-L338

Assisted-by: Claude
Signed-off-by: Adam Cmiel <acmiel@redhat.com>
@chmeliik chmeliik merged commit 513a39f into main Mar 24, 2026
7 checks passed
@chmeliik chmeliik deleted the shellquote-logs branch March 24, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants