Skip to content

Conversation

@sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Oct 29, 2025

RFC: Icon Packs Support in Mermaid via config

Status: Review

Created: August 15, 2025
Target Release: Mermaid v11.x
Authors: @sidharthv96, @VanceVisarisTenenbaum
Reviewers: Core, CLI, Live Editor teams


1. Summary

Mermaid should support icons natively via a first‑class, declarative configuration in MermaidConfig, enabling icons in browsers and CLI/headless renders (no SSR) without custom JavaScript. Users can register Iconify-compatible icon packs through config or programmatic API. This unblocks icons in environments like the Live Editor, Mermaid CLI, GitHub/GitLab renderers, MkDocs, Obsidian, etc., and makes Mermaid's icon story consistent across the ecosystem.

2. Motivation & Goals

Problems today

  • Icon usage (e.g., in Architecture diagrams) requires programmatic mermaid.registerIconPacks(...), which isn't possible in many contexts (Live Editor, some Markdown renderers, static pipelines, CLI-only flows).
  • Users are confused when icons render in app contexts but not in the Live Editor/CLI.

Goals

  1. Declarative config to register icon packs (URL or package+version), consumed by all runtimes.
  2. Deterministic & portable: require version pinning for package specs to guarantee reproducible renders.
  3. Lazy & safe: load packs only when needed; allow opt‑in remote fetches with clear security guardrails.
  4. Backwards‑compatible: keep current JS API; add config-based path; maintain existing built‑in fallback icons (e.g., cloud, database, etc.).

Non‑Goals

  • Shipping large icon packs in the core bundle.
  • Curating/maintaining third‑party icon sets in the Mermaid repo.
  • Automatic discovery of icons from arbitrary HTML or Markdown content.

3. User Stories

  • CLI user: "I want my diagram to render AWS icons in CI without writing custom JS."
  • Docs/hosted renderers: "We need pinned icon packs with no client JS (e.g., via CLI/headless renders or provider renderers)."
  • Live Editor: "If I set icon packs in the editor's config, the preview should 'just work'."

4. Proposal

4.1 Config Additions (Core)

Nest all icon-related settings under a top-level icons object in MermaidConfig (and schema):

icons:
  packs:
    logos: '@iconify-json/logos@1'
  cdnTemplate: 'https://cdn.jsdelivr.net/npm/${packageSpec}/icons.json'
  maxFileSizeMB: 5
  timeout: 5000
  allowedHosts:
    - 'unpkg.com'
    - 'cdn.jsdelivr.net'
    - 'npmjs.com'
securityLevel: 'strict'

Rules

  • Value forms:

    • Package spec: @scope/package@<version> (must include major version; minors/patches optional).
  • Relative paths and local filesystem loading are not supported.

  • Only package specs are supported - direct URLs are not allowed for security reasons.

  • Lazy registration: Core resolves and registers icon packs after initialize(config) and before first render/parse.

  • Version pinning: package specs must include at least a major version; otherwise initialization throws (config validation error).

  • Security: see §7.

4.2 Public API

  • Keep existing mermaid.registerIconPacks([...]) for advanced/offline scenarios.
  • No new public API is required for config-based loading; initialization uses icons from MermaidConfig.

4.3 Diagram Syntax

  • Architecture diagram keeps its syntax: service db(logos:aws-aurora)[Database].

  • For diagrams that support icon attribute (e.g., new shapes/flowchart nodes), allow:

    • A@{ icon: "logos:aws-s3", label: "S3" }
  • Fallbacks: If an icon cannot be found, render the label and show a warning marker (not fatal).

4.4 CLI

  • Config file path works as today; CLI passes config with icons to core.

4.5 Live Editor

  • Live Editor does not need any changes, as core reads icons from the config JSON and applies it automatically on render.
  • For hosted editors (e.g., mermaid.live), ship a sane default:
{
  "icons": {
    "packs": {
      "logos": "@iconify-json/logos@1"
    },
    "cdnTemplate": "https://cdn.jsdelivr.net/npm/${packageSpec}/icons.json",
    "maxFileSizeMB": 5,
    "timeout": 5000,
    "allowedHosts": ["unpkg.com", "cdn.jsdelivr.net", "npmjs.com"]
  }
}
  • In hosted contexts, platforms may customize the CDN template and allowed hosts via the secure configuration to control which external resources can be accessed.

4.6 Hosted Renderers (GitHub/GitLab/mermaidchart.com/mermaid.ink)

  • Can use the secure config to set the level of customisation, ensuring the users cannot override values set by the platform via diagram config.
    • Block icons completely: Not preferred, as that would impact the UX, and diagrams with icons would not render the icons when viewed inside the platform. { secure: ["icons"] } will block users from using the icons feature.
    • Set the CDN: Can set the CDN URL to an internal mirror, to prevent external network calls, or a trusted external CDN. { icons: { cdnTemplate: "<internal mirror>" }, secure: ["icons.cdnTemplate"]}
    • Control allowed hosts: {icons: {allowedHosts: ["internal.cdn.com"]}, secure: ["icons.allowedHosts"]}

4.7 Schema & Types

  • Update config.schema.yaml:

    icons:
      description: |
        Configuration for icon packs and CDN template.
        Enables icons in browsers and CLI/headless renders without custom JavaScript.
      type: object
      properties:
        packs:
          description: |
            Icon pack configuration. Key is the local pack name. 
            Value is a package spec with version that complies with Iconify standards.
            Package specs must include at least a major version (e.g., '@iconify-json/logos@1').
          type: object
          additionalProperties:
            type: string
        cdnTemplate:
          description: |
            URL template for resolving package specs (must contain ${packageSpec}).
            Used to build URLs for package specs in icons.packs.
          type: string
          pattern: '^https://.*\$\{packageSpec\}.*$'
          default: 'https://cdn.jsdelivr.net/npm/${packageSpec}/icons.json'
        maxFileSizeMB:
          description: |
            Maximum file size in MB for icon pack JSON files.
          type: integer
          default: 5
          minimum: 1
          maximum: 10
        timeout:
          description: |
            Network timeout in milliseconds for icon pack fetches.
          type: integer
          default: 5000
          minimum: 1000
          maximum: 30000
        allowedHosts:
          description: |
            List of allowed hosts to fetch icons from
          type: array
          items:
            type: string
          default:
            - 'unpkg.com'
            - 'cdn.jsdelivr.net'
            - 'npmjs.com'
  • TypeScript:

    interface MermaidConfig {
      icons?: {
        packs?: Record<string, string>;
        cdnTemplate?: string;
        maxFileSizeMB?: number;
        timeout?: number;
        allowedHosts?: string[];
      };
    }

5. Detailed Design

5.1 Resolution & Loading

  • For each entry (name, spec) in icons.packs:

    1. Package spec → build URL by interpolating ${packageSpec} into icons.cdnTemplate (or default to jsDelivr template if not provided).
    2. Parse JSON, validate Iconify schema minimal fields (prefix, icons).
    3. Call existing mermaid.registerIconPacks([{ name, loader|icons }]).
  • Lazy: Defer network fetch until an icon in that pack is referenced during render (cache miss triggers fetch). Provide in-memory LRU cache by pack name.

5.2 Determinism

  • Mandatory version for package specs (at least a major).
  • For CI/air‑gapped builds, recommend bundling icons.json directly in project and hosting it over HTTPS.

5.3 Telemetry

  • No telemetry in core. (Live Editor may log anonymous fetchIcons events; core must remain telemetry‑free.)

5.4 Errors & Diagnostics

  • Initialization errors for invalid specs are surfaced as config validation failures.
  • At render time, unknown icons issue non‑fatal warnings collected via mermaid.parse diagnostics API (and exposed in CLI as stderr warnings), with actionable messages and the attempted icon key.

6. Backwards Compatibility

  • Existing registerIconPacks continues to work.
  • Existing diagrams with built-in icons continue to render.
  • If icons is absent, behavior is unchanged.

7. Security & CSP

  • Remote fetches occur over HTTPS only (including CDN-resolved package specs via icons.cdnTemplate).
  • Only package specs are supported - direct URLs are not allowed for security reasons.
  • Input validation: CDN template must contain ${packageSpec} placeholder; URLs must be valid HTTPS URLs.
  • Size limits: Maximum file size enforced via icons.maxFileSizeMB (default: 5MB, range: 1-10MB).
  • Timeouts: Network requests timeout after icons.timeout milliseconds (default: 5s).
  • Host allowlisting: Only fetch from hosts explicitly listed in icons.allowedHosts.
  • Platforms can disable the feature entirely using secure: ["icons"], or selectively lock fields like icons.cdnTemplate, icons.allowedHosts, icons.maxFileSizeMB, and icons.timeout with the secure option.
  • Respect host CSP; render icons as inline SVG paths from the icon JSON; no inline scripts/styles added.
  • SVGs should be sanitized to prevent XSS attacks.

8. Performance

  • Lazy loading by pack; cache JSON by (name, url); deduplicate concurrent fetches.
  • Optional size cap per pack; evict least‑recently used packs in long sessions.
  • Document guidance: prefer subset packs or hosted icons.json containing only used icons.

9. Licensing

  • Iconify JSON is MIT, but individual icons may have varying licenses.
  • Documentation must warn users to verify the licenses of the packs they configure.
  • Mermaid does not redistribute third‑party icon assets in the core bundle.

10. Alternatives Considered

  1. Status quo (programmatic only): fails in Live Editor/CLI and non‑JS environments.
  2. Bundle popular packs in core: increases bundle size and creates licensing burden.
  3. Custom sprite sheets: harder authoring and doesn't align with UX.

11. Rollout & Migration

  • Phase 1 (Core + CLI): add icons to core schema, implement loader, add CLI flags & tests.
  • Phase 2 (Live Editor): read icons from UI config (default logos@1), surface errors in UI.
  • Phase 3 (Docs & Samples): update docs to show config‑based registration; add end‑to‑end examples for CLI, browser, SSR.
  • Phase 4 (Ecosystem): file issues/PRs for popular wrappers to pass icons to Mermaid.

12. Open Questions

  • Should we permit data: URLs for tiny vendored packs?
  • Do we want a blocklist/allowlist of hosts for remote packs in strict mode?

13. API & Type Signatures (Illustrative)

export interface MermaidConfig {
  icons?: {
    packs?: Record<string, string>;
    cdnTemplate?: string;
    maxFileSizeMB?: number;
    timeout?: number;
    allowedHosts?: string[];
  };
}

export function initialize(config: MermaidConfig): void;
export function registerIconPacks(packs: IconPackSpec[]): void; // existing

14. Examples

14.1 Browser (no bundler)

<script type="module">
  import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs';
  mermaid.initialize({
    startOnLoad: true,
    icons: {
      packs: {
        logos: '@iconify-json/logos@1',
      },
      cdnTemplate: 'https://cdn.jsdelivr.net/npm/${packageSpec}/icons.json',
      maxFileSizeMB: 5,
      timeout: 5000,
      allowedHosts: ['unpkg.com', 'cdn.jsdelivr.net', 'npmjs.com'],
    },
  });
</script>

14.2 CLI

mmdc -i arch.mmd -o arch.svg --configFile mermaid.json
# mermaid.json contains { "icons": { "packs": { "logos": "@iconify-json/logos@1" } } }

14.3 Hosted Custom CDN

{
  "icons": {
    "packs": { "logos": "@iconify-json/logos@1" },
    "cdnTemplate": "https://internal.cdn.com/${packageSpec}/icons.json",
    "maxFileSizeMB": 2,
    "timeout": 10000,
    "allowedHosts": ["internal.cdn.com"]
  }
}

15. Test Plan

  • Unit: spec validation (must include major version), package resolution, cache behavior, security gates, input validation, size limits, timeouts, host allowlisting.
  • Integration: CLI renders with icons (remote), Live Editor loads pinned packs.
  • Docs e2e: samples verified in CI (render PNG+SVG snapshots).
  • Perf: measure first‑icon render latency and memory footprint per pack size.
  • Security: test SSRF prevention, size limit enforcement, timeout handling, host validation.

16. Docs Impact

  • Update Config › Icons and Architecture pages with config-first flow.
  • Add troubleshooting guide for CSP, offline mode, and licensing.
  • Document security considerations and best practices.

17. Risks & Mitigations

  • Network flakiness → caches & retries; document CDN alternatives.
  • Licensing confusion → prominent docs notes; default to zero packs.
  • Bundle bloat → lazy loading only; no packs shipped in core.
  • Security vulnerabilities → input validation, host allowlisting, size limits, timeouts.

Appendix A: Minimal Loader Reference

  • Default CDN template: https://cdn.jsdelivr.net/npm/${packageSpec}/icons.json (configurable via icons.cdnTemplate).
  • JSON must expose prefix and icons per Iconify format.

Appendix B: Example Diagram (Architecture)

architecture-beta
  group api(logos:aws-lambda)[API]
  service db(logos:aws-aurora)[Database] in api
  service obj(logos:aws-s3)[Object Storage] in api
  db:L -- R:obj
Loading

