Security hardening — round 7: 11 confirmed defects

[REPPL]

Round 7 of the autonomous security-hardening loop (state: #9).

Six parallel hunters swept the tracked tree; every candidate was then challenged by an independent adversarial reviewer whose explicit job was to refute it. Only findings that survived refutation are fixed here. Each behaviour change has a reproduction test observed failing before the change and passing after.

Baseline before any change: npm ci, typecheck, lint (0 errors), build, and 944 tests all green. Now 985 tests (+41).

Confirmed and fixed

1. settings.json maxVisibleCards could disable card flipping. The bound was tested before the floor and there was no finiteness check, so 0.5 stored 0 and 1e400 stored Infinity (serialising to null). Either makes CardGrid discard every card on flip. The value persists globally and is a defaults key, so the round-6 forced-settings restore does not cover it — it follows the visitor to every later collection. This was the only maxVisibleCards ingress with no upper bound and no lower bound applied after flooring; the cap of 10 matches the settings-panel stepper's own ceiling. src/loaders/settingsLoader.ts:229

2. settings.json searchFields was uncapped. Type-filtered but unbounded, and search resolves every field on every card per settled query. Measured: 5 000 fields (43 KiB) against a 500-card collection costs 1.6 s per search versus 5.7 ms at the 3-field default. Also persisted globally with no UI to edit it back. src/loaders/settingsLoader.ts:268

3. Non-string DetailLink.source/label crashed the card grid. The entity schema is loose and the normalisation choke point validated only the URL. A numeric source throws where the sources overlay lowercases it; an object-valued source/label reaches JSX as a child. Either replaces the whole grid with the error boundary, two ordinary clicks after load. src/types/links.ts:74

4. Duplicate entity ids caused a render-time throw. CardGrid's random-selection guard proves "every selected id still exists" by comparing counts, which only holds if ids are unique — a duplicate lets the count match while an id is absent. Duplicates also produced duplicate React keys in every view and made one flip toggle several cards. Fixed at the root: ids and single-file entity arrays are deduplicated at load, which also drops the redundant fetch per repeat. src/loaders/collectionLoader.ts:113

5. Quiz generation froze the tab. Wrong-answer selection scanned the correct-answer array linearly for every candidate value; both dimensions are collection-sized. Measured through the real default path: 1.45 MB → 2.0 s, 3.67 MB → 16.6 s, 7.37 MB → 168 s of frozen main thread, against under 200 ms to load and validate. The emitted question also carried one rendered option per alternative (399 options measured in the fill-the-blank case). Both affected generators are fixed and tested. relationshipToName.ts:256, fillTheBlank.ts:144

6. A card id of "__proto__" was unscoreable in Snap Ranking. Assigning a primitive through the inherited __proto__ setter is a silent no-op, so the card was dealt but its value never stored, and the undefined guard read back Object.prototype. Perfect play scored 20 against a displayed maximum of 30, and the card rendered as [object Object]. src/mechanics/snap-ranking/store.ts:145

7. Relationship resolution was quadratic in payload size. The whole relationship record was rebuilt per entity in both the resolve and the rank pass, with neither the record nor the entity list capped. Measured 10 000 relationships × 1 000 entities: 11.9 s before, 44 ms after. Relationships are grouped by entity type once when the context is built. src/loaders/relationshipResolver.ts:60

8. Image preloading could lock the app behind the loading overlay. No aggregate cap, and the cache was probed one URL at a time with the first progress tick emitted only after the whole loop. The overlay clears only at 100% with no skip, and the active source persists, so a reload re-enters the same state. src/services/imageCache.ts:534

9. Image caching was quadratic, and its budget check was not concurrency-safe. Storing an image recomputed the totals with a full store scan, so caching N images cost N(N+1)/2 record reads. Totals are now maintained incrementally — and, after the first review round, admission, eviction and the write happen in a single transaction over both stores: previously the "do I need to evict" check read a snapshot taken before the write transaction opened, so the preloader's five concurrent writes each concluded independently that the image still fitted (reproduced: budget overshot to 140%, and concurrent evictions sharing one stale baseline over-evicted). Reading the totals falls back to a full scan if the stored record is missing or has drifted negative, so a bad record cannot permanently disable eviction. 800 stores: 5.0 s to under 0.4 s. src/services/imageCache.ts:181

10. The filter dropdown had no option ceiling. It mounts one checkbox per option and re-reconciles the list on every toggle. genres is an uncapped per-entity array, and platform/year were bounded only by the 10 000-entity loader cap — far beyond anything usable as a filter. All three fields are capped where the lists are assembled, since many entities contributing a few values each reach the same total. src/components/CardGrid/CardGrid.tsx:614

11. "Never cache" did not stop image caching. The stored preference only suppressed the consent prompt; preloading still fetched every image from the third-party host and wrote it to IndexedDB, making "Never cache" strictly weaker than declining once. Caching permission is now a rule separate from whether to ask. src/components/LoadingScreen/LoadingScreen.tsx:155

Review history

Both pre-merge reviewers returned NO-SHIP on the first pass, and one remediation round addressed every blocking finding: the non-atomic eviction admission check (9), the filter cap covering only one of three fields (10), three behaviour changes shipping without a reproduction test (8, 5-fillTheBlank, 11 — each now has one, observed failing first), and ~580 lines of unrelated reformatting bundled into a security commit (both files restored to their original formatting with only the substantive edits; CardGrid.tsx is now a 13-line diff).

Confirmed but deferred

• Entity edits are stored in one flat, source-unscoped map. Switching collections bleeds one collection's private title/summary/myVerdict onto another's cards, flags them as edited, and silently overwrites the first collection's note if the colliding card is then edited. Collision is routine rather than adversarial — the project's own tutorial mints item-1/books, and context edits key on ids like context:snes. Refuted as a security finding (no exfiltration path: the plugin getCards bridge is unwired and the sandbox blocks network), but it contradicts a written requirement in R-020. Every correct fix changes the persisted shape of a store that has no version or migrate, and the migration needs a product decision about provenance-less existing edits. Deferred to a dedicated, human-reviewed round.
• "Import Collection" is inert. It writes an unvalidated, unbounded blob to a localStorage key that nothing in the tree reads, then promises "the page will reload to apply changes". Correctness/UX, not security — the data is never read, rendered, or executed, and the vector is a deliberate settings-panel file pick. The right fix is to remove the button or implement the feature; both are product calls.

Considered and rejected

• Forced cardBackStyle/titleDisplayMode enum mismatch, re-raised as substantive. The mismatch is real but the claimed impact is not: the --card-title-* custom properties are defined and never read (titles hard-code their layout), and cardBackStyle has no renderer or UI control at all. Round 6's backup/restore also reverts the value on source change. Remains the round-6 nitpick.
• Re-confirmed from earlier rounds: GitHub-API URL interpolation (host-pinned, unauthenticated), provider .. traversal (escapes only a UX template), un-gated health/update/manifest fetches (CSP-neutralised), and the dormant src/plugins/** subsystem.

Known gaps, not addressed here

• "Never cache" is enforced at the loading-screen call site, not inside preloadImages/imageCache.set. The explicit "Preload images" button in storage settings is a deliberate user action, and useCachedImage has no callers today — but the rule would be more robust at the boundary.
• Truncation is inconsistent: the filter, preload, and entity caps warn; the maxVisibleCards and searchFields clamps are silent.
• The 2000-image preload cap has no in-UI signal; progress still reports 100% against the capped list.

Dependency review (report only)

npm audit --omit=dev reports 0 advisories in the production tree. All 25 advisories are devDependency-only, concentrated in dev-server and build tooling. No dependency, package.json, or lockfile change was made. Worth a maintainer's attention outside this loop: the vite and storybook dev-server advisories are drive-by exploitable while npm run dev/storybook is listening, and there is no dependabot.yml/renovate.json to keep the SHA-pinned actions and versions current.