fix(security): derive team session host identity from the authenticated session (Closes #105) - #164
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
e0ad734
into
harsharajkumar-273:main
Closes #105
Problem
POST /team-sessions/createtakescreatedBystraight fromreq.bodyand hands it toteamSessionStore.createSession(), which uses it to populatehostNameandhostLogin:Those two fields are what every participant joining the room sees attributed to its host. Because they originate in the request body, any authenticated user can create a team session presenting as any name and login they choose.
Both named files needed the fix, and here's why
The endpoint is registered twice:
backend/src/routes/system.routes.ts(line 91)backend/src/routes/team.routes.ts→backend/src/controllers/team.controller.tsIn
server.ts,createSystemRouter()mounts at line 96 andcreateTeamRouter()at line 104. Express matches in registration order, so the system route is the live handler andteam.controller.createTeamSessionis currently unreachable for this path. (ItsGET /team-sessions/:codesibling in the same controller is live.)Fixing only the reachable copy would leave a spoofable handler sitting one mount-order change away from being live again, so both are corrected identically.
Change
Both handlers now ignore
createdByfrom the body entirely and build it fromreq.authSession.user:The explicit 403 covers the bearer-token path:
requireAccessTokenaccepts a raw bearer token and leavesreq.authSessionnull, and a session hosted under an unverifiable identity is exactly what this change exists to prevent.Compatibility
This is behaviour-preserving for real clients.
EditorPage.tsxalready sendscreatedBy: { login: userData?.login, name: userData?.name }built from its own session-derived user, and calls the API withcredentials: 'include', so the cookie session is present and server-side derivation yields the same values it was sending.Local test mode is also unaffected —
localTestRepoService.getUser()returns{ login: 'local-tester', name: 'Local Test User' }, so the session user is populated there too.No frontend change is required; the now-ignored
createdByin the request body is harmless to leave in place.Verification
npx tsc --noEmitinbackend/clean.server.tstraced to confirm which handler is live.createSession()reads only.nameand.loginoffcreatedBy, so the substituted object is complete.Note:
npm testis red on cleanmainindependently of this branch.