Merge pull request #254 from RivianTrackr/claude/plugin-review-sweep-1.45.0

Plugin review sweep: security, performance, a11y (1.45.0)

Author: RivianTrackr

CHANGELOG.md

@@ -4,6 +4,84 @@ All notable changes to the Rivian Tire Guide plugin will be documented in this f
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
+## [1.45.0] - 2026-04-15
+
+Plugin review sweep — security hardening, performance, and accessibility fixes
+across the PHP backend and JS frontend. No feature changes, no schema changes.
+
+### Security
+- **AI API key can live in `wp-config.php`** — Define `RTG_ANTHROPIC_API_KEY` to
+  keep the Anthropic credential out of `wp_options`. The plugin settings field
+  still works as a fallback when the constant is not set. (`includes/class-rtg-ai.php`)
+- **Guest rate limiting no longer trusts raw `REMOTE_ADDR`** — Replaced the naive
+  IP check with a per-visitor fingerprint (`md5(IP + truncated User-Agent)`) so a
+  single spoofed IP from multiple clients can't bypass the limit. Logged-in users
+  are fingerprinted by user ID. (`includes/class-rtg-ajax.php`)
+- **Admin tire-delete hardens `$_GET['tire_id']`** — Input is now unslashed,
+  sanitized, and validated before the nonce check, rejecting malformed IDs with
+  a `wp_die()` instead of silently building an unpredictable nonce token.
+  (`includes/class-rtg-admin.php`)
+- **Security event logging** — Nonce failures and rate-limit hits now emit a
+  compact `[RTG] {json}` line to `error_log` when `WP_DEBUG` is on, giving an
+  audit trail without changing user-facing messages. (`includes/class-rtg-ajax.php`)
+
+### Performance
+- **Dashboard stats are now cached** — `RTG_Database::get_dashboard_stats()` runs
+  roughly ten aggregation queries; results are memoised in a 5-minute transient
+  and invalidated automatically by `flush_cache()` on any tire/rating write.
+  (`includes/class-rtg-database.php`)
+- **N+1 eliminated in AI context build** — `build_tire_context()` used to call
+  `get_all_tires()` then a second `get_tire_ratings()` query. Added
+  `get_tires_with_ratings()`, a single `LEFT JOIN` query returning tires with
+  aggregated rating columns, and switched the AI path to use it. (`includes/class-rtg-database.php`, `includes/class-rtg-ai.php`)
+- **Link checker `set_time_limit` is bounded and guarded** — The 300s / 120s
+  hard-coded ceilings were replaced with `BATCH_SIZE × (REQUEST_TIMEOUT + 2)`
+  and `PROGRESS_BATCH_SIZE × (REQUEST_TIMEOUT + 2)`, wrapped in a
+  `function_exists('set_time_limit')` check for hosts that disable it.
+  (`includes/class-rtg-ajax.php`)
+- **Card cache LRU tightened to 20 entries** — Previous limit was 100 with a
+  20-entry batch eviction; that effectively held 100 cloned DOM subtrees plus
+  their image references in memory. Dropped to 20 with single-entry LRU eviction
+  using `Map`'s insertion-order iteration. (`frontend/js/modules/cards.js`)
+- **`IntersectionObserver` is now disposable** — Added `disconnectImageObserver()`
+  and a `pagehide` listener so the shared observer is released on teardown.
+  (`frontend/js/modules/cards.js`)
+
+### Accessibility
+- **Pagination controls are announced properly** — `#paginationControls` now
+  carries `role="navigation"` + `aria-label`, each button has a descriptive
+  `aria-label`, and the page info span is a `role="status"` / `aria-live="polite"`
+  region so screen readers announce page changes. (`frontend/js/modules/filters.js`)
+- **Image modal traps focus and returns focus on close** — Opening the modal
+  remembers the launching element, focuses the dialog, traps Tab/Shift+Tab
+  inside, and returns focus to the original element when closed. (`frontend/js/modules/image-modal.js`)
+
+### Refactors
+- **Single-source-of-truth `RTG_Database::validate_tire_id()`** — The
+  `preg_match('/^[a-zA-Z0-9\-_]+$/', ...) && strlen <= 50` rule was duplicated
+  across 15+ AJAX handlers and the database helper. Extracted to one static
+  method and threaded through all callers. (`includes/class-rtg-database.php`,
+  `includes/class-rtg-ajax.php`, `includes/class-rtg-admin.php`)
+- **`get_filtered_tires()` split into builders** — Extracted
+  `build_filter_where_clause()` and `build_filter_sort_clause()` so the main
+  method is now a short count + fetch pair. No behaviour change, but the query
+  body is dramatically easier to read and modify. (`includes/class-rtg-database.php`)
+- **Memory-safe search listener binding** — `initializeSmartSearch()` used
+  `cloneNode(true)` to strip old handlers, which orphaned references held in
+  module-level caches. Now tracks the handler and calls `removeEventListener()`.
+  (`frontend/js/modules/search.js`)
+- **Pagination event delegation** — Replaced per-render `.onclick` assignments
+  with a single delegated click handler on `#paginationControls`, using
+  `data-pagination="prev|next"` attributes. (`frontend/js/modules/filters.js`)
+- **Inline `style.cssText` replaced with CSS classes** — `.info-tooltip-trigger`
+  and `.tire-card-tag-list` now live in `rivian-tires.css`, eliminating the
+  hardcoded style blocks and mouseenter/mouseleave handlers in `cards.js`.
+  Hover and focus states also get proper focus-visible treatment.
+  (`frontend/css/rivian-tires.css`, `frontend/js/modules/cards.js`)
+
+### Changed
+- **Plugin version** — Bumped to 1.45.0.
+
 ## [1.44.2] - 2026-04-08
 
 ### Fixed

