Summary
Several computed fields.Html fields use sanitize=False and build HTML via f-strings with unescaped dynamic values. While the practical risk is low (all views are internal/admin-only, and data originates from ORM field values), adding markupsafe.escape() on dynamic content would provide defense-in-depth against stored XSS from malicious internal users or compromised external data intake.
Flagged by Gemini Code Assist on PR #49.
Affected Fields
spp_audit/models/spp_audit_log.py
data_html (line ~170): Old/new field values inserted into <td> tags without escaping
parent_data_html (line ~213): Same pattern for parent record changes
spp_change_request_v2/models/change_request.py
registrant_summary_html (line ~336): reg.name, reg.spp_id, address fields inserted without escaping
preview_html (line ~723): Change request field diff values (display_value, display_key) inserted without escaping
preview_html_snapshot: Stores snapshot of preview_html, inherits same issue
spp_change_request_v2/wizards/preview_changes_wizard.py
preview_html (line ~88): Similar pattern with display_key and display_value
Recommended Fix
Use markupsafe.escape() (or odoo.tools.html_escape) on all dynamic values before inserting into HTML strings:
from markupsafe import escape
# Before
html_parts.append(f"<td>{item}</td>")
# After
html_parts.append(f"<td>{escape(item)}</td>")
Risk Assessment
- Severity: Low (internal-only views, requires authenticated access to store payload)
- Impact: An authenticated user could store
<script> content in a model field that executes when an admin views audit logs or CR previews
- Priority: Low — hardening / defense-in-depth