firmware flash: refuse an app image at the bootloader offset

cli/src/mpftp/cli.py

@@ -2042,6 +2042,8 @@ def cmd_firmware(ns: argparse.Namespace) -> None:
             extra += ["--artifact", ns.artifact]
         if getattr(ns, "family", None):
             extra += ["--family", ns.family]
+        if getattr(ns, "offset", None):
+            extra += ["--offset", ns.offset]
         if getattr(ns, "erase", False):
             extra.append("--erase")
         if getattr(ns, "uf2", False):
@@ -2615,6 +2617,8 @@ def build_parser() -> argparse.ArgumentParser:
     fwf = fwsub.add_parser("flash", parents=[fw_sel, device_opts], help="Flash a built or downloaded artifact")
     fwf.add_argument("--artifact", help="Explicit firmware file (else last build)")
     fwf.add_argument("--family", default="", help="MCU family for flash offset (download mode)")
+    fwf.add_argument("--offset", default="",
+                     help="esp32 flash offset (default: the bootloader offset for the chip)")
     fwf.add_argument("--erase", action="store_true", help="esp32: erase flash first")
     fwf.add_argument("--uf2", action="store_true",
                      help="Copy a .uf2 to a bootloader volume instead of flashing over serial")

cli/src/mpftp/firmware.py

@@ -1963,6 +1963,56 @@ def _esp32_layout_check(
     }
 
 
+# An ESP-IDF image starts with 0xE9. An application image carries its
+# esp_app_desc_t at the start of its first segment: after the 24-byte image
+# header and the 8-byte segment header, so at byte 32. A bootloader, and the
+# combined firmware.bin that starts with one, has something else there.
+_ESP_IMAGE_MAGIC = 0xE9
+_ESP_APP_DESC_MAGIC = 0xABCD5432
+_ESP_APP_DESC_AT = 32
+
+
+def esp_image_is_app(artifact: Path) -> bool:
+    """True when ``artifact`` is an ESP-IDF application image (micropython.bin)."""
+    try:
+        with artifact.open("rb") as fh:
+            head = fh.read(_ESP_APP_DESC_AT + 4)
+    except OSError:
+        return False
+    if len(head) < _ESP_APP_DESC_AT + 4 or head[0] != _ESP_IMAGE_MAGIC:
+        return False
+    return struct.unpack_from("<I", head, _ESP_APP_DESC_AT)[0] == _ESP_APP_DESC_MAGIC
+
+
+def app_image_at_bootloader_error(artifact: Path, offset: Any) -> Optional[str]:
+    """Why writing ``artifact`` at ``offset`` would brick the boot, or None.
+
+    Everything below the partition table (0x8000) is the second-stage
+    bootloader's. An application image written there is loaded by the ROM as a
+    bootloader, so the board boot-loops on a watchdog reset, and the write
+    runs over the partition table too. The image meant for that offset is the
+    combined ``firmware.bin``, which starts with the bootloader.
+    """
+    try:
+        at = int(str(offset), 0)
+    except (TypeError, ValueError):
+        return None
+    if at >= _PARTITION_TABLE_OFFSET or not esp_image_is_app(artifact):
+        return None
+    combined = artifact.parent / "firmware.bin"
+    hint = (
+        f"Flash {combined} instead"
+        if combined.is_file() and combined != artifact
+        else "Flash the build's combined firmware.bin instead"
+    )
+    return (
+        f"{artifact.name} is an application image, and {hex(at)} is the "
+        "bootloader's offset: the board would boot-loop and lose its partition "
+        f"table. {hint}, or pass the application partition's offset "
+        "(usually 0x10000) with --offset."
+    )
+
+
 def flash_esp32(ns: argparse.Namespace, mp: Optional[Path], artifact: Path) -> None:
     port_dir = (mp / "ports" / ns.port) if mp else Path(".")
     family = getattr(ns, "family", "") or ""
@@ -1977,6 +2027,10 @@ def flash_esp32(ns: argparse.Namespace, mp: Optional[Path], artifact: Path) -> N
         ),
     )
     emit_log(f"[mpftp] flash offset {offset}")
+    wrong_image = app_image_at_bootloader_error(artifact, offset)
+    if wrong_image:
+        emit_result(False, error=wrong_image)
+        return
     fw = str(artifact)
     if HOST == "wsl":
         fw = _wslpath_w(fw)

