Session resurrection V2

[TheAdnan]

mainly AI slop - trying to get this up to date!

[TheAdnan]

Code Review: Session Resurrection (Firefox Add-on)

Executive Summary

The "Session Resurrection" add-on provides a simple and useful functionality for saving and restoring browser sessions. However, the current implementation has several critical security vulnerabilities (HTML injection), privacy concerns (excessive permissions), and architectural weaknesses (non-persistent settings, storage pollution) that should be addressed before wider distribution or submission to AMO (Add-ons Mozilla).

---

1. Security & Privacy

🚩 Critical: HTML Injection (XSS)

In sessions-pop-up/save-session.js, the session name provided by the user is directly injected into the DOM using jQuery's $() template string:

const $li = $(`
<li id="${key_id}" class="collection-item _cao_session_item">
    <p class="_cao_session_item_label">${key}</p>
    ...
</li>
`);

Impact: A user could accidentally or maliciously enter a session name containing <script> tags or onerror handlers (e.g., <img src=x onerror=alert(1)>), leading to script execution within the context of the extension popup.
Recommendation: Use .text() to set content or sanitize the input before injection.

🚩 High: Excessive Permissions

The manifest.json requests <all_urls> in host_permissions:

"host_permissions": [
    "<all_urls>"
]

Impact: This permission allows the extension to read and modify data on every website the user visits. It triggers a high-risk warning during installation.
Observation: The extension only uses browser.tabs.query to get URLs and browser.tabs.create to restore them. These do NOT require <all_urls> when the "tabs" permission is already present.
Recommendation: Remove <all_urls> to respect user privacy and pass AMO review easily.

⚠️ Medium: Remote Resource Loading

The extension loads fonts and icons from Google Fonts (fonts.googleapis.com).
Impact:

1. Privacy: The user's IP and browser info are shared with Google when the popup opens.
2. CSP: Strict Content Security Policies in Manifest V3 may block remote styles/fonts.
   Recommendation: Bundle the fonts and icons locally within the extension.

---

2. Architecture & Design

⚠️ Storage Strategy

The extension stores each session as a top-level key in browser.storage.local.
Impact: This "pollutes" the storage namespace and makes it difficult to add other configuration settings later without collision.
Recommendation: Store all sessions under a single sessions key (e.g., browser.storage.local.set({ sessions: { ... } })).

⚠️ Non-Persistent Configuration

The "Tabs from all windows" setting is stored in a volatile variable:

const srConfig = { captureWindows: false };

Impact: When the popup is closed, this state is lost. If a user checks the box, closes the popup, and reopens it, the checkbox remains visually unchecked (by default) and the logic resets to false.
Recommendation: Store user preferences in browser.storage.local.

⚠️ Logic Fragility (DOM IDs)

The code generates DOM IDs from session names:

const key_id = key.replace(/\s+/g, '-').toLowerCase();

Impact: Different session names (e.g., "My Session" and "my session") will produce the same ID, leading to duplicate IDs in the DOM and broken event listeners/UI updates.

---

3. Code Quality & Best Practices

• Over-Engineering (Dependencies): Using both jQuery and Materialize CSS for a popup with one input and a list is excessive. This increases the extension's footprint and load time.
• Error Handling: Errors are logged to the console (onError), but the user receives no visual feedback if a save or restore operation fails.
• Background Script Efficiency: In background.js, tabs are created sequentially using await in a loop. For very large sessions, this might be slightly slow, though generally acceptable for user experience to maintain order.

---

4. UI/UX Improvements

• Destructive Actions: The "Delete All Sessions" button performs a permanent action without a confirmation dialog.
• Input Validation: There is minimal validation on the session name (only length check).
• Empty State: When no sessions are saved, the list is just empty. Adding a "No sessions saved" message would improve UX.

---

Recommended Action Plan

1. Fix XSS: Replace HTML injection with safe DOM manipulation.
2. Reduce Permissions: Remove <all_urls> from manifest.json.
3. Refactor Storage: Move sessions into a nested object and persist configuration.
4. Localize Assets: Download and bundle Google Fonts/Icons.
5. Add Confirmations: Implement a confirmation prompt for "Delete All".

[TheAdnan]

Code Review: Session Resurrection (Firefox Add-on)

Executive Summary

The "Session Resurrection" add-on provides a simple and useful functionality for saving and restoring browser sessions. However, the current implementation has several critical security vulnerabilities (HTML injection), privacy concerns (excessive permissions), and architectural weaknesses (non-persistent settings, storage pollution) that should be addressed before wider distribution or submission to AMO (Add-ons Mozilla).

1. Security & Privacy

🚩 Critical: HTML Injection (XSS)

In sessions-pop-up/save-session.js, the session name provided by the user is directly injected into the DOM using jQuery's $() template string:

const $li = $(`
<li id="${key_id}" class="collection-item _cao_session_item">
    <p class="_cao_session_item_label">${key}</p>
    ...
</li>
`);

Impact: A user could accidentally or maliciously enter a session name containing <script> tags or onerror handlers (e.g., <img src=x onerror=alert(1)>), leading to script execution within the context of the extension popup. Recommendation: Use .text() to set content or sanitize the input before injection.

🚩 High: Excessive Permissions

The manifest.json requests <all_urls> in host_permissions:

"host_permissions": [
    "<all_urls>"
]

Impact: This permission allows the extension to read and modify data on every website the user visits. It triggers a high-risk warning during installation. Observation: The extension only uses browser.tabs.query to get URLs and browser.tabs.create to restore them. These do NOT require <all_urls> when the "tabs" permission is already present. Recommendation: Remove <all_urls> to respect user privacy and pass AMO review easily.

⚠️ Medium: Remote Resource Loading

The extension loads fonts and icons from Google Fonts (fonts.googleapis.com). Impact:

1. **Privacy:** The user's IP and browser info are shared with Google when the popup opens.

2. **CSP:** Strict Content Security Policies in Manifest V3 may block remote styles/fonts.
   **Recommendation:** Bundle the fonts and icons locally within the extension.

2. Architecture & Design

⚠️ Storage Strategy

The extension stores each session as a top-level key in browser.storage.local. Impact: This "pollutes" the storage namespace and makes it difficult to add other configuration settings later without collision. Recommendation: Store all sessions under a single sessions key (e.g., browser.storage.local.set({ sessions: { ... } })).

⚠️ Non-Persistent Configuration

The "Tabs from all windows" setting is stored in a volatile variable:

const srConfig = { captureWindows: false };

Impact: When the popup is closed, this state is lost. If a user checks the box, closes the popup, and reopens it, the checkbox remains visually unchecked (by default) and the logic resets to false. Recommendation: Store user preferences in browser.storage.local.

⚠️ Logic Fragility (DOM IDs)

The code generates DOM IDs from session names:

const key_id = key.replace(/\s+/g, '-').toLowerCase();

Impact: Different session names (e.g., "My Session" and "my session") will produce the same ID, leading to duplicate IDs in the DOM and broken event listeners/UI updates.

3. Code Quality & Best Practices

* **Over-Engineering (Dependencies):** Using both **jQuery** and **Materialize CSS** for a popup with one input and a list is excessive. This increases the extension's footprint and load time.

* **Error Handling:** Errors are logged to the console (`onError`), but the user receives no visual feedback if a save or restore operation fails.

* **Background Script Efficiency:** In `background.js`, tabs are created sequentially using `await` in a loop. For very large sessions, this might be slightly slow, though generally acceptable for user experience to maintain order.

4. UI/UX Improvements

* **Destructive Actions:** The "Delete All Sessions" button performs a permanent action without a confirmation dialog.

* **Input Validation:** There is minimal validation on the session name (only length check).

* **Empty State:** When no sessions are saved, the list is just empty. Adding a "No sessions saved" message would improve UX.

Recommended Action Plan

1. **Fix XSS:** Replace HTML injection with safe DOM manipulation.

2. **Reduce Permissions:** Remove `<all_urls>` from `manifest.json`.

3. **Refactor Storage:** Move sessions into a nested object and persist configuration.

4. **Localize Assets:** Download and bundle Google Fonts/Icons.

5. **Add Confirmations:** Implement a confirmation prompt for "Delete All".

After doing these suggested improvements, here's what the review says:

Summary

• Performed a comprehensive code review and refactored the extension to improve UX, performance, and robustness.
• Replaced intrusive browser dialogs (alert, confirm) with Materialize components and optimized data migration logic.

Changes

• UX Improvements: Replaced alert() and confirm() with Materialize M.toast and a custom confirmation Modal in save-session.js and index.html.
• Performance Optimization: Moved session migration logic to a separate function (migrateOldSessions) that runs once on startup, rather than every time the session list is refreshed.
• Robustness: Enhanced background.js to gracefully handle cases where no browser window is focused, including smart creation of a new window without redundant blank tabs.
• Feedback: Added informative toasts when restoring sessions and deleting data to provide immediate user feedback.
• Clean Code: Refactored getSessions to use more targeted storage queries and improved error handling throughout the popup script.

Verification

• Manual code inspection confirms all alert and confirm calls are now either replaced or have safe fallbacks.
• Background script logic for window creation was verified to correctly handle empty/new window states.
• Modal integration in index.html matches Materialize requirements.