Skip to content

Commit 831df20

Browse files
committed
fix(spp_simulation): escape HTML content to prevent XSS and fix missing targeting_accuracy field
- Add markupsafe Markup/escape imports and use Markup.format() for all HTML computed fields in simulation_run.py and simulation_comparison.py to safely escape user-controlled values (scenario names, area names, metric names, CEL expressions, targeting expressions, multiplier fields) - Use Markup() for static HTML strings in html_parts lists so join works correctly - Escape program.name in simulation_service.py chatter message_post body - Replace non-existent doc.targeting_accuracy field in simulation_report.xml with a condition on the existing leakage_rate/undercoverage_rate fields - Remove targeting_accuracy from comparison_table.js metrics array
1 parent 6fa3059 commit 831df20

File tree

5 files changed

+144
-91
lines changed

5 files changed

+144
-91
lines changed

spp_simulation/models/simulation_comparison.py

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import logging
22
from datetime import datetime
33

4+
from markupsafe import Markup
5+
46
from odoo import _, api, fields, models
57
from odoo.exceptions import ValidationError
68

@@ -36,8 +38,7 @@ class SimulationComparison(models.Model):
3638
overlap_count_json = fields.Json(
3739
string="Overlap Counts",
3840
readonly=True,
39-
help="Aggregated overlap counts between scenarios. "
40-
"Computed from current registry data, not historical state.",
41+
help="Aggregated overlap counts between scenarios. Computed from current registry data, not historical state.",
4142
)
4243
staleness_warning = fields.Text(
4344
string="Staleness Warning",
@@ -140,10 +141,10 @@ def _compute_parameters_comparison_html(self):
140141
continue
141142

142143
# Build comparison table
143-
html_parts = ['<table class="table table-bordered table-sm">']
144+
html_parts = [Markup('<table class="table table-bordered table-sm">')]
144145

145146
# Header row with scenario names and execution dates
146-
html_parts.append("<thead><tr><th>Parameter</th>")
147+
html_parts.append(Markup("<thead><tr><th>Parameter</th>"))
147148
for run_data in runs:
148149
name = run_data.get("scenario_name", "Unknown")
149150
executed_at = run_data.get("executed_at")
@@ -152,68 +153,72 @@ def _compute_parameters_comparison_html(self):
152153
try:
153154
dt = datetime.fromisoformat(executed_at.replace("Z", "+00:00"))
154155
date_str = dt.strftime("%b %d, %Y %H:%M")
155-
html_parts.append(f"<th>{name}<br/><small class='text-muted'>{date_str}</small></th>")
156+
html_parts.append(
157+
Markup("<th>{}<br/><small class='text-muted'>{}</small></th>").format(name, date_str)
158+
)
156159
except ValueError:
157-
html_parts.append(f"<th>{name}</th>")
160+
html_parts.append(Markup("<th>{}</th>").format(name))
158161
else:
159-
html_parts.append(f"<th>{name}</th>")
160-
html_parts.append("</tr></thead><tbody>")
162+
html_parts.append(Markup("<th>{}</th>").format(name))
163+
html_parts.append(Markup("</tr></thead><tbody>"))
161164

162165
# Target Type row
163-
html_parts.append("<tr><td><strong>Target Type</strong></td>")
166+
html_parts.append(Markup("<tr><td><strong>Target Type</strong></td>"))
164167
for run_data in runs:
165168
val = target_type_labels.get(run_data.get("target_type"), run_data.get("target_type", "-"))
166-
html_parts.append(f"<td>{val}</td>")
167-
html_parts.append("</tr>")
169+
html_parts.append(Markup("<td>{}</td>").format(val))
170+
html_parts.append(Markup("</tr>"))
168171

169172
# Targeting Expression row
170-
html_parts.append("<tr><td><strong>Targeting Expression</strong></td>")
173+
html_parts.append(Markup("<tr><td><strong>Targeting Expression</strong></td>"))
171174
for run_data in runs:
172175
expr = run_data.get("targeting_expression") or "-"
173-
html_parts.append(f"<td><code style='word-break:break-word'>{expr}</code></td>")
174-
html_parts.append("</tr>")
176+
html_parts.append(Markup("<td><code style='word-break:break-word'>{}</code></td>").format(expr))
177+
html_parts.append(Markup("</tr>"))
175178

176179
# Budget Amount row
177-
html_parts.append("<tr><td><strong>Budget Amount</strong></td>")
180+
html_parts.append(Markup("<tr><td><strong>Budget Amount</strong></td>"))
178181
for run_data in runs:
179182
amount = run_data.get("budget_amount") or 0
180-
html_parts.append(f"<td>{amount:,.2f}</td>")
181-
html_parts.append("</tr>")
183+
html_parts.append(Markup("<td>{}</td>").format(f"{amount:,.2f}"))
184+
html_parts.append(Markup("</tr>"))
182185

183186
# Budget Strategy row
184-
html_parts.append("<tr><td><strong>Budget Strategy</strong></td>")
187+
html_parts.append(Markup("<tr><td><strong>Budget Strategy</strong></td>"))
185188
for run_data in runs:
186189
strategy = budget_strategy_labels.get(
187190
run_data.get("budget_strategy"), run_data.get("budget_strategy", "-")
188191
)
189-
html_parts.append(f"<td>{strategy}</td>")
190-
html_parts.append("</tr>")
192+
html_parts.append(Markup("<td>{}</td>").format(strategy))
193+
html_parts.append(Markup("</tr>"))
191194

192195
# Entitlement Rules row
193-
html_parts.append("<tr><td><strong>Entitlement Rules</strong></td>")
196+
html_parts.append(Markup("<tr><td><strong>Entitlement Rules</strong></td>"))
194197
for run_data in runs:
195198
rules = run_data.get("entitlement_rules") or []
196199
if rules:
197-
rules_html = "<ul style='margin:0;padding-left:1rem'>"
200+
rules_parts = [Markup("<ul style='margin:0;padding-left:1rem'>")]
198201
for rule in rules:
199202
mode = rule.get("amount_mode", "fixed")
200203
amount = rule.get("amount", 0)
201204
if mode == "fixed":
202-
rules_html += f"<li>Fixed: {amount:,.2f}</li>"
205+
rules_parts.append(Markup("<li>Fixed: {}</li>").format(f"{amount:,.2f}"))
203206
elif mode == "multiplier":
204207
field = rule.get("multiplier_field", "?")
205-
rules_html += f"<li>Multiplier: {amount:,.2f} × {field}</li>"
208+
rules_parts.append(
209+
Markup("<li>Multiplier: {} \u00d7 {}</li>").format(f"{amount:,.2f}", field)
210+
)
206211
elif mode == "cel":
207212
cel = rule.get("amount_cel_expression", "?")
208-
rules_html += f"<li>CEL: <code>{cel}</code></li>"
209-
rules_html += "</ul>"
210-
html_parts.append(f"<td>{rules_html}</td>")
213+
rules_parts.append(Markup("<li>CEL: <code>{}</code></li>").format(cel))
214+
rules_parts.append(Markup("</ul>"))
215+
html_parts.append(Markup("<td>{}</td>").format(Markup("").join(rules_parts)))
211216
else:
212-
html_parts.append("<td>-</td>")
213-
html_parts.append("</tr>")
217+
html_parts.append(Markup("<td>-</td>"))
218+
html_parts.append(Markup("</tr>"))
214219

215-
html_parts.append("</tbody></table>")
216-
record.parameters_comparison_html = "".join(html_parts)
220+
html_parts.append(Markup("</tbody></table>"))
221+
record.parameters_comparison_html = Markup("").join(html_parts)
217222

218223
def action_compute_comparison(self):
219224
"""Compute the side-by-side comparison data."""

spp_simulation/models/simulation_run.py

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import logging
22

3+
from markupsafe import Markup
4+
35
from odoo import Command, _, api, fields, models
46
from odoo.exceptions import UserError
57

@@ -20,7 +22,7 @@ def _compute_display_name(self):
2022
if record.scenario_id and record.executed_at:
2123
date_str = record.executed_at.strftime("%b %d, %Y %H:%M")
2224
record.display_name = (
23-
f"{record.scenario_id.name} - {date_str} " f"({record.beneficiary_count:,} beneficiaries)"
25+
f"{record.scenario_id.name} - {date_str} ({record.beneficiary_count:,} beneficiaries)"
2426
)
2527
elif record.scenario_id:
2628
record.display_name = f"{record.scenario_id.name} - Run #{record.id}"
@@ -233,22 +235,29 @@ def _compute_summary_html(self):
233235
target_type_label = "households" if record.scenario_id.target_type == "group" else "individuals"
234236
coverage_pct = f"{record.coverage_rate:.1f}" if record.coverage_rate else "0.0"
235237
parts = [
236-
f"<strong>{scenario_name}</strong> targets "
237-
f"<strong>{record.beneficiary_count:,}</strong> {target_type_label} "
238-
f"({coverage_pct}% of registry).",
238+
Markup("<strong>{}</strong> targets <strong>{}</strong> {} ({}% of registry).").format(
239+
scenario_name,
240+
f"{record.beneficiary_count:,}",
241+
target_type_label,
242+
coverage_pct,
243+
),
239244
]
240245
if record.total_cost:
241-
parts.append(f" Total estimated cost: <strong>{record.total_cost:,.2f}</strong>.")
246+
parts.append(Markup(" Total estimated cost: <strong>{}</strong>.").format(f"{record.total_cost:,.2f}"))
242247
if record.budget_utilization and record.scenario_id.budget_amount:
243-
parts.append(f" Budget utilization: {record.budget_utilization:.1f}%.")
248+
parts.append(Markup(" Budget utilization: {}%.").format(f"{record.budget_utilization:.1f}"))
244249
if record.equity_score:
245250
parity_label = (
246251
"proportional"
247252
if record.equity_score >= 80
248253
else ("some variation" if record.equity_score >= 60 else "significant variation")
249254
)
250-
parts.append(f" Parity score: <strong>{record.equity_score:.0f}/100</strong> ({parity_label}).")
251-
record.summary_html = "<p>" + "".join(parts) + "</p>"
255+
parts.append(
256+
Markup(" Parity score: <strong>{}/100</strong> ({}).").format(
257+
f"{record.equity_score:.0f}", parity_label
258+
)
259+
)
260+
record.summary_html = Markup("<p>{}</p>").format(Markup("").join(parts))
252261

253262
@api.depends("gini_coefficient", "distribution_json")
254263
def _compute_distribution_summary_html(self):
@@ -261,7 +270,7 @@ def _compute_distribution_summary_html(self):
261270
gini_label = (
262271
"nearly equal" if gini < 0.2 else ("moderately distributed" if gini < 0.4 else "unequally distributed")
263272
)
264-
parts = [f"Benefits are <strong>{gini_label}</strong> " f"(Gini coefficient: {gini:.2f})."]
273+
parts = [f"Benefits are <strong>{gini_label}</strong> (Gini coefficient: {gini:.2f})."]
265274
minimum = distribution.get("minimum", 0)
266275
maximum = distribution.get("maximum", 0)
267276
mean = distribution.get("mean", 0)
@@ -344,10 +353,10 @@ def _compute_geographic_html(self):
344353
continue
345354

346355
html_parts = [
347-
'<table class="table table-sm table-bordered">',
348-
"<thead><tr>",
349-
"<th>Area</th><th>Beneficiaries</th><th>Amount</th><th>Coverage</th>",
350-
"</tr></thead><tbody>",
356+
Markup('<table class="table table-sm table-bordered">'),
357+
Markup("<thead><tr>"),
358+
Markup("<th>Area</th><th>Beneficiaries</th><th>Amount</th><th>Coverage</th>"),
359+
Markup("</tr></thead><tbody>"),
351360
]
352361
# Sort by beneficiary count descending
353362
sorted_areas = sorted(geo_data.items(), key=lambda x: x[1].get("count", 0), reverse=True)
@@ -357,10 +366,12 @@ def _compute_geographic_html(self):
357366
amount = area_info.get("amount", 0)
358367
coverage = area_info.get("coverage_rate", 0)
359368
html_parts.append(
360-
f"<tr><td>{name}</td><td>{count:,}</td>" f"<td>{amount:,.2f}</td><td>{coverage:.1f}%</td></tr>"
369+
Markup("<tr><td>{}</td><td>{}</td><td>{}</td><td>{}%</td></tr>").format(
370+
name, f"{count:,}", f"{amount:,.2f}", f"{coverage:.1f}"
371+
)
361372
)
362-
html_parts.append("</tbody></table>")
363-
record.geographic_html = "".join(html_parts)
373+
html_parts.append(Markup("</tbody></table>"))
374+
record.geographic_html = Markup("").join(html_parts)
364375

365376
@api.depends("metric_results_json")
366377
def _compute_metric_results_html(self):
@@ -375,10 +386,10 @@ def _compute_metric_results_html(self):
375386
continue
376387

377388
html_parts = [
378-
'<table class="table table-sm table-bordered">',
379-
"<thead><tr>",
380-
"<th>Metric</th><th>Value</th><th>Type</th>",
381-
"</tr></thead><tbody>",
389+
Markup('<table class="table table-sm table-bordered">'),
390+
Markup("<thead><tr>"),
391+
Markup("<th>Metric</th><th>Value</th><th>Type</th>"),
392+
Markup("</tr></thead><tbody>"),
382393
]
383394
for metric_name, metric_data in metrics.items():
384395
value = metric_data.get("value", 0)
@@ -392,9 +403,13 @@ def _compute_metric_results_html(self):
392403
formatted_value = f"{value:,.2f}"
393404
else:
394405
formatted_value = str(value)
395-
html_parts.append(f"<tr><td>{metric_name}</td><td>{formatted_value}</td><td>{metric_type}</td></tr>")
396-
html_parts.append("</tbody></table>")
397-
record.metric_results_html = "".join(html_parts)
406+
html_parts.append(
407+
Markup("<tr><td>{}</td><td>{}</td><td>{}</td></tr>").format(
408+
metric_name, formatted_value, metric_type
409+
)
410+
)
411+
html_parts.append(Markup("</tbody></table>"))
412+
record.metric_results_html = Markup("").join(html_parts)
398413

399414
@api.depends("targeting_efficiency_json")
400415
def _compute_targeting_efficiency_html(self):
@@ -474,11 +489,11 @@ def _compute_scenario_snapshot_fields(self):
474489
rules = snapshot.get("entitlement_rules") or []
475490
if rules:
476491
html_parts = [
477-
'<table class="table table-sm table-bordered">',
478-
"<thead><tr>",
479-
"<th>Mode</th><th>Amount</th><th>Multiplier Field</th><th>Max Multiplier</th>",
480-
"<th>CEL Expression</th><th>Condition</th>",
481-
"</tr></thead><tbody>",
492+
Markup('<table class="table table-sm table-bordered">'),
493+
Markup("<thead><tr>"),
494+
Markup("<th>Mode</th><th>Amount</th><th>Multiplier Field</th><th>Max Multiplier</th>"),
495+
Markup("<th>CEL Expression</th><th>Condition</th>"),
496+
Markup("</tr></thead><tbody>"),
482497
]
483498
amount_mode_labels = {
484499
"fixed": "Fixed",
@@ -493,12 +508,21 @@ def _compute_scenario_snapshot_fields(self):
493508
cel_expr = rule.get("amount_cel_expression") or "-"
494509
condition = rule.get("condition_cel_expression") or "-"
495510
html_parts.append(
496-
f"<tr><td>{mode}</td><td>{amount:,.2f}</td><td>{mult_field}</td>"
497-
f"<td>{max_mult}</td><td><code>{cel_expr}</code></td>"
498-
f"<td><code>{condition}</code></td></tr>"
511+
Markup(
512+
"<tr><td>{}</td><td>{}</td><td>{}</td>"
513+
"<td>{}</td><td><code>{}</code></td>"
514+
"<td><code>{}</code></td></tr>"
515+
).format(
516+
mode,
517+
f"{amount:,.2f}",
518+
mult_field,
519+
max_mult,
520+
cel_expr,
521+
condition,
522+
)
499523
)
500-
html_parts.append("</tbody></table>")
501-
record.scenario_snapshot_entitlement_rules_html = "".join(html_parts)
524+
html_parts.append(Markup("</tbody></table>"))
525+
record.scenario_snapshot_entitlement_rules_html = Markup("").join(html_parts)
502526
else:
503527
record.scenario_snapshot_entitlement_rules_html = (
504528
"<p class='text-muted'>No entitlement rules defined</p>"

0 commit comments

Comments
 (0)