[REFACTOR][UI]: Consolidate innerHTML patterns with auto-escaping helpers

[crivetimihai]

🛠️ Refactor: Consolidate innerHTML Patterns with Auto-Escaping Helpers

Overview

Refactor frontend rendering in admin.js to use standardized, safe-by-default utilities. This consolidates ~260 innerHTML usages into consistent patterns with auto-escaping helpers. This is a code quality and defense-in-depth improvement—the current implementation is secure due to robust backend validation.

Background

Current State: Secure ✅

A comprehensive security audit confirmed the application has no XSS vulnerabilities:

• Backend validation rejects HTML special characters in entity names
• sanitize_display_text() HTML-escapes all descriptions before storage
• Jinja2 autoescape=True protects template rendering
• Import service uses same validators—no bypass path

Why Refactor?

The current architecture relies on trusting the backend. While this works, security best practice is defense-in-depth: each layer should protect itself regardless of what other layers do.

Current Pattern	Risk if Backend Has Bug	After Refactor
innerHTML = ${name}	Potential XSS	Auto-escaped via safeHtml
innerHTML = backendHtml	Potential XSS	Sanitized via DOMPurify
Backend validation fails	Frontend vulnerable	Frontend still protected

This refactor ensures the frontend remains secure even if:

• A future backend change introduces a validation gap
• Legacy data exists from before validation was added
• A new entity type forgets to add validators
• Backend is compromised

---

Implementation

1. Create safeHtml Tagged Template Literal

Auto-escapes all interpolated values by default:

/**
 * Tagged template literal that auto-escapes all interpolated values.
 * Use for any innerHTML assignment with dynamic data.
 * 
 * @example
 * row.innerHTML = safeHtml`<td>${tool.name}</td><td>${tool.description}</td>`;
 */
function safeHtml(strings, ...values) {
    return strings.reduce((result, str, i) => {
        const value = values[i - 1];
        const escaped = value == null ? '' : escapeHtml(String(value));
        return result + escaped + str;
    });
}

/**
 * For cases where a value is known-safe (e.g., hardcoded SVG icons),
 * wrap with rawHtml() to bypass escaping.
 */
function rawHtml(value) {
    return { __raw: true, value };
}

// Enhanced safeHtml that respects rawHtml markers
function safeHtml(strings, ...values) {
    return strings.reduce((result, str, i) => {
        const value = values[i - 1];
        if (value == null) return result + '' + str;
        if (value.__raw) return result + value.value + str;
        return result + escapeHtml(String(value)) + str;
    });
}

2. Create setSafeInnerHTML for Backend Partials

Sanitize all HTML received from backend endpoints:

/**
 * Safely set innerHTML from backend HTML partials.
 * Applies DOMPurify sanitization for defense-in-depth.
 * 
 * @example
 * const html = await response.text();
 * setSafeInnerHTML(container, html);
 */
function setSafeInnerHTML(element, html) {
    if (typeof DOMPurify !== 'undefined') {
        element.innerHTML = DOMPurify.sanitize(html, {
            USE_PROFILES: { html: true },
            ADD_ATTR: ['hx-get', 'hx-post', 'hx-trigger', 'hx-target', 'hx-swap', 
                       'hx-vals', 'hx-indicator', 'x-data', 'x-show', 'x-on:click',
                       '@click', ':class'],  // Allow HTMX and Alpine attributes
        });
    } else {
        // Fallback if DOMPurify not loaded
        console.warn('DOMPurify not available, setting innerHTML directly');
        element.innerHTML = html;
    }
}

3. Migration Patterns

Pattern A: API Data in Template Literals

// Before
const rows = tools.map(tool => `
    <tr>
        <td>${tool.name}</td>
        <td>${tool.description}</td>
    </tr>
`).join('\);
toolBody.innerHTML = rows;

// After
const rows = tools.map(tool => safeHtml`
    <tr>
        <td>${tool.name}</td>
        <td>${tool.description}</td>
    </tr>
`).join('\);
toolBody.innerHTML = rows;

Pattern B: Mixed Safe and Unsafe Values

// Before
statusSpan.innerHTML = `${statusText} ${statusIcon}`;

// After (statusIcon is hardcoded SVG, statusText is hardcoded string)
statusSpan.innerHTML = safeHtml`${statusText} ${rawHtml(statusIcon)}`;

Pattern C: Backend HTML Partials

// Before
fetch(`${rootPath}/admin/tools/partial`)
    .then(response => response.text())
    .then(html => {
        container.innerHTML = html;
    });

// After
fetch(`${rootPath}/admin/tools/partial`)
    .then(response => response.text())
    .then(html => {
        setSafeInnerHTML(container, html);
    });

Pattern D: Simple Text Display

// Before
cell.innerHTML = tool.name;

// After (prefer textContent for plain text)
cell.textContent = tool.name;

Pattern E: Clearing Content

// Before
container.innerHTML = '\;

// After (more explicit)
container.textContent = '\;
// Or for removing child nodes:
container.replaceChildren();

---

📋 Tasks

Phase 1: Infrastructure

• Add safeHtml() tagged template function to admin.js
• Add rawHtml() marker function for known-safe values
• Add setSafeInnerHTML() wrapper for backend partials
• Configure DOMPurify to allow HTMX/Alpine attributes

Phase 2: Migrate Template Literals with API Data (~40 instances)

• Tool table rendering (lines ~11050-11115)
• Gateway rendering
• Resource rendering
• Prompt rendering
• A2A agent rendering
• Search result displays
• Export bundle displays

Phase 3: Migrate Backend HTML Partials (~75 instances)

• MCP registry partial loading
• Tools partial loading
• Resources partial loading
• Plugins partial loading
• Team management modals
• All other container.innerHTML = html from fetch

Phase 4: Cleanup Simple Cases (~45 instances)

• Replace el.innerHTML = text with el.textContent = text where appropriate
• Replace el.innerHTML = '' with el.textContent = '' or el.replaceChildren()

Phase 5: Tooling & Enforcement

• Add eslint-plugin-no-unsanitized to devDependencies
• Configure ESLint rules:

  {
    "plugins": ["no-unsanitized"],
    "rules": {
      "no-unsanitized/property": ["error", {
        "escape": {
          "methods": ["safeHtml", "escapeHtml", "DOMPurify.sanitize"]
        }
      }]
    }
  }

• Add to CI pipeline
• Add pre-commit hook

Phase 6: Documentation

• Document safeHtml, rawHtml, setSafeInnerHTML usage in code comments
• Add frontend security patterns section to developer guide
• Document when to use each pattern

---

📊 Audit Summary

For reference, the security audit that informed this refactor:

Category	Count	Current Risk	After Refactor
Backend HTML partials	~75	✅ Safe (Jinja2 autoescape)	✅ + DOMPurify
Template literals with API data	~40	✅ Safe (backend validation)	✅ + auto-escape
Template literals with hardcoded values	~15	✅ Safe (no user data)	✅ + explicit markers
Static HTML strings	~100	✅ Safe (no interpolation)	✅ No change needed
Empty string assignments	~30	✅ Safe	✅ + cleaner syntax

Current protection layers:

1. ✅ Pydantic schema validators (validate_name, validate_tool_name)
2. ✅ sanitize_display_text() HTML-escapes descriptions
3. ✅ Jinja2 autoescape=True
4. ✅ escapeHtml() used selectively in JS
5. ✅ DOMPurify for markdown rendering

After refactor (additional layers):
6. ✅ safeHtml auto-escapes all template interpolations
7. ✅ setSafeInnerHTML sanitizes all backend HTML
8. ✅ ESLint blocks unsafe patterns in CI

---

✅ Success Criteria

• All 260 innerHTML usages reviewed and migrated appropriately
• safeHtml used for all template literals with dynamic data
• setSafeInnerHTML used for all backend HTML partials
• textContent used for simple text display
• ESLint rule catches any new unsafe innerHTML usage
• No functional regressions (all Playwright tests pass)
• DOMPurify config allows HTMX/Alpine attributes

---

🏁 Definition of Done

• safeHtml(), rawHtml(), setSafeInnerHTML() utilities implemented
• All innerHTML usages migrated per patterns above
• ESLint no-unsanitized configured and passing
• Developer documentation updated
• All existing tests pass
• Code passes make verify
• PR reviewed and approved

---

🔗 Related Issues

• [EPIC][SECURITY]: Third-party script isolation and sandboxing #2559 - Third-party script isolation (CSP improvements)
• [TESTING][SECURITY]: Input validation manual test plan (path traversal, injection, XSS, ReDoS) #2398 - Input validation test plan
• [TESTING][SECURITY]: Security headers manual test plan (CSP, HSTS, CORS, clickjacking) #2396 - Security headers test plan
• [EPIC][SECURITY][UI]: Click-to-Reveal UI Components (UX Improvements) #2564 - Click-to-reveal UI components