Appendix C: Migration Notes for Hosts

  • Live Editor: expose icons in the config UI; default logos@1.
  • GitHub/MkDocs/Obsidian: ensure wrappers pass icons to core and allow fetch if needed.
  • CLI: recommend pinning versions and/or using custom CDN templates.

@changeset-bot
Copy link

changeset-bot bot commented Oct 29, 2025

⚠️ No Changeset found

Latest commit: f8a0a61

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@netlify
Copy link

netlify bot commented Oct 29, 2025

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit f8a0a61
🔍 Latest deploy log https://app.netlify.com/projects/mermaid-js/deploys/69024f2a1c4c790008d98f55
😎 Deploy Preview https://deploy-preview-7113--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
Copy link
Contributor

The following issue(s) were detected:
• pnpm-lock.yaml changed without any package.json modification

Please address these and push an update.

Posted automatically by GitHub Actions

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 29, 2025

Open in StackBlitz

@mermaid-js/examples

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

mermaid

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

@mermaid-js/layout-elk

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

@mermaid-js/layout-tidy-tree

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

@mermaid-js/mermaid-zenuml

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

@mermaid-js/parser

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

@mermaid-js/tiny

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

commit: f8a0a61

@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

❌ Patch coverage is 0% with 203 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.54%. Comparing base (f28f3c2) to head (f8a0a61).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
packages/mermaid/src/rendering-util/icons.ts 0.00% 195 Missing ⚠️
packages/mermaid/src/Diagram.ts 0.00% 4 Missing ⚠️
packages/mermaid/src/utils/sanitizeDirective.ts 0.00% 3 Missing ⚠️
.build/jsonSchema.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #7113      +/-   ##
==========================================
- Coverage     3.55%   3.54%   -0.02%     
==========================================
  Files          473     473              
  Lines        47498   47644     +146     
  Branches       730     730              
==========================================
  Hits          1687    1687              
- Misses       45811   45957     +146     
Flag Coverage Δ
unit 3.54% <0.00%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
packages/mermaid/src/config.type.ts 100.00% <ø> (ø)
.build/jsonSchema.ts 0.00% <0.00%> (ø)
packages/mermaid/src/utils/sanitizeDirective.ts 5.88% <0.00%> (-0.28%) ⬇️
packages/mermaid/src/Diagram.ts 0.00% <0.00%> (ø)
packages/mermaid/src/rendering-util/icons.ts 0.44% <0.00%> (-0.74%) ⬇️
🚀 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.

@argos-ci
Copy link

argos-ci bot commented Oct 29, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 18 added Oct 29, 2025, 5:55 PM

@mermaid-js mermaid-js deleted a comment from github-actions bot Oct 29, 2025
@mermaid-js mermaid-js deleted a comment from github-actions bot Oct 29, 2025
@mermaid-js mermaid-js deleted a comment from github-actions bot Oct 29, 2025
@mermaid-js mermaid-js deleted a comment from github-actions bot Oct 29, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements declarative icon pack configuration support for Mermaid, enabling icons to be registered via configuration files rather than only through programmatic JavaScript APIs. This addresses a major gap where icons couldn't be used in environments like CLI, Live Editor, and hosted renderers without custom JavaScript. The implementation follows an RFC-driven approach with comprehensive security controls including host allowlisting, version pinning, size limits, and HTTPS-only fetches.

Key changes include:

  • Addition of icons configuration schema with support for package specs, CDN templates, security constraints (allowed hosts, file size limits, timeouts)
  • Dual icon manager architecture (global for programmatic API, ephemeral for diagram-scoped config)
  • Integration with diagram parsing lifecycle to register icons before parsing

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
packages/mermaid/src/schemas/config.schema.yaml Adds IconsConfig schema definition with properties for packs, cdnTemplate, security settings
packages/mermaid/src/config.type.ts Adds TypeScript interface for IconsConfig matching the schema
packages/mermaid/src/rendering-util/icons.ts Implements IconManager class, icon loading/validation logic, security checks, and registerDiagramIconPacks function
packages/mermaid/src/rendering-util/icons.spec.ts Comprehensive unit tests for icon loading, validation, network handling, and error cases
packages/mermaid/src/utils/sanitizeDirective.ts Adds special handling to skip sanitization of icons config (handled separately)
packages/mermaid/src/Diagram.ts Integrates icon registration into diagram parsing lifecycle
packages/mermaid/src/docs/config/icons.md Comprehensive documentation covering both declarative and programmatic approaches
docs/config/icons.md Mirror of source documentation for published docs
cypress/integration/rendering/icons.spec.ts Integration tests for icon rendering in various configurations
.build/jsonSchema.ts Adds icons to config keys for default value generation
.cspell/mermaid-terms.txt Adds mmdc term to spell checker dictionary
docs/config/setup/mermaid/type-aliases/ParseErrorFunction.md Updates line number reference due to import reordering
docs/config/setup/mermaid/interfaces/MermaidConfig.md Documents new icons configuration property

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

