Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: Add Parse Server optionsmaxIncludeQueryComplexity,maxGraphQLQueryComplexity to limit query complexity for performance protection#9920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Moumouls wants to merge12 commits intoparse-community:alpha
base:alpha
Choose a base branch
Loading
fromMoumouls:moumouls/include-prevention-complexity

Conversation

@Moumouls
Copy link
Member

@MoumoulsMoumouls commentedNov 8, 2025•
edited by coderabbitaibot
Loading

Pull Request

Issue

Give developers the option to strengthen the protection of the Parse Server REST and GraphQL APIs based on complexity factors such as fields and query depth.

Approach

Currently parse-server can't have default values because it's a breaking change.
Also if in a futur major release we introduce some large default values (Depth 10 + Fields 100 on rest) and (Depth 20 and fields 200 on GQL). The includeAll option should be then masterKey only

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Addsecurity check
  • Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • New Features

    • Configurable query complexity limits for GraphQL and REST include queries (depth and fields/count); enforcement blocks excessive requests (including includeAll) and allows master/maintenance-key bypass; GraphQL complexity check runs in request pipeline.

  • Documentation

    • Public options, types, and docs updated to expose new complexity settings and cross-option validation guidance.

  • Tests

    • Comprehensive test suites covering GraphQL and REST complexity rules, fragments/includes, bypass keys, combined constraints, and no-limit scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistantparse-github-assistantbot changed the titlefeat: max depth and max fields protection systemfeat: Max depth and max fields protection systemNov 8, 2025
@parse-github-assistant
Copy link

parse-github-assistantbot commentedNov 8, 2025•
edited
Loading

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitaibot commentedNov 8, 2025•
edited
Loading

📝 Walkthrough

Walkthrough

Adds configurable query-complexity limits for GraphQL and REST includes, validates their cross-relations in Config, enforces GraphQL complexity via an Apollo plugin (with fragment/cycle handling and auth bypass), blocks/validates REST includes, and adds test suites for both features.

Changes

Cohort / File(s)Summary
Configuration & Types
src/Config.js,src/Options/index.js,src/Options/Definitions.js,src/Options/docs.js,types/Options/index.d.ts		IntroducemaxGraphQLQueryComplexity andmaxIncludeQueryComplexity option shapes, typings, and docs; addConfig.validateQueryComplexityOptions and wire validation into option processing; update public option types.
GraphQL Complexity Validation
src/GraphQL/helpers/queryComplexity.js,src/GraphQL/ParseGraphQLServer.js		New AST traversal computing depth and field counts (handles Field, InlineFragment, FragmentSpread, cycle detection); addedcreateComplexityValidationPlugin(config) and conditionally attach it to ApolloServer; plugin skips checks when master/maintenance auth present or when unset.
REST Include Validation
src/RestQuery.js		Enforce include complexity (count/depth) post-parse; blockincludeAll and wildcard* includes for non-master/maintenance users when configured; throwINVALID_QUERY with specific messages on violations.
Tests
spec/ParseGraphQLQueryComplexity.spec.js,spec/RestQuery.spec.js		Add comprehensive tests covering GraphQL fields/depth limits (named/inline/cyclic fragments, combined constraints, bypass keys) and REST include count/depth, includeAll blocking, wildcard includes, and bypass scenarios.

Sequence Diagram(s)

