Security hardening — round 11: 9 confirmed defects

src/components/CollectionPicker/CollectionPicker.tsx

@@ -67,15 +67,18 @@ export function CollectionPicker({ onSelect, initialUsername, notice }: Collecti
   }, [inputValue, username]);
 
   const handleCollectionSelect = useCallback((collection: CollectionEntry) => {
-    // Add the source and set it as active
+    // Pair the folder with the username the entry was scanned from, not the
+    // current input state: a scan is asynchronous, so the entry on screen may
+    // predate a username change. Using the current username would persist and
+    // activate a source that can never resolve.
     const sourceId = addMyPlausibleMeSource(
-      username,
+      collection.username,
       collection.folder,
       collection.name
     );
     setActiveSource(sourceId);
     onSelect(sourceId);
-  }, [username, addMyPlausibleMeSource, setActiveSource, onSelect]);
+  }, [addMyPlausibleMeSource, setActiveSource, onSelect]);
 
   // Handle example collection selection (F-112)
   const handleExampleSelect = useCallback((example: ExampleCollection) => {

src/components/EditForm/EditForm.tsx

@@ -55,6 +55,21 @@ function getContextEditId(contextId: string): string {
   return `context:${contextId}`;
 }
 
+/**
+ * Coerce an untrusted entity field to an editable string.
+ *
+ * The v2 entity schema is `.loose()`, so text fields may hold arbitrary JSON.
+ * An object left as-is renders as "[object Object]" in the textarea and then
+ * fails `cardFieldsSchema` on Save, leaving the modal open with no explanation.
+ * Mirror the collection display coercion: keep strings, stringify numbers,
+ * drop everything else.
+ */
+function toEditableString(value: unknown): string | undefined {
+  if (typeof value === "string") return value;
+  if (typeof value === "number") return String(value);
+  return undefined;
+}
+
 /**
  * Edit form modal for modifying entity fields.
  */
@@ -82,7 +97,7 @@ export function EditForm({ card, onClose }: EditFormProps) {
     const editSummary = existingCardEdit?.fields.summary as string | undefined;
     const editVerdict = existingCardEdit?.fields.myVerdict as string | undefined;
     const cardSummary = card.summary;
-    const cardVerdict = card.myVerdict as string | undefined;
+    const cardVerdict = toEditableString(card.myVerdict);
 
     return {
       title: editTitle ?? card.title,
@@ -331,7 +346,14 @@ export function EditForm({ card, onClose }: EditFormProps) {
                     }}
                     placeholder="Brief description..."
                     rows={3}
+                    aria-invalid={!!cardErrors.summary}
+                    aria-describedby={cardErrors.summary ? "edit-card-summary-error" : undefined}
                   />
+                  {cardErrors.summary && (
+                    <span id="edit-card-summary-error" className={styles.error}>
+                      {cardErrors.summary}
+                    </span>
+                  )}
                 </div>
 
                 {/* My Verdict */}
@@ -348,7 +370,14 @@ export function EditForm({ card, onClose }: EditFormProps) {
                     }}
                     placeholder="Your personal opinion..."
                     rows={3}
+                    aria-invalid={!!cardErrors.myVerdict}
+                    aria-describedby={cardErrors.myVerdict ? "edit-card-verdict-error" : undefined}
                   />
+                  {cardErrors.myVerdict && (
+                    <span id="edit-card-verdict-error" className={styles.error}>
+                      {cardErrors.myVerdict}
+                    </span>
+                  )}
                 </div>
               </>
             ) : (

src/hooks/useMyPlausibleMeDiscovery.ts

@@ -8,14 +8,23 @@
  * @see F-087: Collection Discovery & Startup Picker
  */
 
-import { useState, useEffect, useCallback } from "react";
+import { useState, useEffect, useCallback, useRef } from "react";
 import { isCollectionCached } from "@/lib/cardCache";
 import { useSourceStore } from "@/stores/sourceStore";
 
 /**
  * Collection entry discovered from repository.
  */
 export interface CollectionEntry {
+  /**
+   * GitHub username this collection was scanned from.
+   *
+   * Carried on the entry rather than read from the caller's current username
+   * state: a scan is asynchronous, so the entry on screen may belong to an
+   * earlier username. Pairing that folder with the newer username would build
+   * (and persist) a source that can never resolve.
+   */
+  username: string;
   /** Collection folder name */
   folder: string;
   /** Display name (from collection.json or folder name) */
@@ -176,10 +185,25 @@ export function useMyPlausibleMeDiscovery(
   const [isLoading, setIsLoading] = useState(false);
   const [error, setError] = useState<string | null>(null);
 
+  /**
+   * Generation counter identifying the newest discovery run.
+   *
+   * A scan is one tree fetch plus up to MAX_DISCOVERED_COLLECTIONS metadata
+   * fetches, so it can take seconds. Without a generation guard a superseded
+   * run finishing late would overwrite the newer run's collections (or wipe
+   * them with a stale error), leaving entries on screen that belong to a
+   * different username.
+   */
+  const runIdRef = useRef(0);
+
   const discover = useCallback(async () => {
+    const runId = ++runIdRef.current;
+    const isStale = () => runIdRef.current !== runId;
+
     if (!username.trim()) {
       setCollections([]);
       setError(null);
+      setIsLoading(false);
       return;
     }
 
@@ -198,6 +222,8 @@ export function useMyPlausibleMeDiscovery(
         },
       });
 
+      if (isStale()) return;
+
       if (!response.ok) {
         if (response.status === 404) {
           setError("Repository not found. Check the username.");
@@ -215,6 +241,8 @@ export function useMyPlausibleMeDiscovery(
 
       const treeData: unknown = await response.json();
 
+      if (isStale()) return;
+
       if (!treeData || typeof treeData !== "object" || !("tree" in treeData)) {
         setError("Invalid repository structure");
         setCollections([]);
@@ -261,6 +289,8 @@ export function useMyPlausibleMeDiscovery(
       let fileCursor = 0;
       const metadataWorker = async (): Promise<void> => {
         while (fileCursor < collectionJsonFiles.length) {
+          // A superseded run keeps no results, so stop spending fetches on it.
+          if (isStale()) return;
           const index = fileCursor;
           fileCursor += 1;
           const file = collectionJsonFiles[index];
@@ -306,6 +336,7 @@ export function useMyPlausibleMeDiscovery(
               pathSegments[pathSegments.length - 1] ?? collectionPath;
 
             validCollections.push({
+              username: trimmedUsername,
               folder: collectionPath,
               name: metadata.name ?? folderName,
               description: metadata.description,
@@ -324,6 +355,8 @@ export function useMyPlausibleMeDiscovery(
         Array.from({ length: metadataWorkerCount }, () => metadataWorker())
       );
 
+      if (isStale()) return;
+
       // Sort: cached collections first, then alphabetically by name
       validCollections.sort((a, b) => {
         // Cached collections come first
@@ -342,6 +375,7 @@ export function useMyPlausibleMeDiscovery(
 
       setIsLoading(false);
     } catch (err) {
+      if (isStale()) return;
       setError(err instanceof Error ? err.message : "Discovery failed");
       setCollections([]);
       setIsLoading(false);
@@ -353,6 +387,7 @@ export function useMyPlausibleMeDiscovery(
     if (!enabled) {
       setCollections([]);
       setError(null);
+      setIsLoading(false);
       return;
     }
 
@@ -363,6 +398,11 @@ export function useMyPlausibleMeDiscovery(
 
     return () => {
       clearTimeout(timeoutId);
+      // Invalidate any run already in flight for the previous username. The
+      // next run only starts after the debounce, so without this bump a scan
+      // resolving inside that window would still be the newest generation and
+      // would publish results for a username the caller has moved on from.
+      runIdRef.current += 1;
     };
   }, [username, enabled, discover]);
 

src/loaders/collectionLoader.ts

@@ -106,8 +106,10 @@ function dedupeIds(ids: string[]): string[] {
 /**
  * Remove entities repeating an id already seen, keeping the first.
  *
- * The single-file array formats bypass the index path, so they need the same
- * uniqueness guarantee as dedupeIds gives the index.
+ * dedupeIds only removes repeated fetch paths, so it cannot stop two distinct
+ * index entries (or single-file array rows) from declaring the same id inside.
+ * Every path that produces entities needs this final check to hold the
+ * uniqueness the render layer depends on.
  */
 function dedupeEntitiesById(entities: Entity[]): Entity[] {
   if (entities.length < 2) {
@@ -432,7 +434,9 @@ async function loadEntitiesFromDirectory(
   const workerCount = Math.min(ENTITY_FETCH_CONCURRENCY, ids.length);
   await Promise.all(Array.from({ length: workerCount }, () => worker()));
 
-  return results.filter((entity): entity is Entity => entity !== null);
+  return dedupeEntitiesById(
+    results.filter((entity): entity is Entity => entity !== null)
+  );
 }
 
 /**

src/mechanics/competing/components.tsx

@@ -292,18 +292,21 @@ function RoundResultOverlay() {
   const phase = useCompetingStore((s) => s.phase);
   const numericFields = useCompetingStore((s) => s.numericFields);
   const nextRound = useCompetingStore((s) => s.nextRound);
+  const autoAdvance = useCompetingStore((s) => s.autoAdvance);
 
   const handleDismiss = useCallback(() => {
     if (phase === "round_end") {
       nextRound();
     }
   }, [phase, nextRound]);
 
+  // Only the timer is gated by the setting; manual dismissal (click or key
+  // press) always works, so the overlay can never soft-lock the game.
   useEffect(() => {
-    if (phase !== "round_end") return;
+    if (phase !== "round_end" || !autoAdvance) return;
     const timer = setTimeout(() => { handleDismiss(); }, 2000);
     return () => { clearTimeout(timer); };
-  }, [phase, handleDismiss]);
+  }, [phase, handleDismiss, autoAdvance]);
 
   useEffect(() => {
     if (phase !== "round_end") return;

src/mechanics/quiz/generators/utils.ts

@@ -16,6 +16,21 @@ export const MIN_CARDS_FOR_QUIZ = 4;
  */
 export const WRONG_ANSWER_COUNT = 3;
 
+/**
+ * Maximum cards scored for similarity when picking distractors.
+ *
+ * Similarity scoring is O(cards x fields) and both come from the loaded
+ * collection, so larger collections are sampled down to this many candidates.
+ */
+export const MAX_SIMILARITY_CANDIDATES = 200;
+
+/**
+ * Maximum entity fields compared by a single similarity score.
+ *
+ * Mirrors the MAX_DISPLAYABLE_FIELDS cap used for card details.
+ */
+export const MAX_SIMILARITY_KEYS = 100;
+
 /**
  * Generate a unique question ID.
  */
@@ -73,7 +88,11 @@ export function calculateSimilarity(
 
   // Check other shared string fields (excluding standard fields)
   const excludeFields = new Set(["id", "title", "imageUrl", "year", "categoryShort", "categoryTitle"]);
-  for (const key of Object.keys(card1)) {
+  let keysVisited = 0;
+  for (const key in card1) {
+    if (!Object.hasOwn(card1, key)) continue;
+    if (keysVisited >= MAX_SIMILARITY_KEYS) break;
+    keysVisited++;
     if (excludeFields.has(key)) continue;
     const val1 = card1[key];
     const val2 = card2[key];
@@ -111,8 +130,14 @@ export function selectWrongAnswerCards(
     return shuffled.slice(0, count);
   }
 
-  // Calculate similarity for each available card
-  const withSimilarity = available.map((card) => ({
+  // Score a bounded pool: large collections are sampled first so the scan
+  // stays cheap regardless of how many cards the collection holds
+  const scorable = available.length > MAX_SIMILARITY_CANDIDATES
+    ? shuffle(available).slice(0, MAX_SIMILARITY_CANDIDATES)
+    : available;
+
+  // Calculate similarity for each scorable card
+  const withSimilarity = scorable.map((card) => ({
     card,
     similarity: calculateSimilarity(correctCard, card),
   }));
@@ -122,7 +147,7 @@ export function selectWrongAnswerCards(
 
   // Take the most similar cards, but add some randomness
   // Take top 2*count similar cards, then shuffle and pick count
-  const candidatePool = withSimilarity.slice(0, Math.min(count * 2, available.length));
+  const candidatePool = withSimilarity.slice(0, Math.min(count * 2, withSimilarity.length));
   const shuffledCandidates = shuffle(candidatePool);
   return shuffledCandidates.slice(0, count).map((c) => c.card);
 }

src/mechanics/quiz/store.ts

@@ -5,6 +5,7 @@
  */
 
 import { create } from "zustand";
+import { shuffleWithSeed } from "@/utils/shuffle";
 import { generateQuestions, canGenerateQuiz } from "./generators";
 import type { GeneratorCardData } from "./generators";
 import type {
@@ -51,6 +52,21 @@ interface QuizStore extends QuizState, QuizSettings {
   getShuffledAnswers: () => Answer[];
 }
 
+/**
+ * Hash a string to a 32-bit seed (djb2).
+ *
+ * Question IDs share a long common prefix and differ only in a short suffix,
+ * so summing character codes collapses them onto a handful of seeds and biases
+ * the answer order. Mixing every character keeps the seeds spread out.
+ */
+function hashString(value: string): number {
+  let hash = 5381;
+  for (let i = 0; i < value.length; i++) {
+    hash = ((hash << 5) + hash + value.charCodeAt(i)) | 0;
+  }
+  return hash >>> 0;
+}
+
 /**
  * Initial state.
  */
@@ -344,9 +360,19 @@ export const useQuizStore = create<QuizStore>((set, get) => ({
     const { answers, maxStreak, quizStartedAt, quizEndedAt, questions, timerMode } = get();
 
     const totalScore = answers.reduce((sum, a) => sum + a.pointsEarned, 0);
-    // Max score includes timer bonus if timer mode is enabled
+    // Max score includes timer bonus if timer mode is enabled.
+    // The streak bonus is scored from the streak held *before* the answer, so
+    // question i can earn at most i streak levels: the full streak bonus is
+    // unreachable on the opening questions and must not inflate the maximum.
     const maxTimerBonus = timerMode ? SCORING.timerBonus.fast.points : 0;
-    const maxScore = questions.length * (SCORING.basePoints + SCORING.maxStreakBonus + maxTimerBonus);
+    const maxScore = questions.reduce(
+      (sum, _question, i) =>
+        sum +
+        SCORING.basePoints +
+        Math.min(i * SCORING.streakBonus, SCORING.maxStreakBonus) +
+        maxTimerBonus,
+      0
+    );
     const correctCount = answers.filter((a) => a.isCorrect).length;
     const incorrectCount = answers.filter((a) => !a.isCorrect && a.selectedAnswerId !== null).length;
     const skippedCount = answers.filter((a) => a.selectedAnswerId === null).length;
@@ -377,20 +403,6 @@ export const useQuizStore = create<QuizStore>((set, get) => ({
 
     // Use a seeded shuffle based on question ID for consistency
     // during re-renders (answers stay in same position)
-    const seed = question.id.split("").reduce((acc, char) => acc + char.charCodeAt(0), 0);
-    const shuffled = [...allAnswers];
-
-    // Simple seeded shuffle
-    for (let i = shuffled.length - 1; i > 0; i--) {
-      const j = Math.floor(((seed * (i + 1)) % 1000) / 1000 * (i + 1));
-      const temp = shuffled[i];
-      const swapItem = shuffled[j];
-      if (temp !== undefined && swapItem !== undefined) {
-        shuffled[i] = swapItem;
-        shuffled[j] = temp;
-      }
-    }
-
-    return shuffled;
+    return shuffleWithSeed(allAnswers, hashString(question.id));
   },
 }));