Security hardening — round 7: 11 confirmed defects

.abcd/work/DECISIONS.md

@@ -8,3 +8,4 @@ Dated, one-line summaries of each autonomous security-hardening round.
 - 2026-08-05 — Round 4: fixed 7 confirmed defects — (1) an entity `rating`/`averageRating` of `null` no longer crashes the whole collection load (`isStructuredRating` is null- and type-safe, `normaliseRating`/`formatRating` coerce malformed values, and the load treats `null` as "no rating"); (2) a relationship `target` or entity field named after an `Object.prototype` member (e.g. `"toString"`) no longer resolves to an inherited function and throws during load (`resolveReference` and the implicit-relationship path use `Object.hasOwn` guards); (3) a non-array `videos` field no longer throws via `.map()`; (4) a non-string resolved platform `title` no longer throws via `.replace()`; (5) `detectNumericFields` runs a single pass over each card's own fields instead of a keys×cards nested scan, so a hostile collection with many uniquely-named fields can no longer freeze the main thread for tens of seconds; (6) `SourceIcon` detects known sources by URL hostname (anchored on the dot boundary) rather than a substring of the whole URL, so an attacker link can no longer be branded with a trusted source's icon/name via a path like `evil.example/en.wikipedia.org/x`; (7) the pre-commit PII and British-English gates read staged paths NUL-terminated with `core.quotePath=false` and fail closed on unreadable blobs, closing a silent bypass for files whose names contain non-ASCII/backslash/quote/newline characters. 24 reproduction tests added. Deferred (confirmed but out of scope for a smallest-diff round): collection `forced` settings persist into global state and are never reverted — the correct fix is a collection-scoped overlay plus a persisted-store migration, carrying moderate/high regression risk. Refuted: stale `dist/` analyser artefact leak (publish root is a clean Cloudflare Pages checkout with `ANALYZE` off).
 - 2026-08-05 — Round 5: fixed 4 confirmed defects — (1) `normaliseDetailUrls` guards each array element with `isDetailLink` before reading `.url`, so an entity `detailUrls: [null]` (an unvalidated `.loose()` passthrough) no longer throws and rejects the whole collection load; (2) entity files load through a fixed-size concurrency pool with a generous id cap (`ENTITY_FETCH_CONCURRENCY`/`MAX_ENTITY_IDS`) instead of a `Promise.all` over the whole untrusted `index.json`, closing a main-thread stall / CDN request-amplification DoS from a hostile index listing tens of thousands of ids; (3) Snap Ranking's `initGame` refuses to start above `MAX_UNIQUE_VALUES` distinct values (one guess button is rendered per value; the default per-card `order` field on a large collection otherwise renders tens of thousands of DOM buttons and freezes the tab); (4) the "Hard Reset" clears every `itemdeck-`-prefixed localStorage key and all three IndexedDB databases (app DB, the idb-keyval cached-collection store, and the plugin cache DB) via `clearAllPersistedData`, honouring the dialog's "delete everything" promise instead of leaving cached remote collections, imported data, config and game state behind. 11 reproduction tests added. Deferred (confirmed, own dedicated round): collection `forced` settings still persist into global state without a revert path — the smallest safe fix is a symmetric backup/restore mirroring `_mechanicOverridesBackup` (snapshot the touched keys on apply, restore on source change, without clearing `appliedCollectionDefaultsSourceId`). Nitpicks/defence-in-depth noted (not fixed): `fontUrl`/`cardBackBackgroundImage` import fields use bare `z.url()` (unreachable — no consumer), `validateForcedSettings` enum allow-lists are stale, `fieldDiscovery` bracket lookups lack `Object.hasOwn` (latent — provider unmounted), image fetch has no size/timeout pre-check, and `cardBackBackground`/`usePlaceholderImages` are absent from `partialize`.
 - 2026-08-06 — Round 6: fixed 7 confirmed defects — (1) collection `forced` settings now snapshot the user's own displaced values per source and restore them when the active source changes, with crash recovery via `onRehydrateStorage` (mirroring `_mechanicOverridesBackup`); previously viewing one hostile allowlisted-CDN source once, reachable via a single unconfirmed `/gh/<user>/` link, permanently overwrote the visitor's global display config with no revert path (`clearCollectionForcedSettings` had zero callers) — the dead action is repurposed into `restoreCollectionForcedSettings`; (2) untrusted entity text fields (`summary`, resolved platform `title`/`shortTitle`/`summary`/`year`) are coerced to strings at the `useCollection` source instead of cast, so an object-typed field (e.g. an i18n `{en: "…"}` value) can no longer reach JSX as a child and crash the whole collection view — the device badge sink fires on first paint in the default grid; (3) `computeCollectionStats` computes min/max in a single pass instead of spreading a per-card array into `Math.min`/`Math.max`, so a large untrusted collection no longer throws `RangeError` in `CollectionToast`, which renders outside the collection error boundary and would blank the app; (4) `loadCollection` caps the untrusted `entityTypes` count (`MAX_ENTITY_TYPES`) and loads types through a fixed-size pool, closing a per-type fetch multiplier that amplified one collection load into ~80k CDN requests plus GitHub-quota exhaustion; (5) `useMyPlausibleMeDiscovery` caps discovered collections (`MAX_DISCOVERED_COLLECTIONS`) and pools metadata fetches, closing a zero-click CDN request flood triggerable via an attacker `/gh/<user>/` URL on the startup picker; (6) `card.imageUrls` (images + videos) is capped per card (`MAX_MEDIA_PER_CARD`), bounding the gallery dot buttons and the load-time image preloader against an entity listing a huge media array; (7) the hard reset runs its three IndexedDB cleanups independently via `Promise.allSettled` and `deleteDB` gained an `onblocked` handler, so a blocked or failed app-DB delete no longer hangs the dialog or silently leaves cached collections and the plugin DB on disk while reporting a complete "delete everything". 19 reproduction tests added. Refuted: `applyMechanicOverrides` second-activation backup clobber (unreachable — the Start Game overlay is gated on `!activeMechanic`, so overrides only apply when no mechanic is active and every deactivation restores first) and the write-only `collectionForcedSettings` mid-session revert (TanStack Query structural sharing keeps `data.settings` identity stable, so the apply effect does not re-fire; folded into the S1 fix). Nitpicks noted (not fixed): TruffleHog pre-commit `--since-commit HEAD` empty-range scan, gitleaks checksum not provenance-anchored, `build:analyse` writing `stats.html` into the deploy root, forced `cardBackStyle`/`titleDisplayMode` enum mismatch vs the store types, and the `getDB()` in-flight dedup that makes the delete-blocked path reachable.
+- 2026-08-07 — Round 7: fixed 11 confirmed defects — (1) `settings.json` `maxVisibleCards` tested its lower bound before flooring and had no finiteness check, so `0.5` stored `0` and `1e400` stored `Infinity` (which serialises to `null`), either of which makes `CardGrid` discard every card on flip and persists globally beyond the collection that supplied it; bounded after the floor and capped at the settings-panel maximum. (2) `settings.json` `searchFields` was type-filtered but uncapped, and search resolves every field on every card per settled query; capped at 32. (3) `DetailLink.source`/`label` were never type-checked (the entity schema is loose and only the URL was validated), so a non-string `source` threw while the sources overlay lowercased it and an object-valued `source`/`label` reached JSX as a child, replacing the whole card grid with the error boundary two clicks after load; both coerced at the normalisation choke point. (4) duplicate entity ids survived the loader and defeated `CardGrid`'s random-selection guard, which proves "every selected id still exists" by comparing counts — a duplicate makes the count match while an id is absent, throwing during render; duplicates also produced duplicate React keys in every view and made one flip toggle several cards. Ids and single-file entity arrays are now deduplicated at load, which also drops the redundant fetch per repeat. (5) quiz wrong-answer selection scanned the correct-answer array linearly per candidate value, and both dimensions are collection-sized, so a 7.4MB payload froze the main thread for 168s and the emitted question carried one rendered option per alternative; matched against a `Set` and capped the offered alternatives, in `relationshipToName` and `fillTheBlank`. (6) Snap Ranking built its card-value map as a plain object literal, so a card id of `"__proto__"` stored nothing (the inherited setter ignores primitives) and the `undefined` guard read back `Object.prototype` — the card was dealt but could never be scored, and rendered as `[object Object]`. (7) the relationship resolver rebuilt the entire relationship record for every entity in both the resolve and the rank pass, with neither the record nor the entity list capped, so cost grew with the square of the payload (10000 relationships × 1000 entities: 11.9s, now 44ms); relationships are grouped by entity type once in `createResolverContext`. (8) image preloading had no aggregate cap and probed the cache one URL at a time, emitting its first progress tick only after the whole probe loop, and the loading overlay clears only at 100% with no skip while the active source persists — a lockout across reloads rather than a slow load; the list is capped and the cached URL set read once. (9) `imageCache.set` recomputed the cache totals with a full store scan, so caching N images cost N(N+1)/2 record reads and eviction added a second scan; admission, eviction and the write now happen in one transaction over both stores (800 stores: 5.0s to under 0.4s). The first pre-merge review caught that an intermediate version still read the totals in a snapshot taken before the write transaction opened, so the preloader's five concurrent writes each judged admission from the same stale state — reproduced as a budget overshoot to 140% of the maximum. (10) the filter dropdown mounts one checkbox per option and re-reconciles the whole list on every toggle, and `genres` is an uncapped per-entity array while platform and year were bounded only by the 10000-entity loader cap, so no field had a usable ceiling; all three are capped where the lists are assembled, since many entities contributing a few values each reach the same total. (11) a stored "never cache" preference only suppressed the consent prompt while preloading still fetched and persisted every image, making "Never cache" weaker than declining once; caching permission is now a rule separate from whether to ask. 41 reproduction tests added (944 → 985). Deferred (confirmed, own dedicated round): entity edits are stored in one flat source-unscoped map, so switching collections bleeds one collection's private notes onto another's cards and silently overwrites them — refuted as a security finding (no exfiltration path) but a real data-integrity defect, and every correct fix changes the persisted shape of a store that has no `version` or `migrate`, plus a product decision about provenance-less existing edits. Also deferred: "Import Collection" writes an unvalidated, unbounded blob to a localStorage key nothing reads, so the feature is inert while reporting success — the correct fix is to remove the button or implement the feature, both product calls. Refuted: forced `cardBackStyle`/`titleDisplayMode` enum mismatch re-raised as substantive (the claimed CSS impact is false — the `--card-title-*` custom properties are defined but never read, and `cardBackStyle` has no renderer at all; it remains the round-6 nitpick). Logged for a future round (confirmed by a pre-merge reviewer, not fixed here): `evictLRU` does not persist corrected totals when its scan fallback runs but nothing needs evicting, so a poisoned metadata record would survive — unreachable today because the method has no callers; and `readTotals` heals downward drift only, so its comment claims more than it delivers. Dependency review: `npm audit --omit=dev` reports 0 advisories in the production tree; all 25 advisories are devDependency-only and no dependency was changed.

