Skip to content

Commit c54cf1d

Browse files
committed
Fix PR review: security, lint errors, and schema alignment
- Prevent XSS via postMessage: validate e.source, strip scripts before innerHTML, only execute new scripts via data-exec tracking - Remove allow-same-origin from widget and preview iframe sandboxes - Fix all lint errors: refactor useEffect setState to adjust-state-during-render pattern, replace cumulative mutation with reduce() pre-computation - Unify template schema: add component_type/component_data to backend UITemplate, remove unsafe casts in frontend - Add console.warn fallback to submitChatPrompt when textarea not found - Remove apply_template runtime=None default to match other tool signatures - Remove trivial assembleDocument() pass-through
1 parent 225b461 commit c54cf1d

4 files changed

Lines changed: 63 additions & 51 deletions

File tree

apps/agent/src/templates.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
from langchain.tools import ToolRuntime, tool
22
from langchain.messages import ToolMessage
33
from langgraph.types import Command
4-
from typing import TypedDict
4+
from typing import Any, Optional, TypedDict
55
import uuid
66
from datetime import datetime
77

88

9-
class UITemplate(TypedDict):
9+
class UITemplate(TypedDict, total=False):
1010
id: str
1111
name: str
1212
description: str
1313
html: str
1414
data_description: str
1515
created_at: str
1616
version: int
17+
component_type: Optional[str]
18+
component_data: Optional[dict[str, Any]]
1719

1820

1921
@tool
@@ -77,7 +79,7 @@ def list_templates(runtime: ToolRuntime):
7779

7880

7981
@tool
80-
def apply_template(name: str = "", template_id: str = "", runtime: ToolRuntime = None):
82+
def apply_template(name: str = "", template_id: str = "", runtime: ToolRuntime):
8183
"""
8284
Retrieve a saved template's HTML so you can adapt it with new data.
8385
After calling this, generate a NEW widget in the same style and render via widgetRenderer.

apps/app/src/components/generative-ui/widget-renderer.tsx

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -349,20 +349,32 @@ document.addEventListener('click', function(e) {
349349
350350
// Listen for streaming content updates from parent
351351
window.addEventListener('message', function(e) {
352+
if (e.source !== window.parent) return;
352353
if (e.data && e.data.type === 'update-content') {
353354
var content = document.getElementById('content');
354355
if (content) {
355-
content.innerHTML = e.data.html;
356-
// Re-run any inline scripts (new ones added by streaming)
357-
var scripts = content.querySelectorAll('script');
358-
scripts.forEach(function(oldScript) {
356+
// Strip script tags from HTML before inserting — scripts are handled separately below
357+
var tmp = document.createElement('div');
358+
tmp.innerHTML = e.data.html;
359+
var incomingScripts = [];
360+
tmp.querySelectorAll('script').forEach(function(s) {
361+
incomingScripts.push({ src: s.src, text: s.textContent });
362+
s.remove();
363+
});
364+
content.innerHTML = tmp.innerHTML;
365+
366+
// Execute only new scripts (not previously executed)
367+
incomingScripts.forEach(function(scriptInfo) {
368+
var key = scriptInfo.src || scriptInfo.text;
369+
if (content.getAttribute('data-exec-' + btoa(key).slice(0, 16))) return;
370+
content.setAttribute('data-exec-' + btoa(key).slice(0, 16), '1');
359371
var newScript = document.createElement('script');
360-
if (oldScript.src) {
361-
newScript.src = oldScript.src;
372+
if (scriptInfo.src) {
373+
newScript.src = scriptInfo.src;
362374
} else {
363-
newScript.textContent = oldScript.textContent;
375+
newScript.textContent = scriptInfo.text;
364376
}
365-
oldScript.parentNode.replaceChild(newScript, oldScript);
377+
content.appendChild(newScript);
366378
});
367379
reportHeight();
368380
}
@@ -384,11 +396,6 @@ setTimeout(function() { clearInterval(_resizeInterval); }, 15000);
384396
`;
385397

386398
// ─── Document Assembly ───────────────────────────────────────────────
387-
/** Full document with content — used for final/complete renders */
388-
function assembleDocument(html: string): string {
389-
return assembleShell(html);
390-
}
391-
392399
/** Empty shell or shell with initial content — iframe loads once, content streamed via postMessage */
393400
function assembleShell(initialHtml: string = ""): string {
394401
return `<!DOCTYPE html>
@@ -440,7 +447,6 @@ function useLoadingPhrase(active: boolean) {
440447
const [index, setIndex] = useState(0);
441448
useEffect(() => {
442449
if (!active) return;
443-
setIndex(0);
444450
const interval = setInterval(() => {
445451
setIndex((i) => (i + 1) % LOADING_PHRASES.length);
446452
}, 1800);
@@ -459,11 +465,12 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
459465
const shellReadyRef = useRef(false);
460466
// Track the last html sent to the iframe to avoid redundant updates
461467
const committedHtmlRef = useRef("");
462-
// Whether streaming has started (html is arriving but may not be complete)
463-
const [streaming, setStreaming] = useState(false);
464468
// Tracks whether html content has settled (stopped changing)
465469
const [htmlSettled, setHtmlSettled] = useState(false);
470+
const [prevHtml, setPrevHtml] = useState(html);
466471
const settledTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
472+
const fadeTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
473+
const [fadingOut, setFadingOut] = useState(false);
467474

468475
const handleMessage = useCallback((e: MessageEvent) => {
469476
// Only handle messages from our own iframe
@@ -482,6 +489,13 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
482489
return () => window.removeEventListener("message", handleMessage);
483490
}, [handleMessage]);
484491

492+
// Reset settled/fade state when html changes (adjust state during render)
493+
if (html !== prevHtml) {
494+
setPrevHtml(html);
495+
setHtmlSettled(false);
496+
setFadingOut(false);
497+
}
498+
485499
// Initialize the iframe shell once when html first appears.
486500
// After that, stream content updates via postMessage — no iframe reload.
487501
useEffect(() => {
@@ -492,7 +506,6 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
492506
shellReadyRef.current = true;
493507
committedHtmlRef.current = html;
494508
iframeRef.current.srcdoc = assembleShell(html);
495-
setStreaming(true);
496509
return;
497510
}
498511

@@ -512,15 +525,19 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
512525
// Detect when html has stopped changing (streaming complete).
513526
// Resets a debounce timer on every html update — settles after 800ms of no changes.
514527
useEffect(() => {
515-
if (!html) {
516-
setHtmlSettled(false);
517-
return;
518-
}
519-
setHtmlSettled(false);
528+
if (!html) return;
520529
if (settledTimerRef.current) clearTimeout(settledTimerRef.current);
521-
settledTimerRef.current = setTimeout(() => setHtmlSettled(true), 800);
530+
if (fadeTimerRef.current) clearTimeout(fadeTimerRef.current);
531+
settledTimerRef.current = setTimeout(() => {
532+
setHtmlSettled(true);
533+
setFadingOut(true);
534+
fadeTimerRef.current = setTimeout(() => {
535+
setFadingOut(false);
536+
}, 600);
537+
}, 800);
522538
return () => {
523539
if (settledTimerRef.current) clearTimeout(settledTimerRef.current);
540+
if (fadeTimerRef.current) clearTimeout(fadeTimerRef.current);
524541
};
525542
}, [html]);
526543

@@ -534,25 +551,12 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
534551
return () => clearTimeout(timeout);
535552
}, [html, loaded, height]);
536553

537-
// Content is "complete" when iframe has loaded and reported a valid height
538-
const ready = loaded && height > 0;
539-
// Show the iframe (even partially) as soon as we have content streaming
540-
const showIframe = !!html && (streaming || ready);
554+
// Show the iframe as soon as we have html (shell initializes on first html)
555+
const showIframe = !!html;
541556
// Streaming is active until html has stopped changing
542557
const isStreaming = !!html && !htmlSettled;
543558
const loadingPhrase = useLoadingPhrase(isStreaming);
544-
545-
// Keep the streaming indicator mounted long enough to fade out
546-
const [showStreamingIndicator, setShowStreamingIndicator] = useState(false);
547-
useEffect(() => {
548-
if (isStreaming) {
549-
setShowStreamingIndicator(true);
550-
} else if (showStreamingIndicator) {
551-
// Keep mounted for fade-out, then unmount
552-
const timeout = setTimeout(() => setShowStreamingIndicator(false), 600);
553-
return () => clearTimeout(timeout);
554-
}
555-
}, [isStreaming, showStreamingIndicator]);
559+
const showStreamingIndicator = isStreaming || fadingOut;
556560

557561
return (
558562
<div className="w-full my-3">
@@ -599,7 +603,7 @@ export function WidgetRenderer({ title, description, html }: WidgetRendererProps
599603
content streamed via postMessage for progressive rendering. */}
600604
<iframe
601605
ref={iframeRef}
602-
sandbox="allow-scripts allow-same-origin"
606+
sandbox="allow-scripts"
603607
className="w-full border-0"
604608
onLoad={() => setLoaded(true)}
605609
style={{

apps/app/src/components/template-library/index.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ function submitChatPrompt(text: string) {
99
const textarea = document.querySelector<HTMLTextAreaElement>(
1010
'[data-testid="copilot-chat-textarea"]'
1111
);
12-
if (!textarea) return;
12+
if (!textarea) {
13+
console.warn("submitChatPrompt: CopilotChat textarea not found — template apply message was not sent.");
14+
return;
15+
}
1316

1417
const setter = Object.getOwnPropertyDescriptor(
1518
window.HTMLTextAreaElement.prototype, "value"
@@ -35,6 +38,7 @@ interface Template {
3538
html: string;
3639
data_description: string;
3740
component_type?: string;
41+
component_data?: Record<string, unknown>;
3842
version: number;
3943
}
4044

@@ -177,8 +181,8 @@ export function TemplateLibrary({ open, onClose }: TemplateLibraryProps) {
177181
name={t.name}
178182
description={t.description}
179183
html={t.html}
180-
componentType={(t as unknown as Record<string, unknown>).component_type as string | undefined}
181-
componentData={(t as unknown as Record<string, unknown>).component_data as Record<string, unknown> | undefined}
184+
componentType={t.component_type}
185+
componentData={t.component_data}
182186
dataDescription={t.data_description}
183187
version={t.version}
184188
onApply={handleApplyClick}

apps/app/src/components/template-library/template-card.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ function PieChartPreview({ data }: { data: { label: string; value: number }[] })
6262
if (total === 0) return null;
6363

6464
const cx = 60, cy = 60, r = 50;
65-
let cumulative = 0;
65+
const cumulativeAngles = data.reduce<number[]>((acc, d) => {
66+
acc.push((acc[acc.length - 1] ?? 0) + d.value);
67+
return acc;
68+
}, []);
6669
const slices = data.map((d, i) => {
67-
const startAngle = (cumulative / total) * 2 * Math.PI - Math.PI / 2;
68-
cumulative += d.value;
69-
const endAngle = (cumulative / total) * 2 * Math.PI - Math.PI / 2;
70+
const startAngle = ((cumulativeAngles[i] - d.value) / total) * 2 * Math.PI - Math.PI / 2;
71+
const endAngle = (cumulativeAngles[i] / total) * 2 * Math.PI - Math.PI / 2;
7072
const largeArc = d.value / total > 0.5 ? 1 : 0;
7173
const x1 = cx + r * Math.cos(startAngle);
7274
const y1 = cy + r * Math.sin(startAngle);
@@ -150,7 +152,7 @@ body {
150152
) : html ? (
151153
<iframe
152154
ref={iframeRef}
153-
sandbox="allow-same-origin allow-scripts"
155+
sandbox="allow-scripts"
154156
onLoad={() => setPreviewReady(true)}
155157
className="border-0 w-[300%] h-[300%] origin-top-left"
156158
style={{

0 commit comments

Comments
 (0)