Add Firebase-based cloud backup/restore for bookmark data#1
Add Firebase-based cloud backup/restore for bookmark data#1
Conversation
📝 WalkthroughWalkthroughThis PR adds a Firebase Cloud Backup feature to the bookmark manager extension. It introduces cloud storage integration via Firebase, including UI components for authentication and operations, REST API integration for backup and restore workflows, local state management for cloud data, and manifest permissions to access Firebase services. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Extension UI
participant Script as script.js
participant AuthAPI as Firebase Auth API
participant FirestoreAPI as Firestore API
User->>UI: Enter Firebase credentials & click Sign Up
UI->>Script: authWithFirebase('signUp')
Script->>AuthAPI: POST /accounts:signUp with credentials
AuthAPI-->>Script: idToken, localId
Script->>Script: Update firebaseAuthState
Script->>Script: persistCloudState()
Script-->>UI: setCloudStatus('Signed in')
Script->>Script: restoreFromCloud()
Script->>FirestoreAPI: GET Firestore document
FirestoreAPI-->>Script: Cloud bookmarks data
Script->>Script: Compare timestamps & merge
Script->>Script: Apply to local state & re-render
UI-->>User: Display restored bookmarks
User->>UI: Make local changes & trigger saveAll
Script->>Script: backupToCloud()
Script->>Script: Assemble cloudPayload()
Script->>FirestoreAPI: POST/PUT Firestore document with updated data
FirestoreAPI-->>Script: Success response
Script-->>UI: setCloudStatus('Backed up')
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script.js (1)
20-55:⚠️ Potential issue | 🟠 Major
restoreFromCloudruns on every new-tab open, adding latency and Firestore reads.Line 53 (
await restoreFromCloud(false)) fires on everyloadAll(), which runs whenever a new tab is opened. This means:
- A Firestore REST read on every new tab open (cost + latency).
- With an expired token, it will fail silently each time.
- The
awaitblocks the initial load — the UI won't finish rendering until the network call completes or fails.Consider making the restore non-blocking, or only auto-restoring on first install / sign-in rather than every page load.
🔧 Quick fix — make restore non-blocking
renderFolders(); populateFolderSelect(); renderGoogleApps(); initializeCloudUI(); - await restoreFromCloud(false); + // Non-blocking: don't delay tab render for cloud fetch + restoreFromCloud(false); });
🤖 Fix all issues with AI agents
In `@newtab.html`:
- Around line 31-35: Add accessible labels for each input element
(firebaseApiKey, firebaseProjectId, authEmail, authPassword) by adding
corresponding <label> elements with for attributes matching each input id;
labels may be visually shown or use an sr-only CSS class for screen-reader-only
text so placeholders are not the only accessible hint for screen readers and
users who forget the placeholder once typing begins. Ensure label text clearly
describes the field (e.g., "Firebase API Key", "Firebase Project ID", "Email",
"Password") and that each label-input pair matches by id to maintain proper
associations.
In `@README.md`:
- Line 81: Update the README's "Enable Firestore Database" instruction to remove
the blanket "start in test mode" advice or prepend a prominent security warning
about test mode allowing unauthenticated reads/writes for 30 days, and add a
recommended production rules example for the bookmarkfolderUsers collection that
enforces request.auth != null and request.auth.uid == userId (place the full
rules snippet in a fenced code block and label it as production rules). Ensure
the warning appears immediately next to the Firestore setup step and include a
short note reminding developers to deploy rules before leaving development.
In `@script.js`:
- Around line 13-15: The firebaseAuthState object (idToken, localId, email) is
sensitive and must not be saved to chrome.storage.sync; update any
persistence/read logic that currently writes or reads firebaseAuthState to use
chrome.storage.local instead, while leaving firebaseConfigState (apiKey,
projectId) in chrome.storage.sync if desired; ensure idToken is never written to
sync storage and migrate/remove any existing synced tokens (use lastUpdatedAt to
coordinate migration/cleanup).
- Around line 42-46: The code sets lastUpdatedAt = res.lastUpdatedAt ||
Date.now(), which makes a fresh install appear newer than cloud backups and
prevents restore; change the default fallback to 0 instead (i.e., set
lastUpdatedAt = res.lastUpdatedAt ?? 0) so that when no stored timestamp exists
cloud data is preferred, and keep the chrome.storage.sync.set call (which writes
firebaseConfigState, firebaseAuthState, lastUpdatedAt) so the updated default is
persisted; check restoreFromCloud to rely on this 0 default for initial
restores.
- Around line 769-788: The restoreFromCloud flow assigns untrusted JSON into
bookmarks/folders/openFolders (in restoreFromCloud) and those values are later
rendered via innerHTML in renderFolders and populateFolderSelect; to fix, ensure
all user-controlled strings are escaped or inserted as text nodes before any DOM
insertion: add/use an escapeHTML utility (or prefer
element.textContent/createElement and appendChild) when rendering folder names,
bookmark names, descriptions and any interpolated fields, and validate/sanitize
b.icon before setting it as an image src; update renderFolders,
populateFolderSelect and any code that builds HTML from bookmarks/folders to use
the sanitizer or textContent approach instead of raw innerHTML.
- Around line 744-793: restoreFromCloud currently calls saveAll() which
immediately triggers backupToCloud and writes the restored data back to
Firestore; stop that by adding a way to save locally without invoking cloud
backup: modify saveAll to accept an optional flag (e.g., saveAll(skipCloud =
false)) or create a saveLocalOnly wrapper that performs the same local
persistence steps but does not call backupToCloud, update saveAll implementation
to call backupToCloud only when skipCloud is false, and change restoreFromCloud
to call saveAll(true) (or saveLocalOnly) after populating
bookmarks/folders/openFolders; ensure other callers of saveAll keep the default
behavior so normal saves still back up to cloud.
- Around line 674-699: The authWithFirebase function currently drops
data.refreshToken; modify it to persist refreshToken into firebaseAuthState
(e.g., firebaseAuthState.refreshToken) when setting idToken/localId/email and
call persistCloudState(); then implement a token refresh helper (e.g.,
refreshFirebaseIdToken) that POSTs to
https://securetoken.googleapis.com/v1/token with grant_type=refresh_token and
the stored refreshToken to obtain a new id_token and refresh_token, update
firebaseAuthState and persist; invoke this helper before cloud operations
(backupToCloud, restoreFromCloud) or on 401 responses from cloud endpoints to
automatically refresh tokens and retry the request, falling back to prompting
re-authentication if refresh fails.
🧹 Nitpick comments (5)
style.css (1)
730-752: Stylelint flags modern color-function notation, but the existing file consistently usesrgba()with decimal alphas.The linter prefers
rgb(255 255 255 / 4%)overrgba(255, 255, 255, 0.04). Since the entire file (~40+ occurrences) uses the legacy notation, this is a codebase-wide concern rather than something to fix only in the new lines. Consider a follow-up to migrate the whole file if you want to satisfy the linter.newtab.html (1)
35-35: Consider addingautocomplete="new-password"on the password field.Without this, the browser may offer to save/autofill credentials from the new-tab page, which could confuse users or leak the Firebase password into their browser's credential manager.
script.js (3)
61-65:saveAllis synchronous but triggers asyncbackupToCloud— errors are silently swallowed.
backupToCloudreturns a promise but it's not awaited (fire-and-forget). If the cloud backup fails, there's no indication to the user in the non-showMessagepath. Additionally, rapid successive calls tosaveAll(e.g., during drag-and-drop) will fire multiple concurrent backup requests with no debouncing.Consider debouncing the cloud backup call (e.g., 2–3 seconds) to avoid excessive API calls during rapid edits.
707-709:cloudDocumentUrldoes not encode path segments.If
firebaseConfigState.projectIdorfirebaseAuthState.localIdever contain special characters, the URL would be malformed. While Firebase-generatedlocalIdvalues are typically safe, defensively encoding the values is good practice.🔧 Proposed fix
function cloudDocumentUrl() { - return `https://firestore.googleapis.com/v1/projects/${firebaseConfigState.projectId}/databases/(default)/documents/bookmarkfolderUsers/${firebaseAuthState.localId}`; + return `https://firestore.googleapis.com/v1/projects/${encodeURIComponent(firebaseConfigState.projectId)}/databases/(default)/documents/bookmarkfolderUsers/${encodeURIComponent(firebaseAuthState.localId)}`; }
638-640:persistCloudStatestores auth tokens butsignOutFirebasedoesn't clear the password field.After
signOutFirebase(line 701–705), the password<input>still retains the user's password in the DOM. Consider clearing the password field on sign-out to reduce credential exposure.🔧 Proposed fix in signOutFirebase
function signOutFirebase() { firebaseAuthState = { idToken: "", localId: "", email: "" }; persistCloudState(); updateCloudStatusFromState(); + const pw = document.getElementById("authPassword"); + if (pw) pw.value = ""; }
| <input id="firebaseApiKey" type="text" placeholder="Firebase Web API Key"> | ||
| <input id="firebaseProjectId" type="text" placeholder="Firebase Project ID"> | ||
|
|
||
| <input id="authEmail" type="email" placeholder="Email"> | ||
| <input id="authPassword" type="password" placeholder="Password"> |
There was a problem hiding this comment.
Inputs lack <label> elements — accessibility concern.
Screen readers may not announce placeholder-only inputs correctly, and placeholders disappear once the user starts typing, making it hard to recall what each field expects. Add visible or sr-only labels for each input.
♿ Proposed fix (example for one input)
+ <label for="firebaseApiKey">Firebase Web API Key</label>
<input id="firebaseApiKey" type="text" placeholder="Firebase Web API Key">
+ <label for="firebaseProjectId">Firebase Project ID</label>
<input id="firebaseProjectId" type="text" placeholder="Firebase Project ID">
+ <label for="authEmail">Email</label>
<input id="authEmail" type="email" placeholder="Email">
+ <label for="authPassword">Password</label>
<input id="authPassword" type="password" placeholder="Password">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <input id="firebaseApiKey" type="text" placeholder="Firebase Web API Key"> | |
| <input id="firebaseProjectId" type="text" placeholder="Firebase Project ID"> | |
| <input id="authEmail" type="email" placeholder="Email"> | |
| <input id="authPassword" type="password" placeholder="Password"> | |
| <label for="firebaseApiKey">Firebase Web API Key</label> | |
| <input id="firebaseApiKey" type="text" placeholder="Firebase Web API Key"> | |
| <label for="firebaseProjectId">Firebase Project ID</label> | |
| <input id="firebaseProjectId" type="text" placeholder="Firebase Project ID"> | |
| <label for="authEmail">Email</label> | |
| <input id="authEmail" type="email" placeholder="Email"> | |
| <label for="authPassword">Password</label> | |
| <input id="authPassword" type="password" placeholder="Password"> |
🤖 Prompt for AI Agents
In `@newtab.html` around lines 31 - 35, Add accessible labels for each input
element (firebaseApiKey, firebaseProjectId, authEmail, authPassword) by adding
corresponding <label> elements with for attributes matching each input id;
labels may be visually shown or use an sr-only CSS class for screen-reader-only
text so placeholders are not the only accessible hint for screen readers and
users who forget the placeholder once typing begins. Ensure label text clearly
describes the field (e.g., "Firebase API Key", "Firebase Project ID", "Email",
"Password") and that each label-input pair matches by id to maintain proper
associations.
|
|
||
| 1. Create a Firebase project | ||
| 2. Enable **Authentication → Email/Password** | ||
| 3. Enable **Firestore Database** (start in test mode while developing) |
There was a problem hiding this comment.
"Start in test mode" advice is a security risk.
Firestore test mode allows unauthenticated reads and writes to all data for 30 days. Users who follow this advice and forget to update rules will expose all stored bookmark data (including any data from other users). At minimum, add a warning and provide recommended production security rules.
📝 Suggested documentation improvement
-3. Enable **Firestore Database** (start in test mode while developing)
+3. Enable **Firestore Database** and set security rules (see below)Then expand the security note at the end:
-> Security note: set Firestore security rules so users can only read/write their own doc.
+> **Security rules (required):** Apply the following Firestore rules so each user can only access their own document:
+>
+> ```
+> rules_version = '2';
+> service cloud.firestore {
+> match /databases/{database}/documents {
+> match /bookmarkfolderUsers/{userId} {
+> allow read, write: if request.auth != null && request.auth.uid == userId;
+> }
+> }
+> }
+> ```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 3. Enable **Firestore Database** (start in test mode while developing) | |
| 3. Enable **Firestore Database** and set security rules (see below) |
🤖 Prompt for AI Agents
In `@README.md` at line 81, Update the README's "Enable Firestore Database"
instruction to remove the blanket "start in test mode" advice or prepend a
prominent security warning about test mode allowing unauthenticated reads/writes
for 30 days, and add a recommended production rules example for the
bookmarkfolderUsers collection that enforces request.auth != null and
request.auth.uid == userId (place the full rules snippet in a fenced code block
and label it as production rules). Ensure the warning appears immediately next
to the Firestore setup step and include a short note reminding developers to
deploy rules before leaving development.
| let firebaseConfigState = { apiKey: "", projectId: "" }; | ||
| let firebaseAuthState = { idToken: "", localId: "", email: "" }; | ||
| let lastUpdatedAt = Date.now(); |
There was a problem hiding this comment.
Firebase auth token stored in chrome.storage.sync — consider chrome.storage.local instead.
chrome.storage.sync replicates data across all signed-in Chrome instances. Storing idToken (a bearer credential) in sync storage means it will be synced to every device the user is signed into Chrome on. Use chrome.storage.local for sensitive auth state, or at minimum store only the non-sensitive config (apiKey, projectId) in sync and keep tokens local.
🤖 Prompt for AI Agents
In `@script.js` around lines 13 - 15, The firebaseAuthState object (idToken,
localId, email) is sensitive and must not be saved to chrome.storage.sync;
update any persistence/read logic that currently writes or reads
firebaseAuthState to use chrome.storage.local instead, while leaving
firebaseConfigState (apiKey, projectId) in chrome.storage.sync if desired;
ensure idToken is never written to sync storage and migrate/remove any existing
synced tokens (use lastUpdatedAt to coordinate migration/cleanup).
| firebaseConfigState = res.firebaseConfig || { apiKey: "", projectId: "" }; | ||
| lastUpdatedAt = res.lastUpdatedAt || Date.now(); | ||
| firebaseAuthState = res.firebaseAuth || { idToken: "", localId: "", email: "" }; | ||
|
|
||
| chrome.storage.sync.set({ bookmarks, folders, openFolders, firebaseConfig: firebaseConfigState, firebaseAuth: firebaseAuthState, lastUpdatedAt }); |
There was a problem hiding this comment.
lastUpdatedAt defaults to Date.now() on first load, which may prevent cloud restore.
Line 43: if lastUpdatedAt is not yet stored, it defaults to the current time. When a user installs the extension on a new device and signs in, the freshly-initialized lastUpdatedAt will be "now", which is newer than any existing cloud backup. The timestamp comparison in restoreFromCloud (line 773) will then skip the restore, defeating the purpose.
Default to 0 instead so cloud data is always preferred on a fresh install.
🐛 Proposed fix
- lastUpdatedAt = res.lastUpdatedAt || Date.now();
+ lastUpdatedAt = res.lastUpdatedAt || 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| firebaseConfigState = res.firebaseConfig || { apiKey: "", projectId: "" }; | |
| lastUpdatedAt = res.lastUpdatedAt || Date.now(); | |
| firebaseAuthState = res.firebaseAuth || { idToken: "", localId: "", email: "" }; | |
| chrome.storage.sync.set({ bookmarks, folders, openFolders, firebaseConfig: firebaseConfigState, firebaseAuth: firebaseAuthState, lastUpdatedAt }); | |
| firebaseConfigState = res.firebaseConfig || { apiKey: "", projectId: "" }; | |
| lastUpdatedAt = res.lastUpdatedAt || 0; | |
| firebaseAuthState = res.firebaseAuth || { idToken: "", localId: "", email: "" }; | |
| chrome.storage.sync.set({ bookmarks, folders, openFolders, firebaseConfig: firebaseConfigState, firebaseAuth: firebaseAuthState, lastUpdatedAt }); |
🤖 Prompt for AI Agents
In `@script.js` around lines 42 - 46, The code sets lastUpdatedAt =
res.lastUpdatedAt || Date.now(), which makes a fresh install appear newer than
cloud backups and prevents restore; change the default fallback to 0 instead
(i.e., set lastUpdatedAt = res.lastUpdatedAt ?? 0) so that when no stored
timestamp exists cloud data is preferred, and keep the chrome.storage.sync.set
call (which writes firebaseConfigState, firebaseAuthState, lastUpdatedAt) so the
updated default is persisted; check restoreFromCloud to rely on this 0 default
for initial restores.
| async function authWithFirebase(mode) { | ||
| const email = document.getElementById("authEmail")?.value.trim(); | ||
| const password = document.getElementById("authPassword")?.value.trim(); | ||
|
|
||
| if (!cloudConfigReady()) return setCloudStatus("Add Firebase API Key + Project ID first.", true); | ||
| if (!email || !password) return setCloudStatus("Enter email and password.", true); | ||
|
|
||
| try { | ||
| const endpoint = `https://identitytoolkit.googleapis.com/v1/accounts:${mode}?key=${firebaseConfigState.apiKey}`; | ||
| const res = await fetch(endpoint, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ email, password, returnSecureToken: true }) | ||
| }); | ||
|
|
||
| const data = await res.json(); | ||
| if (!res.ok) throw new Error(data.error?.message || "Authentication failed"); | ||
|
|
||
| firebaseAuthState = { idToken: data.idToken, localId: data.localId, email: data.email || email }; | ||
| persistCloudState(); | ||
| updateCloudStatusFromState(); | ||
| await restoreFromCloud(false); | ||
| } catch (err) { | ||
| setCloudStatus(`Auth failed: ${err.message}`, true); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Firebase Identity Toolkit API refresh token endpoint documentation
💡 Result:
Firebase (Identity Toolkit / Firebase Auth) refresh-token exchange is documented under “Exchange a refresh token for an ID token” and uses the Secure Token API:
- HTTP method:
POST - Endpoint:
https://securetoken.googleapis.com/v1/token?key=[API_KEY] - Content-Type:
application/x-www-form-urlencoded - Body fields:
grant_type=refresh_token(always)refresh_token=[REFRESH_TOKEN](the user’s Firebase refresh token)
- Response includes:
id_token,refresh_token,expires_in,token_type,user_id,project_id[1] (also documented for Identity Platform) [2]
Example (from the docs):
curl 'https://securetoken.googleapis.com/v1/token?key=[API_KEY]' \
-H 'Content-Type: application/x-www-form-urlencoded' \
--data 'grant_type=refresh_token&refresh_token=[REFRESH_TOKEN]'Sources: Firebase Auth REST API docs [1], Google Cloud Identity Platform REST docs [2].
🏁 Script executed:
# Search for refreshToken in the codebase
rg "refreshToken" -i --type js
# Check the firebaseAuthState structure and its usage
rg "firebaseAuthState" --type js -A 3 -B 1
# Look for token-related code around the auth function
rg "idToken|token" script.js -i -B 2 -A 2Repository: rishnudk/BookmarkFolder
Length of output: 4641
🏁 Script executed:
# Check for 401 error handling in cloud operations
rg "401|unauthorized|token.*expire|Bearer" script.js -i -B 2 -A 2
# Look at the full backupToCloud and restoreFromCloud functions
rg "async function (backupToCloud|restoreFromCloud)" -A 30 script.jsRepository: rishnudk/BookmarkFolder
Length of output: 2294
🌐 Web query:
Firebase ID token expiration duration hours
💡 Result:
Firebase Authentication ID tokens expire after 1 hour (≈ 3600 seconds) — i.e., 1 hour. The client SDKs automatically refresh them (using the refresh token) about every hour. [1], [2]
(If you meant custom tokens you mint server-side: their exp can be at most 1 hour after iat.) [5]
🌐 Web query:
Firebase Identity Toolkit signupNewUser signin REST API response format refreshToken
💡 Result:
For Firebase Auth / Identity Toolkit REST (Identity Platform), the refreshToken is returned in the JSON response when you send returnSecureToken: true.
signupNewUser (aka accounts:signUp)
Endpoint: POST https://identitytoolkit.googleapis.com/v1/accounts:signUp?key=API_KEY
Response JSON (success):
{
"kind": "identitytoolkit#SignupNewUserResponse",
"idToken": "…",
"email": "user@example.com",
"refreshToken": "…",
"expiresIn": "3600",
"localId": "…"
}(expiresIn is a string containing the number of seconds until the ID token expires.) [1][2]
signin (aka accounts:signInWithPassword)
Endpoint: POST https://identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=API_KEY
Response JSON (success):
{
"kind": "identitytoolkit#VerifyPasswordResponse",
"localId": "…",
"email": "user@example.com",
"displayName": "",
"idToken": "…",
"registered": true,
"refreshToken": "…",
"expiresIn": "3600"
}
``` [2][3]
If you’re not seeing `refreshToken` in responses, it’s typically because `returnSecureToken` was omitted or set to `false`. [2]
**Sources:** [1] SignUpResponse schema, [2] “Using the REST API” (sample requests/responses), [3] `accounts.signInWithPassword` reference response schema.
---
</details>
**Store `refreshToken` and implement token refresh before cloud operations.**
`authWithFirebase` receives and discards the `refreshToken` returned by the Identity Toolkit API. Since ID tokens expire after 1 hour, all cloud operations (`backupToCloud`, `restoreFromCloud`) will fail with a 401 when `firebaseAuthState.idToken` becomes invalid. The current error handling will mask this as a generic "Backup failed" or "Restore failed" message without recovery.
Store `data.refreshToken` in `firebaseAuthState` and refresh the token using `https://securetoken.googleapis.com/v1/token` (with `grant_type=refresh_token`) before making cloud calls, or detect 401 responses and prompt re-authentication.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @script.js around lines 674 - 699, The authWithFirebase function currently
drops data.refreshToken; modify it to persist refreshToken into
firebaseAuthState (e.g., firebaseAuthState.refreshToken) when setting
idToken/localId/email and call persistCloudState(); then implement a token
refresh helper (e.g., refreshFirebaseIdToken) that POSTs to
https://securetoken.googleapis.com/v1/token with grant_type=refresh_token and
the stored refreshToken to obtain a new id_token and refresh_token, update
firebaseAuthState and persist; invoke this helper before cloud operations
(backupToCloud, restoreFromCloud) or on 401 responses from cloud endpoints to
automatically refresh tokens and retry the request, falling back to prompting
re-authentication if refresh fails.
</details>
<!-- fingerprinting:phantom:medusa:phoenix -->
<!-- This is an auto-generated comment by CodeRabbit -->
| async function restoreFromCloud(showMessage = true) { | ||
| if (!cloudConfigReady() || !cloudAuthReady()) { | ||
| if (showMessage) setCloudStatus("Cloud restore skipped: sign in first.", true); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const res = await fetch(cloudDocumentUrl(), { | ||
| headers: { Authorization: `Bearer ${firebaseAuthState.idToken}` } | ||
| }); | ||
|
|
||
| if (res.status === 404) { | ||
| if (showMessage) setCloudStatus("No cloud backup found yet."); | ||
| return; | ||
| } | ||
|
|
||
| const data = await res.json(); | ||
| if (!res.ok) throw new Error(data.error?.message || "Restore failed"); | ||
|
|
||
| const encoded = data.fields?.data?.stringValue; | ||
| if (!encoded) { | ||
| if (showMessage) setCloudStatus("Cloud document is empty.", true); | ||
| return; | ||
| } | ||
|
|
||
| const cloud = JSON.parse(encoded); | ||
| const localUpdatedAt = Number(lastUpdatedAt || 0); | ||
| const cloudUpdatedAt = Number(cloud.updatedAt || 0); | ||
|
|
||
| if (cloudUpdatedAt < localUpdatedAt) { | ||
| if (showMessage) setCloudStatus("Local data is newer than cloud."); | ||
| return; | ||
| } | ||
|
|
||
| bookmarks = Array.isArray(cloud.bookmarks) ? cloud.bookmarks : []; | ||
| folders = Array.isArray(cloud.folders) && cloud.folders.length ? cloud.folders : ["Default"]; | ||
| openFolders = cloud.openFolders && typeof cloud.openFolders === "object" ? cloud.openFolders : {}; | ||
|
|
||
| folders.forEach(f => { | ||
| if (openFolders[f] === undefined) openFolders[f] = f === "Default"; | ||
| }); | ||
|
|
||
| saveAll(); | ||
| renderFolders(); | ||
| populateFolderSelect(); | ||
|
|
||
| if (showMessage) setCloudStatus("Restore complete."); | ||
| } catch (err) { | ||
| setCloudStatus(`Restore failed: ${err.message}`, true); | ||
| } |
There was a problem hiding this comment.
restoreFromCloud → saveAll → backupToCloud creates a wasteful round-trip.
Line 786 calls saveAll(), which (line 64) immediately calls backupToCloud(false), pushing the just-restored data right back to Firestore. This is a redundant write on every restore and could also cause unexpected timestamp advancement.
Consider splitting the persistence concern: save to local storage without triggering cloud backup after a restore.
🔧 Proposed approach — add a flag to skip cloud backup
-function saveAll() {
+function saveAll({ skipCloudBackup = false } = {}) {
lastUpdatedAt = Date.now();
chrome.storage.sync.set({ bookmarks, folders, openFolders, firebaseConfig: firebaseConfigState, firebaseAuth: firebaseAuthState, lastUpdatedAt });
- backupToCloud(false);
+ if (!skipCloudBackup) backupToCloud(false);
}Then in restoreFromCloud:
- saveAll();
+ saveAll({ skipCloudBackup: true });🤖 Prompt for AI Agents
In `@script.js` around lines 744 - 793, restoreFromCloud currently calls saveAll()
which immediately triggers backupToCloud and writes the restored data back to
Firestore; stop that by adding a way to save locally without invoking cloud
backup: modify saveAll to accept an optional flag (e.g., saveAll(skipCloud =
false)) or create a saveLocalOnly wrapper that performs the same local
persistence steps but does not call backupToCloud, update saveAll implementation
to call backupToCloud only when skipCloud is false, and change restoreFromCloud
to call saveAll(true) (or saveLocalOnly) after populating
bookmarks/folders/openFolders; ensure other callers of saveAll keep the default
behavior so normal saves still back up to cloud.
| const cloud = JSON.parse(encoded); | ||
| const localUpdatedAt = Number(lastUpdatedAt || 0); | ||
| const cloudUpdatedAt = Number(cloud.updatedAt || 0); | ||
|
|
||
| if (cloudUpdatedAt < localUpdatedAt) { | ||
| if (showMessage) setCloudStatus("Local data is newer than cloud."); | ||
| return; | ||
| } | ||
|
|
||
| bookmarks = Array.isArray(cloud.bookmarks) ? cloud.bookmarks : []; | ||
| folders = Array.isArray(cloud.folders) && cloud.folders.length ? cloud.folders : ["Default"]; | ||
| openFolders = cloud.openFolders && typeof cloud.openFolders === "object" ? cloud.openFolders : {}; | ||
|
|
||
| folders.forEach(f => { | ||
| if (openFolders[f] === undefined) openFolders[f] = f === "Default"; | ||
| }); | ||
|
|
||
| saveAll(); | ||
| renderFolders(); | ||
| populateFolderSelect(); |
There was a problem hiding this comment.
XSS risk: cloud-restored data is rendered via innerHTML without sanitization.
restoreFromCloud parses arbitrary JSON from Firestore (line 769) and assigns it directly to bookmarks and folders. These values are later interpolated into innerHTML (e.g., line 122: <span class="folder-name">${folder}</span> and line 217: <div class="tile-name">${b.name}</div>).
A compromised or tampered Firestore document could inject malicious HTML/JS into the page. Sanitize or escape all user-controlled strings before DOM insertion, or switch to textContent/createElement for rendering.
🛡️ Minimal fix — escape HTML in rendered values
Add a utility:
function escapeHTML(str) {
const div = document.createElement("div");
div.textContent = str;
return div.innerHTML;
}Then use it in renderFolders:
- <span class="folder-name">${folder}</span>
+ <span class="folder-name">${escapeHTML(folder)}</span>- <div class="tile-name">${b.name}</div>
+ <div class="tile-name">${escapeHTML(b.name)}</div>Also escape b.icon in the <img src=...> or validate it's a proper URL.
🤖 Prompt for AI Agents
In `@script.js` around lines 769 - 788, The restoreFromCloud flow assigns
untrusted JSON into bookmarks/folders/openFolders (in restoreFromCloud) and
those values are later rendered via innerHTML in renderFolders and
populateFolderSelect; to fix, ensure all user-controlled strings are escaped or
inserted as text nodes before any DOM insertion: add/use an escapeHTML utility
(or prefer element.textContent/createElement and appendChild) when rendering
folder names, bookmark names, descriptions and any interpolated fields, and
validate/sanitize b.icon before setting it as an image src; update
renderFolders, populateFolderSelect and any code that builds HTML from
bookmarks/folders to use the sanitizer or textContent approach instead of raw
innerHTML.
1 participant
Motivation
Description
Firebase Web API Key,Firebase Project ID, email/password auth fields and buttons forSign Up,Sign In,Sign Out,Backup to CloudandRestore from Cloud(newtab.htmlandstyle.css).script.jsincluding local state forfirebaseConfig,firebaseAuth, andlastUpdatedAt, plus functionsauthWithFirebase,backupToCloud,restoreFromCloud,cloudDocumentUrl, and UI helpers (initializeCloudUI,setCloudStatus,persistCloudState).loadAll()restores cloud state on load andsaveAll()updateslastUpdatedAt, persists local state and triggers a background backup when signed in.manifest.jsonand document setup + usage inREADME.md(Firestore document path:bookmarkfolderUsers/{firebaseLocalUserId}).Testing
node --check script.jsto validate JavaScript syntax and it completed successfully.manifest.jsonformatting withpython -m json.tool manifest.jsonand it parsed successfully.python -m http.server 4173and captured a visual verification screenshot using Playwright showing the new Firebase panel (artifact produced).Codex Task
Summary by CodeRabbit
New Features
Documentation