src/components/CardGrid/CardGrid.tsx

@@ -16,6 +16,7 @@ import { useSettingsStore, CARD_ASPECT_RATIOS } from "@/stores/settingsStore";
 import { useMechanicContext, useMechanicCardActions } from "@/mechanics";
 import { createFieldSortComparator, resolveFieldPath } from "@/utils/fieldPathResolver";
 import { shuffle } from "@/utils/shuffle";
+import { capFilterOptions } from "@/utils/filterOptions";
 import { LoadingSkeleton } from "@/components/LoadingSkeleton";
 import { springPresets, getItemDelay } from "@/config/animationPresets";
 import type { CardDisplayConfig } from "@/types/display";
@@ -616,10 +617,16 @@ export function CardGrid() {
       }
     }
 
+    // The filter dropdown mounts one checkbox per option for every field and
+    // re-reconciles the whole list on each toggle. `genres` is an uncapped
+    // per-entity array, and the card-derived fields are only bounded by the
+    // loader's entity cap, so all three need a ceiling. Cap the assembled
+    // lists rather than the per-entity arrays: many entities contributing a
+    // few values each reach the same total.
     return {
-      platforms: Array.from(platforms).sort(),
-      years: Array.from(years).sort((a, b) => b - a),
-      genres: Array.from(genres).sort(),
+      platforms: capFilterOptions(Array.from(platforms).sort(), "platform"),
+      years: capFilterOptions(Array.from(years).sort((a, b) => b - a), "year"),
+      genres: capFilterOptions(Array.from(genres).sort(), "genres"),
     };
   }, [sourceCards]);
 

src/components/LoadingScreen/LoadingScreen.tsx

@@ -15,6 +15,7 @@ import { collectionKeys } from "@/hooks/queryKeys";
 import { useCollectionData } from "@/context/CollectionDataContext";
 import { useImagePreloader } from "@/hooks/useImageCache";
 import { useSettingsStore } from "@/stores/settingsStore";
+import { mayCacheCollection } from "@/utils/cacheConsent";
 import { useSourceStore } from "@/stores/sourceStore";
 import { useOnlineStatus } from "@/hooks/useOnlineStatus";
 import { isCollectionCached, listCachedCollections, type CacheInfo } from "@/lib/cardCache";
@@ -152,6 +153,21 @@ export function LoadingScreen({
     return !hasCacheConsent(activeSourceId);
   }, [activeSourceId, activeSource?.isBuiltIn, cacheConsentPreference, hasCacheConsent]);
 
+  // Whether caching is actually permitted, which is a different question from
+  // whether to ask. A stored "never" answers the prompt permanently but must
+  // still forbid caching: preloading writes every image to IndexedDB, so
+  // gating only the prompt made "Never cache" weaker than declining once.
+  const mayCacheImages = useMemo(
+    () =>
+      mayCacheCollection({
+        hasActiveSource: Boolean(activeSourceId),
+        isBuiltIn: Boolean(activeSource?.isBuiltIn),
+        preference: cacheConsentPreference,
+        hasSourceConsent: activeSourceId ? hasCacheConsent(activeSourceId) : false,
+      }),
+    [activeSourceId, activeSource?.isBuiltIn, cacheConsentPreference, hasCacheConsent]
+  );
+
   // Start image preloading; a failed preload must not strand the
   // loading screen, so advance to complete on rejection
   const startImagePreload = useCallback(() => {
@@ -197,13 +213,13 @@ export function LoadingScreen({
       if (needsCacheConsent) {
         setPhase("consent");
         setConsentDialogOpen(true);
-      } else if (shouldPreloadImages && imageUrls.length > 0) {
+      } else if (mayCacheImages && shouldPreloadImages && imageUrls.length > 0) {
         startImagePreload();
       } else {
         setPhase("complete");
       }
     }
-  }, [isLoadingCollection, error, activeSourceId, phase, shouldPreloadImages, imageUrls, startImagePreload, needsCacheConsent]);
+  }, [isLoadingCollection, error, activeSourceId, phase, shouldPreloadImages, imageUrls, startImagePreload, needsCacheConsent, mayCacheImages]);
 
   // Handle image preloading complete
   useEffect(() => {