- 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
174 lines
7.5 KiB
Diff
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
|
|
|