Skip to content

Potential fix for code scanning alert no. 45: Client-side cross-site scripting#330

Merged
rajbos merged 5 commits intomainfrom
alert-autofix-45
Mar 1, 2026
Merged

Potential fix for code scanning alert no. 45: Client-side cross-site scripting#330
rajbos merged 5 commits intomainfrom
alert-autofix-45

Conversation

@rajbos
Copy link
Owner

@rajbos rajbos commented Mar 1, 2026

Potential fix for https://github.com/rajbos/github-copilot-token-usage/security/code-scanning/45

In general, to fix this kind of problem you must ensure that any data coming from postMessage (or other untrusted sources) is either (a) validated and normalized into safe types (numbers, booleans, enums) before being used, or (b) sanitized/encoded before being written to the DOM, particularly when using innerHTML. Where feasible, replacing innerHTML construction with textContent and DOM APIs is even safer.

For this specific code, the minimal, non‑functional‑changing fix is to:

  1. Explicitly validate and coerce message.data into a trusted UsageAnalysisStats object before passing it to renderLayout, and
  2. Add a small helper sanitizeStats that ensures all interpolated fields in the template are safe primitive types (numbers/booleans) and cannot contain attacker-controlled HTML.

Because we must not assume anything outside the shown snippets, the fix will be self-contained in src/webview/usage/main.ts:

  • Define a helper function sanitizeStats(raw: any): UsageAnalysisStats | null just above renderLayout (line ~282). This function will:
    • Check that raw is an object and that all required properties (today, last30Days, backendConfigured, etc.) exist with the expected shape.
    • For each numeric statistic, coerce using Number(...) and default to 0 if the result is not a finite number.
    • For booleans, coerce using !!raw.prop.
    • Return null if the object is grossly malformed, so we can refuse to render.
  • Update the 'updateStats' case in the message handler to call sanitizeStats(message.data) and only pass the sanitized result into renderLayout. If sanitization fails, we simply do not re-render (or could add a minimal fallback).

This approach keeps the existing innerHTML template intact (so layout and behavior remain unchanged) but removes the ability for arbitrary strings to flow into the template: everything is normalized to non-executable primitives before use. That directly breaks the taint flow path identified by CodeQL.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…scripting

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@rajbos rajbos marked this pull request as ready for review March 1, 2026 18:11
@rajbos rajbos enabled auto-merge March 1, 2026 18:11
@rajbos
Copy link
Owner Author

rajbos commented Mar 1, 2026

@copilot , build fails with this error:

Run npm run check-types

> copilot-token-tracker@0.0.14 check-types
> tsc --noEmit

Error: src/webview/usage/main.ts(300,69): error TS2740: Type '{ file: number; selection: number; implicitSelection: number; symbol: number; codebase: number; workspace: number; }' is missing the following properties from type 'ContextReferenceUsage': terminal, vscode, terminalLastCommand, terminalSelection, and 8 more.
Error: src/webview/usage/main.ts(317,5): error TS2353: Object literal may only specify known properties, and 'toolUsage' does not exist in type 'UsageAnalysisPeriod'.
Error: src/webview/usage/main.ts(330,5): error TS2353: Object literal may only specify known properties, and 'toolUsage' does not exist in type 'UsageAnalysisPeriod'.
Error: Process completed with exit code 2.

Copy link
Contributor

Copilot AI commented Mar 1, 2026

@rajbos I've opened a new pull request, #331, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits March 1, 2026 18:19
…issing properties

Co-authored-by: rajbos <6085745+rajbos@users.noreply.github.com>
Fix TypeScript errors in sanitizeStats XSS security fix
@rajbos rajbos merged commit cf28e1d into main Mar 1, 2026
13 checks passed
@rajbos rajbos deleted the alert-autofix-45 branch March 1, 2026 19:52
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