From f47fc232659b912a5c06b223739395b66a17715e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 14:30:13 -0400 Subject: [PATCH] fix(ui): delete prompts use data-confirm, so a quote in a name no longer runs script (#5099) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5099 "A name containing a quote runs script when an admin clicks Delete". Inline handlers like onclick="return confirm('Delete {{ rule.name }}?')" are HTML-decoded before the JavaScript is parsed, so autoescaping did not stop a name such as x');fetch(…);// from running. - Every prompt (22 handlers, plus the docker prune and token revoke ones) is now data-confirm="…", read through dataset by one submit listener in base.html; nothing rendered is parsed as JavaScript. - The remaining inline handlers take integers only (|int), and the time range buttons pass this.dataset.range. - tests/test_inline_handlers.py fails on any {{ }} in an inline handler that is not |int, with a self-test; tests/_templates.py is the one template walker the template guards now share. Co-Authored-By: Claude Opus 5.5 --- plugins/docker/templates/docker/disk.html | 9 ++-- plugins/host_agent/templates/panel.html | 6 +-- .../host_agent/templates/settings_list.html | 4 +- .../host_agent/templates/widget_history.html | 2 +- steward/templates/_time_range.html | 2 +- steward/templates/ai_access/index.html | 2 +- steward/templates/alerts/list.html | 2 +- steward/templates/alerts/maintenance.html | 2 +- steward/templates/ansible/browse.html | 2 +- .../templates/ansible/inventory/groups.html | 2 +- .../templates/ansible/inventory/targets.html | 2 +- .../templates/ansible/playbook_editor.html | 2 +- steward/templates/ansible/run_detail.html | 2 +- steward/templates/ansible/schedules.html | 2 +- steward/templates/base.html | 7 +++ steward/templates/dashboard/list.html | 2 +- steward/templates/dashboard/share.html | 2 +- steward/templates/hosts/detail.html | 4 +- steward/templates/hosts/list.html | 2 +- steward/templates/monitors/rows.html | 2 +- .../templates/settings/_ansible_sources.html | 6 +-- steward/templates/settings/_plugin_repos.html | 4 +- steward/templates/settings/ansible.html | 2 +- steward/templates/settings/plugins.html | 4 +- tests/_templates.py | 12 ++++++ tests/core/test_formatting.py | 9 +--- tests/test_inline_handlers.py | 43 +++++++++++++++++++ tests/test_style_tokens.py | 6 +-- 28 files changed, 97 insertions(+), 49 deletions(-) create mode 100644 tests/_templates.py create mode 100644 tests/test_inline_handlers.py diff --git a/plugins/docker/templates/docker/disk.html b/plugins/docker/templates/docker/disk.html index 806b68f..5b007c0 100644 --- a/plugins/docker/templates/docker/disk.html +++ b/plugins/docker/templates/docker/disk.html @@ -53,20 +53,17 @@ {% if ansible_available and g.has_target %}
{{ csrf_field() }} + data-confirm="Remove all STOPPED containers on {{ g.host_name }}? This cannot be undone.">{{ csrf_field() }}
{{ csrf_field() }} + data-confirm="Remove ALL unused images on {{ g.host_name }} (docker image prune -a)? Any image not used by a container is deleted.">{{ csrf_field() }}
{{ csrf_field() }} + data-confirm="Run a full system prune on {{ g.host_name }}? Removes stopped containers, unused networks, dangling images and build cache.">{{ csrf_field() }}
diff --git a/plugins/host_agent/templates/panel.html b/plugins/host_agent/templates/panel.html index 7223e32..d106e52 100644 --- a/plugins/host_agent/templates/panel.html +++ b/plugins/host_agent/templates/panel.html @@ -26,11 +26,11 @@ {% endif %}
{{ csrf_field() }} + data-confirm="Rotate token? The agent stops reporting until reinstalled/updated with the new token.">{{ csrf_field() }}
{{ csrf_field() }} + data-confirm="Remove this agent registration? Metrics stop until re-registered.">{{ csrf_field() }}
{% endif %} @@ -138,7 +138,7 @@ {% if reg and session.user_role == 'admin' %}
{{ csrf_field() }} + data-confirm="Clear this pending registration? Its token is discarded.">{{ csrf_field() }}
{% endif %} diff --git a/plugins/host_agent/templates/settings_list.html b/plugins/host_agent/templates/settings_list.html index 6c2ad0d..ce05d4f 100644 --- a/plugins/host_agent/templates/settings_list.html +++ b/plugins/host_agent/templates/settings_list.html @@ -156,12 +156,12 @@
{{ csrf_field() }} + data-confirm="Rotate the token for this host? The existing agent will stop working until reinstalled with the new token.">{{ csrf_field() }}
{{ csrf_field() }} + data-confirm="Delete this host registration? The agent will be unable to report metrics until re-registered.">{{ csrf_field() }}
diff --git a/plugins/host_agent/templates/widget_history.html b/plugins/host_agent/templates/widget_history.html index 01e7c5b..9e7c3bc 100644 --- a/plugins/host_agent/templates/widget_history.html +++ b/plugins/host_agent/templates/widget_history.html @@ -22,7 +22,7 @@ {% endif %} {% for opt in [1, 6, 24] %} - diff --git a/steward/templates/_time_range.html b/steward/templates/_time_range.html index f950885..2ea0119 100644 --- a/steward/templates/_time_range.html +++ b/steward/templates/_time_range.html @@ -10,6 +10,6 @@ + onclick="setTimeRange(this.dataset.range)">{{ key }} {% endfor %}
diff --git a/steward/templates/ai_access/index.html b/steward/templates/ai_access/index.html index cbe2175..777d700 100644 --- a/steward/templates/ai_access/index.html +++ b/steward/templates/ai_access/index.html @@ -59,7 +59,7 @@ {% if row.state == 'active' %}
{{ csrf_field() }} + data-confirm="Revoke “{{ t.name }}”? Any client using it stops working immediately.">{{ csrf_field() }}
{% endif %} diff --git a/steward/templates/alerts/list.html b/steward/templates/alerts/list.html index afa53fd..0f09d30 100644 --- a/steward/templates/alerts/list.html +++ b/steward/templates/alerts/list.html @@ -99,7 +99,7 @@
{{ csrf_field() }} + data-confirm="Delete rule {{ rule.name }}?">Delete
diff --git a/steward/templates/alerts/maintenance.html b/steward/templates/alerts/maintenance.html index f2b941f..32bf36e 100644 --- a/steward/templates/alerts/maintenance.html +++ b/steward/templates/alerts/maintenance.html @@ -62,7 +62,7 @@
{{ csrf_field() }} + data-confirm="Delete window "{{ w.name }}"?">Delete
diff --git a/steward/templates/ansible/browse.html b/steward/templates/ansible/browse.html index 96aa26d..de61c3b 100644 --- a/steward/templates/ansible/browse.html +++ b/steward/templates/ansible/browse.html @@ -28,7 +28,7 @@ {% if view_editable and session.user_role == 'admin' %} Edit
{{ csrf_field() }} + data-confirm="Delete {{ view_path }}?">{{ csrf_field() }} diff --git a/steward/templates/ansible/inventory/groups.html b/steward/templates/ansible/inventory/groups.html index 7288bbd..c271db6 100644 --- a/steward/templates/ansible/inventory/groups.html +++ b/steward/templates/ansible/inventory/groups.html @@ -40,7 +40,7 @@ Edit {{ csrf_field() }} + data-confirm="Delete group {{ grp.name }}?">×
diff --git a/steward/templates/ansible/inventory/targets.html b/steward/templates/ansible/inventory/targets.html index 874563f..a909aee 100644 --- a/steward/templates/ansible/inventory/targets.html +++ b/steward/templates/ansible/inventory/targets.html @@ -49,7 +49,7 @@ Edit
{{ csrf_field() }} + data-confirm="Delete target {{ tgt.name }}?">×
diff --git a/steward/templates/ansible/playbook_editor.html b/steward/templates/ansible/playbook_editor.html index a12930e..bcb05c1 100644 --- a/steward/templates/ansible/playbook_editor.html +++ b/steward/templates/ansible/playbook_editor.html @@ -54,7 +54,7 @@ {% if editing %}
{{ csrf_field() }} + data-confirm="Delete {{ playbook_path }}?">{{ csrf_field() }} diff --git a/steward/templates/ansible/run_detail.html b/steward/templates/ansible/run_detail.html index 6ff9ce8..8e1e93b 100644 --- a/steward/templates/ansible/run_detail.html +++ b/steward/templates/ansible/run_detail.html @@ -7,7 +7,7 @@

Run Detail

{% if run.status.value in ("running", "queued") and session.user_role in ("operator", "admin") %} - {{ csrf_field() }} + {{ csrf_field() }} {% endif %} diff --git a/steward/templates/ansible/schedules.html b/steward/templates/ansible/schedules.html index e49586d..446b9c6 100644 --- a/steward/templates/ansible/schedules.html +++ b/steward/templates/ansible/schedules.html @@ -62,7 +62,7 @@ Edit
{{ csrf_field() }} + data-confirm="Delete schedule {{ s.name }}?">{{ csrf_field() }}
diff --git a/steward/templates/base.html b/steward/templates/base.html index 24f8ad2..c655012 100644 --- a/steward/templates/base.html +++ b/steward/templates/base.html @@ -10,6 +10,13 @@ document.addEventListener('htmx:configRequest', function (ev) { ev.detail.headers['X-CSRF-Token'] = document.querySelector('meta[name="csrf-token"]').content; }); + // A form (or its submit button) with data-confirm="…" asks before it + // submits. The prompt is attribute text read through dataset, never + // parsed as JavaScript, so a name inside it cannot run as script (#5099). + document.addEventListener('submit', function (ev) { + var ask = (ev.submitter && ev.submitter.dataset.confirm) || ev.target.dataset.confirm; + if (ask && !window.confirm(ask)) { ev.preventDefault(); ev.stopPropagation(); } + }, true); {% block title %}Steward{% endblock %} {# Every asset is served by Steward itself: it runs with no outbound network (rule 164). #} diff --git a/steward/templates/dashboard/list.html b/steward/templates/dashboard/list.html index 5019382..f174b47 100644 --- a/steward/templates/dashboard/list.html +++ b/steward/templates/dashboard/list.html @@ -57,7 +57,7 @@ {% if my_dashboards | length > 1 %}
{{ csrf_field() }} + data-confirm="Delete '{{ dash.name }}'?">{{ csrf_field() }}
{% endif %} diff --git a/steward/templates/dashboard/share.html b/steward/templates/dashboard/share.html index 3bc69a7..4236f1a 100644 --- a/steward/templates/dashboard/share.html +++ b/steward/templates/dashboard/share.html @@ -32,7 +32,7 @@
{{ csrf_field() }} + style="margin:0;" data-confirm="Revoke this share link?">{{ csrf_field() }}
diff --git a/steward/templates/hosts/detail.html b/steward/templates/hosts/detail.html index 1ecb133..6c42415 100644 --- a/steward/templates/hosts/detail.html +++ b/steward/templates/hosts/detail.html @@ -16,7 +16,7 @@ Edit {% if session.user_role == 'admin' %}
{{ csrf_field() }} + data-confirm="Delete host {{ host.name }}? Monitors and history are removed.">{{ csrf_field() }}
{% endif %} @@ -78,7 +78,7 @@
Edit
{{ csrf_field() }} + data-confirm="Delete monitor {{ m.name }}?">{{ csrf_field() }}
diff --git a/steward/templates/hosts/list.html b/steward/templates/hosts/list.html index 989d8d9..a239ed7 100644 --- a/steward/templates/hosts/list.html +++ b/steward/templates/hosts/list.html @@ -80,7 +80,7 @@ Edit
{{ csrf_field() }} + data-confirm="Delete {{ host.name }}?">Delete
diff --git a/steward/templates/monitors/rows.html b/steward/templates/monitors/rows.html index 5a2d3a2..cba82af 100644 --- a/steward/templates/monitors/rows.html +++ b/steward/templates/monitors/rows.html @@ -59,7 +59,7 @@ {{ "Pause" if m.enabled else "Resume" }}
{{ csrf_field() }} + data-confirm="Delete monitor {{ m.name }}?">{{ csrf_field() }}
diff --git a/steward/templates/settings/_ansible_sources.html b/steward/templates/settings/_ansible_sources.html index 6a449aa..9f80c9f 100644 --- a/steward/templates/settings/_ansible_sources.html +++ b/steward/templates/settings/_ansible_sources.html @@ -46,7 +46,7 @@ {% endif %} + onclick="toggleAnsibleEdit({{ idx|int }})">Edit + onclick="toggleAnsibleEdit({{ idx|int }})">Cancel diff --git a/steward/templates/settings/_plugin_repos.html b/steward/templates/settings/_plugin_repos.html index 6ad83f7..78453d1 100644 --- a/steward/templates/settings/_plugin_repos.html +++ b/steward/templates/settings/_plugin_repos.html @@ -18,7 +18,7 @@
+ onclick="toggleRepoEdit({{ idx|int }})">Edit + onclick="toggleRepoEdit({{ idx|int }})">Cancel
diff --git a/steward/templates/settings/ansible.html b/steward/templates/settings/ansible.html index 24572f2..48990f8 100644 --- a/steward/templates/settings/ansible.html +++ b/steward/templates/settings/ansible.html @@ -39,7 +39,7 @@ Click to select — add to a host's authorized_keys to authorize Steward manually.
{{ csrf_field() }} + data-confirm="Regenerate the managed key? Already-provisioned hosts will reject Steward until you re-provision them.">{{ csrf_field() }}
{% else %} diff --git a/steward/templates/settings/plugins.html b/steward/templates/settings/plugins.html index 1ccba46..f6a32b0 100644 --- a/steward/templates/settings/plugins.html +++ b/steward/templates/settings/plugins.html @@ -54,7 +54,7 @@
Monitoring capabilities
{{ csrf_field() }}
@@ -134,7 +134,7 @@ {% if orphan.removable %}
{{ csrf_field() }}
diff --git a/tests/_templates.py b/tests/_templates.py new file mode 100644 index 0000000..70eb1bb --- /dev/null +++ b/tests/_templates.py @@ -0,0 +1,12 @@ +"""Where Steward's templates live, for the guards that read every one.""" +import pathlib + +import steward + +REPO = pathlib.Path(steward.__file__).parent.parent + + +def template_files() -> list[pathlib.Path]: + """Every first-party .html template: core and each plugin's.""" + roots = [REPO / "steward" / "templates", *(REPO / "plugins").glob("*/templates")] + return sorted(p for root in roots for p in root.rglob("*.html")) diff --git a/tests/core/test_formatting.py b/tests/core/test_formatting.py index 3300ac4..5792924 100644 --- a/tests/core/test_formatting.py +++ b/tests/core/test_formatting.py @@ -1,16 +1,14 @@ """The shared format filters and status_class (#5093): one way to write a size, a rate, a span and a moment, and one grading against Settings → Thresholds.""" -import pathlib from datetime import datetime, timedelta, timezone import jinja2 import pytest -import steward from steward.core import formatting as f from steward.core.settings import threshold_style_for, to_thresholds_cfg +from tests._templates import REPO as _REPO, template_files as _templates -_REPO = pathlib.Path(steward.__file__).parent.parent TH = to_thresholds_cfg({}) NOW = datetime(2026, 10, 6, 18, 0, 0, tzinfo=timezone.utc) @@ -88,11 +86,6 @@ def test_threshold_style_is_graded_by_status_class(): # ── every template compiles against the app's filters ──────────────────────── -def _templates(): - roots = [_REPO / "steward" / "templates", *(_REPO / "plugins").glob("*/templates")] - return sorted(p for root in roots for p in root.rglob("*.html")) - - async def test_compiling_catches_an_unknown_filter(app): with pytest.raises(jinja2.TemplateAssertionError, match="no_such"): app.jinja_env.compile("{{ 1|no_such }}") diff --git a/tests/test_inline_handlers.py b/tests/test_inline_handlers.py new file mode 100644 index 0000000..8d40b97 --- /dev/null +++ b/tests/test_inline_handlers.py @@ -0,0 +1,43 @@ +"""No template puts a rendered value into inline JavaScript (#5099). + +An onclick/onsubmit attribute is HTML-decoded before its JavaScript is +parsed, so autoescaping does not protect a name placed inside it: a monitor +called x');fetch(…);// ran as script when an admin clicked Delete. A +prompt goes in data-confirm="…" (base.html asks with it); any other value a +handler needs is an integer, rendered with |int. +""" +import re + +from tests._templates import REPO as _REPO, template_files as _templates + +_HANDLER = re.compile(r"""\bon[a-z]+\s*=\s*("[^"]*"|'[^']*')""", re.I) +_VALUE = re.compile(r"\{\{(.*?)\}\}", re.S) + + +def unsafe_handler_values(text: str) -> list[str]: + """Each {{ … }} inside an inline handler that is not an |int.""" + return [m.group(0) for h in _HANDLER.finditer(text) for m in _VALUE.finditer(h.group(1)) + if not re.search(r"\|\s*int\s*$", m.group(1).strip())] + + +def test_the_guard_can_fail(): + page = """ + +
+ + + """ + assert unsafe_handler_values(page) == ["{{ rule.name }}", "{{ msg|tojson }}"] + + +def test_no_template_renders_a_value_into_an_inline_handler(): + files = _templates() + assert len(files) > 100 + found = {str(p.relative_to(_REPO)): hits for p in files if (hits := unsafe_handler_values(p.read_text()))} + assert not found, f"use data-confirm, or render an integer with |int: {found}" + + +def test_base_asks_before_a_data_confirm_form_submits(): + base = (_REPO / "steward" / "templates" / "base.html").read_text() + assert "dataset.confirm" in base and "window.confirm(ask)" in base + assert any("data-confirm=" in p.read_text() for p in _templates()) diff --git a/tests/test_style_tokens.py b/tests/test_style_tokens.py index 09bad2f..05865e8 100644 --- a/tests/test_style_tokens.py +++ b/tests/test_style_tokens.py @@ -11,6 +11,7 @@ import pathlib import re import steward +from tests._templates import template_files as _templates _REPO = pathlib.Path(steward.__file__).parent.parent _STATIC = _REPO / "steward" / "static" @@ -24,11 +25,6 @@ _HEX = re.compile(r"""(?:[:(,]\s*|(?