refactor(nvd3): extract testable generateAnnotationTooltipContent helper by rusackas · Pull Request #40620 · apache/superset

rusackas · 2026-06-01T23:06:05Z

SUMMARY

The annotation-tooltip XSS fix this PR originally proposed already landed in #40502, which wrapped tipFactory's annotation HTML (along with the other nvd3 tooltip sinks) in dompurify.sanitize. Rather than close this as a duplicate, I've trimmed it to the part that adds value on top of #40502:

Refactor: extract the annotation-tooltip HTML construction out of tipFactory's inline .html() callback into a pure, exported generateAnnotationTooltipContent(layer, d) helper. This matches the file's existing pattern of standalone tooltip-content helpers (generateCompareTooltipContent, generateMultiLineTooltipContent, etc.) and makes the annotation path independently unit-testable without standing up d3-tip.
Tests fix(charts): sanitize tooltip HTML across nvd3, rose and partition plugins #40502 doesn't have: title + description rendering, fallback to the layer name when the title column is empty, and an explicit title-column XSS strip (fix(charts): sanitize tooltip HTML across nvd3, rose and partition plugins #40502 only covers the description column).

No behavior change — the output is still run through dompurify.sanitize before d3-tip inserts it via .html().

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — no visual change.

TESTING INSTRUCTIONS

cd superset-frontend && npx jest plugins/legacy-preset-chart-nvd3/test/utils.test.ts

17/17 pass (master's 13 + 4 net-new annotation-path tests).

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

🤖 Generated with Claude Code

bito-code-review · 2026-06-01T23:06:14Z

Code Review Agent Run #f953c7

Actionable Suggestions - 0

Additional Suggestions - 1

superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts - 1

Missing test: Object.values fallback · Line 126-166

The new test suite for `generateAnnotationTooltipContent()` covers the array path (line 281: `Array.isArray(layer.descriptionColumns)`) but the `Object.values(d)` fallback (line 283) is never exercised. Per rule [6262], tests should verify all logical paths. Add a test with `descriptionColumns` omitted or set to a non-array value to cover this branch.

Code suggestion

 @@ -159,10 +159,23 @@
      test('strips a script payload from a description column', () => {
        const html = generateAnnotationTooltipContent(layer, {
          title: 'Release',
          description: '<script>alert(document.cookie)</script>',
        });
        expect(html).not.toContain('<script>');
      });
+
+    test('falls back to Object.values when descriptionColumns is not an array', () => {
+      const layerWithoutDescriptionColumns = {
+        name: 'My annotations',
+        titleColumn: 'title',
+      };
+      const html = generateAnnotationTooltipContent(layerWithoutDescriptionColumns, {
+        title: 'Release',
+        description: 'Shipped v1',
+      });
+      expect(html).toContain('Release - My annotations');
+      expect(html).toContain('Shipped v1');
+    });
    });

Review Details

Files reviewed - 2 · Commit Range: eb8f9f8..eb8f9f8
- superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts
- superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.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

netlify · 2026-06-01T23:10:35Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`24c1f15`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a1f73bc172ab70008e5c9d4
😎 Deploy Preview	https://deploy-preview-40620--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR hardens the legacy NVD3 annotation-layer tooltip rendering against XSS by sanitizing the HTML passed to d3-tip (which is inserted via innerHTML). It aligns the annotation tooltip path with other tooltip helpers in the same module that already use dompurify.

Changes:

