fix(vehicle): clear_pin_to_drive_admin now clears PIN-to-Drive, not speed limit#86
Merged
Conversation
added 3 commits
July 21, 2026 20:00
…peed limit Live-verified over BLE: the command built DrivingClearSpeedLimitPinAction (the Speed-Limit-Mode pin clear) instead of a PIN-to-Drive action - the vehicle itself rejected a live call with reason "speed_limit_mode_active", a condition meaningful only to Speed Limit Mode. Now builds the correct VehicleControlResetPinToDriveAdminAction, matching the method's own docstring and the cloud REST endpoint's semantics. That action has no pin field and requires Tesla account credentials, so it always fails over raw BLE signing with TeslaFleetMessageFaultCommandRequiresAccountCredentials - expected, not a regression (points callers at the Fleet-API-relayed path).
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
Fix a live-verified, security-sensitive BLE bug: clear_pin_to_drive_admin (tesla_fleet_api/tesla/vehicle/commands.py) was documented as clearing PIN-to-Drive but actually built DrivingClearSpeedLimitPinAction - the Speed-Limit-Mode pin clear, a completely different vehicle feature. This was flagged as suspected (not yet proven) in an earlier cross-transport parity audit (capstone F1). The captain explicitly authorized a live, physical experiment on his own garaged vehicle tonight to verify it: set both a temporary PIN-to-Drive password and a temporary Speed-Limit-Mode PIN via BLE, fire clear_pin_to_drive_admin once, and observe the result via an independent cloud telemetry channel (Teslemetry Fleet Telemetry), with a mandatory both-pins-cleared cleanup contract regardless of outcome. The vehicle itself rejected the live call with reason 'speed_limit_mode_active' - a rejection reason meaningful only to Speed Limit Mode, not PIN-to-Drive - which is decisive, vehicle-confirmed proof of the mismapping. (Two earlier attempts this session were blocked/inconclusive on unrelated BLE transport flakiness - reconnect churn and transient GATT errors - before this held-BLE-session run got a clean, confirmed rejection from the car.) The fix changes the built action to VehicleControlResetPinToDriveAdminAction (the action that actually matches the method's own docstring and the cloud REST endpoint's semantics), keeping the pin parameter only for cross-transport signature parity with the cloud path (the correct action has no pin field) - this mirrors an existing documented pattern in this codebase for other cross-transport form gaps (e.g. set_scheduled_departure's dead preconditioning args). A second live finding surfaced during cleanup and is documented in the fix: the corrected action requires Tesla account/OAuth credentials, so calling it over raw local BLE signing (VehicleBluetooth, no OAuth session) will always raise TeslaFleetMessageFaultCommandRequiresAccountCredentials - this is intentional, correct, vehicle-enforced behavior (not a regression to avoid), since the same signed action relayed through the Fleet-API-authenticated VehicleSigned cloud path can supply that proof. Added regression tests: a BLE-mocked test asserting the corrected proto action is built and the old speed-limit action is not, a sibling regression test pinning that speed_limit_clear_pin still builds DrivingClearSpeedLimitPinAction (so the two commands stay decoupled going forward), and a cross-transport parity test documenting the now-accepted form gap (cloud still sends pin in the REST body, BLE ignores it). All local checks pass: ruff, pyright --strict, and the full pytest suite (371 tests).
What Changed
clear_pin_to_drive_admin(tesla_fleet_api/tesla/vehicle/commands.py) now delegates to the existingreset_pin_to_drive_admin, building the signedVehicleControlResetPinToDriveAdminAction, instead of the previously mismappedDrivingClearSpeedLimitPinAction(Speed-Limit-Mode's PIN clear, a different vehicle feature). Thepinparam is kept only for cross-transport signature parity with the cloud REST endpoint, since the corrected action has no pin field; the docstring documents this gap plus that the action requires Tesla account/OAuth credentials and will raiseTeslaFleetMessageFaultCommandRequiresAccountCredentialsover raw BLE signing.clear_pin_to_drive_admin, a sibling test pinning thatspeed_limit_clear_pinstill buildsDrivingClearSpeedLimitPinAction, and a cross-transport parity test documenting the accepted form gap (cloud sendspinin the REST body, BLE ignores it).CLAUDE.md's cross-transport-parity note to reflect the fix instead of describing the mismapping as still open, and made a minor formatting fix in the new BLE test.Risk Assessment
✅ Low: Both previously reported findings (duplicate action construction, stale CLAUDE.md/AGENTS.md parity note) are cleanly resolved with minimal, non-functional changes; clear_pin_to_drive_admin now delegates to reset_pin_to_drive_admin() and the doc note accurately reflects the fix, with no new issues introduced.
Testing
Ran the new BLE-mocked and cross-transport regression tests plus the full 371-test suite (all passing), and additionally drove clear_pin_to_drive_admin through the real mocked-BLE signed-command build/encrypt/decrypt path in a standalone script to directly observe the wire-level protobuf action: it now sends
vehicleControlResetPinToDriveAdminAction(correct) rather than the old mismappeddrivingClearSpeedLimitPinAction, whilespeed_limit_clear_pinstill independently sendsdrivingClearSpeedLimitPinAction, confirming the fix and the decoupling the intent describes. No findings.Evidence: CLI transcript: clear_pin_to_drive_admin wire-level protobuf action (after fix)
clear_pin_to_drive_admin(pin='1234') -> reply: {'response': {'result': True, 'reason': ''}} protobuf action actually sent over BLE: 'vehicleControlResetPinToDriveAdminAction' => CORRECT: builds the PIN-to-Drive admin reset action.Evidence: Demo script used to produce the transcript (drives the real mocked-BLE signed-command path)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
CLAUDE.md:142- CLAUDE.md's cross-transport-parity entry (line 142) still describes clear_pin_to_drive_admin's speed-limit mismapping as 'One open divergence left unfixed pending live verification', but this PR fixes exactly that divergence and also introduces a new, now-accepted form gap (BLE ignores pin, cloud still sends it) that isn't reflected there either. A future agent relying on this doc will think the bug is still open.tesla_fleet_api/tesla/vehicle/commands.py:1009- clear_pin_to_drive_admin (line 990) now builds the exact same empty VehicleControlResetPinToDriveAdminAction as the pre-existing reset_pin_to_drive_admin (line 2366/2371), making them duplicate implementations of the same signed command with no shared code path. Not a functional bug, but a simplification opportunity (e.g. have one delegate to the other).🔧 Fix: Dedupe pin-reset action and refresh stale CLAUDE.md parity note
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_ble_mocked_commands.py -k "ClearPinToDriveAdmin or SpeedLimitClearPin"— 3 passeduv run pytest tests/test_cross_transport_parity.py -k ClearPinToDriveAdmin— 1 passeduv run pytest tests— full suite, 371 passedManual CLI demo: drovevehicle.clear_pin_to_drive_admin(pin='1234')through the mocked-BLE signed-command harness (real sign/encrypt/decrypt path, no live vehicle) and decoded the actual protobufActionplaced on the wire✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.