Skip to content

Commit

Permalink
Extend strict mypy checks
Browse files Browse the repository at this point in the history
Strict mypy checks cannot be enabled on a per-module level, so we used
the disallow_untyped_defs option to replace them.  But actually there
are many more options that would be set in strict mode.  This patch adds
these options to the mypy configuration, fixes some missing or wrong
annotations and adds some ignores for more complex issues so that the
checks still pass.

Fixes: Nitrokey#443
  • Loading branch information
robin-nitrokey committed Sep 20, 2023
1 parent dc48ea4 commit 3780bea
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 44 deletions.
2 changes: 1 addition & 1 deletion pynitrokey/cli/nk3/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def secrets(ctx: click.Context) -> None:
pass


def repeat_if_pin_needed(func) -> Callable: # type: ignore[no-untyped-def]
def repeat_if_pin_needed(func) -> Callable: # type: ignore[no-untyped-def, type-arg]
"""
Repeat the call of the decorated function, if PIN is required.
Decorated function should have at least one argument,
Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/cli/nk3/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def request_pin(self, permissions: Any, rd_id: Any) -> str:
if self.pin:
return self.pin
else:
raise PinRequiredError()
raise PinRequiredError() # type: ignore[no-untyped-call]

def request_uv(self, permissions: Any, rd_id: Any) -> bool:
return True
Expand Down
4 changes: 2 additions & 2 deletions pynitrokey/fido2/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import time
from typing import Callable, Dict, List, Optional, Tuple, Union
from typing import Any, Callable, List, Optional, Union

import usb
from fido2.hid import CtapHidDevice
Expand All @@ -25,7 +25,7 @@ def newdel(self): # type: ignore

# @todo: remove this, HidOverUDP is not available anymore!
def _UDP_InternalPlatformSwitch(
funcname: Callable, *args: Tuple, **kwargs: Dict
funcname: str, *args: tuple[Any, Any], **kwargs: dict[Any, Any]
) -> None:
if funcname == "__init__":
return HidOverUDP(*args, **kwargs) # type: ignore
Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/fido2/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ def isCorrectVersion(current: Tuple[int, int, int], target: str) -> bool:
| (target_num_toks[2] << 0)
)
current_num = (current[0] << 16) | (current[1] << 8) | (current[2] << 0)
return eval(str(current_num) + comp + str(target_num))
return eval(str(current_num) + comp + str(target_num)) # type: ignore [no-any-return]

firmware_file_data = None
if name.lower().endswith(".json"):
Expand Down
6 changes: 3 additions & 3 deletions pynitrokey/fido2/dfu.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def find(
altsetting: int = 0,
ser: Optional[str] = None,
dev: Optional[usb.core.Device] = None,
) -> None:
) -> usb.core.Device:

if dev is not None:
self.dev = dev
Expand Down Expand Up @@ -156,7 +156,7 @@ def upload(self, block: int, size: int) -> List[int]:
address is ((block – 2) × size) + 0x08000000
"""
# bmReqType, bmReq, wValue, wIndex, data/size
return self.dev.ctrl_transfer(
return self.dev.ctrl_transfer( # type: ignore[no-any-return]
DFU.type.RECEIVE, DFU.bmReq.UPLOAD, block, self.intNum, size
)

Expand All @@ -166,7 +166,7 @@ def set_addr(self, addr: int) -> int:

def dnload(self, block: int, data: List[int]) -> int:
# bmReqType, bmReq, wValue, wIndex, data/size
return self.dev.ctrl_transfer(
return self.dev.ctrl_transfer( # type: ignore[no-any-return]
DFU.type.SEND, DFU.bmReq.DNLOAD, block, self.intNum, data
)

Expand Down
6 changes: 3 additions & 3 deletions pynitrokey/fido2/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import binascii
import struct
from typing import Dict, List, Optional
from typing import Any, List, Optional

import ecdsa
from intelhex import IntelHex
Expand Down Expand Up @@ -185,7 +185,7 @@ def flash_addr(num: int) -> int:

def sign_firmware(
sk_name: str, hex_file: str, APPLICATION_END_PAGE: int = 20, PAGES: int = 128
) -> Dict:
) -> dict[str, Any]:
v1 = sign_firmware_for_version(sk_name, hex_file, 19)
v2 = sign_firmware_for_version(sk_name, hex_file, 20, PAGES=PAGES)

Expand All @@ -206,7 +206,7 @@ def sign_firmware(

def sign_firmware_for_version(
sk_name: str, hex_file: str, APPLICATION_END_PAGE: int, PAGES: int = 128
) -> Dict:
) -> dict[str, Any]:
# Maybe this is not the optimal module...

import base64
Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/nk3/bootloader/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def open(path: str) -> Optional[Nitrokey3Bootloader]:
return None


def get_firmware_filename_pattern(variant: Variant) -> Pattern:
def get_firmware_filename_pattern(variant: Variant) -> Pattern[str]:
from .lpc55 import FILENAME_PATTERN as FILENAME_PATTERN_LPC55
from .nrf52 import FILENAME_PATTERN as FILENAME_PATTERN_NRF52

Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/nk3/bootloader/lpc55.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def reboot(self) -> bool:
return True

def uuid(self) -> Optional[Uuid]:
uuid = self.device.get_property(PropertyTag.UNIQUE_DEVICE_IDENT) # type: ignore[arg-type]
uuid = self.device.get_property(PropertyTag.UNIQUE_DEVICE_IDENT)
if not uuid:
raise ValueError("Missing response for UUID property query")
if len(uuid) != UUID_LEN:
Expand Down
43 changes: 23 additions & 20 deletions pynitrokey/nk3/secrets_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
from hashlib import pbkdf2_hmac
from secrets import token_bytes
from struct import pack
from typing import List, Optional
from typing import Any, Callable, List, Optional, Sequence, Tuple, Union

import semver
import tlv8

from pynitrokey.nk3 import Nitrokey3Device
from pynitrokey.nk3.device import Nitrokey3Device
from pynitrokey.start.gnuk_token import iso7816_compose

LogFn = Callable[[str], Any]
WriteCorpusFn = Callable[[typing.Union["Instruction", "CCIDInstruction"], bytes], Any]


@dataclasses.dataclass
class ListItemProperties:
Expand Down Expand Up @@ -75,7 +78,7 @@ class PasswordSafeEntry:
properties: Optional[bytes] = None
name: Optional[bytes] = None

def tlv_encode(self) -> List[tlv8.Entry]:
def tlv_encode(self) -> list[tlv8.Entry]:
entries = [
tlv8.Entry(Tag.PwsLogin.value, self.login)
if self.login is not None
Expand All @@ -94,7 +97,7 @@ def tlv_encode(self) -> List[tlv8.Entry]:

@dataclasses.dataclass
class RawBytes:
data: List
data: list[int]


@dataclasses.dataclass
Expand Down Expand Up @@ -292,28 +295,30 @@ class SecretsApp:
"""

log: logging.Logger
logfn: typing.Callable
logfn: LogFn
dev: Nitrokey3Device
write_corpus_fn: Optional[typing.Callable]
write_corpus_fn: Optional[WriteCorpusFn]
_cache_status: Optional[SelectResponse]
_metadata: dict
_metadata: dict[Any, Any]

def __init__(self, dev: Nitrokey3Device, logfn: Optional[typing.Callable] = None):
def __init__(self, dev: Nitrokey3Device, logfn: Optional[LogFn] = None):
self._cache_status = None
self.write_corpus_fn = None
self.log = logging.getLogger("otpapp")
if logfn is not None:
self.logfn = logfn # type: ignore [assignment]
self.logfn = logfn
else:
self.logfn = self.log.info # type: ignore [assignment]
self.logfn = self.log.info
self.dev = dev
self._metadata = {}

def _custom_encode(self, structure: Optional[List] = None) -> bytes:
def _custom_encode(
self, structure: Optional[Sequence[Union[tlv8.Entry, RawBytes, None]]] = None
) -> bytes:
if not structure:
return b""

def transform(d: typing.Union[tlv8.Entry, RawBytes, None]) -> bytes:
def transform(d: Union[tlv8.Entry, RawBytes, None]) -> bytes:
if not d:
return b""
if isinstance(d, RawBytes):
Expand All @@ -332,7 +337,7 @@ def transform(d: typing.Union[tlv8.Entry, RawBytes, None]) -> bytes:
def _send_receive(
self,
ins: typing.Union[Instruction, CCIDInstruction],
structure: Optional[List] = None,
structure: Optional[Sequence[Union[tlv8.Entry, RawBytes, None]]] = None,
) -> bytes:
encoded_structure = self._custom_encode(structure)
ins_b, p1, p2 = self._encode_command(ins)
Expand Down Expand Up @@ -367,7 +372,7 @@ def _send_receive_inner(self, data: bytes, log_info: str = "") -> bytes:
)
log_multipacket = True
ins_b, p1, p2 = self._encode_command(Instruction.SendRemaining)
bytes_data = iso7816_compose(ins_b, p1, p2, data=[])
bytes_data = iso7816_compose(ins_b, p1, p2)
try:
result = self.dev.otp(data=bytes_data)
except Exception as e:
Expand Down Expand Up @@ -423,17 +428,15 @@ def reset(self) -> None:
self.logfn("Executing reset")
self._send_receive(Instruction.Reset)

def list(
self, extended: bool = False
) -> typing.List[typing.Union[typing.Tuple[bytes, bytes], bytes]]:
def list(self, extended: bool = False) -> list[Union[Tuple[bytes, bytes], bytes]]:
"""
Return a list of the registered credentials
:return: List of bytestrings, or tuple of bytestrings, if "extended" switch is provided
@deprecated
"""
raw_res = self._send_receive(Instruction.List)
resd: tlv8.EntryList = tlv8.decode(raw_res)
res: List[typing.Union[typing.Tuple[bytes, bytes], bytes]] = []
res: list[Union[Tuple[bytes, bytes], bytes]] = []
for e in resd:
# e: tlv8.Entry
if extended:
Expand Down Expand Up @@ -635,7 +638,7 @@ def encode_properties_to_send(
| (0x04 if pin_based_encryption else 0x00),
]
structure = list(filter(lambda x: x is not None, structure))
return RawBytes(structure)
return RawBytes(structure) # type: ignore[arg-type]

def calculate(self, cred_id: bytes, challenge: Optional[int] = None) -> bytes:
"""
Expand Down Expand Up @@ -759,7 +762,7 @@ def validate_raw(self, challenge: bytes, response: bytes) -> bytes:
]
raw_res = self._send_receive(Instruction.Validate, structure=structure)
resd: tlv8.EntryList = tlv8.decode(raw_res)
return resd.data
return resd.data # type: ignore[no-any-return]

def select(self) -> SelectResponse:
"""
Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/nk3/updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import pynitrokey
from pynitrokey.helpers import Retries
from pynitrokey.nk3 import Nitrokey3Base
from pynitrokey.nk3.base import Nitrokey3Base
from pynitrokey.nk3.bootloader import (
FirmwareContainer,
Nitrokey3Bootloader,
Expand Down
9 changes: 8 additions & 1 deletion pynitrokey/start/gnuk_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ def icc_compose(msg_type, data_len, slot, seq, param, data):
return pack("<BiBBBH", msg_type, data_len, slot, seq, 0, param) + data


def iso7816_compose(ins, p1, p2, data, cls=0x00, le=None):
def iso7816_compose(
ins: int,
p1: int,
p2: int,
data: bytes = b"",
cls: int = 0x00,
le: Optional[int] = None,
) -> bytes:
data_len = len(data)
if data_len == 0:
if not le:
Expand Down
14 changes: 8 additions & 6 deletions pynitrokey/updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import os.path
import urllib.parse
from dataclasses import dataclass
from typing import BinaryIO, Callable, Dict, Generator, Optional, Pattern
from typing import Any, BinaryIO, Callable, Dict, Generator, Optional, Pattern

import requests

Expand Down Expand Up @@ -97,7 +97,7 @@ class Release:
def __str__(self) -> str:
return self.tag

def find_asset(self, url_pattern: Pattern) -> Optional[Asset]:
def find_asset(self, url_pattern: Pattern[str]) -> Optional[Asset]:
urls = []
for asset in self.assets:
if url_pattern.search(asset):
Expand All @@ -112,7 +112,7 @@ def find_asset(self, url_pattern: Pattern) -> Optional[Asset]:
else:
return None

def require_asset(self, url_pattern: Pattern) -> Asset:
def require_asset(self, url_pattern: Pattern[str]) -> Asset:
update = self.find_asset(url_pattern)
if not update:
raise ValueError(
Expand All @@ -121,7 +121,7 @@ def require_asset(self, url_pattern: Pattern) -> Asset:
return update

@classmethod
def _from_api_response(cls, release: dict) -> "Release":
def _from_api_response(cls, release: dict[Any, Any]) -> "Release":
tag = release["tag_name"]
assets = [asset["browser_download_url"] for asset in release["assets"]]
if not assets:
Expand Down Expand Up @@ -150,14 +150,16 @@ def get_release_or_latest(self, tag: Optional[str] = None) -> Release:
return self.get_release(tag)
return self.get_latest_release()

def _call(self, path: str, errors: Dict[int, str] = dict()) -> dict:
def _call(self, path: str, errors: Dict[int, str] = dict()) -> dict[Any, Any]:
url = self._get_url(path)
response = requests.get(url)
for code in errors:
if response.status_code == code:
raise ValueError(errors[code])
response.raise_for_status()
return response.json()
data = response.json()
assert isinstance(data, dict)
return data

def _get_url(self, path: str) -> str:
return API_BASE_URL + path
26 changes: 23 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,32 @@ extend_skip = ["pynitrokey/nethsm/client"]
mypy_path = "stubs"
show_error_codes = true
python_version = "3.9"
warn_unused_configs = true
warn_redundant_casts = true

# enable strict checks for new code
# enable strict checks for new code, see
# - https://github.com/python/mypy/issues/11401
# - https://mypy.readthedocs.io/en/stable/existing_code.html#introduce-stricter-options
[[tool.mypy.overrides]]
module = [
"pynitrokey.cli.fido2.*",
"pynitrokey.cli.nk3.*",
"pynitrokey.fido2.*",
"pynitrokey.nk3.*",
"pynitrokey.updates.*",
"pynitrokey.fido2.*",
"pynitrokey.cli.fido2.*",
]
check_untyped_defs = true
disallow_any_generics = true
disallow_incomplete_defs = true
disallow_subclassing_any = true
disallow_untyped_calls = true
disallow_untyped_decorators = true
disallow_untyped_defs = true
no_implicit_reexport = true
strict_concatenate = true
strict_equality = true
warn_unused_ignores = true
warn_return_any = true

# pynitrokey.nethsm.client is auto-generated
[[tool.mypy.overrides]]
Expand All @@ -103,6 +118,11 @@ ignore_errors = true
module = "pynitrokey.nk3.bootloader.nrf52_upload.*"
ignore_errors = true

# nrf52 has to use the untyped nrf52_upload module
[[tool.mypy.overrides]]
module = "pynitrokey.nk3.bootloader.nrf52"
disallow_untyped_calls = false

# libraries without annotations
[[tool.mypy.overrides]]
module = [
Expand Down

0 comments on commit 3780bea

Please sign in to comment.