fix: Malformed $regex query leaks database error details in API response (GHSA-9cp7-3q5w-j92g)#10101
Merged
mtrezza merged 4 commits intoparse-community:alphafrom Mar 5, 2026
Merged
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have already agreed with your approach. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the significant higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughSanitizes unexpected internal errors for invalid regex queries by extending the sanitized-error API to accept a custom sanitized message and updating DatabaseController to convert non-Parse errors into sanitized INTERNAL_SERVER_ERROR responses; adds tests asserting no internal details are leaked and logger records a sanitized message. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DBController as DatabaseController
participant ErrorMod as Error.createSanitizedError
participant Logger
Client->>DBController: send query with invalid $regex
DBController->>DBController: parse/execute query -> throws Error (invalid regex)
alt error is Parse.Error
DBController-->>Client: rethrow original Parse.Error
else non-Parse error
DBController->>ErrorMod: createSanitizedError(INTERNAL_SERVER_ERROR, detailedMessage, config, sanitizedMessage)
ErrorMod->>Logger: log detailed internal error (internal)
ErrorMod-->>DBController: sanitized Parse.Error
DBController->>Logger: log sanitized message ("Regular expression ...")
DBController-->>Client: respond with sanitized INTERNAL_SERVER_ERROR (no internal fields)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10101 +/- ##
==========================================
+ Coverage 92.64% 92.67% +0.02%
==========================================
Files 191 191
Lines 15849 15853 +4
Branches 180 180
==========================================
+ Hits 14683 14691 +8
+ Misses 1154 1150 -4
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Error.js (1)
32-44: Consider addingsanitizedMessageparameter tocreateSanitizedHttpErrorfor consistency.
createSanitizedErrornow supports a customizable sanitized message, butcreateSanitizedHttpErrorstill uses the hardcoded'Permission denied'string. If there's a use case for customizing the HTTP error message (similar to the database error case), this function should be updated for consistency.♻️ Suggested change for consistency
-function createSanitizedHttpError(statusCode, detailedMessage, config) { +function createSanitizedHttpError(statusCode, detailedMessage, config, sanitizedMessage = 'Permission denied') { // On testing we need to add a prefix to the message to allow to find the correct call in the TestUtils.js file if (process.env.TESTING) { defaultLogger.error('Sanitized error:', detailedMessage); } else { defaultLogger.error(detailedMessage); } const error = new Error(); error.status = statusCode; - error.message = config?.enableSanitizedErrorResponse !== false ? 'Permission denied' : detailedMessage; + error.message = config?.enableSanitizedErrorResponse !== false ? sanitizedMessage : detailedMessage; return error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Error.js` around lines 32 - 44, The createSanitizedHttpError function currently hardcodes the sanitized HTTP message; update its signature to accept an optional sanitizedMessage parameter (e.g., createSanitizedHttpError(statusCode, detailedMessage, config, sanitizedMessage)) and use sanitizedMessage as the value when config?.enableSanitizedErrorResponse !== false, falling back to 'Permission denied' if sanitizedMessage is not provided; ensure logging behavior (defaultLogger.error) remains unchanged and update any call sites of createSanitizedHttpError to pass the new sanitizedMessage where appropriate to match createSanitizedError's behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Error.js`:
- Around line 32-44: The createSanitizedHttpError function currently hardcodes
the sanitized HTTP message; update its signature to accept an optional
sanitizedMessage parameter (e.g., createSanitizedHttpError(statusCode,
detailedMessage, config, sanitizedMessage)) and use sanitizedMessage as the
value when config?.enableSanitizedErrorResponse !== false, falling back to
'Permission denied' if sanitizedMessage is not provided; ensure logging behavior
(defaultLogger.error) remains unchanged and update any call sites of
createSanitizedHttpError to pass the new sanitizedMessage where appropriate to
match createSanitizedError's behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6bc4ad42-dd6d-45e3-bce0-6d8c603b9d16
📒 Files selected for processing (3)
spec/vulnerabilities.spec.jssrc/Controllers/DatabaseController.jssrc/Error.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/vulnerabilities.spec.js (1)
516-546: Consider extracting a shared helper for malformed-regex sanitization assertions.Lines 488-513 and Lines 519-544 duplicate the same control flow and assertions; a helper would keep future sanitization updates in one place.
♻️ Proposed refactor
describe('Malformed $regex information disclosure', () => { + async function expectMalformedRegexSanitized({ url, where }) { + const logger = require('../lib/logger').default; + const loggerErrorSpy = spyOn(logger, 'error').and.callThrough(); + try { + await request({ + method: 'GET', + url, + headers: { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }, + qs: { where: JSON.stringify(where) }, + }); + fail('Request should have failed'); + } catch (e) { + expect(e.data.code).toBe(Parse.Error.INTERNAL_SERVER_ERROR); + expect(e.data.error).toBe('An internal server error occurred'); + expect(typeof e.data.error).toBe('string'); + expect(JSON.stringify(e.data)).not.toContain('errmsg'); + expect(JSON.stringify(e.data)).not.toContain('codeName'); + expect(JSON.stringify(e.data)).not.toContain('errorResponse'); + expect(loggerErrorSpy).toHaveBeenCalledWith( + 'Sanitized error:', + jasmine.stringMatching(/[Rr]egular expression/i) + ); + } + } + it('should not leak database error internals for invalid regex pattern in class query', async () => { - const logger = require('../lib/logger').default; - const loggerErrorSpy = spyOn(logger, 'error').and.callThrough(); const obj = new Parse.Object('TestObject'); await obj.save({ field: 'value' }); - - try { - await request({ ... }); - fail('Request should have failed'); - } catch (e) { - ... - } + await expectMalformedRegexSanitized({ + url: 'http://localhost:8378/1/classes/TestObject', + where: { field: { $regex: '[abc' } }, + }); }); it('should not leak database error internals for invalid regex pattern in role query', async () => { - const logger = require('../lib/logger').default; - const loggerErrorSpy = spyOn(logger, 'error').and.callThrough(); - try { - await request({ ... }); - fail('Request should have failed'); - } catch (e) { - ... - } + await expectMalformedRegexSanitized({ + url: 'http://localhost:8378/1/roles', + where: { name: { $regex: '[abc' } }, + }); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/vulnerabilities.spec.js` around lines 516 - 546, Duplicate control flow and assertions for malformed regex sanitization in spec/vulnerabilities.spec.js should be extracted into a single reusable helper (e.g., expectMalformedRegexSanitization or assertMalformedRegexSanitization). Move the try/catch + assertions that create loggerErrorSpy, perform the request with qs: { where: JSON.stringify({ name: { $regex: '[abc' } }) }, check e.data.code, e.data.error string, and ensure no 'errmsg'/'codeName'/'errorResponse' in the serialized error and the loggerErrorSpy call into that helper, then call it from both places to remove the duplicated blocks while preserving the same expectations and use of the logger variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 516-546: Duplicate control flow and assertions for malformed regex
sanitization in spec/vulnerabilities.spec.js should be extracted into a single
reusable helper (e.g., expectMalformedRegexSanitization or
assertMalformedRegexSanitization). Move the try/catch + assertions that create
loggerErrorSpy, perform the request with qs: { where: JSON.stringify({ name: {
$regex: '[abc' } }) }, check e.data.code, e.data.error string, and ensure no
'errmsg'/'codeName'/'errorResponse' in the serialized error and the
loggerErrorSpy call into that helper, then call it from both places to remove
the duplicated blocks while preserving the same expectations and use of the
logger variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b66f832f-7319-49e0-8592-ba9207c91a8b
📒 Files selected for processing (1)
spec/vulnerabilities.spec.js
mtrezza
changed the title
$regex query leaks database error details in API response (GHSA-9cp7-3q5w-j92g)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/vulnerabilities.spec.js (1)
482-547: Consider extracting the duplicated malformed-regex assertion flow into a helper.Both tests repeat near-identical setup and assertions; a shared helper would reduce drift and make future sanitizer changes easier to update.
♻️ Optional refactor sketch
+async function expectSanitizedMalformedRegex({ url, where }) { + const logger = require('../lib/logger').default; + const loggerErrorSpy = spyOn(logger, 'error').and.callThrough(); + try { + await request({ + method: 'GET', + url, + headers: { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }, + qs: { where: JSON.stringify(where) }, + }); + fail('Request should have failed'); + } catch (e) { + expect(e.data.code).toBe(Parse.Error.INTERNAL_SERVER_ERROR); + expect(e.data.error).toBe('An internal server error occurred'); + expect(JSON.stringify(e.data)).not.toContain('errmsg'); + expect(JSON.stringify(e.data)).not.toContain('codeName'); + expect(JSON.stringify(e.data)).not.toContain('errorResponse'); + expect(loggerErrorSpy).toHaveBeenCalledWith( + 'Sanitized error:', + jasmine.stringMatching(/[Rr]egular expression/i) + ); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/vulnerabilities.spec.js` around lines 482 - 547, Extract the duplicated malformed-regex test flow into a shared helper to avoid repetition: create a helper function (e.g., assertMalformedRegexSanitization) in spec/vulnerabilities.spec.js that accepts the request URL and the where JSON (and optionally a pre-save setup callback), performs the logger spy setup (spyOn(logger,'error').and.callThrough()), issues the request via request({... qs: { where: JSON.stringify(where) } }), catches the error, and runs the common assertions (internal server error code/message, absence of errmsg/codeName/errorResponse, and loggerErrorSpy call matching /[Rr]egular expression/i); then replace the two test bodies with calls to that helper, passing `'/1/classes/TestObject'`/`{ field: { $regex: '[abc' } }` and `'/1/roles'`/`{ name: { $regex: '[abc' } }` respectively (for the roles test keep the role creation step as the helper's optional setup callback or run it before calling the helper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 482-547: Extract the duplicated malformed-regex test flow into a
shared helper to avoid repetition: create a helper function (e.g.,
assertMalformedRegexSanitization) in spec/vulnerabilities.spec.js that accepts
the request URL and the where JSON (and optionally a pre-save setup callback),
performs the logger spy setup (spyOn(logger,'error').and.callThrough()), issues
the request via request({... qs: { where: JSON.stringify(where) } }), catches
the error, and runs the common assertions (internal server error code/message,
absence of errmsg/codeName/errorResponse, and loggerErrorSpy call matching
/[Rr]egular expression/i); then replace the two test bodies with calls to that
helper, passing `'/1/classes/TestObject'`/`{ field: { $regex: '[abc' } }` and
`'/1/roles'`/`{ name: { $regex: '[abc' } }` respectively (for the roles test
keep the role creation step as the helper's optional setup callback or run it
before calling the helper).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92cbe686-319e-41dd-b7fb-1f0ea579d458
📒 Files selected for processing (1)
spec/vulnerabilities.spec.js
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 5, 2026
# [9.5.0-alpha.6](9.5.0-alpha.5...9.5.0-alpha.6) (2026-03-05) ### Bug Fixes * Malformed `$regex` query leaks database error details in API response ([GHSA-9cp7-3q5w-j92g](GHSA-9cp7-3q5w-j92g)) ([#10101](#10101)) ([9792d24](9792d24))
Contributor
|
🎉 This change has been released in version 9.5.0-alpha.6 |
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.
Pull Request
Issue
Malformed
$regexquery leaks database error details in API response (GHSA-9cp7-3q5w-j92g)Tasks
Summary by CodeRabbit
Bug Fixes
Tests