Repository navigation
fix: the password is masked in every trace log line; release notes say how to send a trace - #515
Merged
Merged
Conversation
…s say how to send a trace The release notes now have a standing section on capturing a TRACE log to attach to an issue. Before asking for those, the password had to stay out of them: - JS: PrivateConnection.open logged the connection string as given at TRACE. It is now masked (PWD= or Password=, any case, braced or not). - C++: OdbcConnection::Open logged it unmasked at DEBUG, and the SQLDriverConnect and diagnose lines masked only an upper case PWD= up to the first ';'. All three now use StringUtils::SanitizeConnectionString, which matches a key only at the start of a field, allows spaces before the '=', and skips a braced value to its closing brace. Also corrects the levels in the configureFor* comments in the template and the logging sample (development is DEBUG, production INFO, testing TRACE). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CodeQL flagged the masking regex as polynomial: an unclosed braced value
made the braced branch scan to the end of the string and fail, again at
every ';pwd={'. The connection string is the app's own, so this was not a
practical risk, but a field by field scan is as short and mirrors the C++
StringUtils::SanitizeConnectionString.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Release notes
.github/RELEASE_TEMPLATE.mdgets a standing section, Hit a problem? Send us a trace. It says how to turn on TRACE logging to a file (JS and C++, including the ODBC calls and their results), reproduce, and attach the file to an issue. It also warns that query text, parameter values and rows are logged as they are, so the file should be checked before posting. TheconfigureFor*comments in the older snippet had the wrong levels. They are corrected here and insamples/javascript/simple-logging-config.js.Password masking
Before asking people for traces, I checked what a TRACE file holds. The password leaked in two places:
PrivateConnection.openlogged the connection string as given (TRACE).OdbcConnection::Openlogged it unmasked (DEBUG).Separately, the
SQLDriverConnectand diagnose lines masked only an upper-casePWD=, up to the first;. Sopwd=,Password=or a bracedPWD={a;b}would partly leak.Now:
maskPasswordfor the connection string it logs.StringUtils::SanitizeConnectionString. It matchesPWD/Passwordin any case, only at the start of a field, allows spaces before the=, and skips a braced value to its closing brace (}}escapes).Verified locally:
PWD=x,pwd = {x}andPassword=xhas no plaintext line in any of the three.test/mask-password.test.jspasses (7 cases).Needs a native rebuild (prebuilds come with the release as usual).
🤖 Generated with Claude Code