fix: look up any MCUboot image hash TLV, not just SHA256

[JPHutchins]

Warning

LLM Disclosure

This post was authored by claude-opus-5[1m] on behalf of @JPHutchins. JP asked me to open a branch and PR fixing #109, following the repo's CLAUDE.md, and to run the full test suite before committing. While writing the regression tests I found that one premise of the issue is wrong — IMAGE_TLV.SHA512 already exists in the pinned smpclient — so the dependency bump is kept for a different, verified reason, documented below.

Summary

smpmgr upgrade looked up IMAGE_TLV.SHA256 unconditionally and exited 1 when it was absent, rejecting every image NCS sysbuild produces for nRF54L / nRF54H / nRF71 — those are signed with SHA512 by default.

get_image_hash_tlv() now tries SHA256, SHA384, SHA512 and returns the first match, because MCUboot writes exactly one of them. The error message is derived from the same tuple that is searched, so it can't drift from what was actually looked for:

-Could not find IMAGE_TLV_SHA256 in image. If this is not an MCUboot image, retry with --format=any.
+Could not find an image hash TLV (SHA256/SHA384/SHA512) in image. If this is not an MCUboot image, retry with --format=any.

Issue items 1–3	all addressed
Tests	1 → 11 passing; coverage 33% → 40%
lint	clean (black, isort, flake8, mypy)
Hardware validated	no — see Residual risk

⚠️ Correction to the issue: the bump is not what fixes this

Issue #109 says the pin ==7.0.1 "predates that fix" and asks for >=7.3.0 "so IMAGE_TLV.SHA512 exists". It already exists in 7.0.1. The reported failure was entirely smpmgr's hardcoded lookup, and is fixed here without any dependency change.

Verified against the released 7.0.1 wheel from PyPI

$ pip download smpclient==7.0.1 --no-deps -d ./7.0.1
$ unzip -q ./7.0.1/smpclient-7.0.1-py3-none-any.whl -d ./x
$ PYTHONPATH=./x python -c "
import smpclient; print('loaded from:', smpclient.__file__)
from smpclient.mcuboot import IMAGE_TLV
print('SHA256=', hex(IMAGE_TLV.SHA256), 'SHA384=', hex(IMAGE_TLV.SHA384), 'SHA512=', hex(IMAGE_TLV.SHA512))"
loaded from: ./x/smpclient/__init__.py
SHA256= 0x10 SHA384= 0x11 SHA512= 0x12

SHA512 = 0x12 is present in the IMAGE_TLV enum of every 7.x release — 7.0.1, 7.1.0, 7.2.0 and 7.3.0 all contain the literal SHA512 = 0x12 in smpclient/mcuboot.py. smpclient#83 landed before 7.0.0, not after 7.0.1.

Empirically: with the pin still at ==7.0.1, 10 of the 11 tests in this PR pass, including all three of test_get_image_hash_tlv[SHA256/SHA384/SHA512].

Why the bump to ==7.3.0 is kept anyway

A real reason turned up while writing the tests. smpclient 7.0.1 hard-fails on the protected TLV region that can precede the image trailer:

# smpclient 7.0.1 mcuboot.py:241 — ImageTLVInfo.__post_init__
if self.magic != IMAGE_TLV_INFO_MAGIC:
    raise MCUBootImageError(f"TLV info magic is {hex(self.magic)}, expected {hex(IMAGE_TLV_INFO_MAGIC)}")

So ImageInfo.load_file raises for any image carrying protected TLVs (SEC_CNT, BOOT_RECORD) — the same NCS families this issue is about, as soon as a security counter or measured boot is enabled. That would fail earlier than the hash lookup, at Inspection of FW image failed. intercreate/smpclient#114 fixed it in 7.3.0, which added ImageInfo.protected_tlv_info / protected_tlvs.

The one test that fails on 7.0.1 and passes on 7.3.0

test_get_image_hash_tlv_of_image_with_a_protected_tlv_region is the executable record of the bump — this is the pre-bump run:

$ pytest tests/ -q          # smpclient 7.0.1
E  smpclient.mcuboot.MCUBootImageError: TLV info magic is 0x6908, expected 0x6907
FAILED tests/test_image_hash_tlv.py::test_get_image_hash_tlv_of_image_with_a_protected_tlv_region
1 failed, 10 passed in 1.10s

and post-bump:

$ pytest --cov               # smpclient 7.3.0
tests/test_TODO.py .                                                     [  9%]
tests/test_image_hash_tlv.py ..........                                  [100%]
TOTAL   787  476  40%
11 passed in 3.20s

The pin uses == rather than the issue's >=, per the convention d459285 set ("smpmgr is an app, not a library").

Dependency impact of the bump — 36 → 69 runtime packages, +56 MB

7.3.0's all extra is much wider than 7.0.1's. Installed site-packages goes 118 MB → 174 MB, which lands in the PyInstaller portable artifact:

+ bumble 0.0.228          + grpcio 1.83.0        + protobuf 7.36.0
+ cryptography 50.0.1     + aiohttp 3.14.3       + libusb1 3.4.0 / libusb-package / pyusb
+ platformdirs 4.9.4      + prompt-toolkit       + websockets 17.1  (and 12 more transitives)
+ zephyr-4-4-0-hci 0.1.5  + 6 × zephyr-4-4-0-hci-{uart,usb}-nrf5{2,3}* firmware bundles
~ smp 4.0.2 -> 4.1.0

smpmgr on main imports only the ble, serial and udp transports, so extras = ["ble", "serial", "udp"] would keep the tree at its current size. all is kept deliberately: feature/bumble-transport adds smpmgr/bumble.py on smpclient.transport.bumble and 80977b8 there bundles the Zephyr HCI firmware into the portable artifact on purpose. Narrowing the extras here would fight that branch. Flagging the numbers so the trade is explicit rather than accidental.

Changes

• smpmgr/image_management.py — IMAGE_HASH_TLVS, IMAGE_HASH_TLV_NAMES, get_image_hash_tlv(). Returns Optional rather than raising, so the caller's error path stays a plain branch.
• smpmgr/main.py — upgrade uses it; local and log line renamed image_tlv_sha256 → image_hash_tlv; IMAGE_TLV / TLVNotFound imports dropped.
• smpmgr/image_management.py — adjacent: the image state-write HASH argument help no longer claims the hash is SHA256. Same bug class, user-facing; the workaround in upgrade: hardcoded IMAGE_TLV_SHA256 lookup rejects every NCS nRF54L/54H/71 image (SHA512 is the default there) #109 passes a SHA512 to exactly this argument.
• pyproject.toml / poetry.lock — smpclient ==7.0.1 → ==7.3.0.
• tests/test_image_hash_tlv.py — new.

Test coverage — 11 tests, and what each one pins down

Images are assembled from smpclient's own structs (IMAGE_HEADER_STRUCT, IMAGE_TLV_INFO_STRUCT, IMAGE_TLV_STRUCT) so the fixtures cannot drift from the parser, and the trailer shape mirrors what NCS sysbuild emits (KEYHASH + hash + ED25519).

Test	Pins down
test_get_image_hash_tlv[SHA256/SHA384/SHA512]	the reported bug — every algorithm is found, not just SHA256
test_get_image_hash_tlv_of_image_without_a_hash	negative case — no hash TLV yields None, not an exception
test_get_image_hash_tlv_takes_the_first_of_IMAGE_HASH_TLVS	IMAGE_HASH_TLVS order wins, not trailer order
test_get_image_hash_tlv_of_image_with_a_protected_tlv_region	the smpclient bump (fails on 7.0.1)
test_upgrade_inspection_accepts_any_hash[SHA256/SHA384/SHA512]	end-to-end via CliRunner: upgrade clears inspection and reaches the transport
test_upgrade_inspection_rejects_an_image_without_a_hash	the reworded error names all three TLVs it searched

The two upgrade cases run the real CLI with no transport option, so they assert on the boundary between inspection and connection without needing hardware.

Deliberately not changed

The if slot != 0 or confirm: guard at smpmgr/main.py:241 that skips ImageStatesWrite for a default upgrade (--slot 0, no --confirm) — the behaviour #109 reports under "The suggested workaround does not do what it says". #109 filed that as an observation rather than a fix and deferred the --slot semantics to #3, so it is untouched here. Worth noting the consequence: after this PR a default upgrade of a SHA512 image uploads and resets without erroring, but still does not mark the image.

Residual risk

Not validated on hardware. 7.0.1 → 7.3.0 spans the serial-transport rework (max_smp_encoded_frame_size deprecated in favour of the BufferSize strategy, MCUmgr params negotiation). Upstream kept the 7.1.0 constructor and that kwarg working, and smpmgr/common.py passes all three of line_length / line_buffers / max_smp_encoded_frame_size, so it should be source-compatible — but the test suite has no transport coverage and cannot detect a behavioural change in serial fragmentation. A serial DFU smoke test before release is worth it. The BLE path this issue was reported on is unaffected by that rework.

Fixes #109

[JPHutchins]

Mostly good, some ammend commit needs to be made to clean up sloppiness.

[JPHutchins]

logger.info

[JPHutchins]

Upstream is the SSOT, not us

[JPHutchins]

LLM doc slop

[JPHutchins]

walrus