leapp/0010-fix-reporting-use-single-quoting-when-converting-rem.patch
Matej Matuska 0190620420 IPU 8.10 -> 9.9: CTC1 candidate 1
- Bump leapp-framework to 6.5
- Change how commands are converted for text based report from the list representation
- Introduce the format_list function for unified presentation of lists in reports and logs
- Resolves: RHEL-169278
2026-04-22 14:56:07 +02:00

174 lines
7.5 KiB
Diff

From eaf4af1a549b3cb675f49f03d46d9d5de362753f Mon Sep 17 00:00:00 2001
From: Peter Mocary <pmocary@redhat.com>
Date: Fri, 10 Apr 2026 21:51:42 +0200
Subject: [PATCH 10/10] fix(reporting): use single quoting when converting
remediation commands
The remediation commands are provided as a list of arguments that is
used in leapp-report.json. The same command is also converted to a
string that is used in leapp-report.txt and can be copy-pasted by the
user into shell.
Originally, the command conversion tried to make the resulting commands
prettier by enabling double quoting whenever there was a single quote in
an argument, while still trying to make the double quoted argument a
literal. This proved to be problematic approach due to multiple cases
that resulted with incorrect commands. Namely, when the command called
`bash -c` with a subcommand that would be evaluated by another
non-interactive shell, or when there was an exclamation mark that would
have different meaning in non-interactive and interactive shell and its
escaping was not possible unless single-quotes are used.
This patch makes the command conversion produce pretty commands in the
most cases, moreover, it makes sure the command is always correct (if
the list version is correct to begin with). This approach leaves
possibility to encounter unintuitive escaping in the resulting converted
command, although this can be prevented by carefully crafting the
command when creating the report.
Note that if there is a subcommand called in the converted command, the
report creator is responsible for its form and for the best resulting
command it should favor the double quoting of its arguments when
possible.
---
leapp/reporting/__init__.py | 28 +------------------
packaging/leapp.spec | 2 +-
tests/scripts/test_reporting.py | 48 +++++----------------------------
3 files changed, 9 insertions(+), 69 deletions(-)
diff --git a/leapp/reporting/__init__.py b/leapp/reporting/__init__.py
index 6bd0b49..43aa17d 100644
--- a/leapp/reporting/__init__.py
+++ b/leapp/reporting/__init__.py
@@ -275,32 +275,6 @@ def _guarantee_decoded_str(text, encoding='utf-8'):
return text
-def _quote_for_shell(s):
- """
- Quote a string for shell usage as a literal.
-
- This function quotes the given string in single-quotes using the
- shlex.quote. However, when the single-quote is present in the string it
- instead uses double-quotes, resulting in cleaner output (e.g. "don't"
- instead of 'don'"'"'t' that would be produced by shlex.quote).
-
- :param str s: String to quote for shell usage
- :return str: Quoted string safe for shell usage
- """
-
- if "'" not in s:
- return shlex.quote(s)
-
- # The string contains a single-quote, use double-quote style instead.
- # Escape following characters to make it literal: \ " $ ` !
- escaped = (s.replace('\\', '\\\\')
- .replace('"', '\\"')
- .replace('$', '\\$')
- .replace('`', '\\`')
- .replace('!', '\\!'))
- return '"{}"'.format(escaped)
-
-
class RemediationCommand(BaseRemediation):
def __init__(self, value=None):
if not isinstance(value, list):
@@ -310,7 +284,7 @@ class RemediationCommand(BaseRemediation):
def __repr__(self):
# NOTE(ivasilev) As the message can contain non-ascii characters let's deal with it properly.
# As per python practices repr has to return an encoded string
- quoted_command_args = [_quote_for_shell(_guarantee_encoded_str(c)) for c in self._value['context']]
+ quoted_command_args = [shlex.quote(_guarantee_encoded_str(c)) for c in self._value['context']]
return "[{}] {}".format(self._value['type'], ' '.join(quoted_command_args))
diff --git a/packaging/leapp.spec b/packaging/leapp.spec
index ba906cf..0fde883 100644
--- a/packaging/leapp.spec
+++ b/packaging/leapp.spec
@@ -13,7 +13,7 @@
# This is kind of help for more flexible development of leapp repository,
# so people do not have to wait for new official release of leapp to ensure
# it is installed/used the compatible one.
-%global framework_version 6.4
+%global framework_version 6.5
# IMPORTANT: everytime the requirements are changed, increment number by one
# - same for Provides in deps subpackage
diff --git a/tests/scripts/test_reporting.py b/tests/scripts/test_reporting.py
index 2a4e724..2010d29 100644
--- a/tests/scripts/test_reporting.py
+++ b/tests/scripts/test_reporting.py
@@ -23,7 +23,6 @@ from leapp.reporting import (
Title,
_add_to_dict,
_create_report_object,
- _quote_for_shell,
create_report_from_deprecation,
create_report_from_error,
)
@@ -113,40 +112,6 @@ def test_report_tags_and_flags():
assert Flags(["This is a new flag", Groups.INHIBITOR]).value == ["This is a new flag", "inhibitor"]
-@pytest.mark.parametrize('string, expected_quoted', [
- # No quoting needed
- ('path/to_the/file-name.txt', 'path/to_the/file-name.txt'),
- ('user@host', 'user@host'),
- ('value=123', 'value=123'),
- ('a:b:c', 'a:b:c'),
- ('99%', '99%'),
- ('a+b', 'a+b'),
-
- # Single-quote style (no single quote in string)
- ('', "''"),
- ('file with spaces.txt', "'file with spaces.txt'"),
- ('$(whoami)', "'$(whoami)'"),
- ('$USER', "'$USER'"),
- ('back`tick', "'back`tick'"),
- ('double"quote', "'double\"quote'"),
- ('exclaim!', "'exclaim!'"),
- ('back\\slash', "'back\\slash'"),
- ('new\nline', "'new\nline'"),
- ('key="value"', "'key=\"value\"'"),
-
- # Double-quote style (a single quote in string)
- ("It's fine", "\"It's fine\""),
- ("It's `whoami`", "\"It's \\`whoami\\`\""),
- ("User's home: $HOME", "\"User's home: \\$HOME\""),
- ("Don't panic!", "\"Don't panic\\!\""),
- ("s/'/\"/g", "\"s/'/\\\"/g\""),
- ("sed -i 's/'/\"/g' file.txt", "\"sed -i 's/'/\\\"/g' file.txt\""),
-])
-def test_quote_for_shell(string, expected_quoted):
- """Test that _quote_for_shell properly quotes strings for shell usage."""
- assert _quote_for_shell(string) == expected_quoted
-
-
@pytest.mark.parametrize('command, expected_command_form', [
(['cmd'], "cmd"),
(['cmd', ''], "cmd ''"),
@@ -174,12 +139,13 @@ def test_quote_for_shell(string, expected_quoted):
# Quoting '`'
(['usermod', '-aG', 'wheel', '`whoami`'], "usermod -aG wheel '`whoami`'"),
- # Double-quote style when single quote is present
- (['echo', "It's fine"], "echo \"It's fine\""),
- (['mv', "user's file.txt", 'newfile.txt'], "mv \"user's file.txt\" newfile.txt"),
- (['sed', '-i', "s/'/\"/g", 'file.txt'], "sed -i \"s/'/\\\"/g\" file.txt"),
- (['bash', '-c', "echo \"user's home: $HOME\""], "bash -c \"echo \\\"user's home: \\$HOME\\\"\""),
- (['grep', "can't find", '/var/log/some report.log'], "grep \"can't find\" '/var/log/some report.log'"),
+ # Single quote in argument — shlex.quote uses the '"'"' pattern
+ (['echo', "It's fine"], "echo 'It'\"'\"'s fine'"),
+ (['mv', "user's file.txt", 'newfile.txt'], "mv 'user'\"'\"'s file.txt' newfile.txt"),
+ (['sed', '-i', "s/'/\"/g", 'file.txt'], "sed -i 's/'\"'\"'/\"/g' file.txt"),
+ (['bash', '-c', "echo \"user's home: $HOME\""], "bash -c 'echo \"user'\"'\"'s home: $HOME\"'"),
+ (['grep', "can't find", '/var/log/some report.log'], "grep 'can'\"'\"'t find' '/var/log/some report.log'"),
+ (['echo', "Don't panic!"], "echo 'Don'\"'\"'t panic!'"),
])
def test_remediation_command_repr_quoting(command, expected_command_form):
"""Test that RemediationCommand.__repr__ properly quotes arguments with special characters."""
--
2.53.0