chore(dashboard): sanitize dangerouslySetInnerHTML usage#1038
Open
imparandodev[bot] wants to merge 1 commit into
Open
chore(dashboard): sanitize dangerouslySetInnerHTML usage#1038imparandodev[bot] wants to merge 1 commit into
imparandodev[bot] wants to merge 1 commit into
Conversation
The bootstrap banner rendered in AsyncRequests and Batches passed an HTML string from window.bootstrapContent (sourced from public/bootstrap.js) straight into dangerouslySetInnerHTML. Even though that source is same-origin and dev-controlled today, that's a fragile assumption: any future fetch-based or operator-customised replacement would silently become an XSS sink. Introduce a SafeHTML wrapper that runs DOMPurify on the input before rendering, allowing only the markup the banner needs (style, basic layout, SVG icons) and explicitly forbidding script/iframe/object/embed plus inline event handlers. FORCE_BODY is enabled so a leading <style> in the input survives sanitisation. Wire the wrapper into both banner sites, and harden the chart.tsx <style> generator (also a dangerouslySetInnerHTML sink) by validating chart ids, config keys, and color values against strict CSS-identifier and CSS-color regexes before interpolating them into the rule. Anything that doesn't match is dropped, which closes the CSS-injection vector that an attacker-controlled config object could otherwise exploit. Add focused tests for SafeHTML covering: allowed markup pass-through, script/event-handler/javascript: URL stripping, and <style> preservation (needed for the banner). Full ts test suite + production build pass locally.
Deploying control-layer with
|
| Latest commit: |
f7172a3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://90c2be93.control-layer.pages.dev |
| Branch Preview URL: | https://chore-sanitize-dangerous-htm.control-layer.pages.dev |
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.
Summary
Three call sites in
dashboard/were piping strings intodangerouslySetInnerHTML. Two of them (the bootstrap banner onAsyncRequests.tsxandBatches.tsx) read fromwindow.bootstrapContent, which today comes from a same-origin static asset (public/bootstrap.js) — but treating that as inherently safe is a brittle assumption. The third (ui/chart.tsx) generates a<style>block from chart config, interpolatingid,key, andcolorstrings directly into CSS — a CSS-injection vector if any of those become attacker-controlled.This PR closes those vectors:
components/ui/safe-html.tsx— a<SafeHTML html=... />wrapper that runs the input through DOMPurify before passing todangerouslySetInnerHTML. Configured to allow<style>(the banner uses inline CSS) plusFORCE_BODY: trueso a leading<style>survives sanitization, and explicitly forbidsscript/iframe/object/embedplus inline event handler attrs.AsyncRequests.tsxandBatches.tsx— replace raw<div dangerouslySetInnerHTML />with<SafeHTML html=... />.ui/chart.tsx— validateid, configkeys andcolorvalues against strict^[A-Za-z0-9_-]+$(identifier) and^[A-Za-z0-9 ,.()#%/_-]+$(color) regexes; drop any entry that fails. The<style>tag now only ever contains values that survive that filter.safe-html.test.tsxcovering: allowed markup pass-through,<script>stripping, inline-event-handler stripping,javascript:URL stripping, and<style>preservation (required for the banner to render correctly).dompurify ^3.4.1(ships its own types).Threat model addressed
bootstrap.jsis ever swapped for an operator-customised file, fetched from an API, or compromised in transit, sanitization keeps the resulting markup within a known-safe profile rather than running arbitrary JS.<style>: if aChartConfigever ends up partially derived from user input (label fragments, search terms, etc.), the regex filters prevent CSS-rule break-out (}; @import url(evil)) and value-based exfiltration tricks.Local verification
just lint ts— ✅ cleanjust test ts— ✅ 503 / 503 passing (498 prior + 5 new SafeHTML tests)pnpm build— ✅ Vite production build succeedsTest plan
just lint tspasses in CIjust test tspasses in CI/asyncand/batcheswith the banner enabled — banner renders identical to main--color-*CSS vars