You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Transform destinationCalendar from single object to array to support multiple host destinations in collective bookings
Update calendar service implementations to handle array-based destination calendars with credential matching
Refactor event manager to process multiple calendar references and credentials for team events
Add credential ID parameter to createEvent method for proper calendar selection
Diagram Walkthrough
flowchart LR
A["destinationCalendar<br/>Single Object"] -->|"Convert to Array"| B["destinationCalendar[]<br/>Multiple Calendars"]
B -->|"Match by credentialId"| C["GoogleCalendarService<br/>LarkCalendarService<br/>Office365Service"]
C -->|"Create/Update Events"| D["Calendar Events<br/>Per Credential"]
E["EventManager<br/>CalendarManager"] -->|"Process Multiple<br/>References"| B
Below is a summary of compliance checks for this PR:
Security Compliance
⚪
Unsafe calendar fallback
Description: When no matching destination calendar is found for the provided credentialId, the code falls back to using the "primary" calendar, which may lead to events being created on an unintended calendar without explicit confirmation. CalendarService.ts [147-155]
Description: Missing explicit validation of destinationCalendar array items (e.g., ensuring credentialId/externalId belong to the organizer or team) could allow misrouting events to unauthorized calendars if upstream data is tampered. EventManager.ts [329-383]
Referred Code
**Whentheoptionaluid is set,itwillbeusedinsteadoftheautogenerateduid.** @paramevent* @paramnoMail* @private*/privateasynccreateAllCalendarEvents(event: CalendarEvent){letcreatedEvents: EventResult<NewCalendarEventType>[]=[];if(event.destinationCalendar&&event.destinationCalendar.length>0){for(constdestinationofevent.destinationCalendar){if(destination.credentialId){letcredential=this.calendarCredentials.find((c)=>c.id===destination.credentialId);if(!credential){// Fetch credential from DBconstcredentialFromDB=awaitprisma.credential.findUnique({include: {app: {select: {slug: true,},
... (clipped34lines)
Broad deletion risk
Description: Cancel flow iterates over all calendar references and may delete events across all connected calendar credentials when references lack credentialId, risking unintended deletions if references are inconsistent or manipulated. handleCancelBooking.ts [418-483]
Referred Code
constbookingCalendarReference=bookingToDelete.references.filter((reference)=>reference.type.includes("_calendar"));if(bookingCalendarReference.length>0){for(constreferenceofbookingCalendarReference){const{ credentialId, uid, externalCalendarId }=reference;// If the booking calendar reference contains a credentialIdif(credentialId){// Find the correct calendar credential under user credentialsletcalendarCredential=bookingToDelete.user.credentials.find((credential)=>credential.id===credentialId);if(!calendarCredential){// get credential from DBconstfoundCalendarCredential=awaitprisma.credential.findUnique({where: {id: credentialId,},});if(foundCalendarCredential){
... (clipped45lines)
Ticket Compliance
⚪
🎫 No ticket provided
Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined
Follow the guide to enable codebase context checks.
Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code
Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting
Status: Passed
Generic: Secure Error Handling
Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging.
Status: Passed
⚪
Generic: Comprehensive Audit Trails
Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance.
Status: Missing audit logging: New flows creating/updating/deleting calendar events across multiple credentials lack explicit audit logs of actor, action, and outcome, making it unclear if critical actions are recorded.
Referred Code
**Whentheoptionaluid is set,itwillbeusedinsteadoftheautogenerateduid.** @paramevent* @paramnoMail* @private*/privateasynccreateAllCalendarEvents(event: CalendarEvent){letcreatedEvents: EventResult<NewCalendarEventType>[]=[];if(event.destinationCalendar&&event.destinationCalendar.length>0){for(constdestinationofevent.destinationCalendar){if(destination.credentialId){letcredential=this.calendarCredentials.find((c)=>c.id===destination.credentialId);if(!credential){// Fetch credential from DBconstcredentialFromDB=awaitprisma.credential.findUnique({include: {app: {select: {slug: true,},
... (clipped261lines)
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation
Status: Swallowed errors: The createEvent path catches errors and returns undefined on 404 and other cases without surfacing actionable context, risking silent failures across multi-calendar creation.
Referred Code
): Promise<EventResult<NewCalendarEventType>>=>{constuid: string=getUid(calEvent);constcalendar=awaitgetCalendar(credential);letsuccess=true;letcalError: string|undefined=undefined;// Check if the disabledNotes flag is set to trueif(calEvent.hideCalendarNotes){calEvent.additionalNotes="Notes have been hidden by the organizer";// TODO: i18n this string?}// TODO: Surface success/error messages coming from apps to improve end user visibilityconstcreationResult=calendar
? awaitcalendar.createEvent(calEvent,credential.id).catch(async(error: {code: number;calError: string})=>{success=false;/** * There is a time when selectedCalendar externalId doesn't match witch certain credential * so google returns 404. * */
... (clipped27lines)
Generic: Secure Logging Practices
Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data.
Status: Potential PII logging: The error log in createEvent logs the entire event object which may include PII, risking exposure of sensitive data in logs.
Referred Code
? awaitcalendar.createEvent(calEvent,credential.id).catch(async(error: {code: number;calError: string})=>{success=false;/** * There is a time when selectedCalendar externalId doesn't match witch certain credential * so google returns 404. * */if(error?.code===404){returnundefined;}if(error?.calError){calError=error.calError;}log.error("createEvent failed",JSON.stringify(error),calEvent);// @TODO: This code will be off till we can investigate an error with it//https://github.com/calcom/cal.com/issues/3949// await sendBrokenIntegrationEmail(calEvent, "calendar");
Generic: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities
Status: Limited validation: Newly added user loading and team destination calendar aggregation rely on database values without visible validation or sanitization for fields like usernames and destinationCalendar arrays.
Referred Code
message: `EventType '${eventType.eventName}' cannot be booked at this time.`,};log.warn({message: `NewBooking: EventType '${eventType.eventName}' cannot be booked at this time.`,});thrownewHttpError({statusCode: 400,message: error.message});}constloadUsers=async()=>{try{if(!eventTypeId){if(!Array.isArray(dynamicUserList)||dynamicUserList.length===0){thrownewError("dynamicUserList is not properly defined or empty.");}constusers=awaitprisma.user.findMany({where: {username: {in: dynamicUserList},},select: {
...userSelect.select,
... (clipped345lines)
Compliance status legend
🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
The PR inconsistently implements multi-calendar event creation. While GoogleCalendarService uses the new credentialId to select the correct calendar, other services like LarkCalendarService and Office365CalendarService default to the first calendar, which needs to be fixed.
asynccreateEvent(calEventRaw: CalendarEvent,credentialId: number): Promise<NewCalendarEventType>{consteventAttendees=calEventRaw.attendees.map(({id: _id, ...rest})=>({
...rest,responseStatus: "accepted",}));// TODO: Check every other CalendarService for team membersconstteamMembers=calEventRaw.team?.members.map((m)=>({email: m.email,displayName: m.name,
... (clipped53lines)
// In GoogleCalendarService (correctly implemented)classGoogleCalendarService{asynccreateEvent(calEvent: CalendarEvent,credentialId: number){constselectedCalendar=calEvent.destinationCalendar?.find((cal)=>cal.credentialId===credentialId)?.externalId;// ... creates event on the correct user's calendar}}// In LarkCalendarService and others (incorrectly implemented)classLarkCalendarService{asynccreateEvent(event: CalendarEvent){// Missing credentialIdconst[mainHostDestinationCalendar]=event.destinationCalendar??[];constcalendarId=mainHostDestinationCalendar?.externalId;// ... always creates event on the first calendar in the list}}
After:
// In GoogleCalendarService (correct)classGoogleCalendarService{asynccreateEvent(calEvent: CalendarEvent,credentialId: number){constselectedCalendar=calEvent.destinationCalendar?.find((cal)=>cal.credentialId===credentialId)?.externalId;// ... creates event on the correct user's calendar}}// In LarkCalendarService and others (corrected)classLarkCalendarService{asynccreateEvent(event: CalendarEvent,credentialId: number){// Added credentialIdconstselectedCalendar=event.destinationCalendar?.find((cal)=>cal.credentialId===credentialId)?.externalId;// ... creates event on the correct user's calendar}}
Suggestion importance[1-10]: 9
__
Why: This is a critical flaw, as the core feature of supporting multiple destination calendars for collective events is only correctly implemented for GoogleCalendarService, leaving it broken for other major calendar integrations.
High
Possible issue
Fix incorrect calendar selection logic
In updateEvent, fix the calendar selection logic by using this.credential.id to find the correct destination calendar instead of incorrectly using externalCalendarId in the find condition.
Why: The suggestion correctly identifies a logical flaw where externalCalendarId (which is null or undefined) is used to find a calendar, which will always fail. The proposed fix to use this.credential.id is a valid approach to correctly identify the calendar associated with the current credential.
Medium
Preserve original error in catch block
In the loadUsers function's catch block, re-throw HttpError and Prisma.PrismaClientKnownRequestError instances directly to preserve their original context and status codes.
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that re-throwing specific errors (HttpError, Prisma.PrismaClientKnownRequestError) instead of wrapping them in a new generic HttpError would preserve valuable error context and status codes, improving error handling and debugging.
Medium
More
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
PR #3
PR Type
Enhancement, Bug fix
Description
Transform
destinationCalendarfrom single object to array to support multiple host destinations in collective bookingsUpdate calendar service implementations to handle array-based destination calendars with credential matching
Refactor event manager to process multiple calendar references and credentials for team events
Add credential ID parameter to
createEventmethod for proper calendar selectionDiagram Walkthrough
File Walkthrough
18 files
Change destinationCalendar to array typeUpdate destinationCalendar property to arrayAdd credentialId parameter and handle array destinationsProcess multiple calendar references and credentialsHandle array destination calendars with credential matchingExtract first destination calendar from arrayExtract main host destination calendar from arrayHandle array-based destination calendars in iCalCollect team destination calendars for collective eventsProcess multiple calendar references for deletionConvert destination calendar to array formatConvert destination calendar to array formatConvert destination calendar to array formatConvert destination calendar to array formatConvert destination calendar to array formatConvert destination calendar to array formatExtract main host destination calendar from arrayConvert destination calendar to array format1 files
Update test expectations for array destination calendar1 files
Fix conditional logic for billing configuration1 files
Refactor import statement organization1 files