-
Notifications
You must be signed in to change notification settings - Fork 1.8k
JS: Handle default 'content-type' header in Response() objects #20918
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
base: main
Are you sure you want to change the base?
Conversation
6ed9a67 to
818f481
Compare
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.
Pull request overview
This PR improves the JavaScript XSS detection by recognizing that the Response() constructor defaults to text/plain;charset=utf-8 content-type when no explicit content-type header is set. This means that new Response(data) without an explicit HTML content-type header is not an XSS sink, reducing false positives.
Key Changes:
- Updated the
ResponseArgumentHeadersclass to model the default content-type header behavior - Removed XSS alerts from test cases where no content-type or non-HTML content-type is used
- Added new test cases to verify the behavior with unrelated headers
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll | Implements logic to add a default text/plain;charset=utf-8 content-type when no explicit content-type header is defined |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js | Updates test cases by removing XSS alert annotations from Response calls with default or plain-text content-type, and adds new test cases |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected | Removes expected XSS alerts that are now correctly not detected due to default content-type |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected | Removes expected XSS alerts consistent with the main test expectations |
| javascript/ql/src/change-notes/2025-11-26-response-default-content-type.md | Documents the analysis improvement in the release notes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js
Outdated
Show resolved
Hide resolved
javascript/ql/src/change-notes/2025-11-26-response-default-content-type.md
Outdated
Show resolved
Hide resolved
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * `new Response(x)` is not longer seen as a reflected XSS sink when no`content-type` header |
Copilot
AI
Nov 27, 2025
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.
Typo: "is not longer" should be "is no longer".
| * `new Response(x)` is not longer seen as a reflected XSS sink when no`content-type` header | |
| * `new Response(x)` is no longer seen as a reflected XSS sink when no`content-type` header |
Co-authored-by: Copilot <[email protected]>
When using
new Response()to construct HTTP responses, thecontent-typeheader defaults totext/plain;charset=utf-8unless explicitly set to something else.This means its argument is not an html-injection sink when the header is omitted.