Add guest management functionality to existing bookings#2
Conversation
* feat: ability to add guests via app.cal.com/bookings * fix: some update * fix: minor issue * fix: final update * update * update * add requested changes * fix type error * small update * final update * fix type error * fix location * update calender event --------- Co-authored-by: Somay Chauhan <somaychauhan98@gmail.com>
![re-entry-ai[bot]](https://avatars.githubusercontent.com/in/3466347?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ re-entry.ai Code Review
@CodingKylo — Please review the following assessment:
🚨 Risk Score: 94/100 · CRITICAL
| Dimension | Level |
|---|---|
| Likelihood | 🔴 Critical |
| Impact | 🔴 Critical |
| Detectability | 🟠 High |
Intent
Add an end-to-end “additional guests” feature for existing bookings, including a UI dialog, a new TRPC mutation, and organizer/attendee email notifications.
Summary
Behaviorally, this PR introduces a new authenticated TRPC mutation to add booking attendees, wires it into a new UI dialog for entering multiple guest emails, and sends new “add guests” emails with generated ICS content. The top risks are (1) a likely authorization bug in the server-side permission gate (using && where || is expected), and (2) email/guest matching logic that is case-sensitive and can cause incorrect recipient selection and duplicate/blacklist bypass behavior. You must verify the permission logic is correct and that email normalization (case/whitespace) is consistent across UI validation, backend uniqueness/blacklist checks, and email recipient selection. Also confirm the email payload/template assumptions are safe for all booking states (null/empty attendee arrays).
🎯 Review Focus
The server-side authorization and email comparison logic in packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts—specifically the isTeamAdminOrOwner boolean gate and all email equality/uniqueness checks must be correct and consistently normalized.
Key Findings
- 🚨 [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L42] CRITICAL:
const isTeamAdminOrOwner = (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));— This is almost certainly the wrong boolean operator for an “admin OR owner” permission gate. With&&, a user must be both team admin and team owner to pass, which will incorrectly deny legitimate access (and could also lead to inconsistent behavior if other parts of the system assume “admin OR owner”). Fix: change to||and also avoid defaulting teamId to0(see next finding). Example:const teamId = booking.eventType?.teamId; const isTeamAdminOrOwner = teamId ? (await isTeamAdmin(user.id, teamId)) || (await isTeamOwner(user.id, teamId)) : false; - 🚨 [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L42] CRITICAL:
booking.eventType?.teamId ?? 0— DefaultingteamIdto0can cause permission checks to run against an unintended team context (or a “team 0” edge case), potentially granting/denying access incorrectly. Fix: do not coerce missing teamId to a sentinel; instead short-circuit:const teamId = booking.eventType?.teamId; const isTeamAdminOrOwner = teamId ? ((await isTeamAdmin(user.id, teamId)) || (await isTeamOwner(user.id, teamId))) : false; ⚠️ [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L70] WARNING:const isAttendee = !!booking.attendees.find((attendee) => attendee.email === user.email);— This is case-sensitive and does not normalize whitespace. If stored attendee emails are normalized differently thanuser.email, the user may fail the attendee check and be blocked (or, depending on other logic, could create inconsistent authorization outcomes). Fix: normalize both sides consistently:const normalize = (e: string) => e.trim().toLowerCase(); const isAttendee = booking.attendees.some(a => normalize(a.email) === normalize(user.email));and apply the same normalization everywhere emails are compared.⚠️ [packages/emails/templates/organizer-add-guests-email.ts:L16] WARNING: `to: toAddresses.join(⚠️ [apps/web/components/dialog/AddGuestsDialog.tsx:L33] WARNING:const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { const uniqueEmails = new Set(emails); return uniqueEmails.size === emails.length; });— Uniqueness is case-sensitive and does not trim. Users can enterTest@x.comandtest@x.comas “unique” and bypass dedupe/blacklist logic if the backend compares case-sensitively. Fix: normalize before uniqueness and before sending to the backend:const normalize = (e:string)=>e.trim().toLowerCase(); const ZAddGuestsInputSchema = z.array(z.string().email()).transform(emails => emails.map(normalize)).refine(emails => new Set(emails).size === emails.length);and send the normalized list to the mutation.
✅ Action Checklist
- [ ] CRITICAL [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L42] —
const isTeamAdminOrOwner = (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));— This is almost certainly the wrong boolean operator for an “admin OR owner” permission gate. With&&, a user must be both team admin and team owner to pass, which will incorrectly deny legitimate access (and could also lead to inconsistent behavior if other parts of the system assume “admin OR owner”). Fix: change to||and also avoid defaulting teamId to0(see next finding). Example:const teamId = booking.eventType?.teamId; const isTeamAdminOrOwner = teamId ? (await isTeamAdmin(user.id, teamId)) || (await isTeamOwner(user.id, teamId)) : false; - [ ] CRITICAL [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L42] —
booking.eventType?.teamId ?? 0— DefaultingteamIdto0can cause permission checks to run against an unintended team context (or a “team 0” edge case), potentially granting/denying access incorrectly. Fix: do not coerce missing teamId to a sentinel; instead short-circuit:const teamId = booking.eventType?.teamId; const isTeamAdminOrOwner = teamId ? ((await isTeamAdmin(user.id, teamId)) || (await isTeamOwner(user.id, teamId))) : false; - [ ] WARNING [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L70] —
const isAttendee = !!booking.attendees.find((attendee) => attendee.email === user.email);— This is case-sensitive and does not normalize whitespace. If stored attendee emails are normalized differently thanuser.email, the user may fail the attendee check and be blocked (or, depending on other logic, could create inconsistent authorization outcomes). Fix: normalize both sides consistently:const normalize = (e: string) => e.trim().toLowerCase(); const isAttendee = booking.attendees.some(a => normalize(a.email) === normalize(user.email));and apply the same normalization everywhere emails are compared. - [ ] WARNING [packages/emails/templates/organizer-add-guests-email.ts:L16] — `to: toAddresses.join(
- [ ] WARNING [apps/web/components/dialog/AddGuestsDialog.tsx:L33] —
const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { const uniqueEmails = new Set(emails); return uniqueEmails.size === emails.length; });— Uniqueness is case-sensitive and does not trim. Users can enterTest@x.comandtest@x.comas “unique” and bypass dedupe/blacklist logic if the backend compares case-sensitively. Fix: normalize before uniqueness and before sending to the backend:const normalize = (e:string)=>e.trim().toLowerCase(); const ZAddGuestsInputSchema = z.array(z.string().email()).transform(emails => emails.map(normalize)).refine(emails => new Set(emails).size === emails.length);and send the normalized list to the mutation. - [ ] SUGGESTION — Verify and align the permission contract end-to-end:
BookingListItem.tsxadds the UI action, but the server gate must be correct. After fixing the&&→||bug inpackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts, add a small UI capability check to avoid showing the dialog to users who will be denied (UX + reduces information leakage). Example: hide the menu item unless the booking viewer role indicates organizer/attendee/team-admin-or-owner. - [ ] SUGGESTION — Make email normalization a single source of truth across UI + backend. Implement a shared helper (e.g.,
normalizeEmail) and use it in:AddGuestsDialog.tsx(transform before mutate),addGuests.handler.ts(uniqueness, blacklist, attendee checks, and recipient selection), and any email template payload builders. - [ ] SUGGESTION — In
packages/emails/email-manager.ts, ensuresendAddGuestsEmailsdoesn’t assumecalendarEvent.attendees[0]exists. The organizer template usesthis.calEvent.attendees[0].namein the subject; if attendees is empty, this will throw. Fix by guarding in the template or (better) ensuring the TRPC handler always supplies at least one attendee when callingsendAddGuestsEmails. - [ ] SUGGESTION — Add regression tests for the permission gate and email normalization. At minimum: (1) team admin-only can add guests, (2) team owner-only can add guests, (3) attendee-only can add guests, (4) case-insensitive dedupe/blacklist behavior. This should be unit tests around the handler logic and an integration test for the mutation.
Suggestions
- Verify and align the permission contract end-to-end:
BookingListItem.tsxadds the UI action, but the server gate must be correct. After fixing the&&→||bug inpackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts, add a small UI capability check to avoid showing the dialog to users who will be denied (UX + reduces information leakage). Example: hide the menu item unless the booking viewer role indicates organizer/attendee/team-admin-or-owner. - Make email normalization a single source of truth across UI + backend. Implement a shared helper (e.g.,
normalizeEmail) and use it in:AddGuestsDialog.tsx(transform before mutate),addGuests.handler.ts(uniqueness, blacklist, attendee checks, and recipient selection), and any email template payload builders. - In
packages/emails/email-manager.ts, ensuresendAddGuestsEmailsdoesn’t assumecalendarEvent.attendees[0]exists. The organizer template usesthis.calEvent.attendees[0].namein the subject; if attendees is empty, this will throw. Fix by guarding in the template or (better) ensuring the TRPC handler always supplies at least one attendee when callingsendAddGuestsEmails. - Add regression tests for the permission gate and email normalization. At minimum: (1) team admin-only can add guests, (2) team owner-only can add guests, (3) attendee-only can add guests, (4) case-insensitive dedupe/blacklist behavior. This should be unit tests around the handler logic and an integration test for the mutation.
📝 This review includes 3 inline comments (2 warnings, 1 note)
Posted by re-entry.ai · Risk governance for autonomous engineering teams
| onClick: () => { | ||
| setIsOpenAddGuestsDialog(true); | ||
| }, | ||
| icon: "user-plus" as const, |
There was a problem hiding this comment.
This new action opens the guest-editing dialog for any rendered booking item, but the actual authorization boundary lives in the TRPC handler. The handler only checks organizer/attendee/team-admin-or-owner, so the UI entry point itself is fine; the real risk is that this path now exposes a mutation that must be enforced server-side for every caller. In particular, the handler’s team-admin/owner check uses && instead of ||, which makes the permission gate much stricter than intended and can incorrectly deny legitimate team admins or owners.
| @@ -0,0 +1,96 @@ | |||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | |||
There was a problem hiding this comment.
💡 NOTE
This file now imports Icon and Tooltip from @calcom/ui and is re-exported through the package barrel, so the build concern is not about missing exports. The real issue is that the component mutates its value prop locally (value = value || []), which hides the fact that the parent is expected to own the array state and can make the component’s behavior harder to reason about. This is a correctness/maintainability smell rather than a build break.
| const { isOpenDialog, setIsOpenDialog, bookingId } = props; | ||
| const utils = trpc.useUtils(); | ||
| const [multiEmailValue, setMultiEmailValue] = useState<string[]>([""]); | ||
| const [isInvalidEmail, setIsInvalidEmail] = useState(false); |
There was a problem hiding this comment.
There was a problem hiding this comment.
🛡️ re-entry.ai Code Review
@CodingKylo — Please review the following assessment:
🚨 Risk Score: 91/100 · CRITICAL
| Dimension | Level |
|---|---|
| Likelihood | 🔴 Critical |
| Impact | 🔴 Critical |
| Detectability | 🟠 High |
Summary
Code Review Summary
Risk Level: CRITICAL (Score: 91/100)
Files Changed: 0
High Risk Areas: 3
Key Concerns
- CHANGEMAGNITUDE (high): This PR adds a new end-to-end feature across 15 files (UI dialog, booking list action, tRPC router/handler/schema, and multiple email templates), increasing integration risk.
- CONFIGURATION (high): Guest addition semantics include filtering blacklisted emails, deduping against existing attendees, creating attendee records, and sending different email types, any of which could be logically incorrect.
- CONFIGURATION (high): The PR touches multiple rarely-changed core areas (tRPC booking router/handler, email manager/templates, and booking UI), which increases the chance of subtle integration issues.
Posted by re-entry.ai · Risk governance for autonomous engineering teams
There was a problem hiding this comment.
🛡️ re-entry.ai Code Review
@CodingKylo — Please review the following assessment:
🚨 Risk Score: 91/100 · CRITICAL
| Dimension | Level |
|---|---|
| Likelihood | 🔴 Critical |
| Impact | 🔴 Critical |
| Detectability | 🟠 High |
Intent
Add an end-to-end “add guests to booking” flow that updates attendees in the backend and notifies organizer/attendees via email.
Summary
This PR introduces a new tRPC mutation (viewer.bookings.addGuests) plus a UI dialog to collect guest emails, persist attendee records, and send organizer/attendee email notifications (including an ICS attachment). The highest risk is a correctness/security bug in the authorization gate: the handler currently requires the user to be both team admin AND team owner due to a boolean &&, which will incorrectly deny legitimate users (and the permission logic is security-sensitive). There are also multiple cross-file contract risks (router export wiring and duplicated Zod validation between UI and backend) that can cause the endpoint to be missing or behave inconsistently. Finally, the email/ICS generation path should be reviewed for correctness and operational behavior (e.g., whether failures are swallowed and whether recipient lists are correct).
🎯 Review Focus
Verify the authorization gate logic and the tRPC router/handler wiring for viewer.bookings.addGuests—specifically that team admin/owner permissions use correct OR semantics and that the procedure is actually registered under the expected name.
Key Findings
- 🚨 [packages/trpc/server/routers/viewer/bookings/_router.tsx:L1] CRITICAL: Contract mismatch risk between router wiring and handler export — the diff analysis indicates
_router.tsxexpectsaddGuests?: typeof import("./addGuests.handler").addGuestsHandlerwhile the handler file exportsaddGuestsHandler(or a differently named symbol). If the named export doesn’t match exactly, the procedure may not be registered at runtime (or may be registered under a different key), leading to 404/undefined procedure errors. Fix: verify the exact named export inaddGuests.handler.tsand the exact import/type in_router.tsx, then add a type-level assertion/unit test thatrouterexposesaddGuests(e.g., a small test that callsappRouter.createCaller(...).viewer.bookings.addGuestsor a compile-time check usingkeyof).
✅ Action Checklist
- [ ] CRITICAL [packages/trpc/server/routers/viewer/bookings/_router.tsx:L1] — Contract mismatch risk between router wiring and handler export — the diff analysis indicates
_router.tsxexpectsaddGuests?: typeof import("./addGuests.handler").addGuestsHandlerwhile the handler file exportsaddGuestsHandler(or a differently named symbol). If the named export doesn’t match exactly, the procedure may not be registered at runtime (or may be registered under a different key), leading to 404/undefined procedure errors. Fix: verify the exact named export inaddGuests.handler.tsand the exact import/type in_router.tsx, then add a type-level assertion/unit test thatrouterexposesaddGuests(e.g., a small test that callsappRouter.createCaller(...).viewer.bookings.addGuestsor a compile-time check usingkeyof). - [ ] SUGGESTION — [apps/web/components/dialog/AddGuestsDialog.tsx:L1] WARNING: UI/backend validation duplication can drift — the UI defines
const ZAddGuestsInputSchema = z.array(z.string().email()).refine(...)and then callsaddGuestsMutation.mutate({ bookingId, guests: multiEmailValue });while the backend has its ownaddGuests.schema.ts. If constraints diverge (e.g., blacklist filtering, dedupe rules, max length, or error messaging), users will see inconsistent behavior. Fix: export the Zod schema frompackages/trpc/server/routers/viewer/bookings/addGuests.schema.ts(or a shared@calcom/trpc/schemas) and import it into the UI so both sides validate identically. - [ ] SUGGESTION — [packages/ui/form/MultiEmailLazy.tsx:L4] WARNING:
as unknown as typeof import("./MultiEmail").defaultsuppresses type safety at the dynamic import boundary — if the default export shape changes, TS won’t catch it and runtime can fail. Fix: remove the double cast by typing the lazy import correctly, e.g.const MultiEmailLazy = lazy(() => import('./MultiEmail').then(m => ({ default: m.default })));or ensure the lazy wrapper’s generic matches the actual module type. - [ ] SUGGESTION — [packages/ui/form/MultiEmail.tsx:L18] INFO: Prop mutation via reassignment (
value = value || []) is confusing — it doesn’t mutate the caller, but it obscures data flow and can hide future bugs ifvalueis later assumed immutable. Fix: use a local normalized variable:const normalizedValue = value ?? [];and then render fromnormalizedValue. - [ ] SUGGESTION — [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L1] WARNING: Authorization model is server-enforced only; UI may present actions that will fail — the UI likely always renders the “add guests” action, but the server gate can return
FORBIDDEN. Fix: either (a) add a lightweight capability query to conditionally render the action, or (b) ensure the UI handlesFORBIDDENdistinctly (e.g., show “not permitted” without implying the user can fix it).
Suggestions
- [apps/web/components/dialog/AddGuestsDialog.tsx:L1] WARNING: UI/backend validation duplication can drift — the UI defines
const ZAddGuestsInputSchema = z.array(z.string().email()).refine(...)and then callsaddGuestsMutation.mutate({ bookingId, guests: multiEmailValue });while the backend has its ownaddGuests.schema.ts. If constraints diverge (e.g., blacklist filtering, dedupe rules, max length, or error messaging), users will see inconsistent behavior. Fix: export the Zod schema frompackages/trpc/server/routers/viewer/bookings/addGuests.schema.ts(or a shared@calcom/trpc/schemas) and import it into the UI so both sides validate identically. - [packages/ui/form/MultiEmailLazy.tsx:L4] WARNING:
as unknown as typeof import("./MultiEmail").defaultsuppresses type safety at the dynamic import boundary — if the default export shape changes, TS won’t catch it and runtime can fail. Fix: remove the double cast by typing the lazy import correctly, e.g.const MultiEmailLazy = lazy(() => import('./MultiEmail').then(m => ({ default: m.default })));or ensure the lazy wrapper’s generic matches the actual module type. - [packages/ui/form/MultiEmail.tsx:L18] INFO: Prop mutation via reassignment (
value = value || []) is confusing — it doesn’t mutate the caller, but it obscures data flow and can hide future bugs ifvalueis later assumed immutable. Fix: use a local normalized variable:const normalizedValue = value ?? [];and then render fromnormalizedValue. - [packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:L1] WARNING: Authorization model is server-enforced only; UI may present actions that will fail — the UI likely always renders the “add guests” action, but the server gate can return
FORBIDDEN. Fix: either (a) add a lightweight capability query to conditionally render the action, or (b) ensure the UI handlesFORBIDDENdistinctly (e.g., show “not permitted” without implying the user can fix it).
📝 This review includes 3 inline comments (1 critical, 1 warning, 1 note)
Posted by re-entry.ai · Risk governance for autonomous engineering teams
| include: { | ||
| attendees: true, | ||
| eventType: true, | ||
| destinationCalendar: true, |
There was a problem hiding this comment.
🚨 CRITICAL
The authorization check is wrong: it uses &&, so a user must be both team admin and team owner to pass. That makes the isTeamAdminOrOwner name misleading and will incorrectly forbid legitimate admins or owners from adding guests. The root cause is a boolean logic error in the permission gate, not just a naming issue.
| import dynamic from "next/dynamic"; | ||
|
|
||
| /** These are like 40kb that not every user needs */ | ||
| const MultiEmail = dynamic(() => import("./MultiEmail")) as unknown as typeof import("./MultiEmail").default; |
There was a problem hiding this comment.
The as unknown as typeof import("./MultiEmail").default cast suppresses the actual module shape at the dynamic import boundary. If the component export changes or the lazy import resolves differently, TypeScript will not catch the mismatch and runtime failures can slip through. This is a maintainability/type-safety issue rather than an immediate bug.
| const inputClassName = | ||
| "dark:placeholder:text-muted focus:border-emphasis border-subtle block w-full rounded-md border-default text-sm focus:ring-black disabled:bg-emphasis disabled:hover:cursor-not-allowed dark:selection:bg-green-500 disabled:dark:text-subtle bg-default"; | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
💡 NOTE
Reassigning the value prop inside the component (value = value || []) is confusing and makes the component rely on prop mutation semantics. It does not change the caller’s data, but it obscures the data flow and can hide bugs if the prop is later treated as immutable elsewhere. Prefer a local normalized variable instead.
There was a problem hiding this comment.
🛡️ re-entry.ai Code Review
@CodingKylo — Please review the following assessment:
🚨 Risk Score: 92/100 · CRITICAL
| Dimension | Level |
|---|---|
| Likelihood | 🔴 Critical |
| Impact | 🔴 Critical |
| Detectability | 🟠 High |
Intent
Add a UI + TRPC mutation to let authorized users add additional guest emails to an existing booking and trigger corresponding email notifications.
Summary
The PR introduces a new TRPC handler (viewer.bookings.addGuests) and a web dialog to collect multiple guest emails, then sends new-guests emails with an attached/embedded iCal payload. The highest risks are (1) a likely authorization logic bug in the handler that uses && where the intent is “admin OR owner”, and (2) a runtime crash in the organizer email template due to direct indexing into attendees[0]. You must verify the server-side authorization is correct for bookingId (IDOR resistance) and that email template rendering is safe for events with empty/undefined attendee lists.
🎯 Review Focus
Verify the TRPC handler’s authorization for the specific bookingId (IDOR resistance) and correct the admin/owner permission logic (&& vs ||) while also ensuring email template rendering is safe when calEvent.attendees is empty.
✅ Action Checklist
- [ ] SUGGESTION — In
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts, add explicit validation/guards forbooking.eventType?.teamIdand forbooking.userIdbefore using them in permission checks and subsequent queries; avoid?? 0fallbacks because they can silently change authorization behavior. Example: computeconst teamId = booking.eventType?.teamId;and if absent, setisTeamAdminOrOwner = false(or throw a clear error). - [ ] SUGGESTION — In
packages/emails/email-manager.ts,sendAddGuestsEmailscurrently pushes organizer emails unconditionally and then iterates team members; ensure the template inputs always include a non-emptyattendeesarray or update the template to not assume it. Add a unit test that renders both organizer and attendee add-guests emails for events withattendees: []andattendees: undefined. - [ ] SUGGESTION — In
apps/web/components/dialog/AddGuestsDialog.tsx, the Zod schema is created inside the component on every render; move it outside the component or memoize it to avoid unnecessary allocations. Also consider trimming/normalizing emails (e.g.,z.string().email().transform(s => s.trim().toLowerCase())) so uniqueness checks aren’t bypassed by casing/whitespace.
Suggestions
- In
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts, add explicit validation/guards forbooking.eventType?.teamIdand forbooking.userIdbefore using them in permission checks and subsequent queries; avoid?? 0fallbacks because they can silently change authorization behavior. Example: computeconst teamId = booking.eventType?.teamId;and if absent, setisTeamAdminOrOwner = false(or throw a clear error). - In
packages/emails/email-manager.ts,sendAddGuestsEmailscurrently pushes organizer emails unconditionally and then iterates team members; ensure the template inputs always include a non-emptyattendeesarray or update the template to not assume it. Add a unit test that renders both organizer and attendee add-guests emails for events withattendees: []andattendees: undefined. - In
apps/web/components/dialog/AddGuestsDialog.tsx, the Zod schema is created inside the component on every render; move it outside the component or memoize it to avoid unnecessary allocations. Also consider trimming/normalizing emails (e.g.,z.string().email().transform(s => s.trim().toLowerCase())) so uniqueness checks aren’t bypassed by casing/whitespace.
📝 This review includes 3 inline comments (3 critical)
Posted by re-entry.ai · Risk governance for autonomous engineering teams
| from: `${APP_NAME} <${this.getMailerOptions().from}>`, | ||
| to: toAddresses.join(","), | ||
| replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)], | ||
| subject: `${this.t("guests_added_event_type_subject", { |
There was a problem hiding this comment.
🚨 CRITICAL
Quote: name: this.calEvent.attendees[0].name,
Issue: Direct indexing into attendees[0] will throw at runtime if attendees is empty (or if attendees is undefined). This is a likely crash path for edge cases (e.g., organizer-only events or data inconsistencies).
Fix: Guard and use a safe fallback:
const firstAttendee = this.calEvent.attendees?.[0];
if (!firstAttendee) {
// choose a fallback name or omit the field
}(see also L26)
| user: { | ||
| include: { | ||
| destinationCalendar: true, | ||
| credentials: true, |
There was a problem hiding this comment.
🚨 CRITICAL
Quote: const isTeamAdminOrOwner = (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
Issue: The variable name and intent suggest “admin OR owner”, but the code uses && (AND). This can unintentionally block legitimate users (admin-only or owner-only) and/or allow incorrect flows depending on how isTeamAdmin/isTeamOwner behave for teamId fallback to 0.
Fix: Use OR and avoid the 0 fallback if teamId can be missing. For example:
const teamId = booking.eventType?.teamId;
const isTeamAdminOrOwner = teamId
? (await isTeamAdmin(user.id, teamId)) || (await isTeamOwner(user.id, teamId))
: false;| }, | ||
| onError: (err) => { | ||
| const message = `${err.data?.code}: ${t(err.message)}`; | ||
| showToast(message || t("unable_to_add_guests"), "error"); |
There was a problem hiding this comment.
🚨 CRITICAL
Quote: const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({ ... })
Issue (security/authorization): The UI passes an arbitrary bookingId prop to a backend mutation. If the server-side viewer.bookings.addGuests procedure does not strictly authorize that the caller can modify that booking, this becomes an IDOR (insecure direct object reference) risk.
Fix: Ensure the backend procedure enforces authorization for bookingId (e.g., verify the viewer is the booking owner/has permission) and do not rely on the client-provided bookingId.
(Verification note: this file alone can’t confirm server-side authz; the risk is that the client exposes bookingId directly.)
2 participants
Martian Code Review Benchmark PR (mirrored from source #10)