fix(python): raise ProtocolError on invalid THP continuation

[romanz]

Fixes #6559.

[coderabbitai]

Walkthrough

The changes improve error handling in THP packet parsing within the thp_io module. Invalid initial headers now raise a specific ProtocolError("Invalid message header") instead of a generic error message. Additionally, continuation header parsing is hardened by wrapping the struct.unpack operation in a try/except block to catch struct.error and raise a specific ProtocolError("Invalid continuation header") exception.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding proper error handling to raise ProtocolError on invalid THP continuation headers.
Description check	✅ Passed	The description is minimal but sufficient, correctly referencing the linked issue #6559 that defines the requirements.
Linked Issues check	✅ Passed	The PR successfully addresses issue #6559 by implementing consistent error handling for THP continuation header parsing, raising ProtocolError as required.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the THP packet parsing error handling requirement in #6559; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch romanz/thp-packet-err

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

en main(all)

model	device_test	click_test	persistence_test
T2T1	test(all) main(all)	test(all) main(all)	test(all) main(all)
T3B1	test(all) main(all)	test(all) main(all)	test(all) main(all)
T3T1	test(all) main(all)	test(all) main(all)	test(all) main(all)
T3W1	test(all) main(all)	test(all) main(all)	test(all) main(all)

Latest CI run: 22725403312

[M1nd3r]

utACK

[coderabbitai]

🧹 Nitpick comments (1)

python/src/trezorlib/thp/thp_io.py (1)

97-98: Chain the original struct.error exception when re-raising ProtocolError to preserve the parsing failure source.

Both handlers currently lose the original exception context. Add as err and chain with from err at lines 97-98 and 123-124.

Proposed diff

-            except struct.error:
-                raise exceptions.ProtocolError("Invalid message header")
+            except struct.error as err:
+                raise exceptions.ProtocolError("Invalid message header") from err
...
-                except struct.error:
-                    raise exceptions.ProtocolError("Invalid continuation header")
+                except struct.error as err:
+                    raise exceptions.ProtocolError("Invalid continuation header") from err

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/src/trezorlib/thp/thp_io.py` around lines 97 - 98, The except blocks
that catch struct.error and re-raise exceptions.ProtocolError must preserve the
original exception context; update both occurrences (the header handler that
currently does "except struct.error: raise exceptions.ProtocolError('Invalid
message header')" and the later body handler that raises ProtocolError for the
message body) to capture the original exception (e.g., "except struct.error as
err") and chain it when re-raising the ProtocolError using "from err" so the
original struct.error is preserved in the exception chain.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@python/src/trezorlib/thp/thp_io.py`:
- Around line 97-98: The except blocks that catch struct.error and re-raise
exceptions.ProtocolError must preserve the original exception context; update
both occurrences (the header handler that currently does "except struct.error:
raise exceptions.ProtocolError('Invalid message header')" and the later body
handler that raises ProtocolError for the message body) to capture the original
exception (e.g., "except struct.error as err") and chain it when re-raising the
ProtocolError using "from err" so the original struct.error is preserved in the
exception chain.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2ac7717d-9af9-4bae-ac53-9ccf714dd26a

📥 Commits

Reviewing files that changed from the base of the PR and between c65ec9c and 5bd0150.

📒 Files selected for processing (1)

• python/src/trezorlib/thp/thp_io.py