-
Notifications
You must be signed in to change notification settings - Fork 432
feat(web): add proxy for supabase storage images #1958
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add /api/images/$ server route to proxy requests to supabase storage - Replace all direct supabase storage URLs with proxy URLs in apps/web - Keeps og.tsx edge function unchanged as it runs on Netlify edge Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR migrates image asset references across the web application from external Supabase storage URLs to internal API-proxied paths. A new API route proxies image requests to Supabase storage with proper path sanitization and caching headers. Only image source strings are modified; no control flow or business logic changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant AppServer
participant Supabase
Browser->>AppServer: GET /api/images/hyprnote/icon.png
Note right of AppServer: sanitizePath(params.path)\nvalidate segments
AppServer->>Supabase: GET https://.../public_images/hyprnote/icon.png
Supabase-->>AppServer: 200 + Content-Type, Cache-Control?, body
Note right of AppServer: if no Cache-Control -> set public, max-age=31536000, immutable
AppServer-->>Browser: 200 + Content-Type + Cache-Control + body
alt Supabase 404
Supabase-->>AppServer: 404
AppServer-->>Browser: 404
else Supabase error
Supabase-->>AppServer: error/5xx
AppServer-->>Browser: 502
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/web/src/components/footer.tsx (1)
21-27: Footer logo path is correct; consider centralizing logo markupUsing
/api/images/hyprnote/logo.svgaligns with the header and new proxy. As a small cleanup, you could eventually factor a shared<Logo />component that encapsulates thissrc+ sizing and reuse it in header/footer/auth to avoid future drift.apps/web/src/components/header.tsx (1)
47-51: Header logo URLs are consistent; consider reusing a shared Logo componentBoth desktop and mobile headers correctly use
/api/images/hyprnote/logo.svg, matching the footer and the new image proxy. To avoid future inconsistencies, you might later extract a small<Logo />component used in header/footer/auth instead of repeating the raw<img>blocks.Also applies to: 147-151
apps/web/src/routes/_view/product/workflows.tsx (1)
370-385: Icon proxy paths look correct; a11y alt text could be refined laterUsing
/api/images/icons/${...}for both the integrations grid and draggable icons is a clean way to route these through the new proxy; the data arrays line up with the constructed paths. For accessibility, the grid images already usealt={integration.name}, which is good. The draggable icons use a generic"Integration"alt; if these are purely decorative, you could later change that toalt=""so they’re ignored by screen readers, or pass a more descriptive label.Also applies to: 562-570
apps/web/src/routes/api/images.$.ts (2)
34-34: Reconsider aggressive cache-control defaults.The 1-year immutable cache is appropriate for versioned assets but prevents cache invalidation if an asset needs updating. Consider whether all images are truly immutable or if a shorter TTL with revalidation would be more appropriate.
Options:
- If assets are versioned/hashed in their paths: keep current strategy
- If assets may change: use
public, max-age=86400, must-revalidate(24 hours)- If dynamic: use
public, max-age=3600, stale-while-revalidate=86400(1 hour + stale-while-revalidate)Additionally, consider respecting the upstream cache-control header when present (which the code already does on line 32).
6-44: Consider adding observability and security enhancements.The proxy route would benefit from additional production-ready features:
Consider these improvements:
- Request logging for monitoring and debugging:
console.log(`[API] Image request: ${path}`);
- Security headers on the response:
headers["X-Content-Type-Options"] = "nosniff"; headers["X-Frame-Options"] = "DENY";
- Error handling for fetch failures:
try { const response = await fetch(url); // ... existing logic } catch (error) { console.error(`[API] Failed to fetch image: ${path}`, error); return new Response("Service unavailable", { status: 503 }); }
- File type validation to restrict to expected image formats:
const allowedExtensions = ['.jpg', '.jpeg', '.png', '.svg', '.webp', '.gif']; const ext = path.toLowerCase().match(/\.[^.]+$/)?.[0]; if (!ext || !allowedExtensions.includes(ext)) { return new Response("Invalid file type", { status: 400 }); }These additions improve security, observability, and maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/header.tsx(2 hunks)apps/web/src/components/video-thumbnail.tsx(1 hunks)apps/web/src/routes/__root.tsx(2 hunks)apps/web/src/routes/_view/about.tsx(6 hunks)apps/web/src/routes/_view/brand.tsx(1 hunks)apps/web/src/routes/_view/changelog/$slug.tsx(1 hunks)apps/web/src/routes/_view/changelog/index.tsx(1 hunks)apps/web/src/routes/_view/download/index.tsx(1 hunks)apps/web/src/routes/_view/index.tsx(15 hunks)apps/web/src/routes/_view/press-kit.tsx(3 hunks)apps/web/src/routes/_view/pricing.tsx(1 hunks)apps/web/src/routes/_view/product/ai-assistant.tsx(2 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(8 hunks)apps/web/src/routes/_view/product/bot.tsx(1 hunks)apps/web/src/routes/_view/product/mini-apps.tsx(2 hunks)apps/web/src/routes/_view/product/workflows.tsx(3 hunks)apps/web/src/routes/_view/vs/$slug.tsx(1 hunks)apps/web/src/routes/api/images.$.ts(1 hunks)apps/web/src/routes/auth.tsx(1 hunks)apps/web/src/routes/join-waitlist.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/product/bot.tsxapps/web/src/components/footer.tsxapps/web/src/routes/_view/product/mini-apps.tsxapps/web/src/routes/_view/download/index.tsxapps/web/src/routes/__root.tsxapps/web/src/routes/_view/vs/$slug.tsxapps/web/src/routes/auth.tsxapps/web/src/routes/_view/product/workflows.tsxapps/web/src/routes/_view/changelog/$slug.tsxapps/web/src/components/header.tsxapps/web/src/routes/_view/brand.tsxapps/web/src/routes/_view/index.tsxapps/web/src/routes/_view/changelog/index.tsxapps/web/src/routes/_view/product/ai-assistant.tsxapps/web/src/routes/_view/press-kit.tsxapps/web/src/routes/_view/about.tsxapps/web/src/routes/join-waitlist.tsxapps/web/src/routes/_view/pricing.tsxapps/web/src/routes/_view/product/ai-notetaking.tsxapps/web/src/components/video-thumbnail.tsxapps/web/src/routes/api/images.$.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/src/routes/api/images.$.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:29.314Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/web/content/changelog/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:29.314Z
Learning: Applies to apps/web/content/changelog/** : Only keep desktop-related changes when maintaining changelog entries from commits and diffs
Applied to files:
apps/web/src/routes/_view/changelog/$slug.tsx
📚 Learning: 2025-11-24T16:32:30.770Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/web/content/changelog/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:30.770Z
Learning: Applies to apps/web/content/changelog/** : Only include desktop-related changes in the changelog when reading through commits and diffs
Applied to files:
apps/web/src/routes/_view/changelog/$slug.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (29)
apps/web/src/components/video-thumbnail.tsx (1)
25-36: Poster path change looks good; just ensure the asset exists behind the proxySwitching
posterto/api/images/hyprnote/poster-image.pngis consistent with the new image proxy and keeps everything same-origin. Please just double‑check that this path is wired correctly in the new/api/imagesroute and that the asset exists so we don’t regress thumbnails.apps/web/src/routes/_view/download/index.tsx (1)
201-213: CTA icon now uses proxied asset; consistent with rest of appUpdating the CTASection icon to
/api/images/hyprnote/icon.pngmatches the new proxy pattern and other usages of the Hyprnote icon. Looks good as long as the proxy serves this path correctly.apps/web/src/routes/_view/product/bot.tsx (1)
177-183: Draggable icon image now uses proxied Hyprnote iconPointing the draggable bot icon at
/api/images/hyprnote/icon.pngis consistent with other views and keeps it on the same origin. No issues from a layout/interaction standpoint.apps/web/src/routes/auth.tsx (1)
53-71: Auth header icon now goes through/api/images; matches global patternThe auth Header now uses
/api/images/hyprnote/icon.png, which keeps it aligned with other screens and the new proxy route. No behavioral changes to the auth flow; just ensure this asset is present in the backing storage and correctly mapped by the/api/imageshandler.apps/web/src/routes/_view/product/workflows.tsx (1)
256-266: Workflows hero icon uses shared proxied Hyprnote iconThe fixed center icon now points at
/api/images/hyprnote/icon.png, which is consistent with other pages and the CTA sections. No issues from the draggable/connection-line logic perspective.apps/web/src/routes/_view/product/ai-assistant.tsx (1)
47-53: AI Assistant hero + CTA images correctly migrated to/api/imagesBoth the hero GIF (
/api/images/hyprnote/ai-assistant.gif) and the CTA icon (/api/images/hyprnote/icon.png) now use the internal image proxy, aligning with the rest of the app. Alt text is appropriate; just ensure these two assets are present and correctly mapped in the backing storage.Also applies to: 420-427
apps/web/src/routes/join-waitlist.tsx (1)
30-36: Background texture URLs updated correctly.Both texture asset URLs have been migrated to the internal API proxy paths. The changes are consistent and straightforward.
apps/web/src/routes/__root.tsx (1)
38-50: Social media preview images migrated successfully.Both
og:imageandtwitter:imagemeta tags now reference the internal API proxy. This ensures consistent image serving across social platforms.apps/web/src/routes/_view/product/mini-apps.tsx (3)
49-73: Floating panel tab images migrated correctly.All five tab image URLs have been updated to use the internal API proxy paths. The changes are consistent and maintain the existing structure.
626-639: Search demo images updated successfully.All three advanced search image URLs now reference the internal API proxy. No structural changes to the images array.
705-705: CTA section icon URL migrated.The Hyprnote icon in the call-to-action section now uses the internal API proxy path.
apps/web/src/routes/_view/pricing.tsx (1)
333-333: Icon URL updated correctly.The CTA section icon now uses the internal API proxy path, consistent with the broader migration pattern.
apps/web/src/routes/_view/about.tsx (1)
51-686: All team and brand asset URLs migrated successfully.All founder images, team photos, icons, and signature assets have been updated to use internal API proxy paths. The changes are consistent throughout and maintain the existing data structure.
apps/web/src/routes/_view/product/ai-notetaking.tsx (6)
49-177: Hero and tab images migrated correctly.All image URLs in the tabs array and hero section have been updated to use internal API proxy paths. Changes are consistent and straightforward.
624-668: Transcription section assets updated.Both on-device transcription illustration images now reference the internal API proxy.
1031-1031: Search section background image migrated.The background texture for the search section now uses the internal API proxy path.
1095-1122: Mock collaborator avatars updated.All six mock user avatar URLs have been migrated to the internal API proxy paths.
2070-2093: Floating panel tab images array updated.All five floating panel images in the second tabs array now reference internal API proxy paths.
2315-2315: CTA section icon migrated.The Hyprnote icon in the call-to-action section now uses the internal API proxy.
apps/web/src/routes/_view/vs/$slug.tsx (1)
139-139: Asset URL migration is correctly implemented and verified.The icon source has been successfully updated to use the internal API proxy at
/api/images/hyprnote/icon.png. The underlying API route (/api/images/$.ts) is properly implemented with a GET handler that proxies requests to Supabase storage, includes appropriate error handling, and preserves content-type and cache-control headers.apps/web/src/routes/_view/press-kit.tsx (1)
80-122: LGTM! Consistent proxy path migration.The changes correctly migrate folder and icon images to the internal API proxy, using appropriate path prefixes:
/api/images/icons/for macOS system icons (folders, mail, GitHub)/api/images/hyprnote/for the app iconapps/web/src/routes/_view/changelog/$slug.tsx (1)
263-268: LGTM! Icon path logic preserved.The conditional icon selection logic remains intact; only the URL paths have been updated to use the internal API proxy.
apps/web/src/routes/_view/changelog/index.tsx (1)
145-150: LGTM! Consistent with changelog detail page.Icon path updates match the pattern used in the changelog detail page (
$slug.tsx), ensuring consistency across the changelog views.apps/web/src/routes/_view/index.tsx (5)
28-103: LGTM! Main feature and detail feature arrays updated consistently.All feature image URLs in both
mainFeaturesanddetailsFeaturesarrays have been migrated to the internal API proxy paths. The structure and logic remain unchanged.
638-734: LGTM! CoolStuffSection images migrated.The "No bots" and "No internet" feature images have been updated to use the internal proxy paths consistently across both mobile and desktop layouts.
1073-1282: LGTM! Main features section images updated.App icon and static placeholder images now use the internal API proxy. The conditional rendering logic and hover states are preserved.
1575-1657: LGTM! Manifesto section assets migrated.Texture backgrounds (white-leather.png, paper.png), team photos, and signature assets have all been updated to use the internal API proxy paths. The styling and layout remain unchanged.
1762-1854: LGTM! CTA section icon updated.The final app icon reference in the CTA section has been migrated consistently with all other icon references throughout the page.
apps/web/src/routes/_view/brand.tsx (1)
28-53: Clarify that this change adds an API proxy layer, not a migration away from external storage.The review mischaracterizes the change. The
/api/images/hyprnote/...URLs are not a migration away from Supabase; they still proxy to the same Supabase storage backend (https://ijoptyyjrfqwaqhyxkxj.supabase.co/storage/v1/object/public/public_images) via the API route handler atapps/web/src/routes/api/images.$.ts.The actual change is:
- Before: Likely direct Supabase URLs in the VISUAL_ASSETS array (not visible in current state)
- Now: Client-side URLs route through
/api/images/$which proxies to SupabaseThe API route implementation is sound—it properly preserves Content-Type headers, handles 404s, and sets aggressive cache headers (
max-age=31536000, immutable). However, this is a proxy layer addition, not an external dependency removal.The verification requests are valid:
- ✓ API proxy route exists and is functional
- ✓ Assets are accessible via the new paths (route handles 404s and fetch errors correctly)
- ✓ Caching headers are properly configured
Approve the code structure, but correct the narrative to accurately reflect that this centralizes image serving through an API proxy while maintaining the same external Supabase backend.
Likely an incorrect or invalid review comment.
…roxy - Add sanitizePath function to validate and sanitize the path parameter - Reject paths with .., backslashes, empty segments, or unsafe characters - Only allow alphanumeric characters, dots, hyphens, and underscores - Return 404 for upstream 404 errors, 502 for other upstream errors Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/routes/api/images.$.ts (1)
3-4: Make Supabase storage URL environment‑configurable.Hardcoding the storage URL makes it harder to switch buckets/environments (staging, prod) or change providers. Prefer reading from env with a sane default.
-const SUPABASE_STORAGE_URL = - "https://ijoptyyjrfqwaqhyxkxj.supabase.co/storage/v1/object/public/public_images"; +const SUPABASE_STORAGE_URL = + process.env.SUPABASE_STORAGE_URL ?? + "https://ijoptyyjrfqwaqhyxkxj.supabase.co/storage/v1/object/public/public_images";
🧹 Nitpick comments (3)
apps/web/src/routes/api/images.$.ts (3)
6-32: Path sanitization is solid; only tiny nits.The decode + traversal checks + strict segment whitelist give good protection against path traversal and malformed encodings, and the
+inclusion addresses thesymbol+logo.pngcase. The only nits are thatsegments.length === 0is unreachable (split always returns at least one element) and could be dropped, and you might add a brief comment explaining the SAFE_SEGMENT policy for future maintainers.
46-53: Add network error handling and propagate upstream status on success.Two robustness points here:
fetch(url)can throw (DNS, timeout, etc.). Right now that would bubble as a generic 500. Wrapping it in try/catch and returning a 502 keeps the proxy behavior consistent with your explicit non‑OK handling.- For successful responses, forcing
status: 200discards upstream statuses like206 Partial Content, which can break range requests/partial downloads. Returningresponse.statuspreserves upstream semantics.- const response = await fetch(url); + let response: Response; + try { + response = await fetch(url); + } catch { + return new Response("Upstream service error", { status: 502 }); + } @@ - return new Response(response.body, { - status: 200, - headers, - }); + return new Response(response.body, { + status: response.status, + headers, + });Also applies to: 68-71
55-66: Confirm assets are truly immutable before using 1‑yearCache-Control.The default
public, max-age=31536000, immutableis great for static, never‑changing assets, but will cause very sticky caching if content ever changes without a path change. Make sure Supabase keys for these images are treated as immutable (e.g., new filenames/paths on change) so you don’t need to shorten the TTL later.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/api/images.$.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/src/routes/api/images.$.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/api/images.$.ts
🧬 Code graph analysis (1)
apps/web/src/routes/api/images.$.ts (1)
apps/web/src/routes/__root.tsx (1)
Route(22-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
- GitHub Check: Pages changed - hyprnote
Summary
This PR adds a server-side proxy route (
/api/images/$) to serve images from Supabase storage, replacing direct Supabase URLs throughout the web app. The proxy fetches images fromhttps://ijoptyyjrfqwaqhyxkxj.supabase.co/storage/v1/object/public/public_images/and forwards them with appropriate caching headers.Changes include:
/api/images/$catch-all server route inapps/web/src/routes/api/images.$.tsog.tsx) intentionally left unchanged as it runs on Netlify's edge networkUpdates since last revision
Added security hardening based on CodeRabbit feedback:
sanitizePath()function that validates path parameters before proxying..,\, absolute paths)[A-Za-z0-9._+-]per segment (updated to include+forsymbol+logo.png)//)Review & Testing Checklist for Human
__root.tsxchangesog:imageandtwitter:imageto/api/images/hyprnote/og-image.jpg. Social media crawlers require absolute URLs. This will break social sharing previews. Consider reverting these specific changes or making them absolute URLs./api/images/hyprnote/icon.pngon the deployed preview/brandpage and verifysymbol+logo.pngloads correctlyRecommended test plan:
/api/images/pathsNotes
max-age=31536000, immutable) means image updates on Supabase won't reflect for up to a year unless cache is busted