fix(core/data-abbr): DOM text reinterpreted as HTML#5196
Conversation
|
@copilot see the CI error about the commit message. Can you fix that please? |
Fixed in 8dc77e0. Updated the commit message to follow the conventional commits format: |
322667e to
8d3976d
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a security finding where DOM text could be reinterpreted as HTML by removing an insertAdjacentHTML sink and constructing the <abbr> node using safe DOM APIs in core/data-abbr.
Changes:
- Replace
insertAdjacentHTMLusage inprocessDfnElement()withcreateElement+ property assignment (title,textContent) and adjacent text insertion for parentheses. - Add a regression test ensuring HTML-special characters in
<dfn>text anddata-abbrare preserved correctly (no HTML reinterpretation).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/core/data-abbr.js | Removes HTML string insertion and builds the <abbr> + surrounding text nodes via DOM APIs to prevent HTML reinterpretation. |
| tests/spec/core/data-abbr-spec.js | Adds coverage for special characters in <dfn> text and data-abbr to validate the safer DOM construction behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Can you rebase, lint, and prettier this? |
1 similar comment
|
@copilot Can you rebase, lint, and prettier this? |
Verify that dfn text containing ampersands and quotes is safely handled via DOM construction.
3c93480 to
4686ccd
Compare
Replace insertAdjacentHTML with DOM construction (dfn.after()) in processDfnElement to safely handle untrusted text without HTML sink. Addresses code scanning alert: DOM text reinterpreted as HTML. Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
Potential fix for https://github.com/speced/respec/security/code-scanning/5
Use DOM APIs to construct nodes and set text/attributes directly instead of
insertAdjacentHTML. This preserves existing behavior while preventing HTML reinterpretation.Best fix in
src/core/data-abbr.js:processDfnElement, replace theinsertAdjacentHTMLcall (lines 33–36 region) with:document.createTextNode(" (")document.createElement("abbr")abbrElem.title = fullFormabbrElem.textContent = abbrdocument.createTextNode(")")dfnin order.generateAbbreviation/elem.textContent.No new imports or dependencies are required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.