Repository navigation
Add redacted URI diagnostics to /healthz - #254
Conversation
The response shows length, whitespace, quotes, the @ count, the host, the username, and the password length. It does not show the password or the full URI. The encode hint is added only when the driver reports a parse error. Co-authored-by: Jared Wray <me@jaredwray.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 1720 1781 +61
Branches 386 405 +19
=========================================
+ Hits 1720 1781 +61 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ead5a9444a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `GET /healthz` reports the bin store. A successful admin ping returns HTTP 200 and `bins.connected: true`. A failed ping returns HTTP 503 with the driver error name, a redacted message, and a code. The body has `uriPresent` (boolean) and `scheme` (`mongodb` or `mongodb+srv`). The body does not have the URI, the username, the password, or the host. On the Node server, this route reports the memory store. Connection failures are also one JSON line in the Worker logs (`mockhttp.bin-store`). Those logs use the same redaction. | ||
| `GET /healthz` reports the bin store. A successful admin ping returns HTTP 200 and `bins.connected: true`. A failed ping returns HTTP 503 with the driver error name, a redacted message, and a code. The body has `uriPresent` (boolean) and `scheme` (`mongodb` or `mongodb+srv`). On the Node server, this route reports the memory store. | ||
|
|
||
| When `MONGODB_URI` is set, `bins.uri` has the URI length, leading or trailing whitespace, a newline, quote marks, the `@` count, the host, the username, and the password length. The host is the text after the last `@` and before the first `/` or `?`. The body does not have the password or the full URI. The percent-encode hint is added only when the driver message is `Invalid connection string` or `unescaped characters`. A DNS error keeps the driver text. Worker logs still omit the URI, the username, the password, and the host. |
There was a problem hiding this comment.
This new sentence exceeds the repository's 25-word limit for descriptive sentences. Split the field list into shorter sentences.
AGENTS.md reference: AGENTS.md:L49-L49
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The long sentence in site/docs/how-to-deploy/cloudflare.md is now four short sentences. Commit 8c26806.
| passwordHasPercent: boolean; | ||
| }; | ||
|
|
||
| const PASSWORD_RESERVED = ["@", ":", "/", "?", "#", "[", "]"] as const; |
There was a problem hiding this comment.
$ in the reserved character list
When a raw $ occurs in a password, this list reports no reserved character. MongoDB requires percent encoding for $, so /healthz gives an incomplete diagnosis and omits the needed %24 fix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
passwordReserved now includes $. The encode hint tells you to use %24. Commit 8c26806.
| atCount: uri.split("@").length - 1, | ||
| host: hostText.length > 0 ? hostText : null, | ||
| username, | ||
| passwordLength: secret === null ? null : secret.length, | ||
| passwordReserved: |
There was a problem hiding this comment.
For mongodb://user:@@@@@host, the public response gives atCount: 5, passwordLength: 4, and passwordReserved: ["@"]. An unauthenticated caller can therefore reconstruct the complete @@@@ password, although this change states that the endpoint never returns it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
When every password character is reserved, passwordLength and passwordReserved stay empty. The body does not rebuild that password. atCount still counts each @. Commit 8c26806.
| const at = uri.lastIndexOf("@"); | ||
| const after = at < 0 ? "" : uri.slice(at + 1); | ||
| const cut = after.search(/[/?]/); | ||
| const hostText = cut < 0 ? after : after.slice(0, cut); |
There was a problem hiding this comment.
When a valid option contains @, as in ...?appName=ops@example.com, lastIndexOf selects that option instead of the user-info delimiter. MongoDB parses user information before the path and connection options, so the response reports the wrong host and password length.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The host is the text after the last @ before / or ?. An @ in the query string does not change the host. Commit 8c26806.
The host stops before the query string. $ is a reserved password character. A password of only reserved characters does not publish its length. Co-authored-by: Jared Wray <me@jaredwray.com>
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.
GET /healthzshows redacted URI diagnostics when MongoDB is configured.Why
Production
/healthzstill returnsMongoParseErrorInvalid connection stringafter password resets. The response now shows the URI shape without the password or the full URI.bins.uriincludes:@@before the first/or?)@ : / ? # [ ] $)%An
@in the query string does not change the host. When every password character is reserved,passwordLengthandpasswordReservedstay empty. Those two values rebuild that password.Code path
workerMongoUrireturns the env value. It does not trim the value, and it does not join the binding withprocess.env. An empty string stays unset.applyWorkerMongoEnvstores that same text.pingMongoBinStorepasses that text tonew MongoClient.@keyv/mongoalso stores the string and later passes it toMongoClient. The Worker options do not seturiorurl, so they do not replace it.The percent-encode hint is added only when the driver message already says
Invalid connection stringorunescaped characters. AquerySrverror stays aquerySrverror. An invalid scheme stays an invalid scheme. Those errors are not rewritten into a parse error.A trailing newline does not make the driver throw
Invalid connection string. A leading space or wrapping quotes throwInvalid schemeinstead.Verification
pnpm testpassed: 565 tests, lines 100% (1485/1485).Test audit
reports redacted URI diagnostics and hides the passwordprotects the diagnostic fields, a query@,$, and an all-@password. A leaked password fails it./healthzbody and the exact string stored byMongoClient, including a trailing newline.passes MONGODB_URI through without trimming or joining textprotects the Worker env path into the ping.Rewritten assertions: the ping test and the two workerd tests no longer require the body to hide the host. The host is now part of the contract. The password and full-URI checks stay.
No test was deleted.
Do not merge, release, or deploy this PR.