fix: toJSONwithObjects leaks ParseACL instances when afterFind hooks modify objects with directAccess

[yog27ray]

Issue

Closes #8473

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

• Tests

  • Added tests verifying after-find hook behavior and ACL handling, including cases with direct access enabled.

• Bug Fixes

  • Improved serialization so pending ACL objects are converted to plain JSON, preserving ACL permissions and date fields.

[parse-github-assistant]

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

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c5d4bcba-2d2c-4b10-a280-8cb6550ac4ae

📥 Commits

Reviewing files that changed from the base of the PR and between 4487c84 and f9148ab.

📒 Files selected for processing (2)

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

🚧 Files skipped from review as they are similar to previous changes (1)

• spec/CloudCode.spec.js

---

📝 Walkthrough

Walkthrough

Added two afterFind hook tests covering ACL serialization and directAccess behavior; updated toJSONwithObjects to serialize pending Parse.ACL instances to plain JSON instead of leaving them as Parse.ACL objects.

Changes

Cohort / File(s)	Summary
Test Coverage spec/CloudCode.spec.js	Added two afterFind tests: one verifies toJSONwithObjects converts a pending Parse.ACL (set via obj.setACL(acl)) into plain JSON and preserves date serialization; the other enables directAccess: true, registers an afterFind hook that re-applies ACL and sets touched, then asserts the hook-updated field and ACL permissions are returned.
Serialization Utility src/triggers.js	Changed toJSONwithObjects fallback for pending values without _toFullJSON: if the value is a Parse.ACL instance serialize it with val.toJSON(), otherwise leave the original val unchanged (previously the raw val was always assigned).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: Protected fields leak via LiveQuery afterEvent trigger (GHSA-5hmj-jcgp-6hff) #10232: Touches the same serialization flow (toJSONwithObjects) and outgoing payload conversion for triggers/events.
• feat: Allow returning objects in Parse.Cloud.beforeFind without invoking database query #9770: Also modifies toJSONwithObjects in src/triggers.js, impacting object serialization for trigger payloads.

Suggested reviewers

• mtrezza
• Moumouls

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	While the PR addresses the ParseACL leak from #8473, reviewer feedback indicates the fix causes a regression: Date fields modified in afterFind hooks now become ISO strings instead of Date objects, breaking SDK Date handling.	Narrow the fix to only serialize Parse.ACL instances (e.g., instanceof check) rather than calling toJSON() on all values to avoid breaking Date field handling on directAccess.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title begins with 'fix:' prefix as required and clearly describes the bug being fixed regarding ParseACL serialization in toJSONwithObjects.
Description check	✅ Passed	The description includes Issue link (#8473), clear Approach explanation with code change, and Tasks checklist with completed items marked.
Out of Scope Changes check	✅ Passed	All changes (one-line fix in src/triggers.js and test additions in spec/CloudCode.spec.js) directly address the ParseACL serialization issue from #8473 with no unrelated modifications.
Security Check	✅ Passed	The PR's code changes using typeof val.toJSON === 'function' safely serialize objects without introducing injection vectors, prototype pollution, or dangerous patterns. The modification actually improves security by preventing live Parse.ACL SDK instances from leaking to clients.
Engage In Review Feedback	✅ Passed	PR author recognized reviewer's concern about Date field regression and implemented the exact narrower fix proposed (instanceof Parse.ACL check), with comprehensive test coverage and explicit acknowledgment in commit message.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.50%. Comparing base (60a58ec) to head (f9148ab).
⚠️ Report is 1 commits behind head on alpha.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha   #10244   +/-   ##
=======================================
  Coverage   92.50%   92.50%           
=======================================
  Files         192      192           
  Lines       16792    16792           
  Branches      234      234           
=======================================
  Hits        15533    15533           
  Misses       1236     1236           
  Partials       23       23           

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

[yog27ray]

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

@yog27ray Is this a security vulnerability?

[yog27ray]

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

@mtrezza is there any change required in this PR?

[yog27ray]

@mtrezza can someone review this?

[yog27ray]

@mtrezza any concern with this PR ?

[mtrezza]

@yog27ray it's in review

[mtrezza]

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	✓

[yog27ray]

@mtrezza requested changes done