Skip to content

Conversation

@coratgerl
Copy link
Contributor

@coratgerl coratgerl commented Oct 27, 2025

Pull Request

Issue

Fixes: #7519

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

Summary by CodeRabbit

  • New Features

    • Adds a Database option allowPublicExplain (defaults to true) to control whether explain queries can run without the master key; includes security guidance and config validation.
  • Behavior Changes

    • Explain queries require the master key when allowPublicExplain is set to false; runtime rejects unauthorized explain requests when disabled.
  • Deprecations

    • allowPublicExplain deprecation entry added (DEPPS12; noted for 8.5.0, removal in 9.0.0).
  • Tests

    • Added tests and security checks covering explain behavior across configurations.

@parse-github-assistant
Copy link

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

@parse-github-assistant parse-github-assistant bot changed the title feat: add deprecation on explain feat: Add deprecation on explain Oct 27, 2025
@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 27, 2025

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Oct 27, 2025

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

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

📝 Walkthrough

Walkthrough

Adds a DatabaseOptions.allowPublicExplain flag (default true, deprecated), validates it, enforces that non-master explain requests are blocked when set false, prevents forwarding the option to the Mongo adapter, registers a deprecation entry, and adds tests covering explain behavior with/without master key.

Changes

Cohort / File(s) Summary
Options definitions & types
src/Options/Definitions.js, src/Options/index.js, src/Options/docs.js, types/Options/index.d.ts
Add databaseOptions.allowPublicExplain (env PARSE_SERVER_DATABASE_ALLOW_PUBLIC_EXPLAIN), boolean parser, help text with security warning, and default true.
Configuration validation
src/Config.js
Validate databaseOptions.allowPublicExplain: apply default when undefined and require a boolean type.
Deprecations & docs
DEPRECATIONS.md, src/Deprecator/Deprecations.js
Add deprecation entry for databaseOptions.allowPublicExplain (DEPPS12) indicating planned default change.
Runtime enforcement
src/rest.js
In runFindTriggers, if restOptions.explain is requested by a non-master, check databaseOptions.allowPublicExplain and throw Parse.Error.INVALID_QUERY when false.
Mongo adapter filtering
src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Stop forwarding allowPublicExplain into adapter _mongoOptions; minor formatting tweak only.
Security checks
src/Security/CheckGroups/CheckGroupServerConfig.js, spec/SecurityCheckGroups.spec.js
Add a server-config check "Public database explain disabled" and tests asserting success/failure depending on allowPublicExplain.
Tests
spec/ParseQuery.spec.js
Import Deprecator and add tests covering explain behavior for allowPublicExplain: true, allowPublicExplain: false, and default (undefined) with/without master key (note: test block duplicated).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ParseServer
    participant Config as ConfigLayer
    participant MongoDB

    Client->>ParseServer: GET /classes/...?explain=1 (with/without masterKey)
    ParseServer->>Config: Read databaseOptions.allowPublicExplain
    Config-->>ParseServer: allowPublicExplain (default: true)

    alt explain requested && not master && allowPublicExplain = false
        ParseServer->>Client: Error INVALID_QUERY ("Using the explain query parameter requires the master key")
    else proceed
        ParseServer->>MongoDB: Run query with explain
        MongoDB-->>ParseServer: Explain result
        ParseServer-->>Client: Explain result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review src/rest.js auth check and exact error/message.
  • Verify Config.validateDatabaseOptions defaulting and type enforcement.
  • Confirm MongoStorageAdapter option filtering does not remove other keys.
  • Check new security check messages and tests (including duplicated test block in spec/ParseQuery.spec.js).

Possibly related PRs

Suggested reviewers

  • Moumouls

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description references the linked issue but marks 'Add security check' as unchecked, though the changeset includes a new security check in CheckGroupServerConfig.js. Verify that the 'Add security check' checkbox accurately reflects the implementation. The code adds a security check, but the box is unchecked.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately and concisely summarizes the main feature: adding an allowPublicExplain option to control access to Parse.Query.explain.
Linked Issues check ✅ Passed The implementation successfully restricts explain to master key by default while allowing configuration, deprecates the permissive behavior, and includes tests and documentation updates as required.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the allowPublicExplain feature and restricting explain to master key, including option definition, validation, enforcement, tests, and security checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ca29fc and 7cab191.

