rclone-jav/bugs-extension-options.md

# Bug Report — Extension Options pages — audit-snapshot-2026-05-24T15-55Z.md

Snapshot: audit-snapshot-2026-05-24T15-55Z.md
Required-reading docs read: ext AGENTS.md / mockup / bug-audit-plan.md / project memory
Auditor agent: fresh Explore agent (chunk 4 auditor)
Verifier agents: fresh Explore agents per candidate, blind context, stricter contract-check prompt + UI-inconvenience rule + chrome.storage.sync quota awareness

**Chunk 4 calibration note:** Moderate verification yielded 2 confirmed bugs with 50% rejection rate (2/4 REFUTED). Auditor's recurring weakness: flagging fire-and-forget message patterns as data-loss without tracing whether downstream reads from storage every call (no in-memory cache to invalidate). Also misjudged contract-guaranteed nullability (C-10 cited entry.path null crash without checking host's `_cache_entry` guarantees). Stricter verifier prompt + UI-inconvenience rule + storage-quota awareness caught both false positives. **Light candidates were NOT verified per audit-plan stop condition** (>30% rejection → halt L verification). See `bugs-candidates-extension-options.md` for unverified L list (C-1, C-2, C-3, C-4, C-7, C-9, C-11, C-12).

---

**Cross-chunk re-rank note:** Per `bugs-fix-queue.md`, this chunk's M-2 (export silently drops keep_ranking) was **promoted to Severe** because silent backup data loss fits the S criterion ("silent success when operation actually failed"). M-1 (sanitizeImportedSettings) remains Moderate. Severity sections below reflect post-rerank placement.

---

## Severe (S)

Definition: data loss/corruption · wrong remote operation · persistent broken workflow no recovery · silent success when operation actually failed.

### S-1 (queue) — Export silently drops keep_ranking when host RPC fails; backup→restore loses user-typed VIP/format prefs

**Re-ranked from chunk M-2 to queue S-1 (silent backup data loss).**

- **File:** `D:\DEV\Extensions\Production\rclone-jav\src\options\options.js:386-416` (export flow), import display at `:540-565`
- **Symptom:** When user clicks Export while host is unreachable, `get-keep-ranking` RPC fails, error swallowed by `try/catch {}`, hostConfig empty, export JSON has `_meta.host_config: {}`, export reports plain "exported." with no warning. On restore, user loses VIP folders / format prefs / size tolerance / tiebreak rules silently.
- **Why it's a bug:** Export try/catch wraps RPC at line ~393 with bare `catch {}` at line ~395. Falls through to line ~401 with `hostConfig = {}`. Status message at line ~415 hardcoded to "exported." regardless of RPC result. Import side reads `data?._meta?.host_config?.keep_ranking`; null shows informational "Not included" message but does NOT block confirm. User loses manually-configured ranking on restore.
- **Reproduction:**
    1. Kill host process (or block network so RPC times out). Open Setup → Backup → Export.
    2. Expected: export blocked with "Cannot export — keep_ranking unreachable" OR file marked partial AND status visibly warns.
    3. Actual: download triggers, file has `_meta.host_config: {}`, status says "exported." User believes complete backup. Later imports → keep-ranking silently absent. User must re-type configuration.
- **Suggested fix sketch:** on RPC failure, EITHER block export with retry prompt OR write file with explicit `_meta.partial: true` flag + prominent "Partial export — keep_ranking unavailable" status; import side must treat absent keep_ranking as hard warning requiring explicit "proceed without" confirmation, not dismissable info.
- **Verifier verdict:** CONFIRMED — high confidence (95%)
- **Mirror check needed in:** any other RPC-sourced data in export `_meta.host_config` (currently only keep_ranking)
- **Status:** fixed
- **Fix:** `D:\DEV\Extensions\Production\rclone-jav\src\options\options.js:386-447` — export now blocks if `get-keep-ranking` RPC fails (any of: missing response, !ok, missing keep_ranking, value not an object, exception). Status shows specific failure reason + retry instruction. No file written on failure. Success path requires `r.keep_ranking` to be a valid object (not the literal `null`/array/etc.) and writes `_meta.host_config.keep_ranking` into the JSON. Status message updated to `"exported (settings + keep_ranking)."` to confirm both halves landed. Manifest bumped 0.1.32 → 0.1.33. JS syntax verified via `node --check`. Runtime proof confirmed both paths: failure → status "export blocked — host could not return keep ranking: Specified native messaging host not found. Confirm native host is running, then retry." + no file; success → file with full keep_ranking object (5 fields populated).

---

## Moderate (M)

### M-1 (queue) — sanitizeImportedSettings doesn't validate array element shape; downstream crash

- **File:** `D:\DEV\Extensions\Production\rclone-jav\src\options\options.js:533-546` (sanitizeImportedSettings), with downstream crash sites at `content.js:tryAdapters` (~line 57-73) and `src\shared\id-extract.js:applyNormalizers` (~line 22-31)
- **Symptom (one sentence):** A settings import with malformed array elements (e.g. `siteAdapters: [{ host: 123, selector: [] }]` or `idNormalizers: [{ re: null, fmt: 42 }]`) passes the import sanitizer because only the outer "array" type is checked — the bad data persists to chrome.storage.sync, then crashes content script's `tryAdapters` at `a.selector.split(",")` (TypeError on `.split` of an Array) the next time the user visits any web page.
- **Why it's a bug:** `sanitizeImportedSettings` (line 533-546) checks `_typeOf(v) !== expected` against SETTINGS_SCHEMA outer types only. The comment at line 491 claims nested objects get recursive validation but the code doesn't implement it for arrays. Result: idNormalizers crash is silently swallowed by `try/catch` in applyNormalizers (search degraded silently). siteAdapters crash is NOT wrapped — content script's tryAdapters throws TypeError, breaks ID extraction on the affected page, and subsequent extraction failures propagate to all auto-check + manual search via content scripts. Import modal does NOT preview adapter contents, so user has no chance to spot bad data before confirming.
- **Reproduction:**
    1. Input: hand-craft a JSON export file with `settings.siteAdapters = [{host: ["array"], selector: 5}]`, import via Setup → Import Settings
    2. Expected: import rejects entry with "siteAdapters[0] malformed" or strips the bad entry
    3. Actual: import succeeds with status "imported." Subsequent web page navigations trigger content script auto-check → `tryAdapters` throws TypeError on `a.selector.split` → ID extraction breaks on that page → manual searches also fail because shared content path is broken
- **Suggested fix sketch:** add per-key element validation in sanitizeImportedSettings for siteAdapters (each element must be `{host: string, selector: string}`) and idNormalizers (each element must be `{re: string|RegExp, fmt: string}`). Drop malformed elements with a note in the import warning.
- **Verifier agent:** fresh Explore, blind context, stricter prompt
- **Verifier verdict:** CONFIRMED
- **Verifier confidence:** medium-high (downstream impact varies — idNormalizers silently degrades, siteAdapters breaks content script)
- **Contract refs verifier read:** SETTINGS_SCHEMA outer-type behavior; tryAdapters string assumptions; applyNormalizers try/catch
- **Mirror check needed in:** profiles[] array (also passes outer type check); partPatterns[] array — same pattern likely applies
- **Status:** fixed
- **Fix:** `D:\DEV\Extensions\Production\rclone-jav\src\options\options.js:552-633` — added `ARRAY_ELEMENT_VALIDATORS` map keyed by setting name (covers siteAdapters, idNormalizers, partPatterns, knownSitePatterns, profiles). Each validator rejects only shapes that would crash consumers; tolerates extra unknown fields for forward-compat with future exports. `sanitizeImportedSettings` now filters malformed elements per-array — good ones pass through, bad ones get logged to `dropped[]` with their index (e.g. `siteAdapters[2](malformed)`); if ALL elements in an array are bad, the key falls through to defaults via mergeSettings instead of persisting empty array. Dropped list capped at 30 entries to keep import modal readable. Mirror check resolved in same commit (validators cover all four flagged keys). Manifest bumped 0.1.33 → 0.1.34. JS syntax verified via `node --check`. Validator unit-tested inline: 9 acceptable shapes accepted + 9 malformed shapes rejected (18/18 pass). Runtime repro requires user test (import a JSON with `siteAdapters: [{host: 123, selector: []}]` → expect modal shows `siteAdapters[0](malformed)` in Ignored keys row + the entry is NOT persisted to chrome.storage.sync; subsequent web page navigations should NOT crash content.js).

(M-2 moved to Severe S-1 above per cross-chunk re-rank)

---

## Light (L)
(none promoted — chunk 4 L verification skipped per stop condition)

---

## Needs Input (N)
(none promoted)

---

## False Positives (discarded)

- `src/options/options.js:210-286` (save fire-and-forget settings-changed message) — flagged as Moderate "stale in-memory config in background after save". REFUTED. Background's `getSettings()` (background.js:74-77) reads from chrome.storage.sync on every call — no in-memory cache to invalidate. The `settings-changed` message is just an optimization for context-menu rebuild; if the message is lost (which MV3 doesn't actually do — `sendMessage` wakes the SW per spec), the next native call still reads fresh settings from storage. No persistent stale state.

- `src/options/options-library-issues.js:134-143` (entry.path null crash in makeReportRow) — flagged as Moderate "TypeError crashes Library Issues modal render". REFUTED via host-side schema contract: `rcjav/library.py:_cache_entry` always sets `"path": f.get("path", "")` (non-null string guarantee). Host's `handle_library_issues` passes through structurally-valid entries; the null-path scenario is unreachable in normal operation. If cache.json was directly corrupted with missing path keys, Python-side direct `f["path"]` access (library.py:257) would raise KeyError before any data reached the JS render layer. Severity if reachable would be CRITICAL (uncaught TypeError crashes modal — no outer try/catch), but the contract closes the attack surface. Note: defensive null check would be cheap if ever wanted as belt-and-suspenders.