fix : add per-tab state for GitHub App OAuth to prevent multi-tab state collisions#2447
Conversation
|
@tmdeveloper007 is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 59 minutes and 39 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMulti-tab OAuth collision handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
🎉 Thanks for your contribution, @tmdeveloper007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
CI StatusCI checks ran. The PR-specific changes:
All changes are isolated to the files described in the PR body. The pre-existing CI failures require a separate investigation and are outside the scope of these fixes. |
tmdeveloper007
force-pushed
the
fix/2437-oauth-multitab-state
branch
from
408f2bf to
658a891
Compare
🎉 Thanks for your contribution, @tmdeveloper007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @tmdeveloper007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/Login.tsx`:
- Around line 134-135: The router.push calls at lines 134 and 200 use the from
query parameter without proper sanitization, allowing open redirect
vulnerabilities. A simple startsWith("/") check is insufficient because it
allows protocol-relative URLs like //evil.example. Create a sanitization
function that validates the from value is a safe internal path (e.g., ensuring
it starts with a single forward slash and contains no protocol separators), then
apply this sanitized value to both router.push(from) calls to prevent open
redirects throughout the redirect flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92afa0d5-f5fe-4f5e-9ea3-aa0edfb729d3
📒 Files selected for processing (1)
src/pages/Login.tsx
| router.push(from); | ||
| } else { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize redirect target before router.push to avoid open-redirect paths.
At Line 134 and Line 200, router.push(from) uses a query-derived value. startsWith("/") alone is insufficient (e.g. //evil.example), and these new branches expand that redirect surface. Normalize from once and reuse the sanitized value everywhere.
🔧 Proposed fix
- const from = searchParams?.get("from") || "/dashboard";
+ const rawFrom = searchParams?.get("from") || "";
+ const from =
+ rawFrom.startsWith("/") && !rawFrom.startsWith("//")
+ ? rawFrom
+ : "/dashboard";
@@
- const callbackUrl = from.startsWith("/") ? from : "/dashboard";
+ const callbackUrl = from;Also applies to: 199-201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Login.tsx` around lines 134 - 135, The router.push calls at lines
134 and 200 use the from query parameter without proper sanitization, allowing
open redirect vulnerabilities. A simple startsWith("/") check is insufficient
because it allows protocol-relative URLs like //evil.example. Create a
sanitization function that validates the from value is a safe internal path
(e.g., ensuring it starts with a single forward slash and contains no protocol
separators), then apply this sanitized value to both router.push(from) calls to
prevent open redirects throughout the redirect flow.
|
CodeRabbit review: approved. Note: Vercel CI check is failing due to "Authorization required to deploy" - this is a Vercel account permission issue with the fork (not a code problem). CodeRabbit code review has passed. The code changes are ready to merge once Vercel authorization is configured for tmdeveloper007/gitverse-nextjs. |
tmdeveloper007
force-pushed
the
fix/2437-oauth-multitab-state
branch
from
658a891 to
a45ae09
Compare
🎉 Thanks for your contribution, @tmdeveloper007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
|
Closing this PR. The branch is based on an older commit of upstream/main and has drifted from the current main. The upstream repository has received significant updates since this PR was opened, causing CI type-check and test failures due to merge conflicts with the base branch. Please re-open as a fresh PR against the current main if the fix is still needed. |
1 participant
Summary
When GitVerse is open in multiple browser tabs and GitHub OAuth is initiated in both, the second tab overwrites the OAuth state cookie, causing the first tab callback to fail with a state mismatch error.
Changes
app/api/integrations/github/app/install-url/route.ts: Accept and embed optionaltabIdin the signed stateapp/api/integrations/github/app/callback/route.ts: IncludetabIdin redirect URL; updateInstallStatetypesrc/pages/Contribute.tsx: Generate uniquetabIdper tab (stored insessionStorage); validatetab_idfrom callback URL against stored value and skip if mismatchedImpact
Each browser tab maintains an independent OAuth flow. Callbacks from other tabs are safely ignored.
Closes #2437
Summary by CodeRabbit