Fix XSS, CSRF, input validation, and related security issues

app.py

@@ -3,11 +3,22 @@ from sqlalchemy import create_engine, func
 from sqlalchemy.orm import scoped_session, sessionmaker
 
 from datetime import datetime, date, timedelta
-import json
+import re
 
 from models import Base, Battery, BatteryPctLog, Device, CapacityTest, ChargeLog
 
 
+def _parse_date(val: str) -> str | None:
+    """Return val if it is a valid YYYY-MM-DD string, else None."""
+    if not val:
+        return None
+    try:
+        datetime.strptime(val, "%Y-%m-%d")
+        return val
+    except ValueError:
+        return None
+
+
 def create_app(config_object="config"):
     app = Flask(__name__)
     app.config.from_object(config_object)
@@ -28,6 +39,9 @@ def create_app(config_object="config"):
     # Home Assistant integration (optional)
     # ------------------------------------------------------------------ #
 
+    from flask_wtf.csrf import CSRFProtect
+    CSRFProtect(app)
+
     from ha_client import HomeAssistantClient
     from ha_poller import HaPoller
 
@@ -109,9 +123,14 @@ def create_app(config_object="config"):
             purchase_date    = f.get("purchase_date", "").strip() or None
             storage_location = f.get("storage_location", "").strip() or None
 
-            existing = db.query(func.count(Battery.id)).filter_by(brand=brand).scalar()
+            existing_labels = [
+                r[0] for r in db.query(Battery.label).filter(Battery.brand == brand).all()
+            ]
+            nums = [int(m.group(1)) for lbl in existing_labels
+                    if (m := re.search(r'(\d+)$', lbl))]
+            next_num = max(nums, default=0)
             for i in range(count):
-                label = f"{brand} {existing + i + 1:03d}"
+                label = f"{brand} {next_num + i + 1:03d}"
                 db.add(Battery(label=label, brand=brand, status="available", notes=notes,
                                size=size, chemistry=chemistry, capacity_mah=capacity_mah,
                                purchase_date=purchase_date, storage_location=storage_location))
@@ -160,26 +179,26 @@ def create_app(config_object="config"):
                     .filter_by(battery_id=battery_id)
                     .order_by(BatteryPctLog.recorded_at.desc())
                     .all())
-        charge_logs_json = json.dumps([
+        charge_logs_data = [
             {"id": l.id, "date": l.charged_date, "cycles": l.increment_cycles, "notes": l.notes or ""}
             for l in charge_logs
-        ])
+        ]
-        capacity_tests_json = json.dumps([
+        capacity_tests_data = [
             {"id": t.id, "date": t.tested_date, "mah": t.tested_capacity_mah, "notes": t.notes or ""}
             for t in sorted(capacity_tests, key=lambda t: (t.tested_date, t.id), reverse=True)
-        ])
+        ]
-        pct_logs_json = json.dumps([
+        pct_logs_data = [
             {"recorded_at": str(l.recorded_at), "pct": l.percentage, "source": l.source or ""}
             for l in pct_logs
-        ])
+        ]
         return render_template("battery_detail.html", battery=battery,
                                storage_locations=storage_locations,
                                capacity_tests=capacity_tests,
                                charge_logs=charge_logs,
                                pct_logs=pct_logs,
-                               charge_logs_json=charge_logs_json,
+                               charge_logs_data=charge_logs_data,
-                               capacity_tests_json=capacity_tests_json,
+                               capacity_tests_data=capacity_tests_data,
-                               pct_logs_json=pct_logs_json)
+                               pct_logs_data=pct_logs_data)
 
     # ------------------------------------------------------------------ #
     # Battery — edit notes
@@ -204,7 +223,8 @@ def create_app(config_object="config"):
         battery.chemistry           = f.get("chemistry", "").strip() or None
         battery.capacity_mah        = _int("capacity_mah")
         battery.charge_cycles       = _int("charge_cycles")
-        battery.purchase_date       = f.get("purchase_date", "").strip() or None
+        purchase_raw                = f.get("purchase_date", "").strip()
+        battery.purchase_date       = _parse_date(purchase_raw) if purchase_raw else None
         battery.storage_location    = f.get("storage_location", "").strip() or None
         new_pct = _int("battery_percentage")
         if new_pct != battery.battery_percentage:
