Skip to content

Commit 1a6e785

Browse files
committed
test(report): capture the screens this release actually changed
CI captures 345 OLED PNGs and the screenshot phase reports healthy, but every suite 7.14.2 changed captured zero of them. The rendering evidence for a release whose entire subject is what reaches the screen did not exist. Three separate causes, all silent: 1. SECTIONS declared no screenshot expectations for test_msg_ethereum_erc20_0x_signtx, test_msg_binance_sign_tx or test_verify_typed_data, so screenshot_filter() never selected them. Add a section for the display-binding paths. 2. test_msg_display_disclosure WAS in the filter and still produced nothing. It answers ButtonRequests through its own recording callback to read the layout, which bypasses the client's capture hook -- so the one suite written specifically to police what the screen shows was the one suite whose screens nobody could look at. Call _capture_oled() from the recorder. 3. The CI gate is `total PNG count > 0`. A single captured suite satisfies it. That cannot distinguish "captured everything" from "captured something", and in this round it passed while the release's own screens were absent. Add --screenshot-audit: fail if any test that DECLARED screens captured none. Skipped tests are excluded -- a version-gated test cannot draw. Run against the rc30 artifact, the audit reports exactly the eight tests that declared screens and captured none. Two entries deliberately carry an EMPTY screenshot list: - S9 test_eip1559_requires_chain_id: the refusal happens before the first confirm(), so no screen is drawn and none should be expected. The absence IS the assertion. - S8 test_contract_handler_streamed_calldata_signs_full_data: every test in test_msg_ethereum_signing_guards currently SKIPs under requires_firmware. Declaring a screen nothing can satisfy would turn the new audit into noise.
1 parent 6c4ad17 commit 1a6e785

2 files changed

Lines changed: 132 additions & 0 deletions

File tree

‎scripts/generate-test-report.py‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,74 @@ def parse_junit(path):
275275
# context = why this test exists, what it proves, what user sees
276276

277277
SECTIONS = [
278+
('S', 'Display Binding - What the Device Signs Is What It Shows', '7.14.2',
279+
'The 7.14.2 security release changed what reaches the OLED on the signing paths. Every '
280+
'defect it fixed was a case of the device hashing bytes it never rendered, or rendering '
281+
'text it could not vouch for. These tests exist to capture those screens: a passing wire '
282+
'assertion proves the device refused or signed, but only the screen proves the user was '
283+
'told the truth about what they approved.',
284+
[
285+
'DISCLOSURE RULE: every byte covered by the signature must be reachable on screen.',
286+
'',
287+
'The defects this section guards against, all shipped at some point:',
288+
'- bytes past an embedded NUL were signed and never drawn ("%s" stops at 0x00)',
289+
'- whitespace padding pushed a tail past the cut with no warning',
290+
'- 456 bytes past the initial chunk were hashed with a clear-sign screen showing',
291+
' confident token amounts for calldata the device had not seen',
292+
'- an unresolved token rendered as the literal "Unknown token value" and signed',
293+
'- a truncated memo dropped its last character (Confirm limit 42 vs 420)',
294+
'',
295+
'A test here with an EMPTY screenshot list is deliberate: refusal paths draw nothing,',
296+
'and their evidence is the Failure on the wire plus the absence of a ButtonRequest.',
297+
],
298+
[
299+
('S1', 'test_msg_ethereum_erc20_0x_signtx', 'test__sign_transformERC20',
300+
'0x transformERC20 raw disclosure',
301+
'A 1480-byte transformERC20 payload exceeds one 1024-byte chunk. The device must NOT '
302+
'clear-sign it as a token swap, because the bytes past the initial chunk are hashed '
303+
'without being decoded. With AdvancedMode on it falls to the raw path, where the byte '
304+
'count shown must be the FULL length (1480), not the chunk length (1024) - a short '
305+
'count would under-report what is being signed.',
306+
['Raw contract data screen showing the full byte count']),
307+
('S2', 'test_msg_ethereum_erc20_0x_signtx', 'test_sign_0x_swap_ERC20_to_ETH',
308+
'0x sellToUniswap names both assets',
309+
'Clear-signing is only honest when BOTH token words resolve to known assets. This '
310+
'payload resolves (USDC -> ETH) and must name both sides with real amounts. The '
311+
'failure this guards is a screen naming a DEX while showing no amount.',
312+
['Swap screen naming both assets and amounts']),
313+
('S3', 'test_msg_ethereum_erc20_0x_signtx', 'test_sign_longdata_swap',
314+
'Long 0x calldata stays disclosed',
315+
'Calldata spanning multiple chunks must not silently lose its tail from the display '
316+
'while remaining inside the signature.',
317+
['Contract data screen']),
318+
('S8', 'test_msg_ethereum_signing_guards',
319+
'test_contract_handler_streamed_calldata_signs_full_data',
320+
'Streamed calldata is fully covered',
321+
'Calldata delivered across several chunks must be hashed in full and disclosed in full. '
322+
'This is the positive control for the chunk-completeness gate. NOTE: every test in '
323+
'test_msg_ethereum_signing_guards currently SKIPS in CI under requires_firmware, so no '
324+
'screen can be captured for it yet - the screenshot list stays empty until the gate '
325+
'opens, rather than declaring an expectation nothing can satisfy.',
326+
[]),
327+
('S9', 'test_msg_ethereum_signing_guards', 'test_eip1559_requires_chain_id',
328+
'Omitted chain_id is refused before any screen',
329+
'Without a chain_id the device cannot name the network, and a signature would be '
330+
'pre-EIP-155 - replayable on every EVM chain. The refusal happens before the first '
331+
'confirm(), so NO screen is drawn and no ButtonRequest is emitted. The empty '
332+
'screenshot list below is the assertion.',
333+
[]),
334+
('S10', 'test_verify_typed_data', 'test_structured_eip712_is_refused',
335+
'Structured EIP-712 is closed by default',
336+
'The legacy JSON parser could not guarantee that every displayed value was the '
337+
'canonical value being hashed, and one screen took its title from the attacker-supplied '
338+
'domain name. The feature is withdrawn rather than shipped with a screen it could not '
339+
'vouch for: zero screens, refusal on the wire.',
340+
[]),
341+
('S11', 'test_msg_binance_sign_tx', 'test_transfer',
342+
'Binance denom renders in full',
343+
'A long denom must render completely and must not overflow the formatting buffer.',
344+
['Transfer screen showing the full denom']),
345+
]),
278346
('X', 'Device Specifications', '0.0.0',
279347
'The KeepKey is an open-source hardware wallet built on an ARM Cortex-M3 (STM32F205, 120MHz) '
280348
'with a 256x64 monochrome OLED, single confirmation button, and micro-USB interface. The '
@@ -1185,6 +1253,46 @@ def screenshot_filter(fw_version):
11851253
return ' or '.join(terms)
11861254

11871255

1256+
def screenshot_audit(fw_version, screenshot_root, junit_path=None):
1257+
"""Which SECTIONS tests DECLARED screens but captured none?
1258+
1259+
The CI gate was `total PNG count > 0`, which a single captured suite
1260+
satisfies. That cannot distinguish "captured everything" from "captured
1261+
something": in the 7.14.2 round, 345 PNGs were produced while every suite
1262+
the release actually changed captured zero, and the phase reported healthy.
1263+
1264+
Returns (ok, missing) where missing is a list of (module, method) that
1265+
declared a non-empty screenshot list, were not skipped, and produced no
1266+
PNG directory. Skipped tests are not missing -- a version-gated test
1267+
cannot draw.
1268+
"""
1269+
import os as _os
1270+
skipped = set()
1271+
if junit_path and _os.path.exists(junit_path):
1272+
import xml.etree.ElementTree as _ET
1273+
root = _ET.parse(junit_path).getroot()
1274+
suites = [root] if root.tag == 'testsuite' else root.findall('testsuite')
1275+
for su in suites:
1276+
for tc in su.findall('testcase'):
1277+
if tc.find('skipped') is not None:
1278+
cn = tc.get('classname', '')
1279+
mod = next((p for p in cn.split('.') if p.startswith('test_')), '')
1280+
skipped.add((mod, tc.get('name')))
1281+
1282+
active = [x for x in SECTIONS if ver_ge(fw_version, x[2])]
1283+
missing = []
1284+
for letter, title, mf, bg, fl, tests in active:
1285+
for tid, mod, meth, ttl, ctx, scr in tests:
1286+
if not scr:
1287+
continue
1288+
if (mod, meth) in skipped:
1289+
continue
1290+
d = _os.path.join(screenshot_root, mod.replace('test_', '', 1), meth)
1291+
if not _os.path.isdir(d) or not [f for f in _os.listdir(d) if f.endswith('.png')]:
1292+
missing.append((mod, meth))
1293+
return (len(missing) == 0, missing)
1294+
1295+
11881296
def validate_junit(fw_version, results):
11891297
"""Check SECTIONS tests against JUnit results. Returns (passed, failed_list).
11901298
@@ -1211,6 +1319,10 @@ def main():
12111319
p.add_argument('--fw-version', default=None)
12121320
p.add_argument('--junit', default=None, help='JUnit XML for pass/fail results')
12131321
p.add_argument('--screenshots', default=None, help='Directory with per-test OLED screenshots')
1322+
p.add_argument('--screenshot-audit', metavar='SCREENSHOT_DIR',
1323+
help='exit 1 if any SECTIONS test that declared screens captured none')
1324+
p.add_argument('--audit-junit', metavar='XML', default=None,
1325+
help='JUnit XML for --screenshot-audit, so skipped tests are not counted missing')
12141326
p.add_argument('--screenshot-filter', action='store_true',
12151327
help='Print pytest -k expression for tests needing screenshots, then exit')
12161328
p.add_argument('--validate-junit', action='store_true',
@@ -1224,6 +1336,15 @@ def main():
12241336
if fw: print(f'Detected: {fw}', file=sys.stderr)
12251337
else: print('No emulator, defaulting to 7.10.0', file=sys.stderr); fw = '7.10.0'
12261338

1339+
if args.screenshot_audit:
1340+
ok, missing = screenshot_audit(fw, args.screenshot_audit, args.audit_junit)
1341+
if ok:
1342+
print('screenshot audit: every declared screen was captured')
1343+
sys.exit(0)
1344+
print('screenshot audit FAILED -- declared screens with no capture:')
1345+
for mod, meth in missing:
1346+
print(' %s::%s' % (mod, meth))
1347+
sys.exit(1)
12271348
if args.screenshot_filter:
12281349
print(screenshot_filter(fw))
12291350
sys.exit(0)

‎tests/test_msg_display_disclosure.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@ def recording_callback(msg):
9090
# A capture failure must not mask the behaviour under test;
9191
# the assertions below check what was captured.
9292
pass
93+
try:
94+
# Also emit the frame as a PNG through the normal capture path.
95+
# This class answers ButtonRequests itself, which bypasses the
96+
# client's own capture hook -- so under KEEPKEY_SCREENSHOT=1
97+
# these tests were selected by the screenshot filter, passed,
98+
# and produced NO images. The screens this suite exists to
99+
# police were the ones nobody could look at.
100+
if getattr(client, 'screenshot_dir', None):
101+
client._capture_oled()
102+
except Exception:
103+
pass
93104
if recorder.answer:
94105
client.debug.press_yes()
95106
else:

0 commit comments

Comments
 (0)