Security hardening — round 6: 7 confirmed defects

.abcd/work/DECISIONS.md

@@ -7,3 +7,4 @@ Dated, one-line summaries of each autonomous security-hardening round.
 - 2026-07-30 — Round 3: fixed 4 confirmed defects — (1) imported theme colours now validated as hex, closing an external-beacon vector where a loose `z.string()` colour in a shared settings file resolved to a CSS `url(...)` background that bypassed the `connect-src` allowlist via `img-src`; (2) `detectNumericFields` 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 crashes the Competing mechanic with a `RangeError`; (3) forced `fieldMapping` values from a remote `settings.json` are validated as strings (plus a defensive guard in `resolveFieldPath`), closing a persistent-DoS where a non-string path threw on every card render and survived reloads via localStorage; (4) `editsStore` `getEdit`/`hasEdits` use own-property guards so an untrusted entity id such as `"toString"` no longer resolves to an inherited function and crashes the edit form. 7 reproduction tests added. Refuted: provider `..` path traversal (escapes only a non-security UX template into a capability the app already grants over a public CDN; schema-validated output), `CollectionDataContext` edits lookup (inert), and assorted nitpicks (CSP tightening, `migrate-collection` argv path, `fillTheBlank` `$&` replace).
 - 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.

src/context/CollectionDataContext.tsx

@@ -70,6 +70,10 @@
   );
   const applyCollectionDefaults = useSettingsStore((s) => s.applyCollectionDefaults);
   const applyCollectionSettings = useSettingsStore((s) => s.applyCollectionSettings);
+  const restoreCollectionForcedSettings = useSettingsStore(
+    (s) => s.restoreCollectionForcedSettings
+  );
+  const forcedSourceId = useSettingsStore((s) => s.collectionForcedSourceId);
   const applySmartSelectionDefault = useSettingsStore((s) => s.applySmartSelectionDefault);
   const hasAppliedDefaults = useSettingsStore((s) => s.hasAppliedCollectionDefaults);
   const edits = useEditsStore((s) => s.edits);
@@ -81,6 +85,16 @@
     }
   }, [data?.config, hasAppliedDefaults, applyCollectionDefaults]);
 
+  // Revert a previous collection's forced settings as soon as the active
+  // source changes. This must run before the apply effect below and cannot be
+  // folded into it: a source with no settings.json never reaches
+  // applyCollectionSettings, so nothing else would restore the user's settings.
+  useEffect(() => {
+    if (forcedSourceId && forcedSourceId !== sourceUrl) {
+      restoreCollectionForcedSettings();
+    }
+  }, [sourceUrl, forcedSourceId, restoreCollectionForcedSettings]);
+
   // Apply collection-specific settings from settings.json (every load)
   // Uses sourceUrl as the sourceId for tracking which collection's defaults have been applied
   useEffect(() => {
@@ -148,7 +162,7 @@
  * }
  * ```
  */
 export function useCollectionData(): CollectionData {
   const context = useContext(CollectionDataContext);
   if (!context) {
     throw new Error("useCollectionData must be used within CollectionDataProvider");

src/db/index.ts

@@ -185,6 +185,13 @@ export function deleteDB(): Promise<void> {
     const request = indexedDB.deleteDatabase(DB_NAME);
     request.onsuccess = () => { resolve(); };
     request.onerror = () => { reject(new Error(request.error?.message ?? "Failed to delete database")); };
+    // Without this, a delete blocked by another open connection never settles,
+    // hanging any caller that awaits it (e.g. the hard reset). Resolve so the
+    // reset proceeds; the delete completes once the blocking connection closes.
+    request.onblocked = () => {
+      console.warn("[itemdeck] Database deletion blocked by another connection");
+      resolve();
+    };
   });
 }
 

src/hooks/useCollection.ts

@@ -150,6 +150,32 @@ interface CollectionResult {
   isStale?: boolean;
 }
 
+/**
+ * Upper bound on media items (images + videos) kept per card.
+ *
+ * The v2 `images`/`videos` arrays are untrusted and uncapped in the schema.
+ * `card.imageUrls` feeds the gallery (one dot button each) and the load-time
+ * preloader (one cache probe + fetch each), so an entity listing a huge array
+ * would freeze the tab / flood the CDN. Real cards carry a few media items.
+ */
+const MAX_MEDIA_PER_CARD = 100;
+
+/**
+ * Coerce an untrusted entity field to a display string.
+ *
+ * The v2 entity schema is `.loose()`, so text fields (summary, platform
+ * title/shortTitle/summary/year, …) may hold arbitrary JSON. An object left
+ * as-is is truthy and reaches JSX as a child, where React throws "Objects are
+ * not valid as a React child" during render — a single such field can deny the
+ * whole collection view. Mirror the inline coercion already used for
+ * `title`/`year`: keep strings, stringify numbers, drop everything else.
+ */
+function toDisplayString(value: unknown): string | undefined {
+  if (typeof value === "string") return value;
+  if (typeof value === "number") return String(value);
+  return undefined;
+}
+
 /**
  * Format attribution from structured Image objects.
  * Collects all unique attributions from all images.
@@ -317,8 +343,15 @@ async function loadFreshCollection(
           .filter((u): u is string => u !== undefined) ?? []),
       ];
 
-      // Combine image URLs with video URLs for the gallery
-      const allMediaUrls = [...imageUrls, ...videoUrls];
+      // Combine image URLs with video URLs for the gallery. The images/videos
+      // arrays are untrusted and uncapped in the schema, and this list feeds
+      // both the gallery (one dot button per entry) and the load-time image
+      // preloader (one cache probe + fetch per entry), so a hostile entity
+      // could otherwise mount tens of thousands of nodes or requests. Cap it.
+      const allMediaUrls = [...imageUrls, ...videoUrls].slice(
+        0,
+        MAX_MEDIA_PER_CARD
+      );
 
       // Get resolved platform
       const platform = entity._resolved?.platform as ResolvedEntity | undefined;
@@ -365,10 +398,10 @@ async function loadFreshCollection(
           | undefined
       );
 
-      // v2: Use generic terminology
-      const categoryShort = (platform?.shortTitle ?? platform?.title) as
-        | string
-        | undefined;
+      // 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);
       const order = rank;
 
       // Build DisplayCard with all entity fields for field path resolution
@@ -377,13 +410,13 @@ async function loadFreshCollection(
         id: entity.id,
         title,
         year,
-        summary: entity.summary as string | undefined,
+        summary: toDisplayString(entity.summary),
         detailUrl: entity.detailUrl as string | undefined,
         imageUrl: primaryImageUrl,
         imageUrls:
           allMediaUrls.length > 0 ? allMediaUrls : [placeholder(entity.id)],
         // v2 terminology
-        categoryTitle: platform?.title as string | undefined,
+        categoryTitle: toDisplayString(platform?.title),
         categoryShort,
         order,
         imageAttribution: formatAttribution(images),
@@ -481,11 +514,8 @@ async function loadFreshCollection(
               return {
                 id: platform.id,
                 title: platformTitle,
-                year:
-                  typeof platform.year === "number"
-                    ? String(platform.year)
-                    : (platform.year as string | undefined),
-                summary: platform.summary as string | undefined,
+                year: toDisplayString(platform.year),
+                summary: toDisplayString(platform.summary),
                 detailUrls:
                   platformDetailUrls.length > 0
                     ? platformDetailUrls

src/hooks/useMyPlausibleMeDiscovery.ts

@@ -83,6 +83,22 @@ function buildCdnUrl(username: string, path: string, branch = "main"): string {
   return `https://cdn.jsdelivr.net/gh/${username}/MyPlausibleMe@${branch}/${path}`;
 }
 
