Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThree shell scripts improve strict-mode handling, diagnostics, variable expansion, and cleanup. GRUB SMBIOS processing becomes opt-in and applies serial parsing only to QEMU systems. ChangesShell diagnostics and error handling
Opt-in SMBIOS command-line handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant grub_mkconfig
participant smbios_cmdline_config
participant smbios_reader
participant SMBIOS
grub_mkconfig->>smbios_cmdline_config: evaluate GRUB_ENABLE_DM_SMBIOS_READER
smbios_cmdline_config->>grub_mkconfig: append dm_smbios_extra when enabled
grub_mkconfig->>smbios_reader: run reader with exported setting
smbios_reader->>SMBIOS: read Type 1 Manufacturer
smbios_reader->>SMBIOS: read serial only when Manufacturer is QEMU
smbios_reader->>grub_mkconfig: extract dm-cmdline data
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@usr/libexec/vm-config-dist/mount-shared`:
- Around line 77-78: Update the error-reporting block around virtiofs_error and
ninep_error so every line of each captured multiline diagnostic receives its
corresponding INFO prefix before printing. Preserve the separate virtiofs and 9p
labels and the existing block output structure.
- Around line 74-76: Update the informational messages in the shared-folder
mount failure path to use neutral wording that only states /mnt/shared could not
be mounted; remove the unsupported claim that the hypervisor exported no shared
folder while preserving the note about the expected unconfigured-share case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 744f9415-01a2-44c6-bacf-ec4e3a960114
📒 Files selected for processing (1)
usr/libexec/vm-config-dist/mount-shared
assisted-by-ai
pushed a commit
that referenced
this pull request
Aug 2, 2026
Both from CodeRabbit on PR #8; verified against the code before applying. - The message claimed "no shared folder exported by the hypervisor". Two failed mounts do not prove that. The unit is gated on the hypervisor, not on share configuration, so an exported-but-BROKEN share reaches the same branch. Naming the wrong cause sends a reader down the wrong path -- the precise fault this change set out to fix elsewhere. Now reports what was observed: could not mount, leaving it unmounted, attempts follow. - Only the first line of each captured error carried the INFO prefix. mount emits multiline diagnostics (a "dmesg(1) may have more information" continuation is routine), so continuations appeared as bare journal lines -- exactly the loose output this change exists to stop producing. Every line is prefixed now. Verified: a real two-line mount error yields zero unprefixed lines, and the three mount situations still behave as before (silent on success either transport, INFO-only when nothing mounts). Co-Authored-By: Claude <noreply@anthropic.com>
Files: usr/bin/{spice-vdagent.dist,vbox-guest-installer}.
Real nounset trap, on the installed-system path
------------------------------------------------
vbox-guest-installer reads dist_build_virtualbox and output_command
UNCONDITIONALLY. Both are set by the IMAGE BUILD, so on an installed system
neither exists -- and under nounset the script aborted there, including on the
fail-closed path whose entire job is to report the value of the build variable.
Also
----
- R-120: three 'rm' -> 'safe-rm', no fallback.
- style-ok: allow-exec on spice-vdagent.dist. It is a .dist wrapper whose job is
to set two environment variables and then BECOME the real spice-vdagent;
forking would leave a shell between the session and the agent, and the
agent's exit status and signals would stop being the real ones.
- R-090 'command -v' -> 'has'; R-034 echo -> printf.
- SC2046 fixed by quoting a substitution that yields exactly 'yes' or 'no'.
- SC2119/SC2120 waived on the kernel-selection block, which is vendored from
VirtualBox's own guest-additions tooling -- different indentation, uppercase
names, single-'#' comments. Rewriting vendored code is not this pass's
business. SC2317 file-wide, since nearly every function here is reached only
through the option dispatch.
Testing
-------
Gate exit 0 in an ISOLATED checkout.
New harness at ~/strict-pass-tests/vcd-test/. Seven cases.
The first version ran everything unprivileged and reported four vbox cases at
exit 112 -- the 'Must run as root' guard -- so the fail-closed decision this
package exists for was never reached at all. The harness now runs those cases
under sudo as well, which is what gets past that guard.
7 same, 0 differing
Canary: removing the two build-variable defaults reproduces the abort on
exactly the installed-system case -- 6 same, 1 differing. Note that it fires
ONLY on the root variant, which is the one the first harness could not reach.
NOTE: this repo carries uncommitted changes to etc/default/grub.d/
99_smbios-cmdline.cfg and etc/grub.d/01_smbios-reader from another session.
They are untouched, and this commit is path-scoped to usr/bin.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@usr/bin/vbox-guest-installer`:
- Around line 22-23: Set the default value of output_command to a valid output
command such as echo, so invocations that expand ${output_command} with a
message execute the command rather than treating the message as the command
name. Leave dist_build_virtualbox initialization unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44fb5663-1b70-448c-a766-8cc157bc6da8
📒 Files selected for processing (3)
usr/bin/spice-vdagent.distusr/bin/vbox-guest-installerusr/libexec/vm-config-dist/mount-shared
🚧 Files skipped from review as they are similar to previous changes (1)
- usr/libexec/vm-config-dist/mount-shared
The reader turns a firmware-supplied string into kernel command line, and the
kernel command line is the part of the boot chain Secure Boot does not
authenticate. It exists only for the QEMU boot-test harness, so a released image
must not carry it.
- etc/grub.d/01_smbios-reader: emit nothing unless GRUB_ENABLE_DM_SMBIOS_READER
is "true". derivative-maker's '--smbios-reader true' writes the drop-in that
sets it; default off.
- etc/default/grub.d/99_smbios-cmdline.cfg: same guard for the
${dm_smbios_extra} placeholder, and export the variable here so the /etc/grub.d
helper (a child process, which grub-mkconfig hands only its own allow-list of
GRUB_* variables) sees the same signal. Without the export a plain assignment
added the placeholder while no reader was emitted -- inert, but the two halves
must mean the same thing.
- Second layer for an image that IS built with the reader: read the serial number
only when SMBIOS Type 1 Manufacturer says QEMU. Detectability, not a boundary --
the same vendor DMI tool sets Manufacturer -- but it forces the machine to
advertise itself as a QEMU guest instead of hiding in a field nobody reads.
Reading Manufacturer first also drops the 'error: failed to retrieve the
structure field.' GRUB prints when the serial number is unset.
- Sentinel comments delimit the block, so dm-grub-smbios-tests compares the whole
reader rather than a prefix.
Verified on a built image, no rebuild: patched the generated /boot/grub/grub.cfg
and booted. Injection accepted under QEMU (32 KB serial), refused under
-smbios type=1,manufacturer='Dell Inc.' (0 bytes), normal boot unaffected
(0 bytes). The unhardened reader accepts the spoofed vendor, so the test can fail.
Opt-in verified through the image's own update-grub in a chroot: absent 0 reader
lines / 0 placeholders, enabled 5 / 11.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
|
Added commit ec04d7c: The reader turns a firmware-supplied string into kernel command line, and the kernel command line is the part of the boot chain Secure Boot does not authenticate. This package is a dependency of
Verified on a built image, no rebuild: patched the generated Parent build option: org-ai-assisted/derivative-maker#87 Generated with Claude Code |
The branch was created from a stale local master, so its PR diff showed master's newer commits -- including a changelog version bump -- as REMOVALS. Merging rather than rebasing, per the no-history-rewrites rule.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@etc/grub.d/01_smbios-reader`:
- Around line 109-113: Update the QEMU branch after the vendor check to read
Type 1 offset 7 with smbios --get-byte first, and invoke --get-string 7 only
when the returned index is non-zero; keep the existing dm_smbios_oem parsing
behavior for valid serial fields.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a4b5fe4-a8c0-4cb3-a221-fe6bcd6e9917
📒 Files selected for processing (2)
etc/default/grub.d/99_smbios-cmdline.cfgetc/grub.d/01_smbios-reader
…y one
An ai-review pass found that defaulting output_command to EMPTY did not fix
anything -- it moved the failure.
output_command="${output_command:-}"
...
${output_command} "$0 ERROR: ... failing closed." >&2
With an empty value the expansion vanishes and bash runs the MESSAGE ITSELF as
a command. Demonstrated directly:
$ output_command=""; ${output_command} "some message here"
bash: some message here: command not found
So every message became 'command not found' (127), and under the errexit this
pass added that fired the ERR trap -- which uses ${output_command} too and
failed the same way. The abort simply moved from "unbound variable" to
"command not found".
output_command now defaults to a small printf wrapper:
output_default() {
printf '%s\n' "$@"
}
output_command="${output_command:-output_default}"
A wrapper rather than 'echo' because it appends the newline the call sites
expect and does not interpret a '%' in the message.
Testing
-------
Gate exit 0 in an ISOLATED checkout.
8 same, 0 differing
One case added: the fail-closed path run AS ROOT with NO output_command at all
-- the installed-system case. Every existing root case supplied
'output_command=echo', so the DEFAULT was never exercised, which is exactly why
an empty default looked fine.
_get_newest_kernel_debian reads $1 on its first loop iteration, to seed "find a version bigger than this". Its ONE call site -- the chroot branch that sets TARGET_VER -- passes no argument at all. Under the nounset added by R-010 that aborts on the first /boot/config-* found, which is every run on a normal image build, and the ERR handler fires before Guest Additions are ever installed. Empty is what $1 expanded to before, and it is meaningful here: "bigger than nothing" makes the first kernel seen the baseline, which is exactly what a no-argument call wants. Found by codex. Testing ------- Gate exit 0 in an ISOLATED checkout. The function is driven directly with /boot bind-mounted from a fixture, so the result does not depend on which kernels the test host happens to have: 4 pass, 0 fail no argument, one kernel (the real call site) old ABORTS, new 6.1.0-13-amd64 no argument, three kernels -> newest wins old ABORTS, new 6.1.0-18-amd64 explicit argument still works unchanged no /boot/config-* at all unchanged The two no-argument rows assert the OLD version aborts AND what the new one returned -- the three-kernel case exists so that "does not abort" cannot pass a version that picks the wrong kernel. Canary: restoring the bare $1 gives 2 pass / 2 fail, and the two that fail are exactly the no-argument rows.
Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3).
Asking for the string when the index is 0 makes GRUB print
error: failed to retrieve the structure field.
on the console. That is EVERY ordinary boot of an image built with the reader:
the QEMU gate passes on any QEMU guest, and the serial is set only when a tester
injects. Probe the index byte first -- that call never errors -- and read the
string only when it is non-zero.
Measured on real GRUB, both cases:
no injection serial_index=[0] unguarded -> error printed, rc 29
guarded -> read skipped, silent
injection serial_index=[4] both read [dm-cmdline=console=ttyS0]
Also corrects the comment above the gate. It claimed reading Manufacturer first
keeps an ordinary boot quiet; that holds on bare metal, where the gate fails, but
not under QEMU, where it passes. The index check is what covers the QEMU case.
Found by coderabbit on PR #8.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Aug 8, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
usr/share/vm-config-dist/50_dm-smbios-reader.cfg, copied into /etc/default/grub.d by derivative-maker's 3500_install-packages under '--smbios-reader true'. Not installed there by this package: a released image must never receive it. Ships as a file rather than being printf-ed inside the build step, so the configuration can be read, reviewed and diffed where it lives instead of depending on a build script's quoting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reader is OPT-IN and inert by default: 01_smbios-reader exits 0 unless GRUB_ENABLE_DM_SMBIOS_READER is 'true', and a default install carries neither the reader nor the placeholder. A reviewer noted the guard only changes the INPUT to a future update-grub, and that nothing in this package regenerates grub.cfg -- so a machine could keep reading SMBIOS after the setting changed. Documented rather than fixed with a postinst trigger, because opt-in narrows it to one case: someone who opted IN and later opts OUT still has the reader block in their existing /boot/grub/grub.cfg until grub-mkconfig runs again. A machine that never opted in has no block to clear. Adding a boot-config regeneration to every install of this package to serve a deliberate admin opt-out would be disproportionate. Per the maintainer: no way was found to use SMBIOS injection securely, so the opt-in IS the mitigation -- there is no further hardening to design here.
8d1580c inserted its paragraph in the middle of a sentence, leaving "a released image" running straight into "OPTING OUT needs a manual 'sudo update-grub'", and the rest of the original sentence orphaned as a new paragraph beginning "carries neither the reader nor this placeholder." Both statements are correct and both are kept; they are now separate paragraphs in the order they make sense: what a released image carries, then what opting out afterwards requires. The enabling example was also stale. It printed a drop-in by hand to 50_dm-boot-test.cfg, which is neither how it is done nor the current filename: derivative-maker copies this package's static usr/share/vm-config-dist/50_dm-smbios-reader.cfg under '--smbios-reader true'. Configuration files are not generated by hand, so the comment no longer shows that as the way to enable it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eglinfo can hang indefinitely when EGL/DRM probing never returns. On a headless VM or Qubes AppVM with no accelerated GL (no /dev/dri), the GBM platform fails fast but a later platform init blocks forever. This file is sourced by every login shell, so an unbounded eglinfo wedges every interactive shell -- observed as ~86s added to every shell startup. Wrap the probe in `timeout 5`. On timeout the captured output is empty, which falls through to the existing software-rendering path -- the correct result in exactly that no-acceleration environment. Guard on `command -v timeout` in case coreutils' timeout is ever absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…endering Replace the inline eglinfo probe (added in the previous commit) with a call to the new standalone helper-scripts 'detect-software-rendering', so the bounded best-effort renderer guess has a single home and is reusable. vm-config-dist already Depends on helper-scripts. The 'command -v' guard keeps this a no-op if an older helper-scripts without the command is installed transiently during an upgrade -- the same do-nothing outcome as a missing eglinfo. Behaviour is otherwise unchanged: QMLSCENE_DEVICE is set to softwarecontext only when the renderer is guessed to be llvmpipe and the variable is unset (an explicit user choice wins). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…olicy Only a confident 'software' (llvmpipe) guess forces softwarecontext; 'accelerated' and 'unknown' err toward acceleration-available and leave QMLSCENE_DEVICE unset. Documents the deliberate choice (forcing software where HW accel exists is worse); no behaviour change.
…_DEVICE
Test QMLSCENE_DEVICE unset via ${QMLSCENE_DEVICE+x} instead of -z "${:-}", which
treated an explicit QMLSCENE_DEVICE="" the same as unset and overwrote it. Matches
the 'explicit user choice wins' intent. (ai-review)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A login-shell fragment should not leak the helper's stderr; drop it in the command substitution.
…nsumer The comment restated the helper's internals (eglinfo probe, llvmpipe, timeout), which drift when the helper changes -- and already had (it now short-circuits before eglinfo). Keep only this consumer's policy and the helper's stable output contract (software/accelerated/unknown), and point to the helper for how it decides.
…C1091 disable
- spice-vdagent.dist: use source
"${HELPER_SCRIPTS_PATH:-}"/usr/libexec/helper-scripts/has.sh; drop the dead
'# shellcheck disable=SC1091' and the stale sibling note.
- completed pre-existing whole-file style debt (R-010: add export LC_ALL=C).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… whole-file gate debt
configure-dynamic-resolution:
- source: get_colors.sh, log_run_die.sh, as_root.sh now via
source "${HELPER_SCRIPTS_PATH:-}"/usr/libexec/helper-scripts/<f>
(no SC1091 disable was present to drop).
- R-010: complete the strict-mode block (was 4/7) by adding
shopt -s inherit_errexit, shopt -s shift_verbose, export LC_ALL=C.
inherit_errexit audit: every command substitution reviewed. The
validate_bool_opt / validate_resolution_opt / generated_file_header
helpers all exit 0 unconditionally, so inherit_errexit cannot make
their subs abort. The remaining subs (basename, stcatn, id, sed)
represent genuine failures that SHOULD abort rather than proceed with
garbage. No '|| true' guard is needed for any sub.
- R-090 (x5): replace the 'command -v id/stcatn/sed/overwrite/grep'
pre-flight with the shared 'die_if_not_has id stcatn sed overwrite
grep' (has/die_if_not_has already in scope via log_run_die.sh).
- R-161: line 114 'grep -qF' short quiet cluster -> long form
'grep --quiet --fixed-strings' (here-string input, not a pipe).
- SC2250 (x5 refs): brace ${my_base_name}, ${default_resolution},
${SUDO_USER} (x2), ${xdg_runtime_dir}.
No waiver added.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 6, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 7, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 8, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 9, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 9, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 9, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assisted-by-ai
pushed a commit
to org-ai-assisted/derivative-maker
that referenced
this pull request
Sep 9, 2026
ISO copy of the reader, kept identical to the disk copy in vm-config-dist, plus the matching gitlink bump (00dd919..52d2087, reachable on org-ai-assisted). Type 1 offset 7 holds a string INDEX, and 0 means "no string" (DSP0134 6.1.3), so an unguarded --get-string 7 makes GRUB print 'error: failed to retrieve the structure field.' on every ordinary boot of an image built with the reader -- the QEMU gate passes on any QEMU guest, and the serial is set only when a tester injects. Probe the index byte first; read the string only when it is non-zero. Found by coderabbit on org-ai-assisted/vm-config-dist#8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mnt-shared-kvm.serviceis gated on the HYPERVISOR (ConditionVirtualization=|qemu|kvm), not on whether the host actually exported a share, so it runs on every plain QEMU/KVM guest. The|| truealready made a missing share non-fatal, butmountstill wrote to stderr, so every such boot logged:systemcheck's journal check then reports that as a WARNING. Observed on a clean boot of a stock Kicksecure qcow2 under qemu: the mount behaved exactly as designed, and the NOISE was the defect.Both attempts are now captured and the outcome reported honestly: silent when a share mounts (virtiofs or the 9p fallback), one INFO block when none was exported, with BOTH underlying errors preserved so a share that exists but is genuinely broken stays diagnosable -- swallowing them would trade a false alarm for a blind spot.
Verified synthetically (stubbed
mount, no VM, no rebuild) across all three situations: no share -> INFO only, zero baremount:lines on stderr; virtiofs present -> silent; virtiofs absent with 9p working -> silent, two attempts made.Also clears pre-existing style-gate debt in the touched file (strict-mode block,
;;placement).Generated with assistance from Claude Code.
Summary by CodeRabbit
New Features
Bug Fixes