fix: prevent crash in AuthWebViewScreen when params are missing or invalid - #2433
fix: prevent crash in AuthWebViewScreen when params are missing or invalid#2433synakr wants to merge 2 commits into
Conversation
Signed-off-by: Chandra Keshav Mishra <chandrakeshavmishra@gmail.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesAuth Safety & Parameter Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 44 minutes and 32 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens AuthWebViewScreen against missing or malformed navigation params to prevent runtime crashes during hostname/URL parsing and redirect handling.
Changes:
- Made
route.paramsdestructuring null-safe. - Guarded
new URL(authorizationURL)parsing with a fallback hostname. - Expanded required-param validation and added a
redirectUripresence check beforestartsWith.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (err) { | ||
| console.warn('Invalid authorizationURL'); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
screens/AuthWebViewScreen.tsx (1)
105-125:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten the redirect match before consuming
code.
startsWithis too loose for an auth callback check and can accept look-alike URLs that share the same prefix. Compare the parsed callback components, or normalize to an exact redirect URI match, before sending the auth code onward.Suggested fix
- if (redirectUri && url.startsWith(redirectUri)) { + if (redirectUri) { try { const uri = new URL(url); + const expected = new URL(redirectUri); + if ( + uri.protocol !== expected.protocol || + uri.host !== expected.host || + uri.pathname !== expected.pathname + ) { + return true; + } const code = uri.searchParams.get('code'); if (!code) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@screens/AuthWebViewScreen.tsx` around lines 105 - 125, The current check uses url.startsWith(redirectUri) which is too permissive; instead parse both the incoming url and the configured redirectUri (new URL(url) and new URL(redirectUri)) and compare their normalized components (protocol, host/hostname and port, and pathname, handling optional trailing slash consistency) to ensure an exact callback match before extracting the authorization code; only after those components match should you call VciClient.getInstance().sendAuthCode(code) and perform controller.CANCEL() / navigation.goBack(), otherwise ignore the URL and continue loading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@screens/AuthWebViewScreen.tsx`:
- Around line 105-125: The current check uses url.startsWith(redirectUri) which
is too permissive; instead parse both the incoming url and the configured
redirectUri (new URL(url) and new URL(redirectUri)) and compare their normalized
components (protocol, host/hostname and port, and pathname, handling optional
trailing slash consistency) to ensure an exact callback match before extracting
the authorization code; only after those components match should you call
VciClient.getInstance().sendAuthCode(code) and perform controller.CANCEL() /
navigation.goBack(), otherwise ignore the URL and continue loading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fab6ff86-5c08-49e6-b245-c5e43abc1eca
📒 Files selected for processing (1)
screens/AuthWebViewScreen.tsx
…valid Signed-off-by: Md Sayan Akram <mdsayanakram@gmail.com>
eb7c001 to
9a96779
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!authorizationURL || !clientId || !redirectUri || !controller) { | ||
| console.error('Missing required parameters for authentication'); | ||
| navigation.goBack(); | ||
| return; |
| } catch { | ||
| console.warn('Invalid authorizationURL'); | ||
| } |
|
@synakr please check coderabbit comment and raise PR for develop |
Fix: Prevent crash when route params are undefined in AuthWebViewScreen
Issue
route.paramsis destructured and used before validation. If it is undefined or contains invalid values, it can cause runtime crashes (e.g., during URL parsing withnew URL()).Changes
Made params destructuring null-safe:
Guarded URL parsing to prevent runtime errors:
Extended validation to include all required parameters:
Added safety check before redirect handling:
Impact
Prevents crashes when navigation parameters are missing or malformed and ensures safe URL handling, without affecting existing behavior for valid inputs.
Summary by CodeRabbit
Release Notes