Deep Review: 20260420-120300-pr-4831

# re-probe to see an inconsistent ext4 checksum and delete the symlink.
# BusyBox blkid doesn't support --label, so we parse the output instead.
# The leading whitespace anchor on LABEL= avoids matching PARTLABEL= on
# util-linux blkid output; quitting after the first match prevents a
# multi-match from producing a newline-separated device list.
DATA_VOLUME=$(blkid | sed -n '/[[:space:]]LABEL="data-volume"/{s/^\([^:]*\):.*/\1/p;q;}')

if [ -n "${DATA_VOLUME}" ]; then
	DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
	# growpart command may be missing in older VMs
	if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then

Codex reported that because sed exits on the first match, blkid can receive SIGPIPE (exit 141) when writing further records into the closed pipe, and pipefail would then abort the persistence script before /mnt/data is mounted.

I reproduced the pattern with seq 1 10000 | sed -n '/^5$/{p;q;}' (exit 141) — SIGPIPE fires when the producer still has bytes to write after sed closes its end. blkid output on Lima is a handful of lines (under 1 KB), far below the 64 KB Linux pipe buffer. blkid writes everything in a single non-blocking write, returns, and exits before sed reads the buffer; the consumer-close window does not open while the producer is still writing. A realistic reproduction (multi-device mock blkid output under set -eux -o pipefail) exits cleanly. The finding is worth recording because the pattern is fragile — any later edit that makes the left side of the pipe produce more than 64 KB would reintroduce the abort path — but no live regression exists today.

Fix (optional, if a defensive reshape is wanted): drop the early q and let sed drain the pipe, using head -n 1 or a second sed -n '1p' to select the first match.

-DATA_VOLUME=$(blkid | sed -n '/[[:space:]]LABEL="data-volume"/{s/^\([^:]*\):.*/\1/p;q;}')
+DATA_VOLUME=$(
+    blkid |
+        sed -n 's/^\([^:]*\):.*[[:space:]]LABEL="data-volume".*/\1/p' |
+        sed -n '1p'
+)

		# partitioned or raw-formatted disk (e.g. the data volume after
		# a failed boot) must not be reformatted.
		if lsblk --list --noheadings --output fstype /dev/"${DISK}" | grep --quiet "[^[:space:]]"; then
			continue
		fi
		if lsblk --list --noheadings --output type /dev/"${DISK}" | grep --quiet "part"; then
			continue
		fi
		if awk '/^\/dev\// {sub("^/dev/", "", $1); print $1}' /proc/mounts | grep --quiet "^${DISK}$"; then
			continue
		fi

The grep --quiet "part" is a substring match against the TYPE column. The current lsblk type vocabulary (disk, part, lvm, crypt, loop, rom, raid0, raid1, mpath, dmraid, …) contains no other token with part as a substring, so the match is correct today. Anchoring to an exact ^part$ makes the intent explicit and guards against future additions to the type vocabulary.

Fix:

-		if lsblk --list --noheadings --output type /dev/"${DISK}" | grep --quiet "part"; then
+		if lsblk --list --noheadings --output type /dev/"${DISK}" | grep --quiet "^part$"; then
 			continue
 		fi

# util-linux blkid output; quitting after the first match prevents a
# multi-match from producing a newline-separated device list.
DATA_VOLUME=$(blkid | sed -n '/[[:space:]]LABEL="data-volume"/{s/^\([^:]*\):.*/\1/p;q;}')

if [ -n "${DATA_VOLUME}" ]; then
	DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
	# growpart command may be missing in older VMs
	if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then
		# Automatically expand the data volume filesystem
		growpart "$DATA_DISK" 1 || true
		# growpart triggers a partition table re-read; settle udev before

If DATA_VOLUME points at a whole disk (e.g., a user who runs mkfs.ext4 -L data-volume /dev/vdb with no partition table), lsblk --output pkname returns an empty string and DATA_DISK becomes the literal /dev/. growpart /dev/ 1 then errors out, caught by the || true on line 71, so the boot survives — but growpart emits misleading stderr noise. Lima always creates a partitioned data disk today (the else-branch runs sfdisk … /dev/"${DISK}" before mkfs.ext4 -L data-volume /dev/"${PART}"), so the path is latent rather than reachable.

if [ -n "${DATA_VOLUME}" ]; then
	DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
	# growpart command may be missing in older VMs
	if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then
		# Automatically expand the data volume filesystem
		growpart "$DATA_DISK" 1 || true
		# growpart triggers a partition table re-read; settle udev before
		# touching the device to avoid racing with the re-probe. The
		# settle is defense-in-depth (blkid above eliminates the core
		# udev dependency), so tolerate its failure rather than crashing
		# the boot under set -e if udevadm is missing or stuck.

Fix (optional): skip the growpart block when DATA_DISK resolves to the /dev/ literal.

-	DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
-	# growpart command may be missing in older VMs
-	if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then
+	DATA_DISK="/dev/$(lsblk --noheadings --output pkname "${DATA_VOLUME}")"
+	# growpart command may be missing in older VMs; skip when DATA_VOLUME
+	# is a whole disk (pkname empty) since there is no partition to grow.
+	if [ "${DATA_DISK}" != "/dev/" ] && command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then

No BATS or integration test in hack/bats/tests/ exercises 04-persistent-data-volume.sh on an Alpine ramdisk VM. The three highest-risk behaviors the patch protects remain unguarded: the blkid-based device resolution, the settle-then-e2fsck sequencing, and the partition-presence check in the else branch. A later refactor could silently reintroduce the symlink dependency or the "partitioned but unmounted" reformat bug without any test breaking.

The author has validated on a live VM (per #4830), and reproducing the timing-dependent udev race in CI is impractical. The check the else branch relies on, however, is deterministic and testable: a minimal harness that boots an Alpine ramdisk VM with a partitioned-but-unmounted disk, exits zero only if the disk is still partitioned afterwards, would guard the data-loss path directly.

Carried over from round 1 S7 (Skipped). Re-raised at the same severity per the round-2 gate; the author's explicit decision to defer test coverage stands.