fix: zero password buffers and rename misleading APDU constant#932
Merged
matrix-agent116 merged 3 commits intoMay 20, 2026
Conversation
- Split LedgerAddressUtil.getTronAddress into getRawAddressResponse + parseTronAddress so callers can inspect APDU status words before parsing - Add LedgerPorts.AppNotOpenException for APDU 0x6511 (Tron app not open), distinct from a null return (device not found / address mismatch) - NonInteractiveLedgerSigner catches AppNotOpenException and returns APP_NOT_OPEN outcome instead of NOT_CONNECTED - ProductionLedgerPorts uses try-finally to unconditionally reset standardCliQuiet after executeSignListen returns, fixing permanent stdout suppression when device times out without a HID callback - AliasResolutionException overrides fillInStackTrace as a no-op to avoid unnecessary stack capture on control-flow exceptions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
On some OS/firmware combinations, HidDevice.open() returns false when the Tron app is not running, causing getLedgerHidDevice() to return null and the signer to report NOT_CONNECTED instead of APP_NOT_OPEN. - Add HidServicesWrapper.hasAnyLedgerAttached() to distinguish "no device" from "device present but not openable" - Throw AppNotOpenException in ProductionLedgerPorts when getHidDevice() returns null but a Ledger is physically attached - Also check the return value of the second device.open() call (B2 path) which was previously ignored, causing the same misclassification - Add test: returnsAppNotOpenWhenFinderThrowsAppNotOpenException Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ng APDU constant
- StdinPasswordReader.readAll wraps the read/parse in try-finally and
Arrays.fill the chunk and bytes buffers before returning, matching the
defensive pattern already used in StandardCliRunner.authenticate. The
returned String still holds the password in its own char[], but the
intermediate byte arrays no longer linger on the heap until GC.
- Rename APDU_APP_IS_OPEN to APDU_APP_NOT_OPEN. The Javadoc and the error
message ("Open the Tron app on your Ledger device") already treat 0x6511
as the "not open" signal; the identifier now matches.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
matrix-agent116
approved these changes
May 20, 2026
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
2 participants
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.
Summary
Addresses two code-review findings from PR #931:
StdinPasswordReader— wrap the read/parse intry-finallyandArrays.fillthechunkandbytesbuffers before returning, matching the existing defensive pattern inStandardCliRunner.authenticate()(added in PR feat: add standard CLI mode with JSON output for AI agents and automation #921). The returnedStringstill holds the password in its ownchar[]— a Java-language limitation — but the intermediate byte arrays no longer linger on the heap until GC.NonInteractiveLedgerSigner— renameAPDU_APP_IS_OPENtoAPDU_APP_NOT_OPEN. The Javadoc and the surfaced error message ("Open the Tron app on your Ledger device") already treat0x6511as the "app not open" signal; the identifier now reads in the same direction, removing a footgun for future maintainers.Test plan
./gradlew test)echo "$pw" | java -jar build/libs/wallet-cli.jar --password-stdin ...still authenticatesledger_app_not_open🤖 Generated with Claude Code