📒 Files selected for processing (2)
  • spec/SecurityCheckGroups.spec.js (1 hunks)
  • src/Security/CheckGroups/CheckGroupServerConfig.js (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: 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/SecurityCheckGroups.spec.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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/Security/CheckGroups/CheckGroupServerConfig.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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/Security/CheckGroups/CheckGroupServerConfig.js
🧬 Code graph analysis (1)
spec/SecurityCheckGroups.spec.js (2)
spec/helper.js (1)
  • reconfigureServer (180-214)
src/Security/Check.js (1)
  • CheckState (75-79)
⏰ 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: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: Redis Cache
  • GitHub Check: Node 22
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Docker Build
🔇 Additional comments (2)
spec/SecurityCheckGroups.spec.js (1)

64-82: LGTM! Well-structured MongoDB-specific tests.

The tests correctly validate the new security check for public database explain:

  • Success case when allowPublicExplain: false (secure configuration)
  • Failure case when allowPublicExplain: true (insecure configuration)

The use of it_only_db('mongo') is appropriate since explain is a MongoDB-specific feature, and the tests properly check the 7th security check (index 6).

src/Security/CheckGroups/CheckGroupServerConfig.js (1)

93-107: LGTM! Security check correctly implemented.

The check properly enforces that public database explain queries should be explicitly disabled for security:

  • Fails when allowPublicExplain is true or undefined/null (insecure configurations)
  • Succeeds only when explicitly set to false (secure configuration)
  • Clear warning explains the risk of exposing database performance information and schema details
  • Solution provides actionable guidance to users

The use of optional chaining and == null to handle both undefined and null cases is appropriate and follows common patterns for defensive configuration checking.


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.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot 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: 3

🧹 Nitpick comments (1)
spec/ParseQuery.spec.js (1)

5407-5411: Clean up commented code or document the intention.

The commented-out code suggests this deprecation is planned but not yet enforced. If this is a phased approach (warn now, enforce later), consider adding a comment explaining the timeline:

-        // fail('Should have thrown an error');
       } catch (error) {
-        // Uncomment this after the Deprecation DEPPS12
-        // expect(error.code).toEqual(Parse.Error.INVALID_QUERY);
-        // expect(error.message).toEqual('Using the explain query parameter without the master key');
+        // TODO: Enforce restriction in next major version (DEPPS12)
+        // expect(error.code).toEqual(Parse.Error.INVALID_QUERY);
+        // expect(error.message).toEqual('Using the explain query parameter without the master key');
       }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00f8d4c and 705b1ef.

📒 Files selected for processing (4)
  • DEPRECATIONS.md (1 hunks)
  • changelogs/CHANGELOG_alpha.md (1 hunks)
  • spec/ParseQuery.spec.js (1 hunks)
  • src/rest.js (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
PR: parse-community/parse-server#9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: 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/rest.js
🧬 Code graph analysis (2)
src/rest.js (1)
spec/helper.js (1)
  • auth (394-394)
spec/ParseQuery.spec.js (2)
spec/helper.js (2)
  • TestObject (279-281)
  • Parse (4-4)
src/rest.js (1)
  • Deprecator (16-16)
🪛 GitHub Check: Lint
spec/ParseQuery.spec.js

[failure] 5394-5394:
'Deprecator' is not defined

🪛 markdownlint-cli2 (0.18.1)
changelogs/CHANGELOG_alpha.md

3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

DEPRECATIONS.md

17-17: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe

(MD055, table-pipe-style)


17-17: Table column count
Expected: 7; Actual: 5; Too few cells, row will be missing data

(MD056, table-column-count)


18-18: Table column count
Expected: 7; Actual: 9; Too many cells, extra data will be missing

(MD056, table-column-count)

⏰ 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). (13)
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Node 18
  • GitHub Check: Node 20
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: Redis Cache
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Docker Build
🔇 Additional comments (3)
changelogs/CHANGELOG_alpha.md (1)

1-6: LGTM!

The changelog entry follows the established format and clearly documents the DEPPS12 deprecation with appropriate links.

src/rest.js (2)

16-16: LGTM!

The Deprecator import is correctly added to support the new deprecation logging functionality.


39-50: LGTM!

The deprecation logging is correctly implemented:

  • Properly detects when explain is used without master key authorization
  • Logs a clear deprecation message without breaking existing functionality
  • Preserves the commented error path for future enforcement when the deprecation period ends

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.07%. Comparing base (b760733) to head (7cab191).
⚠️ Report is 3 commits behind head on alpha.

