You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/01 23:30:42 UTC

[PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

rusackas opened a new pull request, #40627:
URL: https://github.com/apache/superset/pull/40627

   ### SUMMARY
   
   The shared HTML sanitizer in `superset-ui-core` (`utils/html.tsx`) allows inline `style` attributes on a set of whitelisted elements, but it disabled CSS value validation by setting `css: false` on the `FilterXSS` instance. With validation disabled, any CSS property — including layout and positioning properties such as `position`, `z-index`, `width`, and `height` — passed through untouched.
   
   This change replaces `css: false` with a restrictive CSS property allowlist limited to safe presentational properties (`color`, `background-color`, `font-*`, `text-*`, spacing, borders, etc.). The `xss` library validates declared properties against this allowlist and also validates their values, so unsupported constructs (e.g. `url(...)`, `expression(...)`) are stripped as well.
   
   This sanitizer is reachable when markup is rendered with HTML rendering enabled (e.g. table/pivot cells with `allow_render_html`), so tightening the allowlist hardens that path while keeping ordinary presentational styling intact.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A
   
   ### TESTING INSTRUCTIONS
   
   - `npx jest packages/superset-ui-core/src/utils/html.test.tsx`
   - New tests cover: allowed presentational properties are preserved, layout/positioning properties are stripped, and unsafe property values are removed.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4597336508

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 61840f211ac917aca5dc93fc97096bb1b9aa4b3c |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a1e1622a466590008fc2bc2 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40627--superset-docs-preview.netlify.app](https://deploy-preview-40627--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQwNjI3LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.YEt12mOl0v-uRp23PbaABx7HauzMlVCQJU0Zhu6fDPQ)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrLDYAgDADQXXrWCESJuoFjQCmRRIFAD37i7nJ7h_dCLunMDCtAB7aYiHuzD9dQqHIJyH01MXB4yPVYa1uO8pHuzbWnjSSplTKj1tMihJg9KosKvh8) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-40627--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4597319599

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40627:
URL: https://github.com/apache/superset/pull/40627


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4598501198

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4607863810

   **Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4605864161

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(core): restrict allowed CSS properties in sanitized HTML [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40627:
URL: https://github.com/apache/superset/pull/40627#issuecomment-4605882832

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40627?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 64.02%. Comparing base ([`87be424`](https://app.codecov.io/gh/apache/superset/commit/87be424f9c4f1841d7185047cab4aaf9450265ac?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7a50cbd`](https://app.codecov.io/gh/apache/superset/commit/7a50cbde75b794c6ab8a43299f4dadeda0252f84?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 68 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #40627   +/-   ##
   =======================================
     Coverage   64.02%   64.02%           
   =======================================
     Files        2648     2648           
     Lines      142275   142276    +1     
     Branches    32695    32695           
   =======================================
   + Hits        91088    91089    +1     
     Misses      49629    49629           
     Partials     1558     1558           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40627/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/40627/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.37% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40627?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org