snapshotmanager / boom-boot

Boom Boot Manager
GNU General Public License v2.0
30 stars 4 forks source link

Incorrect option parsing with --add-opts causes boot_id to change #13

Closed bmr-cymru closed 1 year ago

bmr-cymru commented 1 year ago

Some values passed to boom create --add-opts cause the BootParams parser to mistakenly classify options during boot entry loading, causing a shift in the entry's boot_id:

[root@localhost ~]# boom create --title "AddOpts Test" --root-lv fedora/root --add-opts "a b c"
Created entry with boot_id 8c9f0c0:
  title AddOpts Test       ^^^^^^^
  machine-id ed4dfa014ba442f0a8f84eb817003560
  version 6.0.7-301.fc37.x86_64
  linux /vmlinuz-6.0.7-301.fc37.x86_64
  initrd /initramfs-6.0.7-301.fc37.x86_64.img
  options root=/dev/fedora/root ro rd.lvm.lv=fedora/root a b c
  grub_users $grub_users
  grub_arg --unrestricted
  grub_class kernel
[root@localhost ~]# boom list
BootID  Version                  Name                     RootDevice      
2c7627b 6.0.7-301.fc37.x86_64    Fedora Linux             /dev/fedora/root
^^^^^^^

This results in the entry being marked read-only and prevents the entry from being deleted with the boom delete command:

[root@localhost ~]# boom delete 2c7627b
Cannot delete read-only boot entry: /boot/loader/entries/ed4dfa014ba442f0a8f84eb817003560-8c9f0c0-6.0.7-301.fc37.x86_64.conf

In this case 8c9f0c0 is the correct boot_id for the entry, but the additional options are mis-parsed during loading, causing the ID reported by boom to change. The additional option "c" is incorrectly parsed as a template-defined option:

[root@localhost ~]# boom list -VV --debug=all
...
DEBUG - Matched root_device=/dev/fedora/root to lvm_root_lv=fedora/root
DEBUG - Matched lvm_root_lv=fedora/root
DEBUG - Found add_opt: a
DEBUG - Found add_opt: b
# Missing "Found add_opt: c"
DEBUG - Parsed BootParams("6.0.7-301.fc37.x86_64", root_device="/dev/fedora/root", lvm_root_lv="fedora/root")
DEBUG - Loading Host profiles from /boot/boom/hosts
DEBUG - Loaded 0 host profiles
...

Additional options are identified by the helper function is_add():

        def is_add(opt):
            """Return ``True`` if ``opt`` was appended to this options line,
                and was not generated from the active ``OsProfile`` template,
                or from expansion of a bootloader environment variable.
            """
            def opt_in_expansion(opt):
                """Return ``True`` if ``opt`` is contained in the expansion of
                    a bootloader environment variable embedded in this entry's
                    options string.

                    :param opt: A kernel command line option.
                    :returns: ``True`` if ``opt`` is defined in a bootloader
                              environment variable, or ``False`` otherwise.
                    :rtype: bool
                """
                if GRUB2_EXPAND_ENV not in be.options:
                    return False
                return opt not in _expand_vars(be.options)
            if opt not in matches.keys():
                if opt not in be._osp.options:
                    if not opt_in_expansion(opt):
                        _log_debug_entry("Found add_opt: %s" % opt)
                        return True
            return False

The problem happens because if opt not in be._osp.options: applies a string membership test to the option, e.g.:

    if "c" not in "root=%{root_device} ro %{root_opts}":

Since "c" is a substring of the options string this evaluates to True and the option is considered to be template-supplied. This is incorrect and the options string should first be split into a list, and then the option tested against that list for membership:

    if "c" not in ["root=%{root_device}", "ro", "%{root_opts}"]:

The same error exists in the helper function opt_in_expansion().