Skip to content

fix(frontend/setup): sanitize returned client error message when shown as HTML content#41768

Merged
rusackas merged 3 commits into
masterfrom
fix/sanitize-client-error-message-when-shown-as-html-content
Jul 7, 2026
Merged

fix(frontend/setup): sanitize returned client error message when shown as HTML content#41768
rusackas merged 3 commits into
masterfrom
fix/sanitize-client-error-message-when-shown-as-html-content

Conversation

@hainenber

@hainenber hainenber commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Resolves https://github.com/apache/superset/security/code-scanning/2559

This has been present quite a long time but no idea why the alert starts ringing recently. Anyhow, this HTML sanitization should be sufficient to prevent accidental XSS.

I manually sanity-test the app. All good

image image

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

Green CI as acceptance criterion.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

…n as HTML content

Signed-off-by: hainenber <dotronghai96@gmail.com>
@dosubot dosubot Bot added the change:frontend Requires changing the frontend label Jul 4, 2026
@bito-code-review

bito-code-review Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #875ef3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: d250eac..d250eac
    • superset-frontend/src/setup/setupApp.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/src/setup/setupApp.ts Outdated
@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. Passing an object-shaped message to sanitizeHtmlIfNeeded causes a runtime error because the utility expects a string. To resolve this, you should ensure resp.message is a string before sanitizing, or provide a safe fallback.

Here is the corrected implementation for superset-frontend/src/setup/setupApp.ts:

    .append(resp.message && typeof resp.message === 'string' ? sanitizeHtmlIfNeeded(resp.message) : (resp.message || ''))

I have checked the PR comments, and there are no other comments to address. Would you like me to proceed with any other changes?

superset-frontend/src/setup/setupApp.ts

.append(resp.message && typeof resp.message === 'string' ? sanitizeHtmlIfNeeded(resp.message) : (resp.message || ''))

Signed-off-by: hainenber <dotronghai96@gmail.com>
@bito-code-review

bito-code-review Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #4f6793

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: d250eac..74f815d
    • superset-frontend/src/setup/setupApp.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.66%. Comparing base (687fafd) to head (f4df5a1).

Files with missing lines Patch % Lines
superset-frontend/src/setup/setupApp.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #41768   +/-   ##
=======================================
  Coverage   64.66%   64.66%           
=======================================
  Files        2685     2685           
  Lines      148593   148593           
  Branches    34282    34282           
=======================================
  Hits        96088    96088           
  Misses      50740    50740           
  Partials     1765     1765           
Flag Coverage Δ
javascript 69.50% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rusackas rusackas requested review from rusackas and sadpandajoe July 6, 2026 17:31

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hainenber, wrapping this in sanitizeHtml before the append is the right move. LGTM!

@rusackas rusackas merged commit 2a77c01 into master Jul 7, 2026
66 checks passed
@rusackas rusackas deleted the fix/sanitize-client-error-message-when-shown-as-html-content branch July 7, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants