Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DEPRECATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The following is a list of deprecations, according to the [Deprecation Policy](h
| DEPPS15 | Config option `readOnlyMasterKeyIps` defaults to `['127.0.0.1', '::1']` | [#10115](https://github.com/parse-community/parse-server/pull/10115) | 9.5.0 (2026) | 10.0.0 (2027) | deprecated | - |
| DEPPS16 | Remove config option `mountPlayground` | [#10110](https://github.com/parse-community/parse-server/issues/10110) | 9.5.0 (2026) | 10.0.0 (2027) | deprecated | - |
| DEPPS17 | Remove config option `playgroundPath` | [#10110](https://github.com/parse-community/parse-server/issues/10110) | 9.5.0 (2026) | 10.0.0 (2027) | deprecated | - |
| DEPPS18 | Config option `requestComplexity` defaults to enabled with limits | [#10204](https://github.com/parse-community/parse-server/pull/10204) | 9.6.0 (2026) | 10.0.0 (2027) | deprecated | - |

[i_deprecation]: ## "The version and date of the deprecation."
[i_change]: ## "The version and date of the planned change."
Expand Down
36 changes: 35 additions & 1 deletion spec/RequestComplexity.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,31 @@ describe('request complexity', () => {
expect(config.requestComplexity.graphQLFields).toBe(200);
});

it('should apply full defaults when not configured', async () => {
it('should have requestComplexity undefined when not configured', async () => {
await reconfigureServer({});
const config = Config.get('test');
expect(config.requestComplexity).toBeUndefined();
});

it('should apply no limits when requestComplexity is undefined', async () => {
await reconfigureServer({});
const config = Config.get('test');
expect(config.requestComplexity).toBeUndefined();

const where = buildNestedInQuery(15);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeResolved();

const includes = Array.from({ length: 100 }, (_, i) => `field${i}`).join(',');
await expectAsync(
rest.find(config, auth.nobody(config), '_User', {}, { include: includes })
).toBeResolved();
});

it('should apply full defaults when empty object is passed', async () => {
await reconfigureServer({ requestComplexity: {} });
const config = Config.get('test');
expect(config.requestComplexity).toEqual({
includeDepth: 5,
includeCount: 50,
Expand All @@ -127,6 +149,18 @@ describe('request complexity', () => {
graphQLFields: 200,
});
});

it('should log deprecation warning when requestComplexity is not set', async () => {
const Deprecator = require('../lib/Deprecator/Deprecator');
const logSpy = spyOn(Deprecator, '_logOption').and.callThrough();
await reconfigureServer({});
expect(logSpy).toHaveBeenCalledWith(
jasmine.objectContaining({
optionKey: 'requestComplexity',
changeNewDefault: jasmine.stringMatching(/includeDepth.*10/),
})
);
});
});

describe('subquery depth', () => {
Expand Down
5 changes: 5 additions & 0 deletions src/Deprecator/Deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,9 @@ module.exports = [
changeNewKey: '',
solution: "Use Parse Dashboard as GraphQL IDE or configure a third-party GraphQL client such as Apollo Sandbox, GraphiQL, or Insomnia with custom request headers.",
},
{
optionKey: 'requestComplexity',
changeNewDefault: '{"includeDepth":10,"includeCount":100,"subqueryDepth":10,"graphQLDepth":20,"graphQLFields":200}',
solution: "Set 'requestComplexity' to the limits you want to enforce, or to the above object to opt-in to the future default behavior.",
},
];
1 change: 0 additions & 1 deletion src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,6 @@ module.exports.ParseServerOptions = {
help: 'Options to limit the complexity of requests to prevent denial-of-service attacks. Limits are enforced for all requests except those using the master or maintenance key. Each property can be set to `-1` to disable that specific limit.',
action: parsers.objectParser,
type: 'RequestComplexityOptions',
default: {},
},
requestContextMiddleware: {
env: 'PARSE_SERVER_REQUEST_CONTEXT_MIDDLEWARE',
Expand Down
3 changes: 1 addition & 2 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,7 @@ export interface ParseServerOptions {
/* Callback when server has closed */
serverCloseComplete: ?() => void;
/* Options to limit the complexity of requests to prevent denial-of-service attacks. Limits are enforced for all requests except those using the master or maintenance key. Each property can be set to `-1` to disable that specific limit.
:ENV: PARSE_SERVER_REQUEST_COMPLEXITY
:DEFAULT: {} */
:ENV: PARSE_SERVER_REQUEST_COMPLEXITY */
requestComplexity: ?RequestComplexityOptions;
/* The security options to identify and report weak security settings.
:DEFAULT: {} */
Expand Down
2 changes: 1 addition & 1 deletion src/Security/CheckGroups/CheckGroupServerConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class CheckGroupServerConfig extends CheckGroup {
check: () => {
const rc = config.requestComplexity;
if (!rc) {
throw 1;
return;
}
Comment on lines 145 to 147
Copy link

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid reporting disabled request-complexity protection as a passed security check.

At Line 146, return causes this check to be marked as success (✅) rather than skipped/failed, so reports can incorrectly imply limits are enabled when requestComplexity is unset.

🔧 Minimal fix at the root cause
         check: () => {
           const rc = config.requestComplexity;
           if (!rc) {
-            return;
+            throw 1;
           }
           const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.graphQLDepth, rc.graphQLFields];
           if (values.some(v => v === -1)) {
             throw 1;
           }
         },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!rc) {
throw 1;
return;
}
check: () => {
const rc = config.requestComplexity;
if (!rc) {
throw 1;
}
const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.graphQLDepth, rc.graphQLFields];
if (values.some(v => v === -1)) {
throw 1;
}
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Security/CheckGroups/CheckGroupServerConfig.js` around lines 145 - 147,
The early return when requestComplexity is not present (the block with "if (!rc)
{ return; }" referencing the rc variable and requestComplexity) causes the check
to be reported as passed; change this to explicitly mark the check as skipped or
failed instead of returning—e.g., set the check result state (skip/failed) via
the module's reporting API (use the same result object/manipulator used
elsewhere in CheckGroupServerConfig.js) or call the existing helper like
skipCheck/markFailed with a message like "requestComplexity not configured" so
the report does not show this as a successful check.

Copy link
Member Author

Choose a reason for hiding this comment

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

we should authorize the undefined option in PS9 it will be changed to throw on PS10

Copy link

Choose a reason for hiding this comment

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

@Moumouls, understood — the return is intentional for PS9. Since requestComplexity is opt-in during this transition period, returning early (instead of throwing) correctly avoids flagging the check as failed for operators who haven't configured the option yet. The deprecation entry (DEPPS18) and updated help text handle communicating the upcoming PS10 default. My suggestion to revert to throw 1 was incorrect in this context — apologies for the noise.


✏️ Learnings added
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 10204
File: src/Security/CheckGroups/CheckGroupServerConfig.js:145-147
Timestamp: 2026-03-14T21:39:29.537Z
Learning: In Parse Server's `src/Security/CheckGroups/CheckGroupServerConfig.js`, the "Request complexity limits enabled" security check intentionally uses `return` (not `throw 1`) when `requestComplexity` is `undefined` in PS9. This is by design: `requestComplexity` is opt-in in the current major version and will only become a hard-failing security check (throw 1 when undefined) in PS10. The deprecation DEPPS18 in `src/Deprecator/Deprecations.js` communicates this upcoming default change to operators.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10130
File: src/RestQuery.js:890-910
Timestamp: 2026-03-07T22:42:53.791Z
Learning: In Parse Server's RestQuery.js, `validateIncludeComplexity` intentionally counts raw `this.include.length` (not deduplicated paths) for the `includeCount` limit. This is by design: the limit caps total request complexity, not unique paths. Deduplicating before the check would weaken the protection. Note: user-supplied include strings are already deduplicated at construction via `pathSet`, but `handleIncludeAll` may add array references with reference-inequality duplicates — that is a pre-existing concern separate from this security check.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10175
File: src/LiveQuery/ParseLiveQueryServer.ts:919-952
Timestamp: 2026-03-10T18:04:39.708Z
Learning: In Parse Server LiveQuery (`src/LiveQuery/ParseLiveQueryServer.ts`), the `addProtectedFields` method in `DatabaseController` uses **intersection** logic: it computes a protected set for each applicable ACL group (`*`, `userId`, `role:X`) and intersects them. Role entries serve as **exemptions** (they narrow/reduce the protected set for that role), not additions. Consequently, passing `userRoles: []` when building the `auth` object for the protected-fields WHERE-clause check is deliberately more restrictive (not less): without role sets, only the `'*'` set applies, which is the maximum protection. The only correctness gap is a rare role-only config (`{ 'role:X': ['field'] }` with no `'*'` entry), where omitting roles yields an empty intersection and the field is unprotected for the role member — but unprivileged users are unaffected, so this is not a security escalation. The same pattern exists pre-existing in `_filterSensitiveData`. This is intentional and documented with tests.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10149
File: src/Controllers/DatabaseController.js:1865-1873
Timestamp: 2026-03-08T21:51:35.204Z
Learning: In `src/Controllers/DatabaseController.js` (`performInitialization`), the `ensureAuthDataUniqueness` index creation is intentionally non-fatal (warn-only, no rethrow). Unlike username/email uniqueness which has been enforced since Parse Server's inception, authData uniqueness is new. Existing production databases may have duplicate authData entries caused by the race condition this PR fixes, so crashing on startup would be a breaking change. The application-level `ensureUniqueAuthDataId` check in `RestWrite.js` still runs as a fallback, and the warning log gives operators visibility to resolve duplicates manually before the index can be created successfully.

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 }.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: 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: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
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-17T15:02:24.824Z
Learning: 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.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T06:55:53.808Z
Learning: When reviewing Parse Server PRs that add or modify Parse Server options, always verify that changes are properly reflected in three files: src/Options/index.js (where changes originate), src/Options/Definitions.js, and src/Options/docs.js. The correct workflow is: make changes in index.js first, then run `npm run definitions` to automatically replicate the changes to Definitions.js and docs.js.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
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.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10117
File: benchmark/performance.js:713-737
Timestamp: 2026-03-06T21:30:03.253Z
Learning: In Parse Server LiveQuery, use liveQuery: { classNames: [...] } in the server config to specify which classes emit pub/sub events (server-side producers). Use ParseServer.createLiveQueryServer(httpServer, { appId, masterKey, serverURL }) to start the WebSocket server that forwards events to subscribed clients (event consumers); this function does not require a classNames parameter, as it simply matches incoming events against active client subscriptions.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10117
File: benchmark/performance.js:52-52
Timestamp: 2026-03-06T21:30:10.326Z
Learning: In reviews of JavaScript files, ensure separation of concerns for LiveQuery in Parse Server: do not rely on the LiveQuery server to perform className filtering. The filtering and class event emission should occur in the ParseServer configuration (classNames within liveQuery) or related server logic, while ParseServer.createLiveQueryServer(httpServer, { appId, masterKey, serverURL }) should only establish the WebSocket-based delivery of already-filtered events. Treat classNames filtering as the responsibility of ParseServer, not the LiveQuery server, and verify that LiveQuery code receives pre-filtered events and does not duplicate filtering logic.

const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.graphQLDepth, rc.graphQLFields];
if (values.some(v => v === -1)) {
Expand Down
Loading