Privatize images bucket and migrate URLs#1526
Conversation
📝 WalkthroughWalkthroughThis PR transitions image storage from public to private accessibility by implementing signed URL generation for app icons, user avatars, and organization logos with 7-day expiration. The images bucket visibility is changed from public to private, with new fine-grained RLS policies managing access. URL normalization ensures backward compatibility. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.0.0)supabase/migrations/20260129121000_private_images_bucket.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8480c08bd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| UPDATE public.apps | ||
| SET icon_url = regexp_replace(split_part(icon_url, '?', 1), '^.*/storage/v1/object/(public/|sign/)?images/', '') | ||
| WHERE icon_url IS NOT NULL |
There was a problem hiding this comment.
This migration rewrites apps.icon_url to a storage path. The public app endpoints (e.g., supabase/functions/_backend/public/app/get.ts and app/post.ts) return icon_url verbatim, so after this runs API/CLI consumers will receive a relative path in a now-private bucket rather than a usable URL. The frontend now signs paths client-side, but external clients won’t, which breaks backward compatibility for any integration that expects an image URL. Consider returning signed URLs (or keeping URL-form values for API responses) to avoid breaking existing consumers.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/AppSetting.vue (1)
724-729: Fix bucket selection for the icon upload.Line 725 uses
.from(\images/org/...`), which incorrectly passes a path as the bucket name. Supabase Storage expects the bucket name only; the path belongs in the.upload()call. Use theimagesbucket and passiconPathto.upload()` instead.🛠️ Proposed fix
- const { error } = await supabase.storage - .from(`images/org/${appRef.value?.owner_org.id}/${props.appId}`) - .upload('icon', blob, { + const { error } = await supabase.storage + .from('images') + .upload(iconPath, blob, { contentType: mimeType, })
🤖 Fix all issues with AI agents
In `@src/services/storage.ts`:
- Around line 17-31: The cache currently stores raw signed URLs (signedUrlCache)
which expire after SIGNED_URL_TTL_SECONDS; modify the caching to store an object
containing { url, expiresAt } (expiresAt = Date.now() +
SIGNED_URL_TTL_SECONDS*1000) keyed by cacheKey, and in the retrieval path (where
signedUrlCache.get(cacheKey) is used) check expiresAt and treat expired entries
as a cache miss that triggers a refresh via the same
useSupabase().storage.from('images').createSignedUrl(normalized,
SIGNED_URL_TTL_SECONDS) flow; update signedUrlCache.set to store the object and
always return data.signedUrl when fresh, returning '' only on real fetch errors.
In `@supabase/migrations/20260129120000_private_images_bucket.sql`:
- Around line 31-45: The SQL policy uses 0-based indices on
storage.foldername(name) which in Postgres is 1-based; update every occurrence
of (storage.foldername(name))[0] → (storage.foldername(name))[1],
(storage.foldername(name))[1] → (storage.foldername(name))[2], and
(storage.foldername(name))[2] → (storage.foldername(name))[3] across all
policies (SELECT/USING checks, INSERT, both UPDATE policies, and DELETE) so path
segment checks (e.g. org/{org_id}/{app_id}/... and user/{user_id}/avatar/...)
work correctly; ensure you change these in expressions that call
public.check_min_rights and public.get_identity_org_appid as well as any direct
comparisons of storage.foldername(name).
🧹 Nitpick comments (4)
src/services/storage.ts (2)
1-1: Use the~/alias for the Supabase import.Line 1 uses a relative import even though this file lives under
src/.
As per coding guidelines: Use~/alias for imports fromsrc/directory in frontend TypeScript and Vue components.♻️ Proposed change
-import { useSupabase } from './supabase' +import { useSupabase } from '~/services/supabase'
10-11: Avoid dropping absolute URLs.If legacy data already stores a public/signed URL, returning
''removes the image entirely. Consider returning the URL as‑is when it’s already absolute.💡 Suggested tweak
- if (path.includes('://')) - return '' + if (path.includes('://')) + return pathsrc/services/photos.ts (2)
9-9: Use~/alias for the new src import.
Switch to~/services/storagefor frontend import consistency.♻️ Proposed change
-import { createSignedImageUrl } from './storage' +import { createSignedImageUrl } from '~/services/storage'As per coding guidelines:
src/**/*.{ts,tsx,vue,js}: Use~/alias for imports fromsrc/directory in frontend TypeScript and Vue components.
16-37: Avoid orphaned uploads when signed URL creation fails.
If upload succeeds butcreateSignedImageUrlreturns empty, we return failure but leave the object behind. Consider deleting the object or retrying to avoid storage bloat.🧹 Example cleanup on signed URL failure
const signedUrl = error ? '' : await createSignedImageUrl(storagePath) isLoading.value = false - if (error || !signedUrl) - await callback(false, '', '') - else - await callback(true, storagePath, signedUrl) + if (error || !signedUrl) { + if (!error) + await supabase.storage.from('images').remove([storagePath]) + await callback(false, '', '') + return + } + await callback(true, storagePath, signedUrl)
There was a problem hiding this comment.
Pull request overview
Makes the Supabase images bucket private and shifts the app to store image paths in DB while resolving them to signed URLs at runtime.
Changes:
- Set the
imagesstorage bucket to private (seed + migration) and migrate existing stored public URLs to bucket-relative paths. - Replace storage RLS on
storage.objectsfor theimagesbucket to support private access (org/app icons + org/user images). - Add a frontend helper to create signed image URLs and update multiple frontend call sites to use it.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/seed.sql | Seeds images bucket as private. |
| supabase/migrations/20260129120000_private_images_bucket.sql | Makes bucket private, migrates URL→path in DB, and replaces storage RLS policies for images. |
| src/stores/organization.ts | Resolves org logos and member avatars via signed URLs when loading org data. |
| src/services/storage.ts | Adds createSignedImageUrl helper (and caching) for private bucket access. |
| src/services/photos.ts | Uploads images to images and persists storage paths; resolves signed URLs for immediate UI use. |
| src/pages/app/index.vue | Resolves apps.icon_url via signed URLs when listing apps. |
| src/modules/auth.ts | Resolves the logged-in user avatar via signed URLs. |
| src/components/dashboard/AppSetting.vue | Stores icon paths in DB and resolves signed URLs for display after upload. |
| if (path.includes('://')) | ||
| return '' |
There was a problem hiding this comment.
createSignedImageUrl returns an empty string whenever the input contains ://. That will blank out valid non-Supabase URLs (e.g., externally hosted icons) and also makes the UI brittle if any legacy full storage URLs slip through. Consider returning the original URL unchanged for absolute URLs, and (optionally) only normalizing/signing when the value refers to the images bucket.
| if (path.includes('://')) | |
| return '' | |
| // If this is an absolute URL (e.g. https://...), return it as-is. | |
| if (path.includes('://')) | |
| return path |
| const SIGNED_URL_TTL_SECONDS = 60 * 60 * 24 * 7 | ||
| const signedUrlCache = new Map<string, string>() | ||
|
|
There was a problem hiding this comment.
signedUrlCache stores signed URLs indefinitely, but createSignedUrl responses expire after SIGNED_URL_TTL_SECONDS. This can lead to stale/broken images after expiry and unbounded memory growth over long sessions. Consider caching with an expiry timestamp (and refreshing when expired) and/or adding a max size / LRU eviction.
| .from(`images/org/${appRef.value?.owner_org.id}/${props.appId}`) | ||
| .upload('icon', blob, { |
There was a problem hiding this comment.
The Storage API .from() expects a bucket name, but this code passes a bucket-like path (images/org/...). That will fail unless a bucket literally named images/org/... exists. Use .from('images') and upload using the full object path (e.g., the iconPath you computed) so the upload path and the value stored in apps.icon_url stay consistent.
| .from(`images/org/${appRef.value?.owner_org.id}/${props.appId}`) | |
| .upload('icon', blob, { | |
| .from('images') | |
| .upload(iconPath, blob, { |
| OR EXISTS ( | ||
| SELECT 1 | ||
| FROM public.orgs o | ||
| JOIN public.org_users ou_self ON ou_self.org_id = o.id | ||
| WHERE ou_self.user_id = auth_user.uid | ||
| AND regexp_replace(o.logo, '^.*/storage/v1/object/(public/|sign/)?[^/]+/', '') = name | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
| OR EXISTS ( | ||
| -- Org logo access for API keys (logo stored as path or public URL) | ||
| SELECT 1 | ||
| FROM public.orgs o | ||
| WHERE regexp_replace(o.logo, '^.*/storage/v1/object/(public/|sign/)?[^/]+/', '') = name | ||
| AND public.check_min_rights( | ||
| 'read'::public.user_min_right, | ||
| public.get_identity_org_allowed('{read,upload,write,all}'::public.key_mode[], o.id), | ||
| o.id, | ||
| NULL::character varying, | ||
| NULL::bigint | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The SELECT policy for storage.objects grants access when regexp_replace(o.logo, '^.*/storage/v1/object/(public/|sign/)?[^/]+/', '') = name, effectively trusting the public.orgs.logo column to decide which objects in the images bucket a caller may read. Because logo is updatable by org admins (public.orgs UPDATE is gated only by check_min_rights('admin', get_identity_org_allowed(...))), an org admin can point their logo at an arbitrary path (e.g., another user’s avatar or a different org’s asset) and then gain read access to that object via this policy, breaking tenant isolation. This policy should instead derive allowed object paths from stable ownership data (e.g., org IDs / user IDs and fixed folder prefixes) rather than from an arbitrary, user-controlled text field like o.logo.
| OR EXISTS ( | |
| SELECT 1 | |
| FROM public.orgs o | |
| JOIN public.org_users ou_self ON ou_self.org_id = o.id | |
| WHERE ou_self.user_id = auth_user.uid | |
| AND regexp_replace(o.logo, '^.*/storage/v1/object/(public/|sign/)?[^/]+/', '') = name | |
| ) | |
| ) | |
| ) | |
| ) | |
| OR EXISTS ( | |
| -- Org logo access for API keys (logo stored as path or public URL) | |
| SELECT 1 | |
| FROM public.orgs o | |
| WHERE regexp_replace(o.logo, '^.*/storage/v1/object/(public/|sign/)?[^/]+/', '') = name | |
| AND public.check_min_rights( | |
| 'read'::public.user_min_right, | |
| public.get_identity_org_allowed('{read,upload,write,all}'::public.key_mode[], o.id), | |
| o.id, | |
| NULL::character varying, | |
| NULL::bigint | |
| ) | |
| ) | |
| ) | |
| ) | |
| ) |
riderx
force-pushed
the
riderx/exif-risk-email
branch
from
407d02e to
f63c87a
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@src/services/photos.ts`:
- Around line 9-10: The imports at the top of src/services/photos.ts use
relative paths; update them to use the project alias by replacing the two
imports (createSignedImageUrl and useSupabase) so they import from '~/storage'
and '~/supabase' respectively (look for the import statements referencing
createSignedImageUrl and useSupabase and swap the path to the '~/...' alias to
satisfy frontend linting and guidelines).
- Around line 16-24: Modify uploadPhotoShared to accept an optional prefix
(e.g., prefix?: string) so org-scoped uploads use an org-based path; inside
uploadPhotoShared build storagePath as `${prefix ??
`${main.user?.id}`}/${fileName}` instead of always using the user id, and update
uploadPhotoOrg (the caller) to pass the org-scoped prefix (e.g.,
`org/${orgId}/${appId}` or `org/${orgId}`) when invoking uploadPhotoShared so
org logos are stored under the org/{org_id}/... path that matches the RLS
policy.
- Around line 30-37: The code currently treats a missing signedUrl as a failure
but leaves the uploaded file in storage; modify the post-upload branch so that
when there is no upload error but createSignedImageUrl(storagePath) returns
falsy (signedUrl is falsy) you call a cleanup routine to remove the uploaded
object (e.g., deleteFile(storagePath) or storage.delete(storagePath)), log any
cleanup error, then invoke callback(false, '', ''); update the block around
signedUrl, createSignedImageUrl, storagePath and callback to perform this
cleanup path and ensure isLoading.value is set false before calling the
callback.
In `@supabase/functions/_backend/public/app/get.ts`:
- Line 7: Reorder the imports in get.ts to match the linter's import grouping:
put external/third-party imports first, then internal/absolute imports, and
place the relative import for createSignedImageUrl (import {
createSignedImageUrl } from '../../utils/storage.ts') in the correct group and
position among the other imports in this file so the linter no longer flags
import order.
In `@supabase/functions/_backend/public/app/post.ts`:
- Line 7: The import order is lint-failing; reorder the imports so they follow
the project's lint rules (external packages first, then internal aliases, then
relative paths). Move the import of createSignedImageUrl and normalizeImagePath
from '../../utils/storage.ts' to the correct position among the other imports so
relative utility imports appear after external/alias imports and before any
local module imports; adjust only the import ordering, not the imported symbols
or file names.
In `@supabase/functions/_backend/public/app/put.ts`:
- Line 7: The import for createSignedImageUrl and normalizeImagePath from
'../../utils/storage.ts' is out of order; move this relative import so it
appears with the other local/relative imports (after external/third-party
imports and before any type-only imports) and order it alphabetically within
that group to satisfy the linter—look for the import line containing
createSignedImageUrl, normalizeImagePath and place it with the other local
imports in the correct group and order.
In `@supabase/functions/_backend/public/organization/get.ts`:
- Around line 110-116: Indentation is off near the end of the handler: align the
closing brace and the return statement so that the return c.json(org) is at the
same indentation level as the if (org.logo) block and the surrounding function
body; adjust whitespace around the lines using createSignedImageUrl, org.logo
assignment, and the final return to match the project's linting style (ensure
the closing `}` for the function and `return c.json(org)` are correctly
indented).
- Line 8: The import order in get.ts is out of lint-compliant sequence; move the
import of createSignedImageUrl from '../../utils/storage.ts' so that it follows
the project's import grouping and alphabetical/structured ordering rules (e.g.,
built-ins, external packages, internal aliases, then relative imports),
adjusting surrounding imports in get.ts to match lint rules while keeping the
createSignedImageUrl symbol and path unchanged.
In `@supabase/functions/_backend/public/organization/members/get.ts`:
- Around line 73-74: The cloudlog call is currently logging member PII via the
signedMembers object; change the logging to avoid emails/image URLs by logging
only non‑PII info (e.g., signedMembers.length or mapping to non‑PII identifiers
like member.id and member.role) and keep returning the full signedMembers in
c.json; update the call to cloudlog to pass the sanitized summary (count or
mapped safe fields) instead of the full signedMembers object.
- Around line 8-9: Reorder the relative imports to satisfy the linter: group
sibling/relative imports together and alphabetize them so the import for
createSignedImageUrl from '../../../utils/storage.ts' appears before the import
for supabaseApikey from '../../../utils/supabase.ts'; update the import
statements in get.ts accordingly so the linter's import order rule passes.
In `@supabase/functions/_backend/public/organization/put.ts`:
- Line 8: The import of createSignedImageUrl and normalizeImagePath should be
reordered to satisfy the project's linting rules; move the "import {
createSignedImageUrl, normalizeImagePath } from '../../utils/storage.ts'" so
that it falls with other internal/project imports (after external/third-party
imports) and order names consistently (alphabetical) within the import
statement; update the single import line in put.ts to follow the project's
import grouping and ordering conventions.
In `@supabase/functions/_backend/utils/storage.ts`:
- Around line 29-48: The createSignedImageUrl function uses supabaseAdmin(c) and
a permissive '://'-based URL passthrough; change it to use the
user-authenticated client supabaseWithAuth(c, c.get('auth')) when calling
.storage.from('images').createSignedUrl to enforce RLS (replace any
supabaseAdmin(c) usage in createSignedImageUrl), and tighten the early-return
check so only URLs that are already signed (e.g., contain
'/storage/v1/object/sign/') are returned unchanged instead of any '://'; keep
normalizeImagePath usage and null guards as-is.
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/storage.ts (1)
15-20: Prefer returning null for non‑images bucket URLs in normalization.When the URL parses but doesn’t match the images bucket pattern, the function returns the full URL, which undermines the “store paths” migration and won’t be signable once the bucket is private. Consider returning
nullso callers can explicitly reject/handle non‑images URLs.♻️ Suggested tweak
const match = url.pathname.match(STORAGE_URL_REGEX) if (match?.[2]) return decodeURIComponent(match[2]) - return trimmed + return null
| import { createSignedImageUrl } from './storage' | ||
| import { useSupabase } from './supabase' |
There was a problem hiding this comment.
Use ~/ alias for src imports.
Frontend guidelines require ~/ alias for src/ imports; this new relative import will likely violate linting. Consider updating both imports in this block for consistency.
🔧 Suggested change
-import { createSignedImageUrl } from './storage'
-import { useSupabase } from './supabase'
+import { createSignedImageUrl } from '~/services/storage'
+import { useSupabase } from '~/services/supabase'📝 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.
| import { createSignedImageUrl } from './storage' | |
| import { useSupabase } from './supabase' | |
| import { createSignedImageUrl } from '~/services/storage' | |
| import { useSupabase } from '~/services/supabase' |
🤖 Prompt for AI Agents
In `@src/services/photos.ts` around lines 9 - 10, The imports at the top of
src/services/photos.ts use relative paths; update them to use the project alias
by replacing the two imports (createSignedImageUrl and useSupabase) so they
import from '~/storage' and '~/supabase' respectively (look for the import
statements referencing createSignedImageUrl and useSupabase and swap the path to
the '~/...' alias to satisfy frontend linting and guidelines).
| async function uploadPhotoShared( | ||
| data: string, | ||
| fileName: string, | ||
| contentType: string, | ||
| isLoading: Ref<boolean>, | ||
| callback: (success: boolean, storagePath: string, signedUrl: string) => Promise<void>, | ||
| ) { | ||
| const storagePath = `${main.user?.id}/${fileName}` | ||
| const { error } = await supabase.storage |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Search for storage policies in SQL files
find . -name "*.sql" | head -20Repository: Cap-go/capgo
Length of output: 1166
🏁 Script executed:
# Search for storage-related RLS policies and bucket configuration
rg -n "storage\.|create policy|images" --type=sql | head -40Repository: Cap-go/capgo
Length of output: 4701
🏁 Script executed:
# Search for uploadPhotoShared usage and org-related storage
rg -n "uploadPhotoShared|storagePath|org.*upload" --type=ts --type=tsx -B2 -A2Repository: Cap-go/capgo
Length of output: 83
🏁 Script executed:
# Read the full storage RLS policy migration
cat -n supabase/migrations/20260129120000_private_images_bucket.sqlRepository: Cap-go/capgo
Length of output: 7413
🏁 Script executed:
# Search for uploadPhotoShared usage
rg -n "uploadPhotoShared" --type=ts -B3 -A3Repository: Cap-go/capgo
Length of output: 1212
🏁 Script executed:
# Read the complete uploadPhotoOrg and uploadPhotoUser functions
cat -n src/services/photos.ts | head -110Repository: Cap-go/capgo
Length of output: 4029
🏁 Script executed:
# Check if there's any path-based logic that switches between user and org paths
rg -n "org|storagePath" src/services/photos.ts -B2 -A2Repository: Cap-go/capgo
Length of output: 3009
Pass org-based path to uploadPhotoShared() for org asset uploads to match RLS policy design.
The new RLS policy distinguishes between org/{org_id}/{app_id}/... paths (org-scoped) and {user_id}/... paths (user-scoped with org-member access). While uploadPhotoOrg() currently works because the RLS policy permits org members to read assets under other org members' user IDs (lines 51-65 of the policy), org logos should use the org/{org_id}/... prefix to align with the intended org-scoped path structure. Modify uploadPhotoShared() to accept an optional prefix parameter, or pass the org ID to it so org uploads use the correct path pattern.
🤖 Prompt for AI Agents
In `@src/services/photos.ts` around lines 16 - 24, Modify uploadPhotoShared to
accept an optional prefix (e.g., prefix?: string) so org-scoped uploads use an
org-based path; inside uploadPhotoShared build storagePath as `${prefix ??
`${main.user?.id}`}/${fileName}` instead of always using the user id, and update
uploadPhotoOrg (the caller) to pass the org-scoped prefix (e.g.,
`org/${orgId}/${appId}` or `org/${orgId}`) when invoking uploadPhotoShared so
org logos are stored under the org/{org_id}/... path that matches the RLS
policy.
| const signedUrl = error ? '' : await createSignedImageUrl(storagePath) | ||
|
|
||
| isLoading.value = false | ||
|
|
||
| if (error || !res.publicUrl) | ||
| await callback(false, '') | ||
| if (error || !signedUrl) | ||
| await callback(false, '', '') | ||
| else | ||
| await callback(true, res.publicUrl) | ||
| await callback(true, storagePath, signedUrl) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/services/photos.ts | head -60Repository: Cap-go/capgo
Length of output: 2315
🏁 Script executed:
sed -n '40-90p' src/services/photos.tsRepository: Cap-go/capgo
Length of output: 107
🏁 Script executed:
cat -n src/services/photos.ts | sed -n '40,90p'Repository: Cap-go/capgo
Length of output: 1930
🏁 Script executed:
cat -n src/services/storage.ts | grep -A 20 "createSignedImageUrl"Repository: Cap-go/capgo
Length of output: 734
🏁 Script executed:
cat -n src/services/storage.ts | sed -n '7,50p'Repository: Cap-go/capgo
Length of output: 1268
🏁 Script executed:
rg "uploadPhotoShared|uploadPhoto" src/services/photos.tsRepository: Cap-go/capgo
Length of output: 828
🏁 Script executed:
rg "storage.*remove|delete.*image" src/services/Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
rg "\.remove\(" src/services/Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
rg "supabase.storage" src/services/photos.tsRepository: Cap-go/capgo
Length of output: 98
🏁 Script executed:
rg "storage.*\(" src/ -A 2 -B 1 | grep -E "(remove|delete)" | head -20Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
rg "from\('images'\)" src/ -A 5 | head -40Repository: Cap-go/capgo
Length of output: 1012
🏁 Script executed:
rg "image_url" src/ -B 2 -A 2 | head -50Repository: Cap-go/capgo
Length of output: 2636
🏁 Script executed:
cat -n src/services/photos.ts | sed -n '55,60p'Repository: Cap-go/capgo
Length of output: 273
🏁 Script executed:
rg "storage\." src/ | grep -E "(remove|delete|clean)"Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
rg "orphan|cleanup|janitor|cron" src/ -i | head -20Repository: Cap-go/capgo
Length of output: 1787
🏁 Script executed:
rg "bucket" src/ | grep -i "rls\|policy\|permission" | head -20Repository: Cap-go/capgo
Length of output: 38
Handle signed-URL failures after successful uploads to avoid orphaned storage files.
If the upload succeeds but createSignedImageUrl fails, the file is written to storage but the callback reports failure, preventing the storage path from being saved to the database. This creates unreferenced (orphaned) files that consume storage and are inaccessible. Consider cleaning up the uploaded file if signing fails.
🧹 Proposed fix (cleanup on signing failure)
- const signedUrl = error ? '' : await createSignedImageUrl(storagePath)
-
- isLoading.value = false
-
- if (error || !signedUrl)
- await callback(false, '', '')
- else
- await callback(true, storagePath, signedUrl)
+ if (error) {
+ isLoading.value = false
+ await callback(false, '', '')
+ return
+ }
+
+ const signedUrl = await createSignedImageUrl(storagePath)
+ if (!signedUrl) {
+ await supabase.storage.from('images').remove([storagePath])
+ isLoading.value = false
+ await callback(false, '', '')
+ return
+ }
+
+ isLoading.value = false
+ await callback(true, storagePath, signedUrl)📝 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.
| const signedUrl = error ? '' : await createSignedImageUrl(storagePath) | |
| isLoading.value = false | |
| if (error || !res.publicUrl) | |
| await callback(false, '') | |
| if (error || !signedUrl) | |
| await callback(false, '', '') | |
| else | |
| await callback(true, res.publicUrl) | |
| await callback(true, storagePath, signedUrl) | |
| if (error) { | |
| isLoading.value = false | |
| await callback(false, '', '') | |
| return | |
| } | |
| const signedUrl = await createSignedImageUrl(storagePath) | |
| if (!signedUrl) { | |
| await supabase.storage.from('images').remove([storagePath]) | |
| isLoading.value = false | |
| await callback(false, '', '') | |
| return | |
| } | |
| isLoading.value = false | |
| await callback(true, storagePath, signedUrl) |
🤖 Prompt for AI Agents
In `@src/services/photos.ts` around lines 30 - 37, The code currently treats a
missing signedUrl as a failure but leaves the uploaded file in storage; modify
the post-upload branch so that when there is no upload error but
createSignedImageUrl(storagePath) returns falsy (signedUrl is falsy) you call a
cleanup routine to remove the uploaded object (e.g., deleteFile(storagePath) or
storage.delete(storagePath)), log any cleanup error, then invoke callback(false,
'', ''); update the block around signedUrl, createSignedImageUrl, storagePath
and callback to perform this cleanup path and ensure isLoading.value is set
false before calling the callback.
| cloudlog({ requestId: c.get('requestId'), message: 'Members', data: signedMembers }) | ||
| return c.json(signedMembers) |
There was a problem hiding this comment.
Avoid logging member PII.
This log includes emails and image URLs for every member, which is a compliance risk. Prefer logging only counts or non‑PII identifiers.
🧹 Safer logging
- cloudlog({ requestId: c.get('requestId'), message: 'Members', data: signedMembers })
+ cloudlog({
+ requestId: c.get('requestId'),
+ message: 'Members fetched',
+ count: signedMembers.length,
+ })📝 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.
| cloudlog({ requestId: c.get('requestId'), message: 'Members', data: signedMembers }) | |
| return c.json(signedMembers) | |
| cloudlog({ | |
| requestId: c.get('requestId'), | |
| message: 'Members fetched', | |
| count: signedMembers.length, | |
| }) | |
| return c.json(signedMembers) |
🤖 Prompt for AI Agents
In `@supabase/functions/_backend/public/organization/members/get.ts` around lines
73 - 74, The cloudlog call is currently logging member PII via the signedMembers
object; change the logging to avoid emails/image URLs by logging only non‑PII
info (e.g., signedMembers.length or mapping to non‑PII identifiers like
member.id and member.role) and keep returning the full signedMembers in c.json;
update the call to cloudlog to pass the sanitized summary (count or mapped safe
fields) instead of the full signedMembers object.
| export async function createSignedImageUrl(c: Context, rawPath?: string | null) { | ||
| if (!rawPath) | ||
| return null | ||
|
|
||
| if (rawPath.includes('://')) | ||
| return rawPath | ||
|
|
||
| const normalized = normalizeImagePath(rawPath) | ||
| if (!normalized) | ||
| return null | ||
|
|
||
| const { data, error } = await supabaseAdmin(c) | ||
| .storage | ||
| .from('images') | ||
| .createSignedUrl(normalized, SIGNED_URL_TTL_SECONDS) | ||
|
|
||
| if (error || !data?.signedUrl) | ||
| return null | ||
|
|
||
| return data.signedUrl |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find where createSignedImageUrl is called to understand if it's user-facing
rg "createSignedImageUrl" --type ts --type js -B 2 -A 2Repository: Cap-go/capgo
Length of output: 11022
🏁 Script executed:
# Check the full context of the storage.ts file
wc -l supabase/functions/_backend/utils/storage.tsRepository: Cap-go/capgo
Length of output: 103
🏁 Script executed:
# Read the entire storage.ts file to understand the function and its context
cat -n supabase/functions/_backend/utils/storage.tsRepository: Cap-go/capgo
Length of output: 1510
🏁 Script executed:
# Search for where rawPath originates - look for callers and their inputs
rg "rawPath" supabase/functions/_backend/utils/storage.ts -B 3 -A 3Repository: Cap-go/capgo
Length of output: 371
🏁 Script executed:
# Check if there are any RLS policies or authorization checks before calling createSignedImageUrl
rg "createSignedImageUrl" -B 10 -A 2 --type ts --type jsRepository: Cap-go/capgo
Length of output: 25201
🏁 Script executed:
# Look for context on how this function is used in API endpoints
find supabase/functions/_backend -name "*.ts" -type f | head -20 | xargs rg -l "createSignedImageUrl"Repository: Cap-go/capgo
Length of output: 40
🏁 Script executed:
# Check if there's a user-authenticated Supabase client function
rg "supabaseApikey|supabaseAdmin" supabase/functions/_backend/utils/ -A 2 -B 2 | head -50Repository: Cap-go/capgo
Length of output: 3316
🏁 Script executed:
# Look for other storage operations to see patterns
rg "\.storage\." supabase/functions/_backend/ --type ts -B 3 -A 3 | head -80Repository: Cap-go/capgo
Length of output: 6679
🏁 Script executed:
# Look at frontend storage.ts to see if it uses a different pattern
cat -n src/services/storage.tsRepository: Cap-go/capgo
Length of output: 1508
🏁 Script executed:
# Check supabase.ts to see available client functions
cat -n supabase/functions/_backend/utils/supabase.ts | head -100Repository: Cap-go/capgo
Length of output: 4188
🏁 Script executed:
# Check how the endpoints use the auth context
rg "c.get\('auth'\)" supabase/functions/_backend/public/ --type ts -B 2 -A 2 | head -40Repository: Cap-go/capgo
Length of output: 3133
🏁 Script executed:
# Check if there's any documentation or comments about why admin is needed for storage
rg "createSignedUrl" supabase/functions/_backend/ --type ts -B 5 -A 5Repository: Cap-go/capgo
Length of output: 781
🏁 Script executed:
# Look at the organization/delete.ts to see if it's also using admin for storage
cat -n supabase/functions/_backend/public/organization/delete.ts | head -60Repository: Cap-go/capgo
Length of output: 2172
Use user-authenticated client instead of admin SDK for signed URL generation on user-facing endpoints.
The function uses supabaseAdmin(c) which violates the coding guideline: "Never use the Supabase admin SDK (with service key) for user-facing APIs; always use the client SDK with user authentication to enforce RLS policies." This bypasses storage RLS entirely. The frontend version (src/services/storage.ts) correctly uses useSupabase() (user-authenticated client) for the same operation. Consider using supabaseWithAuth(c, c.get('auth')) instead, which is already available in the backend utilities.
Additionally, the :// check at line 33–34 returns any full URL unchanged, bypassing the signing logic. If storage buckets are private, these legacy URLs will break. Restrict this to already-signed URLs (e.g., paths containing /storage/v1/object/sign/) to be more explicit about intent.
🤖 Prompt for AI Agents
In `@supabase/functions/_backend/utils/storage.ts` around lines 29 - 48, The
createSignedImageUrl function uses supabaseAdmin(c) and a permissive '://'-based
URL passthrough; change it to use the user-authenticated client
supabaseWithAuth(c, c.get('auth')) when calling
.storage.from('images').createSignedUrl to enforce RLS (replace any
supabaseAdmin(c) usage in createSignedImageUrl), and tighten the early-return
check so only URLs that are already signed (e.g., contain
'/storage/v1/object/sign/') are returned unchanged instead of any '://'; keep
normalizeImagePath usage and null guards as-is.
riderx
force-pushed
the
riderx/exif-risk-email
branch
from
e797583 to
8c0a693
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/services/storage.ts`:
- Line 1: Replace the relative import in storage.ts with the project alias;
change the import of useSupabase (currently "import { useSupabase } from
'./supabase'") to use the '~/` alias that points to src, e.g. import from
'~/services/supabase' so the file uses the mandated alias for frontend
TypeScript/Vue imports.
In `@supabase/functions/_backend/public/app/get.ts`:
- Around line 35-38: The get handler is calling createSignedImageUrl which uses
the service-role supabaseAdmin client and bypasses RLS; refactor so signing uses
a request-scoped Supabase client (apikey/jwt) instead: change
createSignedImageUrl to accept a Supabase client parameter (or add a new helper
createSignedImageUrlWithClient), and update the get handler to pass its
request-scoped client (the same client used in getAll or created in the request
context) instead of supabaseAdmin; ensure supabaseAdmin is not used in
user-facing endpoints and reuse the new helper from both get and getAll.
| @@ -0,0 +1,43 @@ | |||
| import { useSupabase } from './supabase' | |||
There was a problem hiding this comment.
Switch to the ~/ alias for src imports.
The relative import can violate frontend lint rules and the alias requirement.
🔧 Suggested change
-import { useSupabase } from './supabase'
+import { useSupabase } from '~/services/supabase'As per coding guidelines: Use ~/ alias for imports from src/ directory in frontend TypeScript and Vue components.
🤖 Prompt for AI Agents
In `@src/services/storage.ts` at line 1, Replace the relative import in storage.ts
with the project alias; change the import of useSupabase (currently "import {
useSupabase } from './supabase'") to use the '~/` alias that points to src, e.g.
import from '~/services/supabase' so the file uses the mandated alias for
frontend TypeScript/Vue imports.
| if (data.icon_url) { | ||
| const signedIcon = await createSignedImageUrl(c, data.icon_url) | ||
| data.icon_url = signedIcon ?? '' | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the createSignedImageUrl implementation
fd -e ts -e js | xargs grep -l "createSignedImageUrl" | head -5Repository: Cap-go/capgo
Length of output: 195
🏁 Script executed:
# Look at the specific file mentioned in the review
cat -n supabase/functions/_backend/public/app/get.ts | head -50Repository: Cap-go/capgo
Length of output: 2483
🏁 Script executed:
# Search for the createSignedImageUrl function definition
rg "export.*createSignedImageUrl|function createSignedImageUrl" -A 10Repository: Cap-go/capgo
Length of output: 1313
🏁 Script executed:
cat -n supabase/functions/_backend/utils/storage.ts | grep -A 20 "export async function createSignedImageUrl"Repository: Cap-go/capgo
Length of output: 669
🏁 Script executed:
# Also check if supabaseAdmin is used anywhere in storage.ts
rg "supabaseAdmin" supabase/functions/_backend/utils/storage.tsRepository: Cap-go/capgo
Length of output: 150
Avoid the admin SDK when signing URLs in user-facing endpoints.
createSignedImageUrl uses the service-role client (supabaseAdmin), which bypasses RLS. For user-facing endpoints, sign with a request-scoped client (apikey/jwt) to preserve access controls. Consider refactoring the helper to accept a Supabase client and reuse it for both get and getAll.
🔧 Possible direction
-import { createSignedImageUrl } from '../../utils/storage.ts'
+import { createSignedImageUrlWithClient } from '../../utils/storage.ts'
@@
- const signedIcon = await createSignedImageUrl(c, data.icon_url)
+ const signedIcon = await createSignedImageUrlWithClient(
+ supabaseApikey(c, apikey.key),
+ data.icon_url,
+ )🤖 Prompt for AI Agents
In `@supabase/functions/_backend/public/app/get.ts` around lines 35 - 38, The get
handler is calling createSignedImageUrl which uses the service-role
supabaseAdmin client and bypasses RLS; refactor so signing uses a request-scoped
Supabase client (apikey/jwt) instead: change createSignedImageUrl to accept a
Supabase client parameter (or add a new helper createSignedImageUrlWithClient),
and update the get handler to pass its request-scoped client (the same client
used in getAll or created in the request context) instead of supabaseAdmin;
ensure supabaseAdmin is not used in user-facing endpoints and reuse the new
helper from both get and getAll.
Summary (AI generated)
Test plan (AI generated)
imagesbucket is private.icon_url,logo, andimage_urlare signed URLs.Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.