sequenceDiagram    participant Client    participant Apollo as ApolloServer    participant Plugin as ComplexityPlugin    participant Resolver    Note over Apollo: GraphQL request lifecycle    Client->>Apollo: POST /graphql (document, headers)    Apollo->>Apollo: Check auth (master/maintenance?)    alt master/maintenance key        Apollo->>Resolver: Execute query (no complexity check)    else normal request        Apollo->>Plugin: didResolveOperation(document)        activate Plugin        Plugin->>Plugin: build fragment map, traverse AST, count depth & fields        Plugin-->>Apollo: ok or throw GraphQLError(403)        deactivate Plugin        alt within limits            Apollo->>Resolver: Execute query            Resolver-->>Client: 200 + data        else exceeds limits            Apollo-->>Client: 403 GraphQLError (limit exceeded)        end    end
Loading
sequenceDiagram    participant Client    participant REST as REST Endpoint    participant Auth as Auth Check    participant Validator as IncludeValidator    participant Handler as QueryHandler    Client->>REST: GET /classes/Thing?include=...    REST->>Auth: Validate credentials    alt master key        Auth->>Handler: Proceed (no include validation)    else normal/maintenance        Auth->>Validator: Validate include count & depth        activate Validator        Validator->>Validator: count include fields, compute max depth        Validator-->>Auth: ok or INVALID_QUERY error        deactivate Validator        alt within limits            Auth->>Handler: Execute query and return results        else exceeds limits            REST-->>Client: 400 INVALID_QUERY        end    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • correctness and early-exit behavior of AST traversal, fragment resolution, and cycle detection insrc/GraphQL/helpers/queryComplexity.js
    • plugin lifecycle hooks and master/maintenance bypass logic insrc/GraphQL/ParseGraphQLServer.js
    • REST include parsing, includeAll/wildcard blocking, and exact error messages insrc/RestQuery.js
    • cross-option validation insrc/Config.js
    • reliability and lifecycle management in the new tests (spec/*)

Possibly related issues

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.You can run@coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and concisely describes the main feature: adding two new Parse Server options for query complexity limits to protect performance. It accurately reflects the primary changes across the changeset.
Description check✅ PassedThe PR description is largely complete with a filled-out issue statement explaining the feature motivation, an approach section describing the problem (no defaults due to breaking change concern), and tasks partially completed (tests added, documentation/security checks pending).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment@coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

parseplatformorg commentedNov 8, 2025•
edited
Loading

✅Snyk checks have passed. No issues have been found so far.

StatusScanner Critical High Medium LowTotal (0)
✅Open Source Security0000 0 issues

💻 Catch issues earlier using the plugins forVS Code,JetBrains IDEs,Visual Studio, andEclipse.

@Moumouls
Copy link
MemberAuthor

@coderabbitai full review

coderabbitai[bot] reacted with eyes emoji

@coderabbitai
Copy link

✅ Actions performed

Full review triggered.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweena85ba19 and278808d.

📒 Files selected for processing (9)
  • spec/ParseGraphQLQueryComplexity.spec.js (1 hunks)
  • spec/RestQuery.spec.js (1 hunks)
  • src/Config.js (3 hunks)
  • src/GraphQL/ParseGraphQLServer.js (2 hunks)
  • src/GraphQL/helpers/queryComplexity.js (1 hunks)
  • src/Options/Definitions.js (2 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
  • src/RestQuery.js (3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93Repo: parse-community/parse-server PR: 9744File: spec/ParseLiveQuery.spec.js:0-0Timestamp: 2025-04-30T19:31:35.344ZLearning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • src/RestQuery.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1312-1338Timestamp: 2025-05-04T20:41:05.147ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • src/Options/index.js
  • src/Options/Definitions.js
  • src/Config.js
  • src/Options/docs.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • src/Options/Definitions.js
  • src/Config.js
  • src/Options/docs.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • src/RestQuery.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/docs.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: src/triggers.js:467-477Timestamp: 2025-08-27T12:33:06.237ZLearning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Applied to files:

  • spec/RestQuery.spec.js
🪛 Biome (2.1.2)
src/Options/index.js

[error] 45-49: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 362-362: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: Node 22
  • GitHub Check: Node 18
  • GitHub Check: Redis Cache
  • GitHub Check: Node 20
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Docker Build

@MoumoulsMoumouls marked this pull request as ready for reviewNovember 9, 2025 18:48
@codecov
Copy link

codecovbot commentedNov 9, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is86.15385% with9 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.05%. Comparing base (d4c6de0) to head (996bdc2).
⚠️ Report is 1 commits behind head on alpha.

Files with missing linesPatch %Lines
src/Config.js33.33%4 Missing⚠️
src/GraphQL/helpers/queryComplexity.js90.90%4 Missing⚠️
src/RestQuery.js90.90%1 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##            alpha    #9920      +/-   ##==========================================- Coverage   93.08%   93.05%   -0.03%==========================================  Files         188      189       +1       Lines       15298    15363      +65       Branches      177      177              ==========================================+ Hits        14240    14296      +56- Misses       1046     1055       +9  Partials       12       12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
src/Options/index.js (1)

354-369:Document the right counters for REST vs GraphQL complexity

The REST comment still describes a{ depth, count } format while the shared type exposesfields, and the GraphQL comment says “fields = number of operations”, which contradicts the actual limiter (it counts field selections). This inconsistency will send users in circles. Please align the prose with the real counters—REST usescount for include paths, GraphQL usesfields for selected fields—and mention thatfields limits field selections, not operations.

-  /* Maximum query complexity for REST API includes. Controls depth and number of include fields.-   * Format: { depth: number, count: number }-   * - depth: Maximum depth of nested includes (e.g., foo.bar.baz = depth 3)-   * - count: Maximum number of include fields (e.g., foo,bar,baz = 3 fields)+  /* Maximum query complexity for REST API includes. Controls include depth and include count.+   * Format: { depth?: number, count?: number }+   * - depth: Maximum depth of nested include paths (e.g., foo.bar.baz = depth 3)+   * - count: Maximum number of include paths across the query (e.g., foo,bar,baz = 3)…-  /* Maximum query complexity for GraphQL queries. Controls depth and number of operations.-   * Format: { depth: number, fields: number }-   * - depth: Maximum depth of nested field selections-   * - fields: Maximum number of operations (queries/mutations) in a single request+  /* Maximum query complexity for GraphQL queries. Controls depth and total field selections.+   * Format: { depth?: number, fields?: number }+   * - depth: Maximum depth of nested field selections+   * - fields: Maximum number of field selections within the resolved operation
spec/ParseGraphQLQueryComplexity.spec.js (1)

16-31:Close the HTTP server with a promise before reconfiguring

Line 19 (and again Line 54):httpServer.close() is callback-based; awaiting it directly never waits for the port to free, so the very nextlisten races and we can hitEADDRINUSE. Please wrap the close call in a Promise (and clearhttpServer) before spinning up the next server, and reuse the same helper inafterEach.

-  async function reconfigureServer(options = {}) {-    if (httpServer) {-      await httpServer.close();-    }+  async function shutdownHttpServer() {+    if (!httpServer) {+      return;+    }+    await new Promise(resolve => httpServer.close(resolve));+    httpServer = null;+  }++  async function reconfigureServer(options = {}) {+    await shutdownHttpServer();…-    if (httpServer) {-      await httpServer.close();-    }+    await shutdownHttpServer();

Also applies to: 52-55

spec/RestQuery.spec.js (1)

1107-1133:Use the actual include-count key in the “no includes” tests

Lines 1110 & 1131 still configure{ paths: 1 }, so we never exercise the include-count limiter branch these tests are meant to cover. Change those objects to use the realcount property so the server actually enforces the limit during the test run.

       maxIncludeQueryComplexity: {         depth: 1,-        paths: 1,+        count: 1,       },…       maxIncludeQueryComplexity: {         depth: 1,-        paths: 1,+        count: 1,       },
src/Options/Definitions.js (1)

402-402:Duplicate: Fix the fields description.

The description of thefields property is misleading as noted in a previous review. It currently states "Maximum number of operations (queries/mutations)" but the implementation enforces the number of field selections, not operations.

🧹 Nitpick comments (1)
src/Options/Definitions.js (1)

416-421:Consider consistent property naming across complexity options.

ThemaxIncludeQueryComplexity option uses acount property whilemaxGraphQLQueryComplexity uses afields property, even though both measure similar concepts (the number of items being selected or included). This inconsistency may cause confusion.

Consider renaming one to match the other for API consistency, such as usingfields for both orcount for both. If the different names are intentional to reflect different semantics (GraphQL field selections vs REST include counts), this distinction should be clearly documented.

⚠️Note: This is generated code. Changes must be made insrc/Options/index.js.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between278808d and9343bc0.

📒 Files selected for processing (8)
  • spec/ParseGraphQLQueryComplexity.spec.js (1 hunks)
  • spec/RestQuery.spec.js (1 hunks)
  • src/Config.js (3 hunks)
  • src/GraphQL/helpers/queryComplexity.js (1 hunks)
  • src/Options/Definitions.js (2 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
  • src/RestQuery.js (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/RestQuery.js
  • src/Options/docs.js
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93Repo: parse-community/parse-server PR: 9744File: spec/ParseLiveQuery.spec.js:0-0Timestamp: 2025-04-30T19:31:35.344ZLearning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1312-1338Timestamp: 2025-05-04T20:41:05.147ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: MoumoulsRepo: parse-community/parse-server PR: 9883File: spec/CloudCodeLogger.spec.js:410-412Timestamp: 2025-10-16T19:27:05.311ZLearning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • spec/RestQuery.spec.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
  • src/Options/Definitions.js
  • src/Config.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • spec/RestQuery.spec.js
  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: src/triggers.js:467-477Timestamp: 2025-08-27T12:33:06.237ZLearning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: src/triggers.js:446-454Timestamp: 2025-08-27T09:08:34.252ZLearning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • spec/RestQuery.spec.js
  • src/Options/Definitions.js
  • src/Config.js
  • src/Options/index.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
🧬 Code graph analysis (4)
spec/ParseGraphQLQueryComplexity.spec.js (1)
spec/helper.js (1)
  • reconfigureServer (180-214)
spec/RestQuery.spec.js (3)
spec/helper.js (2)
  • reconfigureServer (180-214)
  • Parse (4-4)
src/RestQuery.js (3)
  • query (372-381)
  • query (382-382)
  • results (800-800)
src/rest.js (1)
  • query (120-130)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
src/Options/index.js (2)
resources/buildConfigDefinitions.js (3)
  • type (121-121)
  • type (159-159)
  • type (216-216)
src/triggers.js (1)
  • type (819-819)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 45-49: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 362-362: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

🔇 Additional comments (1)
src/Options/Definitions.js (1)

399-404:The core claim about an undefined option reference is incorrect.

The review comment states that line 402 references a non-existentmaxQueryComplexity option. However, searching the entire codebase confirmsmaxQueryComplexity does not appear anywhere. More importantly, line 402 actually references onlymaxIncludeQueryComplexity andmaxGraphQLQueryComplexity—both of which are properly defined options in the codebase and validated insrc/Config.js.

A separate but valid concern exists: The environment variablePARSE_SERVER_MAX_GRAPH_QLQUERY_COMPLEXITY uses inconsistent naming. Other GraphQL-related environment variables in the same file useGRAPHQL (e.g.,PARSE_SERVER_GRAPHQL_PATH,PARSE_SERVER_GRAPHQL_PUBLIC_INTROSPECTION,PARSE_SERVER_MOUNT_GRAPHQL), while two variables oddly useGRAPH_QL (PARSE_SERVER_GRAPH_QLSCHEMA andPARSE_SERVER_MAX_GRAPH_QLQUERY_COMPLEXITY). This is worth addressing, though it's a separate issue from what the review comment claimed.

Likely an incorrect or invalid review comment.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
spec/ParseGraphQLQueryComplexity.spec.js (1)

18-55:Await the HTTP server shutdown before restarting.

httpServer.close() is callback-based;await httpServer.close() returns immediately and the port can still be bound when the next test starts, causing intermittentEADDRINUSE. Wrap the close in a Promise (and mirror the change inafterEach) so we only continue once the listener is actually closed:

if (httpServer) {-  await httpServer.close();+  await new Promise(resolve => httpServer.close(resolve));+  httpServer = null;}

Apply the same promise wrapper insideafterEach.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between9343bc0 and5d405e9.

📒 Files selected for processing (5)
  • spec/ParseGraphQLQueryComplexity.spec.js (1 hunks)
  • src/GraphQL/helpers/queryComplexity.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
  • types/Options/index.d.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/GraphQL/helpers/queryComplexity.js
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/docs.js
  • src/Options/index.js
  • spec/ParseGraphQLQueryComplexity.spec.js
  • types/Options/index.d.ts
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/docs.js
  • src/Options/index.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • src/Options/docs.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • src/Options/index.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93Repo: parse-community/parse-server PR: 9744File: spec/ParseLiveQuery.spec.js:0-0Timestamp: 2025-04-30T19:31:35.344ZLearning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1312-1338Timestamp: 2025-05-04T20:41:05.147ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: MoumoulsRepo: parse-community/parse-server PR: 9883File: spec/CloudCodeLogger.spec.js:410-412Timestamp: 2025-10-16T19:27:05.311ZLearning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Applied to files:

  • spec/ParseGraphQLQueryComplexity.spec.js
🧬 Code graph analysis (3)
src/Options/index.js (2)
resources/buildConfigDefinitions.js (3)
  • type (121-121)
  • type (159-159)
  • type (216-216)
src/triggers.js (1)
  • type (819-819)
spec/ParseGraphQLQueryComplexity.spec.js (1)
spec/helper.js (1)
  • reconfigureServer (180-214)
types/Options/index.d.ts (6)
src/Adapters/Analytics/AnalyticsAdapter.js (1)
  • AnalyticsAdapter (6-23)
src/Adapters/Logger/LoggerAdapter.js (1)
  • LoggerAdapter (9-18)
src/Adapters/Cache/CacheAdapter.js (1)
  • CacheAdapter (6-32)
src/Adapters/Email/MailAdapter.js (1)
  • MailAdapter (8-23)
src/Adapters/PubSub/PubSubAdapter.js (1)
  • PubSubAdapter (6-15)
src/Adapters/WebSocketServer/WSSAdapter.js (1)
  • WSSAdapter (17-57)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 49-53: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 366-366: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

[error] 373-373: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: Node 22
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: Node 20
  • GitHub Check: Redis Cache
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Docker Build
  • GitHub Check: Benchmarks

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/Options/Definitions.js (1)

399-404:Thefields description still incorrectly states "operations" instead of "field selections".

This issue was flagged in a previous review and marked as addressed, but the incorrect description remains. Line 402 still describesfields as limiting "operations (queries/mutations)" when it should describe limiting the number of field selections in a single request.

Since this is generated code, the fix must be applied insrc/Options/index.js and thennpm run definitions must be executed to regenerate this file.

Based on learnings.

🧹 Nitpick comments (1)
src/Options/Definitions.js (1)

400-400:Verify the environment variable naming convention.

The environment variablePARSE_SERVER_MAX_GRAPH_QLQUERY_COMPLEXITY has inconsistent underscore placement. Based on other GraphQL-related variables in this file (e.g.,PARSE_SERVER_GRAPHQL_PATH on line 298), it should likely bePARSE_SERVER_MAX_GRAPHQL_QUERY_COMPLEXITY withGRAPHQL as a single word.

Since this is generated code, verify and correct the naming insrc/Options/index.js, then regenerate by runningnpm run definitions.

Based on learnings.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between5d405e9 and1b6d5c7.

📒 Files selected for processing (2)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Options/docs.js
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/Definitions.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.917ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/Definitions.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • src/Options/Definitions.js

@Moumouls
Copy link
MemberAuthor

I suggest that developers set the parameters sufficiently high to prevent client issues, since the goal is to prevent major abuse, not to match the most complex query in the codebase.

 maxGraphQLQueryComplexity: {          depth: 50,          fields: 250,        },
maxIncludeQueryComplexity: {          depth: 10,          count: 50,        },

coderabbitai[bot]
coderabbitaibot previously approved these changesNov 9, 2025
@mtrezza
Copy link
Member

This is not a breaking change, correct? We'll make it a breaking change with#9928.

@Moumouls
Copy link
MemberAuthor

exactly@mtrezza no breaking change currently

mtrezza reacted with thumbs up emoji

@mtrezzamtrezza changed the titlefeat: Max depth and max fields protection systemfeat: Add Parse Server optionsmaxIncludeQueryComplexity,maxGraphQLQueryComplexity to limit query complexity for performance protectionNov 17, 2025
@parse-communityparse-community deleted a comment fromcoderabbitaibotNov 18, 2025
@parse-communityparse-community deleted a comment fromcoderabbitaibotNov 18, 2025
@Moumouls
Copy link
MemberAuthor

Moumouls commentedNov 22, 2025•
edited
Loading

@mtrezza i currently don't see a proper time window on my side to ship this correctly asap, i suggest to go ahead, it will be an opt in in PS9 and a breaking change for PS10, what do you think ?

@mtrezza
Copy link
Member

mtrezza commentedNov 22, 2025•
edited
Loading

I think we could go ahead and merge and add this as a caveat. I've opened#9939 to track the feature improvement. If you could just fix the conflict and#9920 (comment).

…xity# Conflicts:#src/GraphQL/ParseGraphQLServer.js
@Moumouls
Copy link
MemberAuthor

it's fixed@mtrezza we will have a merge conflict also between this one and apollo server update

mtrezza reacted with thumbs up emoji

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/Options/index.js (1)

1-1:Consider a more concise PR title with scope for the changelog.

The current title is quite long. For a clearer changelog entry, consider:

feat(security): add query complexity limits for REST and GraphQL APIs

This follows Angular convention with a scope and is more concise while still clearly conveying the feature's impact.

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between251e9b8 and025ea8a.

📒 Files selected for processing (4)
  • src/GraphQL/ParseGraphQLServer.js (2 hunks)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/GraphQL/ParseGraphQLServer.js
  • src/Options/docs.js
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:24.824ZLearning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • src/Options/Definitions.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • src/Options/index.js
🧬 Code graph analysis (1)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 49-53: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 368-368: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

[error] 377-377: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: Redis Cache
  • GitHub Check: Node 18
  • GitHub Check: Node 20
  • GitHub Check: Node 22
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: Benchmarks
  • GitHub Check: Docker Build
🔇 Additional comments (2)
src/Options/Definitions.js (1)

1-5:Note: This is auto-generated code.

Based on lines 1-5, this file is generated byresources/buildConfigDefinitions.js fromsrc/Options/index.js. Any issues should be fixed in the source file and then regenerated by runningnpm run definitions.

src/Options/index.js (1)

46-53:LGTM! Type definitions are well-structured.

The separate type definitions forGraphQLQueryComplexityOptions andIncludeComplexityOptions correctly reflect their different runtime shapes, and all fields are properly marked optional to match actual usage patterns.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/Options/index.js (1)

358-375:Option docs and HTML formatting for query complexity look good; minor follow-ups

The newmaxIncludeQueryComplexity /maxGraphQLQueryComplexity JSDoc blocks:

  • Use the required HTML formatting (<br><br>,<ul><li>) so they are parsable by the docs generator.
  • Correctly describedepth andcount /fields in terms of include paths vs field selections and mention the cross‑option constraint, matching the validator behavior.

Two optional polish ideas:

  • Consider explicitly noting thatdepth /count /fields are individually optional in the prose (since the types allow partial objects), e.g. “Supported keys:depth,count …”.
  • Suggested Angular-style PR title for the changelog:
    feat(graphql,rest): add query complexity limits for GraphQL and REST includes.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between025ea8a and672dda1.

📒 Files selected for processing (3)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Options/docs.js
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:24.824ZLearning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • src/Options/Definitions.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:24.824Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:24.824ZLearning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Applied to files:

  • src/Options/index.js
🧬 Code graph analysis (2)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
src/Options/index.js (2)
src/triggers.js (1)
  • type (821-821)
resources/buildConfigDefinitions.js (3)
  • type (121-121)
  • type (159-159)
  • type (216-216)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 49-53: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 367-367: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

[error] 375-375: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Node 22
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: Docker Build
  • GitHub Check: Redis Cache
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: Benchmarks
  • GitHub Check: MongoDB 7, ReplicaSet
🔇 Additional comments (2)
src/Options/index.js (1)

46-53:Flow type aliases correctly model complexity options; Biome warnings are benign

GraphQLQueryComplexityOptions /IncludeComplexityOptions with optionaldepth /fields /count align with the runtime behavior (supporting partial configs) and resolve earlier type-shape issues for these options. The Biome parse errors abouttype aliases and? are just the TS parser misreading this Flow-typed file; there’s nothing to change code-wise, but you may want to exclude this file or configure Biome to treat it as Flow.

src/Options/Definitions.js (1)

399-410:Definitions for new complexity options are consistent with Options/index.js

ThemaxGraphQLQueryComplexity /maxIncludeQueryComplexity entries correctly:

  • Use the intended env vars (PARSE_SERVER_MAX_GRAPH_QLQUERY_COMPLEXITY,PARSE_SERVER_MAX_INCLUDE_QUERY_COMPLEXITY).
  • Mirror the HTML-formatted help text and semantics fromsrc/Options/index.js, including the field-selection vs include-path distinction and the “include < graphQL” constraint.
  • Parse viaobjectParser, matching how other structured options are handled.

This confirmsnpm run definitions has been run and keeps options/docs in sync; no changes needed here. (README coverage can follow later with the planned docs task.)

<li>`depth`: Maximum depth of nested includes (e.g., foo.bar.baz = depth 3)</li>
<li>`count`: Maximum number of include fields (e.g., foo,bar,baz = 3 fields)</li>
</ul>
If both `maxIncludeQueryComplexity` and `maxGraphQLQueryComplexity` are provided, `maxIncludeQueryComplexity` values must be lower than `maxGraphQLQueryComplexity` values to avoid validation conflicts. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Add note about includeAll not usable

@mtrezza
Copy link
Member

mtrezza commentedNov 24, 2025•
edited
Loading

@Moumouls if we set default values at some point with#9928, but the value0 means no depth is allowed, what should a user set to disable the limits?undefined may be complicated as it's almost the same as not defined, and I don't think we make such nuanced distinctions for other options. It would be good to consider this already in this PR, so we don't have to make large changes later.

Should we define a value < 0 as disabling the options?

maxIncludeQueryComplexity: {  depth: -1,  count: -1,}

@mtrezza
Copy link
Member

@Moumouls Friendly ping so we can merge this.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
spec/RestQuery.spec.js (1)

1019-1188:Fixpaths typo in configs and consider tightening the “queries without includes” tests

The includeAll /include('*') and “no limits configured” blocks look good: they correctly assert that:

  • includeAll /include('*') are rejected withINVALID_QUERY when any include complexity limit is set,
  • but still work with master key and when no complexity limits are configured.

Two issues in the “Queries without includes” section though:

  1. Config key typo (paths vscount)

Both tests configure:

maxIncludeQueryComplexity:{depth:1,paths:1,},

but the option surface and validator usecount, notpaths. This means the tests never exercise the count branch and are inconsistent with the public option shape.

Changepaths tocount in both configs:

       maxIncludeQueryComplexity: {         depth: 1,-        paths: 1,+        count: 1,       },
  1. Two tests currently cover the same scenario

'should allow queries without includes regardless of complexity limits' and'should allow queries with empty includes array' currently execute identical logic (both justnew Parse.Query('SimpleObject').find() without any includes), so they don’t differentiate “no includes” from “empty includes”.

If you want distinct coverage, you could, for example, turn the second test into an explicit REST-level call withinclude= or use the SDK’s empty-include form so it truly hits the “empty includes” code path; otherwise, consider dropping or renaming it to avoid confusion. This is optional and doesn’t affect runtime behavior.

Also applies to: 1191-1230, 1232-1267

🧹 Nitpick comments (3)
src/Options/index.js (1)

358-375:Surface includeAll / include("*") restrictions in the option docs and keep generated docs in sync

The option comments describe depth / count / fields and the cross-limit rule, but they don’t mention that whenmaxIncludeQueryComplexity is configured, wildcard includes viaincludeAll() andinclude('*') are rejected for non-master / non-maintenance requests, as exercised inREST Query Complexity tests. That’s important behavior for operators.

Consider extending themaxIncludeQueryComplexity comment with a short HTML-formatted note about wildcard includes being disabled under these limits (and that explicit include paths should be used instead), then runnpm run definitions sosrc/Options/docs.js andsrc/Options/Definitions.js pick up the updated wording as well.

Example diff for this file only:

   /* Maximum query complexity for REST API includes. Controls depth and number of include fields.   <br><br>   Format: `{ depth: number, count: number }`   <ul>   <li>`depth`: Maximum depth of nested includes (e.g., foo.bar.baz = depth 3)</li>   <li>`count`: Maximum number of include fields (e.g., foo,bar,baz = 3 fields)</li>-  </ul>+  </ul>+  <br>+  When this option is configured, wildcard includes via `includeAll()` or `include("*")` are disabled for non-maintenance and non-master key requests to prevent uncontrolled include expansion. Use explicit include paths instead.+  <br><br>   If both `maxIncludeQueryComplexity` and `maxGraphQLQueryComplexity` are provided, `maxIncludeQueryComplexity` values must be lower than `maxGraphQLQueryComplexity` values to avoid validation conflicts. */   maxIncludeQueryComplexity: ?IncludeComplexityOptions;

Also, per the changelog guidelines, a concise Angular-style PR title that reflects this feature could be:

feat(security): add query complexity limits for GraphQL and REST includes

src/Options/docs.js (1)

74-75:Docs for new complexity options are aligned; extend via generator if you add includeAll note

These@property entries formaxGraphQLQueryComplexity andmaxIncludeQueryComplexity correctly describe:

  • the option shapes ({ depth, fields } vs{ depth, count }),
  • what each dimension controls, and
  • the cross-limit constraint between REST includes and GraphQL complexity.

If you adopt the suggested includeAll /include("*") note insrc/Options/index.js, please rely onnpm run definitions to regenerate this file instead of editing it directly, so the JSDoc and generated docs stay in sync.

src/Options/Definitions.js (1)

399-410:Generated definitions for new options look correct; keep them driven fromOptions/index.js

ThemaxGraphQLQueryComplexity andmaxIncludeQueryComplexity entries have:

  • sensible env var names,
  • help text consistent with the Flow option docs (depth vs fields / count and the cross-limit rule),
  • and useobjectParser, which matches the{ depth, fields/count } structure.

Given this file is generated, any further wording tweaks (for example, if you add an includeAll /include("*") restriction note insrc/Options/index.js) should be made inOptions/index.js and then propagated here vianpm run definitions, not edited by hand. No changes needed to this snippet itself.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between672dda1 and996bdc2.

📒 Files selected for processing (5)
  • spec/RestQuery.spec.js (1 hunks)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (2 hunks)
  • src/RestQuery.js (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/RestQuery.js
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:24.824ZLearning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/docs.js
  • spec/RestQuery.spec.js
  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-08T13:46:04.940ZLearning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Applied to files:

  • src/Options/docs.js
  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:48.786ZLearning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Applied to files:

  • src/Options/docs.js
  • src/Options/Definitions.js
  • src/Options/index.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9858File: src/GraphQL/ParseGraphQLServer.js:176-178Timestamp: 2025-09-21T15:43:32.265ZLearning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Applied to files:

  • src/Options/docs.js
  • src/Options/Definitions.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93Repo: parse-community/parse-server PR: 9744File: spec/ParseLiveQuery.spec.js:0-0Timestamp: 2025-04-30T19:31:35.344ZLearning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: spec/CloudCode.spec.js:446-469Timestamp: 2025-08-26T14:06:31.853ZLearning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Applied to files:

  • spec/RestQuery.spec.js
  • src/Options/index.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1312-1338Timestamp: 2025-05-04T20:41:05.147ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: src/triggers.js:467-477Timestamp: 2025-08-27T12:33:06.237ZLearning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: MoumoulsRepo: parse-community/parse-server PR: 9883File: spec/CloudCodeLogger.spec.js:410-412Timestamp: 2025-10-16T19:27:05.311ZLearning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDevRepo: parse-community/parse-server PR: 9770File: src/triggers.js:446-454Timestamp: 2025-08-27T09:08:34.252ZLearning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 9445File: spec/ParseLiveQuery.spec.js:1340-1375Timestamp: 2025-05-09T09:59:06.289ZLearning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/RestQuery.spec.js
📚 Learning: 2025-11-17T15:02:24.824Z
Learnt from: mtrezzaRepo: parse-community/parse-server PR: 0File: :0-0Timestamp: 2025-11-17T15:02:24.824ZLearning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Applied to files:

  • src/Options/index.js
🧬 Code graph analysis (2)
spec/RestQuery.spec.js (3)
spec/helper.js (2)
  • reconfigureServer (180-214)
  • Parse (4-4)
spec/ParseQuery.FullTextSearch.spec.js (1)
  • Parse (4-4)
src/RestQuery.js (3)
  • query (373-382)
  • query (383-383)
  • results (801-801)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 49-53: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

TypeScript only syntax

(parse)

[error] 367-367: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

[error] 375-375: Expected a statement but instead found '?'.

Expected a statement here.

(parse)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Node 22
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Node 20
  • GitHub Check: Redis Cache
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Docker Build
  • GitHub Check: Benchmarks
🔇 Additional comments (2)
src/Options/index.js (1)

46-53:GraphQL / include complexity option types look consistent with runtime behavior

GraphQLQueryComplexityOptions andIncludeComplexityOptions correctly model the two dimensions used at runtime (depth +fields vsdepth +count), and marking all properties optional matches how the validator accepts partial configs in tests and Config. No changes needed here.

spec/RestQuery.spec.js (1)

769-1016:Depth and combined depth+count coverage is thorough and matches the option semantics

ThemaxIncludeQueryComplexity.depth and “Combined depth and fields validation” suites exercise:

  • Allowed vs rejected depth-only queries (depth: 2 vsdepth: 1 withpost.author).
  • Multiple sibling nested paths under the same depth budget (post.author +post.category).
  • Master-key and maintenance-key bypasses for over-depth includes.
  • A scenario where depth is generous butcount is tight, ensuring the count limit alone can reject a query.

This aligns well with how thedepth andcount options are documented and implemented. No changes needed here.

Comment on lines +697 to +767
it('should allow queries with master key even when exceeding fields limit',async()=>{
awaitreconfigureServer({
maxIncludeQueryComplexity:{
count:2,
},
});

// Create test objects with relationships
constuser=newParse.User();
user.setUsername('testuser3');
user.setPassword('password');
awaituser.signUp(null,{useMasterKey:true});

constpost=newParse.Object('Post');
post.set('title','Test Post');
post.set('author',user);
awaitpost.save(null,{useMasterKey:true});

constcomment=newParse.Object('Comment');
comment.set('text','Test Comment');
comment.set('post',post);
awaitcomment.save(null,{useMasterKey:true});

// Query with include that exceeds limit but using master key
constquery=newParse.Query('Comment');
query.include('post');
query.include('post.author');
constresults=awaitquery.find({useMasterKey:true});

expect(results.length).toBeGreaterThan(0);
});

it('should allow queries with maintenance key even when exceeding fields limit',async()=>{
awaitreconfigureServer({
maintenanceKey:'maintenanceKey456',
maxIncludeQueryComplexity:{
count:2,
},
});

// Create test objects with relationships using Parse SDK
constuser=newParse.User();
user.setUsername('testuser4');
user.setPassword('password');
awaituser.signUp();

constpost=newParse.Object('Post');
post.set('title','Test Post');
post.set('author',user);
awaitpost.save();

constcomment=newParse.Object('Comment');
comment.set('text','Test Comment');
comment.set('post',post);
awaitcomment.save();

// Query with include that exceeds limit but using maintenance key via REST API
constheaders={
'X-Parse-Application-Id':'test',
'X-Parse-REST-API-Key':'rest',
'X-Parse-Maintenance-Key':'maintenanceKey456',
};
constresponse=awaitrequest({
headers,
url:`http://localhost:8378/1/classes/Comment?include=post,post.author`,
json:true,
});

expect(response.data.results.length).toBeGreaterThan(0);
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

⚠️ Potential issue |🟠 Major

Master / maintenance count tests never exceed the configured limit

In both:

  • it('should allow queries with master key even when exceeding fields limit', …)
  • it('should allow queries with maintenance key even when exceeding fields limit', …)

you configure:

maxIncludeQueryComplexity:{count:2,},

but the queries only include:

query.include('post');query.include('post.author');

which is 2 include paths, i.e. at or below the limit. These tests would succeed even for non-privileged requests, so they don’t actually verify the intended master / maintenance bypass for an over-limit query.

To exercise the bypass, mirror the earlier “reject queries exceeding fields limit” case by adding a 3rd include (e.g.reply) and wiring the extra relation:

@@-      const comment = new Parse.Object('Comment');-      comment.set('text', 'Test Comment');-      comment.set('post', post);-      await comment.save(null, { useMasterKey: true });+      const reply = new Parse.Object('Comment');+      reply.set('text', 'Test Reply');+      await reply.save(null, { useMasterKey: true });++      const comment = new Parse.Object('Comment');+      comment.set('text', 'Test Comment');+      comment.set('post', post);+      comment.set('reply', reply);+      await comment.save(null, { useMasterKey: true });@@-      query.include('post');-      query.include('post.author');+      query.include('post');+      query.include('post.author');+      query.include('reply');@@-      const results = await query.find({ useMasterKey: true });+      const results = await query.find({ useMasterKey: true });

and analogously for the maintenance-key test:

@@-      const comment = new Parse.Object('Comment');-      comment.set('text', 'Test Comment');-      comment.set('post', post);-      await comment.save();+      const reply = new Parse.Object('Comment');+      reply.set('text', 'Test Reply');+      await reply.save();++      const comment = new Parse.Object('Comment');+      comment.set('text', 'Test Comment');+      comment.set('post', post);+      comment.set('reply', reply);+      await comment.save();@@-        url: `http://localhost:8378/1/classes/Comment?include=post,post.author`,+        url: `http://localhost:8378/1/classes/Comment?include=post,post.author,reply`,

This way, only master / maintenance requests succeed for an over-limit include set, and the tests validate the bypass path.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@mtrezzamtrezzamtrezza left review comments

@coderabbitaicoderabbitai[bot]coderabbitai[bot] requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Moumouls@parseplatformorg@mtrezza
[8]ページ先頭
©2009-2025 Movatter.jp