frontend/css/rivian-tires.css

@@ -2860,7 +2860,45 @@ a.rtg-user-review-tire-link:hover {
 }
 .rtg-pagination-info {
   font-size: 14px;
-  color: var(--rtg-text-muted, #8493a5);
+  font-weight: 500;
+  display: flex;
+  align-items: center;
+  color: var(--rtg-text-primary, #e5e7eb);
+}
+
+/* Info tooltip button used inside efficiency badges and spec rows.
+   Previously rendered via style.cssText in JS — now a proper class. */
+.info-tooltip-trigger {
+  background: none;
+  border: none;
+  color: var(--rtg-text-muted, #9ca3af);
+  font-size: 14px;
+  cursor: pointer;
+  padding: 2px;
+  border-radius: 50%;
+  width: 20px;
+  height: 20px;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  transition: color 0.2s ease, background-color 0.2s ease;
+}
+.info-tooltip-trigger:hover,
+.info-tooltip-trigger:focus-visible {
+  color: var(--rtg-accent, #fba919);
+  background-color: color-mix(in srgb, var(--rtg-accent, #fba919) 10%, transparent);
+}
+.info-tooltip-trigger:focus-visible {
+  outline: 2px solid var(--rtg-accent, #fba919);
+  outline-offset: 2px;
+}
+
+/* Tag list row inside tire cards. */
+.tire-card-tag-list {
+  display: flex;
+  flex-wrap: wrap;
+  gap: 4px;
+  justify-content: flex-end;
 }
 
 /* === Favorite Button Overlay === */

frontend/css/rivian-tires.min.css

@@ -5,7 +5,7 @@
  */
 
 import { state, ROWS_PER_PAGE } from './state.js';
-import { rtgColor, rtgIcon, escapeHTML, safeString, getDOMElement } from './helpers.js';
+import { rtgIcon, escapeHTML, safeString, getDOMElement } from './helpers.js';
 import { VALIDATION_PATTERNS, NUMERIC_BOUNDS, validateNumeric, safeImageURL, safeLinkURL, safeReviewLinkURL } from './validation.js';
 import { TOOLTIP_DATA, createInfoTooltip } from './tooltips.js';
 import { createRatingHTML } from './ratings.js';
@@ -48,6 +48,23 @@ export function observeCardImages(container) {
   images.forEach(img => imageObserver.observe(img));
 }
 
+/**
+ * Disconnect the shared IntersectionObserver. Call when the guide is
+ * being torn down or the card container is about to be cleared in bulk.
+ */
+export function disconnectImageObserver() {
+  if (imageObserver) {
+    imageObserver.disconnect();
+    imageObserver = null;
+  }
+}
+
+// Clean up when the page is unloaded so we don't hold the observer across
+// page transitions in persistent-navigation setups.
+if (typeof window !== 'undefined') {
+  window.addEventListener('pagehide', disconnectImageObserver, { once: true });
+}
+
 function removeSkeletonLoader() {
   const skeleton = document.getElementById('rtg-skeleton-loader');
   if (skeleton) skeleton.remove();
@@ -364,31 +381,6 @@ export function createSingleCard(row) {
       infoButton.dataset.tooltipKey = 'Efficiency Score';
       infoButton.setAttribute('aria-label', 'More info about Efficiency Score');
       infoButton.setAttribute('type', 'button');
-      infoButton.style.cssText = `
-        background: none;
-        border: none;
-        color: var(--rtg-text-muted);
-        font-size: 14px;
-        cursor: pointer;
-        padding: 2px;
-        border-radius: 50%;
-        width: 20px;
-        height: 20px;
-        display: flex;
-        align-items: center;
-        justify-content: center;
-        transition: all 0.2s ease;
-      `;
-
-      infoButton.addEventListener('mouseenter', () => {
-        infoButton.style.color = rtgColor('accent');
-        infoButton.style.backgroundColor = `color-mix(in srgb, ${rtgColor('accent')} 10%, transparent)`;
-      });
-
-      infoButton.addEventListener('mouseleave', () => {
-        infoButton.style.color = rtgColor('text-muted');
-        infoButton.style.backgroundColor = 'transparent';
-      });
 
       scoreSection.appendChild(scoreText);
       scoreSection.appendChild(infoButton);
@@ -432,31 +424,6 @@ export function createSingleCard(row) {
         }
         roamerInfoBtn.setAttribute('aria-label', 'More info about Real-World Efficiency');
         roamerInfoBtn.setAttribute('type', 'button');
-        roamerInfoBtn.style.cssText = `
-          background: none;
-          border: none;
-          color: var(--rtg-text-muted);
-          font-size: 14px;
-          cursor: pointer;
-          padding: 2px;
-          border-radius: 50%;
-          width: 20px;
-          height: 20px;
-          display: flex;
-          align-items: center;
-          justify-content: center;
-          transition: all 0.2s ease;
-        `;
-
-        roamerInfoBtn.addEventListener('mouseenter', () => {
-          roamerInfoBtn.style.color = rtgColor('accent');
-          roamerInfoBtn.style.backgroundColor = `color-mix(in srgb, ${rtgColor('accent')} 10%, transparent)`;
-        });
-
-        roamerInfoBtn.addEventListener('mouseleave', () => {
-          roamerInfoBtn.style.color = rtgColor('text-muted');
-          roamerInfoBtn.style.backgroundColor = 'transparent';
-        });
 
         roamerScore.appendChild(roamerText);
         roamerScore.appendChild(roamerInfoBtn);
@@ -527,8 +494,7 @@ export function createSingleCard(row) {
       tagLabel.textContent = 'Tags';
 
       const tagValue = document.createElement('span');
-      tagValue.className = 'tire-card-spec-value';
-      tagValue.style.cssText = 'display: flex; flex-wrap: wrap; gap: 4px; justify-content: flex-end;';
+      tagValue.className = 'tire-card-spec-value tire-card-tag-list';
 
       tagList.forEach(tag => {
         const tagEl = document.createElement('span');
@@ -593,10 +559,14 @@ export function createSingleCard(row) {
 
   card.appendChild(actionsContainer);
 
-  if (state.cardCache.size >= 100) {
-    // Evict oldest 20 entries to avoid thrashing on every new card.
-    const keysToDelete = [...state.cardCache.keys()].slice(0, 20);
-    keysToDelete.forEach(k => state.cardCache.delete(k));
+  // Tight LRU ceiling. Map iteration order is insertion order, so evicting
+  // the first key removes the oldest entry. Kept small (20) because cloned
+  // card nodes retain their subtree and we don't want them holding images
+  // off the GC indefinitely.
+  const CARD_CACHE_MAX = 20;
+  if (state.cardCache.size >= CARD_CACHE_MAX) {
+    const firstKey = state.cardCache.keys().next().value;
+    if (firstKey !== undefined) state.cardCache.delete(firstKey);
   }
   state.cardCache.set(cacheKey, card.cloneNode(true));
 

frontend/js/modules/cards.js

@@ -5,7 +5,7 @@
  */
 
 import { state, filterIndexes, ROWS_PER_PAGE } from './state.js';
-import { rtgColor, rtgIcon, safeString, getDOMElement, debounce, updateSliderBackground, styleButton, escapeHTML } from './helpers.js';
+import { rtgIcon, safeString, getDOMElement, debounce, updateSliderBackground, styleButton, escapeHTML } from './helpers.js';
 import { VALIDATION_PATTERNS, NUMERIC_BOUNDS, ALLOWED_SORT_OPTIONS, sanitizeInput, validateNumeric } from './validation.js';
 import { RTG_ANALYTICS } from './analytics.js';
 import { renderCards, preloadNextPageImages } from './cards.js';
@@ -269,41 +269,70 @@ function render() {
   });
 }
 
+// Delegated once at module load: every click inside #paginationControls
+// checks for a data-pagination attribute and dispatches, so re-rendering
+// the pagination buttons doesn't leak handlers.
+let paginationDelegated = false;
+function ensurePaginationDelegation() {
+  if (paginationDelegated) return;
+  const container = getDOMElement("paginationControls");
+  if (!container) return;
+  paginationDelegated = true;
+  container.addEventListener("click", (e) => {
+    const btn = e.target.closest('[data-pagination]');
+    if (!btn || btn.disabled) return;
+    const action = btn.dataset.pagination;
+    if (action === "prev" && state.currentPage > 1) {
+      state.currentPage--;
+    } else if (action === "next") {
+      state.currentPage++;
+    } else {
+      return;
+    }
+    render();
+    updateURLFromFilters();
+    const filterTop = getDOMElement("filterTop");
+    if (filterTop) filterTop.scrollIntoView({ behavior: "smooth" });
+  });
+}
+
 function renderPaginationControls(totalRows) {
   const container = getDOMElement("paginationControls");
   container.innerHTML = "";
   const totalPages = Math.ceil(totalRows.length / ROWS_PER_PAGE);
-  if (totalPages <= 1) return;
+  if (totalPages <= 1) {
+    container.removeAttribute("role");
+    container.removeAttribute("aria-label");
+    return;
+  }
+
+  ensurePaginationDelegation();
+  container.setAttribute("role", "navigation");
+  container.setAttribute("aria-label", "Tire list pagination");
 
   const prev = document.createElement("button");
+  prev.type = "button";
   prev.textContent = "Previous";
   prev.disabled = state.currentPage === 1;
+  prev.dataset.pagination = "prev";
+  prev.setAttribute("aria-label", `Previous page (currently on page ${state.currentPage} of ${totalPages})`);
   styleButton(prev);
-  prev.onclick = () => {
-    state.currentPage--;
-    render();
-    updateURLFromFilters();
-    const filterTop = getDOMElement("filterTop");
-    if (filterTop) filterTop.scrollIntoView({ behavior: "smooth" });
-  };
   container.appendChild(prev);
 
   const pageInfo = document.createElement("span");
+  pageInfo.className = "rtg-pagination-info";
+  pageInfo.setAttribute("role", "status");
+  pageInfo.setAttribute("aria-live", "polite");
   pageInfo.textContent = `Page ${state.currentPage} of ${totalPages}`;
-  pageInfo.style.cssText = `color: ${rtgColor('text-primary')}; font-weight: 500; display: flex; align-items: center;`;
   container.appendChild(pageInfo);
 
   const next = document.createElement("button");
+  next.type = "button";
   next.textContent = "Next";
   next.disabled = state.currentPage === totalPages;
+  next.dataset.pagination = "next";
+  next.setAttribute("aria-label", `Next page (currently on page ${state.currentPage} of ${totalPages})`);
   styleButton(next);
-  next.onclick = () => {
-    state.currentPage++;
-    render();
-    updateURLFromFilters();
-    const filterTop = getDOMElement("filterTop");
-    if (filterTop) filterTop.scrollIntoView({ behavior: "smooth" });
-  };
   container.appendChild(next);
 }