@@ -193,9 +207,8 @@
Edit User: {{ user.username
-{% endfor %}
-
+
+
{% endblock %}
diff --git a/tests/smoke_test.py b/tests/smoke_test.py
index 3f853bae..2b80ed9b 100644
--- a/tests/smoke_test.py
+++ b/tests/smoke_test.py
@@ -1919,6 +1919,38 @@ def _banner_tag(html):
finally:
_am._cron_restart_pending.pop(gs_id, None)
+ # ── The users page renders ONE edit modal, not one per user ───────────────────────────────────
+ # It used to emit a full 2KB modal per row — 670KB of HTML at 100 accounts, all of it for a
+ # dialog you can only have open once. The rows now carry an id and the data comes from a single
+ # JSON island. These assert the shape holds AND that the data is actually right, because a
+ # smaller page that opens the wrong user's details would be a much worse bug than a big one.
+ import json as _json_u
+ _uh = c.get("/users").get_data(as_text=True)
+ check("users page: exactly one edit modal, however many accounts exist",
+ _uh.count('id="editUserModal"') == 1 and 'id="editUserModal-' not in _uh,
+ "found %d" % _uh.count('id="editUserModal'))
+ _isl = _re_ab.search(r'',
+ _uh, _re_ab.S)
+ check("users page: the JSON island is present", _isl is not None)
+ if _isl:
+ _rows = _json_u.loads(_isl.group(1)) # must be VALID json, not just present
+ with app.app_context():
+ # Materialise inside the context: .groups is a lazy relationship and reading it after
+ # the context closes raises DetachedInstanceError.
+ _want = {u.username: (bool(u.is_superadmin), bool(u.is_active),
+ sorted(g.id for g in u.groups)) for u in User.query.all()}
+ check("users page: one island entry per account", len(_rows) == len(_want),
+ "%d rows vs %d users" % (len(_rows), len(_want)))
+ _bad = [r["username"] for r in _rows
+ if (r["is_superadmin"], r["is_active"], sorted(r["groups"])) != _want[r["username"]]]
+ check("users page: each entry matches that account's real flags and groups", not _bad,
+ "wrong: %s" % _bad[:3])
+ check("users page: every row's Edit button opens the shared modal by id",
+ _uh.count('data-action="openEditUser"') == len(_rows),
+ "%d buttons for %d users" % (_uh.count('data-action="openEditUser"'), len(_rows)))
+ check("users page: no password is ever put in the island",
+ not any("password" in r for r in _rows))
+
# ── Bearer API tokens: the other way into every route ─────────────────────────────────────────
# A token authenticates AS its owner and inherits exactly that user's RBAC, and app.py exempts
# Bearer requests from CSRF — so this is a full authentication path that had no test at all.
@@ -2020,6 +2052,12 @@ def _bearer(tok):
check("custom command: a missing command is refused, not an exception",
_crcc(_adm, None, _gs) is False)
+except Exception:
+ # A crash part-way through otherwise just prints fewer checks and still reads as green-ish.
+ # That has hidden three separate mistakes while writing these; a crash is a FAILURE.
+ import traceback as _tb
+ _tb.print_exc()
+ results.append((False, "suite crashed before finishing — see the traceback above", ""))
finally:
passed = sum(1 for ok, _, _ in results if ok)
for ok, name, detail in results:
diff --git a/tests/template_actions_test.py b/tests/template_actions_test.py
index 95f6004c..3eecd2da 100644
--- a/tests/template_actions_test.py
+++ b/tests/template_actions_test.py
@@ -46,7 +46,12 @@ def check(cond, name, detail=""):
# fine and looks harmless. Static analysers read the TEMPLATE, though, and to a JS parser "{#" is
# a private-field sigil: CodeQL raised two js/syntax-error alerts on exactly this. A whole file
# that fails to parse is a file nothing is checking, which is the real cost.
-_INLINE_SCRIPT = re.compile(r"
]*\bsrc=)[^>]*>(.*?)", re.S)
+# Only script elements a JS parser actually reads: no src=, and either no type or a JS one.
+# A ", re.S)
_jinja_in_js = []
for _name, _src in srcs.items():
if not _name.endswith(".html"):
From f62f0a5f52ec007a12fe99553ffa2345f96b38e3 Mon Sep 17 00:00:00 2001
From: FMSMITH91 <12152698+FMSMITH91@users.noreply.github.com>
Date: Sun, 9 Aug 2026 12:05:10 -0500
Subject: [PATCH 2/2] fix: coerce the user id before it reaches the form action
(js/xss-through-dom)
CodeQL flagged the id flowing from DOM text into a form action, rated high
because an action is a URL sink. The value is always an integer from our own
database, but it arrives as text read out of the DOM and nothing said so.
parseInt proves it cannot carry meta-characters and rejects a tampered island
outright.
---
static/js/manage_users.js | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/static/js/manage_users.js b/static/js/manage_users.js
index 4b7ffada..9a2203f1 100644
--- a/static/js/manage_users.js
+++ b/static/js/manage_users.js
@@ -20,9 +20,15 @@ window.openEditUser = function (id) {
}
if (!u) return;
+ // Coerce the id to a number before it reaches the form action. It is always an integer from our
+ // own database, but it arrives here as text read out of the DOM, and a form action is a URL sink
+ // — CodeQL flags that flow (js/xss-through-dom) and is right to. parseInt both proves the value
+ // cannot carry meta-characters and rejects a tampered island outright.
+ var uid = parseInt(u.id, 10);
+ if (!(uid > 0)) return;
+
var form = document.getElementById('edit-user-form');
- // The action is per-user, so it is set here rather than rendered N times.
- form.setAttribute('action', (window.MOUNT || '') + '/users/' + u.id + '/edit');
+ form.setAttribute('action', (window.MOUNT || '') + '/users/' + uid + '/edit');
document.getElementById('eu-name').textContent = u.username; // textContent: never HTML
document.getElementById('eu-display').value = u.display_name || '';