rusackas commented on code in PR #40627:
URL: https://github.com/apache/superset/pull/40627#discussion_r3342874574
##########
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:
Good catch in principle, but these are already covered: opacity, max-width,
overflow and text-overflow are all in the allowlist (see the "Needed by ECharts
tooltips" entries), added specifically so tooltipHtml truncation/ellipsis and
row opacity keep working. The sharded Jest suite passes on this branch.
##########
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:
Fair point. I reworded the comment to reflect the actual goal: the
exclusions are about blocking overlay/positioning and page-covering styles
(position, z-index, transform, width/height), and the comment now explicitly
notes that the allowlisted margin/padding/border can still affect layout within
the container, which is acceptable.
--
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]
