Skip to content

fix(ble): raise on rejected VCSEC whitelist operations in signed commands - #153

Merged
Bre77 merged 1 commit into
mainfrom
fm/pytfa-vcsec-whitelist-status-decode
Sep 20, 2026
Merged

Bre77 merged 1 commit into
mainfrom
fm/pytfa-vcsec-whitelist-status-decode

Conversation

@Bre77

@Bre77 Bre77 commented Sep 20, 2026

Copy link
Copy Markdown
Member

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 decodes commandStatus.whitelistOperationStatus.whitelistOperationInformation on an otherwise-OK VCSEC reply, raising the mapped WhitelistOperationStatus subclass (e.g. WhitelistOperationNoPermissionToAdd) instead of returning {"result": True} when a WhitelistOperation was actually rejected by the vehicle; an out-of-range status code raises a generic WhitelistOperationStatus with the raw code.
  • Adds tests/test_vcsec_whitelist_status_decode.py covering the new decode branch against synthetic FromVCSECMessage replies.
  • Bumps package version to 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
tests/test_vcsec_whitelist_status_decode.py::test_rejected_whitelist_operation_raises

Against cf3bfa9's commands.py (pre-fix, reverted in-place then restored):
FAILED - AssertionError: WhitelistOperationNoPermissionToAdd not raised
(i.e. _command() returned {"result": True} for a vehicle-rejected WhitelistOperation, exactly the false-success reported)

Against 52506d3's commands.py (the fix):
PASSED - raises WhitelistOperationNoPermissionToAdd as expected
test_accepted_whitelist_operation_succeeds also PASSED (status 0 still returns {"result": True})

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ℹ️ 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 -v at target commit 52506d3 — both new tests pass
  • Reverted only tesla_fleet_api/tesla/vehicle/commands.py to its cf3bfa9 (pre-fix) content via git 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 test
  • Restored commands.py via git checkout HEAD -- tesla_fleet_api/tesla/vehicle/commands.py, confirmed git status --short clean
  • uv 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.

…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).
@Bre77 Bre77 added the fm Opened by a Firstmate crewmate label Sep 20, 2026
@Bre77
Bre77 merged commit c0618e0 into main Sep 20, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fm Opened by a Firstmate crewmate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant