You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat(ble): allow private_key=False to disable command signing (#137)
* feat(ble): allow explicit null private_key to disable signing
A passive BLE listener that only decodes broadcasts has no reason to
generate or load a key it will never use. Commands.__init__ (and
VehicleBluetooth/vehicles.create*'s key argument) now distinguish an
omitted key from an explicit None via a sentinel default: omitting it
keeps today's fallback-to-parent-or-raise behaviour, while passing
None explicitly disables signing. Any operation that needs a signed
session raises a clear SigningDisabled instead of failing deep in the
crypto path.
* no-mistakes(review): Raise SigningDisabled up front in pair() when private_key is None
* no-mistakes(document): docs: correct AGENTS.md choke-point claim after pair() guard fix
* fix(ble): use False, not None, to disable command signing
An explicit `private_key=None` used to fall through to the parent's key and
raise `ValueError("No private key.")` when there was none. The sentinel
design in this PR changed that: `None` became the opt-out, so a caller who
wrote `private_key=None` meaning "I haven't got one" silently got a vehicle
that could not sign instead of the error that told them so. That is a
behaviour change on a published, security-adjacent library, not an additive
one.
Drop `KeyOmitted`/`KEY_OMITTED` and restore `None` as the default. `False` is
now the explicit "signing is disabled" value - a value no current caller can
already be passing, so opting out has to be deliberate, and `None` keeps its
existing meaning for both omitted and explicit-`None` callers.
Because `False` and `None` are both falsy, the constructor branches on
identity (`is False` / `is not None`); a truthiness check would collapse the
two states and reintroduce the bug in a new form.
`Vehicles.createBluetooth` had no `key` parameter at all, so the opt-out was
unreachable from the Fleet-parented factory; it gains one as keyword-only
(no positional shift), with the matching `NotImplementedError` overrides in
Teslemetry/Tessie kept in step with their "parameters match the Fleet API
Bluetooth factory" contract.
The `SigningDisabled` guards at `_handshake` and in `pair()`'s fast path are
unchanged.
Teslemetry/Tessie override `Vehicles` with their own vehicle classes (`TeslemetryVehicle`, `TessieVehicle`) extending `VehicleFleet` with service-specific commands (e.g., `closure()`, `seat_heater()` for Teslemetry; `wake()`, `lock()` for Tessie).
84
84
@@ -153,6 +153,7 @@ Keep the `tesla-protocol` floor at `>=0.5.0`; earlier releases have generated `.
153
153
- **`VehicleAction`/`GetVehicleData` proto coverage is locked by test, not just by convention**: `tests/test_proto_coverage_lock.py` walks both descriptors and fails if any field has no wrapper (`commands.py`) or reader (`bluetooth.py`) and isn't on one of its two small, reasoned allowlists — keep that test in sync with any future `tesla-protocol` bump rather than special-casing new fields elsewhere. The only fields deliberately left unwrapped today are the 7-field push-style subscription/streaming family (`createStreamSession`/`streamMessage`/`vehicleDataSubscription`/`vehicleDataAck`/`vitalsSubscription`/`vitalsAck`/`cancelVehicleDataSubscription`, which need a public lifecycle/iterator API atop the private `_stream_sinks` routing above) and `getVehicleImageState` (needs chunked binary-transfer paging). CarServer's `GetVehicleState` sub-state is exposed as `legacy_vehicle_state()` (`bluetooth.py`), matching the `VehicleData.legacy_vehicle_state` reply field name, to avoid confusion with `vehicle_state()` (VCSEC `VehicleStatus`, a different message/domain). `set_rate_tariff`/`add_managed_charging_site` (`commands.py`) take `tesla_protocol` message types directly for their deeply-nested arguments rather than a parallel flattened dataclass API.
154
154
-**Energy-gateway authorized-client pairing has security- and protocol-specific constraints**: use RSA for LAN TEDapi v1r, treat `PENDING_VERIFICATION_TIMEOUT` as terminal, and account for presence-free key removal. The authoritative pairing, retry, encoding, and removal guidance is in `docs/energy_local_control.md`; enum values and API contracts live in `const.py` and the relevant method docstrings.
155
155
-**`register_client()` (`teslemetry/teslemetry.py`) is Teslemetry-only OAuth Dynamic Client Registration (RFC 7591)**: a module-level function, not a `Teslemetry` instance method, since registration precedes having a `client_id` or access token — callers pass a bare `aiohttp.ClientSession`. It always registers a new client (no dedup/caching) and raises `TeslemetryRegistrationError` (`exceptions.py`) on transport failure, a non-2xx response, a non-JSON body, or a response missing a usable `client_id`; a non-dict-but-valid-JSON body (list/scalar) is treated as the same malformed-response error rather than raising an uncaught `AttributeError`. Fleet API and Tessie have no equivalent — don't add one speculatively. See `docs/teslemetry.md`'s "OAuth Dynamic Client Registration" section and `tests/test_teslemetry_register_client.py`.
156
+
- **`False`, not `None`, is the "signing is disabled" value for `Commands.__init__`'s `private_key` (and `VehicleBluetooth.__init__`/`Vehicles.createBluetooth`/`VehiclesBluetooth.create`/`createBluetooth`'s `key`)**: `None` — the default and an explicit `None` — keeps its long-standing meaning of falling back to the parent's key, raising `ValueError("No private key.")` if it has none; `False` disables signing for a passive BLE listener that only observes broadcasts. `None` is deliberately *not* the opt-out: a caller already passing `private_key=None` to mean "I haven't got one" must keep getting that `ValueError`, not a silently unsignable vehicle. Because `False` and `None` are both falsy, every branch on this argument must test **identity** (`is False`/`is not None`) — a truthiness check (`if private_key:`) collapses the two states and reintroduces the bug. `self.private_key` is `EllipticCurvePrivateKey | None`, its `None` meaning signing-disabled — `_handshake` (reached by `_command`, i.e. every signed command, and by `_ensure_handshake`, used by signed reads) raises `SigningDisabled` (`exceptions.py`) up front rather than failing deep in the signing/crypto path. `pair()`'s fast path never calls `_handshake` (it builds and sends its own whitelist request directly), so it carries its own identical guard at the top instead — `_handshake` is not a single choke point every signed-session entry point routes through; each entry point that doesn't call it needs its own `self.private_key is None` check. Tests: `tests/test_ble_null_key.py`.
0 commit comments