Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughinviteUser now accepts an origin, validates it against trusted team domains (including localhost and pattern matching), constructs callbackUrl from the validated origin and Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant DashboardActions as Dashboard Actions
participant InviteAPI as Team Invite Service
note right of DashboardActions `#DFF2E1`: validate origin -> build callbackUrl
Client->>DashboardActions: inviteUser(teamId, email, origin)
DashboardActions->>DashboardActions: assertTrustedOrigin(teamId, origin)\n(hostname pattern / localhost checks)
alt origin trusted
DashboardActions->>InviteAPI: POST /invite { email, callbackUrl }
InviteAPI-->>DashboardActions: 200 OK
DashboardActions-->>Client: success
else origin untrusted
DashboardActions-->>Client: throw validation error (REDIRECT_URL_NOT_WHITELISTED)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Changed inviteUser function to accept origin parameter instead of full callbackUrl, constructing the callback URL internally using new URL(stackServerApp.urls.teamInvitation, origin). This centralizes URL construction logic.
Key changes:
- Parameter change from
callbackUrl: stringtoorigin: stringon line 32 - Added URL construction:
new URL(stackServerApp.urls.teamInvitation, origin).toString()on line 33 - Caller in
page-client.tsx:200now passeswindow.location.origininstead of full URL
Issues found:
- Missing origin validation creates security vulnerability - the
originparameter is directly used to construct callback URLs without validation against trusted domains, which could allow attackers to redirect invitation flows to malicious sites
Confidence Score: 2/5
- This PR has a critical security issue - missing origin validation could allow redirect attacks
- Score reflects the security vulnerability introduced by accepting untrusted origin without validation. While the refactoring improves code organization, the lack of origin validation against trusted domains violates the custom security rules and creates a potential attack vector for phishing or redirect attacks in the team invitation flow
- Pay close attention to
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts- requires origin validation
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts | 2/5 | Changed parameter from callbackUrl to origin for constructing invitation URLs, but missing origin validation which creates security risk |
Sequence Diagram
sequenceDiagram
participant User as User (Browser)
participant PageClient as page-client.tsx
participant Actions as actions.ts
participant StackServer as stackServerApp
participant Team as Team Service
participant Email as Email Service
User->>PageClient: Click "Invite" button
PageClient->>PageClient: Get window.location.origin
PageClient->>Actions: inviteUser(teamId, email, origin)
Actions->>Actions: Construct callbackUrl from origin
Note over Actions: callbackUrl = new URL(stackServerApp.urls.teamInvitation, origin)
Actions->>StackServer: getUser()
StackServer-->>Actions: Return user
Actions->>StackServer: user.getTeam(teamId)
StackServer-->>Actions: Return team
Actions->>Team: team.inviteUser({ email, callbackUrl })
Team->>Email: Send invitation email with callbackUrl
Email-->>User: Team invitation email
User->>User: Click invitation link
User->>PageClient: Navigate to callbackUrl
1 file reviewed, 1 comment
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
Show resolved
Hide resolved
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts (1)
packages/stack-shared/src/utils/urls.tsx (3)
createUrlIfValid(4-10)isLocalhost(199-205)matchHostnamePattern(144-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Vercel Agent Review
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: build (22.x)
🔇 Additional comments (3)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts (3)
3-4: LGTM! Appropriate utility imports for URL validation.The imported functions (
createUrlIfValid,isLocalhost,matchHostnamePattern) are correctly sourced and will be used for the origin validation logic.
16-18: Localhost validation relies on the flawed fallback logic.While the localhost check itself is reasonable, it operates on
projectsToCheckwhich may include unrelated projects due to the fallback on line 14. Once that critical issue is resolved, verify whether allowing localhost if any team project allows it is the intended behavior, or if stricter per-project validation is required.
79-88: Addresses security concern from previous review, but depends on fixing critical issue.This refactor correctly implements origin validation before constructing the callback URL, addressing the security concern raised in the previous review. The validated origin is used as the base for constructing the callback URL on line 81.
However, the security effectiveness depends on resolving the critical fallback issue in
assertTrustedOrigin(lines 13-14). Once that's fixed, this implementation will properly prevent invitations with malicious callback URLs.
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
Outdated
Show resolved
Hide resolved
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
Outdated
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts (1)
529-557: LGTM! Good security test coverage.This test correctly validates that untrusted callback URLs are rejected with the appropriate error response. The test structure follows existing patterns and properly exercises server-side whitelist validation.
Minor optional optimization:
receiveMailbox(line 531) is created but never used since the request fails early. You could remove it for efficiency, though keeping it maintains consistency with the test pattern at lines 107-143.it("should error when untrusted callback URL is provided", async ({ expect }) => { const { teamId } = await Team.create(); - const receiveMailbox = createMailbox(); backendContext.set({ userAuth: null }); const sendTeamInvitationResponse = await niceBackendFetch("/api/v1/team-invitations/send-code", { method: "POST", accessType: "server", body: { - email: receiveMailbox.emailAddress, + email: "test@example.com", team_id: teamId, callback_url: "https://malicious.com/callback", }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/actions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts (1)
apps/e2e/tests/backend/backend-helpers.ts (3)
createMailbox(59-66)backendContext(35-57)niceBackendFetch(109-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Vercel Agent Review
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
2 participants
Summary by CodeRabbit
Refactor
Bug Fixes / Security
Tests