diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 17802a4..9a78369 100644 --- a/.abcd/work/DECISIONS.md +++ b/.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//` 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//` 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//` 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. diff --git a/src/hooks/useCollection.ts b/src/hooks/useCollection.ts index 8cb7ebe..8b313db 100644 --- a/src/hooks/useCollection.ts +++ b/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 = {}; 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; + } } } diff --git a/src/hooks/useMyPlausibleMeDiscovery.ts b/src/hooks/useMyPlausibleMeDiscovery.ts index 7b721aa..b1a044b 100644 --- a/src/hooks/useMyPlausibleMeDiscovery.ts +++ b/src/hooks/useMyPlausibleMeDiscovery.ts @@ -110,12 +110,31 @@ async function fetchCollectionMetadata( collectionPath: string ): Promise { 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; + 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, diff --git a/src/loaders/settingsLoader.ts b/src/loaders/settingsLoader.ts index 49305a6..b152db4 100644 --- a/src/loaders/settingsLoader.ts +++ b/src/loaders/settingsLoader.ts @@ -149,18 +149,23 @@ function validateForcedSettings(raw: Record): 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"]; diff --git a/src/utils/editExport.ts b/src/utils/editExport.ts index f2b3052..b012eb6 100644 --- a/src/utils/editExport.ts +++ b/src/utils/editExport.ts @@ -9,9 +9,24 @@ import type { EntityEdit, ExportedEdits } from "@/stores/editsStore"; /** * Schema for validating imported edits. + * + * Field values are restricted to JSON primitives. Edits are text overlays + * produced by the edit form (string / null values only), but the imported file + * is untrusted: the merged edit is spread over the source card and rendered + * directly as a React child, so a non-primitive value (object/array) would + * throw "Objects are not valid as a React child" on every render and, because + * edits persist, brick the collection view until localStorage is cleared. + * Primitives all render safely; objects and arrays are rejected here. */ +const editFieldValueSchema = z.union([ + z.string(), + z.number(), + z.boolean(), + z.null(), +]); + const entityEditSchema = z.object({ - fields: z.record(z.string(), z.unknown()), + fields: z.record(z.string(), editFieldValueSchema), editedAt: z.number(), }); diff --git a/src/utils/entityFields.ts b/src/utils/entityFields.ts index cf0c4ea..1f3b9be 100644 --- a/src/utils/entityFields.ts +++ b/src/utils/entityFields.ts @@ -5,6 +5,18 @@ * and format them for presentation. */ +/** + * Upper bound on the number of auto-discovered fields rendered per entity. + * + * The entity schema is `.loose()`, so an untrusted collection can attach an + * unbounded number of unknown keys to a single entity. Each becomes a DOM row + * in the expanded card's "More" overlay, and the render is a single synchronous + * commit — a pathological entity would freeze or OOM the tab. Cap the count, + * matching the per-card media cap (`MAX_MEDIA_PER_CARD = 100`). A real entity + * displays a handful of fields. + */ +const MAX_DISPLAYABLE_FIELDS = 100; + /** * Fields to skip when auto-discovering entity fields. * These are internal/display fields that shouldn't be shown to users. @@ -416,11 +428,14 @@ export function getDisplayableFields( } } - return orderedFields; + return orderedFields.slice(0, MAX_DISPLAYABLE_FIELDS); } - // Default: sort alphabetically by label - return fields.sort((a, b) => a.label.localeCompare(b.label)); + // Default: sort alphabetically by label, then cap the count so an untrusted + // entity with an unbounded key set cannot mount an unbounded DOM subtree. + return fields + .sort((a, b) => a.label.localeCompare(b.label)) + .slice(0, MAX_DISPLAYABLE_FIELDS); } /** diff --git a/src/utils/settingsExport.ts b/src/utils/settingsExport.ts index c4bc96a..8c0967c 100644 --- a/src/utils/settingsExport.ts +++ b/src/utils/settingsExport.ts @@ -155,7 +155,11 @@ function migrateSettings( // v19 added searchFields, groupByField if (fromVersion < 19) { - migrated.searchFields = migrated.searchFields ?? ["title", "summary", "verdict"]; + migrated.searchFields = migrated.searchFields ?? [ + "title", + "summary", + "verdict", + ]; migrated.groupByField = migrated.groupByField ?? null; } @@ -192,6 +196,14 @@ function migrateSettings( function applySettings(settings: ExportableSettings, mode: ImportMode): void { const store = useSettingsStore.getState(); + // Preserve the one-time "collection defaults applied" marker across a replace + // reset. resetToDefaults() clears it to false, which makes the + // CollectionDataContext effect re-fire and re-apply the active collection's + // defaults over the values being imported (silently discarding the user's + // backup for those fields). The marker is internal state, not a user-facing + // setting, so importing settings must not disturb it. + const hadAppliedCollectionDefaults = store.hasAppliedCollectionDefaults; + // Reset to defaults first if replace mode if (mode === "replace") { store.resetToDefaults(); @@ -199,37 +211,67 @@ function applySettings(settings: ExportableSettings, mode: ImportMode): void { // Apply each setting if it exists in the import if (settings.layout !== undefined) store.setLayout(settings.layout); - if (settings.cardSizePreset !== undefined) store.setCardSizePreset(settings.cardSizePreset); - if (settings.cardAspectRatio !== undefined) store.setCardAspectRatio(settings.cardAspectRatio); - if (settings.maxVisibleCards !== undefined) store.setMaxVisibleCards(settings.maxVisibleCards); - if (settings.cardBackDisplay !== undefined) store.setCardBackDisplay(settings.cardBackDisplay); - if (settings.cardBackStyle !== undefined) store.setCardBackStyle(settings.cardBackStyle); - if (settings.cardBackBackground !== undefined) store.setCardBackBackground(settings.cardBackBackground); - if (settings.showRankBadge !== undefined) store.setShowRankBadge(settings.showRankBadge); - if (settings.showDeviceBadge !== undefined) store.setShowDeviceBadge(settings.showDeviceBadge); - if (settings.rankPlaceholderText !== undefined) store.setRankPlaceholderText(settings.rankPlaceholderText); - if (settings.defaultCardFace !== undefined) store.setDefaultCardFace(settings.defaultCardFace); - if (settings.shuffleOnLoad !== undefined) store.setShuffleOnLoad(settings.shuffleOnLoad); - if (settings.dragModeEnabled !== undefined) store.setDragModeEnabled(settings.dragModeEnabled); + if (settings.cardSizePreset !== undefined) + store.setCardSizePreset(settings.cardSizePreset); + if (settings.cardAspectRatio !== undefined) + store.setCardAspectRatio(settings.cardAspectRatio); + if (settings.maxVisibleCards !== undefined) + store.setMaxVisibleCards(settings.maxVisibleCards); + if (settings.cardBackDisplay !== undefined) + store.setCardBackDisplay(settings.cardBackDisplay); + if (settings.cardBackStyle !== undefined) + store.setCardBackStyle(settings.cardBackStyle); + if (settings.cardBackBackground !== undefined) + store.setCardBackBackground(settings.cardBackBackground); + if (settings.showRankBadge !== undefined) + store.setShowRankBadge(settings.showRankBadge); + if (settings.showDeviceBadge !== undefined) + store.setShowDeviceBadge(settings.showDeviceBadge); + if (settings.rankPlaceholderText !== undefined) + store.setRankPlaceholderText(settings.rankPlaceholderText); + if (settings.defaultCardFace !== undefined) + store.setDefaultCardFace(settings.defaultCardFace); + if (settings.shuffleOnLoad !== undefined) + store.setShuffleOnLoad(settings.shuffleOnLoad); + if (settings.dragModeEnabled !== undefined) + store.setDragModeEnabled(settings.dragModeEnabled); if (settings.dragFace !== undefined) store.setDragFace(settings.dragFace); - if (settings.randomSelectionEnabled !== undefined) store.setRandomSelectionEnabled(settings.randomSelectionEnabled); - if (settings.randomSelectionCount !== undefined) store.setRandomSelectionCount(settings.randomSelectionCount); - if (settings.visualTheme !== undefined) store.setVisualTheme(settings.visualTheme); - if (settings.reduceMotion !== undefined) store.setReduceMotion(settings.reduceMotion); - if (settings.highContrast !== undefined) store.setHighContrast(settings.highContrast); - if (settings.titleDisplayMode !== undefined) store.setTitleDisplayMode(settings.titleDisplayMode); - if (settings.showHelpButton !== undefined) store.setShowHelpButton(settings.showHelpButton); - if (settings.showSettingsButton !== undefined) store.setShowSettingsButton(settings.showSettingsButton); - if (settings.showDragIcon !== undefined) store.setShowDragIcon(settings.showDragIcon); - if (settings.showStatisticsBar !== undefined) store.setShowStatisticsBar(settings.showStatisticsBar); - if (settings.showSearchBar !== undefined) store.setShowSearchBar(settings.showSearchBar); - if (settings.searchBarMinimised !== undefined) store.setSearchBarMinimised(settings.searchBarMinimised); - if (settings.showViewButton !== undefined) store.setShowViewButton(settings.showViewButton); - if (settings.usePlaceholderImages !== undefined) store.setUsePlaceholderImages(settings.usePlaceholderImages); - if (settings.searchFields !== undefined) store.setSearchFields(settings.searchFields); - if (settings.searchScope !== undefined) store.setSearchScope(settings.searchScope); - if (settings.groupByField !== undefined) store.setGroupByField(settings.groupByField); - if (settings.editModeEnabled !== undefined) store.setEditModeEnabled(settings.editModeEnabled); + if (settings.randomSelectionEnabled !== undefined) + store.setRandomSelectionEnabled(settings.randomSelectionEnabled); + if (settings.randomSelectionCount !== undefined) + store.setRandomSelectionCount(settings.randomSelectionCount); + if (settings.visualTheme !== undefined) + store.setVisualTheme(settings.visualTheme); + if (settings.reduceMotion !== undefined) + store.setReduceMotion(settings.reduceMotion); + if (settings.highContrast !== undefined) + store.setHighContrast(settings.highContrast); + if (settings.titleDisplayMode !== undefined) + store.setTitleDisplayMode(settings.titleDisplayMode); + if (settings.showHelpButton !== undefined) + store.setShowHelpButton(settings.showHelpButton); + if (settings.showSettingsButton !== undefined) + store.setShowSettingsButton(settings.showSettingsButton); + if (settings.showDragIcon !== undefined) + store.setShowDragIcon(settings.showDragIcon); + if (settings.showStatisticsBar !== undefined) + store.setShowStatisticsBar(settings.showStatisticsBar); + if (settings.showSearchBar !== undefined) + store.setShowSearchBar(settings.showSearchBar); + if (settings.searchBarMinimised !== undefined) + store.setSearchBarMinimised(settings.searchBarMinimised); + if (settings.showViewButton !== undefined) + store.setShowViewButton(settings.showViewButton); + if (settings.usePlaceholderImages !== undefined) + store.setUsePlaceholderImages(settings.usePlaceholderImages); + if (settings.searchFields !== undefined) + store.setSearchFields(settings.searchFields); + if (settings.searchScope !== undefined) + store.setSearchScope(settings.searchScope); + if (settings.groupByField !== undefined) + store.setGroupByField(settings.groupByField); + if (settings.editModeEnabled !== undefined) + store.setEditModeEnabled(settings.editModeEnabled); // Handle nested objects if (settings.fieldMapping !== undefined) { @@ -238,13 +280,21 @@ function applySettings(settings: ExportableSettings, mode: ImportMode): void { // Handle theme customisations per theme if (settings.themeCustomisations !== undefined) { - for (const theme of Object.keys(settings.themeCustomisations) as (keyof typeof settings.themeCustomisations)[]) { + for (const theme of Object.keys( + settings.themeCustomisations + ) as (keyof typeof settings.themeCustomisations)[]) { const customisation = settings.themeCustomisations[theme]; if (customisation !== undefined) { store.setThemeCustomisation(theme, customisation); } } } + + // Restore the collection-defaults marker cleared by resetToDefaults, so the + // just-imported values are not overwritten by the active collection. + if (mode === "replace") { + store.setHasAppliedCollectionDefaults(hadAppliedCollectionDefaults); + } } /** @@ -259,10 +309,18 @@ function countSettings(settings: ExportableSettings): number { for (const key of Object.keys(settings) as (keyof ExportableSettings)[]) { const value = settings[key]; if (value !== undefined && value !== null) { - if (key === "themeCustomisations" && typeof value === "object" && !Array.isArray(value)) { + if ( + key === "themeCustomisations" && + typeof value === "object" && + !Array.isArray(value) + ) { // Count each theme customisation separately count += Object.keys(value as object).length; - } else if (key === "fieldMapping" && typeof value === "object" && !Array.isArray(value)) { + } else if ( + key === "fieldMapping" && + typeof value === "object" && + !Array.isArray(value) + ) { // Count each field mapping separately count += Object.keys(value as object).length; } else { @@ -297,7 +355,9 @@ export async function importSettingsFromFile( const result = settingsExportSchema.safeParse(parsed); if (!result.success) { - throw new Error(`Invalid settings file:\n${formatSettingsValidationError(result.error)}`); + throw new Error( + `Invalid settings file:\n${formatSettingsValidationError(result.error)}` + ); } const { version, settings } = result.data; diff --git a/tests/hooks/useMyPlausibleMeDiscovery.test.ts b/tests/hooks/useMyPlausibleMeDiscovery.test.ts index b7cda0e..f55e14a 100644 --- a/tests/hooks/useMyPlausibleMeDiscovery.test.ts +++ b/tests/hooks/useMyPlausibleMeDiscovery.test.ts @@ -196,3 +196,80 @@ describe("useMyPlausibleMeDiscovery fan-out is bounded", () => { expect(console.warn).toHaveBeenCalledWith(expect.stringContaining("5000")); }); }); + +describe("useMyPlausibleMeDiscovery sanitises untrusted metadata", () => { + beforeEach(() => { + vi.clearAllMocks(); + useSourceStore.setState({ + sources: [], + activeSourceId: null, + defaultSourceId: null, + }); + vi.mocked(isCollectionCached).mockResolvedValue(false); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("coerces non-primitive name/description/itemCount to safe values", async () => { + // collection.json is untrusted; its metadata renders directly as a React + // child in the startup picker (which has no error boundary above it), so a + // non-primitive value would throw "Objects are not valid as a React child" + // and blank the whole app. The discovery boundary must coerce to primitives. + const tree = { + sha: "abc", + truncated: false, + tree: [ + { + path: "data/collections/books/collection.json", + type: "blob" as const, + sha: "def", + }, + ], + }; + + vi.stubGlobal( + "fetch", + vi.fn((input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("api.github.com")) { + return Promise.resolve( + new Response(JSON.stringify(tree), { status: 200 }) + ); + } + if (url.endsWith("/collection.json")) { + return Promise.resolve( + new Response( + JSON.stringify({ + name: { evil: "object" }, + description: ["array", "value"], + itemCount: { not: "a number" }, + }), + { status: 200 } + ) + ); + } + return Promise.resolve(new Response("not found", { status: 404 })); + }) + ); + + const { result } = renderHook(() => useMyPlausibleMeDiscovery("EVIL")); + + await waitFor( + () => { + expect(result.current.collections).toHaveLength(1); + }, + { timeout: 5000 } + ); + + const entry = result.current.collections[0]; + // Object name is dropped, falling back to the folder name (a string). + expect(typeof entry?.name).toBe("string"); + expect(entry?.name).toBe("books"); + // Non-primitive description and itemCount are dropped entirely. + expect(entry?.description).toBeUndefined(); + expect(entry?.itemCount).toBeUndefined(); + }); +}); diff --git a/tests/loaders/settingsLoader.test.ts b/tests/loaders/settingsLoader.test.ts index 9e83a1c..1253b93 100644 --- a/tests/loaders/settingsLoader.test.ts +++ b/tests/loaders/settingsLoader.test.ts @@ -97,6 +97,51 @@ describe("loadCollectionSettings allowlist", () => { topBadgeField: "myRank", }); }); + + it("accepts valid forced cardBackStyle and titleDisplayMode enum values", async () => { + // These allowlists were previously inverted: they admitted only out-of-enum + // values and silently dropped every legitimate one, so an honest author's + // forced value never took effect and an out-of-enum value persisted into + // global settings (bricking the user's own settings export on reimport). + fetchMock.mockResolvedValue({ + ok: true, + headers: { get: () => "application/json" }, + json: async () => ({ + version: 1, + forced: { + cardBackStyle: "colour", + titleDisplayMode: "truncate", + }, + }), + }); + + const result = await loadCollectionSettings("/data/collections/demo"); + + expect(result?.forced?.cardBackStyle).toBe("colour"); + expect(result?.forced?.titleDisplayMode).toBe("truncate"); + }); + + it("drops out-of-enum forced cardBackStyle and titleDisplayMode values", async () => { + fetchMock.mockResolvedValue({ + ok: true, + headers: { get: () => "application/json" }, + json: async () => ({ + version: 1, + forced: { + // The stale allowlist admitted exactly these; they are not valid + // members of the CardBackStyle / TitleDisplayMode enums and the + // settings-export schema rejects them, so they must be dropped here. + cardBackStyle: "plain", + titleDisplayMode: "always", + }, + }), + }); + + const result = await loadCollectionSettings("/data/collections/demo"); + + expect(result?.forced?.cardBackStyle).toBeUndefined(); + expect(result?.forced?.titleDisplayMode).toBeUndefined(); + }); }); describe("loadCollectionSettings default bounds", () => { diff --git a/tests/utils/editExport.test.ts b/tests/utils/editExport.test.ts index 055b018..1d41d4a 100644 --- a/tests/utils/editExport.test.ts +++ b/tests/utils/editExport.test.ts @@ -3,7 +3,11 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { exportEditsToFile, importEditsFromFile, exportedEditsSchema } from "@/utils/editExport"; +import { + exportEditsToFile, + importEditsFromFile, + exportedEditsSchema, +} from "@/utils/editExport"; import type { EntityEdit, ExportedEdits } from "@/stores/editsStore"; describe("editExport", () => { @@ -24,19 +28,24 @@ describe("editExport", () => { // Mock createElement to capture anchor properties const originalCreateElement = document.createElement.bind(document); - vi.spyOn(document, "createElement").mockImplementation((tagName: string) => { - const element = originalCreateElement(tagName); - if (tagName === "a") { - element.click = mockClick; - // Capture the anchor when click is called - const originalClick = element.click; - element.click = function() { - capturedAnchor = { href: element.href, download: element.download }; - return originalClick.call(this); - }; + vi.spyOn(document, "createElement").mockImplementation( + (tagName: string) => { + const element = originalCreateElement(tagName); + if (tagName === "a") { + element.click = mockClick; + // Capture the anchor when click is called + const originalClick = element.click; + element.click = function () { + capturedAnchor = { + href: element.href, + download: element.download, + }; + return originalClick.call(this); + }; + } + return element; } - return element; - }); + ); }); afterEach(() => { @@ -80,7 +89,9 @@ describe("editExport", () => { exportEditsToFile(edits, "game-collection"); expect(capturedAnchor).not.toBeNull(); - expect(capturedAnchor!.download).toMatch(/^game-collection-edits-\d{4}-\d{2}-\d{2}\.json$/); + expect(capturedAnchor!.download).toMatch( + /^game-collection-edits-\d{4}-\d{2}-\d{2}\.json$/ + ); }); it("handles empty edits", () => { @@ -94,7 +105,9 @@ describe("editExport", () => { describe("importEditsFromFile", () => { // Create a proper mock File with text() method const createMockFile = (content: string): File => { - const file = new File([content], "test.json", { type: "application/json" }); + const file = new File([content], "test.json", { + type: "application/json", + }); // JSDOM File doesn't implement text(), so we mock it file.text = vi.fn().mockResolvedValue(content); return file; @@ -124,7 +137,9 @@ describe("editExport", () => { it("throws error for invalid JSON", async () => { const file = createMockFile("not valid json {"); - await expect(importEditsFromFile(file)).rejects.toThrow("Invalid JSON file"); + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid JSON file" + ); }); it("throws error for invalid format", async () => { @@ -135,7 +150,9 @@ describe("editExport", () => { const file = createMockFile(JSON.stringify(invalidData)); - await expect(importEditsFromFile(file)).rejects.toThrow("Invalid edits file format"); + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid edits file format" + ); }); it("throws error for unsupported version", async () => { @@ -149,7 +166,9 @@ describe("editExport", () => { const file = createMockFile(JSON.stringify(futureVersionData)); - await expect(importEditsFromFile(file)).rejects.toThrow("Invalid edits file format"); + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid edits file format" + ); }); it("validates entity edit structure", async () => { @@ -165,11 +184,13 @@ describe("editExport", () => { const file = createMockFile(JSON.stringify(invalidEntityData)); - await expect(importEditsFromFile(file)).rejects.toThrow("Invalid edits file format"); + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid edits file format" + ); }); - it("allows complex field values", async () => { - const complexData: ExportedEdits = { + it("allows primitive field values", async () => { + const primitiveData: ExportedEdits = { version: 1, exportedAt: "2024-01-15T10:30:00.000Z", collectionId: "test", @@ -179,20 +200,67 @@ describe("editExport", () => { fields: { title: "Text value", year: 2024, - tags: ["tag1", "tag2"], - metadata: { nested: true }, + published: true, + summary: null, }, editedAt: 1000, }, }, }; - const file = createMockFile(JSON.stringify(complexData)); + const file = createMockFile(JSON.stringify(primitiveData)); const result = await importEditsFromFile(file); expect(result.edits["entity-1"].fields.title).toBe("Text value"); expect(result.edits["entity-1"].fields.year).toBe(2024); - expect(result.edits["entity-1"].fields.tags).toEqual(["tag1", "tag2"]); + expect(result.edits["entity-1"].fields.published).toBe(true); + expect(result.edits["entity-1"].fields.summary).toBeNull(); + }); + + it("rejects an object-valued edit field (React-child render-crash guard)", async () => { + // Edits are text overlays spread over the source card and rendered as a + // React child. An object/array value would throw "Objects are not valid + // as a React child" on every render and, because edits persist, brick the + // collection view. The import boundary must reject non-primitive values. + const objectData = { + version: 1, + exportedAt: "2024-01-15T10:30:00.000Z", + collectionId: "test", + editCount: 1, + edits: { + "entity-1": { + fields: { title: { malicious: "object" } }, + editedAt: 1000, + }, + }, + }; + + const file = createMockFile(JSON.stringify(objectData)); + + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid edits file format" + ); + }); + + it("rejects an array-valued edit field", async () => { + const arrayData = { + version: 1, + exportedAt: "2024-01-15T10:30:00.000Z", + collectionId: "test", + editCount: 1, + edits: { + "entity-1": { + fields: { summary: ["a", "b"] }, + editedAt: 1000, + }, + }, + }; + + const file = createMockFile(JSON.stringify(arrayData)); + + await expect(importEditsFromFile(file)).rejects.toThrow( + "Invalid edits file format" + ); }); }); diff --git a/tests/utils/entityFields.test.ts b/tests/utils/entityFields.test.ts index 6e42661..1d3ee07 100644 --- a/tests/utils/entityFields.test.ts +++ b/tests/utils/entityFields.test.ts @@ -8,7 +8,7 @@ */ import { describe, it, expect } from "vitest"; -import { formatFieldValue } from "@/utils/entityFields"; +import { formatFieldValue, getDisplayableFields } from "@/utils/entityFields"; describe("formatFieldValue star ratings", () => { it("formats an in-range 5-star rating", () => { @@ -63,3 +63,35 @@ describe("formatFieldValue star ratings", () => { expect((result as unknown as string).length).toBeLessThan(1000); }); }); + +describe("getDisplayableFields fan-out cap", () => { + it("caps the number of auto-discovered fields for an untrusted entity", () => { + // The entity schema is loose, so a hostile collection can attach an + // unbounded set of unknown keys. Each becomes a DOM row in the expanded + // card's "More" overlay in a single synchronous commit; without a ceiling + // a pathological entity would freeze or OOM the tab. + const entity: Record = { id: "x" }; + for (let i = 0; i < 5000; i++) { + entity[`field${String(i).padStart(5, "0")}`] = `value-${i}`; + } + + const fields = getDisplayableFields(entity); + + expect(fields.length).toBeLessThanOrEqual(100); + expect(fields.length).toBeGreaterThan(0); + }); + + it("does not cap a normal entity below its real field count", () => { + const entity: Record = { + id: "x", + developer: "Studio", + publisher: "Publisher", + players: "1-4", + }; + + const fields = getDisplayableFields(entity); + + // All three displayable fields survive (id is skipped as internal). + expect(fields.length).toBe(3); + }); +}); diff --git a/tests/utils/settingsExport.test.ts b/tests/utils/settingsExport.test.ts index 93ac843..3167536 100644 --- a/tests/utils/settingsExport.test.ts +++ b/tests/utils/settingsExport.test.ts @@ -28,18 +28,23 @@ describe("settingsExport", () => { global.URL.revokeObjectURL = mockRevokeObjectURL; const originalCreateElement = document.createElement.bind(document); - vi.spyOn(document, "createElement").mockImplementation((tagName: string) => { - const element = originalCreateElement(tagName); - if (tagName === "a") { - element.click = mockClick; - const originalClick = element.click; - element.click = function () { - capturedAnchor = { href: element.href, download: element.download }; - return originalClick.call(this); - }; + vi.spyOn(document, "createElement").mockImplementation( + (tagName: string) => { + const element = originalCreateElement(tagName); + if (tagName === "a") { + element.click = mockClick; + const originalClick = element.click; + element.click = function () { + capturedAnchor = { + href: element.href, + download: element.download, + }; + return originalClick.call(this); + }; + } + return element; } - return element; - }); + ); // Reset store to defaults useSettingsStore.getState().resetToDefaults(); @@ -62,7 +67,9 @@ describe("settingsExport", () => { exportSettingsToFile(); expect(capturedAnchor).not.toBeNull(); - expect(capturedAnchor!.download).toMatch(/^itemdeck-settings-\d{4}-\d{2}-\d{2}\.json$/); + expect(capturedAnchor!.download).toMatch( + /^itemdeck-settings-\d{4}-\d{2}-\d{2}\.json$/ + ); }); it("includes version and exportedAt in blob", () => { @@ -81,7 +88,9 @@ describe("settingsExport", () => { describe("importSettingsFromFile", () => { const createMockFile = (content: string): File => { - const file = new File([content], "test.json", { type: "application/json" }); + const file = new File([content], "test.json", { + type: "application/json", + }); file.text = vi.fn().mockResolvedValue(content); return file; }; @@ -161,7 +170,9 @@ describe("settingsExport", () => { it("throws error for invalid JSON", async () => { const file = createMockFile("not valid json {"); - await expect(importSettingsFromFile(file, "merge")).rejects.toThrow("Invalid JSON file"); + await expect(importSettingsFromFile(file, "merge")).rejects.toThrow( + "Invalid JSON file" + ); }); it("throws error for invalid structure", async () => { @@ -173,7 +184,9 @@ describe("settingsExport", () => { const file = createMockFile(JSON.stringify(invalidData)); - await expect(importSettingsFromFile(file, "merge")).rejects.toThrow("Invalid settings file"); + await expect(importSettingsFromFile(file, "merge")).rejects.toThrow( + "Invalid settings file" + ); }); it("throws error for missing settings", async () => { @@ -185,7 +198,9 @@ describe("settingsExport", () => { const file = createMockFile(JSON.stringify(invalidData)); - await expect(importSettingsFromFile(file, "merge")).rejects.toThrow("Invalid settings file"); + await expect(importSettingsFromFile(file, "merge")).rejects.toThrow( + "Invalid settings file" + ); }); it("handles older version imports with migration", async () => { @@ -359,4 +374,38 @@ describe("settingsExport", () => { expect("isDirty" in extracted).toBe(false); }); }); + + describe("applySettings preserves the collection-defaults marker", () => { + const { applySettings } = _testExports; + + beforeEach(() => { + useSettingsStore.getState().resetToDefaults(); + }); + + it("keeps hasAppliedCollectionDefaults true across a replace import", () => { + // Simulate a collection having been loaded (its defaults were applied). + useSettingsStore.getState().setHasAppliedCollectionDefaults(true); + + applySettings({ layout: "list" }, "replace"); + + // The marker must survive the replace reset. Otherwise the + // CollectionDataContext effect re-fires and the active collection's + // defaults silently overwrite the just-imported settings. + const state = useSettingsStore.getState(); + expect(state.hasAppliedCollectionDefaults).toBe(true); + expect(state.layout).toBe("list"); + }); + + it("keeps hasAppliedCollectionDefaults false when none were applied", () => { + useSettingsStore.getState().setHasAppliedCollectionDefaults(false); + + applySettings({ layout: "list" }, "replace"); + + // No collection loaded yet: the first collection should still get to + // apply its defaults, so the marker stays false (unchanged behaviour). + expect(useSettingsStore.getState().hasAppliedCollectionDefaults).toBe( + false + ); + }); + }); });