Security hardening — round 8: 5 confirmed defects

.abcd/work/DECISIONS.md

@@ -9,3 +9,4 @@ Dated, one-line summaries of each autonomous security-hardening round.
 - 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.
+- 2026-08-13 — Round 8: fixed 5 confirmed defects — (1) the startup collection picker rendered discovered `collection.json` metadata (`name`/`description`/`itemCount`) straight into JSX as React children while `fetchCollectionMetadata` only checked `typeof data === "object"`, and the picker sits above every error boundary, so an attacker `/gh/<user>/` link (discovery auto-runs with no click) serving a non-primitive `name`/`description`/`itemCount` threw "Objects are not valid as a React child" and blanked the whole app, with the poisoned `name` also persisting into the source store; the discovery boundary now coerces all three to primitives. (2) `getDisplayableFields` and the platform `additionalFields` copy emitted one DOM row per entity key with no ceiling (the entity schema is `.loose()`), so a single entity carrying ~100k scalar keys mounted ~300k DOM nodes in one synchronous commit when its "More" overlay opened — both paths are now capped at 100, matching `MAX_MEDIA_PER_CARD`. (3) `validateForcedSettings` guarded `cardBackStyle` and `titleDisplayMode` against inverted allowlists (`plain|pattern|gradient` and `always|hover|never`) that matched neither the real `CardBackStyle` (`bitmap|svg|colour`) nor `TitleDisplayMode` (`truncate|wrap`) enums, so every honest author's forced value was silently dropped while out-of-enum values were accepted, persisted globally, and then rejected wholesale by the settings-export schema on reimport — bricking the user's own backup; the allowlists now match the real enums (this fixes the round-6/7 mismatch on its genuine grounds — the correctness drop plus the export-brick self-DoS — not the previously-refuted CSS-clamping claim, which remains inert). (4) a "replace"-mode settings import called `resetToDefaults()`, which cleared the one-time `hasAppliedCollectionDefaults` marker to false and re-armed the `CollectionDataContext` effect, so the active untrusted collection's `defaults` (including `fieldMapping`) immediately overwrote seven just-imported fields while the UI reported success; the marker is now preserved across the replace reset. (5) imported edits typed field values as `z.unknown()` and merged them raw over the source card, which is rendered as a React child, so an "edits backup" with an object/array-valued `title` threw on every grid render and persisted across reloads and collections; the import boundary now restricts edit field values to JSON primitives (the only shape the edit form produces). 9 reproduction tests added (985 → 994). Refuted: the `?reset=1` settings wipe (a documented, in-app-surfaced self-service reset that clears only the `itemdeck-settings` key — edits/themes/sources/plugins survive — and whose substring match has no realistic colliding URL) and the unbounded `collection.json` `maxVisibleCards` (`z.number().int().positive()` already blocks the harmful non-finite/`<1`/float inputs, and the sole consumer uses the value only as a downward cap min'd against the real card count, so a large value is inert rather than a DoS). Nitpicks/defence-in-depth noted (not fixed): the `?reset=1` substring test and the missing `.max()` on the v2 `maxVisibleCards` schema are minor hardening items; plus the previously-logged raw `edits[card.id]` inherited-key lookup, dead discovery/theme-loader code, and the dev-only `migrate-collection.ts` `parseInt`→`null` / hardcoded-licence correctness bugs. Dependency review: `npm audit --omit=dev` reports 0 advisories in the production tree; no dependency was changed.

src/hooks/useCollection.ts

@@ -401,7 +401,8 @@ async function loadFreshCollection(
       // v2: Use generic terminology. `shortTitle`/`title` are untrusted (loose
       // schema) and this feeds the device badge, rendered as a JSX child.
       const categoryShort =
-        toDisplayString(platform?.shortTitle) ?? toDisplayString(platform?.title);
+        toDisplayString(platform?.shortTitle) ??
+        toDisplayString(platform?.title);
       const order = rank;
 
       // Build DisplayCard with all entity fields for field path resolution
@@ -471,6 +472,11 @@ async function loadFreshCollection(
                 "_resolved",
                 "logoUrl",
               ]);
