Skip to content

Commit 029547c

Browse files
test(spp_audit,spp_change_request_v2): add HTML escaping coverage tests
Verify that computed Html fields properly escape XSS payloads in dynamic values (script tags, img onerror, etc.) for audit logs, change request previews, registrant summaries, and preview wizard.
1 parent 6f25405 commit 029547c

5 files changed

Lines changed: 340 additions & 0 deletions

File tree

spp_audit/tests/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
from . import test_audit_backend
2+
from . import test_html_escaping
23
from . import test_spp_audit_rule
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
from odoo.tests.common import TransactionCase
2+
3+
4+
class TestAuditHtmlEscaping(TransactionCase):
5+
"""Tests that audit log HTML fields properly escape dynamic values."""
6+
7+
@classmethod
8+
def setUpClass(cls):
9+
super().setUpClass()
10+
cls.model_partner = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
11+
cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.model_partner.id)], limit=1)
12+
if not cls.audit_rule:
13+
cls.audit_rule = cls.env["spp.audit.rule"].create(
14+
{
15+
"name": "Test Rule",
16+
"model_id": cls.model_partner.id,
17+
"is_log_create": True,
18+
"is_log_write": True,
19+
"is_log_unlink": False,
20+
}
21+
)
22+
23+
def _create_audit_log(self, old_vals, new_vals):
24+
"""Create an audit log record with given old/new values."""
25+
data = repr({"old": old_vals, "new": new_vals})
26+
return self.env["spp.audit.log"].create(
27+
{
28+
"audit_rule_id": self.audit_rule.id,
29+
"user_id": self.env.uid,
30+
"model_id": self.model_partner.id,
31+
"res_id": 1,
32+
"method": "write",
33+
"data": data,
34+
}
35+
)
36+
37+
def test_data_html_escapes_script_tags(self):
38+
"""Verify data_html escapes <script> in field values."""
39+
xss_payload = '<script>alert("xss")</script>'
40+
log = self._create_audit_log(
41+
old_vals={"name": "Safe Name"},
42+
new_vals={"name": xss_payload},
43+
)
44+
html = log.data_html
45+
self.assertNotIn("<script>", html)
46+
self.assertIn("&lt;script&gt;", html)
47+
48+
def test_data_html_escapes_html_entities(self):
49+
"""Verify data_html escapes angle brackets and ampersands."""
50+
log = self._create_audit_log(
51+
old_vals={"name": "Before <b>bold</b>"},
52+
new_vals={"name": "After <img src=x onerror=alert(1)>"},
53+
)
54+
html = log.data_html
55+
self.assertNotIn("<img ", html)
56+
self.assertIn("&lt;img ", html)
57+
self.assertNotIn("<b>bold</b>", html)
58+
self.assertIn("&lt;b&gt;bold&lt;/b&gt;", html)
59+
60+
def test_data_html_renders_table_structure(self):
61+
"""Verify data_html still produces valid table structure."""
62+
log = self._create_audit_log(
63+
old_vals={"name": "Old"},
64+
new_vals={"name": "New"},
65+
)
66+
html = log.data_html
67+
self.assertIn("<table", html)
68+
self.assertIn("<thead>", html)
69+
self.assertIn("<tbody>", html)
70+
self.assertIn("<td>", html)
71+
72+
def test_parent_data_html_escapes_script_tags(self):
73+
"""Verify parent_data_html escapes <script> in field values."""
74+
xss_payload = '<script>alert("xss")</script>'
75+
parent_model = self.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
76+
log = self.env["spp.audit.log"].create(
77+
{
78+
"audit_rule_id": self.audit_rule.id,
79+
"user_id": self.env.uid,
80+
"model_id": self.model_partner.id,
81+
"res_id": 1,
82+
"method": "write",
83+
"data": repr({"old": {"name": "Safe"}, "new": {"name": xss_payload}}),
84+
"parent_model_id": parent_model.id,
85+
"parent_res_ids_str": "1",
86+
}
87+
)
88+
html = log.parent_data_html
89+
self.assertNotIn("<script>", html)

spp_change_request_v2/tests/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@
2323
from . import test_approval_hooks_and_audit
2424
from . import test_dynamic_approval
2525
from . import test_conflict_dynamic_approval
26+
from . import test_html_escaping
27+
from . import test_wizard_html_escaping
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
from unittest.mock import patch
2+
3+
from odoo.tests import tagged
4+
5+
from .test_change_request import TestChangeRequestBase
6+
7+
8+
@tagged("post_install", "-at_install")
9+
class TestHtmlEscaping(TestChangeRequestBase):
10+
"""Tests that computed HTML fields properly escape dynamic values."""
11+
12+
def _create_cr(self, registrant=None, cr_type=None):
13+
"""Helper to create a CR for testing."""
14+
vals = {
15+
"request_type_id": (cr_type or self.cr_type_add_member).id,
16+
}
17+
if registrant is not False:
18+
vals["registrant_id"] = (registrant or self.group).id
19+
return self.cr_model.create(vals)
20+
21+
def test_registrant_summary_escapes_name(self):
22+
"""Verify registrant_summary_html escapes XSS in registrant name."""
23+
xss_name = '<script>alert("xss")</script>'
24+
registrant = self.partner_model.create(
25+
{
26+
"name": xss_name,
27+
"is_registrant": True,
28+
"is_group": False,
29+
}
30+
)
31+
cr = self._create_cr(registrant=registrant)
32+
cr.invalidate_recordset()
33+
html = cr.registrant_summary_html
34+
self.assertNotIn("<script>", html)
35+
self.assertIn("&lt;script&gt;", html)
36+
37+
def test_registrant_summary_escapes_address(self):
38+
"""Verify registrant_summary_html escapes XSS in street/city."""
39+
registrant = self.partner_model.create(
40+
{
41+
"name": "Safe Name",
42+
"is_registrant": True,
43+
"is_group": False,
44+
"street": '<img src=x onerror=alert(1)>',
45+
"city": '<b onmouseover=alert(2)>City</b>',
46+
}
47+
)
48+
cr = self._create_cr(registrant=registrant)
49+
cr.invalidate_recordset()
50+
html = cr.registrant_summary_html
51+
self.assertNotIn("<img ", html)
52+
self.assertNotIn("<b ", html)
53+
self.assertIn("&lt;img ", html)
54+
self.assertIn("&lt;b ", html)
55+
56+
def test_registrant_summary_escapes_spp_id(self):
57+
"""Verify registrant_summary_html escapes XSS in spp_id."""
58+
registrant = self.partner_model.create(
59+
{
60+
"name": "Safe Name",
61+
"is_registrant": True,
62+
"is_group": False,
63+
}
64+
)
65+
if hasattr(registrant, "spp_id"):
66+
registrant.spp_id = '<script>alert("id")</script>'
67+
cr = self._create_cr(registrant=registrant)
68+
cr.invalidate_recordset()
69+
html = cr.registrant_summary_html
70+
self.assertNotIn("<script>", html)
71+
72+
def test_preview_html_escapes_field_values(self):
73+
"""Verify _generate_preview_html escapes dynamic values."""
74+
cr = self._create_cr()
75+
xss_changes = {
76+
"_action": "update",
77+
"name": {
78+
"old": '<script>alert("old")</script>',
79+
"new": '<img src=x onerror=alert("new")>',
80+
},
81+
"notes": '<script>alert("notes")</script>',
82+
}
83+
# Mock the strategy.preview() to return XSS payloads
84+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
85+
mock_strategy.return_value.preview.return_value = xss_changes
86+
cr.invalidate_recordset()
87+
html = cr._generate_preview_html()
88+
89+
self.assertNotIn("<script>", html)
90+
self.assertNotIn("<img ", html)
91+
self.assertIn("&lt;script&gt;", html)
92+
self.assertIn("&lt;img ", html)
93+
94+
def test_preview_html_escapes_list_values(self):
95+
"""Verify _generate_preview_html escapes list values."""
96+
cr = self._create_cr()
97+
xss_changes = {
98+
"_action": "update",
99+
"tags": ['<script>alert(1)</script>', "safe value"],
100+
}
101+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
102+
mock_strategy.return_value.preview.return_value = xss_changes
103+
cr.invalidate_recordset()
104+
html = cr._generate_preview_html()
105+
106+
self.assertNotIn("<script>", html)
107+
self.assertIn("&lt;script&gt;", html)
108+
self.assertIn("safe value", html)
109+
110+
def test_preview_html_escapes_action_label(self):
111+
"""Verify _generate_preview_html escapes unknown action labels."""
112+
cr = self._create_cr()
113+
xss_changes = {
114+
"_action": '<img src=x onerror=alert("action")>',
115+
"field": "value",
116+
}
117+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
118+
mock_strategy.return_value.preview.return_value = xss_changes
119+
cr.invalidate_recordset()
120+
html = cr._generate_preview_html()
121+
122+
# .title() changes case, so check case-insensitively
123+
self.assertNotIn("<img ", html.lower())
124+
self.assertIn("&lt;", html)
125+
126+
def test_preview_html_preserves_safe_content(self):
127+
"""Verify normal content renders correctly after escaping."""
128+
cr = self._create_cr()
129+
safe_changes = {
130+
"_action": "update",
131+
"full_name": {"old": "Old Name", "new": "New Name"},
132+
}
133+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
134+
mock_strategy.return_value.preview.return_value = safe_changes
135+
cr.invalidate_recordset()
136+
html = cr._generate_preview_html()
137+
138+
self.assertIn("Old Name", html)
139+
self.assertIn("New Name", html)
140+
self.assertIn("Full Name", html)
141+
self.assertIn("<table", html)
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import json
2+
from unittest.mock import patch
3+
4+
from odoo.tests import tagged
5+
6+
from .test_change_request import TestChangeRequestBase
7+
8+
9+
@tagged("post_install", "-at_install")
10+
class TestWizardHtmlEscaping(TestChangeRequestBase):
11+
"""Tests that the preview wizard properly escapes HTML in dynamic values."""
12+
13+
def _create_cr(self, registrant=None, cr_type=None):
14+
"""Helper to create a CR for testing."""
15+
vals = {
16+
"request_type_id": (cr_type or self.cr_type_add_member).id,
17+
}
18+
if registrant is not False:
19+
vals["registrant_id"] = (registrant or self.group).id
20+
return self.cr_model.create(vals)
21+
22+
def _create_wizard(self, cr):
23+
"""Create a preview wizard for the given CR."""
24+
return self.env["spp.cr.preview.wizard"].create(
25+
{"change_request_id": cr.id}
26+
)
27+
28+
def test_wizard_preview_escapes_action(self):
29+
"""Verify wizard preview_html escapes XSS in action value."""
30+
cr = self._create_cr()
31+
wizard = self._create_wizard(cr)
32+
xss_changes = {
33+
"_action": '<script>alert("action")</script>',
34+
"field": "value",
35+
}
36+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
37+
mock_strategy.return_value.preview.return_value = xss_changes
38+
wizard.invalidate_recordset()
39+
html = wizard.preview_html
40+
41+
self.assertNotIn("<script>", html)
42+
self.assertIn("&lt;script&gt;", html)
43+
44+
def test_wizard_preview_escapes_field_values(self):
45+
"""Verify wizard preview_html escapes XSS in field keys and values."""
46+
cr = self._create_cr()
47+
wizard = self._create_wizard(cr)
48+
xss_changes = {
49+
"_action": "update",
50+
"safe_field": '<img src=x onerror=alert(1)>',
51+
}
52+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
53+
mock_strategy.return_value.preview.return_value = xss_changes
54+
wizard.invalidate_recordset()
55+
html = wizard.preview_html
56+
57+
self.assertNotIn("<img ", html)
58+
self.assertIn("&lt;img ", html)
59+
60+
def test_wizard_preview_escapes_list_values(self):
61+
"""Verify wizard preview_html escapes list items."""
62+
cr = self._create_cr()
63+
wizard = self._create_wizard(cr)
64+
xss_changes = {
65+
"_action": "update",
66+
"tags": ['<script>alert(1)</script>', "safe"],
67+
}
68+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
69+
mock_strategy.return_value.preview.return_value = xss_changes
70+
wizard.invalidate_recordset()
71+
html = wizard.preview_html
72+
73+
self.assertNotIn("<script>", html)
74+
self.assertIn("&lt;script&gt;", html)
75+
self.assertIn("safe", html)
76+
77+
def test_wizard_preview_from_snapshot_escapes(self):
78+
"""Verify wizard escapes values from stored JSON snapshots."""
79+
cr = self._create_cr()
80+
xss_snapshot = json.dumps({
81+
"_action": "update",
82+
"name": '<script>alert("snapshot")</script>',
83+
})
84+
cr.write({"is_applied": True, "preview_json_snapshot": xss_snapshot})
85+
wizard = self._create_wizard(cr)
86+
wizard.invalidate_recordset()
87+
html = wizard.preview_html
88+
89+
self.assertNotIn("<script>", html)
90+
self.assertIn("&lt;script&gt;", html)
91+
92+
def test_wizard_preview_preserves_safe_content(self):
93+
"""Verify safe content renders correctly in wizard preview."""
94+
cr = self._create_cr()
95+
wizard = self._create_wizard(cr)
96+
safe_changes = {
97+
"_action": "create",
98+
"full_name": "John Doe",
99+
}
100+
with patch.object(type(cr.request_type_id), "get_apply_strategy") as mock_strategy:
101+
mock_strategy.return_value.preview.return_value = safe_changes
102+
wizard.invalidate_recordset()
103+
html = wizard.preview_html
104+
105+
self.assertIn("John Doe", html)
106+
self.assertIn("Full Name", html)
107+
self.assertIn("create", html)

0 commit comments

Comments
 (0)