Fix bugs found in second multi-agent review

XSS / security:
- markdown.js: sanitize footnote labels to a safe slug before using them in
  id/href attributes. Labels are captured before the HTML-escape pass, so a
  label like x"><img onerror=...> broke out → stored XSS (the earlier quote-
  escape fix didn't cover this path). Verified neutralized.
- RateLimitMiddleware: only trust X-Forwarded-For / X-Real-IP when REMOTE_ADDR
  is a configured trusted proxy, and use the rightmost (proxy-appended) entry.
  Previously any client could rotate XFF to escape the per-IP rate limit.
- .env.example: document TRUSTED_PROXIES so fresh deploys aren't fail-open on
  the Authelia forward-auth spoofing protection.

Correctness:
- notifications.php: my previous assigned-to LIKE fix anchored only on '}', so
  BULK assignments (logged {"assigned_to":N,"bulk_operation_id":..}) produced
  no "assigned to you" notification — now matches both '}' and ',' delimiters.
- notifications.php: implement the documented @mention notifications (query
  action_type='mention' rows for the current user); they were never delivered.
- NotificationHelper::notifyWatchers: guard unchecked prepare() so a missing
  ticket_watchers table can't fatal the request after its DB write committed.
- AuditLogModel::getTicketTimeline: JSON_UNQUOTE the extracted ticket_id so
  comment events actually match (string vs JSON-number comparison never did).
- AuditLogModel/audit_log.php: CSV export no longer silently truncates to the
  1000-row UI cap; uses a dedicated higher export limit.
- DashboardView: quick-preview drawer read .ticket-link from the title cell
  (which has none), so the title was always blank — use the cell text.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

.env.example

@@ -24,6 +24,14 @@ APP_DOMAIN=
 # Include all domains that can access this application
 ALLOWED_HOSTS=localhost,127.0.0.1
 
+# Trusted reverse proxy IP(s), comma-separated (e.g. the Authelia/nginx proxy).
+# STRONGLY RECOMMENDED in production: Authelia forward-auth (Remote-User /
+# Remote-Groups) and forwarded client IPs are only trusted when REMOTE_ADDR is
+# in this list. Leaving it empty disables that protection (relies solely on
+# network topology) and lets anything reaching PHP directly spoof admin login.
+# Exact IP match only (no CIDR). Example: TRUSTED_PROXIES=10.10.10.27
+TRUSTED_PROXIES=
+
 # Timezone (default: America/New_York)
 TIMEZONE=America/New_York
 

api/audit_log.php

@@ -46,8 +46,10 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
             $filters['ip_address'] = $_GET['ip_address'];
         }
 
-        // Get all matching logs (no limit for CSV export)
-        $result = $auditLogModel->getFilteredLogs($filters, 10000, 0);
+        // Get all matching logs for export. The forExport flag raises the cap
+        // (model clamps to its export limit) so the CSV isn't silently truncated
+        // to the 1000-row UI page limit.
+        $result = $auditLogModel->getFilteredLogs($filters, PHP_INT_MAX, 0, true);
         $logs = $result['logs'];
 
         // Set CSV headers

api/notifications.php

@@ -55,15 +55,18 @@ $assignSql = "SELECT
     AND al.entity_type = 'ticket'
     AND al.user_id != ?
     AND al.created_at >= DATE_SUB(NOW(), INTERVAL 7 DAY)
-    AND al.details LIKE ?
+    AND (al.details LIKE ? OR al.details LIKE ?)
   ORDER BY al.created_at DESC
   LIMIT 15";
 
 // Match the exact JSON value with a trailing delimiter so user 12 doesn't also
-// match 120/123/etc. The assign detail is logged as {"assigned_to":<int>}.
-$assignLike = '%"assigned_to":' . (int)$userId . '}%';
+// match 120/123/etc. Single assigns log {"assigned_to":5} (closing brace) while
+// bulk assigns log {"assigned_to":5,"bulk_operation_id":N} (comma) — match both.
+$assignId    = (int)$userId;
+$assignEnd   = '%"assigned_to":' . $assignId . '}%';
+$assignMid   = '%"assigned_to":' . $assignId . ',%';
 $stmt = $conn->prepare($assignSql);
-$stmt->bind_param('is', $userId, $assignLike);
+$stmt->bind_param('iss', $userId, $assignEnd, $assignMid);
 $stmt->execute();
 $assignRows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
 $stmt->close();
@@ -150,10 +153,32 @@ $stmt->execute();
 $statusRows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
 $stmt->close();
 