+/**
+ * Upper bound on the number of collections discovered from one repository.
+ *
+ * The username is attacker-controlled (it comes from the `/gh/<user>/` URL and
+ * auto-runs on the picker with no click), and the GitHub tree can list tens of
+ * thousands of `collection.json` files. Firing a metadata fetch for each would
+ * amplify one page load into a CDN request flood. Cap the count; a real user
+ * repository holds a handful of collections.
+ */
+const MAX_DISCOVERED_COLLECTIONS = 200;
+
+/**
+ * Number of metadata fetches kept in flight at once.
+ */
+const METADATA_FETCH_CONCURRENCY = 8;
+
 /**
  * Fetch collection metadata from collection.json.
  *
@@ -189,29 +205,48 @@ export function useMyPlausibleMeDiscovery(
 
       // Step 2: Find all collection.json files under data/collections/
       // Pattern: data/collections/{path}/collection.json
-      const collectionJsonFiles = tree.filter(
+      const allCollectionJsonFiles = tree.filter(
         (entry) =>
           entry.type === "blob" &&
           entry.path.startsWith("data/collections/") &&
           entry.path.endsWith("/collection.json")
       );
 
-      if (collectionJsonFiles.length === 0) {
+      if (allCollectionJsonFiles.length === 0) {
         setError("No collections found in repository");
         setCollections([]);
         setIsLoading(false);
         return;
       }
 
+      // The username is untrusted, so a hostile repository can list a huge
+      // number of collection.json files. Cap before fanning out a metadata
+      // fetch per file.
+      const collectionJsonFiles = allCollectionJsonFiles.slice(
+        0,
+        MAX_DISCOVERED_COLLECTIONS
+      );
+      if (allCollectionJsonFiles.length > MAX_DISCOVERED_COLLECTIONS) {
+        console.warn(
+          `Repository lists ${String(allCollectionJsonFiles.length)} collections; showing the first ${String(MAX_DISCOVERED_COLLECTIONS)}.`
+        );
+      }
+
       // Step 3: Extract collection paths and fetch metadata
       // e.g., "data/collections/retro/games/collection.json" -> "retro/games"
       const validCollections: CollectionEntry[] = [];
 
-      await Promise.all(
-        collectionJsonFiles.map(async (file) => {
+      // Fetch metadata through a fixed-size pool rather than all at once.
+      let fileCursor = 0;
+      const metadataWorker = async (): Promise<void> => {
+        while (fileCursor < collectionJsonFiles.length) {
+          const index = fileCursor;
+          fileCursor += 1;
+          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);
-          if (!match?.[1]) return;
+          if (!match?.[1]) continue;
 
           const collectionPath = match[1];
           const metadata = await fetchCollectionMetadata(trimmedUsername, collectionPath);
@@ -251,7 +286,15 @@ export function useMyPlausibleMeDiscovery(
               isCached: cached,
             });
           }
-        })
+        }
+      };
+
+      const metadataWorkerCount = Math.min(
+        METADATA_FETCH_CONCURRENCY,
+        collectionJsonFiles.length
+      );
+      await Promise.all(
+        Array.from({ length: metadataWorkerCount }, () => metadataWorker())
       );
 
       // Sort: cached collections first, then alphabetically by name

src/lib/clearPersistedData.ts

@@ -55,9 +55,13 @@ export async function clearAllPersistedData(): Promise<void> {
     }
   }
 
-  await deleteDB(); // the app database ("itemdeck")
-  await clearAllCollectionCaches(); // cached collections (idb-keyval store)
-  for (const name of SATELLITE_DATABASES) {
-    await deleteIndexedDb(name);
-  }
+  // The three IndexedDB cleanups are independent. Run them so a failure or
+  // block in one never aborts the others — previously a rejected/blocked app-DB
+  // delete (the first awaited step) silently left the cached-collection store
+  // and the plugin cache on disk while the UI reported a complete reset.
+  await Promise.allSettled([
+    deleteDB(), // the app database ("itemdeck")
+    clearAllCollectionCaches(), // cached collections (idb-keyval store)
+    ...SATELLITE_DATABASES.map((name) => deleteIndexedDb(name)),
+  ]);
 }