fix(rest): Prevent default ACL from overwriting custom ACL on update

[walhalabi-code]

Summary

Fixed an issue where the default ACL defined in Class Level Permissions was incorrectly applied during object updates, overwriting any existing custom ACL.

Problem

When classLevelPermissions.ACL is configured for a class, the default ACL was being applied on every save operation (both create and update). This caused existing objects to lose their custom ACL settings when updated without an explicit ACL in the request.

Solution

Added a !this.query condition check before applying the default ACL. This ensures the default ACL is only set during object creation (when this.query is null), not during updates. This behavior is consistent with how other default fields like createdAt are handled in RestWrite.

Summary by CodeRabbit

• Bug Fixes

  • Default ACLs are now applied only on object creation; updates no longer receive automatic default ACLs, preventing unintended permission changes.

• Tests

  • Added tests confirming updates do not overwrite existing ACLs and that explicit ACL changes on update are honored.

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

[parse-github-assistant]

🚀 Thanks for opening this pull request!

[parseplatformorg]

✅ 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]

📝 Walkthrough

Walkthrough

Default ACL assignment in src/RestWrite.js was restricted to CREATE operations (no query); default ACLs are no longer auto-applied on UPDATE. Two tests in spec/ParseACL.spec.js were added to validate ACL behavior on updates and explicit ACL modifications.

Changes

Cohort / File(s)	Summary
ACL Logic Update src/RestWrite.js	Modified condition that applies default ACLs so it runs only for creates (no query present); prevents automatic insertion/overwriting of ACLs on updates.
ACL Behavior Tests spec/ParseACL.spec.js	Added tests: one verifies default ACL is not applied/overwritten on update; another verifies explicit ACL changes on update are preserved.
Manifest package.json	Manifest present in diff; no exported/public API signature changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the problem, solution, and behavioral rationale, but does not include the required 'Closes: issue-link' field or a structured 'Approach' section as specified in the template.	Add 'Closes: #ISSUE_NUMBER' to link the PR to a GitHub issue and reorganize content under 'Approach' section as per the template structure.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: preventing default ACL from overwriting custom ACL on update, which aligns with the core fix in the changeset.
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: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9414f35 and 5c830dc.

📒 Files selected for processing (2)

• spec/ParseACL.spec.js
• src/RestWrite.js

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

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

📚 Learning: 2025-12-02T06:55:53.808Z

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.

Applied to files:

• spec/ParseACL.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/ParseACL.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/ParseACL.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/ParseACL.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/ParseACL.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/ParseACL.spec.js

🧬 Code graph analysis (1)

spec/ParseACL.spec.js (3)

spec/schemas.spec.js (2)

• Parse (3-3)
• obj (13-13)

spec/SchemaPerformance.spec.js (2)

• schema (164-164)
• user (104-104)

spec/ParseObject.spec.js (2)

• user (1524-1524)
• user (1708-1708)

🔇 Additional comments (3)

src/RestWrite.js (1)

373-390: LGTM! Clean fix that aligns default ACL handling with other default fields.

The !this.query guard correctly restricts default ACL assignment to CREATE operations only, preventing existing ACLs from being overwritten during updates. This matches the pattern used for createdAt, updatedAt, and objectId (line 393), ensuring consistent behavior across all default fields.

spec/ParseACL.spec.js (2)

955-983: Excellent test coverage for the bug fix.

This test properly validates that the default ACL configured in class-level permissions is not automatically reapplied during object updates. The test correctly:

• Sets up a class with default ACL via schema.updateClass
• Captures the ACL after initial creation
• Performs an update without explicit ACL changes
• Verifies the ACL remains unchanged

---

985-1014: Great test for validating explicit ACL modifications remain functional.

This test ensures that while default ACLs are no longer auto-applied on updates, developers can still explicitly modify ACLs when needed. The test correctly validates that a custom ACL (removing public read access) is properly applied during an update operation.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/RestWrite.js (1)

373-390: Add test for default ACL behavior on UPDATE operations.

The existing test "support defaultACL in schema" verifies that default ACL is applied during CREATE. However, there's no test verifying that updates to objects with default ACLs do not reapply or overwrite the existing ACL. Add a test that:

1. Creates an object with a class-level default ACL
2. Updates the object
3. Verifies the ACL from creation is preserved and not replaced by the default ACL

Also cover the edge case where a user explicitly modifies the ACL during an update.

🧹 Nitpick comments (1)

src/RestWrite.js (1)

373-390: Suggestion: Refine PR title to include scope.

Based on coding guidelines, consider updating the PR title to:

fix(rest): prevent default ACL from overwriting custom ACL on update

This format makes it clearer in the changelog what specific behavior was fixed and why it matters to developers.

Based on learnings, Angular commit convention suggests including a scope (e.g., rest, acl) to identify the subsystem affected.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 774cc54 and 35d142a.

📒 Files selected for processing (1)

• src/RestWrite.js

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

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

🔇 Additional comments (1)

src/RestWrite.js (1)

373-390: LGTM! Correct fix that prevents ACL overwrite on updates.

The addition of !this.query && correctly restricts default ACL assignment to CREATE operations only, preventing custom ACLs from being overwritten during UPDATE operations. This change is consistent with how other default fields like createdAt and objectId are handled (lines 393-422).

[parse-github-assistant]

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

[parse-github-assistant]

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

[parse-github-assistant]

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