Fix unsafe JSON.parse, overly broad permissions, and selection capture race conditions

[Copilot]

Six medium-severity issues affecting crash safety, privacy, and timing correctness in the service worker and storage layer.

Storage: Safe JSON.parse with fallback (StorageService.ts)

• getSettings() and getUploadQueueIds() now wrap JSON.parse in try-catch
• On corrupted data: logs the error, resets the offending key to its default, and returns a safe value instead of throwing into the service worker

Manifest: Drop overly broad tabs permission (manifest.template.json)

• Removed "tabs" — activeTab (already declared) covers all active-tab URL/title access needed during capture

Selection capture: URL validation + race condition + stale timeout (service-worker.ts)

• URL guard: rejects injection before attempting it on non-http(s) pages with a descriptive error
• Race condition: cancels and rejects any in-flight pendingSelectionResolve/pendingSelectionReject before overwriting them with a new capture
• Stale timeout: setTimeout handle stored in pendingSelectionTimeoutId and explicitly clearTimeout-ed on every exit path (success, cancel, error, and new-capture preemption)

// Before: silent leak if two captures race
pendingSelectionResolve = resolve;
pendingSelectionReject = reject;
setTimeout(() => { /* never cleared on success */ }, 60000);

// After: preempt previous, clear on all paths
if (pendingSelectionReject) {
  pendingSelectionReject(new Error('Selection cancelled: a new selection was started'));
}
clearTimeout(pendingSelectionTimeoutId);
pendingSelectionTimeoutId = setTimeout(...);

Upload queue: defer processing until auth is ready (UploadService.ts + service-worker.ts)

• restoreQueue() no longer auto-calls processQueue() — removes the window where uploads fire before the auth token is restored
• New startProcessing() method called explicitly from the service worker after NumbersApiManager.initialize() completes

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Security][Medium] Unsafe JSON.parse, overly broad permissions, and race conditions</issue_title>
<issue_description>## Summary

Six medium-severity security findings affecting resilience, permissions, and timing safety.

---

1. Unsafe JSON.parse Without Error Handling

File: src/services/StorageService.ts:135,169

JSON.parse() is called on chrome.storage.local data without try-catch. Corrupted storage (partial writes, manual tampering) causes unhandled exceptions that can crash the service worker.

Fix: Wrap in try-catch, fall back to defaults, and log corruption for debugging.

---

2. Content Script Injection Without Tab URL Validation

File: src/background/service-worker.ts:176-184

handleSelectionCapture injects content scripts without checking the tab URL scheme. Injection attempts on chrome-extension://, file://, or about: pages generate confusing errors.

Fix: Validate tab.url?.match(/^https?:\/\//) before injection. Show a clear user-facing error for unsupported pages.

---

3. Race Condition in Selection Promise Global State

File: src/background/service-worker.ts:162-164

Module-level pendingSelectionResolve/pendingSelectionReject variables are overwritten if two rapid selection captures are triggered. The first caller's promise never resolves — a resource leak and potential UI hang.

Fix: Reject any existing pending selection before starting a new one.

---

4. Overly Broad tabs Permission

File: manifest.template.json:9

The tabs permission grants access to tab.url, tab.title, tab.favIconUrl for ALL tabs. The extension only needs this for the active tab during capture, which activeTab (already declared) provides after user gesture.

Fix: Remove the tabs permission from the manifest. Use activeTab + chrome.tabs.query for specific needs.

---

5. Upload Queue Auto-Starts Before Auth Is Restored

File: src/services/UploadService.ts:40-59

The restoreQueue() method is called from the constructor and immediately calls processQueue(). At this point, the auth token may not yet be set by NumbersApiManager.initialize(). Queued uploads fail with 401 errors, wasting bandwidth.

Fix: Separate queue restoration from processing. Add an explicit startProcessing() method called after auth initialization completes.

---

6. Selection Timeout Not Cleared on Completion

File: src/background/service-worker.ts:187-199

The 60-second setTimeout for selection is never cleared when the selection completes successfully. A stale timeout can reject a subsequent selection's promise if a new capture starts within the window.

Fix: Store and clear the timeout ID on both completion and cancellation.

Impact

These issues affect extension reliability (crashes from corrupted storage, race conditions), privacy (excessive permissions), and user experience (failed uploads after restart, confusing error messages).

Generated by Health Monitor with Omni</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [Security][Medium] Unsafe JSON.parse, overly broad permissions, and race conditions #21

---

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.