security(CASA-41): Fix XSS vulnerability in innerHTML usage #1028

orhanrauf · 2025-10-28T13:48:12Z

Overview

Fixes ENG-181 - Replaced all 12 instances of unsafe .innerHTML assignments with secure DOM manipulation to prevent XSS attacks.

Security Impact

✅ Eliminates XSS vulnerability: Even if an attacker injects malicious HTML into short_name fields, it will be rendered as plain text rather than executed as code
✅ Defense-in-depth: Provides additional security layer beyond existing CSP headers
✅ Zero innerHTML usage: Confirmed via grep - no unsafe innerHTML assignments remain

Changes Summary

Before (Vulnerable)

e.currentTarget.parentElement!.innerHTML = \`<span>\${shortName.substring(0, 2).toUpperCase()}</span>\`;

After (Safe)

const span = document.createElement('span');
span.className = '...';
span.textContent = shortName.substring(0, 2).toUpperCase(); // Safe - no HTML interpretation
parent.appendChild(span);

Files Modified (12 instances)

Button Components:

frontend/src/components/dashboard/SourceButton.tsx
frontend/src/components/dashboard/AuthProviderButton.tsx
frontend/src/components/dashboard/SmallSourceButton.tsx

View Components:

frontend/src/components/shared/views/EditAuthProviderView.tsx
frontend/src/components/shared/views/CreateCollectionView.tsx
frontend/src/components/shared/views/ConnectionErrorView.tsx
frontend/src/components/auth-providers/AuthProviderDetailView.tsx (2 instances)

Other Components:

frontend/src/pages/SemanticMcp.tsx (2 instances)
frontend/src/components/collection/EditSourceConnectionDialog.tsx
frontend/src/components/sync/nodes/TransformerNode.tsx

Testing Checklist

Force icon failures (block icon CDNs in DevTools)
Verify fallback initials display correctly
Check no console errors appear
Test edge cases (empty/single char/special char shortNames)

Compliance

Addresses CASA TAC Security requirement 41: proper output encoding for all contexts.

Summary by cubic

Fixes XSS risk by replacing all unsafe innerHTML fallbacks with safe DOM manipulation for icon initials. Addresses ENG-181 and meets CASA TAC Security requirement 41.

Bug Fixes
- Replaced innerHTML with createElement + textContent in 12 places across buttons, views, dialogs, and nodes.
- Fallback initials now render as plain text; theme classes applied via classList.
- Verified no remaining innerHTML usage.

Replace all 12 instances of unsafe innerHTML assignments with secure DOM manipulation using createElement() and textContent to prevent XSS attacks. The vulnerable pattern used template literals with user-controlled data (shortName fields) without sanitization. Even though these fields currently come from internal sources, this provides defense-in-depth protection. Changes: - Replace innerHTML with createElement() + textContent in icon fallbacks - Affected components: buttons, views, dialogs, and transformer nodes - All fallback initials now safely rendered as plain text Resolves: ENG-181

cubic-dev-ai

4 issues found across 10 files

Prompt for AI agents (all 4 issues)


Understand the root cause of the following 4 issues and fix them.


<file name="frontend/src/components/collection/EditSourceConnectionDialog.tsx">

<violation number="1" location="frontend/src/components/collection/EditSourceConnectionDialog.tsx:115">
Use replaceChildren (or clear the parent) before appending the fallback wrapper so multiple error events don&#39;t stack duplicate fallback nodes.</violation>
</file>

<file name="frontend/src/components/sync/nodes/TransformerNode.tsx">

<violation number="1" location="frontend/src/components/sync/nodes/TransformerNode.tsx:43">
Each onError run appends another fallback span, so repeated icon load failures (e.g., after theme toggles) stack multiple initials badges instead of keeping just one. Please clear or reuse the existing fallback node before appending a new one.</violation>
</file>

<file name="frontend/src/components/auth-providers/AuthProviderDetailView.tsx">

<violation number="1" location="frontend/src/components/auth-providers/AuthProviderDetailView.tsx:575">
This fallback also uses appendChild without clearing the parent, so repeated onError executions stack multiple fallback wrappers and distort the button layout. Please replace the existing child nodes before inserting the fallback.</violation>
</file>

<file name="frontend/src/components/shared/views/EditAuthProviderView.tsx">

<violation number="1" location="frontend/src/components/shared/views/EditAuthProviderView.tsx:269">
Appending the fallback wrapper without clearing the parent means each image error adds another fallback element, diverging from the prior behavior and producing duplicate initials when the handler fires again (e.g., on theme/icon changes). Replace the parent’s children instead of appending so only one fallback remains.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-10-28T13:59:23Z

frontend/src/components/collection/EditSourceConnectionDialog.tsx

+                                            span.textContent = sourceConnection.short_name.substring(0, 2).toUpperCase();
+
+                                            wrapper.appendChild(span);
+                                            parent.appendChild(wrapper);


Use replaceChildren (or clear the parent) before appending the fallback wrapper so multiple error events don't stack duplicate fallback nodes.

Prompt for AI agents

Address the following comment on frontend/src/components/collection/EditSourceConnectionDialog.tsx at line 115: <comment>Use replaceChildren (or clear the parent) before appending the fallback wrapper so multiple error events don't stack duplicate fallback nodes.</comment> <file context> @@ -101,13 +101,18 @@ export const EditSourceConnectionDialog: React.FC<EditSourceConnectionDialogProp + span.textContent = sourceConnection.short_name.substring(0, 2).toUpperCase(); + + wrapper.appendChild(span); + parent.appendChild(wrapper); }} /> </file context>

cubic-dev-ai · 2025-10-28T13:59:23Z

frontend/src/components/sync/nodes/TransformerNode.tsx

+              const span = document.createElement('span');
+              span.className = 'text-foreground font-medium text-xs';
+              span.textContent = initials;
+              parent.appendChild(span);


Each onError run appends another fallback span, so repeated icon load failures (e.g., after theme toggles) stack multiple initials badges instead of keeping just one. Please clear or reuse the existing fallback node before appending a new one.

Prompt for AI agents

Address the following comment on frontend/src/components/sync/nodes/TransformerNode.tsx at line 43: <comment>Each onError run appends another fallback span, so repeated icon load failures (e.g., after theme toggles) stack multiple initials badges instead of keeping just one. Please clear or reuse the existing fallback node before appending a new one.</comment> <file context> @@ -33,8 +33,14 @@ export const TransformerNode = memo(({ data, selected, ...props }: NodeProps) => + const span = document.createElement('span'); + span.className = 'text-foreground font-medium text-xs'; + span.textContent = initials; + parent.appendChild(span); }} /> </file context>

cubic-dev-ai · 2025-10-28T13:59:23Z

frontend/src/components/auth-providers/AuthProviderDetailView.tsx

+                                                    span.textContent = 'AW';
+
+                                                    wrapper.appendChild(span);
+                                                    parent.appendChild(wrapper);


This fallback also uses appendChild without clearing the parent, so repeated onError executions stack multiple fallback wrappers and distort the button layout. Please replace the existing child nodes before inserting the fallback.

Prompt for AI agents

Address the following comment on frontend/src/components/auth-providers/AuthProviderDetailView.tsx at line 575: <comment>This fallback also uses appendChild without clearing the parent, so repeated onError executions stack multiple fallback wrappers and distort the button layout. Please replace the existing child nodes before inserting the fallback.</comment> <file context> @@ -561,13 +561,18 @@ export const AuthProviderDetailView: React.FC<AuthProviderDetailViewProps> = ({ + span.textContent = 'AW'; + + wrapper.appendChild(span); + parent.appendChild(wrapper); }} /> </file context>

Suggested change

parent.appendChild(wrapper);

parent.replaceChildren(wrapper);

cubic-dev-ai · 2025-10-28T13:59:24Z

frontend/src/components/shared/views/EditAuthProviderView.tsx

+                                    span.textContent = authProviderShortName.substring(0, 2).toUpperCase();
+
+                                    wrapper.appendChild(span);
+                                    parent.appendChild(wrapper);


Appending the fallback wrapper without clearing the parent means each image error adds another fallback element, diverging from the prior behavior and producing duplicate initials when the handler fires again (e.g., on theme/icon changes). Replace the parent’s children instead of appending so only one fallback remains.

Prompt for AI agents

Address the following comment on frontend/src/components/shared/views/EditAuthProviderView.tsx at line 269: <comment>Appending the fallback wrapper without clearing the parent means each image error adds another fallback element, diverging from the prior behavior and producing duplicate initials when the handler fires again (e.g., on theme/icon changes). Replace the parent’s children instead of appending so only one fallback remains.</comment> <file context> @@ -253,16 +253,21 @@ export const EditAuthProviderView: React.FC<EditAuthProviderViewProps> = ({ + span.textContent = authProviderShortName.substring(0, 2).toUpperCase(); + + wrapper.appendChild(span); + parent.appendChild(wrapper); + }} /> </file context>

Suggested change

parent.appendChild(wrapper);

parent.replaceChildren(wrapper);

@cubic-dev-ai i'm not sure if i understand, can you explain a bit more?

@orhanrauf Of course. My concern was about what happens if the onError handler runs more than once.

The previous code used innerHTML = '...', which completely replaced the parent's content with the fallback initials. Your new code uses parent.appendChild(wrapper), which adds the fallback initials without removing what was already there.

If an event (like a theme change) causes the component to re-render and the image fails to load again, onError will fire a second time. Your current implementation would append a second set of initials, leading to duplicates.

I suggested using parent.replaceChildren(wrapper) because it clears any existing elements from the parent before adding the new wrapper. This mimics the original behavior and ensures only one set of initials is ever displayed.

orhanrauf temporarily deployed to dev October 28, 2025 13:48 — with GitHub Actions Inactive

orhanrauf temporarily deployed to dev October 28, 2025 13:49 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 28, 2025

View reviewed changes

orhanrauf force-pushed the main branch from d341d41 to 11689d2 Compare October 28, 2025 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

security(CASA-41): Fix XSS vulnerability in innerHTML usage #1028

security(CASA-41): Fix XSS vulnerability in innerHTML usage #1028

Uh oh!

orhanrauf commented Oct 28, 2025 •

edited by cubic-dev-ai bot

Loading

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

Uh oh!

orhanrauf Oct 28, 2025

Uh oh!

cubic-dev-ai bot Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	parent.appendChild(wrapper);
	parent.replaceChildren(wrapper);

security(CASA-41): Fix XSS vulnerability in innerHTML usage #1028

Are you sure you want to change the base?

security(CASA-41): Fix XSS vulnerability in innerHTML usage #1028

Uh oh!

Conversation

orhanrauf commented Oct 28, 2025 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Security Impact

Changes Summary

Before (Vulnerable)

After (Safe)

Files Modified (12 instances)

Testing Checklist

Compliance

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orhanrauf Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

orhanrauf commented Oct 28, 2025 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading

cubic-dev-ai bot Oct 28, 2025 •

edited

Loading