fix(flashcard): stop logging withdraw parameters from NFC reads - #721
Open
islandbitcoin wants to merge 2 commits into
Open
islandbitcoin wants to merge 2 commits into
islandbitcoin wants to merge 2 commits into
Conversation
The Flashcard context logged the LNURL-withdraw k1 and callback on every payment tap, the card's LNURL on every balance read, and the raw error object (which for axios embeds the request URL with the card's p/c parameters) on failure. Together those values authorise a withdrawal from the card, and nothing strips console output from release builds. Remove the debug dumps, log only the error message on failure, and add a unit guard that drives both tap paths through the provider and asserts the parameters land in state but never reach any console method. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review on #721: an Error message or a thrown string can itself contain the card's withdraw URL, so passing message text to console.warn still leaked it. Replace the helper with one that emits only the error class name (or typeof for non-Errors) and, when present, the numeric HTTP status. Cover both failure paths with synthetic parameter-bearing messages: an axios-style Error with the URL in message and config plus a 500 status, an Error from the lnurl lookup, and a thrown string. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Why
app/contexts/Flashcard.tsxlogged the LNURL-withdrawk1andcallbackon every payment tap, the card's LNURL on every balance read, and the raw error object on failure. An axios error embeds the request URL, which carries the card'sp/cparameters. Together those values authorise a withdrawal from the card, and there is notransform-remove-consoleinbabel.config.js, so they ship in release builds too.What
errorMessagehelper instead of the whole error.__tests__/contexts/flashcard-no-withdraw-logging.spec.tsx: drives both tap paths (payment and balance) and the fetch-failure path throughFlashcardProviderwith the NFC, lnurl and axios boundaries mocked, and asserts the parameters land in context state but never reach any console method.No payment, custody, or NFC behaviour changes. No dependency changes. No native changes.
Adjacent logging reviewed
console.warn("Oops!", ex)at the top ofhandleTagcatchesNfcManagererrors, which do not carry card parameters, so it is left alone. The redeem-lnurl-withdrawal screens log the LNURL server response (status/reason) only. Out of scope but worth a follow-up: the error toast ingetPayDetailsrenders the rawlnurlwpayload on screen.Validation
yarn check-code(tsc, translations, codegen, graphql) passes.yarn test: 97 suites, 962 tests pass.Flashcard.tsxare pre-existing lines this PR does not touch.Buzz issue:
461d8d90cfa90173cf2de35e582cfd222a90e6719dcad5287094ea65dac6d479🤖 Generated with Claude Code