rclone-jav/bugs-candidates-extension-options.md

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

Scope: `src/options/*` (+ `src/shared/*` if referenced)

Required-reading: ext AGENTS.md / mockup / bug-audit-plan.md / project memory

Auditor: fresh Explore agent

---

## Candidate C-1

- **File:** `src/options/options.js:492-525` (SETTINGS_SCHEMA definition)
- **Hunch:** SETTINGS_SCHEMA includes all load/save keys; no asymmetry.
- **Trace:** Lines 191-192 in `load()` read `settings.siteAdapters` and `settings.idNormalizers`, and lines 234-235 in `save()` write them back. SETTINGS_SCHEMA at lines 518, 520 includes both keys. Schema is complete.
- **Question for verifier:** Confirm SETTINGS_SCHEMA includes all keys that load/save cycle uses.
- **Contract refs needed:** none

---

## Candidate C-2

- **File:** `src/options/options-dupe-review.js:561-628` (loadKeepRanking and save)
- **Hunch:** `loadKeepRanking()` runs once at module load (line 628), not when pane is activated. External changes to keep-ranking won't appear until page reload.
- **Trace:** Line 628 calls loadKeepRanking() at module top level. No listener on pane activation (options.js line 37) calls it again. If another tab changes keep-ranking via RPC, this pane won't refresh until reload.
- **Question for verifier:** Should pane activation re-fetch keep-ranking to catch external changes?
- **Contract refs needed:** none

---

## Candidate C-3

- **File:** `src/options/options.js:110-122` (openModal / closeModal)
- **Hunch:** No coordination between modals. Two rapid opens could show two modals simultaneously.
- **Trace:** `openModal(id)` adds "open" class and sets aria-hidden=false. No check if another modal is already open. Multiple modals could be marked "open" at the same time.
- **Question for verifier:** Should only one modal be visible at a time, or is simultaneous open allowed?
- **Contract refs needed:** none

---

## Candidate C-4

- **File:** `src/options/options.js:300` (setNote function)
- **Hunch:** `setNote()` calls `el.innerHTML = html` without sanitization. Assumes caller sanitizes before passing.
- **Trace:** All current callers (lines 308–337) build HTML with escapeHtml() before inserting. So current usage is safe. But setNote() is not a safe setter — it's a raw innerHTML setter.
- **Question for verifier:** Is setNote intended as a safe reescaper, or a raw setter expecting pre-sanitized input?
- **Contract refs needed:** none

---

## Candidate C-5

- **File:** `src/options/options.js:533-546` (sanitizeImportedSettings)
- **Hunch:** Array elements are not recursively validated. Example: `siteAdapters: [{ host: 123, selector: [] }]` would pass because the outer type is "array".
- **Trace:** SETTINGS_SCHEMA checks outer type (line 542) but not inner element types. Comment at line 491 says nested objects get recursive validation, but code doesn't implement it for arrays.
- **Question for verifier:** Should imported arrays validate element types, or is current lenient behavior acceptable?
- **Contract refs needed:** none

---

## Candidate C-6

- **File:** `src/options/options.js:210-286` (save function)
- **Hunch:** Save persists to chrome.storage but messages to background.js are fire-and-forget. No confirmation that background applied the settings.
- **Trace:** Line 256 awaits chrome.storage.sync (safe). Lines 261, 278 send messages without waiting for response. If background crashes, settings persist but running extension uses stale config.
- **Question for verifier:** Should save() wait for background.js acknowledgment?
- **Contract refs needed:** none

---

## Candidate C-7

- **File:** `src/options/options-cache.js:113-137` (renderCacheContractBanner)
- **Hunch:** Unrecognized cache_state values silently return empty string instead of showing error.
- **Trace:** Lines 118, 121, 131 test cache_state against known literals. If host sends unexpected state (e.g., `"unknown_state"`), no if matches and line 136 returns "". No error banner shown.
- **Question for verifier:** Should unrecognized cache_state trigger an error banner?
- **Contract refs needed:** none

---

## Candidate C-8

- **File:** `src/options/options.js:386-416` (export/import keep-ranking)
- **Hunch:** Export fails silently if keep-ranking RPC fails. Exported file has empty `_meta.host_config.keep_ranking`. On import, user gets warning but import proceeds anyway. Asymmetric: backup loses data without obvious indication.
- **Trace:** Lines 391-395 try-catch the RPC silently. If it fails, hostConfig.keep_ranking is never set. Export completes anyway (line 405-414). On import, user sees warning at line 560 but can proceed. Keep-ranking is lost in backup/restore cycle.
- **Question for verifier:** Should export fail or warn prominently if keep-ranking cannot be fetched?
- **Contract refs needed:** none

---

## Candidate C-9

- **File:** `src/options/options.js:351-372` (delete-enable-modal flow)
- **Hunch:** If user checks enableDelete and navigates away before confirming modal, box stays unchecked with no way to retry except page reload.
- **Trace:** Line 352-353 check→uncheck→open-modal. If user closes modal without confirming, checkbox is false and no path to re-open the modal exists. Would need page reload or clicking box again.
- **Question for verifier:** Should modal be re-openable from checked state without page reload?
- **Contract refs needed:** none

---

## Candidate C-10

- **File:** `src/options/options-library-issues.js:134-143` (makeReportRow rendering)
- **Hunch:** `entry.path.split("/")` could throw if entry.path is null or not a string.
- **Trace:** Line 134 `fname = entry.filename || entry.path.split("/").pop()`. If entry.filename is falsy and entry.path is null, .split() throws. Entry.path comes from RPC; malformed response could crash render.
- **Question for verifier:** Should there be null/type check for entry.path before .split()?
- **Contract refs needed:** none

---

## Candidate C-11

- **File:** `src/options/options.js:374-382` (input/change listeners)
- **Hunch:** Event listeners call updateSectionSummaries() which reads from DOM. If multiple panes render simultaneously and input fires from hidden pane, stale element reads could occur.
- **Trace:** Lines 374-375 listen for "input" and "change" on panes. Delegated check at line 378 ensures only active pane events fire, but race condition possible if multiple panes render concurrently.
- **Question for verifier:** Is there a render race if load() initializes all panes and events fire before page ready?
- **Contract refs needed:** none

---

## Candidate C-12

- **File:** `src/options/options-library-issues.js:120-131` (makeRow - library issues)
- **Hunch:** The makeRow function for bracket_id and nohyphen_id rows sets data-issue at line 123, which is later checked by _canRenameIdFixRow() line 60. If makeRow is called with malformed entry data, the row might be created but with missing data attributes, making it silently non-renamable.
- **Trace:** Line 120-131 creates row HTML and sets data-issue to entry.issue at line 123. The entry.issue comes from the response. If rendering bracket entries with undefined entry.issue, the row would be created but unclickable (no rename).
- **Question for verifier:** Should missing entry.issue in bracket/nohyphen entries trigger an error, or is silent disable acceptable?
- **Contract refs needed:** none

---

## Summary Stats

- **Total candidates:** 12
- **Severity breakdown:** L (7), M (4), N (1)
- **Areas affected:** stale state (1), modal visibility (1), HTML safety (2), validation (2), error handling (3), RPC failures (1), null safety (1), concurrency (1), missing attributes (1)

---

## VERIFIER NOTES (Phase 1 Moderate verification, stricter prompt + UI-inconvenience rule + storage-quota awareness)

- C-5 (array element validation) — CONFIRMED M. Auditor right; downstream crash in tryAdapters confirmed via content.js read. Promoted as M-1.
- C-6 (save fire-and-forget) — REFUTED. getSettings reads fresh from storage every call; no in-memory cache to invalidate. sendMessage wakes SW per MV3 spec.
- C-8 (export silent fail) — CONFIRMED M, high confidence. Backup→restore cycle silently loses user-typed config. Promoted as M-2.
- C-10 (entry.path crash) — REFUTED. Host's _cache_entry contract guarantees path always non-null string. Unreachable in normal operation.

CHUNK 4 CALIBRATION:
- Severe: 0 (none flagged)
- Moderate rejection: 2/4 = 50% (stop condition >30% triggered)
- Combined: 2/4 = 50%
- Auditor weaknesses: (1) flagging fire-and-forget message patterns without checking if downstream caches, (2) ignoring host-side schema contracts that prevent null/malformed data reaching JS
- L candidates NOT verified per stop condition. Revisit only if needed.