fix: Server crash via deeply nested query condition operators (GHSA-9xp9-j92r-p88v)#10202
Conversation
|
🚀 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 reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. 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 higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
✅ 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. |
📝 WalkthroughWalkthroughAdds a new RequestComplexity option Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DBController as DatabaseController
participant Options
participant DB as Database
Client->>DBController: send query (contains $or/$and/$nor)
DBController->>Options: read requestComplexity.queryDepth
DBController->>DBController: validateQuery(query, ..., options, _depth=0)
alt depth <= queryDepth or queryDepth == -1
DBController->>DB: execute query
DB-->>DBController: results
DBController-->>Client: return results
else depth > queryDepth
DBController-->>Client: throw INVALID_QUERY (nesting depth exceeded)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10202 +/- ##
=======================================
Coverage 92.55% 92.55%
=======================================
Files 192 192
Lines 16281 16284 +3
Branches 199 199
=======================================
+ Hits 15069 15072 +3
Misses 1195 1195
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mtrezza
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/RequestComplexity.spec.js (1)
72-87: Add explicit invalid-value tests forqueryDepth.Consider adding rejection assertions for
queryDepth: 0and a non-integer, mirroring the existingincludeDepthvalidation checks, to lock down this new config path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/RequestComplexity.spec.js` around lines 72 - 87, Add explicit invalid-value tests for requestComplexity.queryDepth in the spec by mirroring the existing includeDepth checks: add assertions that reconfigureServer({ requestComplexity: { queryDepth: 0 } }) is rejected and that reconfigureServer({ requestComplexity: { queryDepth: 1.5 } }) (or a non-integer like 'a') is rejected. Locate the test block using reconfigureServer and the requestComplexity config keys and use the same expectAsync(...).toBeRejected() (or equivalent) pattern used for includeDepth validation to ensure queryDepth rejects zero and non-integer values.
🤖 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/RequestComplexity.spec.js`:
- Around line 72-87: Add explicit invalid-value tests for
requestComplexity.queryDepth in the spec by mirroring the existing includeDepth
checks: add assertions that reconfigureServer({ requestComplexity: { queryDepth:
0 } }) is rejected and that reconfigureServer({ requestComplexity: { queryDepth:
1.5 } }) (or a non-integer like 'a') is rejected. Locate the test block using
reconfigureServer and the requestComplexity config keys and use the same
expectAsync(...).toBeRejected() (or equivalent) pattern used for includeDepth
validation to ensure queryDepth rejects zero and non-integer values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f5bfb19-98ca-428b-b39d-165b02ffa95b
📒 Files selected for processing (5)
spec/RequestComplexity.spec.jsspec/vulnerabilities.spec.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Options/Definitions.js
- spec/vulnerabilities.spec.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/SecurityCheckGroups.spec.js (1)
40-40: Consider making the success fixture fully explicit for request-complexity keys.This currently depends on implicit/default behavior for the other limits. Setting all request-complexity properties explicitly in this test would make intent and future behavior clearer.
Proposed test-fixture refinement
- config.requestComplexity = { queryDepth: 10 }; + config.requestComplexity = { + includeDepth: 10, + includeCount: 100, + subqueryDepth: 10, + queryDepth: 10, + graphQLDepth: 10, + graphQLFields: 1000, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/SecurityCheckGroups.spec.js` at line 40, The test currently sets only config.requestComplexity = { queryDepth: 10 } which relies on implicit defaults for other request-complexity keys; update the fixture to explicitly set all requestComplexity properties (e.g., queryDepth, queryScore, maxDepthPerField or whatever keys the config supports) so the spec (SecurityCheckGroups.spec.js) is fully explicit about every limit; modify the object assigned to config.requestComplexity used in the test to include each supported key with intended values rather than a single queryDepth entry.
🤖 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/SecurityCheckGroups.spec.js`:
- Line 40: The test currently sets only config.requestComplexity = { queryDepth:
10 } which relies on implicit defaults for other request-complexity keys; update
the fixture to explicitly set all requestComplexity properties (e.g.,
queryDepth, queryScore, maxDepthPerField or whatever keys the config supports)
so the spec (SecurityCheckGroups.spec.js) is fully explicit about every limit;
modify the object assigned to config.requestComplexity used in the test to
include each supported key with intended values rather than a single queryDepth
entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b933b70a-ef80-4f42-a848-ddc2fbd60f85
📒 Files selected for processing (1)
spec/SecurityCheckGroups.spec.js
# [9.6.0-alpha.21](9.6.0-alpha.20...9.6.0-alpha.21) (2026-03-15) ### Bug Fixes * Server crash via deeply nested query condition operators ([GHSA-9xp9-j92r-p88v](https://github.com/parse-community/parse-server/security/advisories/GHSA-9xp9-j92r-p88v)) ([#10202](#10202)) ([f44e306](f44e306))
|
🎉 This change has been released in version 9.6.0-alpha.21 |
2 participants
Issue
Server crash via deeply nested query condition operators (GHSA-9xp9-j92r-p88v)
Tasks
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests