Merge pull request #52 from BitHighlander/fix/capability-gates-not-source-greps

fix(tests): gate on firmware capability, not host bindings or source text

Author: BitHighlander

tests/common.py

@@ -139,6 +139,43 @@ def requires_taproot(self):
         if not getattr(self.client.features, 'supports_taproot', False):
             self.skipTest("Firmware does not report supports_taproot")
 
+    def requires_structured_eip712(self):
+        """Skip unless the FIRMWARE drives the structured EIP-712 walk.
+
+        requires_message() cannot answer this. It asks whether
+        python-keepkey's own bindings define a message, which is a property of
+        the pinned submodule and not of the firmware under test -- so it passes
+        on every branch regardless, and a branch without eip712_stream.c fails
+        these tests as though the feature were broken rather than absent.
+
+        Probes the device instead: firmware that does not implement the walk
+        answers the opening message with Failure_UnexpectedMessage. A firmware
+        that DOES implement it answers with a struct request, and we cancel.
+        Anything else is left to fail the test, because "the feature is present
+        but misbehaving" must never be mistaken for "the feature is absent".
+        """
+        from keepkeylib import messages_ethereum_pb2 as _eth
+        from keepkeylib import messages_pb2 as _proto
+
+        probe = _eth.EthereumSignTypedData()
+        for n in (0x8000002C, 0x8000003C, 0x80000000, 0, 0):
+            probe.address_n.append(n)
+        probe.primary_type = "EIP712Domain"
+        probe.metamask_v4_compat = True
+
+        resp = self.client.call_raw(probe)
+        if isinstance(resp, _proto.Failure):
+            self.client.init_device()
+            if resp.code == _proto.Failure_UnexpectedMessage:
+                self.skipTest(
+                    "Firmware does not implement structured EIP-712 "
+                    "(EthereumSignTypedData is not handled)")
+            # Any other Failure is a real problem; let the test run and report it.
+            return
+        # Feature is present -- put the device back before the test starts.
+        self.client.call_raw(_proto.Cancel())
+        self.client.init_device()
+
     def requires_message(self, msg_name):
         """Skip if firmware does not handle this message type.
         Use alongside requires_firmware for per-feature gating:

tests/test_msg_eip712_streaming.py

@@ -87,7 +87,7 @@ def setUp(self):
         super(TestMsgEip712Streaming, self).setUp()
         self.requires_firmware("7.15.0")
         self.requires_fullFeature()
-        self.requires_message("EthereumSignTypedData")
+        self.requires_structured_eip712()
         self.setup_mnemonic_nopin_nopassphrase()
         self.client.apply_policy('AdvancedMode', 1)
 

tests/test_storage_version_gate.py

@@ -455,22 +455,50 @@ def test_active_flash_format_is_v20(self):
             "release (7.15 = V17) and moves in the release commit that tags "
             "7.16, not when a format lands in the tree." % self.last_shipped)
 
-    def test_burned_versions_have_no_reader(self):
-        """18 and 19 must never be parsed by 7.16.
+    def test_burned_versions_are_dispatched_to_the_wipe_path(self):
+        """18 and 19 must never be PARSED by 7.16.
 
         They were real formats in alpha builds before the 7.15 revert, so
         devices carrying them exist. A reader for either would parse a
-        clear-sign identity block or a PIN-KDF blob as passkey state. The
-        absence of a case in the dispatch is what sends them to the wipe path,
-        and this test is what stops one being added back by someone tidying up
-        the switch.
+        clear-sign identity block or a PIN-KDF blob as passkey state.
+
+        This used to assert the absence of a `case StorageVersion_18:` label,
+        on the theory that falling to the default is what sends them to the
+        wipe path. That was wrong twice over: storage_fromFlash has NO default
+        case -- deliberately, so -Werror=switch names any version we forget --
+        so an unlisted version does not fall anywhere, it fails the ARM build.
+
+        So the labels must exist. What must NOT exist is a reader behind them.
+        Assert the real property: 18 and 19 are dispatched, and what they
+        dispatch to is SUS_Invalid rather than any storage_readVxx call.
         """
-        self.assertNotIn("case StorageVersion_18:", self.c,
-                         "18 is a burned format; a reader would misparse blobs "
-                         "written by pre-revert alpha builds")
-        self.assertNotIn("case StorageVersion_19:", self.c,
-                         "19 is a burned format; a reader would misparse blobs "
-                         "written by pre-revert alpha builds")
+        for burned in (18, 19):
+            label = "case StorageVersion_%d:" % burned
+            self.assertIn(
+                label, self.c,
+                "%s must be listed; storage_fromFlash has no default case, so "
+                "an unlisted version breaks the -Werror=switch build" % label)
+
+        # The two labels must sit together and return SUS_Invalid before any
+        # other case begins. Slice from the first burned label to the next
+        # `case ` that is not one of the burned ones.
+        i = self.c.index("case StorageVersion_18:")
+        rest = self.c[i:]
+        j = len(rest)
+        for m in re.finditer(r"\n\s*case StorageVersion_(\w+):", rest):
+            if m.group(1) not in ("18", "19"):
+                j = m.start()
+                break
+        arm = rest[:j]
+
+        self.assertIn(
+            "SUS_Invalid", arm,
+            "the burned versions must return SUS_Invalid (the wipe path); "
+            "arm was:\n%s" % arm)
+        self.assertNotIn(
+            "storage_read", arm,
+            "a reader behind a burned version would misparse blobs written by "
+            "pre-revert alpha builds; arm was:\n%s" % arm)
 
     def test_version_never_drops_below_a_shipped_release(self):
         """Lowering STORAGE_VERSION wipes every device upgrading FROM a shipped