Skip to content

Conversation

Moumouls
Copy link
Member

@Moumouls Moumouls commented Oct 14, 2025

Pull Request

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened protection against prototype pollution by ignoring dangerous keys and non-own properties across encoding, decoding, object state updates, and server commits.
    • Replaced plain prototype-linked containers with prototype-free maps for safer nested-path, pending-state, and cache handling.
  • Tests

    • Added comprehensive security tests verifying malicious keys, nested structures, arrays, and class-name inputs cannot pollute prototypes.

Copy link

parse-github-assistant bot commented Oct 14, 2025

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Oct 14, 2025

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.

Copy link

coderabbitai bot commented Oct 14, 2025

📝 Walkthrough

Walkthrough

Adds prototype-pollution defenses across state mutation, serialization, and class registry: introduces isDangerousKey, restricts iterations to own properties, uses Object.create(null) for internal maps, and updates tests to assert no pollution via dangerous keys or malicious class names.

Changes

Cohort / File(s) Summary
State mutations hardening
src/ObjectStateMutations.ts
Adds isDangerousKey checks; replaces plain {} with Object.create(null) for serverData, pendingOps, and objectCache; skips prototype/inherited and dangerous keys in setServerData, setPendingOp, pending-state pushes/merges, estimateAttribute(s), nestedSet, and commitServerChanges.
Class map initialization
src/ParseObject.ts
Initializes classMap with Object.create(null) to avoid prototype-inherited keys.
Serialization: decode safeguards
src/decode.ts
Imports isDangerousKey; iterates only own properties (uses hasOwnProperty) and skips dangerous keys when decoding objects/paths.
Serialization: encode safeguards
src/encode.ts
Iterates only own properties and skips keys flagged by isDangerousKey when encoding; preserves existing recursive encoding behavior otherwise.
Dangerous-key utility
src/isDangerousKey.ts, types/isDangerousKey.d.ts
New exported isDangerousKey(key: string): boolean that treats __proto__, constructor, prototype (and any dotted path segments) as dangerous; includes TS declaration.
Tests: ObjectStateMutations pollution protection
src/__tests__/ObjectStateMutations-test.js
Adds tests and mocks for prototype-pollution scenarios; ensures estimateAttributes and commitServerChanges do not pollute Object.prototype; includes setup/teardown cleanup.
Tests: ParseObject pollution protection
src/__tests__/ParseObject-test.js
Adds tests preventing pollution via registerSubclass, fromJSON, and malicious classNames; includes setup/teardown to clear prototype flags; minor whitespace tweaks.
Tests: Decode pollution protection
src/__tests__/decode-test.js
Adds tests (and mock import) to ensure decoding ignores dangerous keys (including nested/arrays) and handles dangerous className; includes cleanup hooks.
Tests: Encode pollution protection
src/__tests__/encode-test.js
Adds tests (and mock import) ensuring encoding excludes dangerous/inherited keys and nested/array cases do not pollute; includes cleanup hooks.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant Obj as ParseObject
  participant OSM as ObjectStateMutations
  participant Guard as isDangerousKey
  participant Stores as serverData/pendingOps/objectCache

  App->>Obj: set / save / update
  Obj->>OSM: setServerData / setPendingOp / nestedSet
  OSM->>Guard: isDangerousKey(key/path)?
  OSM->>OSM: hasOwnProperty check
  alt dangerous or inherited key
    OSM-->>Obj: skip mutation
  else safe key
    OSM->>Stores: write using Object.create(null)
  end

  App->>Obj: commitServerChanges(changes)
  Obj->>OSM: commitServerChanges
  loop each change (own props only)
    OSM->>Guard: isDangerousKey(key/path)?
    OSM->>OSM: hasOwnProperty check
    alt dangerous or inherited key
      OSM-->>Obj: skip apply
    else safe key
      OSM->>Stores: update serverData/objectCache
    end
  end
Loading
sequenceDiagram
  autonumber
  actor App
  participant Enc as encode
  participant Dec as decode
  participant Guard as isDangerousKey

  App->>Enc: encode(value)
  loop each enumerable key
    Enc->>Enc: hasOwnProperty(value, key)
    Enc->>Guard: isDangerousKey(key)?
    alt own && safe
      Enc-->>App: include encoded key
    else skip
      Enc-->>App: omit key
    end
  end

  App->>Dec: decode(value)
  loop each enumerable key
    Dec->>Dec: hasOwnProperty(value, key)
    Dec->>Guard: isDangerousKey(key)?
    alt own && safe
      Dec-->>App: include decoded key
    else skip
      Dec-->>App: omit key
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the required template because it omits the “## Issue” and “## Approach” sections and does not include the tasks checklist, instead providing a custom summary by CodeRabbit. This makes it unclear which issue is being resolved and what the specific implementation approach was. As a result the description is largely incomplete with respect to the repository’s contribution guidelines. Please add an “## Issue” section with a link to the issue and a proper “Closes:” line, fill out the “## Approach” section with a description of the changes, and update or remove the “## Tasks” checklist to reflect added tests and documentation updates.
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly identifies the security fix by specifying prototype pollution and the affected components (Parse.Object and internal APIs), which aligns directly with the core changes introduced across encoding, decoding, and object state logic. It is concise, descriptive, and allows a reviewer to understand the main purpose of the pull request at a glance. There are no extraneous details or vague terms that would obscure its intent.
✨ 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

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

Copy link

codecov bot commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 83.72093% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.88%. Comparing base (9e7c1ba) to head (7919f36).
⚠️ Report is 4 commits behind head on alpha.

Files with missing lines Patch % Lines
src/ObjectStateMutations.ts 73.07% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             alpha    #2749      +/-   ##
===========================================
- Coverage   100.00%   99.88%   -0.12%     
===========================================
  Files           63       64       +1     
  Lines         6185     6219      +34     
  Branches      1472     1488      +16     
===========================================
+ Hits          6185     6212      +27     
- Misses           0        7       +7     

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

@mtrezza
Copy link
Member

mtrezza commented Oct 14, 2025

Proposed PR title:

fix: Prototype pollution in Parse.Object and internal APIs; fixes security vulnerability GHSA-9f2h-7v79-mxw

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (9)
src/ParseObject.ts (1)

105-105: Good hardening; refine the type of classMap

Initializing with Object.create(null) is the right defense. The type AttributeMap is misleading here (values are constructors). Prefer a constructor map type.

Apply this minimal change:

-const classMap: AttributeMap = Object.create(null);
+const classMap: Record<string, any> = Object.create(null);
src/ObjectStateMutations.ts (2)

22-39: Scope the dangerousKeys constant once

Function-local allocation is fine, but you can hoist dangerousKeys to module scope to avoid re-alloc per call.

- function isDangerousKey(key: string): boolean {
-  const dangerousKeys = ["__proto__", "constructor", "prototype"];
+ const DANGEROUS_KEYS = ['__proto__', 'constructor', 'prototype'] as const;
+ function isDangerousKey(key: string): boolean {
   // Check if the key itself is dangerous
-  if (dangerousKeys.includes(key)) {
+  if (DANGEROUS_KEYS.includes(key)) {
     return true;
   }
   // Check if any part of a dotted path is dangerous
   if (key.includes(".")) {
     const parts = key.split(".");
-    return parts.some((part) => dangerousKeys.includes(part));
+    return parts.some((part) => DANGEROUS_KEYS.includes(part));
   }
   return false;
 }

141-197: Avoid shadowing the function parameter named object

Within the dotted-path branch, a local object variable shadows the object: ParseObject parameter, reducing clarity and risking mistakes.

Apply this renaming within the inner block:

-          let object = data;
+          let container = data;
           for (let i = 0; i < fields.length - 1; i++) {
             const key = fields[i];
-            if (!(key in object)) {
+            if (!(key in container)) {
               const nextKey = fields[i + 1];
               if (!isNaN(nextKey)) {
-                object[key] = [];
+                container[key] = [];
               } else {
-                object[key] = Object.create(null);
+                container[key] = Object.create(null);
               }
             } else {
-              if (Array.isArray(object[key])) {
-                object[key] = [...object[key]];
+              if (Array.isArray(container[key])) {
+                container[key] = [...container[key]];
               } else {
-                object[key] = { ...object[key] };
+                container[key] = { ...container[key] };
               }
             }
-            object = object[key];
+            container = container[key];
           }
-          object[last] = pendingOps[i][attr].applyTo(object[last]);
+          container[last] = pendingOps[i][attr].applyTo(container[last]);
src/__tests__/encode-test.js (1)

197-339: Add an assertion that the encoded result’s prototype stays intact

Once encode skips dangerous keys (as suggested), also assert that the result’s prototype wasn’t mutated.

For example, in the 'proto' and nested tests, add:

expect(Object.getPrototypeOf(result)).toBe(Object.prototype);

This guards against inadvertent prototype rewiring of the returned object.

src/__tests__/decode-test.js (1)

124-310: Solid pollution-protection coverage; add a couple assertions in the arrays case (optional)

Current tests don’t assert array elements were sanitized. Consider asserting no dangerous own keys on decoded array items. Also consider DRYing the prototype cleanup via a shared helper/setup file used across test suites.

Example patch for the arrays test:

       // Verify result array
       expect(Array.isArray(result)).toBe(true);
       expect(result.length).toBe(3);
       expect(result[2].normalKey).toBe('value');
+      // Ensure dangerous own keys were not materialized
+      expect(Object.prototype.hasOwnProperty.call(result[0], '__proto__')).toBe(false);
+      expect(Object.prototype.hasOwnProperty.call(result[1], 'constructor')).toBe(false);
src/__tests__/ObjectStateMutations-test.js (1)

355-405: Good tests; optionally assert outputs are sanitized and state isn’t tainted

To strengthen the tests:

  • Capture and assert the return of estimateAttributes has no dangerous keys.
  • Assert commitServerChanges didn’t materialize a proto own key on serverData/objectCache.

Example tweaks:

-  ObjectStateMutations.estimateAttributes(serverData, pendingOps, {
+  const attrs = ObjectStateMutations.estimateAttributes(serverData, pendingOps, {
     className: 'TestClass',
     id: 'test123',
   });
+  expect(Object.prototype.hasOwnProperty.call(attrs, '__proto__')).toBe(false);
+  expect(Object.prototype.hasOwnProperty.call(attrs, 'constructor')).toBe(false);
   ObjectStateMutations.commitServerChanges(serverData, objectCache, {
     '__proto__.polluted': 'exploited',
   });
+  expect(Object.prototype.hasOwnProperty.call(serverData, '__proto__')).toBe(false);
src/__tests__/ParseObject-test.js (3)

169-171: Use jest.fn() for InstallationController stubs

Make these mockable/introspectable functions instead of bare methods. Keeps tests clearer and avoids accidental signature mismatches.

 CoreManager.setInstallationController({
   currentInstallationId() {
     return Promise.resolve('iid');
   },
-  currentInstallation() { },
-  updateInstallationOnDisk() { },
+  currentInstallation: jest.fn(),
+  updateInstallationOnDisk: jest.fn(),
 });

1416-1418: Return resolved promises from fetch spies to avoid brittle tests

No-op implementations may break if the implementation chains .then(). Prefer resolved promises for stability.

-  .mockImplementationOnce(() => { })
-  .mockImplementationOnce(() => { })
-  .mockImplementationOnce(() => { });
+  .mockResolvedValueOnce(undefined)
+  .mockResolvedValueOnce(undefined)
+  .mockResolvedValueOnce(undefined);

Apply the same change to the similar fetch spies at Lines 1521-1523 and 1549-1551.

Also applies to: 1521-1523, 1549-1551


1627-1627: Return resolved promises from EventuallyQueue.poll spies

Keep these asynchronous and future-proof.

-jest.spyOn(EventuallyQueue, 'poll').mockImplementationOnce(() => { });
+jest.spyOn(EventuallyQueue, 'poll').mockResolvedValueOnce(undefined);

Apply to all occurrences listed.

Also applies to: 1642-1642, 2710-2710, 2742-2742

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e7c1ba and b7e8bc3.

📒 Files selected for processing (8)
  • src/ObjectStateMutations.ts (9 hunks)
  • src/ParseObject.ts (1 hunks)
  • src/__tests__/ObjectStateMutations-test.js (1 hunks)
  • src/__tests__/ParseObject-test.js (15 hunks)
  • src/__tests__/decode-test.js (1 hunks)
  • src/__tests__/encode-test.js (1 hunks)
  • src/decode.ts (1 hunks)
  • src/encode.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/__tests__/ObjectStateMutations-test.js (2)
src/__tests__/SingleInstanceStateController-test.js (1)
  • ParseOps (23-23)
src/__tests__/UniqueInstanceStateController-test.js (1)
  • ParseOps (26-26)
src/__tests__/encode-test.js (1)
src/__tests__/LocalDatastore-test.js (1)
  • encode (7-7)
src/ObjectStateMutations.ts (4)
types/ObjectStateMutations.d.ts (3)
  • State (7-13)
  • AttributeMap (4-4)
  • OpsMap (5-5)
src/SingleInstanceStateController.ts (2)
  • setServerData (57-60)
  • popPendingState (80-83)
src/UniqueInstanceStateController.ts (2)
  • setServerData (51-54)
  • popPendingState (74-77)
src/ParseObject.ts (1)
  • attributes (210-213)
src/__tests__/ParseObject-test.js (1)
src/__tests__/encode-test.js (1)
  • json (31-34)
src/ParseObject.ts (2)
src/ObjectStateMutations.ts (1)
  • AttributeMap (10-10)
types/ObjectStateMutations.d.ts (1)
  • AttributeMap (4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (Node 18, 18.20.4)
  • GitHub Check: build (Node 22, 22.4.1)
  • GitHub Check: build (Node 20, 20.15.1)
🔇 Additional comments (8)
src/ObjectStateMutations.ts (8)

40-45: Null-prototype maps: LGTM

serverData, pendingOps seed, and objectCache now use Object.create(null), which prevents prototype traversal. Good hardening.


50-66: Guarding setServerData: LGTM

Own-property and dangerous-key checks are correct. This prevents polluted inputs from landing in serverData.


81-91: Pending state containers: LGTM

Using Object.create(null) on push/reset keeps the chain safe.


93-115: mergeFirstPendingState guards: LGTM

Own-property and dangerous-key filters are correct in the merge path.


116-139: estimateAttribute dangerous-key early return: LGTM

This blocks reads of reserved keys and prevents side effects.


199-229: nestedSet uses null-prototype for intermediate objects: LGTM

This prevents creating prototype-bearing containers along dotted paths.


230-258: commitServerChanges guards: LGTM

Own-property and dangerous-key checks before nestedSet, with objectCache updates via encode(true). Solid.


68-79: Controllers correctly delegate setPendingOp Confirmed that SingleInstanceStateController and UniqueInstanceStateController both call ObjectStateMutations.setPendingOp, ensuring the dangerous-key guard is applied.

@mtrezza
Copy link
Member

mtrezza commented Oct 14, 2025

'proto', 'constructor', 'prototype'

I think we should have the dangerous keys centrally defined somewhere, so they can be modified at all checks at once.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 14, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/isDangerousKey.ts (1)

7-19: Consider extracting dangerousKeys as a module-level constant.

The function logic is correct and handles both direct keys and dotted paths appropriately. However, the dangerousKeys array is recreated on every function call.

Apply this diff to improve performance and testability:

+/**
+ * Keys that are potentially dangerous for prototype pollution
+ */
+const DANGEROUS_KEYS = ["__proto__", "constructor", "prototype"];
+
 /**
  * Check if a property name or path is potentially dangerous for prototype pollution
- * Dangerous keys include: __proto__, constructor, prototype
  * @param key - The property name or dotted path to check
  * @returns true if the key is dangerous, false otherwise
  */
 export function isDangerousKey(key: string): boolean {
-  const dangerousKeys = ["__proto__", "constructor", "prototype"];
   // Check if the key itself is dangerous
-  if (dangerousKeys.includes(key)) {
+  if (DANGEROUS_KEYS.includes(key)) {
     return true;
   }
   // Check if any part of a dotted path is dangerous
   if (key.includes(".")) {
     const parts = key.split(".");
-    return parts.some((part) => dangerousKeys.includes(part));
+    return parts.some((part) => DANGEROUS_KEYS.includes(part));
   }
   return false;
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7e8bc3 and cba326d.

📒 Files selected for processing (7)
  • src/ObjectStateMutations.ts (10 hunks)
  • src/__tests__/ObjectStateMutations-test.js (3 hunks)
  • src/__tests__/decode-test.js (2 hunks)
  • src/__tests__/encode-test.js (2 hunks)
  • src/decode.ts (2 hunks)
  • src/encode.ts (2 hunks)
  • src/isDangerousKey.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/tests/decode-test.js
  • src/encode.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/__tests__/encode-test.js (1)
src/__tests__/LocalDatastore-test.js (1)
  • encode (7-7)
src/ObjectStateMutations.ts (4)
src/SingleInstanceStateController.ts (2)
  • setServerData (57-60)
  • popPendingState (80-83)
src/UniqueInstanceStateController.ts (2)
  • setServerData (51-54)
  • popPendingState (74-77)
src/ParseObject.ts (1)
  • attributes (210-213)
src/isDangerousKey.ts (1)
  • isDangerousKey (7-19)
src/decode.ts (1)
src/isDangerousKey.ts (1)
  • isDangerousKey (7-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (Node 22, 22.4.1)
  • GitHub Check: build (Node 18, 18.20.4)
  • GitHub Check: build (Node 20, 20.15.1)
🔇 Additional comments (11)
src/decode.ts (1)

53-59: LGTM! Addresses past review concern with centralized utility.

The own-property guard combined with the isDangerousKey check effectively prevents prototype pollution during decoding. Using the centralized utility aligns with the suggestion to define dangerous keys in a single location.

src/__tests__/encode-test.js (1)

198-340: LGTM! Comprehensive prototype pollution test coverage.

The test suite thoroughly covers various pollution vectors:

  • Direct dangerous keys (proto, constructor, prototype)
  • Nested objects and arrays with dangerous keys
  • Inherited and prototype chain properties

The beforeEach/afterEach cleanup ensures test isolation.

src/__tests__/ObjectStateMutations-test.js (1)

356-406: LGTM! Tests cover key mutation and commit paths.

The test suite verifies that prototype pollution protections work in:

  1. estimateAttributes with dangerous key names in pending ops
  2. commitServerChanges with nested dotted paths containing dangerous keys

The beforeEach/afterEach cleanup ensures proper test isolation.

src/ObjectStateMutations.ts (8)

23-30: LGTM! Prototype-free objects prevent accidental pollution.

Using Object.create(null) for internal data structures (serverData, pendingOps, objectCache) eliminates the prototype chain, preventing both accidental and intentional prototype pollution through property assignment.


33-49: LGTM! Proper guards against prototype pollution.

The combination of own-property guard and isDangerousKey check ensures that only safe, own properties are processed. The undefined check preserves the existing behavior of deleting attributes when set to undefined.


51-62: LGTM! Dangerous keys blocked from pending ops.

The early return for dangerous keys prevents them from ever entering the pendingOps array, maintaining safety across all mutation paths.


64-74: LGTM! Consistent use of prototype-free objects.

Both functions now use Object.create(null) for new pending state entries, maintaining consistency with the defaultState approach and preventing prototype pollution.


76-97: LGTM! Guards prevent pollution during state merge.

The own-property guard and isDangerousKey check ensure that dangerous keys are not propagated when merging pending states, maintaining safety across the operation lifecycle.


99-122: LGTM! Dangerous attributes return undefined.

Returning undefined for dangerous keys prevents them from being estimated, ensuring the attributes API surface remains safe.


124-180: LGTM! Comprehensive protection in attribute estimation.

The changes ensure safety throughout the estimation flow:

  • Result object uses Object.create(null) (line 129)
  • Own-property and dangerous-key guards in the loop (lines 137-143)
  • Nested object creation uses Object.create(null) (line 161)

This provides defense-in-depth against prototype pollution.


192-241: LGTM! Complete protection in commit path.

The changes ensure safety when committing server changes:

  • nestedSet uses Object.create(null) for intermediate objects (line 201), preventing pollution in nested paths
  • Own-property guard and dangerous-key check in commitServerChanges (lines 221-227) prevent processing of inherited or dangerous properties

This completes the protection across all mutation surfaces.

@mtrezza mtrezza merged commit 0097398 into parse-community:alpha Oct 14, 2025
10 of 13 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/isDangerousKey.ts (1)

7-19: Consider extracting dangerous keys to a module-level constant.

The function correctly identifies dangerous keys and dotted paths containing them. However, the dangerous keys list is currently defined inline.

Based on the PR comments from @mtrezza suggesting centralization, consider extracting the list:

+const DANGEROUS_KEYS = ["__proto__", "constructor", "prototype"];
+
 export function isDangerousKey(key: string): boolean {
-  const dangerousKeys = ["__proto__", "constructor", "prototype"];
   // Check if the key itself is dangerous
-  if (dangerousKeys.includes(key)) {
+  if (DANGEROUS_KEYS.includes(key)) {
     return true;
   }
   // Check if any part of a dotted path is dangerous
   if (key.includes(".")) {
     const parts = key.split(".");
-    return parts.some((part) => dangerousKeys.includes(part));
+    return parts.some((part) => DANGEROUS_KEYS.includes(part));
   }
   return false;
 }

This makes it easier to maintain and update the list in one place if needed in the future.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7e8bc3 and 7919f36.

📒 Files selected for processing (8)
  • src/ObjectStateMutations.ts (10 hunks)
  • src/__tests__/ObjectStateMutations-test.js (3 hunks)
  • src/__tests__/decode-test.js (2 hunks)
  • src/__tests__/encode-test.js (2 hunks)
  • src/decode.ts (2 hunks)
  • src/encode.ts (2 hunks)
  • src/isDangerousKey.ts (1 hunks)
  • types/isDangerousKey.d.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/encode.ts
🧰 Additional context used
🧬 Code graph analysis (6)
types/isDangerousKey.d.ts (1)
src/isDangerousKey.ts (1)
  • isDangerousKey (7-19)
src/__tests__/ObjectStateMutations-test.js (2)
src/__tests__/SingleInstanceStateController-test.js (1)
  • ParseOps (23-23)
src/__tests__/UniqueInstanceStateController-test.js (1)
  • ParseOps (26-26)
src/__tests__/decode-test.js (1)
src/decode.ts (1)
  • decode (8-62)
src/decode.ts (1)
src/isDangerousKey.ts (1)
  • isDangerousKey (7-19)
src/ObjectStateMutations.ts (3)
src/SingleInstanceStateController.ts (2)
  • setServerData (57-60)
  • popPendingState (80-83)
src/UniqueInstanceStateController.ts (2)
  • setServerData (51-54)
  • popPendingState (74-77)
src/isDangerousKey.ts (1)
  • isDangerousKey (7-19)
src/__tests__/encode-test.js (1)
src/__tests__/LocalDatastore-test.js (1)
  • encode (7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (Node 20, 20.15.1)
  • GitHub Check: build (Node 22, 22.4.1)
  • GitHub Check: build (Node 18, 18.20.4)
🔇 Additional comments (20)
src/decode.ts (2)

6-6: LGTM!

The import of isDangerousKey centralizes dangerous key detection as recommended in the PR comments.


53-59: LGTM!

The dual guard (own-property check + dangerous key filter) correctly prevents prototype pollution during decoding. The use of continue ensures dangerous keys are skipped entirely rather than being assigned.

types/isDangerousKey.d.ts (1)

1-7: LGTM!

The type declaration is correct and well-documented. The JSDoc clearly describes the function's purpose and behavior for both simple keys and dotted paths.

src/__tests__/decode-test.js (2)

3-3: LGTM!

Correctly unmocks isDangerousKey to ensure the real implementation is tested.


125-311: LGTM!

The test suite is comprehensive and covers all critical prototype pollution vectors:

  • Direct dangerous keys (__proto__, constructor, prototype)
  • Nested objects with dangerous keys
  • Arrays containing objects with dangerous keys
  • Inherited vs. own properties
  • Prototype chain properties
  • Deep nesting
  • Parse types with dangerous className

The beforeEach/afterEach cleanup ensures test isolation and prevents pollution between tests.

src/__tests__/encode-test.js (2)

2-2: LGTM!

Correctly unmocks isDangerousKey to ensure the real implementation is tested.


198-340: LGTM!

The test suite provides symmetric coverage for encoding, mirroring the decode tests. It thoroughly validates that encoding:

  • Filters dangerous keys (__proto__, constructor, prototype)
  • Handles nested objects and arrays safely
  • Processes only own properties
  • Does not propagate prototype chain properties

The cleanup logic ensures test isolation.

src/__tests__/ObjectStateMutations-test.js (3)

4-4: LGTM!

Correctly unmocks isDangerousKey to ensure the real implementation is tested.


15-15: LGTM!

The explicit empty function body improves code clarity over the implicit return.


356-406: LGTM!

The test suite validates prototype pollution protection in ObjectStateMutations:

  • estimateAttributes test ensures dangerous keys in pendingOps don't pollute the prototype
  • commitServerChanges test ensures nested dangerous paths (e.g., __proto__.polluted) don't pollute

Both tests verify that Object.prototype remains unpolluted, and the cleanup logic ensures test isolation.

src/ObjectStateMutations.ts (10)

9-9: LGTM!

The import of isDangerousKey centralizes dangerous key detection across all mutation paths.


25-27: LGTM!

Using Object.create(null) for the default state creates prototype-free objects that cannot be polluted via prototype chain manipulation. This is a strong defense against prototype pollution attacks.


35-48: LGTM!

The dual guard (own-property check + dangerous key filter) correctly prevents prototype pollution when setting server data. The logic ensures only safe, own properties are copied.


52-55: LGTM!

Early return for dangerous keys prevents polluting pendingOps.


65-65: LGTM!

Using Object.create(null) for pending state objects ensures they cannot be polluted via prototype chain manipulation.

Also applies to: 71-71


79-96: LGTM!

The dual guard (own-property check + dangerous key filter) correctly prevents prototype pollution during pending state merges.


105-108: LGTM!

Early return for dangerous keys prevents attempting to estimate polluted attributes.


219-240: LGTM!

The dual guard (own-property check + dangerous key filter) correctly prevents prototype pollution when committing server changes. The implementation ensures only safe changes are applied to serverData and objectCache.


192-211: Object.create(null) safe for nestedSet: no code invokes toString, hasOwnProperty, valueOf, or constructor on nested path objects.


129-179: Verify prototype-free nested objects usage
No calls to toString or hasOwnProperty were detected on the returned attribute data; manually confirm that no code relies on built-in object methods for nested objects.

parseplatformorg pushed a commit that referenced this pull request Oct 14, 2025
# [7.0.0-alpha.1](6.2.0-alpha.3...7.0.0-alpha.1) (2025-10-14)

### Bug Fixes

*  Prototype Pollution vulnerability in `SingleInstanceStateController`; fixes security vulnerability [GHSA-9g8m-v378-pcg3](GHSA-9g8m-v378-pcg3) ([#2745](#2745)) ([9e7c1ba](9e7c1ba))
* HTTP status code 3XX redirection for Parse Server URL not handled properly ([#2608](#2608)) ([58e7f58](58e7f58))
* Incorrect type in `ParseObject.fetch` parameter `options` ([#2726](#2726)) ([dc78419](dc78419))
* Prototype pollution in `Parse.Object` and internal APIs; fixes security vulnerability [GHSA-9f2h-7v79-mxw](GHSA-9f2h-7v79-mxw3) ([#2749](#2749)) ([0097398](0097398))
* Unhandled exception when calling `Parse.Cloud.run` with option value `null` ([#2622](#2622)) ([#2623](#2623)) ([2818ed9](2818ed9))

### Features

* Add `Parse.File` upload and download progress in browser and Node environments ([#2503](#2503)) ([d3ca465](d3ca465))
* Add option `Parse.nodeLogging` to fully log `Parse.Object` in Node.js environments ([#1594](#1594)) ([de9d057](de9d057))
* Remove `Parse.serverAuthType`, `Parse.serverAuthToken` infavor of CoreManager `REQUEST_HEADERS` config ([#2639](#2639)) ([ddc66a1](ddc66a1))

### BREAKING CHANGES

* The methods `Parse.serverAuthType()` and `Parse.serverAuthToken()` have been removed; use the CoreManager `REQUEST_HEADER` config to set authorization headers instead. ([ddc66a1](ddc66a1))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0-alpha.1

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 14, 2025
parseplatformorg pushed a commit that referenced this pull request Oct 14, 2025
# [7.0.0](6.1.1...7.0.0) (2025-10-14)

### Bug Fixes

*  Prototype Pollution vulnerability in `SingleInstanceStateController`; fixes security vulnerability [GHSA-9g8m-v378-pcg3](GHSA-9g8m-v378-pcg3) ([#2745](#2745)) ([9e7c1ba](9e7c1ba))
* HTTP status code 3XX redirection for Parse Server URL not handled properly ([#2608](#2608)) ([58e7f58](58e7f58))
* Incorrect type in `ParseObject.fetch` parameter `options` ([#2726](#2726)) ([dc78419](dc78419))
* Missing error message when returning an internal server error ([#2543](#2543)) ([f91f3f1](f91f3f1))
* Prototype pollution in `Parse.Object` and internal APIs; fixes security vulnerability [GHSA-9f2h-7v79-mxw](GHSA-9f2h-7v79-mxw3) ([#2749](#2749)) ([0097398](0097398))
* Unhandled exception when calling `Parse.Cloud.run` with option value `null` ([#2622](#2622)) ([#2623](#2623)) ([2818ed9](2818ed9))

### Features

* Add `Parse.File` upload and download progress in browser and Node environments ([#2503](#2503)) ([d3ca465](d3ca465))
* Add `Uint8Array` support for `Parse.File` data ([#2548](#2548)) ([6f6bb66](6f6bb66))
* Add option `Parse.nodeLogging` to fully log `Parse.Object` in Node.js environments ([#1594](#1594)) ([de9d057](de9d057))
* Remove `Parse.serverAuthType`, `Parse.serverAuthToken` infavor of CoreManager `REQUEST_HEADERS` config ([#2639](#2639)) ([ddc66a1](ddc66a1))

### Performance Improvements

* Optimize bundle packaging with Vite ([#2553](#2553)) ([a4b19e5](a4b19e5))

### BREAKING CHANGES

* The methods `Parse.serverAuthType()` and `Parse.serverAuthToken()` have been removed; use the CoreManager `REQUEST_HEADER` config to set authorization headers instead. ([ddc66a1](ddc66a1))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released Released as stable version state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants