From 13267ddd6b8d00553b19de679e1f6a3bf295d113 Mon Sep 17 00:00:00 2001 From: niwciu Date: Mon, 18 May 2026 16:23:10 +0200 Subject: [PATCH 1/3] feat: configurable product ID sections, mandatory keyring, full-width PID viz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Core: IdSectionDef / get_sections() for nibble-level productId splitting; DEFAULT_ID_SECTIONS matches EncryptBIN convention (custom_id/hw_id/license_id/unique_id) - CLI: sld fetch --section NAME=VALUE replaces --license/--unique; config show includes all http.* and product_id.sections fields - GUI: redesigned Server Settings dialog — scrollable section editor with live colour-coded 16-nibble strip (full-width QPainter widget, inclusive end display, position-based overlap prevention); URL path structure panel with ordering and live preview - Security: keyring promoted to required dependency; HTTP password always stored in OS keychain, never written to config.ini; legacy plaintext auto-migrated - Protocol: wire header reduced 48 B → 44 B (prevAppVersion stripped from CMD_START); DEVICE_HEADER_SIZE = 44 - FirmwareIdentifier redesigned to dict-backed immutable class with __getattr__ --- CHANGELOG.md | 85 ++- README.md | 20 +- docs/ARCHITECTURE.md | 95 ++- docs/FIRMWARE_FORMAT.md | 32 +- docs/PROTOCOL.md | 10 +- docs/USER_GUIDE.md | 71 ++- pyproject.toml | 5 +- secureloader_header_v1_1_migration.md | 112 ++++ src/secure_loader/__init__.py | 2 +- src/secure_loader/cli/main.py | 89 ++- src/secure_loader/config.py | 79 ++- src/secure_loader/core/firmware.py | 33 +- src/secure_loader/core/id_sections.py | 99 ++++ src/secure_loader/core/protocol.py | 11 + src/secure_loader/core/sources/__init__.py | 3 + src/secure_loader/core/sources/base.py | 88 ++- src/secure_loader/gui/main_window.py | 17 +- .../gui/server_settings_dialog.py | 560 ++++++++++++++---- tests/test_cli.py | 23 +- tests/test_config.py | 44 +- tests/test_firmware.py | 25 +- tests/test_github_source.py | 4 +- tests/test_gui.py | 68 +-- tests/test_http_source.py | 20 +- tests/test_local_source.py | 2 +- tests/test_workers.py | 14 +- 26 files changed, 1234 insertions(+), 377 deletions(-) create mode 100644 secureloader_header_v1_1_migration.md create mode 100644 src/secure_loader/core/id_sections.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f65b61..6ae26dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,88 @@ versioning follows [Semantic Versioning](https://semver.org/). ## [Unreleased] -### Planned +### Added + +- **Mandatory OS keychain credential storage** — `keyring` is now a required + dependency (previously the optional `[security]` extra). The HTTP password is + always stored in macOS Keychain, Windows Credential Manager, or a D-Bus secret + store on Linux and is **never written to `config.ini`**. Existing configs that + still contain a plaintext password are silently migrated to the keychain on the + first load. + +### Changed + +- **Section editor — inclusive end display** — the *End* spinbox now shows the + last nibble *included* in the section (range 0–15) instead of the + Python-exclusive upper bound (range 1–16), eliminating the confusing appearance + that adjacent sections share a boundary nibble. Internal `IdSectionDef` + storage is unchanged. +- **Section editor — position-based spinbox constraints** — overlap prevention + now operates on nibble position rather than GUI row order, so a section can be + freely repositioned across the ID without being blocked by a neighbouring row. + New sections default to the first uncovered nibble. +- **Product ID visualisation** — the 16-nibble colour strip is now rendered by a + native `QPainter` widget that scales to the full dialog width. Section names + and nibble ranges are displayed in a larger font. + +## [2.0.0] — 2026-05-17 + +This release completes the HTTP firmware download feature, introducing +fully configurable Product ID sectioning and a redesigned Server Settings dialog. + +> ⚠️ **Breaking change (bootloader):** the UART wire header shrinks from 48 B to +> 44 B. Devices running **bootloader v1.0.0** will stall on `CMD_START` (they +> wait for 48 bytes and misparse the first page command). Flash the updated +> bootloader (v1.1+) to the target device before deploying this release. + +> ⚠️ **Breaking change (CLI):** `sld fetch --license / --unique` are replaced by +> `sld fetch --section NAME=VALUE` (repeatable). Update any scripts that call +> `sld fetch` with the old flags. + +### Added + +- **Configurable Product ID sections** (`core/id_sections.py`) — the 64-bit + `productId` can now be split into any number of named sections at nibble + (half-byte) granularity (0–16). Each section has a `name`, `start`, and `end` + nibble position. The default split matches the EncryptBIN / SecureBootloader + convention: `custom_id [0:8]`, `hw_id [8:10]`, `license_id [10:12]`, + `unique_id [12:16]`. +- **`product_id.sections` config key** — serialised as `name:start:end,…`; stored + in the `[product_id]` INI section. Configurable via + `sld config set product_id.sections` or the new GUI section editor. +- **`get_sections(defs)` on `FirmwareHeader` and `DeviceInfo`** — extracts section + values from `productId` using a list of `IdSectionDef` objects. +- **Redesigned Server Settings dialog** — four groups: + - *Server* — base URL + allow-insecure checkbox (unchanged). + - *Credentials* — login / password (unchanged). + - *Product ID sections* — scrollable section editor (name + start/end spinboxes + + delete per row, "Add section" button). A 16-nibble colour-coded hex strip + visualises the current layout dynamically. + - *URL path structure* — checklist of defined sections with Up/Down ordering and + a live URL preview. + +### Changed + +- **`sld fetch`** — `--license` / `--unique` flags replaced by repeatable + `--section NAME=VALUE` (e.g. `--section license_id=AB --section unique_id=C0FE`). + Any section name defined in `product_id.sections` is accepted. +- **`sld config show`** — now includes all `http.*` fields, `product_id.sections`, + and `ui.instruction_url`. +- **`FirmwareIdentifier`** — redesigned from a frozen dataclass with fixed keyword + fields to a dict-backed immutable class. Sections are accessed as attributes + (`identifier.license_id`) via `__getattr__`; all existing callers using the + default section names continue to work without changes. +- **Wire header reduced from 48 B to 44 B** — `prevAppVersion` is no longer + included in the `CMD_START` payload. The field is still read from the `.bin` + file and available for host-side rollback logic, but the bootloader + `header_t` struct no longer contains it (bootloader v1.1+ change). + `build_device_header()` now returns `file[0:16] + file[20:48]`. +- **`DEVICE_HEADER_SIZE`** constant updated from `48` to `44`. + +### Fixed -- `GithubReleasesFirmwareSource` — complete integration with private - GitHub repo (see [docs/GITHUB_SOURCE_MIGRATION.md](docs/GITHUB_SOURCE_MIGRATION.md)). -- Settings dialog in GUI (firmware source selection, PAT entry). +- **CI workflow** — updated action versions for compatibility with current + GitHub Actions runner environment. ## [1.2.0] — 2026-05-09 diff --git a/README.md b/README.md index 5e0b17d..64779af 100644 --- a/README.md +++ b/README.md @@ -191,17 +191,31 @@ sld info --port COM3 SecureLoader can download firmware directly from an HTTP server before flashing — no manual file transfer needed. +The server URL path is built from configurable **Product ID sections**: named nibble-level slices of +the device's 64-bit `productId`. The default layout matches the EncryptBIN / SecureBootloader +convention (`license_id` and `unique_id` as path segments), but any split and naming scheme is supported. + ```bash # 1. Configure your server URL once (HTTPS is required by default) sld config set http.base_url https://myserver/update -# 2. Download the latest firmware (license/unique IDs come from the device) -sld fetch --license AB --unique C0FE --output firmware.bin +# 2. Download the latest firmware using the default sections +sld fetch --section license_id=AB --section unique_id=C0FE --output firmware.bin # 3. Flash as usual sld flash --file firmware.bin --port /dev/ttyUSB0 ``` +To use custom Product ID sections matching your server layout: + +```bash +# Define a custom split (once, saved to config) +sld config set product_id.sections "product:0:6,serial:6:16" + +# Download using the custom section names +sld fetch --section product=AABB --section serial=C0FE0001 --output firmware.bin +``` + If the server requires authentication, set credentials once: ```bash @@ -212,7 +226,7 @@ sld config set-password # secure interactive prompt — avoids shell hi > ⚠️ Plain HTTP (`http://`) is rejected by default. Pass `--allow-insecure` to `fetch` > only in isolated lab environments where HTTPS is not available. -The GUI exposes the same feature via the **Fetch from server** and **Get Previous Firmware** buttons — both become active once a device is connected. +The GUI exposes the same feature via the **Fetch from server** and **Get Previous Firmware** buttons — both become active once a device is connected. The **Settings → Server settings** dialog lets you visually design the Product ID section layout and URL path structure. See the [User Guide](https://niwciu.github.io/SecureLoader/USER_GUIDE/) for HTTP server path requirements and full configuration details. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3499108..7035fd3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -68,19 +68,32 @@ graph TB Five modules encapsulating all business logic. +### `core/id_sections.py` + +**Responsibility:** defining how the 64-bit `productId` is split into named sections. + +Key objects: +- `IdSectionDef(name, start, end)` — an immutable nibble-range slice of the 16-character hex + representation of `productId`. `start` and `end` are 0-based nibble positions (0–16). + `extract(hex_id)` returns the substring `hex_id[start:end]`. +- `DEFAULT_ID_SECTIONS` — the default split matching the ecosystem convention: + `custom_id [0:8]`, `hw_id [8:10]`, `license_id [10:12]`, `unique_id [12:16]`. +- `serialize_id_sections(defs) -> str` — serialises to `name:start:end,…` for INI storage. +- `parse_id_sections(s) -> list[IdSectionDef]` — parses the above format; returns defaults if `s` is blank. + ### `core/firmware.py` **Responsibility:** parsing the `.bin` binary format. Key objects: - `HEADER_SIZE = 48` — total header size on disk. -- `DEVICE_HEADER_SIZE = 48` — header size sent to the device (identical to the file header). -- `FirmwareHeader` — dataclass with fields and helpers (`format_product_id`, - `license_id`, `unique_id`, `payload_size`). +- `DEVICE_HEADER_SIZE = 44` — header size sent to the device (prevAppVersion stripped). +- `FirmwareHeader` — frozen dataclass with fields and helpers (`format_product_id`, + `format_app_version`, `payload_size`, `get_sections(defs)`). - `parse_header(bytes) -> FirmwareHeader` — parses the first 48 bytes. - `load_firmware(path) -> (FirmwareHeader, bytes)` — parses from file. -- `build_device_header(bytes) -> bytes` — returns the 48-byte wire header - (identical to the file header). +- `build_device_header(bytes) -> bytes` — returns the 44-byte wire header + (file bytes [0:16] + [20:48]; prevAppVersion at [16:20] is omitted). - `split_pages(payload, page_size) -> list[bytes]` — splits payload into equal pages; an incomplete final page is dropped (matching C++ behaviour). @@ -143,8 +156,16 @@ class FirmwareSource(ABC): def fetch_previous(self, identifier, progress=None) -> bytes: ... ``` -`FirmwareIdentifier` carries `license_id`, `unique_id`, optionally -`app_version` (used by `fetch_previous`). +`FirmwareIdentifier` is a dict-backed immutable class that carries named product-ID +sections extracted from the device's `productId` via `get_sections(config.id_section_defs)`. +Sections are accessed as attributes (`identifier.license_id`) or via `identifier.get("name", "")`. +`app_version` is a reserved attribute for rollback fetches (holds `prevAppVersion`). + +```python +# Building an identifier from a connected device: +sections = device_info.get_sections(config.id_section_defs) +identifier = FirmwareIdentifier(sections, app_version=prev_version) +``` `FirmwareSource` is used by the network-fetch paths only: the CLI `fetch` command and the GUI _Fetch from server_ / _Get Previous Firmware_ buttons both use @@ -223,7 +244,8 @@ Files: [gui/](https://github.com/niwciu/SecureLoader/tree/main/src/secure_loader gui/ ├── app.py — QApplication bootstrap, icon, language ├── main_window.py — QMainWindow reproducing mainwindow.ui 1:1 -├── server_settings_dialog.py — QDialog for server URL, credentials, and URL path structure +├── server_settings_dialog.py — QDialog for server URL, credentials, product-ID section editor, +│ and URL path structure ├── login_dialog.py — legacy QDialog (credentials only; kept for tests) └── workers.py — ProtocolWorker, DownloadWorker (QThread wrappers) ``` @@ -313,6 +335,9 @@ password = use_credentials = false path_segments = license_id,unique_id +[product_id] +sections = custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16 + [ui] language = auto instruction_url = @@ -333,10 +358,15 @@ Fields: is installed; otherwise in plaintext with `chmod 0600` on Unix. _Backward compat_: if `use_credentials` is absent in an existing file, it is inferred as `true` when `login` is non-empty, so pre-existing credentials continue to work. -- `http.path_segments` — comma-separated list of `FirmwareIdentifier` field names that form - the URL path between `base_url` and the filename. Defaults to `license_id,unique_id`. - Available names: `custom_id`, `hw_id`, `license_id`, `unique_id`. +- `http.path_segments` — comma-separated list of section names (defined in `product_id.sections`) + that form the URL path between `base_url` and the filename. Defaults to `license_id,unique_id`. Configured via **Settings → Server settings → URL path structure**. +- `product_id.sections` — comma-separated section definitions in `name:start:end` format, where + `start` and `end` are nibble (half-byte) positions within the 16-character hex representation + of the 64-bit `productId`. Defaults to the four-section ecosystem convention + (`custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16`). Configured via + **Settings → Server settings → Product ID sections**. Can also be set with: + `sld config set product_id.sections name1:0:6,name2:6:16` - `ui.language` — `"auto" | "en" | "de" | "fr" | "es" | "it" | "pl"`. - `ui.instruction_url` — optional URL opened by the _Update instruction…_ GUI menu item. When empty, the menu item is hidden. @@ -414,35 +444,36 @@ offset size field 48 … encrypted pages ``` -Wire header = bytes `[0:48]` = 48 B (sent with the `START` command, identical to the file header). +Wire header = bytes `[0:16]` + `[20:48]` = 44 B (sent with the `START` command; prevAppVersion at `[16:20]` is omitted). -### Product ID Convention +### Product ID Sections -The 64-bit `productId` is treated as an **opaque value** by SecureLoader — it -is compared as a whole between the firmware file and the device response. -No partial matching is performed. +The 64-bit `productId` is treated as an **opaque value** for compatibility checks +(compared as a whole between the firmware file and the device response). For HTTP +firmware routing, it is split into named sections at nibble (half-byte) granularity. -By convention, the ecosystem tools (EncryptBIN, SecureBootloader) lay out the -8 bytes as follows: +The default split matches the ecosystem convention (EncryptBIN / SecureBootloader): ``` Hex digit: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 Byte: ╔══════════════════════╗ ╔══╗ ╔═══╗ ╔════════╗ - ║ custom (4 B) ║ ║hw║ ║lic║ ║unique ║ + ║ custom_id (4 B) ║ ║hw║ ║lic║ ║unique ║ ╚══════════════════════╝ ╚══╝ ╚═══╝ ╚════════╝ - free for product use hw_id lic unique_id + nibbles [0:8] [8:10][10:12] [12:16] ``` -| Slice | Chars | Bytes | Name | Used by SecureLoader | -|-------|-------|-------|------|----------------------| -| `[0:8]` | 0–7 | 0–3 | custom | no (opaque) | -| `[8:10]` | 8–9 | 4 | `hw_id` | no (opaque) | -| `[10:12]` | 10–11 | 5 | `license_id` | HTTP fetch routing | -| `[12:16]` | 12–15 | 6–7 | `unique_id` | HTTP fetch routing | +| Section name | Nibble range | Bytes | Default purpose | +|--------------|-------------|-------|-----------------| +| `custom_id` | `[0:8]` | 0–3 | Product-specific, free for use | +| `hw_id` | `[8:10]` | 4 | Hardware revision | +| `license_id` | `[10:12]`| 5 | License tier | +| `unique_id` | `[12:16]`| 6–7 | Device serial number | -`license_id` and `unique_id` are extracted only when routing a firmware -download request to the HTTP server (`FirmwareIdentifier`). For all other -operations (compatibility check, device matching) the full 64-bit value is used. +This layout is the default — it can be changed at any time in +**Settings → Server settings → Product ID sections** or via +`sld config set product_id.sections `. Any names and nibble +boundaries (0–16) are accepted, as long as names are unique and `start < end`. +Only the sections listed in `http.path_segments` appear in download URLs. ### Serial Commands (1 byte each) @@ -458,7 +489,7 @@ operations (compatibility check, device matching) the full 64-bit value is used. After ACK for `GET_VERSION` the device sends 16 bytes of device info (`u32 bootloaderVersion`, `u64 productId`, `u32 flashPageSize`). -The host sends `START` (`0x02`) immediately followed by the 48 B wire header +The host sends `START` (`0x02`) immediately followed by the 44 B wire header as a single transmission. The device responds ACK/NAK. Then for each page the host sends `NEXT_PAGE` (`0x03`) + `flashPageSize` bytes, and the device ACKs. @@ -479,9 +510,9 @@ Files: [tests/](https://github.com/niwciu/SecureLoader/tree/main/tests/). - `test_protocol.py` — state machine, tested by injecting bytes into `_handle_byte()` (no pyserial mocking). - `test_updater.py` — `check_device_matches_firmware`. -- `test_config.py` — config load/save and keyring integration. +- `test_config.py` — config load/save, `id_section_defs` round-trip, and keyring integration. - `test_http_source.py` — `HttpFirmwareSource`: URL encoding, TLS enforcement, - auth passthrough, 100 MB download cap, version string validation. + auth passthrough, 100 MB download cap, version string validation, configurable path segments. - `test_local_source.py` — `LocalFirmwareSource`. - `test_github_source.py` — `GithubReleasesFirmwareSource` with mocked GitHub API. - `test_cli.py` — CLI commands: `config set/show/path`, `fetch` (HTTP rejection, diff --git a/docs/FIRMWARE_FORMAT.md b/docs/FIRMWARE_FORMAT.md index c0d6212..d16203a 100644 --- a/docs/FIRMWARE_FORMAT.md +++ b/docs/FIRMWARE_FORMAT.md @@ -14,7 +14,7 @@ Host implementation: [`src/secure_loader/core/firmware.py`](https://github.com/n 2. [Header Layout (48 B)](#-header-layout-48-b) 3. [Field Semantics](#-field-semantics) 4. [productId Derived Fields](#-productid-derived-fields) -5. [Wire Header (48 B)](#-wire-header-48-b) +5. [Wire Header (44 B)](#-wire-header-44-b) 6. [Payload](#-payload) 7. [Host Parsing API](#-host-parsing-api) @@ -89,7 +89,8 @@ informational purposes. Not validated by the bootloader protocol. Version of the previous application image. Used by the host to request an older firmware from the HTTP source (`fetch_previous`). GitHub Releases support is planned — see [Roadmap](GITHUB_SOURCE_MIGRATION.md). -This field is transmitted to the device as part of the 48-byte wire header — see [Wire Header](#-wire-header-48-b). +**This field is read by the host but is NOT forwarded to the device** — the +44-byte wire header omits it (see [Wire Header](#-wire-header-44-b)). ### `pageCount` (u32) @@ -148,11 +149,16 @@ for the full fetch workflow. --- -## 📡 Wire Header (48 B) +## 📡 Wire Header (44 B) -The header transmitted to the device during a firmware update is identical to -the full 48-byte file header. All fields — including `prevAppVersion` — are -sent as-is. +The header transmitted to the device during a firmware update is **44 bytes**. +`prevAppVersion` is read from the file and used internally by the host (e.g. +to fetch a rollback image), but is **not** included in the UART payload. +This matches the bootloader v1.1+ `header_t` struct layout exactly. + +> **Compatibility note:** the 44-byte wire header requires **bootloader v1.1+**. +> A bootloader built from v1.0.0 expects 48 bytes and will stall waiting for the +> missing 4 bytes. Confirm the target device runs v1.1+ before deploying. ```mermaid flowchart LR @@ -161,23 +167,23 @@ flowchart LR B["bytes 16–19
prevAppVersion"] C["bytes 20–47
pageCount · flashPageSize
IV · crc32"] end - subgraph wire["Wire header — 48 B (sent to device)"] + subgraph wire["Wire header — 44 B (sent to device)"] W["START command payload"] end A -- "copied" --> W - B -- "copied" --> W + B -. "host-only\n(not forwarded)" .-> W C -- "copied" --> W ``` Expressed as a slice operation: ``` -wire_header = disk_header[0:48] +wire_header = disk_header[0:16] + disk_header[20:48] = protocolVersion + productId (MSB+LSB) + appVersion - + prevAppVersion + pageCount + flashPageSize + IV + crc32 + + pageCount + flashPageSize + IV + crc32 ``` -This 48-byte block is the payload of the `START` command. +This 44-byte block is the payload of the `START` command. --- @@ -203,7 +209,7 @@ flowchart LR file["📄 .bin file"] --> lf["load_firmware(path)"] lf --> hdr["FirmwareHeader\n(frozen dataclass)"] lf --> raw["raw bytes"] - raw --> bdh["build_device_header(raw)\n→ 48 B wire header"] + raw --> bdh["build_device_header(raw)\n→ 44 B wire header"] raw --> sp["split_pages(payload, page_size)\n→ list[bytes]"] hdr --> fields["license_id · unique_id\npayload_size\nformat_product_id() · …"] ``` @@ -212,7 +218,7 @@ flowchart LR from secure_loader.core.firmware import ( parse_header, # parse_header(data: bytes) -> FirmwareHeader load_firmware, # load_firmware(path) -> (FirmwareHeader, bytes) - build_device_header, # build_device_header(raw: bytes) -> bytes (48 B) + build_device_header, # build_device_header(raw: bytes) -> bytes (44 B, prevAppVersion stripped) split_pages, # split_pages(payload, page_size) -> list[bytes] ) ``` diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index dde58bf..8352713 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -51,7 +51,7 @@ delimiter. | Name | Code | Payload | Meaning | |------|------|---------|---------| | `GET_VERSION` | `0x01` | — | Request device info | -| `START` | `0x02` | 48 B wire header | Begin firmware transfer | +| `START` | `0x02` | 44 B wire header | Begin firmware transfer | | `NEXT_PAGE` | `0x03` | `flashPageSize` B | One payload page | | `RESET` | `0x04` | — | Soft-reset the device | | `NONE` | `0x00` | — | Reserved, unused | @@ -100,8 +100,8 @@ One rule covers all commands — no translation table needed on either side. ### `START` (`0x02`) -**Host sends:** `0x02`, then the **48-byte wire header** -(see [Firmware Format — Wire Header](FIRMWARE_FORMAT.md#-wire-header-48-b)). +**Host sends:** `0x02`, then the **44-byte wire header** +(see [Firmware Format — Wire Header](FIRMWARE_FORMAT.md#-wire-header-44-b)). **Device responds:** @@ -157,7 +157,7 @@ sequenceDiagram Note over H: validates compatibility
(bootloaderVersion == protocolVersion, productId match) - H->>D: 0x02 START + 48 B wire header + H->>D: 0x02 START + 44 B wire header D->>H: 0x42 ACK (flash erased, decryption ready) loop pageCount pages @@ -272,7 +272,7 @@ stateDiagram-v2 [*] --> WAIT_CMD : power on / reset WAIT_CMD --> WAIT_CMD : 0x01 GET_VERSION → reply 0x41 + 16 B info - WAIT_CMD --> READ_HEADER : 0x02 START → read 48 B wire header + WAIT_CMD --> READ_HEADER : 0x02 START → read 44 B wire header WAIT_CMD --> [*] : 0x04 RESET → reply 0x44 · reset READ_HEADER --> WAIT_CMD : header invalid → 0x82 NAK diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index cfa53bf..318d4d4 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -217,43 +217,53 @@ sld info --file firmware.bin --port /dev/ttyUSB0 --parity odd Download a firmware binary from the configured HTTP server and save it to disk. +Section values are the named slices of the device's 64-bit Product ID as defined in +`product_id.sections` (default: `custom_id`, `hw_id`, `license_id`, `unique_id`). +Use `sld info --port ...` to read them from a connected device. + ```bash -# Download the latest firmware for a device -sld fetch --license AB --unique C0FE --output firmware.bin +# Download the latest firmware for a device (default sections) +sld fetch --section license_id=AB --section unique_id=C0FE --output firmware.bin # Download a specific (previous) version -sld fetch --license AB --unique C0FE --previous 0x01020300 --output prev.bin +sld fetch --section license_id=AB --section unique_id=C0FE --previous 0x01020300 --output prev.bin # Override the server URL for a one-off download (HTTPS recommended) -sld fetch --license AB --unique C0FE --output fw.bin --base-url https://myserver/update +sld fetch --section license_id=AB --section unique_id=C0FE \ + --output fw.bin --base-url https://myserver/update # Allow a plain HTTP server in a controlled environment (not for production) -sld fetch --license AB --unique C0FE --output fw.bin --base-url http://myserver/update --allow-insecure +sld fetch --section license_id=AB --section unique_id=C0FE \ + --output fw.bin --base-url http://myserver/update --allow-insecure + +# Custom sections matching your product_id.sections config +sld fetch --section product=AABB --section serial=C0FE0001 --output fw.bin ``` | Option | Description | |--------|-------------| -| `--license TEXT` | License ID (expected: two hex characters). Required. | -| `--unique TEXT` | Unique device ID (expected: four hex characters). Required. | +| `--section NAME=VALUE` | Product ID section value. Repeatable. The section names must match those in `product_id.sections`. At least one required. | | `--previous TEXT` | Fetch the version identified by this `appVersion` string instead of the latest. | | `--output FILE` | Destination file path (required). | | `--base-url TEXT` | Override `http.base_url` from config for this call only. | | `--allow-insecure` | Allow plain HTTP URLs and disabled TLS verification. Use only in controlled environments. | -The license and unique IDs can be read from the device using `sld info --port ...`. - #### 🌐 HTTP server requirements -`fetch` expects your server to expose files at the following URL structure (default path segments): +`fetch` expects your server to expose files at the following URL structure. +The path between `base_url` and the filename is built from the sections listed in +`http.path_segments` (default: `license_id,unique_id`): ``` -{base_url}/{license_id}/{unique_id}/info.txt +{base_url}/{license_id}/{unique_id}/info.txt # default path_segments {base_url}/{license_id}/{unique_id}/{version}.bin + +{base_url}/{product}/{serial}/info.txt # custom sections example +{base_url}/{product}/{serial}/{version}.bin ``` -The path segments between `base_url` and the filename are configurable. You can include any -combination of `custom_id`, `hw_id`, `license_id`, `unique_id` in any order via -`http.path_segments` in the config file, or through **Settings → Server settings → URL path structure** in the GUI. +The path segments and section definitions are fully configurable. +See `product_id.sections` and `http.path_segments` in the [Configuration Reference](#️-configuration-reference). | File | Content | Required for | |------|---------|--------------| @@ -279,7 +289,7 @@ Credentials are stored in the **OS keychain** when the `[security]` extra is ins (`pip install -e ".[security]"`). Without it, credentials fall back to the INI config file with `0600` permissions on Unix. See the [Configuration Reference](#️-configuration-reference) for details. -How `license_id` and `unique_id` are derived from the device's `productId`, and the full server path layout, is documented in [Firmware Format](FIRMWARE_FORMAT.md). +How the Product ID is split into sections, and the full server path layout, is documented in [Firmware Format](FIRMWARE_FORMAT.md). --- @@ -318,7 +328,7 @@ The command sequence: 1. Opens the serial port and polls `GET_VERSION` every 500 ms until the bootloader responds. 2. Reads device info (bootloader version, product ID, page size). 3. Checks compatibility — product ID and protocol version must match the firmware header (unless `--force`). -4. Sends `START` with the 48-byte wire header. +4. Sends `START` with the 44-byte wire header (prevAppVersion stripped). 5. Streams all `pageCount` pages, reporting progress to stdout. 6. Reports success or failure. @@ -354,6 +364,7 @@ Available keys: | `http.allow_insecure` | `true` / `false`. Allow plain `http://` URLs. Default `false`. | | `http.login` | HTTP server login (stored in plaintext, `0600` permissions on Unix). | | `http.password` | HTTP server password. | +| `product_id.sections` | Comma-separated section definitions in `name:start:end` nibble format. Default: `custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16`. | | `ui.language` | Display language: `auto`, `en`, `de`, `fr`, `es`, `it`, `pl`. | | `ui.instruction_url` | URL opened by **Help → Update instruction…**. Leave empty to hide the menu item. | @@ -432,18 +443,21 @@ Both buttons are **fully implemented**. They are disabled at startup and become After a successful download the binary is **loaded directly into the firmware fields** — it is ready to flash without selecting a file manually. -Server connection settings are configured via **Settings → Server settings**, which opens a dialog with three sections: +Server connection settings are configured via **Settings → Server settings**, which opens a dialog with four sections: - **Server** — the HTTPS base URL for your firmware server. An **Allow plain HTTP** checkbox is available for controlled lab environments where HTTPS is not available; leave it off for any network where credentials or firmware could be intercepted. - **Credentials** — enable the checkbox to activate HTTP Basic Auth, then enter login and - password. When the checkbox is off, requests are sent without authentication. The Product ID - field layout is shown as a colour-coded hex strip so you can see exactly which bytes map to - which fields. -- **URL path structure** — choose which Product ID fields (`custom_id`, `hw_id`, `license_id`, - `unique_id`) to include in the download path and in what order. A live preview shows the - resulting URL template. + password. When the checkbox is off, requests are sent without authentication. +- **Product ID sections** — define how the 64-bit Product ID is split into named sections at + nibble (half-byte) granularity. Add rows, give each a name and a nibble range (0–16). A + colour-coded hex strip visualises the current layout across all 16 nibbles. The default + split is `custom_id [0:8]`, `hw_id [8:10]`, `license_id [10:12]`, `unique_id [12:16]` and + matches the EncryptBIN / SecureBootloader ecosystem convention. +- **URL path structure** — choose which defined sections to include in the download URL path + and in what order. Use the Up/Down buttons to reorder. A live preview shows the resulting + URL template. The same [HTTP server requirements](#-http-server-requirements) as for the CLI `fetch` command apply. @@ -468,6 +482,9 @@ password = use_credentials = false path_segments = license_id,unique_id +[product_id] +sections = custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16 + [ui] language = auto instruction_url = @@ -486,7 +503,13 @@ firmware_1 = /home/user/projects/firmware_prev.bin | `use_credentials` | `true` / `false`. When `false` (default), `login` and `password` are ignored and requests are unauthenticated. Set to `true` via **Settings → Server settings → Credentials** checkbox. | | `login` | HTTP Basic Auth username, used only when `use_credentials = true`. | | `password` | HTTP Basic Auth password. Stored in OS keychain when `[security]` extra is installed; otherwise plaintext with `0600` permissions. Use `sld config set-password` to avoid shell history exposure. | -| `path_segments` | Comma-separated list of Product ID fields used to build the download URL path. Default: `license_id,unique_id`. Available: `custom_id`, `hw_id`, `license_id`, `unique_id`. | +| `path_segments` | Comma-separated list of section names (from `product_id.sections`) used to build the download URL path. Default: `license_id,unique_id`. | + +### `[product_id]` section + +| Key | Description | +|-----|-------------| +| `sections` | Comma-separated section definitions in `name:start:end` nibble format. Each entry maps a name to a half-byte range of the 16-character hex representation of `productId`. `start` is inclusive, `end` is exclusive (0–16). Names must be unique. Default: `custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16`. Configure via **Settings → Server settings → Product ID sections** or: `sld config set product_id.sections name1:0:6,name2:6:16` | ### `[ui]` section diff --git a/pyproject.toml b/pyproject.toml index d30eadc..5098c4f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ build-backend = "setuptools.build_meta" [project] name = "secureloader" -version = "1.2.0" +version = "2.0.0" description = "Encrypted firmware upload tool for embedded devices over serial" readme = "README.md" requires-python = ">=3.10" @@ -38,11 +38,11 @@ dependencies = [ "requests>=2.31", "click>=8.1", "platformdirs>=4.0", + "keyring>=24.0", ] [project.optional-dependencies] gui = ["PySide6>=6.6"] -security = ["keyring>=24.0"] dev = [ "pytest>=7.4", "pytest-cov>=4.1", @@ -55,7 +55,6 @@ dev = [ "types-requests>=2.31", "types-pyserial>=3.5", "pip-tools>=7.0", - "keyring>=24.0", "bandit[toml]>=1.7", "pip-audit>=2.7", ] diff --git a/secureloader_header_v1_1_migration.md b/secureloader_header_v1_1_migration.md new file mode 100644 index 0000000..a45f370 --- /dev/null +++ b/secureloader_header_v1_1_migration.md @@ -0,0 +1,112 @@ +# SecureLoader Migration: header_t v1.0 → v1.1 + +## Overview of the header flow + +Understanding who owns which part of the header is essential before making any changes. + +``` +┌─────────────┐ 48-byte header ┌──────────────┐ 44-byte header ┌─────────────┐ +│ EncryptBIN │ ─────────────────► │ SecureLoader │ ─────────────────► │ Bootloader │ +│ (creates │ (encrypted file) │ (host tool) │ (UART) │ header_t │ +│ .bin pkg) │ │ │ │ 44 bytes) │ +└─────────────┘ └──────────────┘ └─────────────┘ +``` + +- **EncryptBIN** produces an encrypted firmware package containing a **48-byte header** that includes `prev_app_version`. This format does **not change**. +- **SecureLoader** reads the 48-byte header from the file, uses `prev_app_version` internally (e.g. to fetch a rollback image from a server), then **strips `prev_app_version`** and transmits only **44 bytes** over UART to the bootloader. +- **The bootloader** receives a **44-byte header** (`header_t`, no `prev_app_version` field). It never sees `prev_app_version`. + +--- + +## What changed in the bootloader + +`prev_app_version` was removed from `header_t` and the downgrade-version guard was removed from `do_start()`. + +### `header_t` before (48 bytes — bootloader v1.0.0) + +```c +typedef struct { + uint32_t protocol_version; // Must match PROTOCOL_VERSION + uint32_t product_ID_MSB; // Upper 32 bits of DEVICE_ID + uint32_t product_ID_LSB; // Lower 32 bits of DEVICE_ID + uint32_t app_version; // Informational; not validated + uint32_t prev_app_version; // Downgrade guard: app_version must be >= this + uint32_t page_count; // 1 … (APP_LAST_PAGE - APP_START_PAGE + 1) + uint32_t flash_page_size; // Must match FLASH_PAGE_SIZE + uint8_t iv[16]; // AES-CBC initialisation vector + uint32_t crc; // Expected CRC-32 of the plaintext image +} header_t; // total: 48 bytes +``` + +### `header_t` after (44 bytes — bootloader v1.1+) + +```c +typedef struct { + uint32_t protocol_version; // Must match PROTOCOL_VERSION + uint32_t product_ID_MSB; // Upper 32 bits of DEVICE_ID + uint32_t product_ID_LSB; // Lower 32 bits of DEVICE_ID + uint32_t app_version; // Informational; not validated + uint32_t page_count; // 1 … (APP_LAST_PAGE - APP_START_PAGE + 1) + uint32_t flash_page_size; // Must match FLASH_PAGE_SIZE + uint8_t iv[16]; // AES-CBC initialisation vector + uint32_t crc; // Expected CRC-32 of the plaintext image +} header_t; // total: 44 bytes +``` + +--- + +## Required changes in SecureLoader + +EncryptBIN requires **no changes** — it continues to produce 48-byte headers. + +SecureLoader must transmit a 44-byte header over UART. The byte layout it sends must match the bootloader's 44-byte `header_t` exactly (all fields little-endian). + +### UART transmission byte layout (44 bytes) + +``` +offset 0 : protocol_version (uint32_t, 4 bytes, little-endian) +offset 4 : product_ID_MSB (uint32_t, 4 bytes, little-endian) +offset 8 : product_ID_LSB (uint32_t, 4 bytes, little-endian) +offset 12 : app_version (uint32_t, 4 bytes, little-endian) +offset 16 : page_count (uint32_t, 4 bytes, little-endian) +offset 20 : flash_page_size (uint32_t, 4 bytes, little-endian) +offset 24 : iv[0..15] (16 bytes) +offset 40 : crc (uint32_t, 4 bytes, little-endian) +``` + +`prev_app_version` (which sits at offset 16 in the 48-byte file header) must **not** be included in the UART transmission. All fields after `app_version` shift down by 4 bytes compared to the file layout. + +### File parsing byte layout (48 bytes — unchanged) + +``` +offset 0 : protocol_version (uint32_t, 4 bytes, little-endian) +offset 4 : product_ID_MSB (uint32_t, 4 bytes, little-endian) +offset 8 : product_ID_LSB (uint32_t, 4 bytes, little-endian) +offset 12 : app_version (uint32_t, 4 bytes, little-endian) +offset 16 : prev_app_version (uint32_t, 4 bytes, little-endian) ← read, use internally, do NOT forward to bootloader +offset 20 : page_count (uint32_t, 4 bytes, little-endian) +offset 24 : flash_page_size (uint32_t, 4 bytes, little-endian) +offset 28 : iv[0..15] (16 bytes) +offset 44 : crc (uint32_t, 4 bytes, little-endian) +``` + +### What SecureLoader must do + +1. Parse the full 48-byte header from the encrypted file (layout unchanged). +2. Read and use `prev_app_version` as needed (e.g. rollback image fetch from server). +3. Build the 44-byte UART header by copying all fields **except** `prev_app_version`, in the order shown in the UART layout table above. +4. Send `CMD_START` (0x02) followed immediately by the 44-byte UART header. + +### Compatibility note + +The 44-byte UART header is only compatible with **bootloader v1.1+**. A bootloader built from v1.0.0 expects 48 bytes; sending 44 bytes will cause it to stall waiting for the remaining 4 bytes and then misparse the first page command. Ensure the target device runs the matching bootloader version before uploading. + +--- + +## Summary checklist + +- [ ] **EncryptBIN** — no changes required; 48-byte file header format is unchanged +- [ ] **SecureLoader file parsing** — no changes required; continue reading the full 48-byte header from file +- [ ] **SecureLoader UART transmission** — build and send a 44-byte header (omit `prev_app_version`, adjust field offsets accordingly) +- [ ] Update any SecureLoader integration tests that construct or compare the raw UART byte sequence +- [ ] Confirm the target device runs bootloader v1.1+ before deploying updated SecureLoader diff --git a/src/secure_loader/__init__.py b/src/secure_loader/__init__.py index 522c713..03234bc 100644 --- a/src/secure_loader/__init__.py +++ b/src/secure_loader/__init__.py @@ -1,4 +1,4 @@ """SecureLoader.""" -__version__ = "1.2.0" +__version__ = "2.0.0" __app_name__ = "SecureLoader" diff --git a/src/secure_loader/cli/main.py b/src/secure_loader/cli/main.py index 903b170..ae482e4 100644 --- a/src/secure_loader/cli/main.py +++ b/src/secure_loader/cli/main.py @@ -5,9 +5,10 @@ sld list-ports sld info --file firmware.bin sld info --port /dev/ttyUSB0 - sld fetch --license 42 --unique C0FE --output firmware.bin + sld fetch --section license_id=42 --section unique_id=C0FE --output firmware.bin sld flash --port /dev/ttyUSB0 --file firmware.bin [--yes] sld config set http.login + sld config set product_id.sections custom_id:0:8,hw_id:8:10,license_id:10:12,unique_id:12:16 All output is intentionally plain (no colour by default, no progress bars by default) to make it easy to embed in scripts. Pass ``--verbose`` to @@ -17,8 +18,8 @@ ``--yes``/``-y`` to skip the prompt in non-interactive scripts. ``config set`` validates ``ui.language`` (must be one of the supported codes -or ``auto``) and ``http.base_url`` (must start with ``http://`` or -``https://``). +or ``auto``), ``http.base_url`` (must start with ``http://`` or ``https://``), +and ``product_id.sections`` (must be a valid nibble-range section list). """ from __future__ import annotations @@ -35,6 +36,7 @@ from ..config import AppConfig, config_path, load_config, save_config from ..core.audit import log_flash from ..core.firmware import FirmwareHeader, load_firmware, parse_header +from ..core.id_sections import DEFAULT_ID_SECTIONS, parse_id_sections, serialize_id_sections from ..core.protocol import ( DeviceInfo, Parity, @@ -63,17 +65,20 @@ def _setup_logging(verbose: int) -> None: ) -def _format_header(header: FirmwareHeader) -> str: +def _format_header(header: FirmwareHeader, config: AppConfig | None = None) -> str: + defs = config.id_section_defs if config is not None else list(DEFAULT_ID_SECTIONS) + sections = header.get_sections(defs) + section_lines = "".join( + f"\n {name:<16} = {value}" for name, value in sections.items() + ) return ( f" protocolVersion = {header.format_protocol_version()}\n" - f" productId = {header.format_product_id()}\n" + f" productId = {header.format_product_id()}{section_lines}\n" f" appVersion = {header.format_app_version()}\n" f" prevAppVersion = {header.format_prev_app_version()}\n" f" pageCount = {header.page_count}\n" f" flashPageSize = {header.flash_page_size} B\n" - f" payloadSize = {header.payload_size} B\n" - f" licenseID = {header.license_id}\n" - f" uniqueID = {header.unique_id}" + f" payloadSize = {header.payload_size} B" ) @@ -160,11 +165,12 @@ def info_cmd( if not file_path and not port: raise click.UsageError("Provide --file and/or --port.") + config: AppConfig = ctx.obj["config"] fw_header: FirmwareHeader | None = None if file_path: fw_header, _data = load_firmware(file_path) click.echo(_("Firmware header:")) - click.echo(_format_header(fw_header)) + click.echo(_format_header(fw_header, config)) if port: device = _query_device(port, Parity.from_label(parity), timeout, baudrate, float(stopbits)) @@ -214,8 +220,17 @@ def on_device(dev: DeviceInfo) -> None: @cli.command("fetch", help="Download a firmware image from the HTTP source.") -@click.option("--license", "license_id", required=True, help="License ID.") -@click.option("--unique", "unique_id", required=True, help="Unique device ID.") +@click.option( + "--section", + "sections", + multiple=True, + metavar="NAME=VALUE", + help=( + "Product ID section value, e.g. --section license_id=42. " + "Repeat for each section required by the server path. " + "Section names must match those defined in product_id.sections config." + ), +) @click.option( "--previous", "prev_version", @@ -239,14 +254,27 @@ def on_device(dev: DeviceInfo) -> None: @click.pass_context def fetch_cmd( ctx: click.Context, - license_id: str, - unique_id: str, + sections: tuple[str, ...], prev_version: str | None, out_path: Path, base_url: str | None, allow_insecure: bool, ) -> None: config: AppConfig = ctx.obj["config"] + + section_dict: dict[str, str] = {} + for entry in sections: + if "=" not in entry: + raise click.UsageError(f"--section must be NAME=VALUE, got: {entry!r}") + name, _, value = entry.partition("=") + section_dict[name.strip()] = value.strip() + + if not section_dict: + raise click.UsageError( + "Provide at least one --section NAME=VALUE to identify the firmware. " + "Example: --section license_id=42 --section unique_id=C0FE" + ) + effective_url = base_url or config.http_base_url source = HttpFirmwareSource( base_url=effective_url, @@ -254,11 +282,7 @@ def fetch_cmd( allow_insecure=allow_insecure, path_segments=config.http_path_segments, ) - identifier = FirmwareIdentifier( - license_id=license_id, - unique_id=unique_id, - app_version=prev_version, - ) + identifier = FirmwareIdentifier(section_dict, app_version=prev_version) def progress(received: int, total: int) -> None: if total: @@ -278,7 +302,7 @@ def progress(received: int, total: int) -> None: try: header = parse_header(data) click.echo(_("Firmware header:")) - click.echo(_format_header(header)) + click.echo(_format_header(header, config)) except Exception: log.exception("downloaded data does not parse as a firmware header") @@ -331,9 +355,10 @@ def flash_cmd( force: bool, yes: bool, ) -> None: + config: AppConfig = ctx.obj["config"] header, firmware = load_firmware(file_path) click.echo(_("Firmware header:")) - click.echo(_format_header(header)) + click.echo(_format_header(header, config)) device_info: dict[str, DeviceInfo] = {} connected = threading.Event() @@ -427,13 +452,17 @@ def config_path_cmd() -> None: @click.pass_context def config_show_cmd(ctx: click.Context) -> None: cfg: AppConfig = ctx.obj["config"] - click.echo(f"http.base_url = {cfg.http_base_url}") - click.echo(f"http.login = {cfg.http_login}") - click.echo(f"http.password = {'***' if cfg.http_password else ''}") - click.echo(f"ui.language = {cfg.language}") - click.echo(f"ui.instruction_url = {cfg.update_instruction_url}") + click.echo(f"http.base_url = {cfg.http_base_url}") + click.echo(f"http.login = {cfg.http_login}") + click.echo(f"http.password = {'***' if cfg.http_password else ''}") + click.echo(f"http.use_credentials = {cfg.http_use_credentials}") + click.echo(f"http.allow_insecure = {cfg.http_allow_insecure}") + click.echo(f"http.path_segments = {','.join(cfg.http_path_segments)}") + click.echo(f"product_id.sections = {serialize_id_sections(cfg.id_section_defs)}") + click.echo(f"ui.language = {cfg.language}") + click.echo(f"ui.instruction_url = {cfg.update_instruction_url}") for i, path in enumerate(cfg.last_firmware_paths): - click.echo(f"recent.firmware_{i} = {path}") + click.echo(f"recent.firmware_{i:<10} = {path}") @config_group.command("set", help="Set a configuration value (key=value).") @@ -449,6 +478,14 @@ def config_set_cmd(ctx: click.Context, key: str, value: str) -> None: "ui.language": "language", "ui.instruction_url": "update_instruction_url", } + if key == "product_id.sections": + try: + cfg.id_section_defs = parse_id_sections(value) + except ValueError as exc: + raise click.UsageError(str(exc)) from exc + save_config(cfg) + click.echo(f"{key} saved.") + return attr = mapping.get(key) if attr is None: raise click.UsageError(f"unknown key: {key}") diff --git a/src/secure_loader/config.py b/src/secure_loader/config.py index c2a8c48..ecf4ec8 100644 --- a/src/secure_loader/config.py +++ b/src/secure_loader/config.py @@ -20,20 +20,20 @@ from dataclasses import dataclass, field from pathlib import Path +import keyring from platformdirs import user_config_dir +from .core.id_sections import ( + DEFAULT_ID_SECTIONS, + IdSectionDef, + parse_id_sections, + serialize_id_sections, +) from .core.sources.http import DEFAULT_BASE_URL, DEFAULT_PATH_SEGMENTS, HttpCredentials log = logging.getLogger(__name__) -try: - import keyring as _keyring - - _KEYRING_SERVICE = "secureloader" - _KEYRING_AVAILABLE = True -except ImportError: # pragma: no cover - _keyring = None # type: ignore[assignment] - _KEYRING_AVAILABLE = False +_KEYRING_SERVICE = "secureloader" APP_DIR_NAME: str = "secureloader" APP_AUTHOR: str = "niwciu" @@ -62,6 +62,9 @@ class AppConfig: http_use_credentials: bool = False http_allow_insecure: bool = False http_path_segments: list[str] = field(default_factory=lambda: list(DEFAULT_PATH_SEGMENTS)) + id_section_defs: list[IdSectionDef] = field( + default_factory=lambda: list(DEFAULT_ID_SECTIONS) + ) language: str = "auto" # "en" | "de" | "fr" | "es" | "it" | "pl" | "auto" update_instruction_url: str = "" # empty = menu item hidden last_firmware_paths: list[str] = field(default_factory=list) @@ -89,6 +92,7 @@ def _load_config_locked(path: Path | None) -> AppConfig: http = parser["http"] if parser.has_section("http") else {} ui = parser["ui"] if parser.has_section("ui") else {} recent = parser["recent"] if parser.has_section("recent") else {} + product_id_sec = parser["product_id"] if parser.has_section("product_id") else {} _raw_segs = http.get("path_segments", "") path_segments = ( @@ -101,27 +105,48 @@ def _load_config_locked(path: Path | None) -> AppConfig: # Backward compat: if the key is absent, infer True when a login is already stored. http_use_credentials = _use_creds_raw.lower() == "true" if _use_creds_raw else bool(_login) + _raw_id_sections = product_id_sec.get("sections", "") + try: + id_section_defs = parse_id_sections(_raw_id_sections) + except ValueError: + log.warning("product_id.sections is malformed — using defaults") + id_section_defs = list(DEFAULT_ID_SECTIONS) + + http_password = _resolve_password(_login, http.get("password", "")) + cfg = AppConfig( http_base_url=http.get("base_url", DEFAULT_BASE_URL), http_login=_login, - http_password=http.get("password", ""), + http_password=http_password, http_use_credentials=http_use_credentials, http_allow_insecure=http.get("allow_insecure", "false").lower() == "true", http_path_segments=path_segments, + id_section_defs=id_section_defs, language=ui.get("language", "auto"), update_instruction_url=ui.get("instruction_url", ""), last_firmware_paths=[recent[key] for key in sorted(recent) if key.startswith("firmware_")], ) - if _KEYRING_AVAILABLE and cfg.http_login: - try: - stored = _keyring.get_password(_KEYRING_SERVICE, cfg.http_login) - if stored is not None: - cfg.http_password = stored - except Exception: - log.debug("keyring read failed — using password from config file") return cfg +def _resolve_password(login: str, ini_password: str) -> str: + """Return the password from the keychain, migrating a plaintext INI value if needed.""" + if not login: + return "" + try: + stored = keyring.get_password(_KEYRING_SERVICE, login) + if stored is not None: + return stored + if ini_password: + # One-time migration: move plaintext password from INI into the keychain. + keyring.set_password(_KEYRING_SERVICE, login, ini_password) + log.info("Migrated HTTP password from config file into the system keychain.") + return ini_password + except Exception: + log.warning("keyring read failed — password not loaded") + return "" + + def save_config(config: AppConfig, path: Path | None = None) -> None: with _config_lock: _save_config_locked(config, path) @@ -131,30 +156,28 @@ def _save_config_locked(config: AppConfig, path: Path | None) -> None: cfg_path = path or config_path() cfg_path.parent.mkdir(parents=True, exist_ok=True) - if _KEYRING_AVAILABLE and config.http_login: + if config.http_login and config.http_password: try: - _keyring.set_password(_KEYRING_SERVICE, config.http_login, config.http_password) - ini_password = "" + keyring.set_password(_KEYRING_SERVICE, config.http_login, config.http_password) except Exception: - log.warning("keyring write failed — storing HTTP password in plaintext") - ini_password = config.http_password - else: - if config.http_password: - log.warning( - "keyring not installed — storing HTTP password in plaintext. " - "Install the 'keyring' package for secure storage." + log.error( + "keyring write failed — password NOT saved to disk. " + "Ensure a keyring backend is available " + "(on Linux, install 'secretstorage' or 'keyrings.alt')." ) - ini_password = config.http_password parser = configparser.ConfigParser() parser["http"] = { "base_url": config.http_base_url, "login": config.http_login, - "password": ini_password, + "password": "", # never written to disk — stored in system keychain only "use_credentials": str(config.http_use_credentials).lower(), "allow_insecure": str(config.http_allow_insecure).lower(), "path_segments": ",".join(config.http_path_segments), } + parser["product_id"] = { + "sections": serialize_id_sections(config.id_section_defs), + } parser["ui"] = { "language": config.language, "instruction_url": config.update_instruction_url, diff --git a/src/secure_loader/core/firmware.py b/src/secure_loader/core/firmware.py index bc25318..3cdee9a 100644 --- a/src/secure_loader/core/firmware.py +++ b/src/secure_loader/core/firmware.py @@ -19,9 +19,11 @@ The 64-bit productId is reconstructed as ``(MSB << 32) | LSB``. -The wire header sent to the device during a firmware update is identical to -the full 48-byte file header. ``prevAppVersion`` is transmitted and the -bootloader struct now includes it (bootloader v1.2+). +The wire header sent to the device during ``CMD_START`` is **44 bytes** — +``prevAppVersion`` is read from the file but **not** forwarded to the +bootloader (compatible with bootloader v1.1+). The bootloader ``header_t`` +struct no longer contains ``prevAppVersion``; all fields after ``appVersion`` +shift down by 4 bytes compared to the file layout. """ from __future__ import annotations @@ -32,13 +34,15 @@ from dataclasses import dataclass from pathlib import Path +from .id_sections import IdSectionDef + log = logging.getLogger(__name__) HEADER_SIZE: int = 48 """Total size of the firmware header in bytes.""" -DEVICE_HEADER_SIZE: int = HEADER_SIZE -"""Size of the header sent to the device (identical to the file header, 48 bytes).""" +DEVICE_HEADER_SIZE: int = 44 +"""Size of the header sent to the device over UART (44 bytes — prevAppVersion stripped).""" IV_SIZE: int = 16 """Size of the initialization vector in bytes.""" @@ -114,6 +118,16 @@ def format_app_version(self) -> str: def format_prev_app_version(self) -> str: return f"0x{self.prev_app_version:08X}" + def get_sections(self, defs: list[IdSectionDef]) -> dict[str, str]: + """Extract section values from ``productId`` using ``defs``. + + Returns a ``{name: hex_value}`` dict where each value is the uppercase + hex substring of the 16-char product ID at the range given by the + corresponding :class:`~secure_loader.core.id_sections.IdSectionDef`. + """ + hex_id = f"{self.product_id:016X}" + return {d.name: d.extract(hex_id) for d in defs} + def parse_header(data: bytes | bytearray | memoryview) -> FirmwareHeader: """Parse a firmware header from the first :data:`HEADER_SIZE` bytes of ``data``. @@ -213,16 +227,17 @@ def load_firmware(path: str | Path) -> tuple[FirmwareHeader, bytes]: def build_device_header(raw: bytes | bytearray) -> bytes: - """Return the 48-byte wire header transmitted to the device during CMD_START. + """Return the 44-byte wire header transmitted to the device during CMD_START. - The wire header is identical to the file header — all fields including - ``prevAppVersion`` are transmitted (bootloader struct is 48 bytes). + ``prevAppVersion`` (file bytes [16:20]) is read by the host but **not** + forwarded. The returned buffer matches the bootloader v1.1+ ``header_t`` + layout exactly: file bytes [0:16] followed by file bytes [20:48]. """ if len(raw) < HEADER_SIZE: raise FirmwareFormatError( f"firmware too short: need at least {HEADER_SIZE} bytes, got {len(raw)}" ) - return bytes(raw[0:HEADER_SIZE]) + return bytes(raw[0:16]) + bytes(raw[20:HEADER_SIZE]) def split_pages(payload: bytes, page_size: int) -> list[bytes]: diff --git a/src/secure_loader/core/id_sections.py b/src/secure_loader/core/id_sections.py new file mode 100644 index 0000000..22f8a09 --- /dev/null +++ b/src/secure_loader/core/id_sections.py @@ -0,0 +1,99 @@ +"""Product-ID section definitions. + +A 64-bit ``productId`` is represented as a 16-character uppercase hex string. +:class:`IdSectionDef` names a contiguous slice of that string at nibble +(hex-digit) granularity, allowing users to mirror any server directory layout. + +Default layout (matches the original hard-coded convention):: + + hex position: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 + ╔══════════════════════╗ ╔══╗ ╔══╗ ╔════════╗ + ║ custom_id [0:8] ║ ║hw║ ║lic║ ║unique ║ + ╚══════════════════════╝ ╚══╝ ╚══╝ ╚════════╝ + +Config key: ``[product_id] sections = name:start:end, …`` (comma-separated). +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass + +log = logging.getLogger(__name__) + +_HEX_LEN: int = 16 + + +@dataclass(frozen=True, slots=True) +class IdSectionDef: + """One named slice of the 16-nibble product ID hex string. + + Positions are 0-based indices into the zero-padded 16-character uppercase + hex representation of the 64-bit ``productId``. Slicing follows Python + convention (left-inclusive, right-exclusive), so + ``IdSectionDef("lic", 10, 12)`` extracts nibbles at positions 10 and 11. + + Constraints: ``0 <= start < end <= 16``. + """ + + name: str + start: int + end: int + + def __post_init__(self) -> None: + if not self.name or not self.name.strip(): + raise ValueError("section name must not be empty") + if not (0 <= self.start < self.end <= _HEX_LEN): + raise ValueError( + f"invalid nibble range [{self.start}:{self.end}] for section {self.name!r}; " + f"must satisfy 0 <= start < end <= {_HEX_LEN}" + ) + + def extract(self, hex_id: str) -> str: + """Return the nibbles ``[start:end]`` from a 16-char uppercase hex string.""" + return hex_id[self.start : self.end] + + +DEFAULT_ID_SECTIONS: list[IdSectionDef] = [ + IdSectionDef("custom_id", 0, 8), + IdSectionDef("hw_id", 8, 10), + IdSectionDef("license_id", 10, 12), + IdSectionDef("unique_id", 12, 16), +] +"""Default four-section layout matching the original hard-coded convention.""" + + +def serialize_id_sections(defs: list[IdSectionDef]) -> str: + """Serialize section definitions to the INI config string format. + + Format: ``name:start:end,name:start:end,…`` + """ + return ",".join(f"{d.name}:{d.start}:{d.end}" for d in defs) + + +def parse_id_sections(s: str) -> list[IdSectionDef]: + """Parse a comma-separated list of ``name:start:end`` section definitions. + + Returns :data:`DEFAULT_ID_SECTIONS` when ``s`` is blank. Raises + :class:`ValueError` on malformed input; the caller should log and fall + back to defaults. + """ + s = s.strip() + if not s: + return list(DEFAULT_ID_SECTIONS) + + result: list[IdSectionDef] = [] + for part in s.split(","): + part = part.strip() + if not part: + continue + pieces = part.split(":") + if len(pieces) != 3: + raise ValueError(f"expected NAME:START:END, got {part!r}") + name, start_s, end_s = pieces + try: + result.append(IdSectionDef(name.strip(), int(start_s), int(end_s))) + except (ValueError, TypeError) as exc: + raise ValueError(f"invalid section definition {part!r}: {exc}") from exc + + return result if result else list(DEFAULT_ID_SECTIONS) diff --git a/src/secure_loader/core/protocol.py b/src/secure_loader/core/protocol.py index 03b7bd9..7414235 100644 --- a/src/secure_loader/core/protocol.py +++ b/src/secure_loader/core/protocol.py @@ -41,6 +41,7 @@ build_device_header, split_pages, ) +from .id_sections import IdSectionDef log = logging.getLogger(__name__) @@ -124,6 +125,16 @@ def format_bootloader_version(self) -> str: def format_product_id(self) -> str: return f"0x{self.product_id:016X}" + def get_sections(self, defs: list[IdSectionDef]) -> dict[str, str]: + """Extract section values from ``productId`` using ``defs``. + + Returns a ``{name: hex_value}`` dict where each value is the uppercase + hex substring of the 16-char product ID at the range given by the + corresponding :class:`~secure_loader.core.id_sections.IdSectionDef`. + """ + hex_id = f"{self.product_id:016X}" + return {d.name: d.extract(hex_id) for d in defs} + POLL_INTERVAL_S: float = 0.5 """Interval at which we re-send GetVersion while idle or connecting.""" diff --git a/src/secure_loader/core/sources/__init__.py b/src/secure_loader/core/sources/__init__.py index 1d91af7..d4ed42a 100644 --- a/src/secure_loader/core/sources/__init__.py +++ b/src/secure_loader/core/sources/__init__.py @@ -15,6 +15,7 @@ transport. """ +from ..id_sections import DEFAULT_ID_SECTIONS, IdSectionDef from .base import ( FirmwareIdentifier, FirmwareSource, @@ -25,10 +26,12 @@ from .local import LocalFirmwareSource __all__ = [ + "DEFAULT_ID_SECTIONS", "FirmwareIdentifier", "FirmwareSource", "FirmwareSourceError", "HttpFirmwareSource", + "IdSectionDef", "LocalFirmwareSource", "ProgressCallback", ] diff --git a/src/secure_loader/core/sources/base.py b/src/secure_loader/core/sources/base.py index 0e5044a..89c8b8d 100644 --- a/src/secure_loader/core/sources/base.py +++ b/src/secure_loader/core/sources/base.py @@ -4,7 +4,6 @@ from abc import ABC, abstractmethod from collections.abc import Callable -from dataclasses import dataclass ProgressCallback = Callable[[int, int], None] """Progress callback receiving ``(bytes_received, bytes_total)``.""" @@ -14,25 +13,86 @@ class FirmwareSourceError(RuntimeError): """Raised when a firmware source cannot provide the requested blob.""" -@dataclass(frozen=True, slots=True) class FirmwareIdentifier: """Keys identifying which firmware image to fetch. - Consumers derive these from the device's ``productId`` response or from - a parsed firmware header. Not every field is meaningful for every source; - providers document which attributes they require. + Section values are derived from the device's 64-bit ``productId`` by + applying the user-configured :class:`~secure_loader.core.id_sections.IdSectionDef` + list (see :meth:`~secure_loader.core.firmware.FirmwareHeader.get_sections` and + :meth:`~secure_loader.core.protocol.DeviceInfo.get_sections`). - The four ``*_id`` fields mirror the product ID byte convention: - ``custom_id`` (bytes 0-3), ``hw_id`` (byte 4), ``license_id`` (byte 5), - ``unique_id`` (bytes 6-7). Which fields are actually used to build a - download URL is determined by :attr:`HttpFirmwareSource.path_segments`. + Section values are accessed by name as regular attributes using + ``__getattr__``, e.g. ``identifier.license_id``. This keeps callers + decoupled from the storage representation and allows any configured section + name to work transparently with :class:`~secure_loader.core.sources.http.HttpFirmwareSource`. + + The ``app_version`` attribute is reserved for the rollback-fetch use case + (``prevAppVersion`` from the firmware header) and is not a product-ID section. + + Construction:: + + sections = device_info.get_sections(config.id_section_defs) + identifier = FirmwareIdentifier(sections, app_version="0.9.1") """ - license_id: str - unique_id: str - custom_id: str = "" - hw_id: str = "" - app_version: str | None = None + def __init__(self, sections: dict[str, str], app_version: str | None = None) -> None: + object.__setattr__(self, "_sections", dict(sections)) + object.__setattr__(self, "app_version", app_version) + + # -------------------------------------------------------------- attribute access + + def __getattr__(self, name: str) -> str: + """Return the value of section ``name``. + + Only called for names not found in ``__dict__`` (i.e. section names, + not ``_sections`` or ``app_version``). + """ + try: + return object.__getattribute__(self, "_sections")[name] + except KeyError: + available = list(object.__getattribute__(self, "_sections")) + raise AttributeError( + f"{type(self).__name__} has no section {name!r}. " + f"Available sections: {available}" + ) from None + + def __setattr__(self, name: str, value: object) -> None: + raise AttributeError("FirmwareIdentifier is immutable") + + # -------------------------------------------------------------- helpers + + def get(self, name: str, default: str = "") -> str: + """Return section value or ``default`` if the section is not present.""" + return object.__getattribute__(self, "_sections").get(name, default) + + @property + def section_names(self) -> list[str]: + """Names of all sections carried by this identifier.""" + return list(object.__getattribute__(self, "_sections")) + + # -------------------------------------------------------------- dunder protocol + + def __repr__(self) -> str: + sections = object.__getattribute__(self, "_sections") + app_version = object.__getattribute__(self, "app_version") + return ( + f"FirmwareIdentifier(sections={sections!r}, app_version={app_version!r})" + ) + + def __eq__(self, other: object) -> bool: + if not isinstance(other, FirmwareIdentifier): + return NotImplemented + return ( + object.__getattribute__(self, "_sections") + == object.__getattribute__(other, "_sections") + and object.__getattribute__(self, "app_version") + == object.__getattribute__(other, "app_version") + ) + + def __hash__(self) -> int: + sections = object.__getattribute__(self, "_sections") + app_version = object.__getattribute__(self, "app_version") + return hash((tuple(sorted(sections.items())), app_version)) class FirmwareSource(ABC): diff --git a/src/secure_loader/gui/main_window.py b/src/secure_loader/gui/main_window.py index 5a580f7..ac7ee99 100644 --- a/src/secure_loader/gui/main_window.py +++ b/src/secure_loader/gui/main_window.py @@ -739,15 +739,14 @@ def _start_fetch(self, *, previous: bool) -> None: return prev_version = self._firmware_header.format_prev_app_version() + sections = self._device_info.get_sections(self._config.id_section_defs) log.info( - "_start_fetch: base_url=%r allow_insecure=%s path_segments=%s identifier: " - "hw=%s lic=%s uniq=%s prev_version=%s", + "_start_fetch: base_url=%r allow_insecure=%s path_segments=%s " + "sections=%s prev_version=%s", self._config.http_base_url, self._config.http_allow_insecure, self._config.http_path_segments, - self._device_info.hw_id, - self._device_info.license_id, - self._device_info.unique_id, + sections, prev_version, ) try: @@ -758,13 +757,7 @@ def _start_fetch(self, *, previous: bool) -> None: path_segments=self._config.http_path_segments, ) log.debug("_start_fetch: HttpFirmwareSource created") - identifier = FirmwareIdentifier( - custom_id=self._device_info.custom_id, - hw_id=self._device_info.hw_id, - license_id=self._device_info.license_id, - unique_id=self._device_info.unique_id, - app_version=prev_version, - ) + identifier = FirmwareIdentifier(sections, app_version=prev_version) log.debug("_start_fetch: FirmwareIdentifier created") self.get_firmware_button.setEnabled(False) self.get_prev_firmware_button.setEnabled(False) diff --git a/src/secure_loader/gui/server_settings_dialog.py b/src/secure_loader/gui/server_settings_dialog.py index 90badff..e05ff82 100644 --- a/src/secure_loader/gui/server_settings_dialog.py +++ b/src/secure_loader/gui/server_settings_dialog.py @@ -1,53 +1,163 @@ -"""Dialog for configuring server URL, credentials, and URL path structure.""" +"""Dialog for configuring server URL, credentials, product-ID sections, and URL path.""" from __future__ import annotations -from PySide6.QtCore import Qt +import contextlib + +from PySide6.QtCore import Qt, QRect +from PySide6.QtGui import QBrush, QColor, QFont, QPainter, QPaintEvent, QPen from PySide6.QtWidgets import ( QCheckBox, QDialog, QDialogButtonBox, + QFrame, QGroupBox, QHBoxLayout, QLabel, QLineEdit, QListWidget, QListWidgetItem, + QMessageBox, QPushButton, + QScrollArea, + QSizePolicy, + QSpinBox, QVBoxLayout, QWidget, ) from ..config import AppConfig, save_config +from ..core.id_sections import IdSectionDef from ..i18n import _ -_SEGMENT_DEFS: list[tuple[str, str]] = [ - ("Custom (bytes 0-3)", "custom_id"), - ("HW ID (byte 4)", "hw_id"), - ("License ID (byte 5)", "license_id"), - ("Unique ID (bytes 6-7)", "unique_id"), -] -"""(display label, FirmwareIdentifier field name) for each product-ID slice.""" - -# Fixed order matches the 64-bit productId hex layout: chars [0:8][8:10][10:12][12:16] -_PID_VIZ: list[tuple[str, str, str, str]] = [ - ("AABBCCDD", "custom_id", "custom", "B 0-3"), - ("11", "hw_id", "hw_id", "B 4"), - ("22", "license_id", "license", "B 5"), - ("3344", "unique_id", "unique", "B 6-7"), +# Up to 8 distinct section colours (background, foreground, border). +_SECTION_COLORS: list[tuple[str, str, str]] = [ + ("#dbeafe", "#1d4ed8", "#93c5fd"), # blue + ("#dcfce7", "#15803d", "#86efac"), # green + ("#fef9c3", "#854d0e", "#fde047"), # yellow + ("#fce7f3", "#9d174d", "#f9a8d4"), # pink + ("#ede9fe", "#5b21b6", "#c4b5fd"), # violet + ("#ffedd5", "#9a3412", "#fdba74"), # orange + ("#cffafe", "#0e7490", "#67e8f9"), # cyan + ("#f0fdf4", "#166534", "#4ade80"), # light green ] -"""(hex placeholder, field_name, short label, byte description) for the PID strip.""" +_INACTIVE_COLORS = ("#f3f4f6", "#9ca3af", "#d1d5db") + + +class _PidVizWidget(QWidget): + """Paints the 16-nibble Product ID section map, expanding to fill container width.""" + + _HEIGHT = 92 + _HEX_H = 52 + _GAP = 3 + + def __init__(self, parent: QWidget | None = None) -> None: + super().__init__(parent) + self._defs: list[IdSectionDef] = [] + self.setFixedHeight(self._HEIGHT) + self.setSizePolicy(QSizePolicy.Policy.Expanding, QSizePolicy.Policy.Fixed) + + def set_defs(self, defs: list[IdSectionDef]) -> None: + self._defs = list(defs) + self.update() + + def paintEvent(self, event: QPaintEvent) -> None: # type: ignore[override] + painter = QPainter(self) + painter.setRenderHint(QPainter.RenderHint.Antialiasing) + + total_w = self.width() + hex_h = self._HEX_H + gap = self._GAP + lbl_top = hex_h + 4 + lbl_name_h = 17 + lbl_range_h = 15 + + n = 16 + cell_w = (total_w - gap * (n - 1)) / n + + nibble_section: dict[int, tuple[int, str]] = {} + for color_idx, sec in enumerate(self._defs): + ci = color_idx % len(_SECTION_COLORS) + for nib in range(sec.start, sec.end): + nibble_section[nib] = (ci, sec.name) + + i = 0 + while i < n: + if i in nibble_section: + ci, sec_name = nibble_section[i] + j = i + 1 + while j < n and nibble_section.get(j) == (ci, sec_name): + j += 1 + bg_str, fg_str, border_str = _SECTION_COLORS[ci] + else: + j = i + 1 + sec_name = "" + bg_str, fg_str, border_str = _INACTIVE_COLORS + + x = int(round(i * (cell_w + gap))) + span_w = int(round(j * (cell_w + gap) - gap)) - x + fg = QColor(fg_str) + + hex_rect = QRect(x, 0, span_w, hex_h) + painter.setBrush(QBrush(QColor(bg_str))) + painter.setPen(QPen(QColor(border_str), 1)) + painter.drawRoundedRect(hex_rect, 5, 5) + + hex_font = QFont("Monospace") + hex_font.setPixelSize(18) + painter.setFont(hex_font) + painter.setPen(QPen(fg)) + painter.drawText(hex_rect, Qt.AlignmentFlag.AlignCenter, "X" * (j - i)) + + if sec_name: + short_name = sec_name if len(sec_name) <= 10 else sec_name[:9] + "…" + name_font = QFont() + name_font.setPixelSize(13) + painter.setFont(name_font) + painter.setPen(QPen(fg)) + painter.drawText( + QRect(x, lbl_top, span_w, lbl_name_h), + Qt.AlignmentFlag.AlignHCenter | Qt.AlignmentFlag.AlignVCenter, + short_name, + ) + range_font = QFont("Monospace") + range_font.setPixelSize(11) + painter.setFont(range_font) + painter.setPen(QPen(QColor("#6b7280"))) + painter.drawText( + QRect(x, lbl_top + lbl_name_h, span_w, lbl_range_h), + Qt.AlignmentFlag.AlignHCenter | Qt.AlignmentFlag.AlignVCenter, + f"[{i}-{j - 1}]", + ) + else: + pos_font = QFont("Monospace") + pos_font.setPixelSize(11) + painter.setFont(pos_font) + painter.setPen(QPen(QColor("#6b7280"))) + painter.drawText( + QRect(x, lbl_top, span_w, lbl_name_h + lbl_range_h), + Qt.AlignmentFlag.AlignCenter, + str(i), + ) + + i = j + + painter.end() class ServerSettingsDialog(QDialog): - """Settings dialog for server URL, HTTP credentials, and URL path structure.""" + """Settings dialog for server URL, credentials, product-ID sections, and URL path.""" def __init__(self, config: AppConfig, parent: QWidget | None = None) -> None: super().__init__(parent) self._config = config + # Mutable working copies — committed on OK + self._section_rows: list[tuple[QLineEdit, QSpinBox, QSpinBox]] = [] + self._section_rows_layout: QVBoxLayout | None = None + self.setWindowTitle(_("Server settings")) self.setWindowFlags(self.windowFlags() & ~Qt.WindowType.WindowContextHelpButtonHint) - self.setMinimumWidth(520) + self.setMinimumWidth(580) self._build_ui() self._populate() root_layout = self.layout() @@ -56,40 +166,53 @@ def __init__(self, config: AppConfig, parent: QWidget | None = None) -> None: sh = root_layout.sizeHint() self.resize(max(self.minimumWidth(), sh.width()), sh.height()) - # ------------------------------------------------------------------ UI + # ------------------------------------------------------------------ UI build def _build_ui(self) -> None: root = QVBoxLayout(self) root.setSpacing(12) root.setContentsMargins(16, 14, 16, 12) - # ---- Server URL - url_box = QGroupBox(_("Server")) - url_layout = QVBoxLayout(url_box) + root.addWidget(self._build_server_group()) + root.addWidget(self._build_credentials_group()) + root.addWidget(self._build_id_sections_group()) + root.addWidget(self._build_url_path_group()) + + buttons = QDialogButtonBox( + QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel + ) + buttons.accepted.connect(self._save_and_accept) + buttons.rejected.connect(self.reject) + root.addWidget(buttons) + + def _build_server_group(self) -> QGroupBox: + box = QGroupBox(_("Server")) + layout = QVBoxLayout(box) url_row = QHBoxLayout() url_row.addWidget(QLabel(_("Base URL"))) self._url_edit = QLineEdit() self._url_edit.setPlaceholderText("https://example.com/firmware") - self._url_edit.textChanged.connect(self._update_preview) + self._url_edit.textChanged.connect(self._update_url_preview) url_row.addWidget(self._url_edit, stretch=1) - url_layout.addLayout(url_row) + layout.addLayout(url_row) self._allow_insecure_chk = QCheckBox( _("Allow plain HTTP (insecure — use only on trusted local networks)") ) - url_layout.addWidget(self._allow_insecure_chk) - root.addWidget(url_box) - - # ---- Credentials - cred_box = QGroupBox(_("Credentials")) - cred_box.setCheckable(True) - cred_box.setChecked(False) - self._cred_box = cred_box - cred_layout = QVBoxLayout(cred_box) + layout.addWidget(self._allow_insecure_chk) + return box + + def _build_credentials_group(self) -> QGroupBox: + box = QGroupBox(_("Credentials")) + box.setCheckable(True) + box.setChecked(False) + self._cred_box = box + layout = QVBoxLayout(box) + login_row = QHBoxLayout() login_row.addWidget(QLabel(_("Login"))) self._login_edit = QLineEdit() login_row.addWidget(self._login_edit, stretch=1) - cred_layout.addLayout(login_row) + layout.addLayout(login_row) pwd_row = QHBoxLayout() pwd_row.addWidget(QLabel(_("Password"))) @@ -100,51 +223,91 @@ def _build_ui(self) -> None: self._show_btn.setCheckable(True) self._show_btn.toggled.connect(self._toggle_password) pwd_row.addWidget(self._show_btn) - cred_layout.addLayout(pwd_row) - root.addWidget(cred_box) + layout.addLayout(pwd_row) + return box - # ---- URL path structure - path_box = QGroupBox(_("URL path structure")) - path_layout = QVBoxLayout(path_box) + def _build_id_sections_group(self) -> QGroupBox: + box = QGroupBox(_("Product ID sections")) + layout = QVBoxLayout(box) - hint = QLabel(_("Select and order the product-ID fields used in the download URL path.")) + hint = QLabel( + _( + "Define how the 64-bit Product ID is split into named sections. " + "Start and End are nibble positions (0-15) within the 16-character hex ID." + ) + ) hint.setWordWrap(True) - path_layout.addWidget(hint) - - # Product ID hex strip visualisation - self._pid_viz_lbl = QLabel() - self._pid_viz_lbl.setTextFormat(Qt.TextFormat.RichText) - path_layout.addWidget(self._pid_viz_lbl) + layout.addWidget(hint) + + # Column headers + hdr = QHBoxLayout() + hdr.addWidget(QLabel(_("Section name")), stretch=3) + hdr.addWidget(QLabel(_("Start")), stretch=1) + hdr.addWidget(QLabel(_("End")), stretch=1) + hdr.addWidget(QLabel(""), stretch=0) # delete-button column + layout.addLayout(hdr) + + sep = QFrame() + sep.setFrameShape(QFrame.Shape.HLine) + sep.setFrameShadow(QFrame.Shadow.Sunken) + layout.addWidget(sep) + + # Scrollable rows area + scroll = QScrollArea() + scroll.setWidgetResizable(True) + scroll.setFrameShape(QFrame.Shape.NoFrame) + scroll.setMaximumHeight(180) + rows_widget = QWidget() + self._section_rows_layout = QVBoxLayout(rows_widget) + self._section_rows_layout.setContentsMargins(0, 0, 0, 0) + self._section_rows_layout.setSpacing(4) + self._section_rows_layout.addStretch() + scroll.setWidget(rows_widget) + layout.addWidget(scroll) + + add_btn = QPushButton(_("+ Add section")) + add_btn.clicked.connect(lambda: self._add_section_row()) + layout.addWidget(add_btn, alignment=Qt.AlignmentFlag.AlignLeft) + + # PID visualization + self._pid_viz = _PidVizWidget() + layout.addWidget(self._pid_viz) + return box + + def _build_url_path_group(self) -> QGroupBox: + box = QGroupBox(_("URL path structure")) + layout = QVBoxLayout(box) + + hint = QLabel( + _( + "Select which sections appear in the download URL path and set their order. " + "Unchecked sections are available on the device but not used in the URL." + ) + ) + hint.setWordWrap(True) + layout.addWidget(hint) list_row = QHBoxLayout() self._seg_list = QListWidget() self._seg_list.currentRowChanged.connect(self._update_move_buttons) - self._seg_list.itemChanged.connect(self._refresh_display) + self._seg_list.itemChanged.connect(self._update_url_preview) list_row.addWidget(self._seg_list, stretch=1) btn_col = QVBoxLayout() btn_col.setAlignment(Qt.AlignmentFlag.AlignTop) self._up_btn = QPushButton(_("↑ Up")) - self._up_btn.clicked.connect(self._move_up) + self._up_btn.clicked.connect(self._move_seg_up) self._down_btn = QPushButton(_("↓ Down")) - self._down_btn.clicked.connect(self._move_down) + self._down_btn.clicked.connect(self._move_seg_down) btn_col.addWidget(self._up_btn) btn_col.addWidget(self._down_btn) list_row.addLayout(btn_col) - path_layout.addLayout(list_row) + layout.addLayout(list_row) self._preview_lbl = QLabel() self._preview_lbl.setWordWrap(True) - path_layout.addWidget(self._preview_lbl) - root.addWidget(path_box) - - # ---- OK / Cancel - buttons = QDialogButtonBox( - QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel - ) - buttons.accepted.connect(self._save_and_accept) - buttons.rejected.connect(self.reject) - root.addWidget(buttons) + layout.addWidget(self._preview_lbl) + return box # ---------------------------------------------------------------- populate @@ -154,50 +317,212 @@ def _populate(self) -> None: self._cred_box.setChecked(self._config.http_use_credentials) self._login_edit.setText(self._config.http_login) self._pwd_edit.setText(self._config.http_password) - self._populate_segments(self._config.http_path_segments) - self._refresh_display() + + for sec_def in self._config.id_section_defs: + self._add_section_row(sec_def) + + self._refresh_seg_list(self._config.http_path_segments) + self._update_pid_viz() + self._update_url_preview() self._update_move_buttons() - def _populate_segments(self, active: list[str]) -> None: - """Fill the list widget: active segments first (in config order), rest at end.""" + # ---------------------------------------------------------- section rows + + def _add_section_row(self, sec_def: IdSectionDef | None = None) -> None: + assert self._section_rows_layout is not None + + row_widget = QWidget() + row_layout = QHBoxLayout(row_widget) + row_layout.setContentsMargins(0, 0, 0, 0) + + name_edit = QLineEdit() + name_edit.setPlaceholderText(_("section_name")) + start_spin = QSpinBox() + start_spin.setRange(0, 15) + end_spin = QSpinBox() + end_spin.setRange(0, 15) + + if sec_def is not None: + name_edit.setText(sec_def.name) + start_spin.setValue(sec_def.start) + end_spin.setValue(sec_def.end - 1) + else: + # Place the new section at the first nibble not covered by any existing valid row. + covered: set[int] = set() + for (_ne, other_s, other_e) in self._section_rows: + if other_s.value() <= other_e.value(): + covered.update(range(other_s.value(), other_e.value() + 1)) + new_start = next((i for i in range(16) if i not in covered), 15) + start_spin.setValue(new_start) + end_spin.setValue(new_start) + + del_btn = QPushButton("✕") + del_btn.setFixedWidth(28) + del_btn.setToolTip(_("Delete this section")) + + row_layout.addWidget(name_edit, stretch=3) + row_layout.addWidget(start_spin, stretch=1) + row_layout.addWidget(end_spin, stretch=1) + row_layout.addWidget(del_btn, stretch=0) + + # Insert before the trailing stretch (last item) + insert_at = self._section_rows_layout.count() - 1 + self._section_rows_layout.insertWidget(insert_at, row_widget) + + self._section_rows.append((name_edit, start_spin, end_spin)) + + # Wire signals + name_edit.textChanged.connect(self._on_section_changed) + start_spin.valueChanged.connect(self._on_section_changed) + end_spin.valueChanged.connect(self._on_section_changed) + del_btn.clicked.connect(lambda: self._delete_section_row(row_widget, name_edit, start_spin, end_spin)) + self._update_spinbox_constraints() + + def _delete_section_row( + self, + row_widget: QWidget, + name_edit: QLineEdit, + start_spin: QSpinBox, + end_spin: QSpinBox, + ) -> None: + assert self._section_rows_layout is not None + self._section_rows_layout.removeWidget(row_widget) + row_widget.setParent(None) # type: ignore[call-overload] + self._section_rows = [ + r for r in self._section_rows if r[0] is not name_edit + ] + self._on_section_changed() + + def _on_section_changed(self) -> None: + """Called whenever any section definition widget changes.""" + self._update_spinbox_constraints() + self._update_pid_viz() + self._sync_seg_list_to_sections() + self._update_url_preview() + + def _update_spinbox_constraints(self) -> None: + """Constrain spinboxes by nibble position, not by row order. + + Only currently-valid rows (start <= end) act as constraints for their + neighbours. This allows a section to freely move through another + section's range during a multi-step repositioning without clamping + unrelated rows. + """ + rows = self._section_rows + if not rows: + return + + for _, s, e in rows: + s.blockSignals(True) + e.blockSignals(True) + + for _, start_spin, end_spin in rows: + s_val = start_spin.value() + # Consider only other rows that are currently in a valid state. + others = [ + (other_s.value(), other_e.value()) + for (_, other_s, other_e) in rows + if other_s is not start_spin and other_s.value() <= other_e.value() + ] + # start_min: right after the nearest valid section that ends before us. + # If we're already inside another section (overlap), release the floor to 0 + # so the user can freely escape to a free nibble. + inside_other = any(s <= s_val <= e for (s, e) in others) + if inside_other: + start_min = 0 + else: + left_ends = [e for (_, e) in others if e < s_val] + start_min = (max(left_ends) + 1) if left_ends else 0 + # end_max: just before the nearest valid section that starts after us. + next_starts = [s for (s, _) in others if s > s_val] + end_max = (min(next_starts) - 1) if next_starts else 15 + + start_spin.setMinimum(start_min) + end_spin.setMaximum(max(0, end_max)) + + for _, s, e in rows: + s.blockSignals(False) + e.blockSignals(False) + + def _current_section_defs(self) -> list[IdSectionDef]: + """Parse the current widget state into IdSectionDef objects (skipping invalid rows).""" + result = [] + for name_edit, start_spin, end_spin in self._section_rows: + name = name_edit.text().strip() + start = start_spin.value() + end = end_spin.value() + 1 + if name and 0 <= start < end <= 16: + with contextlib.suppress(ValueError): + result.append(IdSectionDef(name=name, start=start, end=end)) + return result + + # -------------------------------------------------------- seg list (URL path) + + def _refresh_seg_list(self, active_segments: list[str]) -> None: + """Populate path-segment list from config, adding any missing section names.""" self._seg_list.blockSignals(True) self._seg_list.clear() + current_names = {d.name for d in self._current_section_defs()} shown: set[str] = set() - for field_name in active: - label = next((lbl for lbl, fn in _SEGMENT_DEFS if fn == field_name), field_name) - self._add_segment_item(label, field_name, checked=True) - shown.add(field_name) - for label, field_name in _SEGMENT_DEFS: - if field_name not in shown: - self._add_segment_item(label, field_name, checked=False) + for seg_name in active_segments: + self._add_seg_item(seg_name, checked=True) + shown.add(seg_name) + for name in current_names: + if name not in shown: + self._add_seg_item(name, checked=False) self._seg_list.blockSignals(False) - def _add_segment_item(self, label: str, field_name: str, *, checked: bool) -> None: - item = QListWidgetItem(label) - item.setData(Qt.ItemDataRole.UserRole, field_name) + def _add_seg_item(self, name: str, *, checked: bool) -> None: + item = QListWidgetItem(name) + item.setData(Qt.ItemDataRole.UserRole, name) item.setFlags(item.flags() | Qt.ItemFlag.ItemIsUserCheckable) item.setCheckState(Qt.CheckState.Checked if checked else Qt.CheckState.Unchecked) self._seg_list.addItem(item) - # --------------------------------------------------------- segment controls + def _sync_seg_list_to_sections(self) -> None: + """Add/remove items in the path-segment list to match current section definitions.""" + current_names = {d.name for d in self._current_section_defs()} + existing_names: dict[str, Qt.CheckState] = {} + for i in range(self._seg_list.count()): + item = self._seg_list.item(i) + if item: + existing_names[item.data(Qt.ItemDataRole.UserRole)] = item.checkState() - def _move_up(self) -> None: + self._seg_list.blockSignals(True) + # Remove items whose section was deleted + for i in range(self._seg_list.count() - 1, -1, -1): + item = self._seg_list.item(i) + if item and item.data(Qt.ItemDataRole.UserRole) not in current_names: + self._seg_list.takeItem(i) + # Add items for new sections + for name in current_names: + if name not in existing_names: + self._add_seg_item(name, checked=False) + # Update display labels (in case name was edited in-place) + for i in range(self._seg_list.count()): + item = self._seg_list.item(i) + if item: + seg_name = item.data(Qt.ItemDataRole.UserRole) + item.setText(seg_name) + self._seg_list.blockSignals(False) + + def _move_seg_up(self) -> None: row = self._seg_list.currentRow() if row <= 0: return item = self._seg_list.takeItem(row) self._seg_list.insertItem(row - 1, item) self._seg_list.setCurrentRow(row - 1) - self._refresh_display() + self._update_url_preview() - def _move_down(self) -> None: + def _move_seg_down(self) -> None: row = self._seg_list.currentRow() if row < 0 or row >= self._seg_list.count() - 1: return item = self._seg_list.takeItem(row) self._seg_list.insertItem(row + 1, item) self._seg_list.setCurrentRow(row + 1) - self._refresh_display() + self._update_url_preview() def _update_move_buttons(self) -> None: row = self._seg_list.currentRow() @@ -205,7 +530,7 @@ def _update_move_buttons(self) -> None: self._up_btn.setEnabled(row > 0) self._down_btn.setEnabled(0 <= row < count - 1) - def _active_segments(self) -> list[str]: + def _active_path_segments(self) -> list[str]: result = [] for i in range(self._seg_list.count()): item = self._seg_list.item(i) @@ -213,53 +538,16 @@ def _active_segments(self) -> list[str]: result.append(item.data(Qt.ItemDataRole.UserRole)) return result - # --------------------------------------------------------- display refresh - - def _refresh_display(self) -> None: - """Refresh both the PID visualisation and the URL preview.""" - self._update_pid_viz() - self._update_preview() + # ------------------------------------------------------- PID visualization def _update_pid_viz(self) -> None: - """Render the 64-bit Product ID strip with active fields highlighted in blue.""" - active = set(self._active_segments()) + self._pid_viz.set_defs(self._current_section_defs()) - hex_cells = ( - '0x ' - ) - lbl_cells = "" - - for hex_val, field, short, byte_desc in _PID_VIZ: - if field in active: - bg, fg, border = "#dbeafe", "#1d4ed8", "#93c5fd" - else: - bg, fg, border = "#f3f4f6", "#9ca3af", "#d1d5db" + # ---------------------------------------------------------- URL preview - hex_cells += ( - f'{hex_val}' - f'' - ) - lbl_cells += ( - f'{short}' - f'
{byte_desc}' - f"" - ) - - html = ( - f'' - f"{hex_cells}" - f'{lbl_cells}' - f"
" - ) - self._pid_viz_lbl.setText(html) - - def _update_preview(self) -> None: + def _update_url_preview(self) -> None: base = self._url_edit.text().rstrip("/") or "{base_url}" - segs = self._active_segments() + segs = self._active_path_segments() if segs: path = "/".join(f"{{{s}}}" for s in segs) preview = f"{base}/{path}/{{version}}.bin" @@ -267,21 +555,41 @@ def _update_preview(self) -> None: preview = f"{base}/{{version}}.bin" self._preview_lbl.setText(_("Preview: ") + preview) - # ---------------------------------------------------------------- password + # ------------------------------------------------------------ password def _toggle_password(self, show: bool) -> None: mode = QLineEdit.EchoMode.Normal if show else QLineEdit.EchoMode.Password self._pwd_edit.setEchoMode(mode) self._show_btn.setText(_("Hide") if show else _("Show")) - # ------------------------------------------------------------------- save + # -------------------------------------------------------------- save def _save_and_accept(self) -> None: + defs = self._current_section_defs() + if not defs: + QMessageBox.warning( + self, + _("Invalid configuration"), + _("At least one valid Product ID section must be defined."), + ) + return + + # Check for duplicate names + names = [d.name for d in defs] + if len(names) != len(set(names)): + QMessageBox.warning( + self, + _("Invalid configuration"), + _("Section names must be unique."), + ) + return + self._config.http_base_url = self._url_edit.text().strip() self._config.http_allow_insecure = self._allow_insecure_chk.isChecked() self._config.http_use_credentials = self._cred_box.isChecked() self._config.http_login = self._login_edit.text().strip() self._config.http_password = self._pwd_edit.text() - self._config.http_path_segments = self._active_segments() + self._config.id_section_defs = defs + self._config.http_path_segments = self._active_path_segments() save_config(self._config) self.accept() diff --git a/tests/test_cli.py b/tests/test_cli.py index e311577..a77adeb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -122,7 +122,12 @@ def test_http_url_rejected_without_allow_insecure(self, runner: CliRunner) -> No with patch("secure_loader.cli.main.load_config", return_value=cfg): result = runner.invoke( cli, - ["fetch", "--license", "CC", "--unique", "3344", "--output", "/dev/null"], + [ + "fetch", + "--section", "license_id=CC", + "--section", "unique_id=3344", + "--output", "/dev/null", + ], catch_exceptions=True, ) assert result.exit_code != 0 @@ -143,12 +148,9 @@ def test_http_url_allowed_with_allow_insecure_flag(self, runner: CliRunner) -> N cli, [ "fetch", - "--license", - "CC", - "--unique", - "3344", - "--output", - "/dev/null", + "--section", "license_id=CC", + "--section", "unique_id=3344", + "--output", "/dev/null", "--allow-insecure", ], catch_exceptions=True, @@ -167,7 +169,12 @@ def test_https_url_no_cleartext_warning(self, runner: CliRunner) -> None: mock_src_cls.return_value = mock_src result = runner.invoke( cli, - ["fetch", "--license", "CC", "--unique", "3344", "--output", "/dev/null"], + [ + "fetch", + "--section", "license_id=CC", + "--section", "unique_id=3344", + "--output", "/dev/null", + ], catch_exceptions=True, ) combined = (result.output or "") + (result.stderr if hasattr(result, "stderr") else "") diff --git a/tests/test_config.py b/tests/test_config.py index b0239b8..9403ba9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -10,6 +10,7 @@ import pytest from secure_loader.config import AppConfig, load_config, save_config +from secure_loader.core.id_sections import DEFAULT_ID_SECTIONS, IdSectionDef @pytest.fixture @@ -77,6 +78,27 @@ def test_partial_config_fills_defaults(self, tmp_cfg: Path) -> None: assert cfg.http_login == "" assert cfg.language == "auto" + def test_id_section_defs_round_trip(self, tmp_cfg: Path) -> None: + custom_defs = [ + IdSectionDef(name="product", start=0, end=6), + IdSectionDef(name="hw", start=6, end=8), + IdSectionDef(name="serial", start=8, end=16), + ] + cfg = AppConfig(id_section_defs=custom_defs) + save_config(cfg, tmp_cfg) + loaded = load_config(tmp_cfg) + assert loaded.id_section_defs == custom_defs + + def test_id_section_defs_default_when_absent(self, tmp_cfg: Path) -> None: + tmp_cfg.write_text("[http]\nbase_url = https://example.com\n", encoding="utf-8") + cfg = load_config(tmp_cfg) + assert cfg.id_section_defs == list(DEFAULT_ID_SECTIONS) + + def test_id_section_defs_malformed_falls_back_to_defaults(self, tmp_cfg: Path) -> None: + tmp_cfg.write_text("[product_id]\nsections = INVALID:X:Y\n", encoding="utf-8") + cfg = load_config(tmp_cfg) + assert cfg.id_section_defs == list(DEFAULT_ID_SECTIONS) + class TestCredentials: def test_credentials_none_when_both_empty(self) -> None: @@ -140,8 +162,7 @@ def test_password_stored_in_keyring_not_ini( ) -> None: mock_kr = MagicMock() mock_kr.get_password.return_value = None - monkeypatch.setattr("secure_loader.config._KEYRING_AVAILABLE", True) - monkeypatch.setattr("secure_loader.config._keyring", mock_kr) + monkeypatch.setattr("secure_loader.config.keyring", mock_kr) cfg = AppConfig(http_login="user", http_password="secret") save_config(cfg, tmp_cfg) @@ -155,8 +176,7 @@ def test_password_loaded_from_keyring( ) -> None: mock_kr = MagicMock() mock_kr.get_password.return_value = "from_keyring" - monkeypatch.setattr("secure_loader.config._KEYRING_AVAILABLE", True) - monkeypatch.setattr("secure_loader.config._keyring", mock_kr) + monkeypatch.setattr("secure_loader.config.keyring", mock_kr) cfg = AppConfig(http_login="user", http_password="") save_config(cfg, tmp_cfg) @@ -164,6 +184,22 @@ def test_password_loaded_from_keyring( assert loaded.http_password == "from_keyring" + def test_plaintext_password_migrated_to_keyring_on_load( + self, tmp_cfg: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + mock_kr = MagicMock() + mock_kr.get_password.return_value = None # nothing in keyring yet + monkeypatch.setattr("secure_loader.config.keyring", mock_kr) + + # Write a legacy config that still has the password in plaintext. + tmp_cfg.write_text( + "[http]\nlogin = user\npassword = legacy_secret\n", encoding="utf-8" + ) + loaded = load_config(tmp_cfg) + + assert loaded.http_password == "legacy_secret" + mock_kr.set_password.assert_called_once_with("secureloader", "user", "legacy_secret") + class TestFilePermissions: @pytest.mark.skipif(os.name == "nt", reason="chmod not meaningful on Windows") diff --git a/tests/test_firmware.py b/tests/test_firmware.py index 2e5bbf1..e4c7cd3 100644 --- a/tests/test_firmware.py +++ b/tests/test_firmware.py @@ -68,19 +68,26 @@ def test_accepts_extra_trailing_bytes(self, sample_header_bytes: bytes) -> None: class TestBuildDeviceHeader: - def test_wire_header_is_full_48_byte_file_header(self, sample_firmware: bytes) -> None: + def test_wire_header_is_44_bytes(self, sample_firmware: bytes) -> None: wire = build_device_header(sample_firmware) - assert len(wire) == 48 + assert len(wire) == 44 assert len(wire) == DEVICE_HEADER_SIZE - # Wire header is identical to the first HEADER_SIZE bytes of the file. - assert wire == sample_firmware[:HEADER_SIZE] - def test_prev_app_version_present_at_offset_16(self, sample_firmware: bytes) -> None: + def test_wire_header_matches_file_bytes_0_to_16_and_20_to_48( + self, sample_firmware: bytes + ) -> None: wire = build_device_header(sample_firmware) - # prevAppVersion is at bytes [16:20] in both the file and the wire header. - assert wire[16:20] == sample_firmware[16:20] - # Confirm the value is non-zero (matches the fixture's 0x01020300). - assert wire[16:20] != b"\x00\x00\x00\x00" + # Bytes [0:16]: protocolVersion + productId MSB+LSB + appVersion + assert wire[0:16] == sample_firmware[0:16] + # Bytes [16:44] of the wire map to file bytes [20:48]: pageCount onwards + assert wire[16:44] == sample_firmware[20:48] + + def test_prev_app_version_absent_from_wire_header(self, sample_firmware: bytes) -> None: + wire = build_device_header(sample_firmware) + # prevAppVersion occupies file bytes [16:20] — must NOT appear in wire header. + # At wire offset 16 we expect pageCount (file[20:24]), not prevAppVersion (file[16:20]). + assert wire[16:20] == sample_firmware[20:24] # pageCount, not prevAppVersion + assert wire[16:20] != sample_firmware[16:20] # differs from prevAppVersion bytes def test_rejects_short_blobs(self) -> None: with pytest.raises(FirmwareFormatError): diff --git a/tests/test_github_source.py b/tests/test_github_source.py index de921e5..99a0b0f 100644 --- a/tests/test_github_source.py +++ b/tests/test_github_source.py @@ -23,7 +23,7 @@ def source(config: GithubConfig) -> GithubReleasesFirmwareSource: @pytest.fixture def identifier() -> FirmwareIdentifier: - return FirmwareIdentifier(license_id="CC", unique_id="3344") + return FirmwareIdentifier({"license_id": "CC", "unique_id": "3344"}) def _json_response(data: object, status: int = 200) -> MagicMock: @@ -122,7 +122,7 @@ class TestFetchPrevious: def test_fetches_by_tag( self, source: GithubReleasesFirmwareSource, identifier: FirmwareIdentifier ) -> None: - ident = FirmwareIdentifier(license_id="CC", unique_id="3344", app_version="1.0.0") + ident = FirmwareIdentifier({"license_id": "CC", "unique_id": "3344"}, app_version="1.0.0") payload = b"\xde\xad" asset = {"name": "cc_3344.bin", "url": "https://api.github.com/assets/5", "size": 2} release_resp = _json_response({"assets": [asset]}) diff --git a/tests/test_gui.py b/tests/test_gui.py index a665fa6..67d0d4f 100644 --- a/tests/test_gui.py +++ b/tests/test_gui.py @@ -474,43 +474,39 @@ def test_default_segments_license_and_unique_checked(self, server_settings_dialo assert "license_id" in checked assert "unique_id" in checked - def test_active_segments_returns_checked_items(self, server_settings_dialog) -> None: - segs = server_settings_dialog._active_segments() + def test_active_path_segments_returns_checked_items(self, server_settings_dialog) -> None: + segs = server_settings_dialog._active_path_segments() assert isinstance(segs, list) assert len(segs) >= 1 - def test_pid_viz_label_exists(self, server_settings_dialog) -> None: - assert server_settings_dialog._pid_viz_lbl is not None + def test_pid_viz_widget_exists(self, server_settings_dialog) -> None: + assert server_settings_dialog._pid_viz is not None - def test_pid_viz_contains_all_hex_placeholders(self, server_settings_dialog) -> None: - html = server_settings_dialog._pid_viz_lbl.text() - assert "AABBCCDD" in html - assert "3344" in html + def test_pid_viz_contains_section_nibble_ranges(self, server_settings_dialog) -> None: + defs = server_settings_dialog._pid_viz._defs + ranges = [(d.start, d.end - 1) for d in defs] # inclusive end for display + assert (0, 7) in ranges # custom_id + assert (8, 9) in ranges # hw_id + assert (10, 11) in ranges # license_id + assert (12, 15) in ranges # unique_id - def test_pid_viz_active_segments_shown_in_blue(self, server_settings_dialog) -> None: - # Default active: license_id and unique_id → blue color - html = server_settings_dialog._pid_viz_lbl.text() - assert "#1d4ed8" in html # active fg colour + def test_pid_viz_active_sections_shown_in_color(self, server_settings_dialog) -> None: + # Default sections are all defined → at least one section is loaded into viz + assert len(server_settings_dialog._pid_viz._defs) > 0 - def test_pid_viz_inactive_segments_shown_in_grey(self, server_settings_dialog) -> None: - html = server_settings_dialog._pid_viz_lbl.text() - assert "#9ca3af" in html # inactive fg colour + def test_pid_viz_inactive_nibbles_shown_in_grey(self, server_settings_dialog) -> None: + from secure_loader.gui.server_settings_dialog import _INACTIVE_COLORS + assert _INACTIVE_COLORS[1] == "#9ca3af" - def test_pid_viz_shows_byte_descriptions(self, server_settings_dialog) -> None: - html = server_settings_dialog._pid_viz_lbl.text() - assert "B 0" in html # custom_id: B 0-3 - assert "B 4" in html # hw_id - assert "B 5" in html # license_id - assert "B 6" in html # unique_id: B 6-7 + def test_pid_viz_shows_section_names(self, server_settings_dialog) -> None: + names = [d.name for d in server_settings_dialog._pid_viz._defs] + assert "hw_id" in names + assert "custom_id" in names - def test_pid_viz_updates_when_checkbox_toggled(self, server_settings_dialog) -> None: - from PySide6.QtCore import Qt - - # Uncheck all segments and verify no active colour remains - for i in range(server_settings_dialog._seg_list.count()): - server_settings_dialog._seg_list.item(i).setCheckState(Qt.CheckState.Unchecked) - html = server_settings_dialog._pid_viz_lbl.text() - assert "#1d4ed8" not in html # no active segment → no blue + def test_pid_viz_updates_when_section_row_deleted(self, server_settings_dialog) -> None: + server_settings_dialog._section_rows.clear() + server_settings_dialog._on_section_changed() + assert server_settings_dialog._pid_viz._defs == [] def test_preview_label_contains_preview(self, server_settings_dialog) -> None: text = server_settings_dialog._preview_lbl.text() @@ -524,7 +520,7 @@ def test_preview_updates_when_url_changes(self, server_settings_dialog) -> None: def test_preview_updates_when_checkbox_toggled(self, server_settings_dialog) -> None: from PySide6.QtCore import Qt - # Uncheck all → preview should not contain any field placeholder + # Uncheck all path segments → preview should not contain any field placeholder for i in range(server_settings_dialog._seg_list.count()): server_settings_dialog._seg_list.item(i).setCheckState(Qt.CheckState.Unchecked) text = server_settings_dialog._preview_lbl.text() @@ -539,7 +535,7 @@ def test_preview_with_no_segments(self, qapp) -> None: cfg.http_path_segments = [] dlg = ServerSettingsDialog(config=cfg) dlg._url_edit.setText("https://example.com") - dlg._update_preview() + dlg._update_url_preview() text = dlg._preview_lbl.text() assert "{version}.bin" in text assert "{license_id}" not in text @@ -561,7 +557,7 @@ def test_move_up_shifts_item(self, server_settings_dialog) -> None: server_settings_dialog._seg_list.setCurrentRow(1) before = server_settings_dialog._seg_list.item(1).data(Qt.ItemDataRole.UserRole) - server_settings_dialog._move_up() + server_settings_dialog._move_seg_up() after = server_settings_dialog._seg_list.item(0).data(Qt.ItemDataRole.UserRole) assert before == after @@ -570,7 +566,7 @@ def test_move_down_shifts_item(self, server_settings_dialog) -> None: server_settings_dialog._seg_list.setCurrentRow(0) before = server_settings_dialog._seg_list.item(0).data(Qt.ItemDataRole.UserRole) - server_settings_dialog._move_down() + server_settings_dialog._move_seg_down() after = server_settings_dialog._seg_list.item(1).data(Qt.ItemDataRole.UserRole) assert before == after @@ -579,7 +575,7 @@ def test_move_up_at_top_is_noop(self, server_settings_dialog) -> None: server_settings_dialog._seg_list.setCurrentRow(0) before = server_settings_dialog._seg_list.item(0).data(Qt.ItemDataRole.UserRole) - server_settings_dialog._move_up() + server_settings_dialog._move_seg_up() after = server_settings_dialog._seg_list.item(0).data(Qt.ItemDataRole.UserRole) assert before == after @@ -589,7 +585,7 @@ def test_move_down_at_bottom_is_noop(self, server_settings_dialog) -> None: last = server_settings_dialog._seg_list.count() - 1 server_settings_dialog._seg_list.setCurrentRow(last) before = server_settings_dialog._seg_list.item(last).data(Qt.ItemDataRole.UserRole) - server_settings_dialog._move_down() + server_settings_dialog._move_seg_down() after = server_settings_dialog._seg_list.item(last).data(Qt.ItemDataRole.UserRole) assert before == after @@ -632,7 +628,7 @@ def test_custom_segments_reflected_in_active_list(self, qapp) -> None: cfg = AppConfig() cfg.http_path_segments = ["hw_id", "license_id"] dlg = ServerSettingsDialog(config=cfg) - segs = dlg._active_segments() + segs = dlg._active_path_segments() assert segs == ["hw_id", "license_id"] dlg.close() diff --git a/tests/test_http_source.py b/tests/test_http_source.py index 37018c2..0b1b1a2 100644 --- a/tests/test_http_source.py +++ b/tests/test_http_source.py @@ -28,7 +28,7 @@ def _make_response(text: str | None = None, content: bytes = b"", status: int = @pytest.fixture def identifier() -> FirmwareIdentifier: - return FirmwareIdentifier(license_id="CC", unique_id="3344") + return FirmwareIdentifier({"license_id": "CC", "unique_id": "3344"}) @pytest.fixture @@ -160,7 +160,7 @@ def test_progress_callback_called( assert calls[-1][0] == len(payload) def test_url_encodes_special_characters(self, source: HttpFirmwareSource) -> None: - ident = FirmwareIdentifier(license_id="A B", unique_id="C/D") + ident = FirmwareIdentifier({"license_id": "A B", "unique_id": "C/D"}) source._session = MagicMock() source._session.get.side_effect = requests.ConnectionError() with pytest.raises(FirmwareSourceError): @@ -175,7 +175,7 @@ def test_url_encodes_special_characters(self, source: HttpFirmwareSource) -> Non class TestFetchPrevious: def test_fetches_named_version(self, source: HttpFirmwareSource) -> None: payload = b"\x01\x02\x03" - ident = FirmwareIdentifier(license_id="AA", unique_id="BBBB", app_version="0.9.1") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "BBBB"}, app_version="0.9.1") bin_resp = _make_response(content=payload) source._session = MagicMock() source._session.get.return_value = bin_resp @@ -186,7 +186,7 @@ def test_fetches_named_version(self, source: HttpFirmwareSource) -> None: assert "0.9.1.bin" in url def test_raises_when_app_version_missing(self, source: HttpFirmwareSource) -> None: - ident = FirmwareIdentifier(license_id="AA", unique_id="BBBB") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "BBBB"}) with pytest.raises(FirmwareSourceError, match="app_version"): source.fetch_previous(ident) @@ -290,7 +290,7 @@ def test_no_credentials_passes_none( class TestPathSegments: def test_default_segments_produce_license_unique_path(self) -> None: src = HttpFirmwareSource(base_url="https://fw.example.com") - ident = FirmwareIdentifier(license_id="AB", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AB", "unique_id": "1234"}) url = src._url(ident, "info.txt") assert url == "https://fw.example.com/AB/1234/info.txt" @@ -299,7 +299,7 @@ def test_custom_segments_change_url_structure(self) -> None: base_url="https://fw.example.com", path_segments=["hw_id", "license_id", "unique_id"], ) - ident = FirmwareIdentifier(license_id="AB", unique_id="1234", hw_id="FF") + ident = FirmwareIdentifier({"license_id": "AB", "unique_id": "1234", "hw_id": "FF"}) url = src._url(ident, "info.txt") assert url == "https://fw.example.com/FF/AB/1234/info.txt" @@ -308,18 +308,18 @@ def test_unchecked_segment_skipped_in_url(self) -> None: base_url="https://fw.example.com", path_segments=["hw_id", "unique_id"], ) - ident = FirmwareIdentifier(license_id="AB", unique_id="1234", hw_id="FF") + ident = FirmwareIdentifier({"license_id": "AB", "unique_id": "1234", "hw_id": "FF"}) url = src._url(ident, "fw.bin") assert "AB" not in url assert url == "https://fw.example.com/FF/1234/fw.bin" def test_empty_segment_value_skipped(self) -> None: - # hw_id not set on identifier → omitted from path + # hw_id not in identifier sections → omitted from path src = HttpFirmwareSource( base_url="https://fw.example.com", path_segments=["hw_id", "unique_id"], ) - ident = FirmwareIdentifier(license_id="AB", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AB", "unique_id": "1234"}) url = src._url(ident, "info.txt") assert url == "https://fw.example.com/1234/info.txt" @@ -328,6 +328,6 @@ def test_no_segments_produces_direct_path(self) -> None: base_url="https://fw.example.com", path_segments=[], ) - ident = FirmwareIdentifier(license_id="AB", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AB", "unique_id": "1234"}) url = src._url(ident, "fw.bin") assert url == "https://fw.example.com/fw.bin" diff --git a/tests/test_local_source.py b/tests/test_local_source.py index 3b12785..c5a50d7 100644 --- a/tests/test_local_source.py +++ b/tests/test_local_source.py @@ -10,7 +10,7 @@ @pytest.fixture def identifier() -> FirmwareIdentifier: - return FirmwareIdentifier(license_id="CC", unique_id="3344") + return FirmwareIdentifier({"license_id": "CC", "unique_id": "3344"}) class TestLocalFirmwareSource: diff --git a/tests/test_workers.py b/tests/test_workers.py index 859825b..8490a64 100644 --- a/tests/test_workers.py +++ b/tests/test_workers.py @@ -135,7 +135,7 @@ def test_init_stores_fields(self, qapp) -> None: from secure_loader.gui.workers import DownloadWorker source = MagicMock() - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident, previous=True) assert worker._source is source @@ -149,7 +149,7 @@ def test_run_latest_calls_fetch_latest_and_emits_finished(self, qapp) -> None: fw = _make_firmware() source = MagicMock() source.fetch_latest.return_value = fw - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident, previous=False) results: list[tuple] = [] @@ -167,7 +167,7 @@ def test_run_previous_calls_fetch_previous(self, qapp) -> None: source = MagicMock() source.fetch_previous.return_value = _make_firmware() - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident, previous=True) worker.run() @@ -181,7 +181,7 @@ def test_run_source_error_emits_error_signal(self, qapp) -> None: source = MagicMock() source.fetch_latest.side_effect = FirmwareSourceError("server down") - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident) errors: list[str] = [] @@ -196,7 +196,7 @@ def test_run_generic_exception_emits_error_signal(self, qapp) -> None: source = MagicMock() source.fetch_latest.side_effect = RuntimeError("unexpected crash") - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident) errors: list[str] = [] @@ -211,7 +211,7 @@ def test_run_unparseable_blob_emits_finished_with_none_header(self, qapp) -> Non source = MagicMock() source.fetch_latest.return_value = b"\x00" * 10 - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident) results: list = [] @@ -234,7 +234,7 @@ def fetch_with_progress(ident, progress=None): source = MagicMock() source.fetch_latest.side_effect = fetch_with_progress - ident = FirmwareIdentifier(license_id="AA", unique_id="1234") + ident = FirmwareIdentifier({"license_id": "AA", "unique_id": "1234"}) worker = DownloadWorker(source, ident) progress_calls: list[tuple] = [] From 8318bf60102da13736def71e8dfa51a5e82a5e5a Mon Sep 17 00:00:00 2001 From: niwciu Date: Mon, 18 May 2026 16:56:17 +0200 Subject: [PATCH 2/3] =?UTF-8?q?docs:=20add=20SecureBootloader=20=E2=86=94?= =?UTF-8?q?=20SecureLoader=20compatibility=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A protocol change in v2.0.0 reduced the UART wire header from 48 → 44 bytes, making the tool pairing version-locked. Added a compatibility matrix to README and a cross-reference callout in the User Guide prerequisites. --- README.md | 16 ++++++++++++++++ docs/USER_GUIDE.md | 2 ++ 2 files changed, 18 insertions(+) diff --git a/README.md b/README.md index 64779af..7deb6ae 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,22 @@ Related repositories: [SecureBootloader](https://github.com/niwciu/SECURE_BOOTLO --- +## 🔗 Tool Compatibility + +The **SecureBootloader ↔ SecureLoader** pairing is version-locked — a protocol change in SecureLoader v2.0.0 reduced the UART wire header from 48 → 44 bytes: + +| SecureLoader | SecureBootloader | Compatible | +|:-------------|:----------------|:----------:| +| < v2.0.0 | v1.0.0 | ✅ | +| ≥ v2.0.0 | ≥ v1.1 | ✅ | +| ≥ v2.0.0 | v1.0.0 | ❌ device stalls on `CMD_START` | +| < v2.0.0 | ≥ v1.1 | ❌ header size mismatch | + +> ⚠️ Always match the SecureLoader version to the SecureBootloader version deployed on your device. +> Flash the updated bootloader (≥ v1.1) to the target before deploying SecureLoader v2.0.0 or later. + +--- + ## 📦 Installation Python 3.10+ required. diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 318d4d4..dc04981 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -17,6 +17,8 @@ This page covers everything an end-user needs: installation, CLI reference, GUI A physical serial port or USB-to-serial adapter is required to communicate with a device. +> **🔗 Tool compatibility:** SecureLoader and SecureBootloader are version-locked due to a wire-header change in v2.0.0 (48 → 44 bytes). See the [compatibility table](https://github.com/niwciu/SecureLoader#-tool-compatibility) before upgrading either component. + --- ## 🚀 Installation From 00ca6afa5991e85a4c59d2897098f6ff0db7427c Mon Sep 17 00:00:00 2001 From: niwciu Date: Mon, 18 May 2026 17:09:25 +0200 Subject: [PATCH 3/3] fix: resolve all CI lint and type-check failures black reformat, mypy no-any-return fixes in FirmwareIdentifier, ruff/flake8 F402 loop variable shadowing, RUF046 redundant int(), N802 noqa on paintEvent, remove stale type: ignore comments. --- src/secure_loader/cli/main.py | 4 +-- src/secure_loader/config.py | 4 +-- src/secure_loader/core/sources/base.py | 12 ++++---- .../gui/server_settings_dialog.py | 28 +++++++++---------- tests/test_cli.py | 27 ++++++++++++------ tests/test_config.py | 12 +++++--- tests/test_gui.py | 5 ++-- 7 files changed, 51 insertions(+), 41 deletions(-) diff --git a/src/secure_loader/cli/main.py b/src/secure_loader/cli/main.py index ae482e4..8ce0701 100644 --- a/src/secure_loader/cli/main.py +++ b/src/secure_loader/cli/main.py @@ -68,9 +68,7 @@ def _setup_logging(verbose: int) -> None: def _format_header(header: FirmwareHeader, config: AppConfig | None = None) -> str: defs = config.id_section_defs if config is not None else list(DEFAULT_ID_SECTIONS) sections = header.get_sections(defs) - section_lines = "".join( - f"\n {name:<16} = {value}" for name, value in sections.items() - ) + section_lines = "".join(f"\n {name:<16} = {value}" for name, value in sections.items()) return ( f" protocolVersion = {header.format_protocol_version()}\n" f" productId = {header.format_product_id()}{section_lines}\n" diff --git a/src/secure_loader/config.py b/src/secure_loader/config.py index ecf4ec8..691b128 100644 --- a/src/secure_loader/config.py +++ b/src/secure_loader/config.py @@ -62,9 +62,7 @@ class AppConfig: http_use_credentials: bool = False http_allow_insecure: bool = False http_path_segments: list[str] = field(default_factory=lambda: list(DEFAULT_PATH_SEGMENTS)) - id_section_defs: list[IdSectionDef] = field( - default_factory=lambda: list(DEFAULT_ID_SECTIONS) - ) + id_section_defs: list[IdSectionDef] = field(default_factory=lambda: list(DEFAULT_ID_SECTIONS)) language: str = "auto" # "en" | "de" | "fr" | "es" | "it" | "pl" | "auto" update_instruction_url: str = "" # empty = menu item hidden last_firmware_paths: list[str] = field(default_factory=list) diff --git a/src/secure_loader/core/sources/base.py b/src/secure_loader/core/sources/base.py index 89c8b8d..27db5ad 100644 --- a/src/secure_loader/core/sources/base.py +++ b/src/secure_loader/core/sources/base.py @@ -48,7 +48,8 @@ def __getattr__(self, name: str) -> str: not ``_sections`` or ``app_version``). """ try: - return object.__getattribute__(self, "_sections")[name] + sections: dict[str, str] = object.__getattribute__(self, "_sections") + return sections[name] except KeyError: available = list(object.__getattribute__(self, "_sections")) raise AttributeError( @@ -63,7 +64,8 @@ def __setattr__(self, name: str, value: object) -> None: def get(self, name: str, default: str = "") -> str: """Return section value or ``default`` if the section is not present.""" - return object.__getattribute__(self, "_sections").get(name, default) + sections: dict[str, str] = object.__getattribute__(self, "_sections") + return sections.get(name, default) @property def section_names(self) -> list[str]: @@ -75,14 +77,12 @@ def section_names(self) -> list[str]: def __repr__(self) -> str: sections = object.__getattribute__(self, "_sections") app_version = object.__getattribute__(self, "app_version") - return ( - f"FirmwareIdentifier(sections={sections!r}, app_version={app_version!r})" - ) + return f"FirmwareIdentifier(sections={sections!r}, app_version={app_version!r})" def __eq__(self, other: object) -> bool: if not isinstance(other, FirmwareIdentifier): return NotImplemented - return ( + return bool( object.__getattribute__(self, "_sections") == object.__getattribute__(other, "_sections") and object.__getattribute__(self, "app_version") diff --git a/src/secure_loader/gui/server_settings_dialog.py b/src/secure_loader/gui/server_settings_dialog.py index e05ff82..7e14f53 100644 --- a/src/secure_loader/gui/server_settings_dialog.py +++ b/src/secure_loader/gui/server_settings_dialog.py @@ -4,7 +4,7 @@ import contextlib -from PySide6.QtCore import Qt, QRect +from PySide6.QtCore import QRect, Qt from PySide6.QtGui import QBrush, QColor, QFont, QPainter, QPaintEvent, QPen from PySide6.QtWidgets import ( QCheckBox, @@ -61,7 +61,7 @@ def set_defs(self, defs: list[IdSectionDef]) -> None: self._defs = list(defs) self.update() - def paintEvent(self, event: QPaintEvent) -> None: # type: ignore[override] + def paintEvent(self, event: QPaintEvent) -> None: # noqa: N802 painter = QPainter(self) painter.setRenderHint(QPainter.RenderHint.Antialiasing) @@ -94,8 +94,8 @@ def paintEvent(self, event: QPaintEvent) -> None: # type: ignore[override] sec_name = "" bg_str, fg_str, border_str = _INACTIVE_COLORS - x = int(round(i * (cell_w + gap))) - span_w = int(round(j * (cell_w + gap) - gap)) - x + x = round(i * (cell_w + gap)) + span_w = round(j * (cell_w + gap) - gap) - x fg = QColor(fg_str) hex_rect = QRect(x, 0, span_w, hex_h) @@ -349,7 +349,7 @@ def _add_section_row(self, sec_def: IdSectionDef | None = None) -> None: else: # Place the new section at the first nibble not covered by any existing valid row. covered: set[int] = set() - for (_ne, other_s, other_e) in self._section_rows: + for _ne, other_s, other_e in self._section_rows: if other_s.value() <= other_e.value(): covered.update(range(other_s.value(), other_e.value() + 1)) new_start = next((i for i in range(16) if i not in covered), 15) @@ -375,7 +375,9 @@ def _add_section_row(self, sec_def: IdSectionDef | None = None) -> None: name_edit.textChanged.connect(self._on_section_changed) start_spin.valueChanged.connect(self._on_section_changed) end_spin.valueChanged.connect(self._on_section_changed) - del_btn.clicked.connect(lambda: self._delete_section_row(row_widget, name_edit, start_spin, end_spin)) + del_btn.clicked.connect( + lambda: self._delete_section_row(row_widget, name_edit, start_spin, end_spin) + ) self._update_spinbox_constraints() def _delete_section_row( @@ -387,10 +389,8 @@ def _delete_section_row( ) -> None: assert self._section_rows_layout is not None self._section_rows_layout.removeWidget(row_widget) - row_widget.setParent(None) # type: ignore[call-overload] - self._section_rows = [ - r for r in self._section_rows if r[0] is not name_edit - ] + row_widget.setParent(None) + self._section_rows = [r for r in self._section_rows if r[0] is not name_edit] self._on_section_changed() def _on_section_changed(self) -> None: @@ -412,16 +412,16 @@ def _update_spinbox_constraints(self) -> None: if not rows: return - for _, s, e in rows: + for _ne, s, e in rows: s.blockSignals(True) e.blockSignals(True) - for _, start_spin, end_spin in rows: + for _ne, start_spin, end_spin in rows: s_val = start_spin.value() # Consider only other rows that are currently in a valid state. others = [ (other_s.value(), other_e.value()) - for (_, other_s, other_e) in rows + for (_ne, other_s, other_e) in rows if other_s is not start_spin and other_s.value() <= other_e.value() ] # start_min: right after the nearest valid section that ends before us. @@ -440,7 +440,7 @@ def _update_spinbox_constraints(self) -> None: start_spin.setMinimum(start_min) end_spin.setMaximum(max(0, end_max)) - for _, s, e in rows: + for _ne, s, e in rows: s.blockSignals(False) e.blockSignals(False) diff --git a/tests/test_cli.py b/tests/test_cli.py index a77adeb..6f835db 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -124,9 +124,12 @@ def test_http_url_rejected_without_allow_insecure(self, runner: CliRunner) -> No cli, [ "fetch", - "--section", "license_id=CC", - "--section", "unique_id=3344", - "--output", "/dev/null", + "--section", + "license_id=CC", + "--section", + "unique_id=3344", + "--output", + "/dev/null", ], catch_exceptions=True, ) @@ -148,9 +151,12 @@ def test_http_url_allowed_with_allow_insecure_flag(self, runner: CliRunner) -> N cli, [ "fetch", - "--section", "license_id=CC", - "--section", "unique_id=3344", - "--output", "/dev/null", + "--section", + "license_id=CC", + "--section", + "unique_id=3344", + "--output", + "/dev/null", "--allow-insecure", ], catch_exceptions=True, @@ -171,9 +177,12 @@ def test_https_url_no_cleartext_warning(self, runner: CliRunner) -> None: cli, [ "fetch", - "--section", "license_id=CC", - "--section", "unique_id=3344", - "--output", "/dev/null", + "--section", + "license_id=CC", + "--section", + "unique_id=3344", + "--output", + "/dev/null", ], catch_exceptions=True, ) diff --git a/tests/test_config.py b/tests/test_config.py index 9403ba9..d4bc058 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -28,7 +28,13 @@ def test_defaults_when_file_missing(self, tmp_cfg: Path) -> None: assert cfg.update_instruction_url == "" assert cfg.last_firmware_paths == [] - def test_save_and_reload(self, tmp_cfg: Path) -> None: + def test_save_and_reload(self, tmp_cfg: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _store: dict[tuple[str, str], str] = {} + mock_kr = MagicMock() + mock_kr.set_password.side_effect = lambda svc, usr, pw: _store.__setitem__((svc, usr), pw) + mock_kr.get_password.side_effect = lambda svc, usr: _store.get((svc, usr)) + monkeypatch.setattr("secure_loader.config.keyring", mock_kr) + original = AppConfig( http_base_url="https://example.com", http_login="user", @@ -192,9 +198,7 @@ def test_plaintext_password_migrated_to_keyring_on_load( monkeypatch.setattr("secure_loader.config.keyring", mock_kr) # Write a legacy config that still has the password in plaintext. - tmp_cfg.write_text( - "[http]\nlogin = user\npassword = legacy_secret\n", encoding="utf-8" - ) + tmp_cfg.write_text("[http]\nlogin = user\npassword = legacy_secret\n", encoding="utf-8") loaded = load_config(tmp_cfg) assert loaded.http_password == "legacy_secret" diff --git a/tests/test_gui.py b/tests/test_gui.py index 67d0d4f..10687b4 100644 --- a/tests/test_gui.py +++ b/tests/test_gui.py @@ -485,8 +485,8 @@ def test_pid_viz_widget_exists(self, server_settings_dialog) -> None: def test_pid_viz_contains_section_nibble_ranges(self, server_settings_dialog) -> None: defs = server_settings_dialog._pid_viz._defs ranges = [(d.start, d.end - 1) for d in defs] # inclusive end for display - assert (0, 7) in ranges # custom_id - assert (8, 9) in ranges # hw_id + assert (0, 7) in ranges # custom_id + assert (8, 9) in ranges # hw_id assert (10, 11) in ranges # license_id assert (12, 15) in ranges # unique_id @@ -496,6 +496,7 @@ def test_pid_viz_active_sections_shown_in_color(self, server_settings_dialog) -> def test_pid_viz_inactive_nibbles_shown_in_grey(self, server_settings_dialog) -> None: from secure_loader.gui.server_settings_dialog import _INACTIVE_COLORS + assert _INACTIVE_COLORS[1] == "#9ca3af" def test_pid_viz_shows_section_names(self, server_settings_dialog) -> None: