feat: Add custom icon support for document layer markers

[heemin32]

Description

Add the ability to style document layer points as icons instead of plain circle markers. Users can choose from built-in icons with customizable fill/outline colors and style (filled or outline), or upload their own SVG icons.

Changes:

• Add marker/icon toggle to document layer style panel
• Add built-in icon set with color template system (FILL_COLOR, STROKE_COLOR)
• Add icon picker component with built-in and custom tabs
• Add custom SVG upload with client-side and server-side security validation
• Add map-icon saved object type for persisting custom icons
• Add CRUD API routes (POST, GET, DELETE) for custom icons
• Update DocumentLayerFunctions to render icons via maplibre symbol layers
• Extend DocumentLayerSpecification.style with markerType and iconConfig
• Add unit tests for icon set, rendering logic, and saved object type

Custom icons are stored as saved objects (shared across maps). When selected, the SVG is stored inline in the layer config so icons always render even if the saved object is later deleted.

Screen.Recording.2026-08-07.at.8.58.03.AM.mov

Issues Resolved

#568
#858

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1bf03ed)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 Security concerns SVG XSS surface: Custom SVGs are user-uploaded and rendered inline via dangerouslySetInnerHTML for built-in preview and stored/served back to clients. The layered validate+sanitize approach mitigates most vectors, but the server sanitizer is regex-based (fragile) and the client's sanitizeSvgForPreview allowlist includes style in the server allowlist but not client — inconsistency could allow style-based CSS attacks server-side. Additionally, the client renders custom icons via <img src=data:...> (safer) but the server allowlist permits style attribute while the validator blocks url()/expression() only via denylist. Recommend using a hardened SVG sanitization library on the server (DOMPurify + jsdom).

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Server-side custom icon saved object type and CRUD API Relevant files: server/routes/icon_router.ts server/routes/index.ts server/plugin.ts server/saved_objects/index.ts server/saved_objects/map_icon_saved_object.ts server/saved_objects/map_icon_saved_object.test.ts Sub-PR theme: Built-in icon set and maplibre symbol layer rendering Relevant files: public/model/documentLayerFunctions.ts public/model/documentLayerFunctions.icon.test.ts public/model/mapLayerType.ts public/model/icon_functions.test.ts public/components/map_icons/default_icons.ts public/components/map_icons/index.ts public/utils/getIntialConfig.ts Sub-PR theme: UI components for icon picker, style panel, and custom SVG upload Relevant files: public/components/layer_config/documents_config/style/custom_icon_upload.tsx public/components/layer_config/documents_config/style/icon_config.tsx public/components/layer_config/documents_config/style/icon_picker.tsx public/components/layer_config/documents_config/style/document_layer_style.tsx

⚡ Recommended focus areas for review Debug Logging Left In console.log('[maps-icon] resolveIconSvg:', ...) and console.log('[maps-icon] loadIconImage:', ...) are left in production code. These will spam the browser console every render. Remove them before merging. // eslint-disable-next-line no-console console.log('[maps-icon] resolveIconSvg:', { iconId, fillColor, strokeColor, iconStyle, svgLength: result?.length }); Performance Concern loadCustomIcons issues one HTTP GET per icon (N+1 pattern). With many custom icons this triggers many round trips every time the picker mounts. Consider returning SVG content in the list endpoint or batching, and cache results across mounts. const loadCustomIcons = useCallback(async () => { try { const response = (await http.get('/api/maps-dashboards/icons')) as { icons: CustomIconMetadata[] }; const iconMetadataList = response.icons || []; // Fetch SVG content for each icon const iconsWithSvg = await Promise.all( iconMetadataList.map(async (meta) => { try { const detail = (await http.get(`/api/maps-dashboards/icons/${meta.id}`)) as CustomIcon; return { id: meta.id, name: meta.name, svg: detail.svg }; } catch { return { id: meta.id, name: meta.name, svg: '' }; } }) ); setCustomIcons(iconsWithSvg.filter((icon) => icon.svg)); } catch (e) { setCustomIcons([]); } }, [http]); UX Issue Deleting a custom icon uses a right-click contextmenu with window.confirm. This is undiscoverable (only hinted at in a tooltip) and inaccessible for keyboard/touch users. Provide a visible delete affordance (e.g., a button on hover or in a menu). onContextMenu={ selectedTab === 'custom' ? (e) => { e.preventDefault(); if ( window.confirm( `Delete icon "${icon.name}"? Layers using it will show a placeholder.` ) ) { deleteCustomIcon(icon.id); } } : undefined } Authorization/Quota The POST/DELETE icon endpoints have no authorization checks or per-tenant/user quotas. Any authenticated user can create unlimited 100KB SVG saved objects (potential storage abuse) and delete icons that other users' maps may reference (icons are shared and stored inline in layers, but deleting still affects the picker UX and any layer that later re-fetches). Consider adding capability checks and a max-icon-count limit. // Create a new custom icon router.post( { path: ICON_API_PATH, validate: { body: schema.object({ name: schema.string({ minLength: 1, maxLength: 100 }), svg: schema.string({ minLength: 1, maxLength: MAX_ICON_SVG_SIZE }), }), }, }, async (context, request, response): Promise<IOpenSearchDashboardsResponse<any>> => { try { const { name, svg } = request.body; // Server-side SVG validation — fast reject obvious attacks const validation = validateSvgContent(svg); if (!validation.valid) { return response.custom({ statusCode: 400, body: validation.reason || 'Invalid SVG content', }); } // Allowlist sanitization: strip all elements and attributes not in the allowlist. // This is the primary defense — even if denylist regex is bypassed, only safe // geometric SVG elements and presentation attributes survive. const sanitized = sanitizeSvgServer(svg); if (!sanitized) { return response.custom({ statusCode: 400, body: 'SVG could not be sanitized — no valid SVG element found', }); } const savedObjectsClient = context.core.savedObjects.client; const savedObject = await savedObjectsClient.create(MAP_ICON_SAVED_OBJECT_TYPE, { name, svg: sanitized, }); return response.ok({ body: { id: savedObject.id, ...savedObject.attributes, }, }); } catch (error: any) { return response.custom({ statusCode: error.statusCode || 500, body: error.message, }); } } ); Sanitizer Robustness The server-side sanitizer uses regex-based tag parsing rather than an XML parser. Regex parsing of XML/HTML is inherently fragile against malformed input (e.g., attributes containing '>' in quoted values, nested comments, mixed quoting). While the validator rejects many attack vectors first, using a proper SVG sanitizer library (e.g., DOMPurify with jsdom) would be more reliable defense-in-depth. Also note style is in ALLOWED_ATTRS but the client sanitizer disallows it — inconsistency. const ALLOWED_ELEMENTS = new Set([ 'svg', 'path', 'circle', 'rect', 'ellipse', 'line', 'polyline', 'polygon', 'g', 'defs', 'clippath', 'title', 'desc', ]); const ALLOWED_ATTRS = new Set([ 'viewbox', 'xmlns', 'width', 'height', 'fill', 'stroke', 'stroke-width', 'stroke-linecap', 'stroke-linejoin', 'stroke-miterlimit', 'opacity', 'fill-opacity', 'stroke-opacity', 'fill-rule', 'clip-rule', 'clip-path', 'd', 'cx', 'cy', 'r', 'rx', 'ry', 'x', 'y', 'x1', 'y1', 'x2', 'y2', 'points', 'transform', 'id', 'class', 'style', ]); function sanitizeSvgServer(svg: string): string | null { // Remove comments, CDATA sections, processing instructions, and DOCTYPE let cleaned = svg .replace(/<!--[\s\S]*?-->/g, '') .replace(/<!\[CDATA\[[\s\S]*?\]\]>/g, '') .replace(/<\?[\s\S]*?\?>/g, '') .replace(/<!DOCTYPE[\s\S]*?>/gi, ''); // Remove any style attribute content that could contain expressions cleaned = cleaned.replace(/style\s*=\s*"[^"]*expression[^"]*"/gi, ''); cleaned = cleaned.replace(/style\s*=\s*'[^']*expression[^']*'/gi, ''); // Strip disallowed elements (replace them and their content with nothing) // Match opening tags of non-allowed elements and remove them cleaned = cleaned.replace(/<(\/?)([\w:-]+)([^>]*)>/g, (match, slash, tagName, attrs) => { const normalizedTag = tagName.toLowerCase().replace(/.*:/, ''); // strip namespace prefix if (!ALLOWED_ELEMENTS.has(normalizedTag)) { return ''; } // For allowed elements, filter attributes if (slash === '/') { return `</${normalizedTag}>`; } const sanitizedAttrs = (attrs.match(/[\w:-]+\s*=\s*(?:"[^"]*"|'[^']*')/g) || []) .filter((attr: string) => { const attrName = attr.split(/\s*=/)[0].toLowerCase().replace(/.*:/, ''); return ALLOWED_ATTRS.has(attrName); }) .map((attr: string) => { // Additional check: reject attribute values containing javascript:, data:, etc. const value = attr.replace(/^[^=]+=\s*/, '').replace(/^["']|["']$/g, ''); if (/javascript:|data:|expression\(|url\(/i.test(value)) { return ''; } return attr; }) .filter(Boolean) .join(' '); const selfClosing = match.endsWith('/>') ? '/' : ''; return `<${normalizedTag}${sanitizedAttrs ? ' ' + sanitizedAttrs : ''}${selfClosing}>`; }); // Verify the result still contains an SVG root element if (!cleaned.includes('<svg')) { return null; } return cleaned.trim(); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1bf03ed

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Remove style attribute from server allowlist Including style in the server-side allowlist is inconsistent with the client-side sanitizer (which excludes it) and undermines the CSS defenses — style attributes can contain url(...), expression(...), or other CSS payloads. Although later value-checking strips some of these, an allowlist approach is safer if style is removed entirely. Remove style from ALLOWED_ATTRS to match the client and eliminate the attack surface. server/routes/icon_router.ts [85-91] const ALLOWED_ATTRS = new Set([ 'viewbox', 'xmlns', 'width', 'height', 'fill', 'stroke', 'stroke-width', 'stroke-linecap', 'stroke-linejoin', 'stroke-miterlimit', 'opacity', 'fill-opacity', 'stroke-opacity', 'fill-rule', 'clip-rule', 'clip-path', 'd', 'cx', 'cy', 'r', 'rx', 'ry', 'x', 'y', 'x1', 'y1', 'x2', 'y2', - 'points', 'transform', 'id', 'class', 'style', + 'points', 'transform', 'id', 'class', ]); Suggestion importance[1-10]: 7 __ Why: Legitimate security concern: style attribute can carry CSS payloads (url(), expression()) and its inclusion is inconsistent with the client-side allowlist. Removing it tightens the allowlist meaningfully.	Medium
	Strip disallowed elements with their content Silently stripping disallowed elements can leave orphaned text/CDATA content between the removed opening and closing tags in the output. Consider removing entire disallowed element blocks (open tag through matching close tag, including inner content) rather than only the tags themselves, or fail the request entirely if disallowed elements are present, to avoid leaking arbitrary text into the sanitized SVG. server/routes/icon_router.ts [107-111] +// Remove entire disallowed element blocks including their content before per-tag filtering +const disallowedBlockRe = /<([\w:-]+)\b[^>]*>[\s\S]*?<\/\1\s*>/g; +cleaned = cleaned.replace(disallowedBlockRe, (block, tagName) => { + const normalized = tagName.toLowerCase().replace(/.*:/, ''); + return ALLOWED_ELEMENTS.has(normalized) ? block : ''; +}); cleaned = cleaned.replace(/<(\/?)([\w:-]+)([^>]*)>/g, (match, slash, tagName, attrs) => { - const normalizedTag = tagName.toLowerCase().replace(/.*:/, ''); // strip namespace prefix + const normalizedTag = tagName.toLowerCase().replace(/.*:/, ''); if (!ALLOWED_ELEMENTS.has(normalizedTag)) { return ''; } Suggestion importance[1-10]: 6 __ Why: Valid defense-in-depth point: the current sanitizer strips disallowed tags but leaves their text content behind, which could leak arbitrary text. However, the primary XSS risk is mitigated since content is rendered via <img> data URLs.	Low
General	Remove debug console.log statements Debug console.log statements were left in production code paths (resolveIconSvg and loadIconImage). These will emit noisy logs on every render/update for map layers. Remove them or guard behind a debug flag before merging. public/model/documentLayerFunctions.ts [46-49] if (builtIn) { - const result = applyIconColors(builtIn.svg, fillColor, strokeColor, iconStyle); - // eslint-disable-next-line no-console - console.log('[maps-icon] resolveIconSvg:', { iconId, fillColor, strokeColor, iconStyle, svgLength: result?.length }); - return result; + return applyIconColors(builtIn.svg, fillColor, strokeColor, iconStyle); } Suggestion importance[1-10]: 6 __ Why: Correctly identifies leftover debug console.log statements in production code paths that will produce noise on every render. Good cleanup suggestion before merge.	Low
	Strip only trailing extension from filename file.name.replace('.svg', '') only strips the first occurrence anywhere in the filename, not necessarily the trailing extension. Use a regex anchored to the end of the string to strip only the .svg suffix (case-insensitive) so filenames like my.svg.icon.svg are handled correctly. public/components/layer_config/documents_config/style/custom_icon_upload.tsx [258] // Auto-fill name from filename if not set if (!iconName) { - const nameFromFile = file.name.replace('.svg', '').replace(/[-_]/g, ' '); + const nameFromFile = file.name.replace(/\.svg$/i, '').replace(/[-_]/g, ' '); setIconName(nameFromFile); } Suggestion importance[1-10]: 4 __ Why: Minor correctness improvement for edge-case filenames containing .svg earlier in the name. Low impact since it only affects the auto-fill name suggestion which the user can edit.	Low

---

Previous suggestions

Suggestions up to commit bda145c

Category	Suggestion	Impact
Possible issue	Fix namespaceType mismatch with tests The saved object test asserts namespaceType is 'agnostic', but the implementation declares 'single'. This mismatch will fail the test, and using 'single' also contradicts the intent (icons available across all maps/workspaces per the upload description). Change to 'agnostic' to align with the test and the described behavior. server/saved_objects/map_icon_saved_object.ts [13] export const mapIconSavedObjectsType: SavedObjectsType = { name: MAP_ICON_SAVED_OBJECT_TYPE, hidden: false, - namespaceType: 'single', + namespaceType: 'agnostic', Suggestion importance[1-10]: 8 __ Why: Correctly identifies a mismatch between the test expectation (agnostic) and the implementation (single), which will cause test failures and also contradicts the described cross-map availability of icons.	Medium
	Async icon load breaks layer ordering ensureIconAndAddLayer is async but addNewLayer continues to synchronously add line/polygon layers below. If geoFieldType === 'geo_shape', the line/polygon layers will be added before the icon symbol layer resolves, resulting in incorrect layer ordering (icons drawn below lines/polygons). Consider chaining the shape-layer additions after the icon load, or reserving/placing the symbol layer using beforeLayerId to preserve z-order. public/model/documentLayerFunctions.ts [376-391] if (isIconMode(layerConfig)) { - // Load icon and add symbol layer + // Load icon and add symbol layer (async — will be placed above already-added layers) ensureIconAndAddLayer(maplibreInstance, layerConfig); } else { addCircleLayer(maplibreInstance, { Suggestion importance[1-10]: 7 __ Why: Valid concern about async icon loading potentially causing z-order issues with subsequently added line/polygon layers, though the improved_code doesn't provide a concrete fix.	Medium
Security	Add authorization to icon endpoints The routes have no authorization checks and no per-user/tenant scoping — any authenticated user can create, list, read, and delete every custom icon globally (namespaceType is also 'single' but the routes use the default client). Deleting an icon that other users' layers reference will silently break their maps. Add capability/permission checks (e.g., verify map.save capability on POST/DELETE) and consider whether icons should be workspace-scoped. server/routes/icon_router.ts [144-200] +// Example: gate mutations on capabilities +// const { capabilities } = context.core; +// if (!capabilities.map?.save) return response.forbidden(); - Suggestion importance[1-10]: 7 __ Why: Legitimate security concern about lack of authorization checks on icon CRUD endpoints, allowing any authenticated user to modify/delete shared icons.	Medium
General	Remove debug console.log calls Remove the debug console.log statements before merging. They will pollute the browser console on every render/update of a map layer in production. There are similar console.log calls inside loadIconImage that should also be removed. public/model/documentLayerFunctions.ts [44-49] const builtIn = getBuiltInIconById(iconId); if (builtIn) { - const result = applyIconColors(builtIn.svg, fillColor, strokeColor, iconStyle); - // eslint-disable-next-line no-console - console.log('[maps-icon] resolveIconSvg:', { iconId, fillColor, strokeColor, iconStyle, svgLength: result?.length }); - return result; + return applyIconColors(builtIn.svg, fillColor, strokeColor, iconStyle); } Suggestion importance[1-10]: 6 __ Why: Correctly identifies debug console.log statements that should be removed before merging to production; a valid cleanup suggestion.	Low

Suggestions up to commit 76def1f

Category	Suggestion	Impact
Possible issue	Fix undefined variable in error handler The iconId variable is not defined in this scope, which will cause a ReferenceError at runtime whenever an icon fails to load. Use imageId (which is in scope) or pass the icon id as a parameter to include it in the error message. public/model/documentLayerFunctions.ts [199-201] img.onerror = (e) => { - reject(new Error(`Failed to load icon image: ${iconId}`)); + reject(new Error(`Failed to load icon image: ${imageId}`)); }; Suggestion importance[1-10]: 9 __ Why: Correctly identifies a ReferenceError bug: iconId is not defined in the img.onerror handler scope; imageId is what's in scope. This would throw at runtime on icon load failure.	High
	Fix broken test imports and references The test imports FILLED_ICONS, OUTLINE_ICONS, and getIconsBySet, and references icon.set and 'pin-filled', but none of these are exported from map_icons/index.ts or defined in default_icons.ts (icons use ids like 'pin', not 'pin-filled', and have no set field). The test file will fail to compile/run. Align the tests with the actual exports and icon ids. public/model/icon_functions.test.ts [6-14] import { ALL_BUILT_IN_ICONS, - FILLED_ICONS, - OUTLINE_ICONS, getBuiltInIconById, getIconsByCategory, - getIconsBySet, DEFAULT_ICON_ID, } from '../components/map_icons'; Suggestion importance[1-10]: 9 __ Why: Correctly identifies that the test imports FILLED_ICONS, OUTLINE_ICONS, getIconsBySet which are not exported, and uses 'pin-filled' id and icon.set which don't exist. Test would fail to compile.	High
Security	Sanitize SVG instead of denylist filtering The route path /api/maps-dashboards/icons is registered without an explicit options.tags for authorization or CSRF handling and, more importantly, accepts arbitrary SVG content that ends up rendered via dangerouslySetInnerHTML on the client. Even with the current denylist, consider using a proper SVG sanitizer (e.g. DOMPurify with SVG profile) on the server before persisting, since regex-based checks can be bypassed (e.g. encoded attributes, CDATA, namespaced handlers). server/routes/icon_router.ts [58-67] +router.post( + { + path: ICON_API_PATH, + validate: { + body: schema.object({ + name: schema.string({ minLength: 1, maxLength: 100 }), + svg: schema.string({ minLength: 1, maxLength: MAX_ICON_SVG_SIZE }), + }), + }, + }, - Suggestion importance[1-10]: 7 __ Why: Valid security concern: regex/denylist-based SVG validation can be bypassed. Using a proper sanitizer like DOMPurify would be more robust given the content is rendered via dangerouslySetInnerHTML.	Medium
General	Check file size before reading content The size check is performed on the decoded string length after the SVG marker check, meaning a very large non-SVG file will still be read fully into memory and processed. Also, content.length measures UTF-16 code units, not bytes; a small file with multi-byte characters may pass while a large ASCII file is measured accurately — inconsistent with the server's byte-based limit. Check file.size from the File object before reading, and compare byte length rather than string length. public/components/layer_config/documents_config/style/custom_icon_upload.tsx [36-52] +if (file.size > MAX_SVG_SIZE) { + setValidationError(i18n.translate('maps.icons.upload.tooLarge', { + defaultMessage: 'SVG file must be less than 100KB', + })); + setSvgContent(null); + return; +} +// ...then read and validate content if (!content.includes('<svg')) { return { valid: false, error: i18n.translate('maps.icons.upload.invalidSvg', { defaultMessage: 'File does not appear to be a valid SVG', }), }; } -if (content.length > MAX_SVG_SIZE) { - Suggestion importance[1-10]: 5 __ Why: Reasonable improvement to check file.size before reading, avoiding loading large files into memory and aligning with server-side byte-based limits.	Low

[github-actions]

Persistent review updated to latest commit bda145c

[github-actions]

Persistent review updated to latest commit 1bf03ed

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 2357698.

⛔ Hard block: Issues at High severity or above will block this PR from merging.

Path	Line	Severity	Description
package.json	33	high	Dependency change: 'nanoid@^5.1.16' added to yarn resolutions, forcing all transitive consumers to this specific version. While nanoid is a well-known package and this appears to be a version-pin fix, all dependency/registry changes must be flagged for maintainer verification of artifact authenticity.
server/routes/icon_router.ts	27	medium	Server-side SVG validation only checks for the presence of the substring ' tags, javascript: hrefs, onload/onerror event handlers, or external resource references (e.g., or ). Malicious SVG content passes validation and is persisted in OpenSearch saved objects. Current rendering via data URLs is safe, but the stored payload is available for exploitation if any future code path renders it inline.
public/components/layer_config/documents_config/style/custom_icon_upload.tsx	37	low	Client-side SVG validation mirrors the same weak check (presence of '

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 1 | Medium: 1 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[heemin32]

For SVG validation is weak, rendering via data URL is the security boundary. The stored SVG is inert because every rendering path uses data URLs. Even if someone stores <script>alert(1)</script> inside an SVG, it never executes.