-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: Uploading a file by providing an origin URL allows for Server-Side Request Forgery (SSRF) #9903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughRemoved automatic URI-based file downloading from the FilesRouter and added tests that spin up a local server to ensure no HTTP requests are made for URI-backed file sources during creation or in beforeSave triggers. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 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 |
✅ 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spec/ParseFile.spec.js(1 hunks)src/Routers/FilesRouter.js(0 hunks)
💤 Files with no reviewable changes (1)
- src/Routers/FilesRouter.js
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 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/ParseFile.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 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/ParseFile.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
Applied to files:
spec/ParseFile.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 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/ParseFile.spec.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Applied to files:
spec/ParseFile.spec.js
🔇 Additional comments (2)
spec/ParseFile.spec.js (2)
657-678: LGTM! Proper test server setup for SSRF verification.The Express server setup correctly uses dynamic port assignment and tracks HTTP requests to verify that URI sources are not accessed. The cleanup in
afterEachproperly closes the server.
680-703: LGTM! Correct verification of SSRF prevention for REST file uploads.The test correctly verifies that a file with a URI source can be saved to an object (status 201) but the server never accesses the URI, preventing SSRF attacks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9903 +/- ##
==========================================
+ Coverage 92.97% 93.06% +0.08%
==========================================
Files 187 187
Lines 15177 15160 -17
Branches 177 177
==========================================
- Hits 14111 14108 -3
+ Misses 1054 1040 -14
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# [8.4.0-alpha.2](8.4.0-alpha.1...8.4.0-alpha.2) (2025-11-05) ### Bug Fixes * Uploading a file by providing an origin URL allows for Server-Side Request Forgery (SSRF); fixes vulnerability [GHSA-x4qj-2f4q-r4rx](GHSA-x4qj-2f4q-r4rx) ([#9903](#9903)) ([9776386](9776386))
|
🎉 This change has been released in version 8.4.0-alpha.2 |
# [8.4.0](8.3.0...8.4.0) (2025-11-05) ### Bug Fixes * Add problematic MIME types to default value of Parse Server option `fileUpload.fileExtensions` ([#9902](#9902)) ([fa245cb](fa245cb)) * Uploading a file by providing an origin URL allows for Server-Side Request Forgery (SSRF); fixes vulnerability [GHSA-x4qj-2f4q-r4rx](GHSA-x4qj-2f4q-r4rx) ([#9903](#9903)) ([9776386](9776386)) ### Features * Add support for Node 24 ([#9901](#9901)) ([25dfe19](25dfe19))
|
🎉 This change has been released in version 8.4.0 |
Summary by CodeRabbit
Bug Fixes
Tests