Skip to content

Reject database URLs carrying embedded credentials at the proxy boundary - #2250

Merged
kmcginnes merged 0 commit into
proxy-server-errno-in-error-payloadfrom
redact-connection-url-in-proxy-logs
Sep 23, 2026
Merged

kmcginnes merged 0 commit into
proxy-server-errno-in-error-payloadfrom
redact-connection-url-in-proxy-logs

Conversation

@kmcginnes

@kmcginnes kmcginnes commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Description

A database URL carrying a username or password passed the proxy server's request validation, even though node-fetch refuses 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 URL object 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 checks and pnpm test clean: 223 files, 2733 tests.

Related Issues

None.

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

@kmcginnes
kmcginnes added this pull request to stack #2253 September 23, 2026 21:40
@kmcginnes
kmcginnes force-pushed the redact-connection-url-in-proxy-logs branch from 20853d5 to ca5fb65 Compare September 23, 2026 22:14
@kmcginnes
kmcginnes marked this pull request as ready for review September 23, 2026 22:23
@kmcginnes
kmcginnes merged commit 123c552 into main Sep 23, 2026
@kmcginnes
kmcginnes deleted the redact-connection-url-in-proxy-logs branch September 23, 2026 23:36
@kmcginnes
kmcginnes force-pushed the redact-connection-url-in-proxy-logs branch from ca5fb65 to 123c552 Compare September 23, 2026 23:36
@kmcginnes

Copy link
Copy Markdown
Collaborator Author

Closed by GitHub, not merged. A stack reorder moved this branch underneath proxy-server-errno-in-error-payload, which was still set as this PR's base, so GitHub saw the head commit already contained in the base and marked it merged with 0 files changed. Its "merge commit" is just this branch's own commit.

Nothing here reached main. Replaced by Reject database URLs carrying embedded credentials at the proxy boundary #2255, which is the same commit rebased directly onto main and independent of the other open PRs.

kmcginnes added a commit that referenced this pull request Sep 23, 2026
…ary (#2255)

## Description

A database URL carrying a username or password passed the proxy server's
request validation, even though `node-fetch` refuses 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 `URL` object 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 checks` and `pnpm test` clean: 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 onto `main`, and
independent of any other open PR.

## Related Issues

None.

### Check List

- [x] I confirm that my contribution is made under the terms of the
Apache 2.0 license.
- [x] I have verified `pnpm checks` passes with no errors.
- [x] I have verified `pnpm test` passes with no failures.
- [x] I have covered new added functionality with unit tests if
necessary.
- [x] I have updated documentation if necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant