fix: Instance comparison with instanceof is not realm-safe#10225
Merged
mtrezza merged 12 commits intoparse-community:alphafrom Mar 16, 2026
Merged
fix: Instance comparison with instanceof is not realm-safe#10225mtrezza merged 12 commits intoparse-community:alphafrom
instanceof is not realm-safe#10225mtrezza merged 12 commits intoparse-community:alphafrom
Conversation
|
🚀 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
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. |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds realm-safe type-checking helpers to Utils (isDate, isRegExp, isMap, isSet, isNativeError, isObject), replaces isPromise with a duck-typed, pollution-safe implementation, updates call sites to use these helpers, adds tests, and extends ESLint rules to discourage Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Utils.js (1)
498-515: Consider updatinggetCircularReplacerto use the new realm-safe helpers.This method still uses
instanceof Mapandinstanceof Set(lines 501, 504), which contradicts the PR's objective of replacinginstanceofwith realm-safe alternatives. Cross-realm Map/Set instances would not be correctly serialized.♻️ Proposed fix
static getCircularReplacer() { const seen = new WeakSet(); return (key, value) => { - if (value instanceof Map) { + if (Utils.isMap(value)) { return Object.fromEntries(value); } - if (value instanceof Set) { + if (Utils.isSet(value)) { return Array.from(value); } if (typeof value === 'object' && value !== null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Utils.js` around lines 498 - 515, The getCircularReplacer function uses instanceof Map/Set which fails for cross-realm objects; update it to use realm-safe checks (e.g., imported helpers like isMap/isSet or a toString-based check) instead of instanceof when inspecting value, keep the existing handling for Map->Object.fromEntries and Set->Array.from and continue using the seen WeakSet to detect cycles; specifically replace the instanceof Map and instanceof Set checks in getCircularReplacer with calls to the realm-safe helpers (or Object.prototype.toString checks) so cross-realm Map/Set instances are serialized correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Utils.js`:
- Around line 498-515: The getCircularReplacer function uses instanceof Map/Set
which fails for cross-realm objects; update it to use realm-safe checks (e.g.,
imported helpers like isMap/isSet or a toString-based check) instead of
instanceof when inspecting value, keep the existing handling for
Map->Object.fromEntries and Set->Array.from and continue using the seen WeakSet
to detect cycles; specifically replace the instanceof Map and instanceof Set
checks in getCircularReplacer with calls to the realm-safe helpers (or
Object.prototype.toString checks) so cross-realm Map/Set instances are
serialized correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 746f52b6-8ccf-4d2e-92a7-df111b705c9a
📒 Files selected for processing (2)
spec/Utils.spec.jssrc/Utils.js
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Controllers/ParseGraphQLController.js (1)
305-312:⚠️ Potential issue | 🟠 MajorReplace realm-unsafe Promise check with realm-safe utility.
Line 311 uses
instanceof Promise, which fails across realms. Replace withUtils.isPromise(obj), which is already available and properly implements realm-safe duck-typing.🔧 Suggested patch
const isValidSimpleObject = function (obj): boolean { return ( typeof obj === 'object' && !Array.isArray(obj) && obj !== null && Utils.isDate(obj) !== true && - obj instanceof Promise !== true + Utils.isPromise(obj) !== true ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Controllers/ParseGraphQLController.js` around lines 305 - 312, The isValidSimpleObject function currently uses a realm-unsafe check "obj instanceof Promise" (inside isValidSimpleObject) which breaks across realms; replace that check with the realm-safe utility Utils.isPromise(obj). Update the condition in isValidSimpleObject to call Utils.isPromise(obj) instead of using instanceof so promise detection uses the existing duck-typed helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/Utils.spec.js`:
- Around line 316-319: The test intentionally uses a cross-realm instanceof
check (crossRealmRegExp instanceof RegExp) which triggers the lint
restricted-syntax rule; add an inline ESLint disable comment immediately above
that assertion (same style as the existing inline disable on Line 298) to bypass
the rule for this specific line so the test keeps its intentional check while
avoiding lint failures; locate the assertion in the 'should return true for a
cross-realm RegExp' test that creates crossRealmRegExp via vm.runInNewContext
and add the single-line disable comment before the instanceof RegExp
expectation.
---
Outside diff comments:
In `@src/Controllers/ParseGraphQLController.js`:
- Around line 305-312: The isValidSimpleObject function currently uses a
realm-unsafe check "obj instanceof Promise" (inside isValidSimpleObject) which
breaks across realms; replace that check with the realm-safe utility
Utils.isPromise(obj). Update the condition in isValidSimpleObject to call
Utils.isPromise(obj) instead of using instanceof so promise detection uses the
existing duck-typed helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f34bb38-76cd-4ae0-a662-89c013b9557e
📒 Files selected for processing (9)
eslint.config.jsspec/Utils.spec.jsspec/eslint.config.jssrc/Adapters/Storage/Mongo/MongoStorageAdapter.jssrc/Adapters/Storage/Mongo/MongoTransform.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Config.jssrc/Controllers/ParseGraphQLController.jssrc/GraphQL/loaders/defaultGraphQLTypes.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/ParseGraphQLServer.spec.js (1)
8478-8567: Refactor to useasync/awaitinstead ofdonecallback.The test at line 8478 uses callback-style with
done, and theapolloClient.mutate()call at line 8555 is not awaited. This allows the test to complete before the mutation finishes, defeating the test's purpose. Convert to async/await to match the suite's established pattern and ensure proper error propagation.♻️ Suggested refactor
- it('should accept different params', done => { + it('should accept different params', async () => { Parse.Cloud.define('hello', async req => { expect(Utils.isDate(req.params.date)).toBe(true); expect(req.params.date.getTime()).toBe(1463907600000); expect(Utils.isDate(req.params.dateList[0])).toBe(true); expect(req.params.dateList[0].getTime()).toBe(1463907600000); expect(Utils.isDate(req.params.complexStructure.date[0])).toBe(true); expect(req.params.complexStructure.date[0].getTime()).toBe(1463907600000); expect(Utils.isDate(req.params.complexStructure.deepDate.date[0])).toBe(true); expect(req.params.complexStructure.deepDate.date[0].getTime()).toBe(1463907600000); expect(Utils.isDate(req.params.complexStructure.deepDate2[0].date)).toBe(true); expect(req.params.complexStructure.deepDate2[0].date.getTime()).toBe(1463907600000); // Regression for `#2294` expect(req.params.file instanceof Parse.File).toBe(true); expect(req.params.file.url()).toEqual('https://some.url'); // Regression for `#2204` expect(req.params.array).toEqual(['a', 'b', 'c']); expect(Array.isArray(req.params.array)).toBe(true); expect(req.params.arrayOfArray).toEqual([ ['a', 'b', 'c'], ['d', 'e', 'f'], ]); expect(Array.isArray(req.params.arrayOfArray)).toBe(true); expect(Array.isArray(req.params.arrayOfArray[0])).toBe(true); expect(Array.isArray(req.params.arrayOfArray[1])).toBe(true); }); @@ - apolloClient.mutate({ + await apolloClient.mutate({ mutation: gql` mutation CallFunction($params: Object) { callCloudCode(input: { functionName: hello, params: $params }) { result } } `, variables: { params, }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/ParseGraphQLServer.spec.js` around lines 8478 - 8567, The test "should accept different params" uses the callback-style done and calls apolloClient.mutate(...) without awaiting it, causing the test to finish before the mutation completes; convert the test to an async function (replace the done callback) and await the apolloClient.mutate(...) call so the Parse.Cloud.define('hello', ...) assertions run before the test completes; update the test declaration (the it(...) callback) to async and await apolloClient.mutate(...) to ensure proper error propagation and timing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/Utils.spec.js`:
- Around line 396-416: In the tests around Utils.isPromise where you
intentionally set Object.prototype.then and create objects with a then property,
add explicit Biome ignore comments to suppress the lint rule
lint/suspicious/noThenProperty; specifically insert a biome-ignore comment (e.g.
// biome-ignore lint/suspicious/noThenProperty) immediately above the lines that
assign Object.prototype.then = () => {} and above the expect(Utils.isPromise({
then: () => {} })) assertion so the intentional prototype pollution and thenable
creation in the Utils.isPromise tests are exempted from the linter.
---
Nitpick comments:
In `@spec/ParseGraphQLServer.spec.js`:
- Around line 8478-8567: The test "should accept different params" uses the
callback-style done and calls apolloClient.mutate(...) without awaiting it,
causing the test to finish before the mutation completes; convert the test to an
async function (replace the done callback) and await the
apolloClient.mutate(...) call so the Parse.Cloud.define('hello', ...) assertions
run before the test completes; update the test declaration (the it(...)
callback) to async and await apolloClient.mutate(...) to ensure proper error
propagation and timing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f45650c9-62b8-42c2-ab0b-ec0d1b6f1aaa
📒 Files selected for processing (13)
eslint.config.jsspec/CloudCode.spec.jsspec/MongoStorageAdapter.spec.jsspec/MongoTransform.spec.jsspec/ParseAPI.spec.jsspec/ParseGraphQLServer.spec.jsspec/ParseQuery.spec.jsspec/ParseUser.spec.jsspec/PushController.spec.jsspec/Utils.spec.jsspec/eslint.config.jssrc/StatusHandler.jssrc/triggers.js
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/eslint.config.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
eslint.config.js (2)
45-67: Consider addinginstanceof Setto the restricted syntax rules.Same as
spec/eslint.config.js- the PR addsUtils.isSet()but doesn't include a corresponding lint rule. Consider adding it for consistency.🔧 Suggested patch
{ selector: "BinaryExpression[operator='instanceof'][right.name='Map']", message: "Use Utils.isMap() instead of instanceof Map (cross-realm safe).", }, + { + selector: "BinaryExpression[operator='instanceof'][right.name='Set']", + message: "Use Utils.isSet() instead of instanceof Set (cross-realm safe).", + }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 45 - 67, Add a rule to the no-restricted-syntax array to forbid "instanceof Set" and point to Utils.isSet(); specifically, add an object with selector "BinaryExpression[operator='instanceof'][right.name='Set']" and message "Use Utils.isSet() instead of instanceof Set (cross-realm safe)." so the lint config is consistent with the newly added Utils.isSet().
44-44: Unrelated change:no-consolerule added.This
"no-console": "warn"rule appears unrelated to the PR's objective of replacinginstanceofchecks with realm-safe alternatives. Consider splitting this into a separate commit or PR for clearer change tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` at line 44, The change adding the ESLint rule "no-console": "warn" is unrelated to the instanceof-to-realm-safe refactor; remove that rule from this PR (revert the `"no-console": "warn"` change in eslint.config.js) or move it into a separate commit/PR so the instanceof replacement changes remain isolated and reviewable.spec/eslint.config.js (1)
66-88: Consider addinginstanceof Setto the restricted syntax rules.The PR adds
Utils.isSet()and tests for it, but the ESLint rule doesn't include a selector forinstanceof Set. Consider adding it for consistency.🔧 Suggested patch
{ selector: "BinaryExpression[operator='instanceof'][right.name='Map']", message: "Use Utils.isMap() instead of instanceof Map (cross-realm safe).", }, + { + selector: "BinaryExpression[operator='instanceof'][right.name='Set']", + message: "Use Utils.isSet() instead of instanceof Set (cross-realm safe).", + }, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/eslint.config.js` around lines 66 - 88, Add a "instanceof Set" restriction to the ESLint no-restricted-syntax rules to mirror the new Utils.isSet() helper; specifically add a selector entry matching BinaryExpression[operator='instanceof'][right.name='Set'] with a message advising to use Utils.isSet() instead of instanceof Set so the rule set (the array containing selectors for Date, RegExp, Error, Promise, Map) also blocks Set usage consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/Utils.spec.js`:
- Around line 352-356: The test "should return true for a cross-realm Set"
intentionally asserts that crossRealmSet instanceof Set is false; add the same
ESLint bypass used in nearby tests to suppress the rule for intentional
instanceof usage by placing an appropriate ESLint disable comment immediately
before the line containing expect(crossRealmSet instanceof Set).toBe(false); so
the linter ignores the instanceof check while leaving the rest of the test
(including Utils.isSet(crossRealmSet)) unchanged.
---
Nitpick comments:
In `@eslint.config.js`:
- Around line 45-67: Add a rule to the no-restricted-syntax array to forbid
"instanceof Set" and point to Utils.isSet(); specifically, add an object with
selector "BinaryExpression[operator='instanceof'][right.name='Set']" and message
"Use Utils.isSet() instead of instanceof Set (cross-realm safe)." so the lint
config is consistent with the newly added Utils.isSet().
- Line 44: The change adding the ESLint rule "no-console": "warn" is unrelated
to the instanceof-to-realm-safe refactor; remove that rule from this PR (revert
the `"no-console": "warn"` change in eslint.config.js) or move it into a
separate commit/PR so the instanceof replacement changes remain isolated and
reviewable.
In `@spec/eslint.config.js`:
- Around line 66-88: Add a "instanceof Set" restriction to the ESLint
no-restricted-syntax rules to mirror the new Utils.isSet() helper; specifically
add a selector entry matching
BinaryExpression[operator='instanceof'][right.name='Set'] with a message
advising to use Utils.isSet() instead of instanceof Set so the rule set (the
array containing selectors for Date, RegExp, Error, Promise, Map) also blocks
Set usage consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44db3402-f90c-4a3a-a953-3b23032f3b16
📒 Files selected for processing (7)
eslint.config.jsspec/Utils.spec.jsspec/eslint.config.jssrc/Controllers/ParseGraphQLController.jssrc/Routers/PagesRouter.jssrc/Security/Check.jssrc/Utils.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Utils.js
There was a problem hiding this comment.
🧹 Nitpick comments (2)
eslint.config.js (1)
45-79: Consider centralizing thisno-restricted-syntaxlist shared withspec/eslint.config.js.This block is duplicated across both ESLint configs; extracting a shared rules constant would reduce drift risk when adding/removing realm-safe selectors later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 45 - 79, The duplicated "no-restricted-syntax" block (the selectors like BinaryExpression[operator='instanceof'][right.name='Date'], RegExp, Error, Promise, Map, Object, Set, Buffer) should be extracted into a shared constant (e.g., REALM_SAFE_INSTANCEOF_RULES or SHARED_INSTANCEOF_SELECTORS) and imported into both ESLint configs so they reference the same array; create a module that exports the array of rule objects, update eslint.config.js to replace the inline array under "no-restricted-syntax" with a reference or spread of that constant, and likewise update spec/eslint.config.js to import and reuse the same exported constant to avoid duplication and drift.src/middlewares.js (1)
2-2: Suggested PR title:fix(security): replace realm-unsafe instanceof checks with realm-safe type guards.Based on learnings: For Parse Server PRs, always suggest an Angular commit convention PR title in
type(scope): descriptionformat.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares.js` at line 2, Change the PR title to follow Angular commit convention and reflect the fix, e.g. "fix(middlewares): replace realm-unsafe instanceof checks with realm-safe type guards"; then update src/middlewares.js by removing any realm-unsafe instanceof usages and replacing them with realm-safe type guards (create or use helpers in Utils such as Utils.isError, Utils.isRequest, Utils.isResponse or similar) and update any functions/methods that relied on instanceof to use those guards (search for "instanceof" references in middleware handlers and validation logic and replace them with calls to the new Utils type-guard functions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eslint.config.js`:
- Around line 45-79: The duplicated "no-restricted-syntax" block (the selectors
like BinaryExpression[operator='instanceof'][right.name='Date'], RegExp, Error,
Promise, Map, Object, Set, Buffer) should be extracted into a shared constant
(e.g., REALM_SAFE_INSTANCEOF_RULES or SHARED_INSTANCEOF_SELECTORS) and imported
into both ESLint configs so they reference the same array; create a module that
exports the array of rule objects, update eslint.config.js to replace the inline
array under "no-restricted-syntax" with a reference or spread of that constant,
and likewise update spec/eslint.config.js to import and reuse the same exported
constant to avoid duplication and drift.
In `@src/middlewares.js`:
- Line 2: Change the PR title to follow Angular commit convention and reflect
the fix, e.g. "fix(middlewares): replace realm-unsafe instanceof checks with
realm-safe type guards"; then update src/middlewares.js by removing any
realm-unsafe instanceof usages and replacing them with realm-safe type guards
(create or use helpers in Utils such as Utils.isError, Utils.isRequest,
Utils.isResponse or similar) and update any functions/methods that relied on
instanceof to use those guards (search for "instanceof" references in middleware
handlers and validation logic and replace them with calls to the new Utils
type-guard functions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72a31b80-addb-4c11-936b-8bb02d2edcff
📒 Files selected for processing (9)
eslint.config.jsspec/AdapterLoader.spec.jsspec/GridFSBucketStorageAdapter.spec.jsspec/ParseAPI.spec.jsspec/Utils.spec.jsspec/eslint.config.jssrc/Routers/FilesRouter.jssrc/Utils.jssrc/middlewares.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Utils.js
- spec/ParseAPI.spec.js
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## alpha #10225 +/- ##
==========================================
+ Coverage 92.18% 92.60% +0.41%
==========================================
Files 192 192
Lines 16307 16322 +15
Branches 199 199
==========================================
+ Hits 15033 15115 +82
+ Misses 1253 1190 -63
+ Partials 21 17 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 16, 2026
# [9.6.0-alpha.32](9.6.0-alpha.31...9.6.0-alpha.32) (2026-03-16) ### Bug Fixes * Instance comparison with `instanceof` is not realm-safe ([#10225](#10225)) ([51efb1e](51efb1e))
Contributor
|
🎉 This change has been released in version 9.6.0-alpha.32 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Issue
Instance comparison with
instanceofis not realm-safe. This may cause issues in certain test or production setups.Approach
instanceofwith safe alternatives.instanceof.Tasks
Summary by CodeRabbit
New Features
Improvements
Tests
Chores