Skip to content

fix(commands): sign drive notes and calendar entries - #8

Merged
Bre77 merged 1 commit into
mainfrom
fm/nlib-signed-review-fixes
Jul 23, 2026
Merged

fix(commands): sign drive notes and calendar entries#8
Bre77 merged 1 commit into
mainfrom
fm/nlib-signed-review-fixes

Conversation

@Bre77

@Bre77 Bre77 commented Jul 23, 2026

Copy link
Copy Markdown
Member

Intent

Security follow-up on the merged signed-command layer (PR #5) of this published Tesla Fleet API library. Codex left findings on PR #5; the goal was to validate EACH against the Tesla vehicle-command protocol (teslamotors/vehicle-command protocol.md) and the merged code, then fix the genuinely-real ones with regression tests. Validation result: four findings (authenticate session-info tag before committing session state; accept VCSEC replies with an empty request_uuid; keep monotonic anti-replay counters on same-epoch resync; hold the VCSEC session lock across the whole dispatch) were confirmed real but ALREADY fixed in merged main (commits b8ae6d8/f756aa1) with existing regression tests, and verified protocol-correct - so no code change for those. The one still-open finding, fixed here: take_drivenote and upcoming_calendar_entries were inherited from VehicleSpecific and dispatched over the plaintext /command/... REST endpoint, silently bypassing /signed_command on vehicles that require the vehicle-command protocol. They are now overridden in Commands as signed Infotainment VehicleActions (takeDrivenoteAction / uiSetUpcomingCalendarEntries), matching python-tesla-fleet-api which now signs both. Added two regression tests (CapturingCommands) asserting each command goes out over the signed _send path carrying the correct protobuf field, updated the now-stale AGENTS.md note, and removed the stale 'intentionally not overridden' comment. A sixth Codex finding posted AFTER merge (undetailed VCSEC OPERATIONSTATUS_ERROR reported as success) was deliberately left unchanged: node matches the python reference there and the protocol's discard-and-wait guidance targets multi-message BLE, not one-shot cloud - flagged for a maintainer call rather than changed unilaterally. Full vitest suite (58 tests) passes and tsc --noEmit is clean.

What Changed

  • Route take_drivenote and upcoming_calendar_entries through signed Infotainment VehicleAction messages instead of the plaintext REST command path.
  • Add regression coverage verifying both commands use the signed _send path with the correct protobuf fields.
  • Update project guidance to reflect that both commands require signing.

Risk Assessment

✅ Low: The changes are narrowly scoped, preserve the existing public method signatures, route both commands through the correct signed Infotainment action path, and include focused regression coverage with no material issues identified.

Testing

The author-reported baseline included 58 passing tests and a clean typecheck; I independently ran the focused end-to-end HMAC/simulator regressions and the complete 58-test runtime suite, captured the signed-transport transcript, and found no failures. This is a non-UI library transport change, so no visual screenshot applies.

Evidence: Signed-command regression transcript

 RUN  v4.1.10 /home/brett/.no-mistakes/worktrees/c872ab22f4aa/01KY7CDK3EA70XWKVX1EH0HKBY

 ↓ test/commands.test.ts > Commands: constructor validation > rejects a VIN that is not 17 characters
 ↓ test/commands.test.ts > Commands: constructor validation > rejects construction with no private key available anywhere
 ↓ test/commands.test.ts > Commands: constructor validation > falls back to the parent Vehicle's private_key when none is passed explicitly, and uses it for real signing
 ↓ test/commands.test.ts > Commands: constructor validation > derives its own public key from the private key when none is passed
 ↓ test/commands.test.ts > Commands: session handshake > performs a handshake before the first command on a domain
 ↓ test/commands.test.ts > Commands: session handshake > does not re-handshake once a domain session is established
 ↓ test/commands.test.ts > Commands: session handshake > handshakes VCSEC and Infotainment domains independently
 ↓ test/commands.test.ts > Commands: session handshake > throws when the vehicle reports this key is not on its whitelist
 ↓ test/commands.test.ts > Commands: session handshake > accepts a real-VCSEC-style handshake reply that omits request_uuid entirely
 ↓ test/commands.test.ts > Commands: session handshake > still rejects a tampered tag even when request_uuid is omitted (the tag remains the real authentication)
 ↓ test/commands.test.ts > Commands: VCSEC dispatch serialization > delivers concurrent VCSEC commands to the vehicle in strict counter order
 ↓ test/commands.test.ts > Commands: session-info authentication (rejects tampered/spoofed handshake state) > rejects a handshake reply whose session_info_tag was tampered with
 ↓ test/commands.test.ts > Commands: session-info authentication (rejects tampered/spoofed handshake state) > does not adopt the tampered session's epoch/counter after rejecting it
 ↓ test/commands.test.ts > Commands: session-info authentication (rejects tampered/spoofed handshake state) > rejects a session-info reply replayed against a request it wasn't tagged for
 ↓ test/commands.test.ts > Commands: session-info authentication (rejects tampered/spoofed handshake state) > rejects a validly-signed but stale (rolled-back clock, same epoch) session-info resync
 ↓ test/commands.test.ts > Commands: session-info authentication (rejects tampered/spoofed handshake state) > never lowers the counter even when a later (authentic) session-info resync claims a smaller one
 ↓ test/commands.test.ts > Commands: HMAC round trip against an independent vehicle simulator > locks and unlocks the vehicle
 ↓ test/commands.test.ts > Commands: HMAC round trip against an independent vehicle simulator > increments the per-domain counter across successive commands
 ↓ test/commands.test.ts > Commands: HMAC round trip against an independent vehicle simulator > rejects a tampered command instead of silently succeeding
 ↓ test/commands.test.ts > Commands: HMAC round trip against an independent vehicle simulator > retries on a WAIT reply and eventually succeeds
 ↓ test/commands.test.ts > Commands: HMAC round trip against an independent vehicle simulator > gives up after exhausting WAIT retries
 ✓ test/commands.test.ts > Commands: drivenote and calendar entries are signed, not sent unsigned > signs take_drivenote as an Infotainment VehicleAction instead of falling through unsigned 12ms
 ✓ test/commands.test.ts > Commands: drivenote and calendar entries are signed, not sent unsigned > signs upcoming_calendar_entries as an Infotainment VehicleAction instead of falling through unsigned 2ms
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > charge_start / charge_stop / charge_max_range / charge_standard all succeed
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > actuate_truck builds a front or rear trunk closure request
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > window_control builds vent/close, not a REST-style command string
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > set_charge_limit, set_charging_amps, set_temps all succeed
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > remote_seat_heater_request rejects an unmapped seat position
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > remote_seat_heater_request accepts every seat Node's Seats enum defines
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > set_climate_keeper_mode accepts both string and numeric modes
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > set_cop_temp accepts the enum index and both unit aliases
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > rejects cross-transport gaps rather than silently dropping data
 ↓ test/commands.test.ts > Commands: protobuf construction for representative commands > navigation_sc_request without an order builds successfully

 Test Files  1 passed (1)
      Tests  2 passed | 31 skipped (33)
   Start at  21:41:56
   Duration  260ms (transform 91ms, setup 0ms, import 149ms, tests 16ms, environment 0ms)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Inspected git diff --unified=80 bae070e915d547a9f7ce09ae8e779b9e3d1c5be8..0cd6d15524faf05b3f254bfabda5b32cf9194eb6 -- src test package.json
  • pnpm install --ignore-scripts --frozen-lockfile
  • pnpm exec vitest run test/commands.test.ts -t 'Commands: drivenote and calendar entries are signed, not sent unsigned' --reporter=verbose
  • pnpm test
  • Repeated the focused command with output recorded via tee /tmp/no-mistakes-evidence/01KY7CDK3EA70XWKVX1EH0HKBY/signed-command-regressions.txt
  • Removed the transient node_modules installation and confirmed the worktree remained clean
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…through unsigned

Both were inherited from VehicleSpecific and dispatched over the plaintext
/command/... REST endpoint, silently bypassing /signed_command on vehicles
that require the vehicle-command protocol. Override them as signed
Infotainment VehicleActions (takeDrivenoteAction / uiSetUpcomingCalendarEntries),
matching python-tesla-fleet-api, with a regression test asserting each goes
out over the signed _send path carrying the right protobuf field.
@Bre77
Bre77 merged commit 78f36b7 into main Jul 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant