From 7be9f6d8b976529b5d9ddc8a773445862c120ffd Mon Sep 17 00:00:00 2001 From: Luke Date: Sat, 10 Oct 2026 07:13:39 -0400 Subject: [PATCH 1/3] fix(storage): atomically persist and serialize inventory mutations --- .../https_server/routes/user/homes/service.py | 3 +- .../https_server/routes/user/scene/service.py | 5 +- .../bundled_backend/shared/inventory_io.py | 62 +++++- src/roborock_local_server/cloud.py | 14 +- src/roborock_local_server/server.py | 176 +++++++++--------- tests/test_inventory_io.py | 71 +++++++ 6 files changed, 230 insertions(+), 101 deletions(-) create mode 100644 tests/test_inventory_io.py diff --git a/src/roborock_local_server/bundled_backend/https_server/routes/user/homes/service.py b/src/roborock_local_server/bundled_backend/https_server/routes/user/homes/service.py index becd33c..547fa73 100644 --- a/src/roborock_local_server/bundled_backend/https_server/routes/user/homes/service.py +++ b/src/roborock_local_server/bundled_backend/https_server/routes/user/homes/service.py @@ -8,7 +8,7 @@ from shared.constants import DEFAULT_HOME_NAME from shared.context import ServerContext from shared.data_helpers import as_bool, as_int, default_home_id, get_value, stable_int -from shared.inventory_io import WEB_API_INVENTORY_FILE, load_inventory, write_inventory +from shared.inventory_io import WEB_API_INVENTORY_FILE, inventory_mutation, load_inventory, write_inventory from ..devices.service import _home_data as _base_home_data from ..devices.service import enrich_home_data_with_cloud_snapshot @@ -40,6 +40,7 @@ def extract_home_id_from_rooms_path(ctx: ServerContext, clean_path: str) -> int: return as_int(match.group(1), default_home_id(ctx)) +@inventory_mutation def upsert_inventory_room(ctx: ServerContext, *, home_id: int, room_name: str) -> tuple[dict[str, Any], bool]: inventory = load_inventory(ctx) if not isinstance(inventory, dict): diff --git a/src/roborock_local_server/bundled_backend/https_server/routes/user/scene/service.py b/src/roborock_local_server/bundled_backend/https_server/routes/user/scene/service.py index 087c757..a7ce206 100644 --- a/src/roborock_local_server/bundled_backend/https_server/routes/user/scene/service.py +++ b/src/roborock_local_server/bundled_backend/https_server/routes/user/scene/service.py @@ -8,7 +8,7 @@ from shared.context import ServerContext from shared.data_helpers import as_bool, as_int, default_home_id, get_value, stable_int -from shared.inventory_io import WEB_API_INVENTORY_FILE, load_inventory, write_inventory +from shared.inventory_io import WEB_API_INVENTORY_FILE, inventory_mutation, load_inventory, write_inventory from shared.routine_runner import RoutineExecutionError, RoutineRunner _LOGGER = logging.getLogger(__name__) @@ -424,6 +424,7 @@ def _scene_device_name(inventory: dict[str, Any], device_id: str) -> str: return "" +@inventory_mutation def _replace_inventory_scene( ctx: ServerContext, *, @@ -486,6 +487,7 @@ def apply_update(updated_scene: dict[str, Any], inventory: dict[str, Any]) -> No return updated_scene +@inventory_mutation def _create_inventory_scene(ctx: ServerContext, scene_request: dict[str, Any]) -> dict[str, Any]: inventory = load_inventory(ctx) if not isinstance(inventory, dict): @@ -697,6 +699,7 @@ def apply_update(updated_scene: dict[str, Any], inventory: dict[str, Any]) -> No return build_scene_payload(updated_scene, home_id=home_id, include_device_context=True) +@inventory_mutation def delete_scene(ctx: ServerContext, scene_id: int) -> None: inventory = load_inventory(ctx) if not isinstance(inventory, dict): diff --git a/src/roborock_local_server/bundled_backend/shared/inventory_io.py b/src/roborock_local_server/bundled_backend/shared/inventory_io.py index 6082cf9..addf2cb 100644 --- a/src/roborock_local_server/bundled_backend/shared/inventory_io.py +++ b/src/roborock_local_server/bundled_backend/shared/inventory_io.py @@ -1,12 +1,66 @@ from __future__ import annotations +from contextlib import contextmanager +from functools import wraps import json +import logging +import os +from pathlib import Path +import tempfile +import threading from typing import Any, TYPE_CHECKING if TYPE_CHECKING: from .context import ServerContext WEB_API_INVENTORY_FILE = "web_api_inventory.json" +_LOGGER = logging.getLogger(__name__) +_LOCKS: dict[Path, Any] = {} +_LOCKS_GUARD = threading.Lock() + + +@contextmanager +def inventory_transaction(path: Path): + """Serialize inventory read/modify/write within this server process. + + Refuse to mutate corrupt existing inventory rather than replacing it with + an empty fallback. External writers must stop the server first. + """ + path = path.resolve() + with _LOCKS_GUARD: + lock = _LOCKS.setdefault(path, threading.RLock()) + with lock: + if path.exists(): + loaded = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(loaded, dict): + raise ValueError("Existing inventory must be a JSON object") + yield + + +def inventory_mutation(function): + """Protect a synchronous context-first inventory mutation.""" + @wraps(function) + def wrapped(ctx, *args, **kwargs): + with inventory_transaction(ctx.http_jsonl.parent / WEB_API_INVENTORY_FILE): + return function(ctx, *args, **kwargs) + return wrapped + + +def atomic_write_inventory(path: Path, inventory: dict[str, Any]) -> None: + """Replace only after a complete write; propagate failures to the caller.""" + payload = json.dumps(inventory, ensure_ascii=False, indent=2) + "\n" + temporary = None + try: + with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", dir=path.parent, + prefix=f".{path.name}.", suffix=".tmp", delete=False) as handle: + temporary = Path(handle.name) + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary, path) + finally: + if temporary is not None: + temporary.unlink(missing_ok=True) def load_inventory(ctx: ServerContext) -> dict[str, Any]: @@ -20,9 +74,11 @@ def load_inventory(ctx: ServerContext) -> dict[str, Any]: return loaded if isinstance(loaded, dict) else {} -def write_inventory(ctx: ServerContext, inventory: dict[str, Any]) -> None: +def write_inventory(ctx: ServerContext, inventory: dict[str, Any]) -> bool: path = ctx.http_jsonl.parent / WEB_API_INVENTORY_FILE try: - path.write_text(json.dumps(inventory, ensure_ascii=False, indent=2), encoding="utf-8") + atomic_write_inventory(path, inventory) except OSError: - return + _LOGGER.exception("Unable to persist inventory") + return False + return True diff --git a/src/roborock_local_server/cloud.py b/src/roborock_local_server/cloud.py index 6f3773d..b0c96c9 100644 --- a/src/roborock_local_server/cloud.py +++ b/src/roborock_local_server/cloud.py @@ -27,6 +27,7 @@ _normalize_schedule_map, _normalize_value_map, ) +from shared.inventory_io import atomic_write_inventory, inventory_transaction def _to_jsonable(value: Any) -> Any: @@ -187,13 +188,14 @@ async def submit_code(self, *, session_id: str, code: str) -> dict[str, Any]: } normalized_inventory = _to_jsonable(inventory) normalized_snapshot = _to_jsonable(snapshot) - normalized_inventory = _merge_existing_inventory_mutations( - normalized_inventory, - _load_existing_inventory_for_merge(self.inventory_path), - ) - self.inventory_path.parent.mkdir(parents=True, exist_ok=True) - self.inventory_path.write_text(json.dumps(normalized_inventory, indent=2) + "\n", encoding="utf-8") + with inventory_transaction(self.inventory_path): + normalized_inventory = _merge_existing_inventory_mutations( + normalized_inventory, + _load_existing_inventory_for_merge(self.inventory_path), + ) + + atomic_write_inventory(self.inventory_path, normalized_inventory) self.snapshot_path.write_text(json.dumps(normalized_snapshot, indent=2) + "\n", encoding="utf-8") with self._lock: diff --git a/src/roborock_local_server/server.py b/src/roborock_local_server/server.py index c210ee6..8cadb12 100644 --- a/src/roborock_local_server/server.py +++ b/src/roborock_local_server/server.py @@ -50,6 +50,7 @@ start_broker, strip_roborock_prefix, ) +from shared.inventory_io import atomic_write_inventory, inventory_transaction from shared.protocol_auth import ProtocolAuthStore from https_server.routes.auth.service import ( build_login_data_response, @@ -1643,97 +1644,92 @@ def _persist_discovered_device_to_inventory( product_id: str, local_key: str, ) -> bool: - inventory_id = (duid or did).strip() - if not inventory_id: - return False - inventory = _load_inventory(self.paths.inventory_path) - devices = inventory.get("devices") - if not isinstance(devices, list): - devices = [] - identifiers = {value for value in (inventory_id, did, duid) if value} - meta = resolve_product_metadata( - model=model, - custom_name=name, - custom_registry_path=self.paths.runtime_dir / "product_registry.custom.json", - ) - resolved_name = meta["product_name"] - resolved_model = meta["model"] - resolved_category = meta["category"] - resolved_product_id = product_id or meta["product_id"] - resolved_schema = meta["schema"] - - for existing in devices: - if not isinstance(existing, dict): - continue - existing_ids = { - str(existing.get(key) or "").strip() - for key in ("duid", "did", "device_id", "deviceId") - } - if identifiers & existing_ids: - if existing.get("source") != "onboarding": - # Cloud-imported records are authoritative; never rewrite them here. - return False - changed = False - if name and (not existing.get("name") or existing.get("name") in identifiers): - existing["name"] = resolved_name - changed = True - if not existing.get("product_name"): - existing["product_name"] = resolved_name - changed = True - if model and not existing.get("model"): - existing["model"] = resolved_model - changed = True - if not existing.get("schema"): - existing["schema"] = resolved_schema - changed = True - if not existing.get("category"): - existing["category"] = resolved_category - changed = True - if changed: - self.paths.inventory_path.write_text( - json.dumps(inventory, ensure_ascii=False, indent=2), - encoding="utf-8", - ) + with inventory_transaction(self.paths.inventory_path): + inventory_id = (duid or did).strip() + if not inventory_id: return False - # Unify the runtime-credentials record so its duid matches the inventory id while - # keeping the already-minted localKey. sync_inventory matches on duid only, so an - # unset duid would otherwise make it create a duplicate device with a fresh key. - self.runtime_credentials.ensure_device( - did=did, - duid=inventory_id, - name=resolved_name, - model=resolved_model, - product_id=resolved_product_id, - assign_localkey=False, - ) - devices.append( - { - "duid": inventory_id, - "did": did, - "name": resolved_name, - "product_name": resolved_name, - "model": resolved_model, - "category": resolved_category, - "product_id": resolved_product_id, - "local_key": local_key, - "source": "onboarding", - "schema": resolved_schema, - } - ) - inventory["devices"] = devices - if not isinstance(inventory.get("home"), dict): - inventory["home"] = {"name": "Local Home", "rooms": [{"id": 1, "name": "Living Room"}]} - self.paths.inventory_path.write_text( - json.dumps(inventory, ensure_ascii=False, indent=2), - encoding="utf-8", - ) - self.root_logger.info( - "persisted onboarded 'new vacuum' to inventory: id=%s did=%s model=%s", - inventory_id, - did or "-", - model or "unknown", - ) - return True + inventory = _load_inventory(self.paths.inventory_path) + devices = inventory.get("devices") + if not isinstance(devices, list): + devices = [] + identifiers = {value for value in (inventory_id, did, duid) if value} + meta = resolve_product_metadata( + model=model, + custom_name=name, + custom_registry_path=self.paths.runtime_dir / "product_registry.custom.json", + ) + resolved_name = meta["product_name"] + resolved_model = meta["model"] + resolved_category = meta["category"] + resolved_product_id = product_id or meta["product_id"] + resolved_schema = meta["schema"] + + for existing in devices: + if not isinstance(existing, dict): + continue + existing_ids = { + str(existing.get(key) or "").strip() + for key in ("duid", "did", "device_id", "deviceId") + } + if identifiers & existing_ids: + if existing.get("source") != "onboarding": + # Cloud-imported records are authoritative; never rewrite them here. + return False + changed = False + if name and (not existing.get("name") or existing.get("name") in identifiers): + existing["name"] = resolved_name + changed = True + if not existing.get("product_name"): + existing["product_name"] = resolved_name + changed = True + if model and not existing.get("model"): + existing["model"] = resolved_model + changed = True + if not existing.get("schema"): + existing["schema"] = resolved_schema + changed = True + if not existing.get("category"): + existing["category"] = resolved_category + changed = True + if changed: + atomic_write_inventory(self.paths.inventory_path, inventory) + return False + # Unify the runtime-credentials record so its duid matches the inventory id while + # keeping the already-minted localKey. sync_inventory matches on duid only, so an + # unset duid would otherwise make it create a duplicate device with a fresh key. + self.runtime_credentials.ensure_device( + did=did, + duid=inventory_id, + name=resolved_name, + model=resolved_model, + product_id=resolved_product_id, + assign_localkey=False, + ) + devices.append( + { + "duid": inventory_id, + "did": did, + "name": resolved_name, + "product_name": resolved_name, + "model": resolved_model, + "category": resolved_category, + "product_id": resolved_product_id, + "local_key": local_key, + "source": "onboarding", + "schema": resolved_schema, + } + ) + inventory["devices"] = devices + if not isinstance(inventory.get("home"), dict): + inventory["home"] = {"name": "Local Home", "rooms": [{"id": 1, "name": "Living Room"}]} + atomic_write_inventory(self.paths.inventory_path, inventory) + self.root_logger.info( + "persisted onboarded 'new vacuum' to inventory: id=%s did=%s model=%s", + inventory_id, + did or "-", + model or "unknown", + ) + return True def clear_onboarding_session(self, *, session_id: str) -> dict[str, Any]: snapshot = self.runtime_state.onboarding_session_snapshot() diff --git a/tests/test_inventory_io.py b/tests/test_inventory_io.py new file mode 100644 index 0000000..1bb588a --- /dev/null +++ b/tests/test_inventory_io.py @@ -0,0 +1,71 @@ +import json +import logging +from concurrent.futures import ThreadPoolExecutor +from threading import Barrier +from types import SimpleNamespace + +import pytest + +from roborock_local_server import backend # initializes bundled import path +from shared import inventory_io + + +def test_replace_failure_preserves_old_inventory_and_cleans_temp(tmp_path, monkeypatch): + path = tmp_path / inventory_io.WEB_API_INVENTORY_FILE + path.write_text('{"devices": [{"did": "original"}]}', encoding="utf-8") + original = path.read_bytes() + + def fail_replace(*args): + raise OSError("disk failure") + + monkeypatch.setattr(inventory_io.os, "replace", fail_replace) + with pytest.raises(OSError): + inventory_io.atomic_write_inventory(path, {"devices": []}) + assert path.read_bytes() == original + assert list(tmp_path.iterdir()) == [path] + + +def test_mutations_from_threads_keep_both_updates(tmp_path): + path = tmp_path / inventory_io.WEB_API_INVENTORY_FILE + inventory_io.atomic_write_inventory(path, {"devices": [], "scenes": []}) + ctx = SimpleNamespace(http_jsonl=tmp_path / "http.jsonl") + start = Barrier(2) + + @inventory_io.inventory_mutation + def mutate(ctx, field): + loaded = inventory_io.load_inventory(ctx) + loaded[field].append({"id": field}) + assert inventory_io.write_inventory(ctx, loaded) + + def run(field): + start.wait(timeout=5) + mutate(ctx, field) + + with ThreadPoolExecutor(max_workers=2) as pool: + futures = [pool.submit(run, field) for field in ("devices", "scenes")] + for future in futures: + future.result(timeout=5) + assert json.loads(path.read_text()) == { + "devices": [{"id": "devices"}], "scenes": [{"id": "scenes"}], + } + + +def test_corrupt_inventory_is_not_replaced_by_mutation(tmp_path): + path = tmp_path / inventory_io.WEB_API_INVENTORY_FILE + path.write_text('{broken', encoding="utf-8") + with pytest.raises(json.JSONDecodeError): + with inventory_io.inventory_transaction(path): + inventory_io.atomic_write_inventory(path, {}) + assert path.read_text() == '{broken' + + +def test_context_write_failure_returns_false_and_logs(tmp_path, monkeypatch, caplog): + ctx = SimpleNamespace(http_jsonl=tmp_path / "http.jsonl") + + def fail(*args): + raise OSError("disk full") + + monkeypatch.setattr(inventory_io, "atomic_write_inventory", fail) + with caplog.at_level(logging.ERROR): + assert inventory_io.write_inventory(ctx, {}) is False + assert "Unable to persist inventory" in caplog.text From 056b0d0671a1f31d28aa663ffd41cf4b5828c187 Mon Sep 17 00:00:00 2001 From: Luke Date: Sat, 10 Oct 2026 07:14:24 -0400 Subject: [PATCH 2/3] fix(bootstrap): retain verified device info metadata with outcome diagnostics --- docs/device-info.md | 92 +++++++++ mkdocs.yml | 1 + .../https_server/endpoint_rules.py | 3 + .../routes/bootstrap/device_info.py | 112 +++++++++++ tests/test_device_info.py | 180 ++++++++++++++++++ 5 files changed, 388 insertions(+) create mode 100644 docs/device-info.md create mode 100644 src/roborock_local_server/bundled_backend/https_server/routes/bootstrap/device_info.py create mode 100644 tests/test_device_info.py diff --git a/docs/device-info.md b/docs/device-info.md new file mode 100644 index 0000000..e90eb7a --- /dev/null +++ b/docs/device-info.md @@ -0,0 +1,92 @@ +# Firmware device information uploads + +`POST /devices/{did}/info` uploads the vacuum's serial number, model and feature +flags. This is a firmware endpoint; it is separate from the app's +`GET /user/devices/{duid}` endpoint. + +The server now selects the `device_info` handler instead of `catchall`. The +previous generic HTTP 200 already acknowledged the upload on the firmware +inspected below. There is no evidence that this fallback blocked onboarding or +camera operation. The missing behavior was retaining the uploaded metadata. + +## Handling and authentication + +The verified firmware signs this exact ASCII form, in this order, using its +device RSA private key with PKCS#1 v1.5 and SHA-256: + +```text +did=&featureset=&newfeatureset=&pid=&sn= +``` + +It appends a URL-escaped base64 `signature`. The server checks the signature +against the device public key recovered during onboarding and checks that the +body DID matches the URL DID. Only unambiguous reports with one value per field +are persisted. Query parameters do not supply or override metadata. + +This handler accepts exactly these six fields, nonempty metadata values of +1–128 ASCII letters, digits, dots, underscores or hyphens, decimal `featureset`, +and hexadecimal `newfeatureset`. These conservative limits match the observed +reports; authentic firmware variants outside them are acknowledged and logged as +unsupported without writing. They require further evidence before expanding the +accepted contract. + +For an existing inventory device matched by DID, or by its runtime DID-to-DUID +link, verified reports update `sn`, `featureSet`, and `newFeatureSet` in +`web_api_inventory.json`. Hex feature flags stay as strings so leading zeroes +survive. The signature is not copied into inventory. Names, keys, product IDs and device +identities are preserved. Firmware `pid` is a model name in the inspected build; +it must not be assigned to the app's opaque `productId`. + +Unknown devices are not created by this endpoint. Missing keys, unsupported +signatures, duplicate fields, malformed values and invalid signatures cause no +inventory changes. Their uploads still receive HTTP 200, preserving the old +acknowledgement behavior. Responses retain the previous fallback body, including +`data` and `result` containing `{"ok": true, "route": "/devices/{did}/info"}`. +The inspected G10S firmware ignores that body; preserving it avoids changing the +response contract for other models whose callback has not been audited. + +There is no nonce or timestamp in this form. A valid recorded report can be +replayed; signature verification establishes its origin, not its freshness. +No replayed report can change identity or credentials through this handler. + +The server logs the DID, an outcome (`stored`, `unchanged`, `unmatched`, +`malformed_fields`, `did_mismatch`, `unsupported_values`, `missing_key`, or +`invalid_signature`, or `write_failed`) and the number of records updated for each upload. These +diagnostics omit serial numbers, signatures and credentials. + +Devices already acknowledged by the old fallback may not upload again until +their metadata changes. The inspected firmware saves a digest on success; whether +that digest survives a reboot has not been characterized. For devices with a +cloud snapshot, existing app response behavior still prefers snapshot serial and +feature fields over this inventory metadata. + +## Evidence and limits + +The local G10S `rriot_rr` ELF (SHA-256 +`27cf67a4a03920e31647e677d8296bcf4f915801e1800d6db9527ccfe69ae752`) +provides these AArch64 virtual addresses: + +| Address | Observed behavior | +| --- | --- | +| `0x16810–0x16834` | Formats the five metadata fields in the order above. | +| `0x16850–0x16894` | Calls signer `0x6010`, URL-escapes signature, appends it. | +| `0x5970–0x5a24` | SHA256 followed by RSA_sign with NID 672 (`sha256`). | +| `0x1696c–0x169a8` | Formats `/devices/%s/info`, starts POST with callback `0x16f60`. | +| `0x4bf8–0x4c60` | HTTP worker accepts status 200 or 201, returns success. | +| `0x4f44–0x4f54` | Passes worker status and response buffer to callback. | +| `0x16fec–0x1706c` | Callback checks worker status only, marks upload done and writes the metadata digest; does not parse the response. | + +The S7 `rriot_rr` ELF (SHA-256 +`824b3128856a3017085b391d690f210420be4e8023365a6e22597bc0f460ba3d`) +also contains the same five-field form and endpoint at file offsets `0x28730` +and `0x288c0`. Its callback has not been audited here. + +All three persisted Qrevo MaxV uploads (October 1 and October 9, 2026 local time) +were independently verified with its recovered RSA public key against this +exact form and PKCS#1 v1.5/SHA-256 contract. No raw signatures, serial numbers or +keys are included here. This confirms the Qrevo's request authentication; its +response callback has not been disassembled. The handler preserves +acknowledgement for other contracts and refuses metadata writes when verification +fails. A cloud response capture is unnecessary +for the inspected G10S callback, but would be needed to describe the cloud's exact +response body or another model's response parser. diff --git a/mkdocs.yml b/mkdocs.yml index 0618742..0b71661 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -57,6 +57,7 @@ nav: - Integrations: - Mobile App Options: roborock_app.md - Reference: + - Firmware Device Information: device-info.md - Known Limitations: known_limitations.md - Tested Vacuums: tested_vacuums.md - Reverse Proxy: reverse_proxy.md diff --git a/src/roborock_local_server/bundled_backend/https_server/endpoint_rules.py b/src/roborock_local_server/bundled_backend/https_server/endpoint_rules.py index 119d3fa..9e333f9 100644 --- a/src/roborock_local_server/bundled_backend/https_server/endpoint_rules.py +++ b/src/roborock_local_server/bundled_backend/https_server/endpoint_rules.py @@ -67,6 +67,8 @@ from .routes.api.v5.product import match as _match_get_products_v5 from .routes.bootstrap.catchall import build as _build_catchall from .routes.bootstrap.catchall import match as _match_catchall +from .routes.bootstrap.device_info import build as _build_device_info +from .routes.bootstrap.device_info import match as _match_device_info from .routes.fw.check import build as _build_fw_check from .routes.fw.check import match as _match_fw_check from .routes.bootstrap.location import build as _build_location @@ -601,6 +603,7 @@ def default_endpoint_rules() -> Sequence[EndpointRule]: EndpointRule("time", _match_time, _build_time), EndpointRule("location", _match_location, _build_location), EndpointRule("fw_check", _match_fw_check, _build_fw_check), + EndpointRule("device_info", _match_device_info, _build_device_info), EndpointRule("catchall", _match_catchall, _build_catchall), ) diff --git a/src/roborock_local_server/bundled_backend/https_server/routes/bootstrap/device_info.py b/src/roborock_local_server/bundled_backend/https_server/routes/bootstrap/device_info.py new file mode 100644 index 0000000..d9e24d7 --- /dev/null +++ b/src/roborock_local_server/bundled_backend/https_server/routes/bootstrap/device_info.py @@ -0,0 +1,112 @@ +"""Acknowledge signed firmware metadata uploads and update known devices. + +The G10S rriot_rr upload callback accepts HTTP 200/201 and ignores the response +body. Its form is RSA PKCS#1 v1.5/SHA-256 signed in the field order below. See +docs/device-info.md for firmware evidence and the limits of this contract. +""" + +from __future__ import annotations + +import base64 +import logging +import re +from typing import Any + +from cryptography.exceptions import InvalidSignature +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import padding + +from shared.context import ServerContext +from shared.inventory_io import WEB_API_INVENTORY_FILE, inventory_transaction, load_inventory, write_inventory + +from .catchall import build as _build_catchall + +_PATH = re.compile(r"/devices/([0-9]+)/info/?") +_FIELDS = ("did", "featureset", "newfeatureset", "pid", "sn") +_SAFE_VALUE = re.compile(r"[A-Za-z0-9._-]{1,128}") +_LOGGER = logging.getLogger(__name__) + + +def match(path: str, method: str = "POST") -> bool: + return method.upper() == "POST" and _PATH.fullmatch(path) is not None + + +def _verified_report( + ctx: ServerContext, params: dict[str, list[str]], did: str +) -> tuple[dict[str, str] | None, str]: + # Duplicates and extra fields would make reconstructing the signed message + # ambiguous. Unsupported forms retain the acknowledgement but cannot write. + if set(params) != {*_FIELDS, "signature"} or any(len(v) != 1 for v in params.values()): + return None, "malformed_fields" + fields = {name: params[name][0] for name in _FIELDS} + if fields["did"] != did: + return None, "did_mismatch" + if any(_SAFE_VALUE.fullmatch(v) is None for v in fields.values()): + return None, "unsupported_values" + if not fields["featureset"].isdigit() or re.fullmatch(r"[0-9a-fA-F]+", fields["newfeatureset"]) is None: + return None, "unsupported_values" + key = ctx.device_public_key(did) + if key is None: + return None, "missing_key" + canonical = "&".join(f"{name}={fields[name]}" for name in _FIELDS) + try: + signature = base64.b64decode(params["signature"][0], validate=True) + key.verify(signature, canonical.encode("ascii"), padding.PKCS1v15(), hashes.SHA256()) + except (InvalidSignature, ValueError, TypeError): + return None, "invalid_signature" + return fields, "verified" + + +def build( + ctx: ServerContext, + _query_params: dict[str, list[str]], + body_params: dict[str, list[str]], + clean_path: str, +) -> dict[str, Any]: + path_match = _PATH.fullmatch(clean_path) + assert path_match is not None # Route matcher already checked the path. + did = path_match.group(1) + report, outcome = _verified_report(ctx, body_params, did) + updated_count = 0 + if report is not None: + identities = {did} + if ctx.runtime_credentials is not None: + device = ctx.runtime_credentials.resolve_device(did=did) + if device and device.get("duid"): + identities.add(device["duid"]) + with inventory_transaction(ctx.http_jsonl.parent / WEB_API_INVENTORY_FILE): + inventory = load_inventory(ctx) + matched_count = 0 + for collection in ("devices", "received_devices", "receivedDevices"): + devices = inventory.get(collection) + if not isinstance(devices, list): + continue + for device in devices: + if not isinstance(device, dict): + continue + if not any(str(device.get(key, "")) in identities for key in ("did", "duid")): + continue + matched_count += 1 + update: dict[str, Any] = { + "sn": report["sn"], + "featureSet": report["featureset"], + "newFeatureSet": report["newfeatureset"], + } + # Inventory readers also accept snake_case fields, with some + # readers preferring them. Keep existing aliases consistent. + for alias, field in (("feature_set", "featureset"), ("new_feature_set", "newfeatureset")): + if alias in device: + update[alias] = report[field] + if any(device.get(key) != value for key, value in update.items()): + device.update(update) + updated_count += 1 + outcome = "unchanged" if matched_count else "unmatched" + if updated_count: + if write_inventory(ctx, inventory): + outcome = "stored" + else: + outcome, updated_count = "write_failed", 0 + _LOGGER.info("Device info did=%s outcome=%s updated=%d", did, outcome, updated_count) + # No response data is consumed by the verified firmware. Continue accepting + # unsupported/unverifiable reports without allowing them to alter inventory. + return _build_catchall(ctx, _query_params, body_params, clean_path) diff --git a/tests/test_device_info.py b/tests/test_device_info.py new file mode 100644 index 0000000..f6a240a --- /dev/null +++ b/tests/test_device_info.py @@ -0,0 +1,180 @@ +"""Signed firmware metadata must never overwrite another device's identity.""" + +import base64 +import json +import logging +from types import SimpleNamespace +from urllib.parse import urlencode + +import pytest +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import padding, rsa + +import roborock_local_server.backend as backend +from https_server.routes.bootstrap import catchall, device_info + + +@pytest.fixture +def setup_report(tmp_path): + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + inventory = { + "home": {"name": "Home"}, + "devices": [ + {"did": "123", "duid": "cloud-a", "name": "My Vacuum", "productId": "product-a", "localKey": "existing-key"}, + {"did": "456", "duid": "cloud-b", "sn": "OTHER"}, + ], + } + path = tmp_path / "web_api_inventory.json" + path.write_text(json.dumps(inventory)) + ctx = SimpleNamespace(http_jsonl=tmp_path / "http.jsonl", runtime_credentials=None, + device_public_key=lambda did: key.public_key() if did == "123" else None) + fields = {"did": "123", "featureset": "123456789", "newfeatureset": "0000000AbCD", "pid": "roborock.vacuum.a87", "sn": "TESTSERIAL"} + canonical = "&".join(f"{name}={value}" for name, value in fields.items()).encode() + params = {name: [value] for name, value in fields.items()} + params["signature"] = [base64.b64encode(key.sign(canonical, padding.PKCS1v15(), hashes.SHA256())).decode()] + return ctx, path, inventory, params + + +def _request(ctx, params, *, path="/devices/123/info", method="POST"): + return backend.resolve_route(rules=backend.default_endpoint_rules(), context=ctx, clean_path=path, + query_params={}, body_params=params, method=method) + + +def test_verified_upload_updates_only_metadata(setup_report): + ctx, path, original, params = setup_report + route, response = _request(ctx, params) + assert route == "device_info" + assert response["code"] == 200 + assert response == catchall.build(ctx, {}, params, "/devices/123/info") + updated = json.loads(path.read_text()) + device = updated["devices"][0] + assert device["sn"] == "TESTSERIAL" + assert device["featureSet"] == "123456789" + assert device["newFeatureSet"] == "0000000AbCD" # Preserve leading zeroes. + assert set(device) - set(original["devices"][0]) == {"sn", "featureSet", "newFeatureSet"} + for key, value in original["devices"][0].items(): + assert device[key] == value + assert updated["devices"][1] == original["devices"][1] + assert updated["home"] == original["home"] + before = path.read_bytes() + _request(ctx, params) + assert path.read_bytes() == before + + +@pytest.mark.parametrize("change", ["tampered", "missing", "duplicate", "extra", "wrong_did", "unknown_key", "bad_base64"]) +def test_unverifiable_upload_acknowledged_without_writes(setup_report, change): + ctx, path, _, params = setup_report + before = path.read_bytes() + if change == "tampered": params["sn"] = ["ALTERED"] + if change == "missing": del params["signature"] + if change == "duplicate": params["did"].append("456") + if change == "extra": params["other"] = ["value"] + if change == "wrong_did": params["did"] = ["456"] + if change == "unknown_key": ctx.device_public_key = lambda did: None + if change == "bad_base64": params["signature"] = ["!!!!"] + route, response = _request(ctx, params) + assert route == "device_info" and response["code"] == 200 + assert path.read_bytes() == before + + +def test_cloud_duid_link_selects_known_device(setup_report): + ctx, path, _, params = setup_report + inventory = json.loads(path.read_text()) + del inventory["devices"][0]["did"] + path.write_text(json.dumps(inventory)) + ctx.runtime_credentials = SimpleNamespace(resolve_device=lambda **kwargs: {"did": "123", "duid": "cloud-a"}) + _request(ctx, params) + assert json.loads(path.read_text())["devices"][0]["sn"] == "TESTSERIAL" + + +def test_verified_unknown_device_does_not_create_inventory_entry(setup_report): + ctx, path, _, params = setup_report + path.write_text('{"devices": []}') + _request(ctx, params) + assert json.loads(path.read_text()) == {"devices": []} + + +def test_existing_feature_aliases_stay_consistent(setup_report): + ctx, path, _, params = setup_report + inventory = json.loads(path.read_text()) + inventory["devices"][0].update(feature_set="old", new_feature_set="old") + path.write_text(json.dumps(inventory)) + _request(ctx, params) + device = json.loads(path.read_text())["devices"][0] + assert device["feature_set"] == device["featureSet"] == "123456789" + assert device["new_feature_set"] == device["newFeatureSet"] == "0000000AbCD" + + +@pytest.mark.parametrize("method,path", [("GET", "/devices/123/info"), ("POST", "/user/devices/123/info"), ("POST", "/devices/abc/info")]) +def test_route_scope(setup_report, method, path): + ctx, inventory_path, _, params = setup_report + before = inventory_path.read_bytes() + route, _ = _request(ctx, params, path=path, method=method) + assert route == "catchall" + assert inventory_path.read_bytes() == before + + +@pytest.mark.parametrize("change,outcome", [ + ("valid", "stored"), ("repeat", "unchanged"), ("tampered", "invalid_signature"), + ("missing_key", "missing_key"), ("unmatched", "unmatched"), + ("write_failed", "write_failed"), +]) +def test_outcome_diagnostics_are_sanitized(setup_report, caplog, monkeypatch, change, outcome): + ctx, path, _, params = setup_report + if change == "repeat": + _request(ctx, params) + elif change == "tampered": + params["sn"] = ["ALTERED"] + elif change == "missing_key": + ctx.device_public_key = lambda did: None + elif change == "unmatched": + path.write_text('{"devices": []}') + elif change == "write_failed": + monkeypatch.setattr(device_info, "write_inventory", lambda *args: False) + with caplog.at_level(logging.INFO, logger=device_info.__name__): + _request(ctx, params) + assert f"did=123 outcome={outcome} updated=" in caplog.text + assert params["sn"][0] not in caplog.text + assert params["signature"][0] not in caplog.text + + +@pytest.mark.parametrize("prefix", ["", "/.roborock.com"]) +def test_real_http_form_preserves_base64_signature(tmp_path, prefix): + from conftest import write_release_config + from fastapi.testclient import TestClient + from roborock_local_server.config import load_config, resolve_paths + from roborock_local_server.server import ReleaseSupervisor + + config_file = write_release_config(tmp_path) + config = load_config(config_file) + paths = resolve_paths(config_file, config) + fields = {"did": "123", "featureset": "123456789", "newfeatureset": "0000000AbCD", + "pid": "roborock.vacuum.a87", "sn": "TESTSERIAL"} + canonical = "&".join(f"{name}={value}" for name, value in fields.items()).encode() + # Select a synthetic key whose signature exercises both URL-escaped chars. + for _ in range(100): + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + signature = base64.b64encode(key.sign(canonical, padding.PKCS1v15(), hashes.SHA256())).decode() + if "+" in signature and "/" in signature: + break + else: + pytest.fail("Could not produce representative synthetic signature") + for path, payload in ( + (paths.inventory_path, {"devices": [{"did": "123", "duid": "cloud-a"}]}), + (paths.device_key_state_path, + {"devices": {"123": {"modulus_hex": format(key.public_key().public_numbers().n, "x")}}}), + ): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload)) + supervisor = ReleaseSupervisor(config=config, paths=paths) + body = urlencode({**fields, "signature": signature}) + assert "%2B" in body and "%2F" in body + with TestClient(supervisor.app) as client: + response = client.post(f"{prefix}/devices/123/info", content=body, + headers={"content-type": "application/x-www-form-urlencoded"}) + assert response.status_code == 200 + assert response.json() == catchall.build(supervisor.context, {}, {}, "/devices/123/info") + inventory = json.loads(paths.inventory_path.read_text()) + assert inventory["devices"][0]["sn"] == fields["sn"] + entries = [json.loads(line) for line in paths.http_jsonl_path.read_text().splitlines()] + assert entries[-1]["route"] == "device_info" From 7968fc6298c38be4d5278b2648942f880a943988 Mon Sep 17 00:00:00 2001 From: Luke Date: Mon, 12 Oct 2026 12:26:17 -0400 Subject: [PATCH 3/3] docs: remove firmware device info from reference navigation --- mkdocs.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/mkdocs.yml b/mkdocs.yml index 74f810d..877f758 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -57,7 +57,6 @@ nav: - Integrations: - Mobile App Options: roborock_app.md - Reference: - - Firmware Device Information: device-info.md - Known Limitations: known_limitations.md - Scene Validity Reports: scene_validity.md - Tested Vacuums: tested_vacuums.md