From c9feff0f4dcb0549adec926a252aa5e7b26fcb71 Mon Sep 17 00:00:00 2001 From: Trey Moen Date: Mon, 1 Sep 2025 10:41:42 -0700 Subject: [PATCH 1/5] feat: add LPA validations --- system/hardware/base.py | 19 +++++ system/hardware/tests/test_lpa_validation.py | 80 ++++++++++++++++++++ system/hardware/tici/esim.py | 21 ++--- 3 files changed, 110 insertions(+), 10 deletions(-) create mode 100644 system/hardware/tests/test_lpa_validation.py diff --git a/system/hardware/base.py b/system/hardware/base.py index ce97bf294d..2e831c4c82 100644 --- a/system/hardware/base.py +++ b/system/hardware/base.py @@ -1,4 +1,5 @@ import os +import re from abc import abstractmethod, ABC from dataclasses import dataclass, fields @@ -96,6 +97,24 @@ class LPABase(ABC): def is_comma_profile(self, iccid: str) -> bool: return any(iccid.startswith(prefix) for prefix in ('8985235',)) + def _validate_iccid(self, iccid: str) -> None: + assert re.match(r'^89\d{17,18}$', iccid), 'invalid ICCID format. expected format: 8988303000000614227' + + def _validate_lpa_activation_code(self, lpa_activation_code: str) -> None: + assert re.match(r'^LPA:1\$.+\$.+$', lpa_activation_code), 'invalid LPA activation code format. expected format: LPA:1$domain$code' + + def _validate_nickname(self, nickname: str) -> None: + assert len(nickname) >= 1 and len(nickname) <= 16, 'nickname must be between 1 and 16 characters' + assert re.match(r'^[a-zA-Z0-9-_ ]+$', nickname), 'nickname must contain only alphanumeric characters, hyphens, underscores, and spaces' + + def _validate_profile_exists(self, iccid: str) -> None: + if not any(p.iccid == iccid for p in self.list_profiles()): + raise LPAProfileNotFoundError(f'profile {iccid} does not exist') + + def _validate_successful(self, msgs: list[dict]) -> None: + assert len(msgs) > 0, 'expected at least one message' + assert msgs[-1]['payload']['message'] == 'success', 'expected success notification' + class HardwareBase(ABC): @staticmethod def get_cmdline() -> dict[str, str]: diff --git a/system/hardware/tests/test_lpa_validation.py b/system/hardware/tests/test_lpa_validation.py new file mode 100644 index 0000000000..5060b358b1 --- /dev/null +++ b/system/hardware/tests/test_lpa_validation.py @@ -0,0 +1,80 @@ +import pytest + +from openpilot.system.hardware.base import LPABase, LPAProfileNotFoundError, Profile + + +class TestLPABase(LPABase): + + def list_profiles(self) -> list[Profile]: + return [] + + def get_active_profile(self) -> Profile | None: + return None + + def delete_profile(self, iccid: str) -> None: + pass + + def download_profile(self, qr: str, nickname: str | None = None) -> None: + pass + + def nickname_profile(self, iccid: str, nickname: str) -> None: + pass + + def switch_profile(self, iccid: str) -> None: + pass + + +class TestLPAValidation: + + def setup_method(self): + self.lpa = TestLPABase() + + def test_validate_iccid(self): + self.lpa._validate_iccid('8988303000000614227') + + with pytest.raises(AssertionError, match='invalid ICCID format'): + self.lpa._validate_iccid('') + + with pytest.raises(AssertionError, match='invalid ICCID format'): + self.lpa._validate_iccid('1234567890123456789') # Doesn't start with 89 + + def test_validate_lpa_activation_code(self): + self.lpa._validate_lpa_activation_code('LPA:1$rsp.truphone.com$QRF-BETTERROAMING-PMRDGIR2EARDEIT5') + + with pytest.raises(AssertionError, match='invalid LPA activation code format'): + self.lpa._validate_lpa_activation_code('') + + with pytest.raises(AssertionError, match='invalid LPA activation code format'): + self.lpa._validate_lpa_activation_code('LPA:1$domain.com') # Missing third part + + def test_validate_nickname(self): + self.lpa._validate_nickname('test_profile') + + with pytest.raises(AssertionError, match='nickname must be between 1 and 16 characters'): + self.lpa._validate_nickname('') + + with pytest.raises(AssertionError, match='nickname must contain only alphanumeric characters'): + self.lpa._validate_nickname('test.profile') # Contains invalid character + + def test_validate_successful(self): + self.lpa._validate_successful([{'payload': {'message': 'success'}}]) + + with pytest.raises(AssertionError, match='expected at least one message'): + self.lpa._validate_successful([]) + + with pytest.raises(AssertionError, match='expected success notification'): + self.lpa._validate_successful([{'payload': {'message': 'error'}}]) + + def test_validate_profile_exists(self, mocker): + existing_profiles = [Profile(iccid='8988303000000614227', nickname='test1', enabled=True, provider='Test Provider')] + + mocker.patch.object(self.lpa, 'list_profiles', return_value=existing_profiles) + self.lpa._validate_profile_exists('8988303000000614227') + + mocker.patch.object(self.lpa, 'list_profiles', return_value=[]) + with pytest.raises(LPAProfileNotFoundError, match='profile 8988303000000614227 does not exist'): + self.lpa._validate_profile_exists('8988303000000614227') + + mocker.patch.object(self.lpa, 'list_profiles', return_value=existing_profiles) + with pytest.raises(LPAProfileNotFoundError, match='profile 8988303000000614229 does not exist'): + self.lpa._validate_profile_exists('8988303000000614229') diff --git a/system/hardware/tici/esim.py b/system/hardware/tici/esim.py index 391ba45531..14dea4865d 100644 --- a/system/hardware/tici/esim.py +++ b/system/hardware/tici/esim.py @@ -4,7 +4,7 @@ import shutil import subprocess from typing import Literal -from openpilot.system.hardware.base import LPABase, LPAError, LPAProfileNotFoundError, Profile +from openpilot.system.hardware.base import LPABase, LPAError, Profile class TiciLPA(LPABase): def __init__(self, interface: Literal['qmi', 'at'] = 'qmi'): @@ -32,6 +32,7 @@ class TiciLPA(LPABase): return next((p for p in self.list_profiles() if p.enabled), None) def delete_profile(self, iccid: str) -> None: + self._validate_iccid(iccid) self._validate_profile_exists(iccid) latest = self.get_active_profile() if latest is not None and latest.iccid == iccid: @@ -39,9 +40,13 @@ class TiciLPA(LPABase): self._validate_successful(self._invoke('profile', 'delete', iccid)) self._process_notifications() - def download_profile(self, qr: str, nickname: str | None = None) -> None: + def download_profile(self, lpa_activation_code: str, nickname: str | None = None) -> None: self._check_bootstrapped() - msgs = self._invoke('profile', 'download', '-a', qr) + self._validate_lpa_activation_code(lpa_activation_code) + if nickname: + self._validate_nickname(nickname) + + msgs = self._invoke('profile', 'download', '-a', lpa_activation_code) self._validate_successful(msgs) new_profile = next((m for m in msgs if m['payload']['message'] == 'es8p_meatadata_parse'), None) if new_profile is None: @@ -51,11 +56,14 @@ class TiciLPA(LPABase): self._process_notifications() def nickname_profile(self, iccid: str, nickname: str) -> None: + self._validate_iccid(iccid) self._validate_profile_exists(iccid) + self._validate_nickname(nickname) self._validate_successful(self._invoke('profile', 'nickname', iccid, nickname)) def switch_profile(self, iccid: str) -> None: self._check_bootstrapped() + self._validate_iccid(iccid) self._validate_profile_exists(iccid) latest = self.get_active_profile() if latest and latest.iccid == iccid: @@ -126,10 +134,3 @@ class TiciLPA(LPABase): Process notifications stored on the eUICC, typically to activate/deactivate the profile with the carrier. """ self._validate_successful(self._invoke('notification', 'process', '-a', '-r')) - - def _validate_profile_exists(self, iccid: str) -> None: - if not any(p.iccid == iccid for p in self.list_profiles()): - raise LPAProfileNotFoundError(f'profile {iccid} does not exist') - - def _validate_successful(self, msgs: list[dict]) -> None: - assert msgs[-1]['payload']['message'] == 'success', 'expected success notification' From 3004059e7a610606781e9777bf8227bbda9304dd Mon Sep 17 00:00:00 2001 From: Trey Moen Date: Mon, 1 Sep 2025 10:46:51 -0700 Subject: [PATCH 2/5] wiki --- system/hardware/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/system/hardware/base.py b/system/hardware/base.py index 2e831c4c82..00f4a2bff7 100644 --- a/system/hardware/base.py +++ b/system/hardware/base.py @@ -98,6 +98,7 @@ class LPABase(ABC): return any(iccid.startswith(prefix) for prefix in ('8985235',)) def _validate_iccid(self, iccid: str) -> None: + # https://en.wikipedia.org/wiki/E.118#ICCID assert re.match(r'^89\d{17,18}$', iccid), 'invalid ICCID format. expected format: 8988303000000614227' def _validate_lpa_activation_code(self, lpa_activation_code: str) -> None: From 07455c77d8cab0d8b903a382b718a122599d2df2 Mon Sep 17 00:00:00 2001 From: Trey Moen Date: Thu, 4 Sep 2025 18:45:51 -0700 Subject: [PATCH 3/5] cleanup --- system/hardware/base.py | 18 +++----- system/hardware/tests/test_lpa_validation.py | 46 +++++++++----------- system/hardware/tici/esim.py | 4 ++ system/hardware/tici/tests/test_esim.py | 3 +- 4 files changed, 30 insertions(+), 41 deletions(-) diff --git a/system/hardware/base.py b/system/hardware/base.py index 00f4a2bff7..269c4022a9 100644 --- a/system/hardware/base.py +++ b/system/hardware/base.py @@ -10,9 +10,6 @@ NetworkType = log.DeviceState.NetworkType class LPAError(RuntimeError): pass -class LPAProfileNotFoundError(LPAError): - pass - @dataclass class Profile: iccid: str @@ -97,24 +94,19 @@ class LPABase(ABC): def is_comma_profile(self, iccid: str) -> bool: return any(iccid.startswith(prefix) for prefix in ('8985235',)) - def _validate_iccid(self, iccid: str) -> None: + def validate_iccid(self, iccid: str) -> None: # https://en.wikipedia.org/wiki/E.118#ICCID assert re.match(r'^89\d{17,18}$', iccid), 'invalid ICCID format. expected format: 8988303000000614227' - def _validate_lpa_activation_code(self, lpa_activation_code: str) -> None: + def validate_lpa_activation_code(self, lpa_activation_code: str) -> None: assert re.match(r'^LPA:1\$.+\$.+$', lpa_activation_code), 'invalid LPA activation code format. expected format: LPA:1$domain$code' - def _validate_nickname(self, nickname: str) -> None: + def validate_nickname(self, nickname: str) -> None: assert len(nickname) >= 1 and len(nickname) <= 16, 'nickname must be between 1 and 16 characters' assert re.match(r'^[a-zA-Z0-9-_ ]+$', nickname), 'nickname must contain only alphanumeric characters, hyphens, underscores, and spaces' - def _validate_profile_exists(self, iccid: str) -> None: - if not any(p.iccid == iccid for p in self.list_profiles()): - raise LPAProfileNotFoundError(f'profile {iccid} does not exist') - - def _validate_successful(self, msgs: list[dict]) -> None: - assert len(msgs) > 0, 'expected at least one message' - assert msgs[-1]['payload']['message'] == 'success', 'expected success notification' + def validate_profile_exists(self, iccid: str) -> None: + assert any(p.iccid == iccid for p in self.list_profiles()), f'profile {iccid} does not exist' class HardwareBase(ABC): @staticmethod diff --git a/system/hardware/tests/test_lpa_validation.py b/system/hardware/tests/test_lpa_validation.py index 5060b358b1..11fec71f17 100644 --- a/system/hardware/tests/test_lpa_validation.py +++ b/system/hardware/tests/test_lpa_validation.py @@ -1,9 +1,12 @@ import pytest -from openpilot.system.hardware.base import LPABase, LPAProfileNotFoundError, Profile +from openpilot.system.hardware.base import LPABase, Profile -class TestLPABase(LPABase): +class MockLPA(LPABase): + + def bootstrap(self) -> None: + pass def list_profiles(self) -> list[Profile]: return [] @@ -27,54 +30,45 @@ class TestLPABase(LPABase): class TestLPAValidation: def setup_method(self): - self.lpa = TestLPABase() + self.lpa = MockLPA() def test_validate_iccid(self): - self.lpa._validate_iccid('8988303000000614227') + self.lpa.validate_iccid('8988303000000614227') with pytest.raises(AssertionError, match='invalid ICCID format'): - self.lpa._validate_iccid('') + self.lpa.validate_iccid('') with pytest.raises(AssertionError, match='invalid ICCID format'): - self.lpa._validate_iccid('1234567890123456789') # Doesn't start with 89 + self.lpa.validate_iccid('1234567890123456789') # Doesn't start with 89 def test_validate_lpa_activation_code(self): - self.lpa._validate_lpa_activation_code('LPA:1$rsp.truphone.com$QRF-BETTERROAMING-PMRDGIR2EARDEIT5') + self.lpa.validate_lpa_activation_code('LPA:1$rsp.truphone.com$QRF-BETTERROAMING-PMRDGIR2EARDEIT5') with pytest.raises(AssertionError, match='invalid LPA activation code format'): - self.lpa._validate_lpa_activation_code('') + self.lpa.validate_lpa_activation_code('') with pytest.raises(AssertionError, match='invalid LPA activation code format'): - self.lpa._validate_lpa_activation_code('LPA:1$domain.com') # Missing third part + self.lpa.validate_lpa_activation_code('LPA:1$domain.com') # Missing third part def test_validate_nickname(self): - self.lpa._validate_nickname('test_profile') + self.lpa.validate_nickname('test_profile') with pytest.raises(AssertionError, match='nickname must be between 1 and 16 characters'): - self.lpa._validate_nickname('') + self.lpa.validate_nickname('') with pytest.raises(AssertionError, match='nickname must contain only alphanumeric characters'): - self.lpa._validate_nickname('test.profile') # Contains invalid character - - def test_validate_successful(self): - self.lpa._validate_successful([{'payload': {'message': 'success'}}]) - - with pytest.raises(AssertionError, match='expected at least one message'): - self.lpa._validate_successful([]) - - with pytest.raises(AssertionError, match='expected success notification'): - self.lpa._validate_successful([{'payload': {'message': 'error'}}]) + self.lpa.validate_nickname('test.profile') # Contains invalid character def test_validate_profile_exists(self, mocker): existing_profiles = [Profile(iccid='8988303000000614227', nickname='test1', enabled=True, provider='Test Provider')] mocker.patch.object(self.lpa, 'list_profiles', return_value=existing_profiles) - self.lpa._validate_profile_exists('8988303000000614227') + self.lpa.validate_profile_exists('8988303000000614227') mocker.patch.object(self.lpa, 'list_profiles', return_value=[]) - with pytest.raises(LPAProfileNotFoundError, match='profile 8988303000000614227 does not exist'): - self.lpa._validate_profile_exists('8988303000000614227') + with pytest.raises(AssertionError, match='profile 8988303000000614227 does not exist'): + self.lpa.validate_profile_exists('8988303000000614227') mocker.patch.object(self.lpa, 'list_profiles', return_value=existing_profiles) - with pytest.raises(LPAProfileNotFoundError, match='profile 8988303000000614229 does not exist'): - self.lpa._validate_profile_exists('8988303000000614229') + with pytest.raises(AssertionError, match='profile 8988303000000614229 does not exist'): + self.lpa.validate_profile_exists('8988303000000614229') diff --git a/system/hardware/tici/esim.py b/system/hardware/tici/esim.py index 14dea4865d..ad46138d4a 100644 --- a/system/hardware/tici/esim.py +++ b/system/hardware/tici/esim.py @@ -134,3 +134,7 @@ class TiciLPA(LPABase): Process notifications stored on the eUICC, typically to activate/deactivate the profile with the carrier. """ self._validate_successful(self._invoke('notification', 'process', '-a', '-r')) + + def _validate_successful(self, msgs: list[dict]) -> None: + assert len(msgs) > 0, 'expected at least one message' + assert msgs[-1]['payload']['message'] == 'success', 'expected success notification' diff --git a/system/hardware/tici/tests/test_esim.py b/system/hardware/tici/tests/test_esim.py index 6fab931cce..c6557a7801 100644 --- a/system/hardware/tici/tests/test_esim.py +++ b/system/hardware/tici/tests/test_esim.py @@ -1,7 +1,6 @@ import pytest from openpilot.system.hardware import HARDWARE, TICI -from openpilot.system.hardware.base import LPAProfileNotFoundError # https://euicc-manual.osmocom.org/docs/rsp/known-test-profile # iccid is always the same for the given activation code @@ -14,7 +13,7 @@ def cleanup(): lpa = HARDWARE.get_sim_lpa() try: lpa.delete_profile(TEST_ICCID) - except LPAProfileNotFoundError: + except AssertionError: pass lpa.process_notifications() From d425c69c9c3bc334ff59153a19c4d34a98ccbb28 Mon Sep 17 00:00:00 2001 From: Trey Moen Date: Thu, 4 Sep 2025 18:49:27 -0700 Subject: [PATCH 4/5] _ --- system/hardware/tici/esim.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/system/hardware/tici/esim.py b/system/hardware/tici/esim.py index ad46138d4a..bb1325a136 100644 --- a/system/hardware/tici/esim.py +++ b/system/hardware/tici/esim.py @@ -32,8 +32,8 @@ class TiciLPA(LPABase): return next((p for p in self.list_profiles() if p.enabled), None) def delete_profile(self, iccid: str) -> None: - self._validate_iccid(iccid) - self._validate_profile_exists(iccid) + self.validate_iccid(iccid) + self.validate_profile_exists(iccid) latest = self.get_active_profile() if latest is not None and latest.iccid == iccid: raise LPAError('cannot delete active profile, switch to another profile first') @@ -44,7 +44,7 @@ class TiciLPA(LPABase): self._check_bootstrapped() self._validate_lpa_activation_code(lpa_activation_code) if nickname: - self._validate_nickname(nickname) + self.validate_nickname(nickname) msgs = self._invoke('profile', 'download', '-a', lpa_activation_code) self._validate_successful(msgs) @@ -56,15 +56,15 @@ class TiciLPA(LPABase): self._process_notifications() def nickname_profile(self, iccid: str, nickname: str) -> None: - self._validate_iccid(iccid) - self._validate_profile_exists(iccid) - self._validate_nickname(nickname) + self.validate_iccid(iccid) + self.validate_profile_exists(iccid) + self.validate_nickname(nickname) self._validate_successful(self._invoke('profile', 'nickname', iccid, nickname)) def switch_profile(self, iccid: str) -> None: self._check_bootstrapped() - self._validate_iccid(iccid) - self._validate_profile_exists(iccid) + self.validate_iccid(iccid) + self.validate_profile_exists(iccid) latest = self.get_active_profile() if latest and latest.iccid == iccid: return From c59d26b8b4c28a46c5aed3376c88e047b7c39f52 Mon Sep 17 00:00:00 2001 From: Trey Moen Date: Thu, 4 Sep 2025 18:50:13 -0700 Subject: [PATCH 5/5] moar --- system/hardware/tici/esim.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/hardware/tici/esim.py b/system/hardware/tici/esim.py index bb1325a136..4203a35456 100644 --- a/system/hardware/tici/esim.py +++ b/system/hardware/tici/esim.py @@ -42,7 +42,7 @@ class TiciLPA(LPABase): def download_profile(self, lpa_activation_code: str, nickname: str | None = None) -> None: self._check_bootstrapped() - self._validate_lpa_activation_code(lpa_activation_code) + self.validate_lpa_activation_code(lpa_activation_code) if nickname: self.validate_nickname(nickname)