Skip to content

fix(block): respect current config when sanitizing labels#7674

Open
cyphercodes wants to merge 1 commit intomermaid-js:developfrom
cyphercodes:bug/7622_block-sanitize-config
Open

fix(block): respect current config when sanitizing labels#7674
cyphercodes wants to merge 1 commit intomermaid-js:developfrom
cyphercodes:bug/7622_block-sanitize-config

Conversation

@cyphercodes
Copy link
Copy Markdown
Contributor

📑 Summary

Fix block diagram label sanitization so it reads the current Mermaid config instead of a module-scope snapshot.

Resolves #7622

📏 Design Decisions

  • Removed the block DB module-scope config capture used for label sanitization.
  • Fetch the current config when sanitizing block labels so runtime dompurifyConfig updates are respected.
  • Added a regression test that updates dompurifyConfig after import and verifies <b> labels are stripped.

📋 Tasks

  • 📖 have read the contribution guidelines
  • 💻 have added necessary unit/e2e tests
  • 📓 documentation not needed for this bug fix
  • 🦋 added a patch changeset

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 27, 2026

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit cc75089
🔍 Latest deploy log https://app.netlify.com/projects/mermaid-js/deploys/69ef6d84d7b5de0008d8b818
😎 Deploy Preview https://deploy-preview-7674--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@github-actions github-actions Bot added the Type: Bug / Error Something isn't working or is incorrect label Apr 27, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 27, 2026

🦋 Changeset detected

Latest commit: cc75089

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mermaid Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 27, 2026

Open in StackBlitz

@mermaid-js/examples

npm i https://pkg.pr.new/@mermaid-js/examples@7674

mermaid

npm i https://pkg.pr.new/mermaid@7674

@mermaid-js/layout-elk

npm i https://pkg.pr.new/@mermaid-js/layout-elk@7674

@mermaid-js/layout-tidy-tree

npm i https://pkg.pr.new/@mermaid-js/layout-tidy-tree@7674

@mermaid-js/mermaid-zenuml

npm i https://pkg.pr.new/@mermaid-js/mermaid-zenuml@7674

@mermaid-js/parser

npm i https://pkg.pr.new/@mermaid-js/parser@7674

@mermaid-js/tiny

npm i https://pkg.pr.new/@mermaid-js/tiny@7674

commit: cc75089

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 3.33%. Comparing base (d23e36d) to head (cc75089).

Files with missing lines Patch % Lines
packages/mermaid/src/diagrams/block/blockDB.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #7674      +/-   ##
==========================================
- Coverage     3.33%   3.33%   -0.01%     
==========================================
  Files          541     542       +1     
  Lines        56870   56879       +9     
  Branches       839     839              
==========================================
  Hits          1899    1899              
- Misses       54971   54980       +9     
Flag Coverage Δ
unit 3.33% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
packages/mermaid/src/diagrams/block/blockDB.ts 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 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.

Copy link
Copy Markdown
Collaborator

@knsv-bot knsv-bot left a comment

Choose a reason for hiding this comment

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

[sisyphus-bot]

Thanks for the focused fix! This is a clean, minimal change that addresses #7622 at the root cause — and the regression test does a great job of demonstrating exactly the user-visible behavior the issue described.

What's working well

  • 🎉 [praise] Right diagnosis, right fix. The const config = getConfig() at module load was capturing a stale snapshot — replacing it with a per-call configApi.getConfig() is the correct fix and lets runtime dompurifyConfig updates flow through to common.sanitizeText as intended (blockDB.ts:18).
  • 🎉 [praise] The new test in block.spec.ts is exactly the right shape — it sets FORBID_TAGS: ['b'] after import and verifies <b> is stripped from the label. That's a tight regression test that maps directly to the bug report.
  • ✅ Changeset present, correctly scoped as patch with the fix(block-beta): prefix.
  • ✅ No XSS or sanitization concerns — the change strengthens the pipeline by honoring runtime config tightening rather than ignoring it. No new DOM sinks, attributes, or HTML construction are introduced; only the config source for the existing common.sanitizeText call is updated.

Things to consider (non-blocking)

  • 🟢 [nit] Convention consistency — most other diagram DBs (classDb.ts:35, sankeyDB.ts:49, stateDb.ts, gitGraphAst.ts, ishikawaDb.ts) import getConfig directly from diagram-api/diagramAPI.js rather than going through configApi.getConfig(). They're functionally identical (diagramAPI re-exports it), so this works either way — but keeping the existing import { getConfig } from '../../diagram-api/diagramAPI.js' and just changing the body of sanitizeText to call getConfig() would minimize the diff and stay consistent with neighboring diagrams. Totally fine to leave as-is if you prefer.
  • 🟢 [nit] In block.spec.ts beforeEach, the configApi.reset() after configApi.setSiteConfig({}) is redundant — setSiteConfig already calls updateCurrentConfig internally (config.ts:80), so currentConfig is already in sync at that point. Harmless, just a touch of noise.
  • 💡 [suggestion] Worth considering a second test case that exercises a different dompurifyConfig knob (e.g., ADD_TAGS or FORBID_ATTR) to lock in the broader contract that the whole dompurify config is now respected at runtime, not just FORBID_TAGS. Not required for this PR — could land as a follow-up.

Nothing here is blocking — happy to see this go in. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug / Error Something isn't working or is incorrect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

block: label sanitization uses stale config captured at import time

2 participants