Conversation
The request parameter of an MPP challenge went straight to JSON.parse, so a challenge whose request was not base64url-encoded JSON escaped parseMppChallenge as an uncaught SyntaxError and exited 1 under E_RUNTIME, while every other invalid-challenge check raises E_USAGE and exits 2. A request that decoded to valid JSON that was not an object was also accepted, degrading to an empty record and failing later on a missing currency.
This branch has not been deployed
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.
parseMppChallengehands therequestparameter of aWWW-Authenticate: Payment …challenge straight toJSON.parse:The value comes from whatever server answered with 402, so it is not trusted input. When it is not base64url-encoded JSON, the
SyntaxErrorescapestransferCreditsuncaught andprintCompatErrorAndExitfalls back to its defaults —E_RUNTIME, exit code 1 — with a raw parser message:Every other malformed-challenge condition in that function raises
usageError, so a caller that branches on exit code 2 orE_USAGEto tell "the server sent me a bad challenge" from "the CLI broke" gets the wrong answer here.Buffer.from(value, "base64url")does not reject invalid characters either, sorequest="!!!!"decodes to empty and producesUnexpected end of JSON inputthe same way.A
requestthat parses but is not an object has the opposite problem.getRecordturnsnull,[]or5into{}, which is truthy, so the existingif (!request)guard cannot catch it — only a completely absent parameter reaches it. The challenge is accepted with an empty request and fails several checks later with a message that points at the wrong field:Fix
Decode the parameter through a local helper that returns
nullfor unparseable JSON and for JSON that is not an object, and raiseusageErroron it. The absent-parameter check moves ahead of the decode so it keeps its existing "Missing request parameter." message, and the new case reports "Malformed request parameter."decodeBase64UrlJsonitself is unchanged. Its other caller,storedSessionChallengeinsrc/commands/sessions.ts, already wraps the decode intry/catchand reads from the local channel store rather than from the network.Tests
Adds four cases to
test/transfer.test.ts, alongside the existing MPP challenge validation tests:requestthat is not JSON,requestthat is not base64url,requestthat decodes tonull/[]/5/"amount", and a challenge carrying norequestat all.The first three fail on
main— the first two as an uncaughtSyntaxError, the third on the misleading currency message. The fourth passes either way and is there to pin the untouched "Missing request parameter." path.Validation
vitest run test/transfer.test.ts— 14 passed, up from 10.pnpm typecheck,pnpm test:types,pnpm check:lintandpnpm changelog:validateare clean.The full suite is not a usable signal on my machine: 7 of its 19 files fail on an unmodified
main—spawn sqlite3 ENOENT, since sqlite3 is not available here and I cannot install it, plus 10s timeouts — and the same 7 fail identically on this branch (37 failed / 208 passed both ways, 249 tests on the branch against 245 onmain). One branch run timed out in three further files,mcp-services,request-ioandwallet-readiness; all three pass when run on their own on this branch, and none of them touchsrc/commands/transfer.ts.pnpm check:formatalso fails onmain, on.agents/friction-log/20260831180632-immutable-releases-were/friction.md, which this branch does not touch.