From aeb4eb5c6340414a4f10e109b080b87a17002773 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:11 +0000 Subject: [PATCH 1/6] fix: bound untrusted field-path length to stop quadratic parse The v1 field-path parser is quadratic in path length for a bracket-free path (parsePath re-scans the remaining string for "[" every iteration). display.card.front.title/subtitle/badge (cardFrontConfigSchema, a bare z.string()) and persisted forced fieldMapping values feed it uncapped untrusted strings, resolved once per card at grid render, so a ~2MB dotted path froze the main thread for tens of seconds with no error boundary reached. Guard the shared getFieldValue/resolveFieldPath chokepoint with a MAX_FIELD_PATH_LENGTH bound so an over-long path resolves to the fallback in bounded time. Real paths are a handful of characters. Assisted-by: Claude:claude-fable-5 --- src/loaders/fieldPath.ts | 28 ++++++++++++++++- tests/loaders/fieldPath.test.ts | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/loaders/fieldPath.ts b/src/loaders/fieldPath.ts index 852be2c..f6fc402 100644 --- a/src/loaders/fieldPath.ts +++ b/src/loaders/fieldPath.ts @@ -23,6 +23,20 @@ interface PathSegment { filterField?: string; } +/** + * Upper bound on a field-path expression. + * + * Paths come from untrusted collection.json display config + * (`display.card.front.title` etc.) and from persisted forced `fieldMapping` + * values in settings.json — both `z.string()` with no length cap. parsePath is + * quadratic in the path length for a bracket-free path (it re-scans the + * remaining string for `[` every iteration), so a multi-megabyte dotted path + * freezes the main thread for tens of seconds, once per card at grid render. + * Real paths are a handful of characters ("images[type=cover][0].url"); an + * expression longer than this is malformed, so it resolves to the fallback. + */ +const MAX_FIELD_PATH_LENGTH = 512; + /** * Parse a field path into segments. * @@ -135,6 +149,13 @@ export function getFieldValue( entity: Entity | ResolvedEntity, path: string ): unknown { + // Guard the quadratic parser against an unbounded untrusted path. This is the + // single chokepoint every resolver (getStringValue/getNumberValue/ + // getImagesValue and resolveFieldPath's fallback loop) funnels through. + if (typeof path !== "string" || path.length > MAX_FIELD_PATH_LENGTH) { + return undefined; + } + const segments = parsePath(path); let current: unknown = entity; @@ -310,7 +331,12 @@ export function resolveFieldPath( ): unknown { // Defence in depth: a non-string expression (e.g. from a malformed forced // fieldMapping) would throw in the split below, crashing every card render. - if (typeof expression !== "string") { + // The length bound also caps the whole fallback chain ("a ?? b ?? …") so a + // pathological expression cannot amplify the per-path work by its split count. + if ( + typeof expression !== "string" || + expression.length > MAX_FIELD_PATH_LENGTH + ) { return undefined; } diff --git a/tests/loaders/fieldPath.test.ts b/tests/loaders/fieldPath.test.ts index 758663e..7168654 100644 --- a/tests/loaders/fieldPath.test.ts +++ b/tests/loaders/fieldPath.test.ts @@ -294,4 +294,58 @@ describe("fieldPath", () => { expect(resolveFieldPathAsNumber(entity, "missing")).toBeNull(); }); }); + + describe("untrusted path length bound (DoS guard)", () => { + // parsePath is quadratic in path length for a bracket-free path (it re-scans + // for '[' every iteration). display.card.front.* and forced fieldMapping + // values are untrusted strings with no upstream length cap, resolved once + // per card at grid render, so a ~2MB dotted path froze the tab for tens of + // seconds. The resolver must reject an over-long path near-instantly and + // fall back, rather than parse it. + const hostileEntity: ResolvedEntity = { + id: "x", + title: "Real Title", + year: 1990, + _resolved: {}, + }; + + it("resolves an oversized dotted path to the fallback in bounded time", () => { + // ~2MB bracket-free path — the pre-fix quadratic parse of this took ~14s. + const hostilePath = "a.".repeat(1_000_000); + + const start = performance.now(); + const result = resolveFieldPathAsString( + hostileEntity, + hostilePath, + "N/A" + ); + const elapsed = performance.now() - start; + + expect(result).toBe("N/A"); + // Enormous margin: the fix returns immediately (<1ms); the pre-fix parse + // was ~14000ms. A 200ms ceiling fails loudly if the guard regresses. + expect(elapsed).toBeLessThan(200); + }); + + it("caps a pathological fallback chain by total expression length", () => { + const hostileExpression = "a??".repeat(1_000_000); + + const start = performance.now(); + const result = resolveFieldPath(hostileEntity, hostileExpression); + const elapsed = performance.now() - start; + + expect(result).toBeUndefined(); + expect(elapsed).toBeLessThan(200); + }); + + it("getFieldValue returns undefined for an over-long path", () => { + expect(getFieldValue(hostileEntity, "b.".repeat(500))).toBeUndefined(); + }); + + it("still resolves ordinary paths of reasonable length", () => { + expect(resolveFieldPathAsString(hostileEntity, "title")).toBe( + "Real Title" + ); + }); + }); }); From f0797d221759a83f0dce1357c97e5edd7369845d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:17 +0000 Subject: [PATCH 2/6] fix: use null-prototype map for Competing card data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Competing store built its cardData map as a plain object literal keyed by untrusted entity ids. An id of "__proto__" (which survives the loader's Set-keyed dedup and reaches card.id) re-pointed the map's prototype to the attacker card instead of creating an own key, so the card was counted in config.cards but omitted from Object.keys — never dealt, and able to drop validCardCount below the four-card minimum. Build the map with Object.create(null), mirroring the snap-ranking store's existing guard. Assisted-by: Claude:claude-fable-5 --- src/mechanics/competing/store.ts | 37 ++++++++++++--- tests/mechanics/competing/store.test.ts | 61 ++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/mechanics/competing/store.ts b/src/mechanics/competing/store.ts index 99f3ce1..c3e397b 100644 --- a/src/mechanics/competing/store.ts +++ b/src/mechanics/competing/store.ts @@ -19,7 +19,11 @@ import type { } from "./types"; import { DEFAULT_SETTINGS } from "./types"; import { getCardValue, compareValues } from "./utils"; -import { getAIStrategy, resetPatternTracker, recordPlayerSelection } from "./ai"; +import { + getAIStrategy, + resetPatternTracker, + recordPlayerSelection, +} from "./ai"; /** * Timeout IDs for cleanup - stored outside store to avoid serialisation issues. @@ -121,7 +125,14 @@ export const useCompetingStore = create((set, get) => ({ resetGame: () => { clearPendingTimeouts(); - const { numericFields, cardData, difficulty, roundLimit, showCpuThinking, autoAdvance } = get(); + const { + numericFields, + cardData, + difficulty, + roundLimit, + showCpuThinking, + autoAdvance, + } = get(); // Reset pattern tracker for new game resetPatternTracker(); @@ -195,8 +206,17 @@ export const useCompetingStore = create((set, get) => ({ return; } - // Build card data map, filtering out cards with missing values - const cardData: Record = {}; + // Build card data map, filtering out cards with missing values. + // + // Card ids come from untrusted collection data, so a null-prototype object + // is used: assigning a card object to "__proto__" on a plain object literal + // re-points the map's prototype instead of creating an own key, silently + // dropping that card from the deck (and skewing validCardCount below). + // Mirrors snap-ranking/store.ts. + const cardData: Record = Object.create(null) as Record< + string, + CardData + >; for (const card of config.cards) { const id = card[config.idField]; if (typeof id !== "string") continue; @@ -299,7 +319,11 @@ export const useCompetingStore = create((set, get) => ({ const gameContext = get().getGameContext(); // CPU selects stat and waits for player confirmation - const selectedStat = ai.selectStat(cpuCardData, state.numericFields, gameContext); + const selectedStat = ai.selectStat( + cpuCardData, + state.numericFields, + gameContext + ); set({ selectedStat, @@ -477,7 +501,8 @@ export const useCompetingStore = create((set, get) => ({ currentTurn = currentTurn === "player" ? "cpu" : "player"; } - const nextPhase: GamePhase = currentTurn === "player" ? "player_select" : "cpu_select"; + const nextPhase: GamePhase = + currentTurn === "player" ? "player_select" : "cpu_select"; set({ playerDeck, diff --git a/tests/mechanics/competing/store.test.ts b/tests/mechanics/competing/store.test.ts index 064bc29..994919d 100644 --- a/tests/mechanics/competing/store.test.ts +++ b/tests/mechanics/competing/store.test.ts @@ -4,7 +4,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { useCompetingStore } from "@/mechanics/competing/store"; -import type { CompetingGameConfig, NumericFieldInfo } from "@/mechanics/competing/types"; +import type { + CompetingGameConfig, + NumericFieldInfo, +} from "@/mechanics/competing/types"; // Mock setTimeout for testing auto-advance vi.useFakeTimers(); @@ -12,7 +15,13 @@ vi.useFakeTimers(); describe("useCompetingStore", () => { const sampleFields: NumericFieldInfo[] = [ { key: "attack", label: "Attack", min: 50, max: 100, higherIsBetter: true }, - { key: "defence", label: "Defence", min: 30, max: 90, higherIsBetter: true }, + { + key: "defence", + label: "Defence", + min: 30, + max: 90, + higherIsBetter: true, + }, ]; const sampleCards = [ @@ -107,7 +116,10 @@ describe("useCompetingStore", () => { }); it("should put odd card in tie pile", () => { - const oddCards = [...sampleCards, { id: "card5", title: "Hero E", attack: 75, defence: 70 }]; + const oddCards = [ + ...sampleCards, + { id: "card5", title: "Hero E", attack: 75, defence: 70 }, + ]; const store = useCompetingStore.getState(); store.activate(); store.initGame({ @@ -117,8 +129,12 @@ describe("useCompetingStore", () => { }); const state = useCompetingStore.getState(); - const totalCards = state.playerDeck.length + state.cpuDeck.length + - (state.playerCard ? 1 : 0) + (state.cpuCard ? 1 : 0) + state.tiePile.length; + const totalCards = + state.playerDeck.length + + state.cpuDeck.length + + (state.playerCard ? 1 : 0) + + (state.cpuCard ? 1 : 0) + + state.tiePile.length; expect(totalCards).toBe(5); expect(state.tiePile.length).toBe(1); @@ -149,6 +165,37 @@ describe("useCompetingStore", () => { const state = useCompetingStore.getState(); expect(state.errorMessage).toContain("numeric fields"); }); + + it("deals a card whose id is __proto__ instead of silently dropping it", () => { + // Card ids come from untrusted collection data. With a plain {} map, + // cardData["__proto__"] = card re-points the map's prototype rather than + // creating an own key, so Object.keys omits the card: it is counted in + // config.cards but never dealt, and can drop validCardCount below the + // minimum. A null-prototype map stores it as a normal own key. + const cards = [ + { id: "__proto__", title: "Hero A", attack: 80, defence: 60 }, + { id: "card2", title: "Hero B", attack: 70, defence: 75 }, + { id: "card3", title: "Hero C", attack: 90, defence: 50 }, + { id: "card4", title: "Hero D", attack: 65, defence: 85 }, + ]; + const store = useCompetingStore.getState(); + store.activate(); + store.initGame({ cards, idField: "id", numericFields: sampleFields }); + + const state = useCompetingStore.getState(); + expect(state.errorMessage).toBeNull(); + // The __proto__ card is a real own entry in the map... + expect(Object.hasOwn(state.cardData, "__proto__")).toBe(true); + expect(state.cardData["__proto__"]?.title).toBe("Hero A"); + // ...and all four cards are dealt, none lost to the prototype. + const totalCards = + state.playerDeck.length + + state.cpuDeck.length + + (state.playerCard ? 1 : 0) + + (state.cpuCard ? 1 : 0) + + state.tiePile.length; + expect(totalCards).toBe(4); + }); }); describe("selectStat", () => { @@ -211,7 +258,9 @@ describe("useCompetingStore", () => { const afterState = useCompetingStore.getState(); expect(afterState.roundResult).not.toBeNull(); - expect(["player", "cpu", "tie"]).toContain(afterState.roundResult?.winner); + expect(["player", "cpu", "tie"]).toContain( + afterState.roundResult?.winner + ); expect(afterState.phase).toBe("collecting"); }); }); From 3abd77fff15a5963e54bfe777c870484383143e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:23 +0000 Subject: [PATCH 3/6] fix: cap verdictFields to stop uncapped display-config DoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The collection-controlled verdictFields ordering list was z.array(z.string()) with no bound, and getDisplayableFields scans every spec against the entity's fields (two lowercase allocations per comparison) on CardExpanded mount — once per card in the non-virtualised list/compact layouts — so a 200k-entry array froze the tab on collection load. Truncate the array and each spec in the schema (matching the uiLabels truncation pattern, so one oversized value cannot deny the whole load), and hoist the per-spec lowercase out of the find callback. Assisted-by: Claude:claude-fable-5 --- src/schemas/v2/collection.schema.ts | 21 +++++++++++++++- src/utils/entityFields.ts | 8 +++--- tests/schemas/collection.v2.test.ts | 38 +++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/schemas/v2/collection.schema.ts b/src/schemas/v2/collection.schema.ts index 5546346..dacffad 100644 --- a/src/schemas/v2/collection.schema.ts +++ b/src/schemas/v2/collection.schema.ts @@ -203,11 +203,30 @@ export const cardBackConfigSchema = z.object({ text: z.string().optional(), }); +/** + * Bounds on the untrusted `verdictFields` ordering list. getDisplayableFields + * scans every entry against the entity's fields (O(specs × fields), two + * lowercase allocations per comparison), and CardExpanded runs that scan on + * mount — once per card in the non-virtualised list/compact layouts. An + * unbounded array therefore freezes the tab on collection load. The matched + * result is already capped at MAX_DISPLAYABLE_FIELDS (100), so more specs than + * that can never contribute an extra field; the per-string cap bounds each + * comparison. Truncate rather than reject, matching uiLabels, so one oversized + * value cannot deny the whole collection load. + */ +const MAX_VERDICT_FIELDS = 100; +const MAX_VERDICT_FIELD_LENGTH = 120; + export const cardDisplayConfigSchema = z.object({ front: cardFrontConfigSchema.optional(), back: cardBackConfigSchema.optional(), /** Fields to display in the Verdict view, in order. If empty/undefined, shows all in alphabetical order. */ - verdictFields: z.array(z.string()).optional(), + verdictFields: z + .array( + z.string().transform((value) => value.slice(0, MAX_VERDICT_FIELD_LENGTH)) + ) + .transform((specs) => specs.slice(0, MAX_VERDICT_FIELDS)) + .optional(), }); export const displayConfigSchema = z.object({ diff --git a/src/utils/entityFields.ts b/src/utils/entityFields.ts index 1f3b9be..df4bb11 100644 --- a/src/utils/entityFields.ts +++ b/src/utils/entityFields.ts @@ -417,11 +417,13 @@ export function getDisplayableFields( const orderedFields: DisplayableField[] = []; for (const fieldSpec of options.verdictFields) { - // Find field by key or label (case-insensitive) + // Find field by key or label (case-insensitive). Lower-case the spec once + // per iteration rather than twice per candidate inside `find`. + const specLower = fieldSpec.toLowerCase(); const field = fields.find( (f) => - f.key.toLowerCase() === fieldSpec.toLowerCase() || - f.label.toLowerCase() === fieldSpec.toLowerCase() + f.key.toLowerCase() === specLower || + f.label.toLowerCase() === specLower ); if (field) { orderedFields.push(field); diff --git a/tests/schemas/collection.v2.test.ts b/tests/schemas/collection.v2.test.ts index 9a70eaa..fbacf3e 100644 --- a/tests/schemas/collection.v2.test.ts +++ b/tests/schemas/collection.v2.test.ts @@ -7,6 +7,7 @@ import { imageSchema, entitySchema, uiLabelsSchema, + cardDisplayConfigSchema, } from "@/schemas/v2/collection.schema"; import { getPrimaryImage, type Image } from "@/types/image"; @@ -80,6 +81,43 @@ describe("uiLabelsSchema", () => { }); }); +describe("cardDisplayConfigSchema verdictFields bound", () => { + it("truncates an unbounded verdictFields array (DoS guard)", () => { + // getDisplayableFields scans every spec against the entity's fields, on + // mount, once per card in the non-virtualised list/compact layouts, so an + // uncapped array froze the tab on collection load. Truncate rather than + // reject so an over-long list cannot deny the whole load. + const result = cardDisplayConfigSchema.safeParse({ + verdictFields: Array.from( + { length: 200_000 }, + (_, i) => `field-${String(i)}` + ), + }); + expect(result.success).toBe(true); + expect(result.data?.verdictFields?.length).toBeLessThanOrEqual(100); + }); + + it("truncates an over-long individual verdictFields spec", () => { + const result = cardDisplayConfigSchema.safeParse({ + verdictFields: ["Z".repeat(50_000)], + }); + expect(result.success).toBe(true); + expect(result.data?.verdictFields?.[0]?.length).toBeLessThanOrEqual(120); + }); + + it("passes an ordinary verdictFields list through unchanged", () => { + const result = cardDisplayConfigSchema.safeParse({ + verdictFields: ["rating", "verdict", "summary"], + }); + expect(result.success).toBe(true); + expect(result.data?.verdictFields).toEqual([ + "rating", + "verdict", + "summary", + ]); + }); +}); + describe("getPrimaryImage", () => { it("prefers a boxart image over untyped images when no isPrimary flag is set", () => { const images: Image[] = [ From 26aed4c9fd1bbcf3180867916407997c48ac93b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:29 +0000 Subject: [PATCH 4/6] fix: mount CardExpanded lazily in list and compact card items CardCompactItem and CardListItem mounted a CardExpanded per card unconditionally, defeating the lazy-mount guard Card.tsx documents: each instance registers a window resize listener via useViewportSize plus several store subscriptions. The list, compact and fit layouts are not virtualised, so a large collection mounted thousands of listeners and subscriptions on view switch. Gate the CardExpanded mount behind a hasOpenedExpanded flag set on first open, matching Card.tsx. Assisted-by: Claude:claude-fable-5 --- .../CardCompactItem/CardCompactItem.tsx | 54 ++++++++++++------- src/components/CardListItem/CardListItem.tsx | 51 ++++++++++-------- .../components/CardExpandedLazyMount.test.tsx | 46 ++++++++++++++++ 3 files changed, 111 insertions(+), 40 deletions(-) create mode 100644 tests/components/CardExpandedLazyMount.test.tsx diff --git a/src/components/CardCompactItem/CardCompactItem.tsx b/src/components/CardCompactItem/CardCompactItem.tsx index 78d022d..ad167af 100644 --- a/src/components/CardCompactItem/CardCompactItem.tsx +++ b/src/components/CardCompactItem/CardCompactItem.tsx @@ -27,30 +27,44 @@ interface CardCompactItemProps { /** * Compact view thumbnail card. */ -export function CardCompactItem({ card, cardNumber, tabIndex = 0, width, height }: CardCompactItemProps) { +export function CardCompactItem({ + card, + cardNumber, + tabIndex = 0, + width, + height, +}: CardCompactItemProps) { const [isModalOpen, setIsModalOpen] = useState(false); const [originRect, setOriginRect] = useState(null); + // Mount CardExpanded lazily: each instance registers a window resize listener + // (via useViewportSize) and several store subscriptions, so collapsed items + // must not mount it — the compact layout is not virtualised, so one instance + // per card would multiply into thousands. Once opened it stays mounted so its + // AnimatePresence exit animation can play on close. Mirrors Card.tsx. + const [hasOpenedExpanded, setHasOpenedExpanded] = useState(false); const handleClick = useCallback((event: React.MouseEvent) => { const target = event.currentTarget as HTMLElement; setOriginRect(target.getBoundingClientRect()); + setHasOpenedExpanded(true); setIsModalOpen(true); }, []); // Custom style for dynamic sizing (fit view) - const customStyle = width && height ? { width: `${String(width)}px`, height: `${String(height)}px` } : undefined; + const customStyle = + width && height + ? { width: `${String(width)}px`, height: `${String(height)}px` } + : undefined; - const handleKeyDown = useCallback( - (event: React.KeyboardEvent) => { - if (event.key === "Enter" || event.key === " ") { - event.preventDefault(); - const target = event.currentTarget as HTMLElement; - setOriginRect(target.getBoundingClientRect()); - setIsModalOpen(true); - } - }, - [] - ); + const handleKeyDown = useCallback((event: React.KeyboardEvent) => { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + const target = event.currentTarget as HTMLElement; + setOriginRect(target.getBoundingClientRect()); + setHasOpenedExpanded(true); + setIsModalOpen(true); + } + }, []); const handleCloseModal = useCallback(() => { setIsModalOpen(false); @@ -85,12 +99,14 @@ export function CardCompactItem({ card, cardNumber, tabIndex = 0, width, height - + {hasOpenedExpanded && ( + + )} ); } diff --git a/src/components/CardListItem/CardListItem.tsx b/src/components/CardListItem/CardListItem.tsx index 5b7a648..cae8af7 100644 --- a/src/components/CardListItem/CardListItem.tsx +++ b/src/components/CardListItem/CardListItem.tsx @@ -23,27 +23,36 @@ interface CardListItemProps { /** * List view card row. */ -export function CardListItem({ card, cardNumber, tabIndex = 0 }: CardListItemProps) { +export function CardListItem({ + card, + cardNumber, + tabIndex = 0, +}: CardListItemProps) { const [isModalOpen, setIsModalOpen] = useState(false); const [originRect, setOriginRect] = useState(null); + // Mount CardExpanded lazily: each instance registers a window resize listener + // (via useViewportSize) and several store subscriptions, so collapsed rows + // must not mount it — the list layout is not virtualised, so one instance per + // card would multiply into thousands. Once opened it stays mounted so its + // AnimatePresence exit animation can play on close. Mirrors Card.tsx. + const [hasOpenedExpanded, setHasOpenedExpanded] = useState(false); const handleClick = useCallback((event: React.MouseEvent) => { const target = event.currentTarget as HTMLElement; setOriginRect(target.getBoundingClientRect()); + setHasOpenedExpanded(true); setIsModalOpen(true); }, []); - const handleKeyDown = useCallback( - (event: React.KeyboardEvent) => { - if (event.key === "Enter" || event.key === " ") { - event.preventDefault(); - const target = event.currentTarget as HTMLElement; - setOriginRect(target.getBoundingClientRect()); - setIsModalOpen(true); - } - }, - [] - ); + const handleKeyDown = useCallback((event: React.KeyboardEvent) => { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + const target = event.currentTarget as HTMLElement; + setOriginRect(target.getBoundingClientRect()); + setHasOpenedExpanded(true); + setIsModalOpen(true); + } + }, []); const handleCloseModal = useCallback(() => { setIsModalOpen(false); @@ -92,18 +101,18 @@ export function CardListItem({ card, cardNumber, tabIndex = 0 }: CardListItemPro - {summary && ( -

{summary}

- )} + {summary &&

{summary}

} - + {hasOpenedExpanded && ( + + )} ); } diff --git a/tests/components/CardExpandedLazyMount.test.tsx b/tests/components/CardExpandedLazyMount.test.tsx new file mode 100644 index 0000000..db4ad21 --- /dev/null +++ b/tests/components/CardExpandedLazyMount.test.tsx @@ -0,0 +1,46 @@ +/** + * Lazy-mount regression tests for CardCompactItem and CardListItem. + * + * Each CardExpanded instance registers a window resize listener (via + * useViewportSize) and several store subscriptions on mount. The list and + * compact layouts are not virtualised, so mounting one CardExpanded per card + * unconditionally multiplied into thousands of listeners/subscriptions on a + * large collection. Both renderers must instead mount CardExpanded lazily, only + * once the card is opened, matching Card.tsx. + */ + +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import type { DisplayCard } from "@/hooks/useCollection"; + +// Replace CardExpanded with a marker so we can observe whether it is mounted. +vi.mock("@/components/CardExpanded", () => ({ + CardExpanded: () =>
, +})); + +import { CardCompactItem } from "@/components/CardCompactItem/CardCompactItem"; +import { CardListItem } from "@/components/CardListItem/CardListItem"; + +const mockCard = { + id: "card-1", + title: "Test Card", + imageUrl: "https://example.com/image.jpg", +} as unknown as DisplayCard; + +describe.each([ + ["CardCompactItem", CardCompactItem], + ["CardListItem", CardListItem], +])("%s lazy CardExpanded mount", (_name, Component) => { + it("does not mount CardExpanded before the card is opened", () => { + render(); + expect( + screen.queryByTestId("card-expanded-mounted") + ).not.toBeInTheDocument(); + }); + + it("mounts CardExpanded after the card is clicked", () => { + render(); + fireEvent.click(screen.getByRole("button")); + expect(screen.getByTestId("card-expanded-mounted")).toBeInTheDocument(); + }); +}); From 8ca22a05ae5633f9197c2ed58497eda29ce8312e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:36 +0000 Subject: [PATCH 5/6] fix: clear service-worker caches on hard reset The hard reset cleared localStorage and three IndexedDB databases but never touched the service worker's Cache Storage buckets (populated by vite-plugin-pwa: jsdelivr-cache, github-raw-cache, image-cache) or unregistered the worker. The viewed collection JSON, settings.json and imagery therefore survived the reset and were served back on the reload, contradicting the dialog's "permanently delete all your ... cached data" promise. Delete every Cache Storage bucket and unregister the service worker as part of clearAllPersistedData, guarded for environments lacking the APIs. Assisted-by: Claude:claude-fable-5 --- src/lib/clearPersistedData.ts | 37 ++++++++++++++++-- tests/lib/clearPersistedData.test.ts | 56 ++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/lib/clearPersistedData.ts b/src/lib/clearPersistedData.ts index 49ec2e6..df44d87 100644 --- a/src/lib/clearPersistedData.ts +++ b/src/lib/clearPersistedData.ts @@ -1,9 +1,11 @@ /** * Clear every piece of persisted itemdeck state (the "hard reset"). * - * State is spread across localStorage and three separate IndexedDB databases - * created by different subsystems, so a naive reset leaves data behind. This - * clears all of it to honour the "delete everything" promise. + * State is spread across localStorage, three separate IndexedDB databases + * created by different subsystems, and the service worker's Cache Storage + * buckets, so a naive reset leaves data behind. This clears all of it — and + * unregisters the service worker so it does not re-populate its caches after + * the reload — to honour the "delete everything" promise. */ import { deleteDB } from "@/db"; @@ -42,6 +44,33 @@ function deleteIndexedDb(name: string): Promise { }); } +/** + * Delete every Cache Storage bucket. The service worker (vite-plugin-pwa) + * caches remote collection JSON, settings.json, entity files and images into + * Cache Storage — a layer entirely separate from the IndexedDB caches above. + * Without this, a hard reset leaves the viewed collection and its imagery on + * disk, contradicting the dialog's "delete all your ... cached data" promise. + */ +async function clearCacheStorage(): Promise { + if (typeof caches === "undefined") return; + const names = await caches.keys(); + await Promise.all(names.map((name) => caches.delete(name))); +} + +/** + * Unregister every service worker so it stops intercepting and re-populating + * Cache Storage after the reset reload. + */ +async function unregisterServiceWorkers(): Promise { + if (typeof navigator === "undefined" || !("serviceWorker" in navigator)) { + return; + } + const registrations = await navigator.serviceWorker.getRegistrations(); + await Promise.all( + registrations.map((registration) => registration.unregister()) + ); +} + /** * Remove all persisted itemdeck data: every `itemdeck-` localStorage key, * the app IndexedDB database, the cached-collection store, and the plugin @@ -63,5 +92,7 @@ export async function clearAllPersistedData(): Promise { deleteDB(), // the app database ("itemdeck") clearAllCollectionCaches(), // cached collections (idb-keyval store) ...SATELLITE_DATABASES.map((name) => deleteIndexedDb(name)), + clearCacheStorage(), // service-worker Cache Storage buckets + unregisterServiceWorkers(), // stop the SW re-populating them after reload ]); } diff --git a/tests/lib/clearPersistedData.test.ts b/tests/lib/clearPersistedData.test.ts index 8de04f1..ab9af8e 100644 --- a/tests/lib/clearPersistedData.test.ts +++ b/tests/lib/clearPersistedData.test.ts @@ -60,11 +60,15 @@ function makeLocalStorage(seed: Record): Storage { } let deletedDatabases: string[]; +let deletedCaches: string[]; +let unregisteredWorkers: number; beforeEach(() => { deleteDBMock.mockClear(); clearAllCollectionCachesMock.mockClear(); deletedDatabases = []; + deletedCaches = []; + unregisteredWorkers = 0; vi.stubGlobal("indexedDB", { deleteDatabase: vi.fn((name: string) => { @@ -78,6 +82,33 @@ beforeEach(() => { return request; }), }); + + // Cache Storage populated by the service worker. + vi.stubGlobal("caches", { + keys: vi.fn(() => + Promise.resolve(["jsdelivr-cache", "github-raw-cache", "image-cache"]) + ), + delete: vi.fn((name: string) => { + deletedCaches.push(name); + return Promise.resolve(true); + }), + }); + + // A registered service worker. + vi.stubGlobal("navigator", { + serviceWorker: { + getRegistrations: vi.fn(() => + Promise.resolve([ + { + unregister: vi.fn(() => { + unregisteredWorkers += 1; + return Promise.resolve(true); + }), + }, + ]) + ), + }, + }); }); afterEach(() => { @@ -116,6 +147,31 @@ describe("clearAllPersistedData", () => { expect(deletedDatabases).toContain("itemdeck-plugins"); }); + it("deletes every Cache Storage bucket and unregisters the service worker", async () => { + vi.stubGlobal("localStorage", makeLocalStorage({})); + + await clearAllPersistedData(); + + // All SW-populated Cache Storage buckets are cleared — without this the + // viewed collection JSON and images survive the "delete everything" reset. + expect(deletedCaches).toEqual([ + "jsdelivr-cache", + "github-raw-cache", + "image-cache", + ]); + // The service worker is unregistered so it cannot re-populate them. + expect(unregisteredWorkers).toBe(1); + }); + + it("resolves when Cache Storage / service worker APIs are unavailable", async () => { + vi.stubGlobal("localStorage", makeLocalStorage({})); + vi.stubGlobal("caches", undefined); + vi.stubGlobal("navigator", {}); + + await expect(clearAllPersistedData()).resolves.toBeUndefined(); + expect(deleteDBMock).toHaveBeenCalledTimes(1); + }); + it("resolves even when deleting a satellite database throws", async () => { vi.stubGlobal("localStorage", makeLocalStorage({})); vi.stubGlobal("indexedDB", { From b47f35665f26b3159db0006ba70a02b11c134e72 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:32:36 +0000 Subject: [PATCH 6/6] docs: log round 10 security-hardening decisions Assisted-by: Claude:claude-fable-5 --- .abcd/work/DECISIONS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 9d274c2..c96bd8c 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -11,3 +11,4 @@ Dated, one-line summaries of each autonomous security-hardening round. - 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. - 2026-08-13 — Round 9: fixed 9 confirmed defects — (1) an untrusted `collection.json` `relationships` record is unbounded and `type.field` keyed, so a record whose keys all share the primary type's prefix collapsed into one bucket and made both the resolve pass and the per-card rank pass `O(relationships × entities)` again (the round-7 grouping bounded only the per-entity rebuild); capped at `MAX_RELATIONSHIPS`, and the single-file entity-array paths (`{type}s.json`/`{type}.json`) now share the `MAX_ENTITY_IDS` ceiling the index path already enforced, removing the entity-count multiplier. (2) `useAvailableFields` walked every key of the sampled cards and turned each into an `