'packet',
'architecture',
'radar',
'icons',
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The icons key is added to MERMAID_CONFIG_DIAGRAM_KEYS, but unlike the other entries in this array (flowchart, sequence, gantt, etc.), icons is not a diagram-specific configuration. It's a top-level configuration that applies globally to all diagrams.

Adding it to this array might cause issues in the generateDefaults function, which expects all keys to be diagram configs with a $ref to a schema definition. The icons property uses $ref: '#/$defs/IconsConfig' (line 333 of config.schema.yaml), so it should work technically, but semantically it doesn't belong with diagram configs.

Consider either:

  1. Removing icons from this array if it doesn't need special default value handling, or
  2. Renaming the constant to better reflect that it includes non-diagram-specific configs as well (e.g., MERMAID_CONFIG_SUBSCHEMA_KEYS).
Suggested change
'icons',

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +223
} catch (error) {
if (error instanceof TypeError) {
if (error.name === 'AbortError') {
throw new Error(`Request timeout after ${timeout}ms while fetching icons from ${url}`);
}
throw new TypeError(`Network error while fetching icons from ${url}: ${error.message}`);
} else if (error instanceof SyntaxError) {
throw new SyntaxError(`Invalid JSON response from ${url}: ${error.message}`);
}
throw error;
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The AbortError detection logic may not work correctly. When AbortController.abort() is called, the fetch API typically throws a DOMException with name 'AbortError', not a TypeError. The current code checks error instanceof TypeError and then checks if error.name === 'AbortError', but this condition will never be true since AbortError is not a TypeError.

Consider updating the error handling to:

catch (error) {
  if (error instanceof Error && error.name === 'AbortError') {
    throw new Error(`Request timeout after ${timeout}ms while fetching icons from ${url}`);
  }
  if (error instanceof TypeError) {
    throw new TypeError(`Network error while fetching icons from ${url}: ${error.message}`);
  } else if (error instanceof SyntaxError) {
    throw new SyntaxError(`Invalid JSON response from ${url}: ${error.message}`);
  }
  throw error;
}

Copilot uses AI. Check for mistakes.
continue;
}

if (key === 'icons') {
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The check for icons key at line 38-41 bypasses the recursive sanitization of the icons object. This means that if a user provides icons in a diagram directive with nested properties that should be sanitized (e.g., malicious nested objects or CSS injection attempts in nested fields), those will not be sanitized.

Since icons is handled separately by registerDiagramIconPacks, consider whether nested sanitization is still needed. If the icons config should be validated/sanitized by registerDiagramIconPacks itself, this is fine. However, if there's a security concern with unsanitized nested content, consider calling sanitizeDirective(args[key]) before continuing, or ensure registerDiagramIconPacks performs proper validation.

Suggested change
if (key === 'icons') {
if (key === 'icons') {
// Sanitize icons object before skipping further processing
if (typeof args[key] === 'object') {
sanitizeDirective(args[key]);
}

Copilot uses AI. Check for mistakes.
const timeout = config?.timeout ?? defaultConfig.icons?.timeout ?? 0;

if (isUrl) {
throw new Error('Direct URLs are not allowed.');
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The error message "Direct URLs are not allowed." should be more descriptive to help users understand what they should use instead. Consider updating to: "Direct URLs are not allowed. Please use package specs with version (e.g., '@iconify-json/logos@1')."

Suggested change
throw new Error('Direct URLs are not allowed.');
throw new Error("Direct URLs are not allowed. Please use package specs with version (e.g., '@iconify-json/logos@1').");

Copilot uses AI. Check for mistakes.
Comment on lines +274 to +275
const maxFileSizeMB = config?.maxFileSizeMB ?? defaultConfig.icons?.maxFileSizeMB ?? 0;
const timeout = config?.timeout ?? defaultConfig.icons?.timeout ?? 0;
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The fallback values for maxFileSizeMB and timeout default to 0 when defaultConfig.icons is undefined or doesn't have these properties. This could cause issues since fetchIconsJson expects positive values (the schema defines minimum: 1 for maxFileSizeMB and minimum: 1000 for timeout).

If these values are 0, the file size check at line 199 (sizeMB > maxFileSizeMB) would always pass (allowing any size), and the timeout at line 178 would trigger immediately. Consider using the schema defaults (5 for maxFileSizeMB and 5000 for timeout) as hardcoded fallbacks:

const maxFileSizeMB = config?.maxFileSizeMB ?? defaultConfig.icons?.maxFileSizeMB ?? 5;
const timeout = config?.timeout ?? defaultConfig.icons?.timeout ?? 5000;
Suggested change
const maxFileSizeMB = config?.maxFileSizeMB ?? defaultConfig.icons?.maxFileSizeMB ?? 0;
const timeout = config?.timeout ?? defaultConfig.icons?.timeout ?? 0;
const maxFileSizeMB = config?.maxFileSizeMB ?? defaultConfig.icons?.maxFileSizeMB ?? 5;
const timeout = config?.timeout ?? defaultConfig.icons?.timeout ?? 5000;

Copilot uses AI. Check for mistakes.
Comment on lines +242 to +243
return hostname === allowedHost || hostname.endsWith(`.${allowedHost}`);
});
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

[nitpick] The subdomain validation allows any subdomain of the allowed host. For example, if jsdelivr.net is in the allowed hosts, then evil.jsdelivr.net, cdn.jsdelivr.net, and any other subdomain would be allowed. While this may be intentional for flexibility, it could be a security concern if users configure overly broad allowed hosts.

Consider adding documentation or a warning that encourages users to specify full hostnames (e.g., cdn.jsdelivr.net instead of just jsdelivr.net) to minimize the risk of accepting unintended hosts.

Copilot uses AI. Check for mistakes.
// Validate package version for package specs
validatePackageVersion(packageSpec);

// Build URL using CDN template
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The CDN template validation at line 285 only checks if ${packageSpec} is present in the template string, but doesn't verify that the template will produce a valid HTTPS URL. The schema pattern should enforce this, but runtime validation should also ensure the template starts with https:// to prevent potential security issues.

Consider adding:

if (!cdnTemplate.startsWith('https://')) {
  throw new Error('CDN template must start with https://');
}
if (!cdnTemplate.includes('${packageSpec}')) {
  throw new Error('CDN template must contain ${packageSpec} placeholder');
}
Suggested change
// Build URL using CDN template
// Build URL using CDN template
if (!cdnTemplate.startsWith('https://')) {
throw new Error('CDN template must start with https://');
}

Copilot uses AI. Check for mistakes.
// Validate package version for package specs
validatePackageVersion(packageSpec);

// Build URL using CDN template
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The validation at line 285 checks if cdnTemplate contains ${packageSpec}, but if cdnTemplate is an empty string (which could happen if the fallback chain returns empty), this check would pass the condition !cdnTemplate.includes('${packageSpec}') and throw an error. However, there's no explicit handling for the case where cdnTemplate is an empty string before this check.

Consider adding an explicit check:

if (!cdnTemplate) {
  throw new Error('CDN template is required but not configured');
}
if (!cdnTemplate.includes('${packageSpec}')) {
  throw new Error('CDN template must contain ${packageSpec} placeholder');
}
Suggested change
// Build URL using CDN template
// Build URL using CDN template
if (!cdnTemplate) {
throw new Error('CDN template is required but not configured');
}

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +45
const mockIconsWithMultipleIcons: IconifyJSON = {
prefix: 'test',
icons: {
'test-icon': {
body: '<path d="M0 0h24v24H0z"/>',
width: 24,
height: 24,
},
'another-icon': {
body: '<path d="M12 12h12v12H12z"/>',
width: 24,
height: 24,
},
},
};

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Unused variable mockIconsWithMultipleIcons.

Suggested change
const mockIconsWithMultipleIcons: IconifyJSON = {
prefix: 'test',
icons: {
'test-icon': {
body: '<path d="M0 0h24v24H0z"/>',
width: 24,
height: 24,
},
'another-icon': {
body: '<path d="M12 12h12v12H12z"/>',
width: 24,
height: 24,
},
},
};

Copilot uses AI. Check for mistakes.

// Build URL using CDN template
if (!cdnTemplate.includes('${packageSpec}')) {
throw new Error('CDN template must contain ${packageSpec} placeholder');
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

This string is not a template literal, but appears to reference the variable packageSpec.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants