Add experimental type checks with ty

This patch prepares the repository for type checks with ty and adds an
experimental CI job that runs the checks.

ty detected these problems in the current code:
- TrussedBootloaderNrf52.update renamed an argument while overriding
  TrussedBootloader.update
- Some ConfigFieldType functions did not handle unexpected variants.
  Ideally, we would use typing.assert_never here, but this has only
  been added in Python 3.11.
- logger.warn is deprecated in favor of logger.warning.

We still have to ignore these false-positives:
- @functools.total_ordering is not handled correctly, see
  astral-sh/ty#1202
- Some imports from fake-winreg are not handled correctly, but this
  could also be a problem with fake-winreg.

Author: robin-nitrokey

.github/workflows/ci.yaml

@@ -8,6 +8,7 @@ on:
 env:
   REQUIRED_PACKAGES: make
   POETRY_SPEC: poetry >=2, <3
+  TY_SPEC: ty ==0.0.1a28
 
 jobs:
   format-code:
@@ -70,6 +71,23 @@ jobs:
         run: make install
       - name: Check code static typing
         run: make check-typing
+  lint-typing-ty:
+    name: Check static typing with ty
+    runs-on: ubuntu-latest
+    container: python:3.10-slim
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+      - name: Install required packages
+        run: apt update && apt install -y ${REQUIRED_PACKAGES}
+      - name: Install Poetry
+        run: pip install "${POETRY_SPEC}"
+      - name: Create virtual environment
+        run: make install
+      - name: Install ty
+        run: poetry add "${TY_SPEC}"
+      - name: Check code static typing
+        run: poetry run ty check
   lint-poetry:
     name: Check poetry configuration
     runs-on: ubuntu-latest

pyproject.toml

@@ -90,3 +90,6 @@ strict = true
 [tool.rstcheck]
 ignore_directives = ["autoclass", "autofunction", "automodule"]
 ignore_substitutions = ["nitrokey_sdk_version"]
+
+[tool.ty.environment]
+extra-paths = ["stubs"]

src/nitrokey/trussed/_bootloader/lpc55.py

@@ -105,7 +105,7 @@ def _list_vid_pid(cls: type[T], vid: int, pid: int) -> list[T]:
             try:
                 devices.append(cls(device))
             except ValueError:
-                logger.warn(
+                logger.warning(
                     f"Invalid Nitrokey 3 LPC55 bootloader returned by enumeration: {device}"
                 )
         return devices
@@ -114,16 +114,16 @@ def _list_vid_pid(cls: type[T], vid: int, pid: int) -> list[T]:
     def _open(cls: type[T], path: str) -> Optional[T]:
         devices = UsbDevice.enumerate(path=path)
         if len(devices) == 0:
-            logger.warn(f"No HID device at {path}")
+            logger.warning(f"No HID device at {path}")
             return None
         if len(devices) > 1:
-            logger.warn(f"Multiple HID devices at {path}: {devices}")
+            logger.warning(f"Multiple HID devices at {path}: {devices}")
             return None
 
         try:
             return cls(devices[0])
         except ValueError:
-            logger.warn(
+            logger.warning(
                 f"No Nitrokey 3 bootloader at path {path}", exc_info=sys.exc_info()
             )
             return None

src/nitrokey/trussed/_bootloader/lpc55_upload/mboot/properties.py

@@ -494,10 +494,10 @@ class AvailableCommandsValue(PropertyValueBase):
     __slots__ = ("value",)
 
     @property
-    def tags(self) -> List[str]:
+    def tags(self) -> List[int]:
         """List of tags representing Available commands."""
         return [
-            cmd_tag.tag  # type: ignore
+            cmd_tag.tag
             for cmd_tag in CommandTag
             if cmd_tag.tag > 0 and (1 << cmd_tag.tag - 1) & self.value
         ]
@@ -516,11 +516,13 @@ def __contains__(self, item: int) -> bool:
 
     def to_str(self) -> str:
         """Get stringified property representation."""
-        return [
-            cmd_tag.label  # type: ignore
-            for cmd_tag in CommandTag
-            if cmd_tag.tag > 0 and (1 << cmd_tag.tag - 1) & self.value
-        ]
+        return ", ".join(
+            [
+                cmd_tag.label
+                for cmd_tag in CommandTag
+                if cmd_tag.tag > 0 and (1 << cmd_tag.tag - 1) & self.value
+            ]
+        )
 
 
 class IrqNotifierPinValue(PropertyValueBase):

src/nitrokey/trussed/_bootloader/nrf52.py

@@ -151,28 +151,28 @@ def reboot(self) -> bool:
     def uuid(self) -> Optional[Uuid]:
         return Uuid(self._uuid)
 
-    def update(self, data: bytes, callback: Optional[ProgressCallback] = None) -> None:
+    def update(self, image: bytes, callback: Optional[ProgressCallback] = None) -> None:
         # based on https://github.com/NordicSemiconductor/pc-nrfutil/blob/1caa347b1cca3896f4695823f48abba15fbef76b/nordicsemi/dfu/dfu.py
         # we have to implement this ourselves because we want to read the files
         # from memory, not from the filesystem
 
-        image = Image.parse(data, self._signature_keys)
+        parsed_image = Image.parse(image, self._signature_keys)
 
         time.sleep(3)
 
         dfu = DfuTransportSerial(self.path)
 
         if callback:
-            total = len(image.firmware_bin)
+            total = len(parsed_image.firmware_bin)
             callback(0, total)
             dfu.register_events_callback(
                 DfuEvent.PROGRESS_EVENT,
                 CallbackWrapper(callback, total),
             )
 
         dfu.open()
-        dfu.send_init_packet(image.firmware_dat)
-        dfu.send_firmware(image.firmware_bin)
+        dfu.send_init_packet(parsed_image.firmware_dat)
+        dfu.send_firmware(parsed_image.firmware_bin)
         dfu.close()
 
     @classmethod
@@ -205,7 +205,7 @@ def _list_ports(vid: int, pid: int) -> list[tuple[str, int]]:
         product_id = int(device.product_id, base=16)
         assert device.com_ports
         if len(device.com_ports) > 1:
-            logger.warn(
+            logger.warning(
                 f"Nitrokey 3 NRF52 bootloader has multiple com ports: {device.com_ports}"
             )
         if vendor_id == vid and product_id == pid:

src/nitrokey/trussed/_bootloader/nrf52_upload/lister/windows/lister_win32.py

@@ -138,7 +138,10 @@ def get_serial_serial_no(
         vendor_id, product_id
     )
     try:
-        vendor_product_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+        vendor_product_hkey = winreg.OpenKeyEx(
+            winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+            hkey_path,
+        )
     except EnvironmentError:
         return None
 
@@ -155,7 +158,10 @@ def get_serial_serial_no(
         )
 
         try:
-            device_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+            device_hkey = winreg.OpenKeyEx(
+                winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+                hkey_path,
+            )
         except EnvironmentError:
             continue
 
@@ -187,7 +193,10 @@ def get_serial_serial_no(
 def com_port_is_open(port: str) -> bool:
     hkey_path = "HARDWARE\\DEVICEMAP\\SERIALCOMM"
     try:
-        device_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+        device_hkey = winreg.OpenKeyEx(
+            winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+            hkey_path,
+        )
     except EnvironmentError:
         #  Unable to check enumerated serialports. Assume open.
         return True
@@ -215,7 +224,10 @@ def list_all_com_ports(
     )
 
     try:
-        device_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+        device_hkey = winreg.OpenKeyEx(
+            winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+            hkey_path,
+        )
     except EnvironmentError:
         return ports
 
@@ -231,7 +243,10 @@ def list_all_com_ports(
         vendor_id, product_id, serial_number
     )
     try:
-        device_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+        device_hkey = winreg.OpenKeyEx(
+            winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+            hkey_path,
+        )
         try:
             COM_port = winreg.QueryValueEx(device_hkey, "PortName")[0]
             ports.append(COM_port)
@@ -257,7 +272,10 @@ def list_all_com_ports(
         )
         iface_id += 1
         try:
-            device_hkey = winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, hkey_path)
+            device_hkey = winreg.OpenKeyEx(
+                winreg.HKEY_LOCAL_MACHINE,  # ty: ignore[possibly-missing-attribute]
+                hkey_path,
+            )
         except EnvironmentError:
             break
 

src/nitrokey/trussed/_device.py

@@ -101,12 +101,14 @@ def open(cls: type[T], path: str) -> Optional[T]:
             else:
                 device = open_device(path)
         except Exception:
-            logger.warn(f"No CTAPHID device at path {path}", exc_info=sys.exc_info())
+            logger.warning(f"No CTAPHID device at path {path}", exc_info=sys.exc_info())
             return None
         try:
             return cls.from_device(device)
         except ValueError:
-            logger.warn(f"No Nitrokey device at path {path}", exc_info=sys.exc_info())
+            logger.warning(
+                f"No Nitrokey device at path {path}", exc_info=sys.exc_info()
+            )
             return None
 
     @classmethod

src/nitrokey/trussed/_utils.py

@@ -232,7 +232,9 @@ class Fido2Certs:
 
     @staticmethod
     def get(certs: Sequence["Fido2Certs"], version: Version) -> Optional["Fido2Certs"]:
-        matching_certs = [c for c in certs if version >= c.start]
+        matching_certs = [
+            c for c in certs if version >= c.start  # ty: ignore[unsupported-operator]
+        ]
         if matching_certs:
             return max(matching_certs, key=lambda c: c.start)
         else:

src/nitrokey/trussed/admin_app.py

@@ -176,12 +176,18 @@ def is_valid(self, value: str) -> bool:
                     return False
             except ValueError:
                 return False
+        else:
+            # TODO: use typing.assert_never from Python 3.11
+            raise ValueError(self)
 
     def __str__(self) -> str:
         if self == ConfigFieldType.BOOL:
             return "Bool"
         elif self == ConfigFieldType.U8:
             return "u8"
+        else:
+            # TODO: use typing.assert_never from Python 3.11
+            raise ValueError(self)
 
 
 @dataclass

src/nitrokey/trussed/updates.py

@@ -130,16 +130,16 @@ def get(
         if variant == Variant.NRF52:
             if (
                 current is None
-                or current <= Version(1, 2, 2)
-                and new >= Version(1, 3, 0)
+                or current <= Version(1, 2, 2)  # ty: ignore[unsupported-operator]
+                and new >= Version(1, 3, 0)  # ty: ignore[unsupported-operator]
             ):
                 migrations.add(cls.NRF_IFS_MIGRATION)
 
         ifs_migration_v2 = Version(1, 8, 2)
         if (
             current is not None
             and current < ifs_migration_v2
-            and new >= ifs_migration_v2
+            and new >= ifs_migration_v2  # ty: ignore[unsupported-operator]
         ):
             migrations.add(cls.IFS_MIGRATION_V2)