fix(reports): code-review findings — XSS, SMTP, blocking, unit link, email guard

- #1 XSS: escape user-controlled values (location name, baseline values, recent-
  report fields, SMTP status message) in the modals via the existing _mergeEsc
  helper — they were concatenated raw into innerHTML (stored XSS via location name).
- #2 SMTP: an unrecognized REPORT_SMTP_SECURITY no longer silently downgrades to a
  plaintext connection while still calling login() — it falls back to starttls and
  warns; warn on intentional security=none + auth.
- #3 scheduler: run the (blocking smtplib + Excel) nightly report in a worker thread
  (asyncio.to_thread + its own DB session) so it can't stall the loop that drives
  time-sensitive device cycles. New _run_one_report helper.
- #4 cycle ingest: set unit_id on the ingested data session (ingest_nrl_zip leaves
  it None) before dropping the empty placeholder, preserving the unit<->session link;
  repoint old_session_id at the real row.
- #7 robustness: wrap send_report_email in the orchestrator and run_nightly_report in
  /view + /run so a render/SMTP error returns a clean error instead of a raw 500
  after artifacts are written.

Verified: SMTP paths (typo->starttls, none, starttls, ssl), off-thread tick stamps
last_run_date + writes the file, /view 200, escaping wired, app imports.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/routers/reports.py

@@ -200,11 +200,17 @@ async def view_nightly_report(
 ):
     """Render the night report and return the HTML inline (preview — no write, no email)."""
     nd, bmode, bs, be, metric_keys = _resolve_params(project_id, db, night_date, baseline_start, baseline_end, metrics)
+    try:
         result = run_nightly_report(
             db, project_id, nd,
             metric_keys=metric_keys, baseline_mode=bmode, baseline_start=bs, baseline_end=be,
             send=False,           # preview: no email
         )
+    except HTTPException:
+        raise
+    except Exception as e:  # noqa: BLE001
+        logger.error("nightly/view failed for %s (%s): %s", project_id, nd, e, exc_info=True)
+        raise HTTPException(status_code=500, detail=f"Report generation failed: {e}")
     return HTMLResponse(result["html"])
 
 
@@ -224,11 +230,17 @@ async def run_nightly_report_endpoint(
     is omitted from the JSON response (it's large and on disk); use /view to see it.
     """
     nd, bmode, bs, be, metric_keys = _resolve_params(project_id, db, night_date, baseline_start, baseline_end, metrics)
+    try:
         result = run_nightly_report(
             db, project_id, nd,
             metric_keys=metric_keys, baseline_mode=bmode, baseline_start=bs, baseline_end=be,
             send=send,
         )
+    except HTTPException:
+        raise
+    except Exception as e:  # noqa: BLE001
+        logger.error("nightly/run failed for %s (%s): %s", project_id, nd, e, exc_info=True)
+        raise HTTPException(status_code=500, detail=f"Report generation failed: {e}")
     result.pop("html", None)  # keep the JSON response lean — view it via /view or the file
     result["view_url"] = (
         f"/api/projects/{project_id}/reports/nightly/view"

backend/services/report_email.py

@@ -136,8 +136,14 @@ def send_report_email(
         )
         return result
 
+    # Validate the security mode: an unrecognized value (typo) must NOT silently
+    # fall through to a plaintext connection while still sending credentials.
+    sec = cfg.security if cfg.security in ("ssl", "starttls", "none") else "starttls"
+    if sec != cfg.security:
+        logger.warning("Unknown REPORT_SMTP_SECURITY=%r — falling back to 'starttls'", cfg.security)
+
     try:
-        if cfg.security == "ssl":
+        if sec == "ssl":
             ctx = ssl.create_default_context()
             with smtplib.SMTP_SSL(cfg.host, cfg.port, timeout=cfg.timeout, context=ctx) as s:
                 if cfg.user:
@@ -146,10 +152,15 @@ def send_report_email(
         else:
             with smtplib.SMTP(cfg.host, cfg.port, timeout=cfg.timeout) as s:
                 s.ehlo()
-                if cfg.security == "starttls":
+                if sec == "starttls":
                     s.starttls(context=ssl.create_default_context())
                     s.ehlo()
                 if cfg.user:
+                    if sec == "none":
+                        logger.warning(
+                            "Sending SMTP credentials over an UNENCRYPTED connection "
+                            "(REPORT_SMTP_SECURITY=none) — set starttls/ssl if the relay supports it."
+                        )
                     s.login(cfg.user, cfg.password)
                 s.send_message(msg)
         result["sent"] = True

backend/services/report_orchestrator.py

@@ -120,9 +120,13 @@ def run_nightly_report(
     # --- Email (best-effort; dry-run until SMTP is configured) ---
     email_result = {"sent": False, "dry_run": False, "skipped": True, "error": None}
     if send:
+        try:
             email_result = send_report_email(
                 subject, html, attachments=attachments, recipients=recipients,
             )
+        except Exception as e:  # noqa: BLE001 — artifacts are already written; never abort on email
+            logger.error("send_report_email raised for %s (%s): %s", project_id, night_date, e, exc_info=True)
+            email_result = {"sent": False, "dry_run": False, "skipped": False, "error": str(e)}
 
     result = {
         "project_id": project_id,

backend/services/scheduler.py

@@ -653,11 +653,22 @@ class SchedulerService:
                         ing = await self._ingest_cycle_folder(db, action.location_id, unit_id, folder_name)
                         result["steps"]["ingest"] = ing
                         db.commit()
-                        # The just-closed "recording" session was only a marker — if the
-                        # ingest created the real data session, drop the empty placeholder.
-                        if ing.get("success") and active_session:
+                        if ing.get("success"):
                             from backend.models import DataFile
-                            if db.query(DataFile).filter_by(session_id=active_session.id).count() == 0:
+                            sid = ing.get("session_id")
+                            # ingest_nrl_zip leaves unit_id None — tie the data session to the
+                            # unit that recorded it so it stays linked after we drop the placeholder.
+                            if sid:
+                                s = db.query(MonitoringSession).filter_by(id=sid).first()
+                                if s and not s.unit_id:
+                                    s.unit_id = unit_id
+                                    db.commit()
+                            # The just-closed "recording" session was only a marker; its data now
+                            # lives in the ingested (unit-linked) session. Drop the empty placeholder
+                            # and repoint old_session_id at the real row.
+                            if active_session and db.query(DataFile).filter_by(session_id=active_session.id).count() == 0:
+                                if sid:
+                                    result["old_session_id"] = sid
                                 db.delete(active_session)
                                 db.commit()
                         logger.info(f"[CYCLE] Ingested {folder_name}: {ing}")
@@ -885,51 +896,76 @@ class SchedulerService:
         last_run_date.  Idempotent across restarts via last_run_date.
         """
         from backend.models import SoundReportConfig
-        from backend.services.report_orchestrator import run_nightly_report
         from backend.utils.timezone import utc_to_local
 
+        # Decide what's due (cheap, on the loop); run each OFF the event loop.
+        due_jobs = []
         db = SessionLocal()
         try:
             configs = db.query(SoundReportConfig).filter_by(enabled=True).all()
             if not configs:
                 return
-
             local_now = utc_to_local(datetime.utcnow())
             night_date = local_now.date() - timedelta(days=1)   # last night's evening date
-
             for cfg in configs:
-                try:
-                    # Past the configured local report time (HH:MM)?
                 try:
                     hh, mm = (int(x) for x in cfg.report_time.split(":"))
                 except (ValueError, AttributeError):
                     hh, mm = 8, 0
-                    due = (local_now.hour, local_now.minute) >= (hh, mm)
-                    if not due or cfg.last_run_date == night_date:
+                if (local_now.hour, local_now.minute) < (hh, mm):
                     continue
+                if cfg.last_run_date == night_date:
+                    continue
+                due_jobs.append({
+                    "project_id": cfg.project_id,
+                    "metric_keys": [m.strip() for m in (cfg.metric_keys or "").split(",") if m.strip()] or None,
+                    "recipients": [r.strip() for r in (cfg.recipients or "").split(",") if r.strip()] or None,
+                    "baseline_mode": cfg.baseline_mode,
+                    "baseline_start": cfg.baseline_start,
+                    "baseline_end": cfg.baseline_end,
+                })
+        finally:
+            db.close()
 
-                    metric_keys = [m.strip() for m in (cfg.metric_keys or "").split(",") if m.strip()] or None
-                    recipients = [r.strip() for r in (cfg.recipients or "").split(",") if r.strip()] or None
-
-                    logger.info(f"[REPORT] Running nightly report for project {cfg.project_id} (night {night_date})")
-                    result = run_nightly_report(
-                        db, cfg.project_id, night_date,
-                        metric_keys=metric_keys,
-                        baseline_mode=cfg.baseline_mode,
-                        baseline_start=cfg.baseline_start,
-                        baseline_end=cfg.baseline_end,
-                        recipients=recipients,
-                    )
-                    cfg.last_run_date = night_date
-                    db.commit()
-                    email = result.get("email", {})
+        # run_nightly_report is synchronous (blocking file I/O + smtplib up to the
+        # SMTP timeout). Run it in a worker thread so it never stalls the scheduler
+        # loop (which also drives time-sensitive device cycles).
+        for job in due_jobs:
+            try:
+                logger.info(f"[REPORT] Running nightly report for project {job['project_id']} (night {night_date})")
+                result = await asyncio.to_thread(self._run_one_report, night_date, job)
+                email = (result or {}).get("email", {})
                 logger.info(
-                        f"[REPORT] project {cfg.project_id}: {result.get('location_count')} location(s); "
+                    f"[REPORT] project {job['project_id']}: {(result or {}).get('location_count')} location(s); "
                     f"email={'sent' if email.get('sent') else ('dry-run' if email.get('dry_run') else (email.get('error') or 'skipped'))}"
                 )
             except Exception as e:
-                    logger.error(f"[REPORT] Failed nightly report for project {cfg.project_id}: {e}", exc_info=True)
+                logger.error(f"[REPORT] Failed nightly report for project {job['project_id']}: {e}", exc_info=True)
+
+    def _run_one_report(self, night_date, job) -> Dict[str, Any]:
+        """Sync worker: build/send one project's report and stamp last_run_date.
+        Uses its own DB session (runs in a thread, off the event loop)."""
+        from backend.models import SoundReportConfig
+        from backend.services.report_orchestrator import run_nightly_report
+
+        db = SessionLocal()
+        try:
+            result = run_nightly_report(
+                db, job["project_id"], night_date,
+                metric_keys=job["metric_keys"],
+                baseline_mode=job["baseline_mode"],
+                baseline_start=job["baseline_start"],
+                baseline_end=job["baseline_end"],
+                recipients=job["recipients"],
+            )
+            cfg = db.query(SoundReportConfig).filter_by(project_id=job["project_id"]).first()
+            if cfg:
+                cfg.last_run_date = night_date
+            db.commit()
+            return result
+        except Exception:
             db.rollback()
+            raise
         finally:
             db.close()
 

templates/partials/projects/project_header.html

@@ -188,7 +188,7 @@ function runNightReport(projectId) {
             var em = res.j.email || {};
             var emailMsg = em.sent ? 'emailed' : (em.dry_run ? 'email dry-run (SMTP not set)' : (em.error || 'email skipped'));
             st.style.color = '#1a7f37';
-            st.innerHTML = 'Done — saved &amp; ' + emailMsg + '. <a href="' + res.j.view_url + '" target="_blank" class="underline">view</a>';
+            st.innerHTML = 'Done — saved &amp; ' + _mergeEsc(emailMsg) + '. <a href="' + _mergeEsc(res.j.view_url) + '" target="_blank" class="underline">view</a>';
             loadRecentReports(projectId);
         })
         .catch(function (e) { st.style.color = '#b00020'; st.textContent = 'Error: ' + e; });
@@ -206,10 +206,10 @@ function loadRecentReports(projectId) {
             }
             box.innerHTML = j.reports.map(function (rp) {
                 var when = (rp.generated_at || '').replace('T', ' ').slice(0, 16);
-                var xlsx = rp.xlsx_url ? ' · <a href="' + rp.xlsx_url + '" class="text-indigo-600 dark:text-indigo-400 hover:underline">Excel</a>' : '';
+                var xlsx = rp.xlsx_url ? ' · <a href="' + _mergeEsc(rp.xlsx_url) + '" class="text-indigo-600 dark:text-indigo-400 hover:underline">Excel</a>' : '';
                 return '<div class="flex items-center justify-between px-3 py-2 text-sm">'
-                     + '<a href="' + rp.view_url + '" target="_blank" class="font-medium text-gray-800 dark:text-gray-200 hover:underline">Night of ' + rp.night_date + '</a>'
-                     + '<span class="text-xs text-gray-400">' + when + ' UTC' + xlsx + '</span></div>';
+                     + '<a href="' + _mergeEsc(rp.view_url) + '" target="_blank" class="font-medium text-gray-800 dark:text-gray-200 hover:underline">Night of ' + _mergeEsc(rp.night_date) + '</a>'
+                     + '<span class="text-xs text-gray-400">' + _mergeEsc(when) + ' UTC' + xlsx + '</span></div>';
             }).join('');
         })
         .catch(function () { box.innerHTML = '<div class="px-3 py-2 text-xs text-red-500">Failed to load.</div>'; });
@@ -380,12 +380,12 @@ function renderRefGrid() {
         var rows = M.map(function (m) {
             var cells = W.map(function (w) {
                 var v = (loc.values[w.key] && loc.values[w.key][m.key] != null) ? loc.values[w.key][m.key] : '';
-                return '<td class="px-1"><input type="number" step="0.1" id="' + _refId(loc.id, w.key, m.key) + '" value="' + v + '" class="w-16 px-1.5 py-1 border border-gray-300 dark:border-gray-600 rounded bg-white dark:bg-slate-700 text-gray-900 dark:text-white text-sm text-center"></td>';
+                return '<td class="px-1"><input type="number" step="0.1" id="' + _refId(loc.id, w.key, m.key) + '" value="' + _mergeEsc(v) + '" class="w-16 px-1.5 py-1 border border-gray-300 dark:border-gray-600 rounded bg-white dark:bg-slate-700 text-gray-900 dark:text-white text-sm text-center"></td>';
             }).join('');
             return '<tr><td class="text-sm text-gray-700 dark:text-gray-300 pr-2">' + m.label + '</td>' + cells + '</tr>';
         }).join('');
         return '<div class="border border-gray-200 dark:border-gray-700 rounded-lg p-2">'
-             + '<div class="text-sm font-medium text-gray-800 dark:text-gray-200 mb-1">' + loc.name + '</div>'
+             + '<div class="text-sm font-medium text-gray-800 dark:text-gray-200 mb-1">' + _mergeEsc(loc.name) + '</div>'
              + '<table class="w-full">' + head + rows + '</table></div>';
     }).join('');
 }