Files with missing lines Patch % Lines
src/Config.js 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9890      +/-   ##
==========================================
- Coverage   93.07%   93.07%   -0.01%     
==========================================
  Files         187      187              
  Lines       15213    15223      +10     
  Branches      177      177              
==========================================
+ Hits        14160    14169       +9     
- Misses       1041     1042       +1     
  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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 27, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 27, 2025
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

This PR requires a new Parse Server config option to enable / disable explain requiring master key. The option will be deprecated, so this is not a RuntimeDeprecation but a deprecation of the config option.

The new option should be in the DatabaseOptions interface, maybe called allowPublicExplain which is true by default.

Then replace the current test with 3 tests:

  • reconfigure server with allowPublicExplain: true and ensure it works with and without master key
  • reconfigure server with allowPublicExplain: false and ensure it works with master key and not without master key
  • do not reconfigure server, i.e. don't explicitly set allowPublicExplain to test default value and ensure it works with and without master key

@coratgerl
Copy link
Contributor Author

Done @mtrezza. Note that I had to remove this line in MongoStorageAdapter because it mutates the databaseOptions. This mutation causes the test to remove the value before it is injected into ParseApp, which then defaults to using the default options. This deletion is weird since we already remove the option below in this._mongoOptions.

image

@coratgerl coratgerl requested a review from mtrezza October 29, 2025 19:27
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 29, 2025
@mtrezza
Copy link
Member

mtrezza commented Nov 6, 2025

Thanks, could you take a look at the conflicts?

@mtrezza mtrezza removed their request for review November 6, 2025 15:53
@coratgerl
Copy link
Contributor Author

Done @mtrezza :)

Copy link

@coderabbitai coderabbitai bot 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)
DEPRECATIONS.md (1)

18-18: Fix malformed DEPPS12 row (too many cells).

Row has 11 cells due to a duplicated tail; markdown/table parsing breaks.

Apply:

