fix(ble): raise on rejected VCSEC whitelist operations in signed commands - #153
Merged
Merged
Conversation
…reply
A WhitelistOperation sent through the signed-command path reported
{"result": True} even when the vehicle rejected it (e.g.
NO_PERMISSION_TO_ADD), because the OPERATIONSTATUS_OK branch never
inspected commandStatus.whitelistOperationStatus. Reuse the same
WHITELIST_OPERATION_STATUS mapping bluetooth.py's
_raise_for_whitelist_reply already uses.
Bumps version to 1.14.1 (bug fix).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Found on 2026-09-20 while preparing the Charge On Solar pairing test the captain authorised: Commands._command()'s VCSEC reply decoding never reads commandStatus.whitelistOperationStatus, so a WhitelistOperation sent through the shared _sendVehicleSecurity() path returns {"result": True} even when the vehicle rejected it with NO_PERMISSION_TO_ADD. Verified on a synthetic FromVCSECMessage traced through the actual branches. The live test that day proved the vehicle really does answer WhitelistOperationNoPermissionToAdd (code 5) to a Fleet virtual key, so this false success is reachable in practice.
What Changed
Commands._command()(tesla_fleet_api/tesla/vehicle/commands.py) now decodescommandStatus.whitelistOperationStatus.whitelistOperationInformationon an otherwise-OK VCSEC reply, raising the mappedWhitelistOperationStatussubclass (e.g.WhitelistOperationNoPermissionToAdd) instead of returning{"result": True}when aWhitelistOperationwas actually rejected by the vehicle; an out-of-range status code raises a genericWhitelistOperationStatuswith the raw code.tests/test_vcsec_whitelist_status_decode.pycovering the new decode branch against syntheticFromVCSECMessagereplies.1.14.1(pyproject.toml,tesla_fleet_api/__init__.py,uv.lock).Risk Assessment
✅ Low: The fix is narrowly scoped to the exact reported bug (decoding whitelistOperationStatus in the shared Commands._command VCSEC reply path), is verified to fail before the fix and pass after it, correctly defaults to a no-op for non-whitelist replies (whitelistOperationInformation defaults to 0), gracefully falls back for undocumented info codes not in the exception table, doesn't disturb locking/retry/logging chokepoints, and follows the project's patch-release convention (version bump + uv.lock).
Testing
The new test tests/test_vcsec_whitelist_status_decode.py demonstrates the exact defect from the intent — a synthetic FromVCSECMessage with outer OPERATIONSTATUS_OK but whitelistOperationInformation=NO_PERMISSION_TO_ADD driven through the real Commands._command()/MockedBleTransportTestCase path — and I confirmed it is a genuine regression test by reverting only commands.py to its pre-fix state and observing the test fail with the reported false-success behavior, then restoring the fix and rerunning alongside adjacent whitelist/session-auth/command-logging/proto-coverage tests, all of which pass with a clean worktree.
Evidence: Regression proof: new test fails against pre-fix commands.py, passes against the fix
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tesla_fleet_api/tesla/vehicle/commands.py:793- The new whitelist-status decode/raise logic in Commands._command (tesla_fleet_api/tesla/vehicle/commands.py:793-802) is byte-for-byte identical to the pre-existing _raise_for_whitelist_reply helper in tesla_fleet_api/tesla/vehicle/bluetooth.py:1622-1639 (same info-lookup, same bounds check, same WHITELIST_OPERATION_STATUS indexing, same fallback exception). Both are reachable on the same Commands/VehicleBluetooth class hierarchy, so this could be a single shared helper (e.g. a method on Commands taking a CommandStatus) called from both _command and pair()'s _raise_for_whitelist_reply, avoiding two places to keep in sync if the mapping ever changes.✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_vcsec_whitelist_status_decode.py -vat target commit 52506d3 — both new tests passReverted only tesla_fleet_api/tesla/vehicle/commands.py to its cf3bfa9 (pre-fix) content viagit checkout cf3bfa9 -- tesla_fleet_api/tesla/vehicle/commands.py, reran the same test — test_rejected_whitelist_operation_raises FAILED with the exact reported symptom (no exception raised, would have returned {"result": True}), proving this is a true regression testRestored commands.py viagit checkout HEAD -- tesla_fleet_api/tesla/vehicle/commands.py, confirmedgit status --shortcleanuv run pytest tests/test_vcsec_whitelist_status_decode.py tests/test_ble_pair.py tests/test_is_key_rejected.py tests/test_session_info_authentication.py tests/test_ble_null_key.py tests/test_proto_coverage_lock.py tests/test_command_logging.py -v— 72 passed, covering the adjacent whitelist/session/logging/coverage-lock surfaces the fix touches✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.