Skip to content

fix(reporting) use single quoting when converting remediation commands - #921

Merged
pirat89 merged 1 commit into
oamg:mainfrom
PeterMocary:simplify-command-conversion
Apr 16, 2026
Merged

fix(reporting) use single quoting when converting remediation commands#921
pirat89 merged 1 commit into
oamg:mainfrom
PeterMocary:simplify-command-conversion

Conversation

@PeterMocary

@PeterMocary PeterMocary commented Apr 13, 2026

Copy link
Copy Markdown
Member

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 (none of the current commands are affected as far as i could tell). Moreover, using shlex exclusively 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.

Bumps specifle version to 6.5
Jira: RHEL-156521

Related leapp-repository PR: PR#1520

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.
@PeterMocary
PeterMocary force-pushed the simplify-command-conversion branch from 1658232 to 5e9f55a Compare April 13, 2026 10:40
@oamg oamg deleted a comment from gemini-code-assist Bot Apr 13, 2026

@pirat89 pirat89 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed and tested on mtg. we agree with the change

@pirat89
pirat89 merged commit eaf4af1 into oamg:main Apr 16, 2026
27 checks passed
@pirat89 pirat89 added this to the 8.10/9.9 milestone Apr 16, 2026
@pirat89 pirat89 added changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant bug and removed changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant labels Apr 16, 2026
@matejmatuska matejmatuska mentioned this pull request Jul 29, 2026
pirat89 pushed a commit that referenced this pull request Jul 29, 2026
## Packaging
- Bump leapp-framework to 6.6 (#913, #919, #921, #929)

### Fixes
- Change how commands are converted for text based report from the list representation (#919, #921)
- Fix the crashing check of conflicts between actor config schemes (#925)

### Enhancements
- Introduce the format_list function for unified presentation of lists in reports and logs (#913)

## stdlib
### Enhancements
- Introduce the format_list function for unified presentation of lists in reports and logs (#913)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants