fix quoting in list representation of commands by PeterMocary · Pull Request #1510 · oamg/leapp-repository

PeterMocary · 2026-03-17T12:29:04Z

Removed redundant quotes included due to incorrect handling of command conversion to string representation in leapp framework's reporting module. The reporting module is fixed by RHEL-156521 and this patch is a followup that fixes usage of the module so that the commands are propagated to the leapp-report.txt correctly.

Note: This depends on the reporting module change in the leapp framework: oamg/leapp#919. For a better description of the problem see the RHEL-156521 and the framework PR description.

Example with both leapp and leapp-repository PRs

leapp-report.txt

Risk Factor: high (inhibitor) 
Title: Upgrade requires links in root directory to be relative 
Summary: After rebooting, parts of the upgrade process can fail if symbolic links in / point to absolute paths. 
Please change these links to relative ones. 
Related links: 
    - leapp upgrade stops with Inhibitor "Upgrade requires links in root directory to be relative": https://access.redhat.com/solutions/6989732 
Remediation: [command] sh -c 'ln -snf usr/bin /bin && ln -snf usr/lib /lib' 
Key: 3d895ad37ceaf4157864d439edb6bd75562061fa 
----------------------------------------

leapp-report.json

    {
      "audience": "sysadmin",
      "detail": {
        "external": [
          {
            "title": "leapp upgrade stops with Inhibitor \"Upgrade requires links in root directory to be relative\"",
            "url": "https://access.redhat.com/solutions/6989732"
          }
        ],
        "remediations": [
          {
            "context": [
              "sh",
              "-c",
              "ln -snf usr/bin /bin && ln -snf usr/lib /lib"
            ],
            "type": "command"
          }
        ]
      },
      "flags": [
        "inhibitor"
      ],
      "key": "3d895ad37ceaf4157864d439edb6bd75562061fa", 
      "severity": "high", 
      "summary": "After rebooting, parts of the upgrade process can fail if symbolic links in / point to absolute paths.\nPlease change these links to relative ones.", 
      "title": "Upgrade requires links in root directory to be relative", 
      "timeStamp": "2026-03-17T12:23:38.556773Z", 
      "hostname": "leapp-20260313125304", 
      "actor": "check_root_symlinks", 
      "id": "7d784af5714dfbe4a1d13e17ab97163a2139bf32336dd7c665756eca5d5b41d9", 
      "tags": [] 
    },

Both have correct equivalent representation of the command.

Jira: RHEL-155517, RHEL-155515

Removed redundant quotes included due to incorrect handling of command conversion to string representation in leapp framework's reporting module. The reporting module is fixed by RHEL-156521 and this patch is a followup that fixes usage of the module so that the commands are propagated to the leapp-report.txt correctly. Jira: RHEL-155517, RHEL-155515

github-actions · 2026-03-17T12:29:15Z

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the leapp-repository contribution and development guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

review please @oamg/developers to notify leapp developers of the review request
/packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
However, here are additional useful commands for packit:

/packit test to re-run manually the default tests
/packit retest-failed to re-run failed tests manually
/packit test oamg/leapp#42 to run tests with leapp builds for the leapp PR#42 (default is latest upstream - main - build)

Note that first time contributors cannot run tests automatically - they need to be started by a reviewer.

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

/packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
/packit test --labels beaker-minimal-8.10to9.4,kernel-rt-8.10to9.4 to schedule kernel-rt and beaker-minimal test sets for 8.10->9.4 upgrade path

See other labels for particular jobs defined in the .packit.yaml file.

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

PeterMocary · 2026-03-25T17:48:12Z

                command = ' '.join(['ln',
                                    '-snf',
-                                    os.path.relpath(item.target, '/'),
-                                    os.path.join('/', item.name)])
+                                    f"'{os.path.relpath(item.target, '/')}'",
+                                    f"'{os.path.join('/', item.name)}'"])
                commands.append(command)
-            rem_commands = [['sh', '-c', '"{}"'.format(' && '.join(commands))]]
+            rem_commands = [['bash', '-c', '{}'.format(' && '.join(commands))]]


This is a problematic place, the quoting in the leapp framework will only reach the top level. The commands in the subshell are on the developer to handle. Here, I've wrapped the paths in ', this works in most of the cases but as soon sa the path contains a ' the command won't work.

This could be handled by simply calling shlex.quote, but that reaches the same problem with ugly quoting as described in the framework PR - there is also a custom quoting function proposed in the PR. So after we decide what we want to use, I'll replace the ' here with shlex or the custom function.

@PeterMocary actually, this one will be broken in the current state for several possible symlinks. see this:

Remediation: [command] bash -c "ln -snf 'usr/sbin' '/sbin' && ln -snf 'usr/lib' '/lib' && ln -snf 'usr/lib64' '/lib64' && ln -snf 'usr/sbin' '/mi'amiga' && ln -snf 'usr/sbin' '/miqu\"oloool' && ln -snf 'usr/sbin' '/mark \!mark'"

(the one with single quote ' and !)

But as I merged already the leapp framework PR, we are now anyway in broken state even if we do not merge this change. Merging the PR will fix more issues actually and we will focus on corner-cases in a follow up.

PeterMocary · 2026-03-25T18:22:53Z

I searched for the places that might be affected by the changes in leapp framework, hopefully catching everything. After we decide what to use in framework, I'll update the changes that depend on the decision.

PeterMocary · 2026-03-25T18:59:43Z

        remediation_commands = [
            f"sed -i 's/^plugins=0/plugins=1/' '{dnf_conf_path}'"
            f"sed -i 's/^enabled=0/enabled=1/' '{rhsm_plugin_conf}'"
            f"sed -i 's/^enabled=0/enabled=1/' '{product_id_plugin_conf}'"
        ]

        reporting.create_report([
            reporting.Title('Required DNF plugins are not being loaded.'),
            reporting.Summary(
                'The following DNF plugins are not being loaded: {}'.format(missing_required_plugins_text)
            ),
            reporting.Remediation(
                hint=(
                    'If you have DNF plugins globally disabled, please enable them by editing the {0}. '
                    'Individually, the DNF plugins can be enabled in their corresponding configurations found at: {1}'
                    .format(dnf_conf_path, plugin_configs_dir)
                ),
                # Provide all commands as one due to problems with satellites
-                commands=[['bash', '-c', '"{0}"'.format('; '.join(remediation_commands))]]
+                commands=[['bash', '-c', '{0}'.format('; '.join(remediation_commands))]]


Here is a mention of satellite problems, not sure what is meant by it. The command could be split, but works ok as is. However, it has to be changed if we decide to use shlex.quote only without the custom change that handles ' escaping in a pretty way. See the framework PR.

Update: since we are moving forward with the double quoting when there is ' in the string, this will work as is namely thanks to the known paths and no special characters in the sed commands.

pirat89

missing updated spec file. without the update framework the solution is broken.

PeterMocary · 2026-03-30T10:39:52Z

Fixed unit tests and updated spec file to require the updated leapp framework.

PeterMocary · 2026-03-31T08:24:27Z

/packit copr-build

pirat89 · 2026-03-31T15:47:11Z

/packit copr-build

pirat89

there are actually still some broken commands (at least one) but merging this PR will fix more problems at this moment than causing if we do not merge it. So merging and we will track the corner cases in a followup PR.

pirat89 · 2026-03-31T16:11:09Z

                command = ' '.join(['ln',
                                    '-snf',
-                                    os.path.relpath(item.target, '/'),
-                                    os.path.join('/', item.name)])
+                                    f"'{os.path.relpath(item.target, '/')}'",
+                                    f"'{os.path.join('/', item.name)}'"])
                commands.append(command)
-            rem_commands = [['sh', '-c', '"{}"'.format(' && '.join(commands))]]
+            rem_commands = [['bash', '-c', '{}'.format(' && '.join(commands))]]


@PeterMocary actually, this one will be broken in the current state for several possible symlinks. see this:

Remediation: [command] bash -c "ln -snf 'usr/sbin' '/sbin' && ln -snf 'usr/lib' '/lib' && ln -snf 'usr/lib64' '/lib64' && ln -snf 'usr/sbin' '/mi'amiga' && ln -snf 'usr/sbin' '/miqu\"oloool' && ln -snf 'usr/sbin' '/mark \!mark'"

(the one with single quote ' and !)

But as I merged already the leapp framework PR, we are now anyway in broken state even if we do not merge this change. Merging the PR will fix more issues actually and we will focus on corner-cases in a follow up.

pirat89 added Requires Leapp PR The patch has to be merged/tested with Leapp PR bug Something isn't working labels Mar 19, 2026

pirat89 added this to the 8.10/9.9 milestone Mar 19, 2026

PeterMocary commented Mar 25, 2026

View reviewed changes

Comment thread repos/system_upgrade/common/actors/verifydialogs/libraries/verifydialogs.py

PeterMocary commented Mar 25, 2026

View reviewed changes

Comment thread repos/system_upgrade/el9toel10/actors/inhibitcgroupsv1/libraries/inhibitcgroupsv1.py Outdated

PeterMocary commented Mar 25, 2026

View reviewed changes

PeterMocary mentioned this pull request Mar 25, 2026

reporting: fix incorrect converting of list based commands oamg/leapp#919

Merged

PeterMocary commented Mar 25, 2026

View reviewed changes

pirat89 requested changes Mar 26, 2026

View reviewed changes

PeterMocary force-pushed the fix-command-quoting-in-reports branch 2 times, most recently from d74b5aa to 5d99386 Compare March 30, 2026 10:23

fixup! fix quoting in list representation of commands

9a5e0ef

PeterMocary force-pushed the fix-command-quoting-in-reports branch from 5d99386 to 9a5e0ef Compare March 30, 2026 11:05

PeterMocary marked this pull request as ready for review March 31, 2026 15:26

pirat89 approved these changes Mar 31, 2026

View reviewed changes

pirat89 merged commit a71fb36 into oamg:main Mar 31, 2026
13 of 23 checks passed

PeterMocary added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Apr 2, 2026

PeterMocary mentioned this pull request Apr 2, 2026

fix(checkyumpluginsenabled): correctly create remediation command #1518

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix quoting in list representation of commands#1510

fix quoting in list representation of commands#1510
pirat89 merged 2 commits into
oamg:mainfrom
PeterMocary:fix-command-quoting-in-reports

PeterMocary commented Mar 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

PeterMocary Mar 25, 2026

Uh oh!

pirat89 Mar 31, 2026

Uh oh!

PeterMocary commented Mar 25, 2026

Uh oh!

PeterMocary Mar 25, 2026 •

edited

Loading

Uh oh!

pirat89 left a comment

Uh oh!

PeterMocary commented Mar 30, 2026

Uh oh!

PeterMocary commented Mar 31, 2026

Uh oh!

pirat89 commented Mar 31, 2026

Uh oh!

pirat89 left a comment

Uh oh!

pirat89 Mar 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

PeterMocary commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Note: This depends on the reporting module change in the leapp framework: oamg/leapp#919. For a better description of the problem see the RHEL-156521 and the framework PR description.

Example with both leapp and leapp-repository PRs

Uh oh!

github-actions Bot commented Mar 17, 2026

Thank you for contributing to the Leapp project!

Uh oh!

Uh oh!

Uh oh!

PeterMocary Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

pirat89 Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

PeterMocary commented Mar 25, 2026

Uh oh!

PeterMocary Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pirat89 left a comment

Choose a reason for hiding this comment

Uh oh!

PeterMocary commented Mar 30, 2026

Uh oh!

PeterMocary commented Mar 31, 2026

Uh oh!

pirat89 commented Mar 31, 2026

Uh oh!

pirat89 left a comment

Choose a reason for hiding this comment

Uh oh!

pirat89 Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PeterMocary commented Mar 17, 2026 •

edited

Loading

PeterMocary Mar 25, 2026 •

edited

Loading