Return 400 for malformed affiliate payout JSON#457
Conversation
Greptile SummaryThis PR fixes a regression from #169 where a malformed JSON request body would bypass the
Confidence Score: 4/5Safe to merge; the core fix is correct and the new test verifies no DB calls occur on malformed input. The JSON-parsing fix is straightforward and well-tested. The two items worth a second look are: the stale mockFrom setup in the existing conversion_id test (misleads readers about control flow) and the reordering of validation before the ownership gate (authenticated non-owners now see 400 for bad input rather than 403, exposing API contract details they shouldn't need to know). Neither affects data integrity or access control, but the ordering change is a deliberate behavior difference that deserves a comment or intentional design decision. Both changed files are worth a quick look: route.ts for the validation-before-authorization ordering, and route.test.ts for the now-unreachable affiliate_offers mock in the non-string conversion_id test case. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Route as POST /conversions/pay
participant Auth as getAuthContext
participant DB as Supabase (admin)
participant LN as Lightning (wallet-utils)
Client->>Route: "POST {body}"
Route->>Auth: getAuthContext(request)
Auth-->>Route: auth / null
alt No auth
Route-->>Client: 401 Unauthorized
end
Note over Route: NEW: parse body here
alt Malformed JSON
Route-->>Client: 400 Invalid JSON body
end
alt invalid / missing conversion_id
Route-->>Client: 400 conversion_id must be non-empty string
end
Route->>DB: affiliate_offers → select seller_id
alt Not owner
Route-->>Client: 403 Not authorized
end
Route->>DB: affiliate_conversions → select conversion
alt Not found / already paid / clawed back / unsettled / zero commission
Route-->>Client: 400 / 404
end
Route->>LN: getUserLnWallet(seller)
Route->>LN: getUserLnWallet(affiliate)
Route->>LN: internalTransfer(seller→affiliate)
Route->>DB: "update affiliate_conversions (status=paid)"
Route->>DB: insert wallet_transactions (x2)
Route-->>Client: "200 { ok: true, commission_sats }"
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let body: unknown; | ||
| try { | ||
| body = await request.json(); | ||
| } catch { | ||
| return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); | ||
| } | ||
| const conversion_id = | ||
| body && typeof body === "object" && !Array.isArray(body) | ||
| ? (body as Record<string, unknown>).conversion_id | ||
| : undefined; | ||
|
|
||
| if (typeof conversion_id !== "string" || conversion_id.trim() === "") { | ||
| return NextResponse.json( | ||
| { error: "conversion_id must be a non-empty string" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
| const conversionId = conversion_id.trim(); |
There was a problem hiding this comment.
Moving body parsing and conversion_id validation above the ownership check changes the observable behavior for non-owners. Previously any authenticated user who didn't own the offer received 403 regardless of body content. Now they receive 400 for malformed JSON or an invalid conversion_id, and only reach 403 when the JSON is well-formed with a string conversion_id. The conventional order (authenticate → authorize → validate) avoids leaking API contract details to callers who have no business touching the resource. The fix is still correct for the stated goal, but the ordering could be tightened by moving body parsing after the offer.seller_id ownership gate — or at minimum noting the trade-off.
|
CI is green for PR #457. Verification:
uGig invoice evidence has been sent for this PR. |
2 participants
Follow-up for #169.
Summary
Verification
vitest run src/app/api/affiliates/offers/[id]/conversions/pay/route.test.tstsc --noEmit