Skip to content

feat(adapters): reject outdated Ledger firmware before signing - #837

Open
woahwhattheheck wants to merge 2 commits into
Stellar-split:mainfrom
woahwhattheheck:feat/775-ledger-firmware-gate
Open

feat(adapters): reject outdated Ledger firmware before signing#837
woahwhattheheck wants to merge 2 commits into
Stellar-split:mainfrom
woahwhattheheck:feat/775-ledger-firmware-gate

Conversation

@woahwhattheheck

@woahwhattheheck woahwhattheheck commented Sep 6, 2026

Copy link
Copy Markdown

Closes #775.

LedgerAdapter signed without checking the device's Stellar app version, so
an outdated build failed part-way through signing with an opaque device error
rather than a message saying what to do about it.

Changes

src/adapters/ledger.ts:

  • MIN_LEDGER_FIRMWARE — exported, "3.0.0".
  • LedgerFirmwareTooOldError — carries foundVersion and
    requiredVersion, and names both in the message.
  • The version is read via getAppConfiguration() before the signing
    request is sent, so an unsupported device is rejected up front.
  • skipFirmwareCheck constructor flag to bypass the check.
  • compareLedgerVersions — exported, so callers can gate their own UI on
    the same comparison.

Constructor stays backward compatible

The adapter previously took a bare path string. It now accepts
string | LedgerAdapterOptions, so new LedgerAdapter("44'/148'/0'")
continues to work unchanged; the options form adds skipFirmwareCheck and
lets the transport and app bindings be injected, which is what makes the gate
testable without a physical device.

Two details

  • Version comparison is numeric, not lexical. "3.10.0" is newer than
    "3.9.0", which a string comparison gets backwards. Missing trailing
    components count as zero, so "3.0", "3" and "3.0.0" all compare equal.
  • It fails closed. A version string that cannot be parsed is treated as
    unsupported rather than waved through. Signing on a device whose
    compatibility could not be established is exactly the outcome this check
    exists to prevent, so an unrecognised value rejects with the same error,
    quoting what the device reported.

getAddress() is deliberately not gated — it does not sign.

Also fixes a pre-existing typecheck error in this file

ledger.ts imported WalletAdapter from ../types.js — the rich wallet
interface requiring name, connect, disconnect and onAccountChange
while the sibling adapters import the two-method adapter interface from
./types.js. That produced a standing Class 'LedgerAdapter' incorrectly implements interface 'WalletAdapter' error. Corrected to ./types.js, which
matches walletconnect.ts and what the class actually implements.

Validation

  • npx vitest run test/ledger.firmware.test.ts — 20/20 passing. Covers
    newer/equal/older firmware, the signing request not being sent when the gate
    rejects, both versions named on the error, the unparseable-version path, the
    transport still being closed when the gate rejects, skipFirmwareCheck,
    getAddress() staying ungated, the bare-string constructor, the default
    derivation path, and the comparator itself (numeric ordering, zero-padded
    components, unparseable inputs).
  • Offline: the transport and Stellar app are stubs, so no device is needed.
  • npx tsc --noEmit reports 209 errors on this branch against 210 on
    unmodified main
    — one fewer, and src/adapters/ledger.ts now reports
    none. The remaining 209 are pre-existing and untouched by this change.

Two notes on the version comparison

A pre-release version is rejected. parseVersion requires every dot-separated
component to be digits, so "3.0.0-rc1" returns null and the gate treats it as
unsupported. That is deliberate — the check exists to refuse a device whose
compatibility cannot be established, and an unrecognised string is exactly that
case. Flagging it explicitly because if Ledger ever ships RC builds to users it
becomes a policy question rather than a parsing one, and skipFirmwareCheck is
the intended escape hatch meanwhile.

The comparison logic is duplicated with #838. That PR adds an equivalent
parseVersion + compare pair for the xBull extension in
src/wallets/adapters/XBullAdapter.ts. I kept them separate so neither PR
depends on the other landing, but if both merge the SDK carries two copies of
the same logic that can drift. Happy to follow up with a shared helper in
src/utils/ once the second one lands, or to fold it into whichever of the two
you take first if you would rather not merge the duplication at all.

The adapter signed without checking the device Stellar app version, so an
outdated build failed part-way through with an opaque device error. The
version is now read via getAppConfiguration() before the signing request and
compared against MIN_LEDGER_FIRMWARE, rejecting with LedgerFirmwareTooOldError
naming both the version found and the version required.

Also fixes a pre-existing typecheck error in this file: WalletAdapter was
imported from ../types.js, the rich wallet interface requiring name/connect/
disconnect/onAccountChange, rather than the two-method adapter interface in
./types.js that the sibling adapters use.
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.

Check Ledger firmware version before attempting to sign

1 participant