Skip to content

Commit 5f872cc

Browse files
committed
fix(tests): the display-disclosure suite was passing vacuously
setUp() inherits a device wipe and never loads a seed, so every SignMessage in this file was refused with Failure_NotInitialized before confirm_bytes() was ever reached. _sign_message_screens() caught the CallException and returned None, and _assert_distinguishable() returned early on None without executing its assertNotEqual. Four tests therefore reported PASS having exercised zero display logic, and the fifth -- written specifically to catch exactly this -- skipped ITSELF on the same condition. Reintroducing the NUL-truncation bug, so that b"benign login\x00 AND APPROVE TRANSFER OF ALL FUNDS" renders identically to b"benign login" while the signature covers all 46 bytes, would leave every test, the JUnit validation and the ci-gate green. The release PDF certifies this coverage as passing. Three changes: - setUp() loads a seed, so the device actually renders the screens under test. - A refusal no longer silently satisfies the property. It asserts the device is initialized first -- a refusal only means something from a device that could have signed and chose not to -- and otherwise fails by name. An intended refusal should be asserted explicitly, not inferred from None. - The anti-vacuity control no longer skips itself. That test exists to prove the rest of the file is not vacuous, so skipping when the device will not sign is the one failure mode it cannot be allowed to have.
1 parent 44d82ef commit 5f872cc

1 file changed

Lines changed: 31 additions & 4 deletions

File tree

tests/test_msg_display_disclosure.py

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,13 @@ class TestDisplayDisclosesSignedContent(common.KeepKeyTest):
131131
def setUp(self):
132132
super(TestDisplayDisclosesSignedContent, self).setUp()
133133
self.requires_firmware(self.MIN_FIRMWARE)
134+
# The inherited setUp wipes the device. Without a seed every
135+
# SignMessage below is refused with Failure_NotInitialized before
136+
# confirm_bytes() is ever reached, _sign_message_screens() returns
137+
# None, and _assert_distinguishable() returns without asserting -- so
138+
# the whole suite passed while exercising zero display logic. Load a
139+
# seed so the device actually renders the screens under test.
140+
self.setup_mnemonic_nopin_nopassphrase()
134141

135142
# ── helpers ─────────────────────────────────────────────────────────
136143

@@ -165,8 +172,22 @@ def _assert_distinguishable(self, a_label, a_msg, b_label, b_msg):
165172
b = self._sign_message_screens(b_msg)
166173

167174
if a is None or b is None:
168-
# Refusing to display something it cannot show honestly is a pass.
169-
return
175+
# A refusal is only meaningful from an initialized device that
176+
# could have signed and chose not to. On an uninitialized device
177+
# every call is refused for an unrelated reason, which is what let
178+
# this suite pass vacuously -- so assert the device can sign at
179+
# all before treating a refusal as the honest-refusal pass.
180+
self.assertTrue(
181+
self.client.features.initialized,
182+
"device is not initialized, so this refusal says nothing "
183+
"about display disclosure -- the assertion below never ran")
184+
refused = a_label if a is None else b_label
185+
raise AssertionError(
186+
"device refused to sign %s. Refusing to display what it "
187+
"cannot show honestly is defensible, but it must be an "
188+
"explicit, reviewed decision rather than a silent pass: if "
189+
"this is intended, assert the refusal here by name."
190+
% refused)
170191

171192
self.assertNotEqual(
172193
a, b,
@@ -244,8 +265,14 @@ def test_signing_shows_at_least_one_screen(self):
244265
empty tuples and the suite would pass while showing the user nothing.
245266
"""
246267
screens = self._sign_message_screens(b"hello")
247-
if screens is None:
248-
self.skipTest("device refused to sign the control message")
268+
# Do NOT skip here. This test exists to prove the rest of the file is
269+
# not vacuous, so skipping itself when the device will not sign is the
270+
# one failure mode it cannot be allowed to have -- that is exactly how
271+
# the whole suite went green against an uninitialized device.
272+
self.assertIsNotNone(
273+
screens,
274+
"device refused to sign the control message, so every comparison "
275+
"in this file compared None against None and asserted nothing")
249276
self.assertGreater(
250277
len(screens), 0,
251278
"signing produced no ButtonRequest, so nothing was shown to the "

0 commit comments

Comments
 (0)