-| DEPPS12 | Database option `allowPublicExplain` defaults to `true` | [#7519](https://github.com/parse-community/parse-server/issues/7519) | 8.3.0 (2025)                    | 9.0.0 (2027)                    | deprecated            | -     || 9.0.0 (2027)                    | deprecated            | -     |
+| DEPPS12 | Database option `allowPublicExplain` defaults to `true` | [#7519](https://github.com/parse-community/parse-server/issues/7519) | 8.3.0 (2025)                    | 9.0.0 (2027)                    | deprecated            | -     |
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb8eedb and 0824ff6.

📒 Files selected for processing (10)
  • DEPRECATIONS.md (1 hunks)
  • changelogs/CHANGELOG_alpha.md (1 hunks)
  • spec/ParseQuery.spec.js (2 hunks)
  • src/Adapters/Storage/Mongo/MongoStorageAdapter.js (2 hunks)
  • src/Config.js (1 hunks)
  • src/Deprecator/Deprecations.js (1 hunks)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (1 hunks)
  • src/rest.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • spec/ParseQuery.spec.js
  • changelogs/CHANGELOG_alpha.md
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: 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:

  • src/rest.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: 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/rest.js
  • src/Deprecator/Deprecations.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: 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/rest.js
🧬 Code graph analysis (2)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
src/Config.js (2)
src/Controllers/DatabaseController.js (1)
  • databaseOptions (1741-1741)
types/Options/index.d.ts (1)
  • DatabaseOptions (229-241)
🪛 Biome (2.1.2)
src/Options/index.js

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

Expected a statement here.

(parse)

🪛 markdownlint-cli2 (0.18.1)
DEPRECATIONS.md

18-18: Table column count
Expected: 7; Actual: 11; Too many cells, extra data will be missing

(MD056, table-column-count)

🔇 Additional comments (6)
src/Options/index.js (1)

658-660: Expose allowPublicExplain in DatabaseOptions (Flow).

Looks good and matches docs/definitions.

Note: If Biome flags a parse error here, ensure the linter is configured for Flow or excludes this file.

src/Adapters/Storage/Mongo/MongoStorageAdapter.js (1)

149-149: Do not forward Parse‑specific options to Mongo client (incl. allowPublicExplain).

Good defensive cleanup; also preserves the original options object.

Also applies to: 158-167, 175-176

src/Options/docs.js (1)

243-243: Docs: add databaseOptions.allowPublicExplain.

Accurate description and deprecation note.

src/Deprecator/Deprecations.js (1)

21-26: Add deprecation entry for databaseOptions.allowPublicExplain.

Matches policy; message is clear.

src/Options/Definitions.js (1)

1086-1092: Generated definitions updated with allowPublicExplain.

Consistent with Options/index.js and docs.

Confirm this file was regenerated from Options/index.js (no manual edits drift).

src/rest.js (1)

38-47: Aggregate endpoint is already protected by master-key-only middleware; no additional guard needed.

The aggregate endpoint at src/Routers/AggregateRouter.js line 128 uses middleware.promiseEnforceMasterKeyAccess middleware, which rejects all non-master requests with a 403 error (src/middlewares.js:510-518). This is more restrictive than the conditional allowPublicExplain guard in the find path—non-master users cannot access aggregate at all, making the concern moot.

The original change is correct. The find and aggregate endpoints use complementary security models: find operations are conditionally gated by allowPublicExplain, while aggregate operations are completely restricted to master key access.

@coratgerl coratgerl requested a review from mtrezza November 8, 2025 13:31
Copy link

@coderabbitai coderabbitai bot 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)

728-730: LGTM with a minor documentation clarity suggestion.

The implementation is correct and follows established patterns in the codebase. The option type, default value, and placement are all appropriate for the deprecation workflow.

Documentation clarity: The comment states "This option is deprecated," which could be confusing since the option itself is newly introduced. Consider clarifying that it's the behavior of allowing public explain queries that is deprecated, and this option controls the transition:

-  /* Set to `true` to allow explain queries without master key. This option is deprecated and the default will change to `false` in a future version.
+  /* Set to `true` to allow explain queries without master key. The behavior of allowing public explain queries is deprecated and the default will change to `false` in a future version.

Static analysis note: The Biome parse error is a false positive. The ?boolean syntax is valid Flow notation for optional types, used consistently throughout this file (see line 727 and numerous other examples).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e36a4ba and 636a307.

📒 Files selected for processing (5)
  • src/Config.js (1 hunks)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (1 hunks)
  • types/Options/index.d.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/Config.js
  • src/Options/Definitions.js
  • types/Options/index.d.ts
  • src/Options/docs.js
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: 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.
🪛 Biome (2.1.2)
src/Options/index.js

[error] 728-728: 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 22
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Redis Cache
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Docker Build
  • GitHub Check: Code Analysis (javascript)

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 8, 2025
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 8, 2025
@mtrezza mtrezza changed the title feat: Add deprecation on explain feat: Add Parse Server option allowPublicExplain to enable Parse.Query.explain without master key Nov 8, 2025
@mtrezza mtrezza changed the title feat: Add Parse Server option allowPublicExplain to enable Parse.Query.explain without master key feat: Add Parse Server option allowPublicExplain to enable Parse.Query.explain without master key Nov 8, 2025
@mtrezza mtrezza changed the title feat: Add Parse Server option allowPublicExplain to enable Parse.Query.explain without master key feat: Add Parse Server option allowPublicExplain to allow Parse.Query.explain without master key Nov 8, 2025
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
Copy link

@coderabbitai coderabbitai bot 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 (5)
DEPRECATIONS.md (1)

18-18: Clarify DEPPS12 wording to reflect the deprecation path.

“will default to true” is misleading; current default is true (deprecated), and the default will change to false to restrict explain to master key.

Apply:

-| DEPPS12 | Database option `allowPublicExplain` will default to `true`                                  | [#7519](https://github.com/parse-community/parse-server/issues/7519) | 8.5.0 (2025)                    | 9.0.0 (2026)                    | deprecated            | -     |
+| DEPPS12 | Restrict `explain` to master key; `databaseOptions.allowPublicExplain` currently defaults to `true` (deprecated; default will change to `false`) | [#7519](https://github.com/parse-community/parse-server/issues/7519) | 8.5.0 (2025)                    | 9.0.0 (2026)                    | deprecated            | -     |
spec/ParseQuery.spec.js (4)

5456-5462: Default-case: omit the option instead of passing undefined.

Passing { allowPublicExplain: undefined } may bypass “option absent” checks. Omit the key to represent default.

-        await reconfigureServer({
-          databaseAdapter: undefined,
-          databaseURI: 'mongodb://localhost:27017/parse',
-          databaseOptions: {
-            allowPublicExplain: undefined,
-          },
-        });
+        await reconfigureServer({
+          databaseAdapter: undefined,
+          databaseURI: 'mongodb://localhost:27017/parse',
+          // omit databaseOptions to use default behavior
+        });

5453-5468: Prefer spying Deprecator over logger for deprecation signal.

Spying logger.warn risks coupling to log formatting. Spy Deprecator.logRuntimeDeprecation to assert deprecation is emitted, then keep functional assertions.

-        const logger = require('../lib/logger').logger;
-        const logSpy = spyOn(logger, 'warn').and.callFake(() => {});
+        const depSpy = spyOn(Deprecator, 'logRuntimeDeprecation').and.callThrough();
...
-        expect(logSpy).toHaveBeenCalledWith(
-          jasmine.stringMatching(/DeprecationWarning.*databaseOptions\.allowPublicExplain.*false/)
-        );
+        expect(depSpy).toHaveBeenCalled();

5389-5485: Add REST path coverage for allowPublicExplain=false.

Complement SDK-based tests with a REST request using ?explain=true without master key to ensure the router enforces the restriction.


5393-5400: Minor: avoid forcing adapter override on every reconfigure.

Passing databaseAdapter: undefined repeatedly is noisy. Only set databaseURI (Mongo-only block already gated by it_only_db('mongo')).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d82310 and 7ca29fc.

📒 Files selected for processing (7)
  • DEPRECATIONS.md (1 hunks)
  • spec/ParseQuery.spec.js (2 hunks)
  • src/Adapters/Storage/Mongo/MongoStorageAdapter.js (2 hunks)
  • src/Config.js (1 hunks)
  • src/Options/Definitions.js (1 hunks)
  • src/Options/docs.js (1 hunks)
  • src/Options/index.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/Options/Definitions.js
  • src/Adapters/Storage/Mongo/MongoStorageAdapter.js
  • src/Config.js
  • src/Options/index.js
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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
  • DEPRECATIONS.md
  • spec/ParseQuery.spec.js
📚 Learning: 2025-11-08T13:46:04.917Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: 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
  • DEPRECATIONS.md
  • spec/ParseQuery.spec.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: 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/ParseQuery.spec.js
📚 Learning: 2025-09-21T15:43:32.265Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: 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/ParseQuery.spec.js
🧬 Code graph analysis (1)
spec/ParseQuery.spec.js (2)
spec/helper.js (4)
  • require (6-6)
  • reconfigureServer (180-214)
  • TestObject (279-281)
  • Parse (4-4)
spec/ParseLiveQuery.spec.js (3)
  • logger (299-299)
  • logger (589-589)
  • logger (632-632)
⏰ 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: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: Node 22
  • GitHub Check: Redis Cache
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Docker Build
  • GitHub Check: MongoDB 7, ReplicaSet
🔇 Additional comments (2)
DEPRECATIONS.md (1)

5-18: Table formatting looks good now.
Cells count and trailing pipes are correct for DEPPS11/DEPPS12.

src/Options/docs.js (1)

260-260: Document the default and sync definitions.

Add “Default is true.” to align with current behavior, and ensure generated docs/definitions are updated. Based on learnings.

- * @property {Boolean} allowPublicExplain Set to `true` to allow `Parse.Query.explain` without master key.<br><br>⚠️ Enabling this option may expose sensitive query performance data to unauthorized users and could potentially be exploited for malicious purposes.
+ * @property {Boolean} allowPublicExplain Set to `true` to allow `Parse.Query.explain` without master key. Default is `true` (deprecated; the default will change to `false`).<br><br>⚠️ Enabling this option may expose sensitive query performance data to unauthorized users and could potentially be exploited for malicious purposes.

Run to verify Option presence across codegen:

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 8, 2025
@mtrezza mtrezza merged commit 4456b02 into parse-community:alpha Nov 8, 2025
24 of 26 checks passed
parseplatformorg pushed a commit that referenced this pull request Nov 8, 2025
# [8.5.0-alpha.5](8.5.0-alpha.4...8.5.0-alpha.5) (2025-11-08)

### Features

* Add Parse Server option `allowPublicExplain` to allow `Parse.Query.explain` without master key ([#9890](#9890)) ([4456b02](4456b02))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 8.5.0-alpha.5

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

Labels

state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restrict explain to the master key

3 participants