Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7011f4b250
ℹ️ 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".
| // somewhere off-site based on a value we round-tripped through storage. | ||
| export function isRelativePath(path: string): boolean { |
There was a problem hiding this comment.
Validate return targets by resolved origin
A crafted return_to such as /\attacker.example passes this check because it starts with one forward slash, but browsers resolve that path against an HTTPS origin as https://attacker.example/. Since /auth/login accepts arbitrary query parameters and the callback later redirects to the stored value, an attacker can turn the completed sign-in flow into an open redirect; resolve the value against the Relisten origin and require the resulting origin to match, or reject backslashes.
Useful? React with 👍 / 👎.
| const response = await ky | ||
| .post(`${ACCOUNTS_AUTH_URL}/connect/token`, { | ||
| body: new URLSearchParams({ | ||
| grant_type: 'refresh_token', | ||
| refresh_token: tokens.refreshToken, |
There was a problem hiding this comment.
Serialize refresh-token rotation
When two requests arrive within the refresh window—such as concurrent tabs or parallel navigations—both read the same cookie and submit the same rotating refresh token here. One rotation succeeds while the other is treated as reuse; the failure path then clears the cookie and may cause the backend to revoke the newly issued token family, producing intermittent forced sign-outs. Coalesce or otherwise serialize refreshes per session instead of independently rotating in every middleware request.
Useful? React with 👍 / 👎.
| RELISTEN | ||
| </Link> | ||
| <SecondaryNavBar artistName={artistName} /> | ||
| {ACCOUNTS_FEATURE_ENABLED && <AccountMenu session={session} />} |
There was a problem hiding this comment.
Add account actions to mobile navigation
On viewports below the lg breakpoint, this account menu is inside the preceding max-lg:hidden container, while the mobile branch only renders the Re link and the existing popover Menu contains no account entry. Consequently, once accounts are enabled, mobile users have no visible way to sign in or sign out.
Useful? React with 👍 / 👎.
| const loginHref = (provider: LoginProvider) => | ||
| `/auth/login?provider=${provider}&return_to=${encodeURIComponent(pathname)}`; |
There was a problem hiding this comment.
Preserve the query string across login
usePathname() excludes the current query string, so signing in from URLs that use repository query state—such as playback source/t, browse slug, or Today month/day—returns the user to the path with all of that state discarded. Include the current search parameters in the encoded return_to value.
Useful? React with 👍 / 👎.
No description provided.