05/06 Fix some notable and quality issues

app/models/api_token.py

@@ -76,6 +76,11 @@ class RefreshToken(db.Model):
         """
         Look up a refresh token by its raw value.
         Returns the RefreshToken row if valid and unexpired, else None.
+
+        Expired rows are not deleted here — passive cleanup runs in the
+        login route (api/auth.py) each time a user authenticates, removing
+        all expired/revoked tokens for that user.  This keeps the table tidy
+        without requiring a dedicated cron job.
         """
         import hashlib
         hashed = hashlib.sha256(raw_token.encode()).hexdigest()

app/models/audit.py

@@ -16,8 +16,8 @@ class AuditLog(db.Model):
     username     = db.Column(db.String(100), nullable=False)          # snapshot at time of action
     user_role    = db.Column(db.String(20),  nullable=False)          # snapshot at time of action
     # What happened
-    action       = db.Column(db.String(50),  nullable=False)          # CREATE / UPDATE / DELETE / LOGIN / LOGOUT / EXPORT
+    action       = db.Column(db.String(50),  nullable=False, index=True)          # CREATE / UPDATE / DELETE / LOGIN / LOGOUT / EXPORT
-    entity_type  = db.Column(db.String(50),  nullable=False)          # User / Facility / Area / Template / Inspection / Issue / …
+    entity_type  = db.Column(db.String(50),  nullable=False, index=True)          # User / Facility / Area / Template / Inspection / Issue / …
     entity_id    = db.Column(db.Integer,     nullable=True)           # PK of the affected record (NULL for bulk ops)
     entity_label = db.Column(db.String(255), nullable=True)           # Human-readable identifier snapshot
     # Extra context stored as free-text (key=value pairs, comma-separated)

app/routes/auth.py

@@ -1,10 +1,9 @@
 from flask import Blueprint, render_template, redirect, url_for, flash, request, abort
 from flask_login import login_user, logout_user, login_required, current_user
-from urllib.parse import urlparse
 from app import db, limiter
 from app.models.user import User
 from app.utils.forms import LoginForm, UserForm, ProfileForm
-from app.utils.decorators import admin_required, supervisor_required
+from app.utils.decorators import admin_required, supervisor_required, safe_redirect_url
 import logging
 from app.utils.audit import log_action, ACTION_CREATE, ACTION_UPDATE, ACTION_DELETE, ACTION_LOGIN, ACTION_LOGOUT
 
@@ -13,22 +12,6 @@ logger = logging.getLogger(__name__)
 bp = Blueprint('auth', __name__, url_prefix='/auth')
 
 
-def _safe_next(next_url: str | None) -> str:
-    """
-    Validate that the redirect target is a relative URL on this host.
-    Returns the safe URL, or the dashboard index if the URL is external/invalid.
-    This prevents open-redirect attacks where an attacker crafts a login link
-    containing next=https://evil.com to hijack post-login redirects.
-    """
-    if not next_url:
-        return url_for('dashboard.index')
-    parsed = urlparse(next_url)
-    # Reject any URL that specifies a network location (external host) or scheme
-    if parsed.netloc or parsed.scheme:
-        return url_for('dashboard.index')
-    return next_url
-
-
 @bp.route('/login', methods=['GET', 'POST'])
 @limiter.limit('20 per minute; 5 per second')
 def login():
@@ -52,7 +35,7 @@ def login():
                 return render_template('auth/login.html', form=form)
             login_user(user, remember=form.remember_me.data)
             # Use validated next URL — never redirect blindly to request.args['next']
-            next_page = _safe_next(request.args.get('next'))
+            next_page = safe_redirect_url(request.args.get('next'))
             log_action(ACTION_LOGIN, 'User', user.id, user.username)
             flash(f'Welcome back, {user.username}!', 'success')
             return redirect(next_page)
@@ -282,7 +265,7 @@ def toggle_active(user_id):
         f'account {action_label} by {current_user.username}',
     )
     flash(f'User {user.username} has been {action_label}.', 'success')
-    return redirect(_safe_next(request.referrer) or url_for('auth.list_users'))
+    return redirect(safe_redirect_url(request.referrer, fallback=url_for('auth.list_users')))
 
 # ── Notification Matrix ───────────────────────────────────────────────────────
 

app/routes/customers.py

@@ -13,7 +13,6 @@ Provides a single screen to:
 """
 
 import logging
-from urllib.parse import urlparse
 from flask import Blueprint, render_template, redirect, url_for, flash, request, abort
 from flask_login import login_required, current_user
 from app import db
@@ -21,21 +20,10 @@ from app.models.user import User
 from app.models.project import Project, CustomerAssignment
 from app.models.facility import Facility
 from app.utils.forms import CustomerUserForm, CustomerAssignmentForm, CustomerInviteForm, SetPasswordForm
-from app.utils.decorators import admin_required, supervisor_required
+from app.utils.decorators import admin_required, supervisor_required, safe_redirect_url
 from app.utils.audit import log_action, ACTION_CREATE, ACTION_UPDATE, ACTION_DELETE
 from app.utils.scope import get_customer_scope
 
-
-def _safe_referrer(fallback: str) -> str:
-    """Return request.referrer only if it is a safe relative URL, else fallback."""
-    ref = request.referrer
-    if not ref:
-        return fallback
-    parsed = urlparse(ref)
-    if parsed.netloc or parsed.scheme:
-        return fallback
-    return ref
-
 logger = logging.getLogger(__name__)
 
 bp = Blueprint('customers', __name__, url_prefix='/customers')
@@ -496,7 +484,7 @@ def toggle_active(customer_id):
     log_action(ACTION_UPDATE, 'User', customer.id, customer.username,
                f'account {label} via customer_mgmt by {current_user.username}')
     flash(f'Customer "{customer.username}" has been {label}.', 'success')
-    return redirect(_safe_referrer(url_for('customers.index')))
+    return redirect(safe_redirect_url(request.referrer, fallback=url_for('customers.index')))
 
 
 # ── AJAX: facilities for a project (used by add-assignment form) ──────────────

app/routes/inspections.py

@@ -370,7 +370,8 @@ def execute(inspection_id):
             inspection.completed_at  = now_eastern()
 
             _save_responses(inspection, responses)
-            db.session.commit()
+            # NOTE: do NOT commit here — inspection fields and all notification
+            # rows are staged together and committed atomically below.
 
             inspection_link = url_for('inspections.view', inspection_id=inspection.id)
             score_display = f'{score:.1f}%' if score is not None else 'N/A'
@@ -388,7 +389,7 @@ def execute(inspection_id):
                 facility_id   = inspection.facility_id,
                 exclude_user_ids = {current_user.id},
             )
-            db.session.commit()
+            db.session.commit()  # Single atomic commit: inspection fields + notification rows
             log_action(ACTION_UPDATE, 'Inspection', inspection.id,
                        f'{inspection.template.name} @ {inspection.facility.name}',
                        f'status=completed; score={score}')

app/routes/scheduled_reports.py

@@ -50,13 +50,20 @@ bp = Blueprint('scheduled_reports', __name__, url_prefix='/scheduled-reports')
 # ── Helpers ───────────────────────────────────────────────────────────────────
 
 def _compute_next_send(frequency: str, from_dt: datetime = None) -> datetime:
-    """Return the next send datetime for a given frequency."""
+    """Return the next send datetime for a given frequency.
+
+    Monthly cadence always targets the 1st of the next month at 07:00.
+    The December branch is required because datetime.replace(month=13)
+    raises ValueError — incrementing the month directly is not safe in
+    general, but targeting day=1 avoids the separate last-day-of-month
+    (28/29/30/31) edge case that would affect mid-month scheduling.
+    """
     now = from_dt or now_eastern()
     if frequency == 'daily':
         return (now + timedelta(days=1)).replace(hour=7, minute=0, second=0, microsecond=0)
     if frequency == 'weekly':
         return (now + timedelta(weeks=1)).replace(hour=7, minute=0, second=0, microsecond=0)
-    # monthly: first of next month
+    # monthly: first of next month — December rolls over to Jan of next year.
     if now.month == 12:
         return now.replace(year=now.year + 1, month=1, day=1, hour=7, minute=0, second=0, microsecond=0)
     return now.replace(month=now.month + 1, day=1, hour=7, minute=0, second=0, microsecond=0)

app/utils/decorators.py

@@ -1,6 +1,32 @@
 from functools import wraps
-from flask import flash, redirect, url_for
+from flask import flash, redirect, url_for, request
 from flask_login import current_user
+from urllib.parse import urlparse
+
+
+# ── Open-redirect guard ───────────────────────────────────────────────────────
+
+def safe_redirect_url(url: str | None, fallback: str | None = None) -> str:
+    """Return *url* only if it is a safe relative URL on this host.
+
+    Rejects any URL that carries a network location (netloc) or an explicit
+    scheme, preventing open-redirect attacks where a crafted link contains
+    next=https://evil.com.
+
+    Parameters
+    ----------
+    url      : The candidate redirect target (may be None).
+    fallback : Returned when *url* is absent or unsafe.
+               Defaults to the dashboard index.
+    """
+    if fallback is None:
+        fallback = url_for('dashboard.index')
+    if not url:
+        return fallback
+    parsed = urlparse(url)
+    if parsed.netloc or parsed.scheme:
+        return fallback
+    return url
 
 def admin_required(f):
     @wraps(f)

app/utils/forms.py

@@ -1,5 +1,5 @@
 from flask_wtf import FlaskForm
-from flask_wtf.file import FileField, FileAllowed
+from flask_wtf.file import FileField, FileAllowed, MultipleFileField
 from wtforms import (StringField, PasswordField, SelectField, TextAreaField,
                      DecimalField, BooleanField, IntegerField, HiddenField,
                      RadioField)
@@ -174,7 +174,7 @@ class IssueUpdateForm(FlaskForm):
     assigned_to  = SelectField('Assign To', coerce=int, validators=[Optional()])
     update_notes = TextAreaField('Update Notes', validators=[Optional(), Length(max=1000)])
     result_notes = TextAreaField('Result Notes', validators=[Optional(), Length(max=2000)])
-    result_photos = FileField('Result Photos', validators=[
+    result_photos = MultipleFileField('Result Photos', validators=[
         Optional(),
         FileAllowed(['jpg','jpeg','png','gif'], 'Images only.')
     ])

app/utils/sla.py

@@ -110,9 +110,12 @@ def send_sla_alerts():
 
     logger = logging.getLogger(__name__)
 
+    # yield_per streams rows in batches of 100 rather than loading all open
+    # issues into memory at once.  At current scale this is a no-op difference,
+    # but it prevents a memory spike if the issue count grows large.
     open_issues = Issue.query.filter(
         Issue.status.in_(['open', 'in_progress', 'pending_verification'])
-    ).all()
+    ).yield_per(100)
 
     total_sent = 0
 

migrations/versions/phase15_audit_log_indexes.py

@@ -0,0 +1,62 @@
+"""phase15 — add indexes on audit_logs action and entity_type
+
+The audit log filter UI allows filtering by action and entity_type.
+Without indexes, every filter invocation performs a full table scan.
+As the log grows toward the 180/365-day purge threshold this degrades
+noticeably.  This migration adds individual indexes on both columns.
+
+A composite index on (action, entity_type, created_at) would be ideal
+for the combined-filter case, but individual indexes are added here to
+keep the migration additive and safe for re-run.  created_at already
+has an index from the model definition.
+
+Existence checks use information_schema so the migration is safe to
+re-run on any MySQL version (compatible back to 5.7).
+
+Revision ID: phase15_audit_log_indexes
+Revises: phase14_facility_created_at
+"""
+
+revision      = 'phase15_audit_log_indexes'
+down_revision = 'phase14_facility_created_at'
+branch_labels = None
+depends_on    = None
+
+from alembic import op
+from sqlalchemy import text
+
+
+def _index_exists(conn, table: str, index_name: str) -> bool:
+    """Return True if the named index already exists on the given table."""
+    result = conn.execute(text(
+        "SELECT COUNT(*) FROM information_schema.statistics "
+        "WHERE table_schema = DATABASE() "
+        "  AND table_name   = :table "
+        "  AND index_name   = :index"
+    ), {'table': table, 'index': index_name})
+    return result.scalar() > 0
+
+
+# (table, index_name, column)
+INDEXES = [
+    ('audit_logs', 'ix_audit_logs_action',      'action'),
+    ('audit_logs', 'ix_audit_logs_entity_type', 'entity_type'),
+]
+
+
+def upgrade():
+    conn = op.get_bind()
+    for table, index_name, column in INDEXES:
+        if not _index_exists(conn, table, index_name):
+            op.execute(text(
+                f'CREATE INDEX {index_name} ON {table} ({column})'
+            ))
+
+
+def downgrade():
+    conn = op.get_bind()
+    for table, index_name, _column in INDEXES:
+        if _index_exists(conn, table, index_name):
+            op.execute(text(
+                f'DROP INDEX {index_name} ON {table}'
+            ))

requirements.txt

@@ -1,3 +1,7 @@
+# Python >= 3.12 required.
+# Several route files use nested f-strings with inner single-quoted expressions
+# (e.g. f'...{x if x else '—'}...') — valid syntax in Python 3.12+ only.
+# Do not downgrade the interpreter without replacing those expressions first.
 Flask
 Flask-SQLAlchemy
 Flask-Login