fix: Security upgrade to parse 7.0.1 #9877
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! |
✅ 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. |
📝 WalkthroughWalkthroughTests and test helpers updated and parse dependency bumped to 7.0.1. Adds a global.fetch mock/restore wrapper, adjusts fetch expectations and several query call patterns, adds retrying profiler checks, and changes prototype-pollution assertions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test
participant mockFetch as mockFetch()
participant Wrapper as global.fetch wrapper
participant Spy as spyRecorder
participant ParseServer as Parse Server URL
Note over mockFetch,Wrapper: mockFetch installs a wrapper around global.fetch and records/resolves mocked endpoints
Test->>mockFetch: install mocks/responses
mockFetch->>Wrapper: replace global.fetch with wrapper (records calls)
Wrapper->>Spy: record non-Parse requests (.calls)
alt request matches mocked endpoint
Wrapper-->>Test: return mocked Response (ok/json)
else request to Parse Server URL
Wrapper->>ParseServer: forward real request (bypass spy)
ParseServer-->>Wrapper: real response
Wrapper-->>Test: forward real response
end
Test->>Wrapper: inspect `global.fetch.calls` / `global.fetch.and` for assertions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
spec/ParseQuery.Comment.spec.js (2)
61-76: Extract duplicated retry logic into a helper function.The same retry pattern (poll profiler with 10 attempts, 100ms delays) is duplicated across all four tests. This violates DRY and makes future maintenance harder.
Consider extracting a reusable helper:
async function waitForProfilerEntry(database, query, options = {}) { const maxRetries = options.maxRetries || 10; const retryDelay = options.retryDelay || 100; for (let i = 0; i < maxRetries; i++) { const result = await database.collection('system.profile').findOne( query, { sort: { ts: -1 } } ); if (result) { return result; } await new Promise(resolve => setTimeout(resolve, retryDelay)); } return null; }Then simplify each test:
- // Wait for profile entry to appear with retry logic - let result; - const maxRetries = 10; - const retryDelay = 100; - for (let i = 0; i < maxRetries; i++) { - result = await database.collection('system.profile').findOne( - { 'command.explain.comment': comment }, - { sort: { ts: -1 } } - ); - if (result) { - break; - } - await new Promise(resolve => setTimeout(resolve, retryDelay)); - } + const result = await waitForProfilerEntry(database, { 'command.explain.comment': comment });Also applies to: 88-103, 121-136, 151-166
64-65: Consider extracting magic numbers to constants.The retry parameters (10 attempts, 100ms delay) are repeated across tests. Extracting them to named constants improves readability and makes tuning easier.
const PROFILER_RETRY_MAX_ATTEMPTS = 10; const PROFILER_RETRY_DELAY_MS = 100;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spec/ParseQuery.Comment.spec.js(4 hunks)spec/helper.js(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/helper.js
🧰 Additional context used
🧬 Code graph analysis (1)
spec/ParseQuery.Comment.spec.js (2)
spec/helper.js (2)
i(340-340)TestObject(273-275)src/Adapters/Storage/Mongo/MongoStorageAdapter.js (1)
database(181-181)
⏰ 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). (12)
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 20
- GitHub Check: Node 18
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 6, ReplicaSet
🔇 Additional comments (1)
spec/ParseQuery.Comment.spec.js (1)
49-49: LGTM: Dynamic comment generation prevents test interference.Using
Date.now()to generate unique comments for each test run is a good practice that prevents cross-test contamination and race conditions when multiple tests query the profiler.Also applies to: 82-82, 109-109, 142-142
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9877 +/- ##
==========================================
- Coverage 93.02% 93.02% -0.01%
==========================================
Files 187 187
Lines 15158 15158
Branches 176 176
==========================================
- Hits 14101 14100 -1
- Misses 1045 1046 +1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Beautiful! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/helper.js (1)
413-445: Harden mockFetch: support string|URL|Request, normalize method, dynamic serverURL, and fallback Response
- Normalize input (string, URL, Request) to a URL string.
- Uppercase HTTP method without mutating the original options.
- Bypass calls to the active Parse.serverURL (via
Parse.serverURLor fallbackserverURL), throwing iforiginalFetchis unavailable.- Return a minimal Response-like fallback (
ok,status,json(),text()).No tests reference
fetch.andstubbing (rg search returned no matches), so no test updates are required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/helper.js(5 hunks)
🔇 Additional comments (2)
spec/helper.js (2)
216-223: Good lifecycle isolation of fetch mocksRestoring fetch in beforeAll/beforeEach/afterEach/afterAll prevents cross‑test pollution. LGTM.
Also applies to: 225-230, 232-236, 275-276
10-16: Remove unnecessary fetch guard suggestion
Theengines.nodefield (>=18.20.4) andci/ciCheck.jsalready enforce running on Node 18+, whereglobal.fetchis defined—no extra guard needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/CloudCodeLogger.spec.js (1)
352-352: LGTM: Hook clearing applied consistently.The addition of
Parse.Cloud._removeAllHooks()ensures proper test isolation when reconfiguring the server with different log levels and hook definitions.Consider applying the same hook clearing pattern to other tests that reconfigure the server (e.g., the
beforeEachat lines 14-35) to ensure consistent test isolation across the entire suite.Also applies to: 376-376
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCodeLogger.spec.js(7 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/CloudCodeLogger.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/CloudCodeLogger.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/CloudCodeLogger.spec.js
🧬 Code graph analysis (1)
spec/CloudCodeLogger.spec.js (2)
spec/helper.js (1)
Parse(4-4)spec/CloudCode.spec.js (1)
Parse(3-3)
⏰ 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). (13)
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Node 18
- GitHub Check: Node 20
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Docker Build
🔇 Additional comments (3)
spec/CloudCodeLogger.spec.js (3)
192-205: LGTM: Async/await conversion improves test quality.The conversion from callback pattern to async/await aligns with project standards and improves readability and error handling.
Based on learnings.
206-218: LGTM: Hook clearing ensures proper test isolation.The addition of
Parse.Cloud._removeAllHooks()before reconfiguration and the subsequent redefinition ofbFunctionensures clean state between test scenarios and prevents cross-test interference.
234-234: LGTM: Consistent test isolation pattern.The hook clearing and arrow function refactoring maintain test isolation and code consistency.
Also applies to: 244-245
mtrezza
changed the title
# [8.3.0-alpha.7](8.3.0-alpha.6...8.3.0-alpha.7) (2025-10-15) ### Bug Fixes * Security upgrade to parse 7.0.1 ([#9877](#9877)) ([abfa94c](abfa94c))
|
🎉 This change has been released in version 8.3.0-alpha.7 |
3 participants
Pull Request
Issue
Upgrade to parse 7.0.1.
Approach
Even though this is a major version upgrade of Parse JS SDK, the breaking change there should only matter then using parse as a client SDK.
Summary by CodeRabbit
Chores
Tests