Compare commits

..

1 Commits

Author SHA1 Message Date
Devin AI
ff98f2a878 fix: Add XSS protection to flow visualization (issue #4326)
- Add DOMPurify CDN to HTML template for sanitizing HTML content
- Add escapeHtml helper function to escape user-controlled text
- Add sanitizeHtml function using DOMPurify with allowed tags/attributes
- Apply sanitization to drawer content before setting innerHTML
- Escape user-controlled values in renderTriggerCondition, renderConditionTree, and renderMetadata methods
- Add tests to verify XSS protection is in place

Co-Authored-By: João <joao@crewai.com>
2026-02-01 13:10:35 +00:00
5 changed files with 109 additions and 154 deletions

View File

@@ -900,11 +900,6 @@ class Agent(BaseAgent):
self.agent_executor.tools_handler = self.tools_handler
self.agent_executor.request_within_rpm_limit = rpm_limit_fn
# Clear accumulated messages and reset iterations from previous tasks
# to prevent context pollution when agent is reused across sequential tasks
self.agent_executor.messages = []
self.agent_executor.iterations = 0
if self.agent_executor.llm:
existing_stop = getattr(self.agent_executor.llm, "stop", [])
self.agent_executor.llm.stop = list(

View File

@@ -64,6 +64,37 @@ function highlightPython(code) {
return Prism.highlight(code, Prism.languages.python, "python");
}
function escapeHtml(text) {
if (text === null || text === undefined) {
return "";
}
const str = String(text);
const div = document.createElement("div");
div.textContent = str;
return div.innerHTML;
}
function sanitizeHtml(html) {
if (typeof DOMPurify !== "undefined") {
return DOMPurify.sanitize(html, {
ALLOWED_TAGS: [
"div", "span", "pre", "code", "ul", "li", "button", "svg", "line",
"rect", "path", "polyline", "i", "label", "select", "option"
],
ALLOWED_ATTR: [
"class", "style", "id", "data-code", "data-node-id", "data-trigger-items",
"data-trigger-group", "data-router-paths", "data-condition-label",
"data-lucide", "viewBox", "fill", "stroke", "stroke-width",
"stroke-dasharray", "stroke-linecap", "stroke-linejoin",
"x", "y", "x1", "y1", "x2", "y2", "width", "height", "rx",
"points", "d", "onmouseover", "onmouseout"
],
ALLOW_DATA_ATTR: true,
});
}
return html;
}
class NodeRenderer {
constructor(nodes, networkManager) {
this.nodes = nodes;
@@ -1428,7 +1459,7 @@ class DrawerManager {
content += this.renderSourceCode(metadata);
}
this.elements.content.innerHTML = content;
this.elements.content.innerHTML = sanitizeHtml(content);
this.attachContentEventListeners(nodeName);
// Initialize Lucide icons in the newly rendered drawer content
@@ -1448,9 +1479,9 @@ class DrawerManager {
<ul class="drawer-list">
${grouped.map((group) => {
if (group.items.length === 1) {
return `<li><span class="drawer-code-link" data-node-id="${group.items[0]}">${group.items[0]}</span></li>`;
return `<li><span class="drawer-code-link" data-node-id="${escapeHtml(group.items[0])}">${escapeHtml(group.items[0])}</span></li>`;
} else {
const groupId = group.items.join(',');
const groupId = escapeHtml(group.items.join(','));
return `
<li>
<div class="trigger-group" data-trigger-items="${groupId}" style="border-left: 2px solid var(--text-secondary); padding: 4px 0 4px 8px; margin: 2px 0; border-radius: 3px; cursor: pointer; transition: background 0.15s ease;">
@@ -1458,7 +1489,7 @@ class DrawerManager {
${group.items.length} routes <span style="opacity: 0.5; font-size: 9px;"></span>
</div>
<div class="trigger-group-items" style="display: flex; flex-wrap: wrap; gap: 4px; pointer-events: none;">
${group.items.map((t) => `<span class="drawer-code" style="opacity: 0.7;">${t}</span>`).join("")}
${group.items.map((t) => `<span class="drawer-code" style="opacity: 0.7;">${escapeHtml(t)}</span>`).join("")}
</div>
</div>
</li>
@@ -1535,17 +1566,17 @@ class DrawerManager {
renderConditionTree(condition, depth = 0) {
if (typeof condition === "string") {
return `<span class="drawer-code-link trigger-leaf" data-node-id="${condition}">${condition}</span>`;
return `<span class="drawer-code-link trigger-leaf" data-node-id="${escapeHtml(condition)}">${escapeHtml(condition)}</span>`;
}
if (condition.type === "AND" || condition.type === "OR") {
const conditionType = condition.type;
const conditionType = escapeHtml(condition.type);
const color = conditionType === "AND" ? "{{ CREWAI_ORANGE }}" : "var(--text-secondary)";
const bgColor = conditionType === "AND" ? "rgba(255,90,80,0.08)" : "rgba(102,102,102,0.06)";
const hoverBg = conditionType === "AND" ? "rgba(255,90,80,0.15)" : "rgba(102,102,102,0.12)";
const triggerIds = this.extractTriggerIds(condition);
const triggerIdsJson = JSON.stringify(triggerIds).replace(/"/g, '&quot;');
const triggerIdsJson = escapeHtml(JSON.stringify(triggerIds));
const stringChildren = condition.conditions.filter(c => typeof c === 'string');
const nonStringChildren = condition.conditions.filter(c => typeof c !== 'string');
@@ -1562,7 +1593,7 @@ class DrawerManager {
if (group.items.length === 1) {
return this.renderConditionTree(group.items[0], depth + 1);
} else {
const groupId = group.items.join(',');
const groupId = escapeHtml(group.items.join(','));
const groupColor = conditionType === "AND" ? "{{ CREWAI_ORANGE }}" : "var(--text-secondary)";
const groupBgColor = conditionType === "AND" ? "rgba(255,90,80,0.08)" : "rgba(102,102,102,0.06)";
const groupHoverBg = conditionType === "AND" ? "rgba(255,90,80,0.15)" : "rgba(102,102,102,0.12)";
@@ -1572,7 +1603,7 @@ class DrawerManager {
${group.items.length} routes <i data-lucide="chevron-down" style="width: 14px; height: 14px; color: ${groupColor};"></i>
</div>
<div class="trigger-group-items" style="display: flex; flex-wrap: wrap; gap: 4px; margin-top: 4px; pointer-events: none;">
${group.items.map((t) => `<span class="drawer-code-link trigger-leaf" style="opacity: 0.7; cursor: default;">${t}</span>`).join("")}
${group.items.map((t) => `<span class="drawer-code-link trigger-leaf" style="opacity: 0.7; cursor: default;">${escapeHtml(t)}</span>`).join("")}
</div>
</div>
`;
@@ -1615,7 +1646,7 @@ class DrawerManager {
console.log('renderMetadata called with:', metadata);
let metadataContent = "";
const nodeType = metadata.type || "unknown";
const nodeType = escapeHtml(metadata.type || "unknown");
const typeBadgeColor =
nodeType === "start" || nodeType === "router"
? "{{ CREWAI_ORANGE }}"
@@ -1640,7 +1671,7 @@ class DrawerManager {
metadataContent += `
<div class="drawer-section">
<div class="drawer-section-title">Condition</div>
<span class="drawer-badge" style="background: ${bg}; color: ${color};">${metadata.condition_type}</span>
<span class="drawer-badge" style="background: ${bg}; color: ${color};">${escapeHtml(metadata.condition_type)}</span>
</div>
`;
}
@@ -1662,14 +1693,14 @@ class DrawerManager {
if (metadata.router_paths && metadata.router_paths.length > 0) {
const uniqueRouterPaths = [...new Set(metadata.router_paths)];
const routerPathsJson = JSON.stringify(uniqueRouterPaths).replace(/"/g, '&quot;');
const routerPathsJson = escapeHtml(JSON.stringify(uniqueRouterPaths));
metadataContent += `
<div class="drawer-section">
<div class="drawer-section-title router-paths-title" data-router-paths="${routerPathsJson}" style="cursor: pointer; display: inline-flex; align-items: center; gap: 4px;">
Router Paths <i data-lucide="chevron-down" style="width: 14px; height: 14px; color: var(--text-primary);"></i>
</div>
<ul class="drawer-list">
${uniqueRouterPaths.map((p) => `<li><span class="drawer-code-link" data-node-id="${p}" style="color: {{ CREWAI_ORANGE }}; border-color: rgba(255,90,80,0.3);">${p}</span></li>`).join("")}
${uniqueRouterPaths.map((p) => `<li><span class="drawer-code-link" data-node-id="${escapeHtml(p)}" style="color: {{ CREWAI_ORANGE }}; border-color: rgba(255,90,80,0.3);">${escapeHtml(p)}</span></li>`).join("")}
</ul>
</div>
`;

View File

@@ -9,6 +9,7 @@
<script src="https://unpkg.com/lucide@latest"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/prism/1.29.0/prism.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/prism/1.29.0/components/prism-python.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/dompurify/3.0.6/purify.min.js"></script>
<script src="'{{ js_path }}'"></script>
</head>
<body>

View File

@@ -2351,138 +2351,3 @@ def test_agent_without_apps_no_platform_tools():
tools = crew._prepare_tools(agent, task, [])
assert tools == []
def test_agent_executor_messages_cleared_between_tasks():
"""Test that agent executor messages are cleared when agent is reused across tasks.
This test verifies the fix for issue #4319 where messages accumulated when
the same agent was reused across multiple sequential tasks, causing:
- Duplicate system messages
- Context pollution
- Eventually crashes with 'Invalid response from LLM call - None or empty'
"""
agent = Agent(
role="Test Agent",
goal="Execute tasks",
backstory="Test agent for message accumulation fix",
allow_delegation=False,
)
task1 = Task(
description="Task 1: Simple calculation",
expected_output="Result",
agent=agent,
)
task2 = Task(
description="Task 2: Another calculation",
expected_output="Result",
agent=agent,
)
task3 = Task(
description="Task 3: Yet another calculation",
expected_output="Result",
agent=agent,
)
# Create the agent executor for the first task
agent.create_agent_executor(tools=[], task=task1)
assert agent.agent_executor is not None
initial_executor = agent.agent_executor
# Simulate messages being added during task execution
agent.agent_executor.messages = [
{"role": "system", "content": "System prompt"},
{"role": "user", "content": "Task 1 prompt"},
{"role": "assistant", "content": "Task 1 response"},
]
agent.agent_executor.iterations = 5
# Reuse the agent for task 2 - this should clear messages
agent.create_agent_executor(tools=[], task=task2)
# Verify the same executor instance is reused
assert agent.agent_executor is initial_executor
# Verify messages were cleared
assert agent.agent_executor.messages == [], (
"Messages should be cleared when agent executor is reused for a new task"
)
# Verify iterations were reset
assert agent.agent_executor.iterations == 0, (
"Iterations should be reset when agent executor is reused for a new task"
)
# Simulate more messages for task 2
agent.agent_executor.messages = [
{"role": "system", "content": "System prompt"},
{"role": "user", "content": "Task 2 prompt"},
{"role": "assistant", "content": "Task 2 response"},
]
agent.agent_executor.iterations = 3
# Reuse the agent for task 3 - this should clear messages again
agent.create_agent_executor(tools=[], task=task3)
# Verify the same executor instance is still reused
assert agent.agent_executor is initial_executor
# Verify messages were cleared again
assert agent.agent_executor.messages == [], (
"Messages should be cleared each time agent executor is reused"
)
# Verify iterations were reset again
assert agent.agent_executor.iterations == 0, (
"Iterations should be reset each time agent executor is reused"
)
def test_agent_executor_state_isolation_across_crews():
"""Test that agent executor state is properly isolated when same agent is used in multiple crews.
This is a common pattern in Flow-based workflows where agents are reused
across multiple crew kickoffs.
"""
agent = Agent(
role="Reusable Agent",
goal="Execute tasks across multiple crews",
backstory="Agent that gets reused",
allow_delegation=False,
)
# First crew with first task
task1 = Task(
description="First crew task",
expected_output="Result 1",
agent=agent,
)
# Create executor for first task
agent.create_agent_executor(tools=[], task=task1)
executor = agent.agent_executor
# Simulate task execution adding messages
executor.messages = [
{"role": "system", "content": "System 1"},
{"role": "user", "content": "User 1"},
]
executor.iterations = 2
# Second crew with second task (simulating Flow pattern)
task2 = Task(
description="Second crew task",
expected_output="Result 2",
agent=agent,
)
# Reuse agent for second task
agent.create_agent_executor(tools=[], task=task2)
# State should be clean for the new task
assert executor.messages == []
assert executor.iterations == 0
assert executor.task == task2

View File

@@ -667,4 +667,67 @@ def test_no_warning_for_properly_typed_router(caplog):
# No warnings should be logged
warning_messages = [r.message for r in caplog.records if r.levelno >= logging.WARNING]
assert not any("Could not determine return paths" in msg for msg in warning_messages)
assert not any("Found listeners waiting for triggers" in msg for msg in warning_messages)
assert not any("Found listeners waiting for triggers" in msg for msg in warning_messages)
def test_visualization_includes_dompurify_for_xss_protection():
"""Test that visualization includes DOMPurify library for XSS protection."""
flow = SimpleFlow()
structure = build_flow_structure(flow)
html_file = visualize_flow_structure(structure, "test_flow.html", show=False)
with open(html_file, "r", encoding="utf-8") as f:
html_content = f.read()
assert "dompurify" in html_content.lower()
assert "cdnjs.cloudflare.com/ajax/libs/dompurify" in html_content
def test_visualization_js_includes_sanitization_functions():
"""Test that generated JS includes escapeHtml and sanitizeHtml functions."""
flow = SimpleFlow()
structure = build_flow_structure(flow)
html_file = visualize_flow_structure(structure, "test_flow.html", show=False)
html_path = Path(html_file)
js_file = html_path.parent / f"{html_path.stem}_script.js"
js_content = js_file.read_text(encoding="utf-8")
assert "function escapeHtml" in js_content
assert "function sanitizeHtml" in js_content
assert "DOMPurify.sanitize" in js_content
def test_visualization_uses_sanitize_html_for_drawer_content():
"""Test that drawer content rendering uses sanitizeHtml."""
flow = SimpleFlow()
structure = build_flow_structure(flow)
html_file = visualize_flow_structure(structure, "test_flow.html", show=False)
html_path = Path(html_file)
js_file = html_path.parent / f"{html_path.stem}_script.js"
js_content = js_file.read_text(encoding="utf-8")
assert "innerHTML = sanitizeHtml(content)" in js_content
def test_visualization_escapes_user_controlled_values():
"""Test that user-controlled values are escaped in render methods."""
flow = SimpleFlow()
structure = build_flow_structure(flow)
html_file = visualize_flow_structure(structure, "test_flow.html", show=False)
html_path = Path(html_file)
js_file = html_path.parent / f"{html_path.stem}_script.js"
js_content = js_file.read_text(encoding="utf-8")
assert "escapeHtml(group.items[0])" in js_content
assert "escapeHtml(condition)" in js_content
assert "escapeHtml(metadata.type" in js_content