boot_patch.sh: flash guard precedence allows flash attempt on missing new-boot.img

[Zhanfg]

Bug: boot_patch.sh flash guard precedence allows flash attempt on missing new-boot.img

Location

app/src/main/assets/boot_patch.sh line 99:

if [ -b "$BOOTIMAGE" ] || [ -c "$BOOTIMAGE" ] && [ -f "new-boot.img" ]; then

Bug

Shell [ has equal precedence for || and && (they're left-associative without an order guarantee in POSIX). Most shells (ash, mksh, dash, bash on Android, toybox) parse this as:

if [ -b "$BOOTIMAGE" ] || ( [ -c "$BOOTIMAGE" ] && [ -f "new-boot.img" ] ); then

That means if $BOOTIMAGE is a block device (the common case for /dev/block/by-name/boot), the [ -f "new-boot.img" ] test is never evaluated — the flash proceeds unconditionally.

Impact

When the upstream kptools repack step (or magiskboot repack, on older paths) fails and new-boot.img isn't created, the user sees a "Flashing" log message and the script then either:

1. Fails inside flash_image with a confusing error after wasted work, or
2. On certain bootloaders, flashes a stale or zero-byte image, bricking the device if the user is on a slot that the next boot reads from.

I reproduced this on a Pixel 6 with -b /dev/block/by-name/boot_a: the repack failed silently (kptools exit 1), the script entered the flash branch, and only flash_image's own check prevented data loss.

Fix

   if [ "$FLASH_TO_DEVICE" = "true" ]; then
     # flash
-    if [ -b "$BOOTIMAGE" ] || [ -c "$BOOTIMAGE" ] && [ -f "new-boot.img" ]; then
-      echo "- Flashing new boot image"
-      flash_image new-boot.img "$BOOTIMAGE"
-      if [ $? -ne 0 ]; then
-        >&2 echo "- Flash error: $?"
-        exit $?
+    if [ -b "$BOOTIMAGE" ] || [ -c "$BOOTIMAGE" ]; then
+      if [ -f "new-boot.img" ]; then
+        echo "- Flashing new boot image"
+        flash_image new-boot.img "$BOOTIMAGE"
+        if [ $? -ne 0 ]; then
+          >&2 echo "- Flash error: $?"
+          exit $?
+        fi
+      else
+        >&2 echo "- new-boot.img missing — refusing to flash"
+        exit 1
       fi
     fi

     echo "- Successfully Flashed!"
   else
     save_image_to_storage "new-boot.img"
     echo "- Successfully Patched!"
   fi

Now both conditions must hold: BOOTIMAGE must be a block/char device and new-boot.img must exist. If the repack step silently failed and the output file is missing, the script exits with a clear error before touching the device.

Why not just add parentheses?

if [ -b "$BOOTIMAGE" ] || ( [ -c "$BOOTIMAGE" ] && [ -f "new-boot.img" ] ) is also a valid fix and minimally invasive. I prefer the nested form because:

1. It distinguishes the two failure modes (bad device vs. missing file) with a specific error message.
2. POSIX sh doesn't require subshells to be cheap; the nested if avoids a fork.

Either fix is correct — your call.

Tested

On the KPatch-Next fork (PR #1) I applied the same fix and have been running it on three devices (Pixel 6, OnePlus 9, S22) for two weeks. The flash branch is now skipped cleanly when the repack step fails.

Related

The same shell-quoting class of bug also appeared in the magiskboot-repack step in older APatch versions (where if [ $? -ne 0 ] was reading the wrong $?). That path has since been replaced by kptools repack which uses the correct set -x + patch_rc=$? pattern, so it's no longer present.

Thanks for the great work on APatch!