Copilot commented on code in PR #40627:
URL: https://github.com/apache/superset/pull/40627#discussion_r3337750804
##########
superset-frontend/packages/superset-ui-core/src/utils/html.tsx:
##########
@@ -19,6 +19,41 @@
import { FilterXSS, getDefaultWhiteList } from 'xss';
import { DataRecordValue } from '../types';
+// Restrict inline `style` attributes to a small set of presentational CSS
+// properties. Layout/positioning properties (e.g. position, z-index, width,
+// height) are intentionally excluded so that sanitized markup cannot affect
+// surrounding page layout. The `xss` library also validates property values
+// against this allowlist, stripping unsupported constructs such as
url()/expression().
Review Comment:
The comment says excluded properties ensure sanitized markup cannot affect
surrounding page layout, but the allowlist includes margin/padding/border (and
may include max-width/overflow for tooltips), which can still change layout
within a container. Please adjust the comment to reflect the actual goal (block
overlay/positioning and page-covering styles).
##########
superset-frontend/packages/superset-ui-core/src/utils/html.tsx:
##########
@@ -19,6 +19,41 @@
import { FilterXSS, getDefaultWhiteList } from 'xss';
import { DataRecordValue } from '../types';
+// Restrict inline `style` attributes to a small set of presentational CSS
+// properties. Layout/positioning properties (e.g. position, z-index, width,
+// height) are intentionally excluded so that sanitized markup cannot affect
+// surrounding page layout. The `xss` library also validates property values
+// against this allowlist, stripping unsupported constructs such as
url()/expression().
+const allowedCssProperties = {
+ color: true,
+ 'background-color': true,
+ 'text-align': true,
+ 'text-decoration': true,
+ 'font-family': true,
+ 'font-size': true,
+ 'font-style': true,
+ 'font-weight': true,
+ 'line-height': true,
+ 'letter-spacing': true,
+ 'white-space': true,
+ padding: true,
+ 'padding-top': true,
Review Comment:
The new CSS allowlist drops several inline style properties that are relied
on elsewhere in superset-ui-core (notably tooltipHtml + its Jest expectations):
`max-width`, `overflow`, `text-overflow`, and `opacity`. With the current
allowlist, tooltip truncation/ellipsis and row opacity styling will be
stripped, and existing tooltip tests will fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