+// Query 4: @mentions of me (logged by add_comment.php as
+// action_type='mention', entity_type='user', entity_id=<mentioned user_id>).
+$mentionSql = "SELECT
+    al.audit_id AS log_id, al.action_type, al.entity_type, al.entity_id, al.details, al.created_at,
+    COALESCE(u.display_name, u.username, 'System') AS actor_name
+  FROM audit_log al
+  LEFT JOIN users u ON al.user_id = u.user_id
+  WHERE al.action_type = 'mention'
+    AND al.entity_type = 'user'
+    AND al.entity_id = ?
+    AND al.user_id != ?
+    AND al.created_at >= DATE_SUB(NOW(), INTERVAL 7 DAY)
+  ORDER BY al.created_at DESC
+  LIMIT 15";
+
+$mentionEntityId = (string)$userId;
+$stmt = $conn->prepare($mentionSql);
+$stmt->bind_param('si', $mentionEntityId, $userId);
+$stmt->execute();
+$mentionRows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
+$stmt->close();
+
 // Merge, deduplicate by log_id, sort by created_at desc
 $all = [];
 $seen = [];
-foreach (array_merge($assignRows, $commentRows, $statusRows) as $row) {
+foreach (array_merge($assignRows, $commentRows, $statusRows, $mentionRows) as $row) {
     $id = (int)$row['log_id'];
     if (isset($seen[$id])) {
         continue;
@@ -172,7 +197,7 @@ foreach ($all as $row) {
     $actionType = ($row['action_type'] === 'create' && $row['entity_type'] === 'comment')
         ? 'comment'
         : $row['action_type'];
-    $ticketId = ($actionType === 'comment')
+    $ticketId = ($actionType === 'comment' || $actionType === 'mention')
         ? ($details['ticket_id'] ?? 0)
         : $row['entity_id'];
     $isRead = $lastSeen && $row['created_at'] <= $lastSeen;
@@ -181,6 +206,7 @@ foreach ($all as $row) {
     $title = match ($actionType) {
         'assign'  => "{$row['actor_name']} assigned ticket #{$ticketId} to you",
         'comment' => "{$row['actor_name']} commented on ticket #{$ticketId}",
+        'mention' => "{$row['actor_name']} mentioned you on ticket #{$ticketId}",
         'update'  => (function () use ($row, $details, $ticketId) {
             // logTicketUpdate stores delta as {"status": {"from": "Open", "to": "In Progress"}}
             $from = $details['status']['from'] ?? ($details['old_value'] ?? '?');

assets/js/markdown.js

@@ -6,6 +6,13 @@
 function parseMarkdown(markdown) {
     if (!markdown) return '';
 
+    // Footnote labels are captured before the HTML-escape pass, so they must be
+    // sanitized to a safe slug before being interpolated into id/href attributes
+    // (otherwise a label like `x"><img onerror=...>` breaks out → stored XSS).
+    var fnSlug = function (label) {
+        return String(label).replace(/[^a-zA-Z0-9_-]/g, '-');
+    };
+
     // Footnotes — collect definitions and mark references with placeholders
     // (must happen before HTML escaping so <sup> tags don't get escaped)
     const footnotes = {};
@@ -146,7 +153,7 @@ function parseMarkdown(markdown) {
     // Restore footnote reference placeholders
     fnRefs.forEach(function(ref, i) {
         html = html.replace('%%FNREF' + i + '%%',
-            '<sup class="fn-ref"><a href="#fn-' + ref.label + '" id="fnref-' + ref.label + '">[' + ref.n + ']</a></sup>');
+            '<sup class="fn-ref"><a href="#fn-' + fnSlug(ref.label) + '" id="fnref-' + fnSlug(ref.label) + '">[' + ref.n + ']</a></sup>');
     });
 
     // Wrap in paragraph if not already wrapped
@@ -158,9 +165,9 @@ function parseMarkdown(markdown) {
     if (footnoteOrder.length) {
         html += '<hr class="fn-hr"><ol class="fn-list">';
         footnoteOrder.forEach(function(label, i) {
-            html += '<li id="fn-' + label + '" class="fn-item">' +
+            html += '<li id="fn-' + fnSlug(label) + '" class="fn-item">' +
                 parseMarkdown(footnotes[label]).replace(/<\/?p>/g, '') +
-                ' <a href="#fnref-' + label + '" class="fn-back">&#x21A9;</a></li>';
+                ' <a href="#fnref-' + fnSlug(label) + '" class="fn-back">&#x21A9;</a></li>';
         });
         html += '</ol>';
     }

helpers/NotificationHelper.php

@@ -168,10 +168,19 @@ class NotificationHelper
         if ($excludeUserId !== null) {
             $sql  = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ? AND tw.user_id != ?";
             $stmt = $conn->prepare($sql);
-            $stmt->bind_param("ii", $ticketId, $excludeUserId);
         } else {
             $sql  = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ?";
             $stmt = $conn->prepare($sql);
+        }
+        // Notifications are best-effort; if the watchers table is absent or the
+        // statement fails to prepare, skip silently rather than fataling the
+        // request that already committed its DB change.
+        if (!$stmt) {
+            return;
+        }
+        if ($excludeUserId !== null) {
+            $stmt->bind_param("ii", $ticketId, $excludeUserId);
+        } else {
             $stmt->bind_param("i", $ticketId);
         }
         $stmt->execute();

middleware/RateLimitMiddleware.php

@@ -41,19 +41,31 @@ class RateLimitMiddleware
      */
     private static function getClientIp(): string
     {
-        // Check for forwarded IP (behind proxy/load balancer)
-        $headers = ['HTTP_X_FORWARDED_FOR', 'HTTP_X_REAL_IP', 'HTTP_CLIENT_IP'];
-        foreach ($headers as $header) {
-            if (!empty($_SERVER[$header])) {
-                // Take the first IP in a comma-separated list
-                $ips = explode(',', $_SERVER[$header]);
-                $ip = trim($ips[0]);
-                if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) {
-                    return $ip;
-                }
+        $remoteAddr = $_SERVER['REMOTE_ADDR'] ?? '127.0.0.1';
+
+        // Forwarded headers are client-controlled, so only believe them when the
+        // request actually came from a trusted reverse proxy. Otherwise a client
+        // could rotate X-Forwarded-For each request to escape the per-IP limit.
+        $trusted = $GLOBALS['config']['TRUSTED_PROXIES'] ?? [];
+        if (empty($trusted) || !in_array($remoteAddr, $trusted, true)) {
+            return $remoteAddr;
+        }
+
+        // The trusted proxy appends the connecting client to X-Forwarded-For, so
+        // the RIGHTMOST entry is the IP it observed (a client-supplied prefix is
+        // not trustworthy). X-Real-IP is set by the proxy itself.
+        if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
+            $ips = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
+            $ip = trim(end($ips));
+            if (filter_var($ip, FILTER_VALIDATE_IP)) {
+                return $ip;
             }
         }
-        return $_SERVER['REMOTE_ADDR'] ?? '127.0.0.1';
+        if (!empty($_SERVER['HTTP_X_REAL_IP']) && filter_var($_SERVER['HTTP_X_REAL_IP'], FILTER_VALIDATE_IP)) {
+            return trim($_SERVER['HTTP_X_REAL_IP']);
+        }
+
+        return $remoteAddr;
     }
 
     /**

models/AuditLogModel.php

@@ -10,6 +10,9 @@ class AuditLogModel
     /** @var int Maximum allowed limit for pagination */
     private const MAX_LIMIT = 1000;
 
+    /** @var int Maximum rows for a CSV/forensic export (higher than the UI cap) */
+    private const EXPORT_LIMIT = 100000;
+
     /** @var int Default limit for pagination */
     private const DEFAULT_LIMIT = 100;
 
@@ -36,12 +39,12 @@ class AuditLogModel
      * @param int $limit Requested limit
      * @return int Validated limit
      */
-    private function validateLimit(int $limit): int
+    private function validateLimit(int $limit, int $max = self::MAX_LIMIT): int
     {
         if ($limit < 1) {
             return self::DEFAULT_LIMIT;
         }
-        return min($limit, self::MAX_LIMIT);
+        return min($limit, $max);
     }
 
     /**
@@ -534,7 +537,7 @@ class AuditLogModel
              FROM audit_log al
              LEFT JOIN users u ON al.user_id = u.user_id
              WHERE (al.entity_type = 'ticket' AND al.entity_id = ?)
-                OR (al.entity_type = 'comment' AND JSON_EXTRACT(al.details, '$.ticket_id') = ?)
+                OR (al.entity_type = 'comment' AND JSON_UNQUOTE(JSON_EXTRACT(al.details, '$.ticket_id')) = ?)
              ORDER BY al.created_at DESC"
         );
         $stmt->bind_param("ss", $ticketId, $ticketId);
@@ -561,10 +564,11 @@ class AuditLogModel
      * @param int $offset Offset for pagination
      * @return array Array containing logs and total count
      */
-    public function getFilteredLogs($filters = [], $limit = 50, $offset = 0)
+    public function getFilteredLogs($filters = [], $limit = 50, $offset = 0, $forExport = false)
     {
-        // Validate pagination parameters
-        $limit = $this->validateLimit((int)$limit);
+        // Validate pagination parameters. Exports allow a much higher cap so a
+        // forensic/compliance CSV isn't silently truncated to the UI page limit.
+        $limit = $this->validateLimit((int)$limit, $forExport ? self::EXPORT_LIMIT : self::MAX_LIMIT);
         $offset = $this->validateOffset((int)$offset);
 
         $whereConditions = [];

views/DashboardView.php

@@ -1342,7 +1342,7 @@ if (advForm) advForm.addEventListener('submit', function(e) {
     var o = hasCheckbox ? 1 : 0; // column offset for checkbox col
 
     var priority  = cells[1 + o] ? cells[1 + o].textContent.trim() : '';
-    var title     = cells[2 + o] ? cells[2 + o].querySelector('.ticket-link')?.textContent.trim() || '' : '';
+    var title     = cells[2 + o] ? cells[2 + o].textContent.trim() : '';
     var category  = cells[3 + o] ? cells[3 + o].textContent.trim() : '';
     var typeVal   = cells[4 + o] ? cells[4 + o].textContent.trim() : '';
     var status    = cells[5 + o] ? cells[5 + o].textContent.trim().replace(/^\s*●\s*/, '') : '';