Skip to content

Conversation

@Fail-Safe
Copy link
Owner

@Fail-Safe Fail-Safe commented Dec 5, 2025

Address security/code-scanning/9: Sanitize dynamic HTML content in the webview and avoid unsanitized innerHTML to reduce DOM XSS risk. Also fixed linter issues in src/lib/usageHistory.ts (removed unused async/unused params).\n\nChanges include:\n- Added escapeHtml() helper in media/webview.js and used it when interpolating user-controlled or extension-provided strings into innerHTML templates (insights, anomalies, included limit text, metrics).\n- Replaced some innerHTML usage with DOM-safe textContent where applicable.\n- Fixed linter warnings in src/lib/usageHistory.ts and removed unnecessary async/await in archiveOldData to avoid require-await ESLint rule.\n\nValidation:\n- npm run lint passes.\n- npm test all suites pass (53 tests).\n\nNotes:\n- This partially addresses CodeQL findings by escaping values before adding them to HTML. If you prefer, we can take a more defensive route and replace templates with DOM construction entirely (safer but larger diff).\n- If you’d like me to further harden webview templates, I can convert the remaining innerHTML blocks to DOM assumptions using createElement + textContent instead of HTML templates.\n


html += '</div>';

container.innerHTML = html;

Check failure

Code scanning / CodeQL

Client-side cross-site scripting High

Cross-site scripting vulnerability due to
user-provided value
.

Copilot Autofix

AI about 1 month ago

To fix the DOM-based XSS vulnerability, all instances where untrusted data is inserted into HTML must be properly escaped before being included in HTML strings set via innerHTML. In this case, the affected code is in the renderMultiMonthAnalysis function, especially where values from the analysis object (ultimately from event.data) are interpolated into the html string.

The safest and least intrusive fix is to create a reusable HTML-escaping function, such as escapeHtml, and apply it to any untrusted values inserted into the HTML string. This method leaves all layout and logic unchanged, only ensuring that inserted content cannot break HTML context or inject scripts. The escapeHtml function can be added near the top of the module, or immediately before its first use.

Approach

  • Add a helper function (e.g., escapeHtml(str)) that escapes <, >, &, ", ', and `.
  • In renderMultiMonthAnalysis, wrap all instances of analysis.dataMonths (and any other untrusted values) inside escapeHtml(String(...)) when interpolating into the HTML string.
  • (If there are other places where untrusted data is injected into HTML in this block, escape those as well; focus on what is shown in the snippet.)
  • No changes needed to dependencies—basic escaping can be accomplished with a simple utility.

Suggested changeset 1
media/webview.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/media/webview.js b/media/webview.js
--- a/media/webview.js
+++ b/media/webview.js
@@ -1100,6 +1100,17 @@
     }
   });
 
+  // Escapes user-provided values before injecting into HTML to prevent XSS
+  function escapeHtml(str) {
+    return String(str)
+      .replace(/&/g, '&amp;')
+      .replace(/</g, '&lt;')
+      .replace(/>/g, '&gt;')
+      .replace(/"/g, '&quot;')
+      .replace(/'/g, '&#39;')
+      .replace(/`/g, '&#96;');
+  }
+
   function renderMultiMonthAnalysis(analysis) {
     try { log('[renderMultiMonthAnalysis] called with: ' + JSON.stringify(analysis)); } catch { }
 
@@ -1127,7 +1138,7 @@
     // Build the HTML
     let html = '<h3>Multi-Month Analysis</h3>';
     html += '<div class="analysis-summary">';
-    html += '<p><strong>Analysis Period:</strong> ' + analysis.dataMonths + ' month' + (analysis.dataMonths > 1 ? 's' : '') + ' of data</p>';
+    html += '<p><strong>Analysis Period:</strong> ' + escapeHtml(analysis.dataMonths) + ' month' + (Number(analysis.dataMonths) > 1 ? 's' : '') + ' of data</p>';
 
     // Growth Trends
     if (analysis.growthTrends && analysis.growthTrends.length > 0) {
EOF
@@ -1100,6 +1100,17 @@
}
});

// Escapes user-provided values before injecting into HTML to prevent XSS
function escapeHtml(str) {
return String(str)
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#39;')
.replace(/`/g, '&#96;');
}

function renderMultiMonthAnalysis(analysis) {
try { log('[renderMultiMonthAnalysis] called with: ' + JSON.stringify(analysis)); } catch { }

@@ -1127,7 +1138,7 @@
// Build the HTML
let html = '<h3>Multi-Month Analysis</h3>';
html += '<div class="analysis-summary">';
html += '<p><strong>Analysis Period:</strong> ' + analysis.dataMonths + ' month' + (analysis.dataMonths > 1 ? 's' : '') + ' of data</p>';
html += '<p><strong>Analysis Period:</strong> ' + escapeHtml(analysis.dataMonths) + ' month' + (Number(analysis.dataMonths) > 1 ? 's' : '') + ' of data</p>';

// Growth Trends
if (analysis.growthTrends && analysis.growthTrends.length > 0) {
Copilot is powered by AI and may make mistakes. Always verify output.
@Fail-Safe Fail-Safe changed the title feat: normalize numeric formatting & add time-range selector for usage trends feat: normalize numeric formatting & add time-range selector for usage trends (sanitization fix) Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants