Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions mcp_zammad/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,12 @@ class TicketCreate(StrictBaseModel):
@field_validator("title", "article_body")
@classmethod
def sanitize_html(cls, v: str) -> str:
"""Escape HTML to prevent XSS attacks."""
return html.escape(v)
"""Escape HTML to prevent XSS attacks.

quote=False: title and the initial article are plain text (no content_type choice here),
sent/stored verbatim, so quotes and apostrophes must not become '/" entities.
"""
return html.escape(v, quote=False)


class TicketUpdate(StrictBaseModel):
Expand Down Expand Up @@ -341,7 +345,10 @@ class ArticleCreate(StrictBaseModel):
def sanitize_body(self) -> "ArticleCreate":
"""Sanitize body content according to content type."""
if self.content_type == "text/plain":
self.body = html.escape(self.body)
# quote=False: plain text is sent/stored verbatim (e.g. in outbound emails), so quotes
# and apostrophes must not be turned into '/" entities. Still neutralize
# <, >, & in case a downstream renderer treats the body as HTML despite the content type.
self.body = html.escape(self.body, quote=False)
else:
self.body = self._sanitize_html_body(self.body)
return self
Expand Down
13 changes: 9 additions & 4 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
customer="test@example.com",
article_body="Test body",
)
assert ticket.title == "&lt;script&gt;alert(&#x27;XSS&#x27;)&lt;/script&gt;"
assert ticket.title == "&lt;script&gt;alert('XSS')&lt;/script&gt;"

def test_html_sanitization_in_body(self):
"""Test that HTML is escaped in article body."""
Expand All @@ -36,7 +36,7 @@
customer="test@example.com",
article_body="<b>Bold</b> and <script>alert('XSS')</script>",
)
assert ticket.article_body == "&lt;b&gt;Bold&lt;/b&gt; and &lt;script&gt;alert(&#x27;XSS&#x27;)&lt;/script&gt;"
assert ticket.article_body == "&lt;b&gt;Bold&lt;/b&gt; and &lt;script&gt;alert('XSS')&lt;/script&gt;"

def test_field_length_limits(self):
"""Test that field length limits are enforced."""
Expand Down Expand Up @@ -74,12 +74,17 @@
"""Test ArticleCreate model validation."""

def test_html_sanitization_in_body(self):
"""Test that HTML is escaped in article body."""
"""Test that HTML is escaped in article body, but quotes/apostrophes are left intact.

Check notice on line 77 in tests/test_models.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/test_models.py#L77

Multi-line docstring summary should start at the second line (D213)

text/plain bodies are sent verbatim (e.g. in outbound emails), so quotes must not
become &#x27;/&quot; entities. <, >, & are still escaped, which already neutralizes
the tag regardless of the quote style used inside it.
"""
article = ArticleCreate(
ticket_id=123,
body="<div onclick='alert()'>Click me</div>",
)
assert article.body == "&lt;div onclick=&#x27;alert()&#x27;&gt;Click me&lt;/div&gt;"
assert article.body == "&lt;div onclick='alert()'&gt;Click me&lt;/div&gt;"

def test_ticket_id_validation(self):
"""Test that ticket_id must be positive."""
Expand Down
3 changes: 3 additions & 0 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,9 @@ def test_add_article_content_type_validation() -> None:
plain_article = ArticleCreate(ticket_id=1, body="<p>plain</p>", content_type="text/plain")
assert plain_article.body == "&lt;p&gt;plain&lt;/p&gt;"

apostrophe_article = ArticleCreate(ticket_id=1, body="we've got it, you'd agree", content_type="text/plain")
assert apostrophe_article.body == "we've got it, you'd agree"

with pytest.raises(ValidationError, match="content_type"):
ArticleCreate(ticket_id=1, body="test", content_type="application/json") # type: ignore[arg-type]

Expand Down