Skip to content

fix(sprout): model-driven bug hunt - #98

Merged
azenla merged 19 commits into
mainfrom
azenla/fix/bug-hunt
Sep 27, 2026
Merged

azenla merged 19 commits into
mainfrom
azenla/fix/bug-hunt

Conversation

@azenla

@azenla azenla commented Sep 27, 2026

Copy link
Copy Markdown
Member

I ran Opus 5.5 across Sprout to find various bugs. Sprout is used often by many of us at Edera but through specific pathways. This should fix a number of issues that could occur in situations that are abnormal to Edera users.

@azenla
azenla requested a review from a team September 27, 2026 01:23
@azenla
azenla force-pushed the azenla/fix/bug-hunt branch from eac78c4 to 2cea91e Compare September 27, 2026 01:24
File names are lowercased before matching kernel prefixes, but the
arm64 prefix was "Image", so it could never match and arm64 kernels
were not autoconfigured. Use a lowercase "image" prefix.
Some firmware signals the key event without a key being available.
This previously aborted boot with "no key was pressed"; treat it as a
no-op and keep waiting instead.
The check for unprintable leading arguments added by firmware used
c < 0x1f, which missed 0x1f itself. Use is_ascii_control() instead.
The hook was installed before retaining the shim protocol and reading
the image, so an error in either step returned without uninstalling
it. Install it immediately before LoadImage instead.
A LoaderConfigTimeout or LoaderConfigTimeoutOneShot value that is not a
number or a known keyword made Sprout abort boot. Log a warning and
treat the variable as unset instead, matching systemd-boot.
EFI string variables written without a trailing NUL, or with extra NUL
padding, failed to decode and were silently treated as unset. Decode up
to the first NUL and terminate the string ourselves, as systemd-boot
does.

Because empty values now decode to an empty string rather than failing,
treat empty LoaderEntryDefault and LoaderEntryOneShot values as unset so
an empty one-shot entry doesn't fail to match any entry.
If the security hook was installed while already installed, the
protocols contained Sprout's own handlers, which were then saved as the
"original" handlers. A verification failure would then call back into
the hook forever. Keep the existing saved state in that case.
The firmware passes BootPolicy as a UEFI BOOLEAN (a u8). Declaring it
as a Rust bool in the extern signature is undefined behavior for any
value other than 0 or 1. Use uefi_raw::Boolean instead.
The chainload action held the child image's LoadedImage protocol open
exclusively while calling start_image. Any image that opens its own
LoadedImage protocol exclusively, such as another uefi-rs application
or Sprout itself, then failed with ACCESS_DENIED. Scope the protocol to
setting the load options.
Autoconfigured BLS actions use "$root\\$initrd" as the initrd path.
For BLS entries without an initrd line this stamps to the root of the
filesystem, and Sprout tried to read that directory as the initrd,
failing the boot. Treat a path that resolves to no file as no initrd.
Autoconfigure detects BLS when either \loader\loader.conf or
\loader\entries exists, but the BLS generator required the entries
directory and failed the boot without it. This broke ESPs that only
have a loader.conf, such as UKI-only systemd-boot setups. Generate no
entries when the directory is missing.
A single BLS entry that could not be read, was not valid UTF-8, or
failed to parse aborted entry generation and therefore the whole boot,
contrary to the generator's documented best-effort behavior. Log a
warning and skip the entry instead.
The '~' check ran after the end-of-string check, so "1~rc1" compared as
newer than "1". Per the UAPI version format specification, '~' must be
handled first so a pre-release sorts older than the release it
precedes. An empty string is kept as the oldest version, matching
systemd's strverscmp_improved.
When only one version had a digit at the current position, both sides
were parsed as numbers and the letter side read as 0. "0" then compared
equal to "a" and the result depended on what followed. Per the
specification, the side with digits is newer.
Numeric segments were parsed into a saturating u64, so any two numbers
above u64::MAX compared equal. Compare the digits instead, by length
and then digit by digit, as systemd does.
Sort keys were compared as Options, so an entry without a sort-key
sorted before one with a sort-key, and machine-id and version were
compared even when neither entry had a sort-key. Per the Boot Loader
Specification and systemd-boot, an entry with a sort-key sorts first,
and machine-id and version are only used when both entries have one.
compare_versions_optional treated a missing version as newer than any
version, so among entries with equal sort-key and machine-id, an entry
without a version sorted first. Treat a missing version as the oldest,
matching systemd's strverscmp_improved.
The late phase is documented to run after the entry is chosen but
before its actions are executed, but it ran before the boot menu was
shown. Run it after the entry is selected and reported to the
bootloader interface, right before executing the entry's actions.
If the device path or filesystem protocol of any detected filesystem
could not be opened, autoconfiguration failed entirely and boot was
aborted. Log a warning and skip that filesystem instead, so the other
filesystems are still autoconfigured.
@azenla
azenla force-pushed the azenla/fix/bug-hunt branch from 2cea91e to 6cb1737 Compare September 27, 2026 01:55
@azenla
azenla merged commit 2da1cea into main Sep 27, 2026
11 checks passed
@azenla
azenla deleted the azenla/fix/bug-hunt branch September 27, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant