Fix 15 audit findings: browser concurrency, silent verification failures, auth gaps

Full read-through of the project. Grouped by what actually breaks:

Browser concurrency (the invariant the design rests on)
- Every Playwright session now goes through scheduler.submit() onto the
  single worker queue. "Refresh Rules", cookie import and the boot sync
  were calling automation.py directly from Flask request threads, so
  with gunicorn's threads=4 a refresh during a scheduled run opened a
  second Chromium and both wrote /data/auth_state.json — which
  Playwright writes itself, non-atomically. A torn write there reads
  back as "session expired" and costs a pointless cookie re-import.
- Request handlers that need a result use run_browser_task(), which
  waits REQUEST_WAIT_SECONDS (90) then raises BrowserBusy -> 503
  without cancelling the work. Previously a slow in-request scrape could
  exceed gunicorn's 120s timeout; killing that worker takes the
  APScheduler jobstore and run queue with it, silently dropping every
  pending verification poll.

Verification correctness (the feature that exists to catch silent failures)
- run_rule() no longer starts a poll loop when the watermark capture
  itself failed. watermark_id was None both for "capture failed" and
  "rule has no prior activity", and check_completion() skips its
  comparison on None — so a failed capture matched the *previous* run's
  Complete entry and reported a false "verified", the exact class of bug
  the verification step was added to catch.
- A refresh now reconciles schedules.json against the live rule list.
  Rule ids are stable, names are not, and the cached name is what the
  Activity-page match keys on: a renamed rule kept running while
  reporting "Not Confirmed" forever, training you to ignore the signal.
  A deleted rule was worse — its card disappears while its cron jobs
  keep firing, with no UI left to disable them. Skipped on an empty
  scrape so a MassPrice redesign can't delete every saved schedule.
- The dashboard's session banner reads a session_state.json flag set by
  _ensure_authenticated() instead of guessing from run_history.json.
  list_rules() writes no history entry, so a failed refresh — often the
  first symptom — never raised the banner, despite the docs saying it
  covered "run/refresh".

Scheduler robustness
- rebuild_jobs() parses and adds each entry under its own guard. It runs
  *after* clearing existing jobs, so one malformed entry escaping as an
  exception left the scheduler holding no jobs at all — every schedule
  silently dead until the next restart. _parse_entry() also accepts
  HH:MM:SS (some browsers emit seconds, which the old two-value unpack
  crashed on) and range-checks hour/minute.
- The API validates entries at save time rather than storing something
  every rebuild skips, and the frontend surfaces save failures instead
  of discarding them.
- An unknown TZ logs and falls back to UTC instead of crash-looping the
  container with a traceback that doesn't obviously point at .env.

Auth and web hygiene
- Empty-but-set LOGIN_PASSWORD/SECRET_KEY now fall back to their
  defaults. docker-compose always sets these, so a blank .env value
  arrived as "" — and compare_digest("", "") is True, i.e. no login at
  all. An empty submitted password is rejected outright too.
- Password compared as bytes: compare_digest raises TypeError on
  non-ASCII str, turning any accented character into a 500.
- CSRF token per session, in a meta tag, echoed as X-CSRF-Token on all
  four POST endpoints; explicit SameSite=Lax/HttpOnly.
- /api/* answers 401 instead of redirecting. fetch() follows a 302 to
  /login, gets 200 HTML, and reads it as success — "Run Now" reported
  "Queued" for runs that were never queued.
- ?next= is restricted to local paths (open redirect).

Smaller correctness fixes
- Timestamps render in TZ via a localtime filter; they were shown as raw
  UTC while schedules run in America/Phoenix.
- Activity's 24h filter excludes unparseable timestamps rather than
  including them, and treats naive values as UTC instead of raising.
- SameSite=None cookies are forced Secure — Chromium drops them
  otherwise, silently, possibly losing the one cookie carrying the login.
- The Activity selector escapes the rule name, so a name containing a
  quote no longer produces an unparseable selector.
- pytz pinned explicitly; it only ever resolved as an apscheduler 3.x
  transitive dep, which 4.x drops.

Docs corrected where they disagreed with the code: auth_state.json is
written by Playwright, not through storage.py's atomic helper; the
banner's real trigger; and the deploy skill's claim that a rename causes
"rule id not found" (it can't — schedules are keyed by the stable id).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6TgffxTvYhqxykAbVQg2v

automation.py

@@ -81,6 +81,22 @@ def _is_logged_out(page):
     return "/Rule" not in page.url
 
 
+def _set_session_expired(expired):
+    """Track whether the saved session is currently known-bad, so the
+    dashboard banner (app.py's _session_warning()) can reflect a failed
+    "Refresh Rules" too — list_rules() writes no run_history entry, so
+    inferring this from run history alone missed refresh failures
+    entirely. Written only on a state change, since the success path
+    runs on every rule run and every verification poll."""
+    state = storage.load("session_state.json", {})
+    if bool(state.get("expired")) == bool(expired):
+        return
+    payload = {"expired": bool(expired)}
+    if expired:
+        payload["at"] = datetime.now(timezone.utc).isoformat()
+    storage.save("session_state.json", payload)
+
+
 def _notify_session_expired_once():
     """Send the email alert only on the first failure after the session
     goes bad, not on every scheduled run that fails while it's down —
@@ -107,11 +123,13 @@ def _notify_session_expired_once():
 def _ensure_authenticated(page):
     page.goto(f"{BASE_URL}/Rule/List")
     if _is_logged_out(page):
+        _set_session_expired(True)
         _notify_session_expired_once()
         raise AutomationError(
             "MassPrice session expired or missing — log in via the "
             "Session page and paste in a fresh cookie export."
         )
+    _set_session_expired(False)
 
 
 def session_status():
@@ -173,8 +191,11 @@ def _find_latest_activity(page, rule_name):
     "new completion appeared" check is done reliably."""
     page.goto(f"{BASE_URL}/Activity/List")
     page.wait_for_timeout(1000)
+    # json.dumps() quotes and escapes the name the same way CSS does for
+    # `"` and `\`; interpolating it raw meant a rule named e.g. `6" Box`
+    # produced an unparseable selector and blew up the whole check.
     rows = page.locator("div.activity[data-activitylogid]").filter(
-        has=page.locator(f'.activity__name[title="{rule_name}"]')
+        has=page.locator(f".activity__name[title={json.dumps(rule_name)}]")
     )
     if rows.count() == 0:
         return None, None
@@ -195,6 +216,10 @@ def run_rule(rule_id, rule_name=None):
     status = "failure"
     message = ""
     watermark_id = None
+    # Distinct from `watermark_id is None`, which is also the legitimate
+    # "this rule has no prior Activity entries at all" result. Only a
+    # successful capture makes the later comparison meaningful.
+    watermark_captured = False
     with sync_playwright() as p:
         browser = p.chromium.launch(headless=True, args=LAUNCH_ARGS)
         try:
@@ -208,10 +233,14 @@ def run_rule(rule_id, rule_name=None):
             # apart from "the same old entry is still sitting there".
             # Best-effort: if this fails for any reason, proceed with the
             # actual run anyway — the run itself matters more than the
-            # optional follow-up verification.
+            # optional follow-up verification — but skip verification
+            # entirely rather than run it without a watermark, which
+            # would match the *previous* run's Complete entry and report
+            # a false "verified" for a run that may never have landed.
             if rule_name:
                 try:
                     watermark_id, _ = _find_latest_activity(page, rule_name)
+                    watermark_captured = True
                 except Exception:
                     logger.exception("Failed to capture activity watermark for %s", rule_name)
                 page.goto(f"{BASE_URL}/Rule/List")
@@ -256,9 +285,13 @@ def run_rule(rule_id, rule_name=None):
             browser.close()
 
     entry_id = _log_history(rule_id, rule_name, status, message)
-    will_verify = status == "sent" and bool(rule_name)
+    will_verify = status == "sent" and bool(rule_name) and watermark_captured
     if will_verify:
         set_verification_pending(entry_id)
+    elif status == "sent":
+        logger.warning(
+            "Run for rule %s sent but not verifiable (no activity watermark)", rule_id
+        )
 
     return {
         "status": status,
@@ -273,7 +306,12 @@ def check_completion(rule_name, watermark_id):
     """One-shot check: has a new "Complete" entry appeared for rule_name
     past watermark_id yet? Returns ("complete", "") or ("not_yet", msg).
     Pure check — no storage side effects; the caller (scheduler.py's
-    poll loop) decides what that means for the stored history entry."""
+    poll loop) decides what that means for the stored history entry.
+
+    A watermark_id of None means the rule had no Activity entries at all
+    when the run was submitted, so any entry that appears is new. That's
+    only ever reached via a *successful* watermark capture — run_rule()
+    refuses to start a poll loop when the capture itself failed."""
     with sync_playwright() as p:
         browser = p.chromium.launch(headless=True, args=LAUNCH_ARGS)
         try:
@@ -371,6 +409,14 @@ def _convert_cookie(raw):
     Playwright's storage_state cookie shape."""
     expires = raw.get("expirationDate")
     same_site_raw = (raw.get("sameSite") or "unspecified").lower()
+    same_site = _SAME_SITE_MAP.get(same_site_raw, "Lax")
+    secure = bool(raw.get("secure", False))
+    # Chromium rejects a SameSite=None cookie that isn't also Secure, and
+    # drops it from the imported session without any error — which can
+    # quietly cost the one cookie that carried the login. Everything here
+    # is https-only in practice, so forcing the flag is safe.
+    if same_site == "None":
+        secure = True
     return {
         "name": raw["name"],
         "value": raw["value"],
@@ -378,8 +424,8 @@ def _convert_cookie(raw):
         "path": raw.get("path", "/"),
         "expires": expires if expires is not None else -1,
         "httpOnly": bool(raw.get("httpOnly", False)),
-        "secure": bool(raw.get("secure", False)),
-        "sameSite": _SAME_SITE_MAP.get(same_site_raw, "Lax"),
+        "secure": secure,
+        "sameSite": same_site,
     }
 
 
@@ -433,6 +479,7 @@ def import_cookies(raw_text):
                     "itself, after finishing login, not from the login page."
                 )
             _save_state(context)
+            _set_session_expired(False)
             storage.save("notify_state.json", {"session_expired_notified": False})
         finally:
             browser.close()