Conversation
Next.js turns a repeated query key into an array. parseAddress called string methods on that value, so includes() checked element membership instead of a substring. Non-string addresses are now rejected. Co-authored-by: David <Klakurka@users.noreply.github.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Balance and transaction-count endpoints convert the new validation error into HTTP 500 instead of 400.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Updates address validation to reject repeated query parameters represented as arrays.
Changes:
- Expands
parseAddressinput handling to reject arrays. - Adds a unit test for repeated address parameters.
| File | Description |
|---|---|
utils/validators.ts |
Rejects array-valued addresses. |
tests/unittests/validators.test.ts |
Tests array rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export const parseAddress = function (addressString: string | string[] | undefined): string { | ||
| // Repeated query keys arrive as arrays. String methods like includes() then | ||
| // check membership instead of a substring, so reject non-strings before use. | ||
| if (Array.isArray(addressString)) throw new Error(RESPONSE_MESSAGES.INVALID_ADDRESS_400.message) |
There was a problem hiding this comment.
Added INVALID_ADDRESS_400 handling to both routes and added repeated-address endpoint response tests in commit 83d123a3.
…preserve parseAddress type coverage' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Klakurka <4713204+Klakurka@users.noreply.github.com>
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/3
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/6
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/7
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/8
Description
CodeQL
js/type-confusion-through-parameter-tamperingflaggedparseAddress. The address routes passreq.query.addressthrough, and a repeatedaddressquery key is an array.includes(':')on an array does not search for a substring.parseAddressnow rejects arrays with the invalid-address error before any string method runs. The balance, transaction, and count routes all go through this function.The GitHub token for this run cannot read code-scanning alert bodies, so this is one of the fixes from a local CodeQL code-scanning run of the same default query suite.
Test plan
npx ts-node -O '{"module":"commonjs"}' node_modules/jest/bin/jest.js tests/unittests/validators.test.ts --forceExitINVALID_ADDRESS_400. Existing address parsing cases still pass.