Fix silent logo upload failure for signed-out users#394
Open
leerob wants to merge 2 commits into
Open
Conversation
Uploads to the avatars bucket require an authenticated session (the bucket's RLS policy only allows the `authenticated` role), but the Add Company flow was reachable while signed out, so the upload was sent as `anon` and rejected with "new row violates row-level security policy". The error was swallowed and the local preview still rendered, making it look like the upload succeeded. - UploadLogo: verify the session before uploading, surface upload and validation errors via toast, revert the optimistic preview on failure, and show an uploading state. - AddCompanyButton: redirect signed-out visitors to /login instead of opening a form whose upload and save can never succeed. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Session loading redirects signed-in users
- Redirect to login only when isAuthenticated is explicitly false, and no-op while session is still loading (null).
- ✅ Fixed: Late FileReader restores failed preview
- FileReader onload updates preview only while allowReaderPreview is true, cleared on auth failure, upload error, or success.
Or push these changes by commenting:
@cursor push d1dde262e7
Preview (d1dde262e7)
diff --git a/apps/cursor/src/components/company/add-company-button.tsx b/apps/cursor/src/components/company/add-company-button.tsx
--- a/apps/cursor/src/components/company/add-company-button.tsx
+++ b/apps/cursor/src/components/company/add-company-button.tsx
@@ -31,11 +31,15 @@
// Adding a company requires an authenticated session: the save action is
// auth-guarded and the logo upload hits an auth-only storage policy. Send
// signed-out visitors to sign in instead of into a form that can't succeed.
- if (!isAuthenticated) {
+ if (isAuthenticated === false) {
router.push(`/login?next=${pathname}`);
return;
}
+ if (isAuthenticated === null) {
+ return;
+ }
+
setAddCompany({ addCompany: true, redirect });
};
diff --git a/apps/cursor/src/components/upload-logo.tsx b/apps/cursor/src/components/upload-logo.tsx
--- a/apps/cursor/src/components/upload-logo.tsx
+++ b/apps/cursor/src/components/upload-logo.tsx
@@ -40,8 +40,10 @@
// Show an optimistic preview while the upload is in flight. It is reverted
// below if the upload fails so the UI never shows an image that wasn't
// actually saved.
+ let allowReaderPreview = true;
const reader = new FileReader();
reader.onload = (e) => {
+ if (!allowReaderPreview) return;
setPreview(e.target?.result as string);
};
reader.readAsDataURL(file);
@@ -59,6 +61,7 @@
if (!user) {
toast.error("Please sign in to upload an image.");
+ allowReaderPreview = false;
setPreview(previousPreview);
return;
}
@@ -82,6 +85,7 @@
data: { publicUrl },
} = supabase.storage.from("avatars").getPublicUrl(path);
+ allowReaderPreview = false;
setPreview(publicUrl);
onUpload?.(publicUrl);
} catch (error) {
@@ -89,6 +93,7 @@
toast.error(
error instanceof Error ? error.message : "Failed to upload image.",
);
+ allowReaderPreview = false;
setPreview(previousPreview);
} finally {
setIsUploading(false);You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 6f9bbb8. Configure here.
Collaborator
Author
- Only redirect to login when session is known to be absent - Ignore FileReader onload after upload auth/storage failure or success Applied via @cursor push command
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
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.


Summary
The Add Company logo upload was silently failing with a confusing
StorageApiError: new row violates row-level security policy(HTTP 400).Root cause: The
avatarsstorage bucket's only INSERT policy allows theauthenticatedrole (WITH CHECK (bucket_id = 'avatars')). There is no path/folder restriction, so theprefixwas never the issue. The Add Company flow was reachable while signed out, so the upload request was sent as theanonrole and rejected by RLS.UploadLogothen swallowed the error (console.erroronly) while still rendering the localFileReaderpreview, so it looked like the upload had worked.Changes
UploadLogo: verifies the session viasupabase.auth.getUser()before uploading; surfaces upload + validation errors viasonnertoasts; reverts the optimistic preview on failure so the UI never shows an unsaved image; adds an "Uploading…" state.AddCompanyButton: now auth-aware — signed-out visitors are redirected to/login?next=…instead of into a form whose upload and save (theupsertCompanyActionis already auth-guarded) can never succeed. Follows the existing client-side auth pattern (mcps-edit-button.tsx).Test plan
/companiesredirects to/login.Notes / follow-ups
middleware.tsin the repo, which@supabase/ssrnormally uses to refresh sessions; expired sessions can leave the browser client unauthenticated. Worth adding separately.Missing Description or aria-describedby for {DialogContent}a11y warning on the dialog — not addressed here.Made with Cursor
Note
Low Risk
Client-only UX and auth checks around company creation and storage uploads; no server policy or data model changes in this diff.
Overview
Fixes Add company and logo upload behavior when the session is missing or still loading.
AddCompanyButtonnow treats auth as three states: signed-out users go to/login?next=…only whenisAuthenticated === false, and clicks are ignored while auth is stillnull(avoids opening the flow beforegetSessionfinishes).UploadLogogates the optimisticFileReaderpreview withallowReaderPreviewso lateonloadcallbacks cannot show a local image after sign-in failure, upload errors, or once the real public URL is set—preview stays aligned with what was actually stored.Reviewed by Cursor Bugbot for commit 85468b7. Bugbot is set up for automated code reviews on this repo. Configure here.