Skip to content

Commit 805eabb

Browse files
committed
fix: resolve all 18 CodeRabbitAI review findings on PR #199
Security: - Restrict deploy-supabase-functions workflow to base repository only (#1) - Fail-closed on missing IP metadata in before-signup hook (#11) - Switch waitlist email from localStorage to sessionStorage (#9) - Remove last_active_at from direct client UPDATE grant; use security definer RPC (#15) Functional correctness: - Clear profileRow on implicit sign-out in AuthProvider (#3) - Prevent email useEffect from clobbering user input on ProComingSoonPage (#5) - Add autoFocus to delete modal for immediate Escape dismissal (#6) - Fix Link rendering outside Router context in ProWaitlistBanner (#2) - Move skip-to-content link above ProWaitlistBanner for a11y (#18) Stability & hardening: - Add timeout to supabase.functions.invoke in accessService (#8) - Add AbortSignal.timeout to Telegram fetch call (#10) - Reorder delete-account to deleteUser before cleanup for atomicity (#12) - Add error handling for all Supabase calls in post-signup (#13) - Add idempotency guard (welcomed_at) to waitlist-welcome (#14) Code quality: - Rename proSort0-5 to kebab-case pro-sort-0-5 with stylelint suppression (#4) - Restore expect import in cspHeaders.test (#7) - Revert destructive column drop in migration (#17) - Add new migration for welcomed_at column (#14) Tests: - Add test for implicit sign-out profile clearing (AuthProvider) - Add test for email input not clobbered by async auth (ProComingSoonPage) - Add test for Escape dismissing delete modal (ProfileSettingsPage) - Add test for anchor rendering outside Router (ProWaitlistBanner)
1 parent 5466833 commit 805eabb

23 files changed

Lines changed: 257 additions & 83 deletions

.github/workflows/deploy-supabase-functions.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ jobs:
1616
runs-on: ubuntu-latest
1717
if: >-
1818
github.event.workflow_run.conclusion == 'success' &&
19+
github.event.workflow_run.head_repository.full_name == github.repository &&
1920
(github.event.workflow_run.head_branch == 'main' ||
2021
github.event.workflow_run.head_branch == 'develop')
2122
permissions:

src/components/ProWaitlistBanner.jsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { isRTL } from '@/utils/rtlManager';
1919
* @param {'landing' | 'app'} source
2020
* @param {string} pathname
2121
*/
22-
function ProWaitlistBannerContent({ source, pathname }) {
22+
function ProWaitlistBannerContent({ source, pathname, inRouter }) {
2323
const { t, i18n } = useTranslation();
2424
const [dismissed, setDismissed] = useState(true);
2525

@@ -46,6 +46,8 @@ function ProWaitlistBannerContent({ source, pathname }) {
4646
? WAITLIST_SOURCES.APP
4747
: WAITLIST_SOURCES.LANDING;
4848
const CtaIcon = isRTL(i18n.language) ? ArrowLeft : ArrowRight;
49+
const LinkComponent = inRouter ? Link : 'a';
50+
const hrefProp = inRouter ? 'to' : 'href';
4951

5052
const handleDismiss = () => {
5153
try {
@@ -68,13 +70,13 @@ function ProWaitlistBannerContent({ source, pathname }) {
6870
<p className="min-w-0 text-sm font-medium leading-snug text-(--color-pro-banner-text) sm:text-base">
6971
{t('pro.banner.message')}
7072
</p>
71-
<Link
72-
to={`/pro?source=${waitlistSource}`}
73+
<LinkComponent
74+
{...{ [hrefProp]: `/pro?source=${waitlistSource}` }}
7375
className="inline-flex min-h-touch shrink-0 items-center justify-center gap-1.5 rounded-lg bg-(--color-pro-banner-cta-bg) px-3.5 text-sm font-semibold text-(--color-pro-banner-cta-text) shadow-sm transition-colors hover:bg-(--color-pro-banner-cta-hover) hover:text-(--color-pro-banner-cta-hover-text) focus:outline-none focus-visible:ring-2 focus-visible:ring-white/50"
7476
>
7577
{t('pro.banner.cta')}
7678
<CtaIcon size={15} weight="bold" aria-hidden />
77-
</Link>
79+
</LinkComponent>
7880
</div>
7981
<button
8082
type="button"
@@ -91,7 +93,9 @@ function ProWaitlistBannerContent({ source, pathname }) {
9193

9294
function ProWaitlistBannerRouted({ source }) {
9395
const { pathname } = useLocation();
94-
return <ProWaitlistBannerContent source={source} pathname={pathname} />;
96+
return (
97+
<ProWaitlistBannerContent source={source} pathname={pathname} inRouter />
98+
);
9599
}
96100

97101
/**
@@ -100,7 +104,9 @@ function ProWaitlistBannerRouted({ source }) {
100104
function ProWaitlistBanner({ source }) {
101105
const inRouter = useInRouterContext();
102106
if (!inRouter) {
103-
return <ProWaitlistBannerContent source={source} pathname="" />;
107+
return (
108+
<ProWaitlistBannerContent source={source} pathname="" inRouter={false} />
109+
);
104110
}
105111
return <ProWaitlistBannerRouted source={source} />;
106112
}

src/components/ProWaitlistBanner.test.jsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,27 @@ describe('ProWaitlistBanner', () => {
7070
).not.toBeInTheDocument();
7171
});
7272

73-
it('hides when user is already enrolled (email in localStorage)', () => {
74-
localStorage.setItem(WAITLIST_EMAIL_STORAGE_KEY, 'user@example.com');
73+
it('hides when user is already enrolled (email in sessionStorage)', () => {
74+
sessionStorage.setItem(WAITLIST_EMAIL_STORAGE_KEY, 'user@example.com');
7575
renderBanner('landing');
7676
expect(
7777
screen.queryByRole('link', { name: /join waitlist/i })
7878
).not.toBeInTheDocument();
7979
});
8080

81+
it('renders anchor tag when outside Router context', () => {
82+
const { container } = renderWithI18n(
83+
<ProWaitlistBanner source="landing" />
84+
);
85+
const anchor = container.querySelector('a[href="/pro?source=landing"]');
86+
expect(anchor).toBeInTheDocument();
87+
expect(anchor.tagName).toBe('A');
88+
});
89+
8190
it('dispatches banner-dismissed event when hidden due to enrollment', () => {
8291
const spy = vi.fn();
8392
window.addEventListener('bayan-flow:banner-dismissed', spy);
84-
localStorage.setItem(WAITLIST_EMAIL_STORAGE_KEY, 'user@example.com');
93+
sessionStorage.setItem(WAITLIST_EMAIL_STORAGE_KEY, 'user@example.com');
8594
renderBanner('landing');
8695
expect(spy).toHaveBeenCalledTimes(1);
8796
window.removeEventListener('bayan-flow:banner-dismissed', spy);

src/contexts/AuthProvider.jsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export function AuthProvider({ children }) {
9292
const checkId = ++accessCheckRef.current;
9393

9494
if (!activeUser) {
95+
setProfileRow(null);
9596
setAccessBlock(null);
9697
return;
9798
}

src/contexts/AuthProvider.test.jsx

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,57 @@ describe('AuthProvider', () => {
460460
unmount();
461461
});
462462

463+
it('clears profileRow on implicit sign-out via SIGNED_OUT event', async () => {
464+
supabaseAuthMock.getSession.mockResolvedValueOnce({
465+
data: {
466+
session: {
467+
user: {
468+
id: 'user-1',
469+
email: 'user@example.com',
470+
user_metadata: { full_name: 'Test User' },
471+
},
472+
},
473+
},
474+
error: null,
475+
});
476+
477+
supabaseFromMock.mockReturnValue({
478+
select: vi.fn().mockReturnThis(),
479+
eq: vi.fn().mockReturnThis(),
480+
maybeSingle: vi.fn(async () => ({
481+
data: {
482+
display_name: 'Test User',
483+
avatar_url: null,
484+
avatar_preference: 'google',
485+
plan: 'free',
486+
email: 'user@example.com',
487+
},
488+
error: null,
489+
})),
490+
});
491+
492+
render(
493+
<AuthProvider>
494+
<AuthProbe />
495+
</AuthProvider>
496+
);
497+
498+
await waitFor(() => {
499+
expect(screen.getByTestId('authenticated')).toHaveTextContent('true');
500+
});
501+
expect(screen.getByTestId('display-name')).toHaveTextContent('Test User');
502+
503+
act(() => {
504+
authStateChangeCallbackRef.current?.('SIGNED_OUT', null);
505+
});
506+
507+
await waitFor(() => {
508+
expect(screen.getByTestId('authenticated')).toHaveTextContent('false');
509+
});
510+
expect(screen.getByTestId('display-name')).toHaveTextContent('');
511+
expect(screen.getByTestId('access-block')).toHaveTextContent('');
512+
});
513+
463514
it('sets accessBlock when profile is banned', async () => {
464515
supabaseAuthMock.getSession.mockResolvedValueOnce({
465516
data: {

src/index.css

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,8 @@ body {
575575
*/
576576

577577
/* Tile 0: compare at 10%, sort at 15% */
578-
@keyframes proSort0 {
578+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
579+
@keyframes pro-sort-0 {
579580
0%,
580581
10% {
581582
background-color: var(--color-primary);
@@ -603,7 +604,8 @@ body {
603604
}
604605

605606
/* Tile 1: compare at 10%, swap right 180px, sort at 35% */
606-
@keyframes proSort1 {
607+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
608+
@keyframes pro-sort-1 {
607609
0%,
608610
10% {
609611
background-color: var(--color-primary);
@@ -637,7 +639,8 @@ body {
637639
}
638640

639641
/* Tile 2: compare at 40%, sort at 45% */
640-
@keyframes proSort2 {
642+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
643+
@keyframes pro-sort-2 {
641644
0%,
642645
40% {
643646
background-color: var(--color-primary);
@@ -665,7 +668,8 @@ body {
665668
}
666669

667670
/* Tile 3: compare at 50%, sort at 55% */
668-
@keyframes proSort3 {
671+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
672+
@keyframes pro-sort-3 {
669673
0%,
670674
50% {
671675
background-color: var(--color-primary);
@@ -693,7 +697,8 @@ body {
693697
}
694698

695699
/* Tile 4: compare at 10%, swap left 180px, sort at 35% */
696-
@keyframes proSort4 {
700+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
701+
@keyframes pro-sort-4 {
697702
0%,
698703
10% {
699704
background-color: var(--color-primary);
@@ -727,7 +732,8 @@ body {
727732
}
728733

729734
/* Tile 5: compare at 60%, sort at 65% */
730-
@keyframes proSort5 {
735+
/* stylelint-disable-next-line keyframe-block-no-duplicate-selectors -- intentional hold-then-jump frames */
736+
@keyframes pro-sort-5 {
731737
0%,
732738
60% {
733739
background-color: var(--color-primary);
@@ -759,22 +765,22 @@ body {
759765
}
760766

761767
.pro-sorting-tile-1 {
762-
animation: proSort0 6s ease-in-out infinite;
768+
animation: pro-sort-0 6s ease-in-out infinite;
763769
}
764770
.pro-sorting-tile-2 {
765-
animation: proSort1 6s ease-in-out infinite;
771+
animation: pro-sort-1 6s ease-in-out infinite;
766772
}
767773
.pro-sorting-tile-3 {
768-
animation: proSort2 6s ease-in-out infinite;
774+
animation: pro-sort-2 6s ease-in-out infinite;
769775
}
770776
.pro-sorting-tile-4 {
771-
animation: proSort3 6s ease-in-out infinite;
777+
animation: pro-sort-3 6s ease-in-out infinite;
772778
}
773779
.pro-sorting-tile-5 {
774-
animation: proSort4 6s ease-in-out infinite;
780+
animation: pro-sort-4 6s ease-in-out infinite;
775781
}
776782
.pro-sorting-tile-6 {
777-
animation: proSort5 6s ease-in-out infinite;
783+
animation: pro-sort-5 6s ease-in-out infinite;
778784
}
779785

780786
@media (prefers-reduced-motion: reduce) {

src/pages/ProComingSoonPage.jsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* See LICENSE for details.
55
*/
66

7-
import { useEffect, useState } from 'react';
7+
import { useEffect, useRef, useState } from 'react';
88
import { useSearchParams } from 'react-router-dom';
99
import {
1010
Columns,
@@ -67,13 +67,16 @@ function ProComingSoonPage() {
6767
})();
6868

6969
const [email, setEmail] = useState(defaultEmail);
70+
const emailEditedRef = useRef(false);
7071
const [submitState, setSubmitState] = useState('idle');
7172
const [position, setPosition] = useState(null);
7273
const [errorKey, setErrorKey] = useState(null);
7374
const [waitlistCount, setWaitlistCount] = useState(0);
7475

7576
useEffect(() => {
76-
setEmail(defaultEmail);
77+
if (!emailEditedRef.current) {
78+
setEmail(defaultEmail);
79+
}
7780
}, [defaultEmail]);
7881

7982
useEffect(() => {
@@ -260,7 +263,10 @@ function ProComingSoonPage() {
260263
autoComplete="email"
261264
required
262265
value={email}
263-
onChange={event => setEmail(event.target.value)}
266+
onChange={event => {
267+
emailEditedRef.current = true;
268+
setEmail(event.target.value);
269+
}}
264270
placeholder={t('pro.form.emailPlaceholder')}
265271
aria-describedby={
266272
errorKey ? 'pro-waitlist-email-error' : undefined

src/pages/ProComingSoonPage.test.jsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,26 @@ describe('ProComingSoonPage', () => {
116116
});
117117
});
118118

119+
it('does not clobber email after user starts typing', async () => {
120+
const { useAuth } = await import('@/hooks/useAuth.js');
121+
useAuth.mockReturnValue({
122+
user: { id: 'u1', email: 'initial@example.com' },
123+
profile: null,
124+
});
125+
126+
renderPage('/pro');
127+
128+
const input = screen.getByLabelText(/email address/i);
129+
expect(input).toHaveValue('initial@example.com');
130+
131+
fireEvent.change(input, { target: { value: 'typed@example.com' } });
132+
expect(input).toHaveValue('typed@example.com');
133+
134+
await waitFor(() => {
135+
expect(input).toHaveValue('typed@example.com');
136+
});
137+
});
138+
119139
it('hides waitlist count when below threshold', async () => {
120140
vi.mocked(getWaitlistPublicCount).mockResolvedValue(30);
121141

src/pages/ProfileSettingsPage.jsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,7 @@ function ProfileSettingsPage() {
501501
damping: 25,
502502
}}
503503
onClick={e => e.stopPropagation()}
504+
autoFocus
504505
>
505506
<h2 className="text-xl font-bold text-text-primary mb-3">
506507
{t('profile.deleteAccount')}

src/pages/ProfileSettingsPage.test.jsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,33 @@ describe('ProfileSettingsPage', () => {
191191
});
192192
});
193193

194+
it('Escape key dismisses delete modal', async () => {
195+
getProfileMock.mockResolvedValue({
196+
display_name: 'Ada',
197+
avatar_url: null,
198+
avatar_preference: 'google',
199+
plan: 'free',
200+
email: 'ada@example.com',
201+
});
202+
203+
renderPage();
204+
205+
await waitFor(() => {
206+
expect(screen.getByLabelText(/display name/i)).toBeInTheDocument();
207+
});
208+
209+
fireEvent.click(screen.getByRole('button', { name: /delete account/i }));
210+
211+
const dialog = await screen.findByRole('dialog');
212+
expect(dialog).toBeInTheDocument();
213+
214+
fireEvent.keyDown(dialog, { key: 'Escape' });
215+
216+
await waitFor(() => {
217+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
218+
});
219+
});
220+
194221
it('deletes account after typing DELETE confirmation via modal', async () => {
195222
getProfileMock.mockResolvedValue({
196223
display_name: 'Ada',

0 commit comments

Comments
 (0)