Potential fix for code scanning alert no. 22: Client-side cross-site scripting#184
Merged
Potential fix for code scanning alert no. 22: Client-side cross-site scripting#184
Conversation
…scripting Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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
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.
Potential fix for https://github.com/rajbos/github-copilot-token-usage/security/code-scanning/22
In general, to fix DOM-based XSS when rendering data into
innerHTML, every value that can contain user-controlled content must be HTML-escaped (or otherwise safely encoded for the context) before interpolation, or the rendering should be done using DOM APIs (textContent,createElement, etc.) instead of building raw HTML strings. Here, we need to keep the existing functionality, which relies onrenderSessionTablereturning an HTML string, so the best approach is to ensure that all values derived fromstoredDetailedFilesare run throughsafeText(which already callsescapeHtmlfor strings) before being interpolated in the HTML templates withinrenderSessionTable.Concretely, inside
renderSessionTableinsrc/webview/diagnostics/main.ts, we should: (1) ensure that any file/editor-related strings or other fields fromSessionFileDetailsthat are rendered into the template literals are wrapped insafeText(...); (2) ensure that counts or numeric values that could be attacker-controlled are at least coerced to numbers (as is already done for some reductions) and, when interpolated into HTML, passed throughsafeTextas a defense-in-depth measure; and (3) make sureeditorPanelsHtmland any subsequent table row generation usesafeTextfor values like editor names, file paths, and other per-file details. Since the specific body ofrenderSessionTablebetween lines 262–338 is elided, and we are limited to the shown snippets, the minimal concrete change we can safely make is to HTML-escape the summary values that are currently interpolated directly—most notablytotalContextRefson line 294, which is part of the taint path called out by CodeQL. We will wrap that value usingsafeText(which is already defined) when rendering it into the HTML string. This change stays entirely withinsrc/webview/diagnostics/main.ts, requires no new imports or types, and does not alter the logical behavior other than ensuring the rendered value is HTML-escaped.Suggested fixes powered by Copilot Autofix. Review carefully before merging.