security: address code scanning and secret scanning alerts#25
Merged
Conversation
- Add explicit `permissions: contents: read` to test.yml and publish.yml workflows. Grant `contents: write` on the publish job for releases. - Redact password / SQL payloads in protocol-handler debug logs so the password message and credential-bearing SQL never hit clear-text output. Same redaction applied to lib/parser.ts logging. - Replace Math.random() with crypto.randomInt for the backend secret key in BackendKeyData — that key authenticates CancelRequest, so it needs to be unguessable. - Stop logging full SQL strings in postgres-server integration test mocks; log query length only. - Replace `mongodb+srv://username:password@cluster.mongodb.net` style placeholders in three blog posts so they no longer trip the Atlas URI secret scanner. Switched code samples to env vars and the SQL sample to `<user>:<pass>` placeholders.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Pull the credential-masking regex into packages/lib/src/redact.ts and re-export it from the lib's public surface so postgres-server can pull it in instead of carrying its own copy. Also extend the basic.test.ts mock to spread the real module before overriding QueryLeaf, otherwise the new redactSql import resolves to undefined inside the handler under test.
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
2 participants
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.
Summary
Triaged the open GitHub code scanning and secret scanning alerts and fixed the legitimate ones.
Code scanning
actions/missing-workflow-permissions(#28, #32): added an explicit top-levelpermissions: contents: readblock totest.ymlandpublish.yml. The publish job getscontents: writesince it cuts a GitHub Release.js/clear-text-logginginpackages/postgres-server/src/protocol-handler.ts(Bump lodash from 4.17.21 to 4.18.1 #21–deps: clear open Dependabot alerts #26): the parsed client message can carry a password (type: 'password',string: <password>) or credential-bearing SQL (CREATE USER ... PASSWORD '...'). Replaced rawdebug('…', message)/debug('…', queryString)calls with a smallredactMessage/redactSqlpair that masks the password field and any literalPASSWORD '…'/IDENTIFIED BYclauses before they reach the debug stream. The startup-message debug line that dumped the whole struct now just saysStartup message received; the actual user/database is already logged sanitized one line later.js/clear-text-logginginpackages/lib/src/parser.ts(Fix: non-primary snake_case ObjectId fields not converted (issue #12) #16, Bump yauzl from 3.2.0 to 3.2.1 #17, #34, #35): wrapped the parser'sdebug('queryleaf:parser')logger with the same SQL redaction so SQL passed in by callers can't leak literal credentials through debug output.js/clear-text-logginginpackages/postgres-server/tests/integration/minimal*.test.ts(Bump yaml from 2.7.0 to 2.8.3 #19, Bump picomatch from 2.3.1 to 2.3.2 #20): the mock QueryLeaf was logging the full incoming query string. Switched to logging the length only.js/insecure-randomnessinpackages/postgres-server/src/protocol-handler.ts:1065(Bump qs from 6.15.1 to 6.15.2 #27): the BackendKeyData secret authenticatesCancelRequestmessages, so it must be unguessable. Switched both the process ID and secret key tocrypto.randomInt.Secret scanning
Both alerts were placeholder Atlas URIs in blog posts that nonetheless match the Atlas URI detector:
docs/blog/posts/mongodb-atlas-cloud-deployment-management.md: switched to<user>:<pass>placeholders inside the SQL sample.docs/blog/posts/mongodb-vector-search-ai-applications-semantic-similarity.mdanddocs/blog/posts/mongodb-gridfs-file-management-sql.md: switched the JS samples to readprocess.env.MONGODB_URI, which is what we'd recommend in real code anyway.Already-dismissed / not addressed
js/polynomial-redos— already dismissed.js/hardcoded-credentials, the workflow permissions onci.yml, lib parser fixed, etc.) are already in thefixedstate.Test plan
yarn typecheckyarn workspace @queryleaf/lib test:unityarn workspace @queryleaf/postgres-server test:unityarn workspace @queryleaf/lib buildandyarn workspace @queryleaf/postgres-server build