fix: `toJSONwithObjects` leaks `ParseACL` instances when afterFind hooks modify objects with directAccess by yog27ray · Pull Request #10244 · parse-community/parse-server

yog27ray · 2026-03-19T05:59:53Z

Issue

When directAccess is enabled, toJSONwithObjects in src/triggers.js leaks live ParseACL instances into the response instead of plain JSON objects. This causes the client-side Parse SDK to throw:

TypeError: Tried to create an ACL with an invalid permission type.

The bug occurs when an afterFind hook modifies objects via .setACL() or .set(). Since ParseACL lacks _toFullJSON, the raw instance is assigned directly to the response JSON. With directAccess, the response bypasses HTTP serialization (JSON.stringify), so the ParseACL instance leaks through to ParseObject._finishFetch, which expects plain JSON.

Approach

In toJSONwithObjects, when a pending op value lacks _toFullJSON, check if it has a toJSON method and call it before assigning. This serializes SDK types like ParseACL and ParseGeoPoint to plain JSON — consistent with what object.toJSON() already does in the same function.

Change in src/triggers.js:198:

// Before
toJSON[key] = val;

// After
toJSON[key] = val && typeof val.toJSON === 'function' ? val.toJSON() : val;

Tasks

Add tests
Add changes to documentation (guides, repository pages, code comments)

Summary by CodeRabbit

Bug Fixes
- Improved conversion of pending access permissions to plain JSON while preserving date values.
- Ensured permission changes made during after-find processing are retained in returned results when direct access is enabled, including public-read and administrator write access.
Tests
- Added coverage for access-permission serialization, date values, and permission updates during after-find processing.

…oks modify objects When `directAccess` is enabled, `toJSONwithObjects` iterates pending ops and calls `object.get(key)` for each dirty field. For ACL, this returns a live `ParseACL` instance. Since `ParseACL` lacks `_toFullJSON`, the instance was assigned directly to the response JSON. With `directAccess`, the response bypasses HTTP serialization, so the raw `ParseACL` leaks to the client SDK. The client's `ParseObject._finishFetch` then calls `new ParseACL(aclData)` expecting plain JSON but receiving a `ParseACL` instance, throwing: "TypeError: Tried to create an ACL with an invalid permission type." The fix calls `val.toJSON()` on values that support it before assigning, ensuring SDK types like `ParseACL` and `ParseGeoPoint` are serialized to plain JSON — consistent with what `object.toJSON()` already does. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

parse-github-assistant · 2026-03-19T05:59:58Z

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Tip

Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
Group code into logical blocks. Add a short comment before each block to explain its purpose.
We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

Note

Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect.

Caution

Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement.

parseplatformorg · 2026-03-19T06:00:05Z

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

Status	Scan Engine	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 · 2026-03-19T06:00:11Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: b4462a23-e978-43de-9fa2-532ed81551ac

📥 Commits

Reviewing files that changed from the base of the PR and between 23d7bbc and 2e6b2b0.

📒 Files selected for processing (2)

spec/CloudCode.spec.js
src/triggers.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

toJSONwithObjects now serializes pending Parse.ACL instances to plain JSON. Tests check ACL serialization, date preservation, and ACL permissions after an afterFind hook runs with direct access enabled.

Changes

ACL serialization

Layer / File(s)	Summary
Serialize pending ACL values `src/triggers.js`, `spec/CloudCode.spec.js`	`toJSONwithObjects` calls `toJSON()` for pending `Parse.ACL` values and leaves other pending values unchanged. Tests check plain ACL output, date preservation, and ACL permissions after an `afterFind` hook runs with direct access enabled.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Suggested reviewers: mtrezza

Merge Risk: ⚪ Minimal · up to 2e6b2

The ACL serialization fix appears ready to merge after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 2e6b2

The change affects 2 systems.

Changed systems: src, spec

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — src (service) was modified; 1 changed file maps to changed impact.
observed — spec (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

observed — Modified behavior in spec/CloudCode.spec.js: Added a toJSONwithObjects test checking that a pending ACL is serialized as a plain object with its read and role-write permissions, while a date field remains a Date.
observed — Modified behavior in spec/CloudCode.spec.js: Added a direct-access test that configures the REST controller, saves an object with an ACL, and has an afterFind hook reapply that ACL and set touched; the fetched object is expected to retain its value, hook-set field, public-read access, and admin-role write access.
observed — Modified behavior in src/triggers.js: Pending Parse.ACL values now use toJSON() during fallback serialization instead of being assigned directly; non-ACL values retain the previous raw assignment behavior.

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

Ignore

❌ Failed checks (1 error, 3 warnings)

Check name	Explanation	Resolution
Title check	The title accurately describes the ACL serialization fix and uses the allowed `fix:` prefix. However, the first character after the prefix is a backtick followed by lowercase `toJSONwithObjects`, not …	Change the title to begin with a capitalized description after the prefix, for example: `fix: Prevent toJSONwithObjects from leaking ParseACL instances`.
Linked Issues check	The directly linked issue [`#8473`] reports a startup role-creation failure. The reported response contains `{ __op: 'Delete' }`, and `ParseRole._handleSaveResponse` then throws the invalid-permission e…	Add an automated regression test for the startup role-creation failure in [`#8473`]. Change the relevant response path so the role save response does not pass `{ __op: 'Delete' }` as ACL input. If the intended target is the `afterFind` and `d…
Out of Scope Changes check	The production change and tests address ACL serialization after `afterFind` modifications with `directAccess`. Issue [`#8473`] addresses invalid ACL input during startup role creation. The available evi…	Unlink [`#8473`] and link the issue for the `afterFind`/`directAccess` ACL leak, or replace the change with implementation and tests for the linked issue's startup role-creation failure.
Docstring Coverage	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check		The description includes the required Issue, Approach, and Tasks sections. It clearly explains the defect, affected behavior, implementation, and tests. The unchecked non-applicable tasks should be re…
Security Check		PASS. The runtime diff changes only `src/triggers.js:198`: it calls `toJSON()` only when the pending value is an actual `Parse.ACL`; all other values retain the prior behavior. This converts an intern…
Engage In Review Feedback		The review feedback was addressed. Reviewer mtrezza requested limiting serialization to Parse.ACL because broad `toJSON()` conversion changed `Date` values. Commit `f9148ab` narrowed the implementatio…

Full details: Title check

Explanation

The title accurately describes the ACL serialization fix and uses the allowed fix: prefix. However, the first character after the prefix is a backtick followed by lowercase toJSONwithObjects, not a capitalized letter as required.

Full details: Linked Issues check

Explanation

The directly linked issue [#8473] reports a startup role-creation failure. The reported response contains { __op: 'Delete' }, and ParseRole._handleSaveResponse then throws the invalid-permission error. This pull request changes toJSONwithObjects for pending ACL values and tests afterFind with directAccess. The reviewed evidence does not show a change to role creation, ParseRole._handleSaveResponse, or the reported response path.

Resolution

Add an automated regression test for the startup role-creation failure in [#8473]. Change the relevant response path so the role save response does not pass { __op: 'Delete' } as ACL input. If the intended target is the afterFind and directAccess bug, link the issue that specifies that behavior instead.

Full details: Out of Scope Changes check

Explanation

The production change and tests address ACL serialization after afterFind modifications with directAccess. Issue [#8473] addresses invalid ACL input during startup role creation. The available evidence establishes no connection between these scenarios. The change is therefore outside the scope of the directly linked issue.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 Biome (2.5.14)

src/triggers.js

File contains syntax errors that prevent linting: Line 230: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 230: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 230: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 230: return types can only be used in TypeScript files

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

codecov · 2026-03-19T06:07:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.00%. Comparing base (36af858) to head (2e6b2b0).
⚠️ Report is 1 commits behind head on alpha.

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha   #10244      +/-   ##
==========================================
- Coverage   94.00%   94.00%   -0.01%     
==========================================
  Files         193      193              
  Lines       17125    17125              
  Branches      259      259              
==========================================
- Hits        16099    16098       -1     
- Misses       1004     1005       +1     
  Partials       22       22

☔ View full report in Codecov by Harness.
📢 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.

yog27ray · 2026-03-20T04:13:10Z

@mtrezza can you re-run the "MongoDB 8, ReplicaSet" looks like a flaky test case. Also "Docker Build" action failing on this PR only?

mtrezza · 2026-03-20T16:02:19Z

@yog27ray Is this a security vulnerability?

yog27ray · 2026-03-21T07:06:40Z

@mtrezza This isn't a security vulnerability — it's a bug that occurs when the directAccess flag is enabled. I previously reported a related issue here: #8473. We've since upgraded to parse-server 9.5.1, but the same bug is still present in this version as well.

yog27ray · 2026-03-24T04:57:24Z

@mtrezza is there any change required in this PR?

yog27ray · 2026-03-31T04:23:18Z

@mtrezza can someone review this?

yog27ray · 2026-04-02T04:49:02Z

@mtrezza any concern with this PR ?

mtrezza · 2026-04-03T01:47:38Z

@yog27ray it's in review

mtrezza · 2026-04-15T12:46:38Z

The ACL fix is correct, but calling val.toJSON() unconditionally silently breaks Date fields modified in afterFind under directAccess.

The client SDK's decode passes raw Date instances through unchanged:

if (value === null || typeof value !== 'object' || value instanceof Date) {
  return value;
}

Before this PR: raw Date leaks → client returns it as a Date → obj.get('date') works.
After this PR: Date.prototype.toJSON() returns an ISO string → decode passes the string through → obj.get('date') returns a string, and .getTime() throws.

Unlike ACL (which is already a hard TypeError), Date under directAccess currently works, so this is a real regression.

A fix could be to narrow the conditional to ACL:

if (val instanceof Parse.ACL) {
  toJSON[key] = val.toJSON();
} else {
  toJSON[key] = val;
}

The root cause is that the pending-ops loop overrides the properly-encoded output of object.toJSON() with raw values from object.get(key). This also breaks Date fields modified in afterFind on the HTTP path (client sees an ISO string instead of {__type: "Date", iso: "..."}). Long-term fix could be using Parse's encode function so both directAccess and HTTP produce canonical wire format. Non-breaking for SDK consumers since the decoder round-trips {__type: ...}.

The broader issue is that if directAccess currently returns a date string, and we cannot even switch to a Parse-style date object, because even though Parse SDKs would be able to parse it correctly, it would be a breaking change for direct REST consumers.

Approach	HTTP REST consumers	SDK consumers	Fixes ACL bug?
Current PR (`val.toJSON()`)	ISO string → ISO string (no change)	`Date` → `string` (broken)	✓
Narrow to `Parse.ACL`	no change	no change	✓
Use `encode`	ISO string → `{__type: "Date"}` (breaking change)	no change	✓

… Date instances Calling val.toJSON() on all objects with a toJSON method converts Date instances to ISO strings, breaking directAccess consumers. Narrowing to Parse.ACL fixes the original bug without regressing Date fields.

yog27ray · 2026-04-16T05:06:36Z

@mtrezza requested changes done

yog27ray · 2026-04-27T02:32:17Z

@mtrezza any further changes required.

yog27ray · 2026-05-20T06:59:03Z

@mtrezza any update on this

yog27ray · 2026-06-03T09:04:29Z

@mtrezza is there any changes required?

yog27ray · 2026-08-19T06:14:36Z

@mtrezza any thing that i can help with to make this PR merge.

coderabbitai

Pre-merge checks failed. Please resolve the failing checks before merging.

yog27ray · 2026-10-12T04:51:48Z

@coderabbitai full review

coderabbitai · 2026-10-12T04:51:55Z

✅ Action performed

Full review finished.

yog27ray · 2026-10-12T06:05:46Z

@mtrezza ?

coderabbitai Bot previously approved these changes Mar 19, 2026

View reviewed changes

Merge branch 'alpha' into fix/afterfind-acl-directaccess

b7836b6

yog27ray added 3 commits March 21, 2026 18:57

Merge branch 'alpha' into fix/afterfind-acl-directaccess

d98b16e

Merge branch 'alpha' into fix/afterfind-acl-directaccess

5048603

Merge branch 'alpha' into fix/afterfind-acl-directaccess

bd982ea

Merge branch 'alpha' into fix/afterfind-acl-directaccess

4487c84

yog27ray and others added 2 commits April 9, 2026 10:07

Merge branch 'alpha' into fix/afterfind-acl-directaccess

e3e1776

Merge branch 'alpha' into fix/afterfind-acl-directaccess

03a62e1

yog27ray dismissed coderabbitai[bot]’s stale review via f9148ab April 16, 2026 04:16

coderabbitai Bot approved these changes Apr 16, 2026

View reviewed changes

Merge branch 'alpha' into fix/afterfind-acl-directaccess

03e7199

Merge branch 'alpha' into fix/afterfind-acl-directaccess

5475024

Merge branch 'alpha' into fix/afterfind-acl-directaccess

9f0f003

yog27ray added 3 commits August 24, 2026 14:35

Merge branch 'alpha' into fix/afterfind-acl-directaccess

6bfb2d0

Merge branch 'alpha' into fix/afterfind-acl-directaccess

6c2cd8b

Merge branch 'alpha' into fix/afterfind-acl-directaccess

2e6b2b0

coderabbitai Bot requested changes Oct 12, 2026

View reviewed changes

Conversation

yog27ray commented Mar 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Approach

Tasks

Summary by CodeRabbit

Uh oh!

parse-github-assistant Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parseplatformorg commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

coderabbitai Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Architecture Summary

Pre-merge checks failed

❌ Failed checks (1 error, 3 warnings)

Uh oh!

codecov Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yog27ray commented Mar 20, 2026

Uh oh!

mtrezza commented Mar 20, 2026

Uh oh!

yog27ray commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yog27ray commented Mar 24, 2026

Uh oh!

yog27ray commented Mar 31, 2026

Uh oh!

yog27ray commented Apr 2, 2026

Uh oh!

mtrezza commented Apr 3, 2026

Uh oh!

mtrezza commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yog27ray commented Apr 16, 2026

Uh oh!

yog27ray commented Apr 27, 2026

Uh oh!

yog27ray commented May 20, 2026

Uh oh!

yog27ray commented Jun 3, 2026

Uh oh!

yog27ray commented Aug 19, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

yog27ray commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yog27ray commented Oct 12, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yog27ray commented Mar 19, 2026 •

edited by coderabbitai Bot

Loading

parse-github-assistant Bot commented Mar 19, 2026 •

edited

Loading

parseplatformorg commented Mar 19, 2026 •

edited

Loading

coderabbitai Bot commented Mar 19, 2026 •

edited

Loading

codecov Bot commented Mar 19, 2026 •

edited

Loading

yog27ray commented Mar 21, 2026 •

edited

Loading

mtrezza commented Apr 15, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading