M8: Chrome extension — login, save, list, delete bookmarks#19
Merged
Conversation
- Add extension/ with manifest, popup HTML/CSS/JS, service worker - Add get-user.ts helper to support Bearer token auth alongside cookies - Fix GET /api/bookmarks: add auth check and filter by userId - Fix XSS in renderBookmarks via escapeHtml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
README Formatting README.md |
Removed three header badges, adjusted "Tech Stack" table alignment/padding, and added blank lines before roadmap subsections. |
Bookmark API Routes client/src/app/api/bookmarks/route.ts, client/src/app/api/bookmarks/[id]/route.ts |
Switched GET to accept NextRequest and use getUserFromRequest; GET now filters bookmarks by user.id. Added authenticated DELETE handler at [id] with 401/404/500 responses. |
Authentication Helper client/src/lib/supabase/get-user.ts |
Added getUserFromRequest(req: NextRequest) that reads Bearer token from Authorization header (calls Supabase Auth with token) and falls back to cookie-based server auth. |
Chrome Extension extension/manifest.json, extension/popup.html, extension/popup.css, extension/popup.js, extension/serviceworker.js |
Added Manifest V3 + popup UI, styles, and JS implementing Supabase signup/login/logout, session persistence via chrome.storage, and bookmark list/save/delete via backend API; service worker stub included. |
Sequence Diagram(s)
sequenceDiagram
actor User
participant Popup as Extension Popup
participant Supabase as Supabase Auth
participant API as Backend API
participant DB as Bookmark Storage
User->>Popup: Open popup
Popup->>Popup: Read stored session
alt No session
User->>Popup: Submit email/password
Popup->>Supabase: POST /auth/v1/token or /auth/v1/signup
Supabase-->>Popup: access_token + user
Popup->>Popup: Store token, show main view
else Session exists
Popup->>API: GET /api/bookmarks (Bearer token)
API->>Supabase: validate token / get user
Supabase-->>API: user.id
API->>DB: findMany bookmarks where userId = user.id
DB-->>API: bookmarks
API-->>Popup: bookmarks
Popup->>Popup: render bookmarks
end
sequenceDiagram
actor User
participant Popup as Extension Popup
participant API as Backend API
participant Supabase as Supabase Auth
participant DB as Bookmark Storage
User->>Popup: Click "Save this page"
Popup->>API: POST /api/bookmarks (Bearer token, URL)
API->>Supabase: validate token / get user
Supabase-->>API: user.id
API->>DB: create bookmark with userId
DB-->>API: created bookmark
API-->>Popup: success
User->>Popup: Click delete on item
Popup->>API: DELETE /api/bookmarks/:id (Bearer token)
API->>Supabase: validate token / get user
Supabase-->>API: user.id
API->>DB: delete where id + userId
DB-->>API: deleted or not found
API-->>Popup: 204 or 404
Popup->>Popup: update UI
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- M7: Supabase Auth #11: Modifies bookmarks API authentication/authorization and user scoping—closely related to GET/DELETE auth changes.
- M6 manual tags #5: Overlaps with bookmarks API auth handling and request/response changes to the GET route.
- M4: bookmark list UI renders saved items #2: Related changes to bookmark route handlers (POST/GET) that intersect with this PR's API updates.
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 22.73% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | The description includes bullet-point summaries of changes and partially fills the template, but all checklist items remain unchecked despite significant code changes. | Please verify and check the build, TypeScript, lint, code review, and manual testing items to confirm the PR meets quality requirements before merging. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main feature added: a Chrome extension with login, bookmark saving, listing, and deletion functionality. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
m8-chrome-extension
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
extension/popup.js (1)
4-6: Consider externalizing configuration for deployment flexibility.The Supabase URL, anon key, and API base are hardcoded. While the anon key is public and safe to include, hardcoding these values makes it harder to deploy the extension for different environments (development, staging, production).
Consider using a build-time configuration approach or Chrome extension's storage for configurable endpoints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/popup.js` around lines 4 - 6, Replace the hardcoded constants SUPABASE_URL, SUPABASE_ANON_KEY, and API_BASE with a configurable source: implement a small config loader (e.g., getConfig() or loadConfig()) that first reads runtime overrides from chrome.storage (chrome.storage.sync or local) and falls back to build-time environment values injected during bundling (process.env/DEFINE_PLUGIN) and then to the current defaults; update references in popup.js to call getConfig().SUPABASE_URL / SUPABASE_ANON_KEY / API_BASE and ensure your build pipeline (or a simple config.json) can inject different values per environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extension/manifest.json`:
- Line 18: The manifest's host_permissions only allow localhost but popup.js
performs Supabase auth calls to https://gfdwrzaeofehgpwuulvr.supabase.co, so add
that Supabase origin (with appropriate scheme and wildcard, e.g.
https://gfdwrzaeofehgpwuulvr.supabase.co/*) to the "host_permissions" array in
manifest.json so the extension can make auth requests; update the array to
include the Supabase domain alongside the existing localhost entries.
In `@extension/popup.js`:
- Around line 143-162: renderBookmarks currently injects user-controlled
b.title, b.url and b.id directly into bookmarksList.innerHTML causing an XSS
hole; implement a safe HTML-escaping helper (e.g., escapeHtml) and use it for
all interpolated values (escape b.title, b.url, and b.id) before building the
template, or better yet construct DOM nodes with document.createElement and set
textContent/href attributes instead of using innerHTML; ensure new URL(b.url) is
wrapped in try/catch and fallback to a safe display if parsing fails. Use the
function names renderBookmarks and escapeHtml and the DOM target bookmarksList
to locate where to apply the fixes.
---
Nitpick comments:
In `@extension/popup.js`:
- Around line 4-6: Replace the hardcoded constants SUPABASE_URL,
SUPABASE_ANON_KEY, and API_BASE with a configurable source: implement a small
config loader (e.g., getConfig() or loadConfig()) that first reads runtime
overrides from chrome.storage (chrome.storage.sync or local) and falls back to
build-time environment values injected during bundling
(process.env/DEFINE_PLUGIN) and then to the current defaults; update references
in popup.js to call getConfig().SUPABASE_URL / SUPABASE_ANON_KEY / API_BASE and
ensure your build pipeline (or a simple config.json) can inject different values
per environment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 274e7951-ff1f-4061-946d-cb8488662ca3
📒 Files selected for processing (9)
README.mdclient/src/app/api/bookmarks/[id]/route.tsclient/src/app/api/bookmarks/route.tsclient/src/lib/supabase/get-user.tsextension/manifest.jsonextension/popup.cssextension/popup.htmlextension/popup.jsextension/serviceworker.js
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
extension/popup.js (2)
92-97: Misleading class name inshowAuthSuccess.The function sets
className = "error"for a success message, then overrides the styling with inline styles. This is confusing and harder to maintain.♻️ Proposed refactor
function showAuthSuccess(msg) { authError.textContent = msg; - authError.className = "error"; - authError.style.cssText = - "color:`#4ade80`;border-color:`#166534`;background:`#052e16`;display:block"; + authError.className = "success"; }Then add a
.successclass in your CSS with the appropriate styles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/popup.js` around lines 92 - 97, The showAuthSuccess function currently sets authError.className = "error" and applies inline success styles; change it to assign a semantic "success" class (authError.className = "success") and remove the inline style assignment from showAuthSuccess so styling is driven by CSS; then add a .success rule in your stylesheet matching the intended color/border/background/display used here so the DOM manipulation is clear and maintainable (refer to showAuthSuccess and authError).
128-132: Response body consumed twice on success path.If
res.okis true, line 132 callsres.json()correctly. However, ensure the API always returns a JSON body on success (201). If the server ever returns 204 No Content or empty body, this will throw.♻️ Defensive handling for empty responses
if (!res.ok) { const data = await res.json(); throw new Error(data.error || "Failed to save"); } - return res.json(); + // Handle potential empty response body + const text = await res.text(); + return text ? JSON.parse(text) : {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/popup.js` around lines 128 - 132, The current response handling calls res.json() on success unconditionally and also reads res.json() on error, which will break if the server returns 204/empty body; update the block around the res.ok check to only parse JSON when a body exists: after checking res.ok, first handle empty responses (e.g., if res.status === 204 or res.headers.get('Content-Length') === '0' or the Content-Type header is not application/json) and return null (or an appropriate empty value), otherwise await and return res.json(); similarly, on the !res.ok branch, guard the await res.json() with the same content checks before attempting to parse so you don't attempt to consume an empty body. Ensure you reference and use the existing res.ok and res.json() calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extension/popup.js`:
- Line 6: API_BASE is hardcoded to localhost in popup.js; replace it with a
configurable value by reading a build or runtime config instead (e.g., use
process.env.API_BASE injected at build time or read from chrome.storage.sync/get
on startup) and provide a sensible production fallback (e.g.,
https://api.example.com). Update the reference to API_BASE in popup.js so code
first tries the injected env var (process.env.API_BASE) or chrome.storage key
"API_BASE" and only falls back to the production URL if neither exists.
- Line 167: The anchor href currently uses escapeHtml(b.url) but doesn't block
dangerous protocols (e.g., "javascript:"); add a URL validation/sanitization
step (e.g., a new isSafeUrl or sanitizeUrl function) that parses b.url and only
allows safe protocols (https:, http:, mailto: or other explicit allowlist), and
use its result for the href (fallback to "#" or a safely prefixed URL when
validation fails) instead of raw escapeHtml(b.url); update the template where
the anchor with class "bookmark-title" is rendered to call the sanitizer before
assigning href and keep using escapeHtml for the link text if needed.
---
Nitpick comments:
In `@extension/popup.js`:
- Around line 92-97: The showAuthSuccess function currently sets
authError.className = "error" and applies inline success styles; change it to
assign a semantic "success" class (authError.className = "success") and remove
the inline style assignment from showAuthSuccess so styling is driven by CSS;
then add a .success rule in your stylesheet matching the intended
color/border/background/display used here so the DOM manipulation is clear and
maintainable (refer to showAuthSuccess and authError).
- Around line 128-132: The current response handling calls res.json() on success
unconditionally and also reads res.json() on error, which will break if the
server returns 204/empty body; update the block around the res.ok check to only
parse JSON when a body exists: after checking res.ok, first handle empty
responses (e.g., if res.status === 204 or res.headers.get('Content-Length') ===
'0' or the Content-Type header is not application/json) and return null (or an
appropriate empty value), otherwise await and return res.json(); similarly, on
the !res.ok branch, guard the await res.json() with the same content checks
before attempting to parse so you don't attempt to consume an empty body. Ensure
you reference and use the existing res.ok and res.json() calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ff70293-1ca6-4d92-b3c2-da97e20d660e
📒 Files selected for processing (2)
extension/manifest.jsonextension/popup.js
✅ Files skipped from review due to trivial changes (1)
- extension/manifest.json
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Mar 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Type of change
Checklist
npm run build)npx tsc --noEmit)npm run lint)Summary by CodeRabbit