fix: Stored XSS via file upload of HTML-renderable file types (GHSA-v5hf-f4c3-m5rv)

[mtrezza]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.

Issue

Stored XSS via file upload of HTML-renderable file types (GHSA-v5hf-f4c3-m5rv)

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)
• Add security check
• Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

• Tests

  • Added tests validating blocking of dangerous upload types (.svgz, .xht, .xml, .xsl, .xslt), including extensionless payloads with XML-related content types, while confirming common safe types remain uploadable.

• Bug Fixes

  • Broadened upload-restriction defaults to cover additional XML-related and other risky extensions.

• Documentation

  • Updated guidance to explicitly reference HTML, SVG, and XML and describe additional problematic patterns.

[parse-github-assistant]

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds tests blocking uploads of .svgz, .xht, .xml, .xsl, .xslt and expands the default file-extension exclusion regex and docs to include XML/XHTML/XSLT and SVG variants; test suite is duplicated in the spec file.

Changes

Cohort / File(s)	Summary
Test Suite spec/vulnerabilities.spec.js	Adds new tests for GHSA-v5hf-f4c3-m5rv validating file upload protections: ensures .svgz, .xht, .xml, .xsl, .xslt uploads are rejected with FILE_SAVE_ERROR; verifies safe types (txt, png, jpg, gif, pdf, doc) remain allowed; includes extensionless payloads with specific content-types. Note: suite appears duplicated.
File Extension Configuration & Docs src/Options/Definitions.js, src/Options/docs.js, src/Options/index.js	Expands FileUploadOptions.fileExtensions default regex to additionally exclude XML/XHTML/XSLT and SVG variants (e.g., svgz, xht, xsl, xslt); updates help/documentation text to mention XML alongside HTML and SVG. No exported API signature changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: Add problematic MIME types to default value of Parse Server option fileUpload.fileExtensions #9902: Modified FileUploadOptions.fileExtensions exclusion regex (focused on HTML/XHTML patterns) — related regex evolution.
• fix: Stored cross-site scripting (XSS) via SVG file upload (GHSA-hcj7-6gxh-24ww) #10136: Updated file upload filtering and tests to block active/browser-rendered file types — related to expanded upload restrictions.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	PR description follows the template structure with Issue and Tasks completed, but the Approach section is empty and critical context is missing.	Complete the Approach section describing the changes made to fix GHSA-v5hf-f4c3-m5rv. Note that documentation updates and security checks remain unchecked despite changes being present in the PR.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title directly describes the main security fix: addressing stored XSS vulnerability via file upload of HTML-renderable types (GHSA-v5hf-f4c3-m5rv), which aligns with all the changes made across the codebase.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[parseplatformorg]

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

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between a6c0926 and 2580457.

📒 Files selected for processing (4)

• spec/vulnerabilities.spec.js
• src/Options/Definitions.js
• src/Options/docs.js
• src/Options/index.js

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2580457 and b28606b.

📒 Files selected for processing (4)

• spec/vulnerabilities.spec.js
• src/Options/Definitions.js
• src/Options/docs.js
• src/Options/index.js

🚧 Files skipped from review as they are similar to previous changes (2)

• src/Options/docs.js
• src/Options/index.js

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between b28606b and 08a6bc7.

📒 Files selected for processing (1)

• spec/vulnerabilities.spec.js

[parseplatformorg]

🎉 This change has been released in version 9.6.0-alpha.4

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.59%. Comparing base (8f82282) to head (08a6bc7).
⚠️ Report is 121 commits behind head on alpha.

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

[parseplatformorg]

🎉 This change has been released in version 9.6.0