fix: Prevent stale data reads during update validation with primary readPreference

[mavriel]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Closes: #9850

Approach

This fix addresses a data consistency issue in distributed database environments where beforeSave trigger validation queries may read from stale read replicas while update operations target the primary database.

Problem

During beforeSave trigger execution, Parse Server performs permission validation by querying the database before applying updates. However:

• Update operations always target the primary database
• Validation queries use default database settings, which may route to read replicas
• In environments with replication lag, validation may fail with "Object not found" errors when the object exists in primary but hasn't propagated to replicas yet

Solution

Modified DatabaseController.update() method to ensure validation queries (validateOnly: true) explicitly use { readPreference: 'primary' }. This guarantees that:

• Validation reads from the same database that updates will target
• Eliminates race conditions caused by replication lag
• Maintains consistency between validation and update operations

Key Changes

• Added { readPreference: 'primary' } option for validation queries in src/Controllers/DatabaseController.js
• Only affects internal validation behavior during beforeSave triggers
• No changes to public APIs or user-facing functionality

Testing

Added comprehensive test coverage in spec/DatabaseController.spec.js:

• Verifies validateOnly: true queries use primary readPreference
• Confirms normal updates maintain existing behavior
• Uses mock storage adapter to validate exact query parameters

Tasks

• Add tests

Summary by CodeRabbit

• Bug Fixes

  • Validation-only updates now read from the primary database to avoid stale reads and intermittent “not found” responses. No API or result changes.

• Tests

  • Added tests to confirm validate-only updates use primary reads and do not perform unintended update calls.

[parse-github-assistant]

🚀 Thanks for opening this pull request!

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a primary-read option to the validation path of DatabaseController.update when validateOnly is true. Updates tests to cover both validateOnly=true and validateOnly=false paths, using a mock storage adapter to assert read vs update calls and their arguments.

Changes

Cohort / File(s)	Summary
Controller: validateOnly read preference src/Controllers/DatabaseController.js	In the validateOnly branch of update, changes adapter find call options from {} to { readPreference: 'primary' }. No other logic changes.
Tests: update with validateOnly spec/DatabaseController.spec.js	Adds a new test suite verifying: when validateOnly is true, find is called with readPreference: 'primary' and findOneAndUpdate is not called; when false, find is not called and findOneAndUpdate is called. Introduces a mock storage adapter with spies.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant DBController as DatabaseController
  participant Adapter as StorageAdapter

  rect rgb(245,250,255)
  note over Client,DBController: validateOnly = true (validation path)
  Client->>DBController: update(request, validateOnly=true)
  DBController->>Adapter: find(query, options={ readPreference: 'primary' })
  alt object not found
    DBController-->>Client: error OBJECT_NOT_FOUND
  else object found
    DBController-->>Client: {}
  end
  end

  rect rgb(250,245,255)
  note over Client,DBController: validateOnly = false (update path)
  Client->>DBController: update(request, validateOnly=false)
  DBController->>Adapter: findOneAndUpdate(query, update, options)
  Adapter-->>DBController: result
  DBController-->>Client: result/ack
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the core change of this pull request by indicating that update validation now uses a primary readPreference to prevent stale reads, matching the modifications in DatabaseController.update; it uses standard conventional commits format and focuses on the main purpose of the change.
Description Check	✅ Passed	The description follows the repository template by including the Pull Request header with licensing and security links, a linked issue in the Issue section, a detailed Approach section describing the problem and solution, and a Tasks section with the applicable test task completed; it accurately reflects the required structure and content.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

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

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/Controllers/DatabaseController.js (1)

596-602: Force-primary read is correct; add limit and projection to cut load

Only existence is needed here. Pass limit: 1 and a minimal projection to reduce I/O without altering semantics.

-              if (validateOnly) {
-                return this.adapter.find(className, schema, query, { readPreference: 'primary' }).then(result => {
+              if (validateOnly) {
+                // Read from primary to avoid replica lag during beforeSave validation (see #9850).
+                return this.adapter.find(
+                  className,
+                  schema,
+                  query,
+                  { readPreference: 'primary', limit: 1, keys: ['objectId'] }
+                ).then(result => {

spec/DatabaseController.spec.js (2)

619-633: Prevent schema cache bleed between tests

Clear the schema cache in this suite to avoid flaky behavior from prior tests.

   describe('update with validateOnly', () => {
+    beforeEach(() => {
+      Config.get(Parse.applicationId).schemaCache.clear();
+    });

---

655-657: Make assertion resilient to future option additions

Use partial match so the test won’t break if more options (e.g., limit, keys) are passed.

-      expect(findCall.args[3]).toEqual({ readPreference: 'primary' }); // options parameter
+      expect(findCall.args[3]).toEqual(jasmine.objectContaining({ readPreference: 'primary' })); // options parameter

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82fdb0d and 59e599b.

📒 Files selected for processing (2)

• spec/DatabaseController.spec.js (1 hunks)
• src/Controllers/DatabaseController.js (1 hunks)

🔇 Additional comments (1)

spec/DatabaseController.spec.js (1)

663-687: LGTM: correctly asserts update path when validateOnly is false

Good coverage ensuring validation read is bypassed and write path is invoked.