fix(auth): return specific status codes from wallet-session, not opaque 500#325
Open
QSchlegel wants to merge 1 commit into
Open
fix(auth): return specific status codes from wallet-session, not opaque 500#325QSchlegel wants to merge 1 commit into
QSchlegel wants to merge 1 commit into
Conversation
…ue 500 The handler wrapped everything in one try/catch that returned 500 for ANY thrown error. checkSignature() from @meshsdk/core-cst THROWS (not just returns false) on malformed COSE or a non-bech32/hex address — e.g. Address.fromBech32() on hex raises `Unknown letter "b"`. Every such case surfaced as an opaque 500, hiding the real cause from the client toast and making login bugs hard to diagnose. - Normalize the address to bech32 up front (defense in depth) before the nonce lookup and signature check. - Wrap checkSignature in its own try/catch: a throw is logged (console.warn) and treated as 401 Invalid signature, never a 500. - Keep 400 for missing/malformed input and no-nonce, 401 for invalid signature; 500 is now reserved for genuine server faults (e.g. DB errors). Adds src/__tests__/walletSessionApi.test.ts covering the 400/401/200/500 paths, including the regression that a throwing checkSignature now yields 401, not 500. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Problem
/api/auth/wallet-sessionwrapped its whole body in onetry/catchthat returned a generic500 Internal Server Errorfor any thrown error. ButcheckSignature()from@meshsdk/core-cstthrows (rather than returningfalse) for several inputs — most notably a non-bech32 / hex-encodedaddress, where it internally callsAddress.fromBech32(address)and raisesUnknown letter "b". Allowed: qpzry9x8gf2tvdw0s3jn54khce6mua7l, as well as malformed COSE_Sign1 / COSE_Key bytes.Every such case surfaced as an opaque 500, so the client (the
WalletAuthModaltoast) only ever showed a generic failure — which made a recent login bug much harder to diagnose.Fix
normalizeAddressToBech32) before the nonce lookup and signature check — defense in depth; the client already normalizes, but other callers may not.checkSignaturein its owntry/catch: a throw is logged viaconsole.warn(so genuine failures stay diagnosable) and treated as401 { error: "Invalid signature" }, never a 500.400for missing/malformed input and no-nonce,401for an invalid (or throwing) signature,200 {ok:true}for success — and500is now reserved for genuine server faults (e.g. DB errors).Tests
Adds
src/__tests__/walletSessionApi.test.ts(6 cases, all passing) covering 400 / 401 / 200 / 500 paths — including the regression: a throwingcheckSignaturenow yields 401, not 500, and the nonce is not consumed.tsc --noEmit: changed files clean, no new type errors.🤖 Generated with Claude Code