@@ -239,10 +259,10 @@ def create_app(config_object="config"):
         if battery is None:
             abort(404)
         mah_raw  = request.form.get("tested_capacity_mah", "").strip()
-        date_val = request.form.get("tested_date", "").strip()
+        date_val = _parse_date(request.form.get("tested_date", "").strip())
         notes    = request.form.get("notes", "").strip() or None
         if not mah_raw or not date_val:
-            flash("Capacity (mAh) and date are required.", "error")
+            flash("Capacity (mAh) and a valid date (YYYY-MM-DD) are required.", "error")
             return redirect(url_for("battery_detail", battery_id=battery_id))
         try:
             mah = int(mah_raw)
@@ -281,9 +301,9 @@ def create_app(config_object="config"):
         battery = db.get(Battery, battery_id)
         if battery is None:
             abort(404)
-        date_val = request.form.get("charged_date", "").strip()
+        date_val = _parse_date(request.form.get("charged_date", "").strip())
         if not date_val:
-            flash("Date is required.", "error")
+            flash("A valid date (YYYY-MM-DD) is required.", "error")
             return redirect(url_for("battery_detail", battery_id=battery_id))
         increment = 1 if request.form.get("increment_cycles") else 0
         notes = request.form.get("notes", "").strip() or None
@@ -544,9 +564,9 @@ def create_app(config_object="config"):
             label = field_name.replace("_", " ").title()
             flash(f"Set {label} on {n} batter{'y' if n == 1 else 'ies'}.", "success")
         elif action == "log_charged":
-            date_val = request.form.get("charged_date", "").strip()
+            date_val = _parse_date(request.form.get("charged_date", "").strip())
             if not date_val:
-                flash("Date is required.", "error")
+                flash("A valid date (YYYY-MM-DD) is required.", "error")
                 return redirect(url_for("dashboard"))
             increment = 1 if request.form.get("increment_cycles") else 0
             for b in batteries:
@@ -857,4 +877,4 @@ def create_app(config_object="config"):
 app = create_app()
 
 if __name__ == "__main__":
-    app.run(debug=True)
+    app.run(debug=False)

config.py

@@ -1,10 +1,19 @@
 import os
+import logging
 
 SQLALCHEMY_DATABASE_URI = os.environ.get(
     "DATABASE_URL",
     "sqlite:///batteries.db",
 )
-SECRET_KEY = os.environ.get("SECRET_KEY", "dev-secret-change-in-prod")
+
+_secret_key = os.environ.get("SECRET_KEY")
+if not _secret_key:
+    logging.warning(
+        "SECRET_KEY not set — using insecure default. "
+        "Set SECRET_KEY env var before running in production."
+    )
+SECRET_KEY = _secret_key or "dev-secret-change-in-prod"
+
 SQLALCHEMY_TRACK_MODIFICATIONS = False
 
 # Home Assistant integration (all optional — app works normally when absent)

requirements.txt

@@ -1,4 +1,5 @@
 Flask>=3.0,<4.0
+Flask-WTF>=1.2,<2.0
 SQLAlchemy>=2.0,<3.0
 PyMySQL>=1.1,<2.0
 waitress>=3.0,<4.0

templates/base.html

@@ -349,6 +349,18 @@
       navigator.serviceWorker.register('/sw.js');
     }
 
+    // Inject CSRF token into all POST forms
+    document.addEventListener('DOMContentLoaded', function() {
+      var token = '{{ csrf_token() }}';
+      document.querySelectorAll('form').forEach(function(form) {
+        if (form.method.toLowerCase() === 'post') {
+          var inp = document.createElement('input');
+          inp.type = 'hidden'; inp.name = 'csrf_token'; inp.value = token;
+          form.appendChild(inp);
+        }
+      });
+    });
+
     (function() {
       var modal     = document.getElementById('confirm-modal');
       var msgEl     = document.getElementById('confirm-modal-msg');

templates/battery_detail.html

@@ -411,7 +411,7 @@ function metaSelectChanged(sel, inputId) {
 (function() {
   var canvas = document.getElementById('pct-chart');
   if (!canvas) return;
-  var rawLogs = {{ pct_logs_json | safe }};
+  var rawLogs = {{ pct_logs_data | tojson }};
   // pct_logs_json is ordered newest-first; chart wants oldest-first
   var logsAsc = rawLogs.slice().reverse();
   var vals   = logsAsc.map(function(l) { return l.pct; });
@@ -472,14 +472,15 @@ function metaSelectChanged(sel, inputId) {
 <style>
 .hist-modal { display:none; position:fixed; inset:0; z-index:1000; background:rgba(0,0,0,.45); align-items:center; justify-content:center; }
 .hist-modal.open { display:flex; }
-.hist-modal-box { background:#fff; border-radius:8px; width:min(700px,96vw); max-height:85vh; display:flex; flex-direction:column; overflow:hidden; box-shadow:0 8px 32px rgba(0,0,0,.2); }
+.hist-modal-box { background:var(--bg-card,#fff); color:var(--text-body,#222); border-radius:8px; width:min(700px,96vw); max-height:85vh; display:flex; flex-direction:column; overflow:hidden; box-shadow:0 8px 32px rgba(0,0,0,.2); }
 .hist-modal-header { display:flex; align-items:center; justify-content:space-between; padding:0.9rem 1.1rem 0.7rem; border-bottom:1px solid var(--border,#e2e8f0); }
-.hist-modal-header h3 { margin:0; font-size:1.05rem; }
+.hist-modal-header h3 { margin:0; font-size:1.05rem; color:var(--text-h2,#334155); }
 .hist-modal-close { background:none; border:none; font-size:1.2rem; cursor:pointer; color:var(--text-muted,#6b7280); line-height:1; padding:0.2rem 0.4rem; }
 .hist-modal-filters { padding:0.6rem 1.1rem; border-bottom:1px solid var(--border,#e2e8f0); display:flex; gap:0.75rem; align-items:center; flex-wrap:wrap; }
 .hist-modal-filters:empty { display:none; }
 .hist-modal-body { overflow-y:auto; flex:1; padding:0.5rem 0.5rem; }
 .hist-modal-footer { display:flex; align-items:center; justify-content:space-between; padding:0.6rem 1.1rem; border-top:1px solid var(--border,#e2e8f0); font-size:0.85rem; color:var(--text-muted,#6b7280); }
+.hist-modal-filters select { background:var(--bg-input,#fff); color:var(--text-body,#222); border:1px solid var(--border-input,#d1d5db); border-radius:4px; padding:0.3rem 0.5rem; font-size:0.875rem; font-family:inherit; }
 </style>
 
 <!-- Capacity modal -->
@@ -549,6 +550,14 @@ function metaSelectChanged(sel, inputId) {
 var HIST_PAGE = 20;
 var _batteryId = {{ battery.id }};
 
+function escHtml(s) {
+  return String(s == null ? '' : s)
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;');
+}
+
 function openHistModal(id) {
   document.getElementById(id).classList.add('open');
 }
@@ -618,7 +627,7 @@ function makeModal(cfg) {
 }
 
 // ── Capacity modal ────────────────────────────────────────────────────────
-var _capAll = {{ capacity_tests_json | safe }};
+var _capAll = {{ capacity_tests_data | tojson }};
 var capModal = makeModal({
   all: _capAll,
   bodyId: 'cap-modal-body', prevId: 'cap-prev', nextId: 'cap-next', pageInfoId: 'cap-page-info',
@@ -626,9 +635,9 @@ var capModal = makeModal({
   thead: '<tr><th>Date</th><th>Capacity</th><th>Notes</th><th></th></tr>',
   renderRow: function(r) {
     return '<tr>' +
-      '<td data-label="Date">' + r.date + '</td>' +
+      '<td data-label="Date">' + escHtml(r.date) + '</td>' +
       '<td data-label="Capacity">' + r.mah + ' mAh</td>' +
-      '<td data-label="Notes" class="text-muted">' + (r.notes || '—') + '</td>' +
+      '<td data-label="Notes" class="text-muted">' + (escHtml(r.notes) || '—') + '</td>' +
       '<td data-label="">' +
         '<form class="inline" method="post" ' +
           'action="/battery/' + _batteryId + '/capacity-test/' + r.id + '/delete" ' +
@@ -645,7 +654,7 @@ document.querySelector('[onclick="openHistModal(\'cap-modal\')"]') &&
   document.querySelector('[onclick="openHistModal(\'cap-modal\')"]').addEventListener('click', function() { capModal.init(); });
 
 // ── Charge modal ──────────────────────────────────────────────────────────
-var _chgAll = {{ charge_logs_json | safe }};
+var _chgAll = {{ charge_logs_data | tojson }};
 var chgModal = makeModal({
   all: _chgAll,
   bodyId: 'chg-modal-body', prevId: 'chg-prev', nextId: 'chg-next', pageInfoId: 'chg-page-info',
@@ -653,9 +662,9 @@ var chgModal = makeModal({
   thead: '<tr><th>Date</th><th>+Cycle</th><th>Notes</th><th></th></tr>',
   renderRow: function(r) {
     return '<tr>' +
-      '<td data-label="Date">' + r.date + '</td>' +
+      '<td data-label="Date">' + escHtml(r.date) + '</td>' +
       '<td data-label="+Cycle">' + (r.cycles ? '✓' : '—') + '</td>' +
-      '<td data-label="Notes" class="text-muted">' + (r.notes || '—') + '</td>' +
+      '<td data-label="Notes" class="text-muted">' + (escHtml(r.notes) || '—') + '</td>' +
       '<td data-label="">' +
         '<form class="inline" method="post" ' +
           'action="/battery/' + _batteryId + '/charge-log/' + r.id + '/delete" ' +
@@ -669,7 +678,7 @@ document.querySelector('[onclick="openHistModal(\'chg-modal\')"]') &&
   document.querySelector('[onclick="openHistModal(\'chg-modal\')"]').addEventListener('click', function() { chgModal.init(); });
 
 // ── Percentage modal ──────────────────────────────────────────────────────
-var _pctAll = {{ pct_logs_json | safe }};
+var _pctAll = {{ pct_logs_data | tojson }};
 var pctModal = makeModal({
   all: _pctAll,
   bodyId: 'pct-modal-body', prevId: 'pct-prev', nextId: 'pct-next', pageInfoId: 'pct-page-info',
@@ -680,9 +689,9 @@ var pctModal = makeModal({
       ? '<span class="badge badge-warning">⚠ ' + r.pct + '%</span>'
       : r.pct + '%';
     return '<tr>' +
-      '<td data-label="Date / Time">' + r.recorded_at + '</td>' +
+      '<td data-label="Date / Time">' + escHtml(r.recorded_at) + '</td>' +
       '<td data-label="%">' + pctHtml + '</td>' +
-      '<td data-label="Source" class="text-muted">' + (r.source || '—') + '</td>' +
+      '<td data-label="Source" class="text-muted">' + (escHtml(r.source) || '—') + '</td>' +
       '</tr>';
   }
 });

tests/conftest.py

@@ -15,6 +15,7 @@ def app():
         SECRET_KEY = "test-secret"
         SQLALCHEMY_TRACK_MODIFICATIONS = False
         TESTING = True
+        WTF_CSRF_ENABLED = False      # disable CSRF validation in tests
         HOMEASSISTANT_URL = None      # prevent HA poller from starting in tests
         HOMEASSISTANT_API_KEY = None
 

tests/test_ha_integration.py

@@ -33,6 +33,7 @@ def ha_app():
         SECRET_KEY = "test-secret"
         SQLALCHEMY_TRACK_MODIFICATIONS = False
         TESTING = True
+        WTF_CSRF_ENABLED = False      # disable CSRF validation in tests
         HOMEASSISTANT_URL = "http://ha.test:8123"
         HOMEASSISTANT_API_KEY = "fake-token"
         HOMEASSISTANT_POLL_INTERVAL = 300