Added generateAnnotationTooltipContent(layer, d) to build and sanitize annotation tooltip HTML.
Updated tipFactory() to use the new helper (preserving the existing falsy-d guard behavior).
Added Jest coverage for normal rendering, title fallback, and common XSS payload stripping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts`	Extracts annotation tooltip HTML generation and sanitizes output via `dompurify` before d3-tip renders it.
`superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.ts`	Adds unit tests validating expected tooltip content and that sanitization removes unsafe markup/attributes.

codecov · 2026-06-01T23:25:16Z

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.40%. Comparing base (3bbb35e) to head (24c1f15).

Files with missing lines	Patch %	Lines
...tend/plugins/legacy-preset-chart-nvd3/src/utils.ts	88.88%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #40620   +/-   ##
=======================================
  Coverage   63.40%   63.40%           
=======================================
  Files        2662     2662           
  Lines      143254   143253    -1     
  Branches    32941    32941           
=======================================
+ Hits        90835    90836    +1     
+ Misses      50816    50814    -2     
  Partials     1603     1603

Flag	Coverage Δ
javascript	`67.51% <88.88%> (+<0.01%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

The annotation-tooltip sanitization landed in #40502 (inline in tipFactory). This extracts that logic into a pure, exported generateAnnotationTooltipContent helper — matching the file's existing standalone tooltip-content helpers and making the annotation path independently unit-testable without d3-tip — and adds coverage #40502 lacks: title/description rendering, fallback to the layer name when the title column is empty, and an explicit title-column XSS strip. No behavior change; output is still run through dompurify before reaching d3-tip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

bito-code-review · 2026-06-03T03:23:25Z

Code Review Agent Run #22ac10

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 24c1f15..24c1f15
- superset-frontend/plugins/legacy-preset-chart-nvd3/src/utils.ts
- superset-frontend/plugins/legacy-preset-chart-nvd3/test/utils.test.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

pull-request-size Bot added the size/M label Jun 1, 2026

rusackas added this to Superset Review Help Wanted Jun 1, 2026

github-project-automation Bot moved this to Needs Review in Superset Review Help Wanted Jun 1, 2026

github-actions Bot added the plugins label Jun 1, 2026

rusackas requested a review from Copilot June 1, 2026 23:22

Copilot started reviewing on behalf of rusackas June 1, 2026 23:22 View session

rusackas requested a review from sha174n June 1, 2026 23:22

Copilot AI reviewed Jun 1, 2026

View reviewed changes

rusackas requested a review from dpgaspar June 1, 2026 23:35

rusackas moved this from Needs Review to Needs Follow-Up Work in Superset Review Help Wanted Jun 2, 2026

rusackas force-pushed the fix/nvd3-annotation-tooltip-sanitize branch from eb8f9f8 to 24c1f15 Compare June 3, 2026 00:22

rusackas changed the title ~~fix(nvd3): sanitize annotation tooltip HTML in tipFactory~~ refactor(nvd3): extract testable generateAnnotationTooltipContent helper Jun 3, 2026

github-actions Bot added the preset-io label Jun 3, 2026

sha174n approved these changes Jun 3, 2026

View reviewed changes

rusackas merged commit 6ea4e22 into master Jun 3, 2026
73 of 74 checks passed

rusackas deleted the fix/nvd3-annotation-tooltip-sanitize branch June 3, 2026 18:56

github-project-automation Bot moved this from Needs Follow-Up Work to Approved and/or Merged in Superset Review Help Wanted Jun 3, 2026

sha174n added the merge-if-green If approved and tests are green, please go ahead and merge it for me label Jun 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(nvd3): extract testable generateAnnotationTooltipContent helper#40620

refactor(nvd3): extract testable generateAnnotationTooltipContent helper#40620
rusackas merged 1 commit into
masterfrom
fix/nvd3-annotation-tooltip-sanitize

rusackas commented Jun 1, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Jun 1, 2026 •

edited

Loading

Code Review Agent Run #f953c7

Uh oh!

netlify Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Jun 3, 2026 •

edited

Loading

Code Review Agent Run #22ac10

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rusackas commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #f953c7

Uh oh!

netlify Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #22ac10

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rusackas commented Jun 1, 2026 •

edited

Loading

bito-code-review Bot commented Jun 1, 2026 •

edited

Loading

netlify Bot commented Jun 1, 2026 •

edited

Loading

codecov Bot commented Jun 1, 2026 •

edited

Loading

bito-code-review Bot commented Jun 3, 2026 •

edited

Loading