Reject database URLs carrying embedded credentials at the proxy boundary - #2255
Merged
Merged
Conversation
node-fetch cannot build a request from a URL with a username or password, so such a connection has never been able to run a query. The request header schema now rejects one, which turns it into a 400 before any URL object exists. Header values logged for a failed request also get their userinfo replaced.
5 tasks
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.
Description
A database URL carrying a username or password passed the proxy server's request validation, even though
node-fetchrefuses to build a request from such a URL. That connection has never been able to run a query, so the only outcome was a failure further along with a less useful message.Validation now rejects a database URL with userinfo, which turns it into a 400 before any
URLobject is built from it. Nothing that used to work stops working.Header values included in the log line for a failed request are also normalised to drop any userinfo.
Validation
All six database routes return 400 for such a URL and dispatch no outbound fetch. A URL carrying only a username is rejected too, and a URL without userinfo still proxies normally. The log-line tests cover a URL value, a URL without userinfo, a value that is not a URL at all, and a value outside the allowed list.
pnpm checksandpnpm testclean: 223 files, 2720 tests.Notes
This replaces #2250, which GitHub closed as merged when a stack reorder made its head commit an ancestor of its own base branch. Nothing from it reached
main. Same commit, rebased directly ontomain, and independent of any other open PR.Related Issues
None.
Check List
pnpm checkspasses with no errors.pnpm testpasses with no failures.