Webapp redesign: fixes from the code review

Blocker
- /reports/export returned 500: REPORT_EXPORT_RANGES and _months_ago were
  deleted with the old reports route. Restored.

Major
- A session without stage_index/stage_total took down /, /imaging and
  /imaging/tiles (int filter on Undefined). The templates default both;
  saving a LAPS password for a session that no longer exists is refused
  (it created a bare session).
- /reports/todo parsed every report four times (23 s at 700 reports).
  blancco_report.parse() now caches by path, mtime and size: 0.09 s.
- Stored XSS through the session serial in the Clear confirm (the
  browser decodes ' before running an inline handler). The message
  moves to data-confirm, and update_session() only accepts serials of
  letters, digits, '.', '_' and '-' (400 from /imaging/status otherwise).
- Reports without a document id and with odd characters in the file name
  got a key the mark routes rejected, so they could never be cleared.
  report_key() now always produces a valid key.

Minor
- failure_note() handles a list-valued erasure target (500 on the report
  and to-do pages).
- _recently_marked() tolerates naive, 'Z' and non-dict state entries.
- Key regexes use fullmatch (a trailing newline was accepted).
- Unreadable reports are no longer counted as to tick / to destroy; the
  reports page already flags them as Unreadable.
- Rollback review recounts restorable files after reading the backups.
- Enrollment: the startnet fallback package (no or empty ppkg.conf) and a
  quoted SOURCE_PPKG are protected too.
- Backup upload overwrite check is case-insensitive (SMB share) and
  replaces the existing file rather than adding a case twin.
- Overview polling keeps the last good rows on an error and pauses while
  the tab is hidden. A shown or typed LAPS password survives the floor's
  refresh.
- hardware_dismiss only redirects to same-site paths; tiles ?limit is
  clamped; the Updates overview survives an unreadable Dell scan or iPXE
  state; describe() uses aversion(); a client cannot label its session
  'inferred'.

webapp/app.py

@@ -135,18 +135,24 @@ def updates_overview():
     """One card per updater (Dell, iPXE, Blancco, BPRT) and the Dell applies
     with their rollback state. Each card links to its own page."""
     from services import blancco_update, bprt_sync, dell_updates, ipxe_update, updates_common
-    scan = dell_updates.last_scan()
+    try:
+        scan = dell_updates.last_scan()
+    except Exception:  # unreadable scan.json: show "not checked" rather than a 500
+        scan = None
     dell = {"checked": scan.get("checked") if scan else None,
             "drivers": len(scan.get("drivers") or []) if scan else 0,
             "bios": len(scan.get("bios") or []) if scan else 0,
             "drv_updates": sum(1 for r in (scan.get("drivers") or []) if r.get("status") == "update") if scan else 0,
             "bios_updates": sum(1 for r in (scan.get("bios") or []) if r.get("status") == "update") if scan else 0}
-    rows = ipxe_update.installed()
+    try:
-    by_name = {r["name"]: r["info"] for r in rows}
+        rows = ipxe_update.installed()
-    latest = ipxe_update.last_check()
+        by_name = {r["name"]: r["info"] for r in rows}
-    efi = by_name.get("ipxe.efi") or {}
+        latest = ipxe_update.last_check()
-    ipxe = {"rows": rows, "version": efi.get("version"), "latest": latest,
+        efi = by_name.get("ipxe.efi") or {}
-            "status": ipxe_update.compare(latest["tag"], efi.get("version")) if latest else None}
+        ipxe = {"rows": rows, "version": efi.get("version"), "latest": latest,
+                "status": ipxe_update.compare(latest["tag"], efi.get("version")) if latest else None}
+    except Exception as ex:
+        ipxe = {"rows": [], "version": None, "latest": None, "status": None, "error": str(ex)}
     try:
         blancco = {"current": blancco_update.current(), "staged": blancco_update.staged()}
     except Exception as ex:  # a broken release folder must not take the page down
@@ -561,9 +567,13 @@ def clonezilla_upload():
     os.makedirs(config.CLONEZILLA_SHARE, exist_ok=True)
     dest = os.path.join(config.CLONEZILLA_SHARE, filename)
     # A backup is a whole disk image: never replace one by accident.
-    if os.path.exists(dest) and request.form.get("overwrite") != "1":
+    # The share is SMB: names differ only in case are the same file to Windows.
-        flash(f"{filename} already exists. Tick Replace the existing backup to overwrite it.", "danger")
+    clash = next((n for n in os.listdir(config.CLONEZILLA_SHARE) if n.lower() == filename.lower()), None)
+    if clash and request.form.get("overwrite") != "1":
+        flash(f"{clash} already exists. Tick Replace the existing backup to overwrite it.", "danger")
         return redirect(url_for("clonezilla_backups"))
+    if clash:
+        dest = os.path.join(config.CLONEZILLA_SHARE, clash)  # replace it, never a case twin
     f.save(dest)
     audit("BACKUP_UPLOAD", filename)
     flash(f"Uploaded {filename} successfully.", "success")
@@ -688,6 +698,17 @@ def blancco_reports():
     )
 
 
+REPORT_EXPORT_RANGES = {"1m": 1, "3m": 3, "6m": 6, "12m": 12}
+
+
+def _months_ago(now, months):
+    # Same day-of-month N months back, clamped (Mar 31 - 1m -> Feb 28/29).
+    import calendar
+    y, m = divmod(now.year * 12 + now.month - 1 - months, 12)
+    day = min(now.day, calendar.monthrange(y, m + 1)[1])
+    return now.replace(year=y, month=m + 1, day=day)
+
+
 @app.route("/reports/export")
 def blancco_export_reports():
     import csv
@@ -810,10 +831,14 @@ def _recently_marked(state, year_items, hours=24):
     for kind in blancco_evidence.TODO_KINDS:
         rows = []
         for key, rec in (state.get(kind) or {}).items():
+            if not isinstance(rec, dict):
+                continue  # hand-edited state: nothing to undo
             try:
-                ts = datetime.fromisoformat(str((rec or {}).get("ts")))
+                ts = datetime.fromisoformat(str(rec.get("ts")).replace("Z", "+00:00"))
-            except ValueError:
+            except (ValueError, TypeError):
                 continue
+            if ts.tzinfo is None:
+                ts = ts.replace(tzinfo=timezone.utc)
             if ts < cutoff:
                 continue
             it = by_key.get(key)
@@ -895,7 +920,11 @@ def hardware_dismiss():
     except ValueError:
         abort(400)
     audit("HARDWARE_DISMISS", f"{model} {'dismissed' if done else 'reopened'}")
-    return redirect(request.form.get("next") or url_for("hardware_page"))
+    nxt = request.form.get("next") or ""
+    # Only same-site paths ("/x", not "//host" or "https://...").
+    if not (nxt.startswith("/") and not nxt.startswith("//") and "\\" not in nxt):
+        nxt = url_for("hardware_page")
+    return redirect(nxt)
 
 
 @app.route("/reports/download/<filename>")
@@ -978,7 +1007,7 @@ def imaging_tiles_partial():
         sessions, _hidden = imaging_status.hide_stale(sessions)
     sessions = imaging_status.floor_order(sessions)
     # ?limit=N (the Overview): the first N PCs, log-only bays left out.
-    limit = request.args.get("limit", type=int)
+    limit = max(0, request.args.get("limit", 0, type=int) or 0)
     more = 0
     if limit:
         real = [x for x in sessions if x["floor_state"] != "logs"]
@@ -1055,6 +1084,8 @@ def imaging_status_post():
         return jsonify({"error": "missing serial"}), 400
     try:
         state = imaging_status.update_session(payload)
+    except ValueError as ex:
+        return jsonify({"error": str(ex)}), 400
     except Exception as ex:
         audit("IMAGING_STATUS_ERROR", str(ex))
         return jsonify({"error": str(ex)}), 500
@@ -1143,6 +1174,10 @@ def imaging_set_laps(serial):
                     except OSError: pass
                     raise
         return {"ok": True, "cleared": True}
+    # Only for a bay that exists: saving for a cleared session would create a
+    # bare session file with no stage fields.
+    if not os.path.isfile(imaging_status._path_for(serial)):
+        return {"ok": False, "error": "no such session"}, 404
     imaging_status.update_session({"serial": serial, "laps_password": pw})
     return {"ok": True}
 
@@ -1202,6 +1237,7 @@ def enrollment():
         mcl_archive=mcl_ppkg.archived(),
         mcl_links=mcl_ppkg.image_links(),
         active_ppkg=read_ppkg_conf(),
+        effective_ppkg=(read_ppkg_conf().get("SOURCE_PPKG", "").strip().strip('"').strip() or STARTNET_DEFAULT_PPKG),
         image_types=config.IMAGE_TYPES,
         friendly_names=config.FRIENDLY_NAMES,
     )
@@ -1313,12 +1349,18 @@ def enrollment_download(filename):
     return send_file(fpath, as_attachment=True)
 
 
+# The package startnet.cmd uses when ppkg.conf is missing or names none
+# (playbook/startnet.cmd: set SOURCE_PPKG=...).
+STARTNET_DEFAULT_PPKG = "GCCH_Prod_SFLD_v4.16.ppkg"
+
+
 @app.route("/enrollment/delete/<filename>", methods=["POST"])
 def enrollment_delete(filename):
     filename = secure_filename(filename)
     fpath = os.path.join(config.ENROLLMENT_PPKG_DIR, filename)
-    active = read_ppkg_conf().get("SOURCE_PPKG", "")
+    # startnet.cmd falls back to this package when ppkg.conf names none.
-    if active and filename.lower() == active.lower():
+    active = read_ppkg_conf().get("SOURCE_PPKG", "").strip().strip('"').strip() or STARTNET_DEFAULT_PPKG
+    if filename.lower() == active.lower():
         # startnet.cmd would keep pointing at a file that no longer exists.
         flash(f"{filename} is the active package. Upload or activate another one before deleting it.",
               "danger")

webapp/services/blancco_evidence.py

@@ -38,7 +38,11 @@ _REPORT_KEY = re.compile(f"{_DOC_ID.pattern}|{_FILE_KEY.pattern}")
 
 def report_key(document_id: str, filename: str) -> str:
     """The state-file key for one report: its document id, else "file:<name>"."""
-    return document_id if _DOC_ID.match(document_id or "") else f"file:{filename}"
+    if _DOC_ID.fullmatch(document_id or ""):
+        return document_id
+    # Keys travel in URLs and must match _FILE_KEY, so odd characters in a
+    # hand-made report's name ("report (1).xml") become "_".
+    return "file:" + re.sub(r"[^A-Za-z0-9._-]", "_", filename)[:200]
 
 
 # --- state ------------------------------------------------------------------
@@ -68,7 +72,7 @@ def _now() -> str:
 
 
 def record_cloud(document_id: str, sent: bool, serial: str = "", detail: str = ""):
-    if not _DOC_ID.match(document_id or ""):
+    if not _DOC_ID.fullmatch(document_id or ""):
         raise ValueError("bad document_id")
     state = load_state()
     state["cloud"][document_id] = {
@@ -80,7 +84,7 @@ def record_cloud(document_id: str, sent: bool, serial: str = "", detail: str = "
 
 
 def mark_servicenow(document_id: str, done: bool = True):
-    if not _REPORT_KEY.match(document_id or ""):
+    if not _REPORT_KEY.fullmatch(document_id or ""):
         raise ValueError("bad document_id")
     state = load_state()
     if done:
@@ -118,10 +122,11 @@ def collect(since=None, until=None) -> list[dict]:
         path = os.path.join(folder, f)
         if not f.lower().endswith(".xml") or f.startswith(".") or not os.path.isfile(path):
             continue
+        unreadable = False
         try:
             data = blancco_report.parse(path)
         except Exception:
-            data = {}
+            data, unreadable = {}, True
         when = blancco_report.report_datetime(data) or datetime.fromtimestamp(
             os.stat(path).st_mtime, timezone.utc)
         if since and when < since:
@@ -135,6 +140,7 @@ def collect(since=None, until=None) -> list[dict]:
             "when": when,
             "data": data,
             "document_id": (data.get("meta") or {}).get("document_id", ""),
+            "unreadable": unreadable,
         })
         out[-1]["key"] = report_key(out[-1]["document_id"], f)
     return out
@@ -197,6 +203,14 @@ def build_evidence_zip(month: str, fileobj) -> int:
     return len(reports)
 
 
+def _target(er: dict) -> dict:
+    """An erasure's target; parse() makes a repeated target a list."""
+    t = er.get("target") or {}
+    if isinstance(t, list):
+        t = t[0] if t else {}
+    return t if isinstance(t, dict) else {}
+
+
 def failure_note(data: dict):
     """(failed erasures, ServiceNow note) for a parsed report. A drive is
     failed only if none of its erasures succeeded (a Purge failure followed
@@ -210,7 +224,7 @@ def failure_note(data: dict):
         return failed, ""
     reasons = [blancco_report.failure_reason(e) for e in failed]
     drives = ", ".join(
-        f"{(e.get('target') or {}).get('model', '')} {(e.get('target') or {}).get('serial', '')}".strip()
+        f"{_target(e).get('model', '')} {_target(e).get('serial', '')}".strip()
         for e in failed)
     return failed, (f"Blancco wipe failed on {drives or 'drive'}: "
                     f"{'; '.join(x.rstrip('. ') for x in reasons if x) or 'no erasure recorded'}. "
@@ -223,6 +237,8 @@ def servicenow_items(days: int = 30, include_done: bool = False) -> list[dict]:
     state = load_state()
     items = []
     for r in reversed(collect(since)):
+        if r.get("unreadable"):
+            continue  # /reports flags it as Unreadable; nothing to tick or destroy
         done = state["servicenow"].get(r["key"])
         if done and not include_done:
             continue
@@ -260,7 +276,7 @@ _EVIDENCE_ZIP = re.compile(r"^ENT-056_.+_(20\d\d-\d\d)\.zip$")
 
 def mark(kind: str, key: str, done: bool = True):
     pattern = TODO_KINDS.get(kind)
-    if not pattern or not pattern.match(key or ""):
+    if not pattern or not pattern.fullmatch(key or ""):
         raise ValueError("bad to-do item")
     state = load_state()
     bucket = state.setdefault(kind, {})

webapp/services/blancco_report.py

@@ -18,6 +18,7 @@ Returned shape (best-effort; missing keys are OK in templates):
     }
 """
 from __future__ import annotations
+import os
 import xml.etree.ElementTree as ET
 
 
@@ -52,7 +53,28 @@ def _text(parent, path, default=""):
     return el.text.strip()
 
 
+# Parsed reports by path, reused while the file's size and mtime are
+# unchanged. The reports list, the to-do (which walks a year of reports
+# several times per request), exports and evidence packs all parse the same
+# files; a report on the share is never edited in place. Callers must not
+# mutate the returned dict.
+_parse_cache: dict = {}
+
+
 def parse(path: str) -> dict:
+    st = os.stat(path)
+    sig = (st.st_mtime_ns, st.st_size)
+    hit = _parse_cache.get(path)
+    if hit and hit[0] == sig:
+        return hit[1]
+    data = _parse(path)
+    if len(_parse_cache) > 5000:  # far above a site's report count; just a bound
+        _parse_cache.clear()
+    _parse_cache[path] = (sig, data)
+    return data
+
+
+def _parse(path: str) -> dict:
     tree = ET.parse(path)
     root = tree.getroot()
     rep = root.find(".//report/blancco_data") or root

webapp/services/dell_updates.py

@@ -1379,9 +1379,6 @@ def file_status(rec):
     return out
 
 
-_PACK_VER = re.compile(r"_(a\d+)", re.I)
-
-
 def _model_for_zip(zip_name, maps):
     """First model token of the map entry that uses zip_name, or ''."""
     for m in maps.values():
@@ -1399,9 +1396,9 @@ def describe(rec, maps=None):
     out = []
     for d in rec.get("drivers") or []:
         model = _model_for_zip(d.get("new", ""), maps) or _model_for_zip(d.get("old", ""), maps)
-        a, b = _PACK_VER.search(d.get("old", "") or ""), _PACK_VER.search(d.get("new", "") or "")
+        a, b = aversion(d.get("old", "")), aversion(d.get("new", ""))
-        if model and a and b:
+        if model and a is not None and b is not None:
-            out.append(f"{model} pack {a.group(1).upper()} to {b.group(1).upper()}")
+            out.append(f"{model} pack A{a:02d} to A{b:02d}")
         else:
             out.append(f"{d.get('old', '')} to {d.get('new', '')}")
     for b in rec.get("bios") or []:
@@ -1448,6 +1445,8 @@ def rollback_preview(apply_id, max_lines=400):
         if len(lines) > max_lines:
             lines = lines[:max_lines] + [f"... {len(lines) - max_lines} more lines"]
         f["diff"] = "\n".join(lines)
+    # A file whose backup could not be read above is no longer restorable.
+    rec["unchanged"] = sum(1 for f in rec["files"] if f["state"] == "unchanged")
     return rec
 
 

webapp/services/imaging_status.py

@@ -86,6 +86,13 @@ def update_session(payload: dict) -> dict:
     serial = (payload.get("serial") or "").strip()
     if not serial:
         raise ValueError("payload missing 'serial'")
+    # "inferred" is the server's own label for log-only bays; a client that
+    # sends it would hide its row from the floor's failure-first order.
+    if payload.get("source") not in (None, "client", "blancco"):
+        payload = {k: v for k, v in payload.items() if k != "source"}
+    # Serials end up in file names and in page markup: keep them plain.
+    if not re.fullmatch(r"[A-Za-z0-9._-]{1,64}", serial):
+        raise ValueError("serial must be 1-64 letters, digits, '.', '_' or '-'")
 
     _ensure_dir()
     path = _path_for(serial)
@@ -234,7 +241,7 @@ def hide_stale(sessions: list[dict], hours: float = ACTIVE_WINDOW_HOURS) -> tupl
                 when = datetime.fromisoformat(s.get("last_updated") or "")
                 if when.tzinfo is None:
                     when = when.astimezone()
-            except ValueError:
+            except (ValueError, TypeError):
                 when = None
             if when is not None and when < cutoff:
                 hidden += 1

webapp/templates/_imaging_tiles.html

@@ -51,8 +51,8 @@
 {% macro floor_row(s) %}
   {% set is_inferred = (s.source == 'inferred') %}
   {% set is_blancco = (s.source == 'blancco' or s.pctype == 'blancco') %}
-  {% set stage_idx = s.stage_index | int(0) %}
+  {% set stage_idx = (s.stage_index or 0) | int(0) %}
-  {% set stage_total = s.stage_total | int(0) %}
+  {% set stage_total = (s.stage_total or 0) | int(0) %}
   {% set state = s.floor_state or 'running' %}
   {% set drive_pct = None %}
   {% if is_blancco and stage_idx == 4 and s.current_stage and '%]' in s.current_stage %}
@@ -178,7 +178,8 @@
         {% if is_blancco %}<a href="{{ url_for('blancco_reports') }}">Erasure reports</a>{% endif %}
         <div class="sep"></div>
         <form method="post" action="{{ url_for('imaging_delete_session', serial=s.serial) }}"
-              onsubmit="return confirm('Clear the session for {{ s.serial }}? Its status and log files are deleted.');">
+              data-confirm="Clear the session for {{ s.serial }}? Its status and log files are deleted."
+              onsubmit="return confirm(this.dataset.confirm);">
           <input type="hidden" name="_csrf_token" value="{{ csrf_token() }}">
           <button type="submit" class="del">Clear session</button>
         </form>

webapp/templates/dashboard.html

@@ -208,8 +208,11 @@
 (function () {
     var url = "{{ url_for('imaging_tiles_partial', limit=6) }}";
     setInterval(function () {
-        if (document.querySelector('#imaging-tiles details[open], #imaging-tiles details.more[open]')) return;
+        if (document.hidden || document.querySelector('#imaging-tiles details[open], #imaging-tiles details.more[open]')) return;
-        fetch(url, {credentials: 'same-origin'}).then(function (r) { return r.text(); }).then(function (html) {
+        fetch(url, {credentials: 'same-origin'}).then(function (r) {
+            if (!r.ok) throw new Error('HTTP ' + r.status);  // keep the last good rows
+            return r.text();
+        }).then(function (html) {
             var box = document.getElementById('imaging-tiles');
             if (box) { box.innerHTML = html; if (window.renderAllQRs) window.renderAllQRs(); }
         }).catch(function () {});

webapp/templates/enrollment.html

@@ -109,7 +109,7 @@
   {% if packages %}
   <div class="sgrid colhead" aria-hidden="true"><span>File</span><span class="num">Size</span><span class="when">Modified</span></div>
   {% for p in packages %}
-  {% set is_active = active_ppkg and active_ppkg.SOURCE_PPKG and p.filename|lower == active_ppkg.SOURCE_PPKG|lower %}
+  {% set is_active = p.filename|lower == effective_ppkg|lower %}
   <div class="ui-row">
     <div class="sgrid">
       <span class="s-name">

webapp/templates/imaging.html

@@ -105,6 +105,11 @@ Content-Type: application/json
         // copy. The next refresh after they hit Make-QR will catch up.
         var active = document.activeElement;
         if (active && active.classList && active.classList.contains('laps-input')) return true;
+        // Also while a password is shown, or typed but not yet made into a QR.
+        var inputs = document.querySelectorAll('.laps-input');
+        for (var i = 0; i < inputs.length; i++) {
+            if (inputs[i].type === 'text' || inputs[i].value !== inputs[i].defaultValue) return true;
+        }
         return false;
     }
 

webapp/templates/imaging_detail.html

@@ -5,8 +5,8 @@
 {% set st = session.status or 'in_progress' %}
 {% set st_chip = {'succeeded': ('Succeeded', 'ok'), 'failed': ('Failed', 'fail')}.get(st, ('In progress', 'run')) %}
 {% set st_kind = {'succeeded': 'ok', 'failed': 'fail'}.get(st, 'run') %}
-{% set stage_idx = session.stage_index | int(0) %}
+{% set stage_idx = (session.stage_index or 0) | int(0) %}
-{% set stage_total = session.stage_total | int(0) %}
+{% set stage_total = (session.stage_total or 0) | int(0) %}
 {% set history = session.stage_history or [] %}
 
 {# Support summary: built here so the copy button carries it (static/app.js