+              // `platform` is an untrusted loose-schema entity, so cap the
+              // copied keys: the platform overlay renders one DOM row per entry
+              // and an unbounded key set would freeze the tab. Matches the
+              // per-entity displayable-fields cap.
+              const MAX_PLATFORM_FIELDS = 100;
               const additionalFields: Record<string, unknown> = {};
               for (const [key, value] of Object.entries(platform)) {
                 if (
@@ -479,6 +485,11 @@ async function loadFreshCollection(
                   value !== null
                 ) {
                   additionalFields[key] = value;
+                  if (
+                    Object.keys(additionalFields).length >= MAX_PLATFORM_FIELDS
+                  ) {
+                    break;
+                  }
                 }
               }
 

src/hooks/useMyPlausibleMeDiscovery.ts

@@ -110,12 +110,31 @@ async function fetchCollectionMetadata(
   collectionPath: string
 ): Promise<CollectionMetadata | null> {
   try {
-    const url = buildCdnUrl(username, `data/collections/${collectionPath}/collection.json`);
+    const url = buildCdnUrl(
+      username,
+      `data/collections/${collectionPath}/collection.json`
+    );
     const response = await fetch(url);
     if (!response.ok) return null;
     const data: unknown = await response.json();
     if (!data || typeof data !== "object") return null;
-    return data as CollectionMetadata;
+    // collection.json is untrusted third-party content. Its metadata is
+    // rendered directly as React children in the startup picker (which sits
+    // outside any error boundary), so a non-primitive name/description/
+    // itemCount would throw "Objects are not valid as a React child" and blank
+    // the whole app. Coerce to primitives at this trust boundary; the same
+    // sanitised value is what persists into the source store on select.
+    const record = data as Record<string, unknown>;
+    return {
+      name: typeof record.name === "string" ? record.name : undefined,
+      description:
+        typeof record.description === "string" ? record.description : undefined,
+      itemCount:
+        typeof record.itemCount === "number" &&
+        Number.isFinite(record.itemCount)
+          ? record.itemCount
+          : undefined,
+    };
   } catch {
     return null;
   }
@@ -185,7 +204,9 @@ export function useMyPlausibleMeDiscovery(
         } else if (response.status === 403) {
           setError("GitHub API rate limit exceeded. Try again later.");
         } else {
-          setError(`Failed to scan repository (HTTP ${String(response.status)})`);
+          setError(
+            `Failed to scan repository (HTTP ${String(response.status)})`
+          );
         }
         setCollections([]);
         setIsLoading(false);
@@ -245,11 +266,16 @@ export function useMyPlausibleMeDiscovery(
           const file = collectionJsonFiles[index];
           if (!file) continue;
           // Extract the collection path (everything between data/collections/ and /collection.json)
-          const match = /^data\/collections\/(.+)\/collection\.json$/.exec(file.path);
+          const match = /^data\/collections\/(.+)\/collection\.json$/.exec(
+            file.path
+          );
           if (!match?.[1]) continue;
 
           const collectionPath = match[1];
-          const metadata = await fetchCollectionMetadata(trimmedUsername, collectionPath);
+          const metadata = await fetchCollectionMetadata(
+            trimmedUsername,
+            collectionPath
+          );
 
           if (metadata !== null) {
             // Cache entries are keyed by the sourceStore source.id (the
@@ -276,7 +302,8 @@ export function useMyPlausibleMeDiscovery(
 
             // Use the last segment of the path as display name fallback
             const pathSegments = collectionPath.split("/");
-            const folderName = pathSegments[pathSegments.length - 1] ?? collectionPath;
+            const folderName =
+              pathSegments[pathSegments.length - 1] ?? collectionPath;
 
             validCollections.push({
               folder: collectionPath,

src/loaders/settingsLoader.ts

@@ -149,18 +149,23 @@ function validateForcedSettings(raw: Record<string, unknown>): ForcedSettings {
       raw.cardBackDisplay as ForcedSettings["cardBackDisplay"];
   }
 
-  // cardBackStyle
+  // cardBackStyle — must match the CardBackStyle enum. The previous allowlist
+  // ("plain"/"pattern"/"gradient") was stale: it silently dropped every valid
+  // forced value and admitted only out-of-enum ones, which then persisted into
+  // global settings and made the user's own settings export non-reimportable
+  // (the export schema rejects them).
   if (
     typeof raw.cardBackStyle === "string" &&
-    ["plain", "pattern", "gradient"].includes(raw.cardBackStyle)
+    ["bitmap", "svg", "colour"].includes(raw.cardBackStyle)
   ) {
     forced.cardBackStyle = raw.cardBackStyle as ForcedSettings["cardBackStyle"];
   }
 
-  // titleDisplayMode
+  // titleDisplayMode — must match the TitleDisplayMode enum (same stale-allowlist
+  // bug as cardBackStyle above).
   if (
     typeof raw.titleDisplayMode === "string" &&
-    ["always", "hover", "never"].includes(raw.titleDisplayMode)
+    ["truncate", "wrap"].includes(raw.titleDisplayMode)
   ) {
     forced.titleDisplayMode =
       raw.titleDisplayMode as ForcedSettings["titleDisplayMode"];