cli/tests/test_app_image_offset.py

@@ -0,0 +1,97 @@
+"""An application image written at the bootloader offset boot-loops the board.
+
+mpftp#67: ``micropython.bin`` flashed with ``--artifact`` on the P4 went to
+0x2000, the ROM loaded it as a bootloader, and the board sat in a watchdog
+reset loop with its partition table overwritten. The combined
+``firmware.bin`` is what belongs there. Each test plants the image that
+caused it, not only a good one.
+"""
+
+from __future__ import annotations
+
+import argparse
+import struct
+import tempfile
+import unittest
+from pathlib import Path
+from unittest import mock
+
+from mpftp import firmware
+
+
+def image(desc_word: int) -> bytes:
+    """An ESP image header, one segment header, then a 4-byte word at byte 32."""
+    header = bytes([0xE9, 1, 2, 0x20]) + b"\x00" * 20
+    segment = struct.pack("<II", 0x40280020, 0x100)
+    return header + segment + struct.pack("<I", desc_word) + b"\x00" * 60
+
+
+APP = image(0xABCD5432)  # esp_app_desc_t: what micropython.bin starts with
+BOOTLOADER = image(0x50)  # esp_bootloader_desc_t: what firmware.bin starts with
+
+
+class ImageKindTests(unittest.TestCase):
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory()
+        self.dir = Path(self.tmp.name)
+
+    def tearDown(self):
+        self.tmp.cleanup()
+
+    def write(self, name: str, data: bytes) -> Path:
+        path = self.dir / name
+        path.write_bytes(data)
+        return path
+
+    def test_an_app_image_is_recognised(self):
+        self.assertTrue(firmware.esp_image_is_app(self.write("micropython.bin", APP)))
+
+    def test_a_bootloader_led_image_is_not_an_app(self):
+        self.assertFalse(firmware.esp_image_is_app(self.write("firmware.bin", BOOTLOADER)))
+
+    def test_a_file_that_is_not_an_esp_image_is_not_an_app(self):
+        self.assertFalse(firmware.esp_image_is_app(self.write("x.bin", b"\x00" * 64)))
+        self.assertFalse(firmware.esp_image_is_app(self.write("short.bin", APP[:20])))
+
+    def test_app_at_the_bootloader_offset_is_refused_and_names_firmware_bin(self):
+        app = self.write("micropython.bin", APP)
+        self.write("firmware.bin", BOOTLOADER)
+        for offset in ("0x2000", "0x1000", "0x0"):
+            with self.subTest(offset=offset):
+                why = firmware.app_image_at_bootloader_error(app, offset)
+                self.assertIsNotNone(why)
+                self.assertIn(str(self.dir / "firmware.bin"), why)
+
+    def test_app_at_the_app_partition_is_allowed(self):
+        app = self.write("micropython.bin", APP)
+        self.assertIsNone(firmware.app_image_at_bootloader_error(app, "0x10000"))
+
+    def test_combined_image_at_the_bootloader_offset_is_allowed(self):
+        fw = self.write("firmware.bin", BOOTLOADER)
+        self.assertIsNone(firmware.app_image_at_bootloader_error(fw, "0x2000"))
+
+
+class FlashEsp32Tests(unittest.TestCase):
+    def test_flash_refuses_before_touching_the_device(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            app = Path(tmp) / "micropython.bin"
+            app.write_bytes(APP)
+            ns = argparse.Namespace(
+                port="esp32", board="", family="esp32p4", offset="", device="COM4",
+                baud=460800, erase=False, before="", after="", board_dir="",
+            )
+            with mock.patch.object(firmware, "emit_result") as result, \
+                    mock.patch.object(firmware, "emit_log"), \
+                    mock.patch.object(firmware, "_esptool_cmd", return_value=["esptool"]), \
+                    mock.patch.object(firmware, "_esp32_layout_check") as layout, \
+                    mock.patch.object(firmware, "stream_process") as run:
+                firmware.flash_esp32(ns, None, app)
+            run.assert_not_called()
+            layout.assert_not_called()
+            args, kwargs = result.call_args
+            self.assertFalse(args[0])
+            self.assertIn("application image", kwargs["error"])
+
+
+if __name__ == "__main__":
+    unittest.main()