refactor: map Zoho Calendar location to known domains#28896
refactor: map Zoho Calendar location to known domains#28896pedroccastro wants to merge 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the Zoho Calendar integration to centralize location-to-domain mapping logic. Previously, the callback handler contained inline conditional logic to map location codes to Zoho domain suffixes. A new shared helper function 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/zohocalendar/lib/zoho-domains.ts`:
- Around line 13-15: The getValidZohoDomain function currently indexes
LOCATION_TO_DOMAIN with the raw location value which allows prototype keys like
"toString" to resolve inherited properties; update getValidZohoDomain to first
ensure location is a plain string and then check membership using a safe
own-property check (e.g.,
Object.prototype.hasOwnProperty.call(LOCATION_TO_DOMAIN, location) or
Object.hasOwn) before returning LOCATION_TO_DOMAIN[location]; if the check fails
return DEFAULT_DOMAIN (still allow the existing "us" default when location is
undefined). Reference getValidZohoDomain, LOCATION_TO_DOMAIN, and DEFAULT_DOMAIN
when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12f15c6f-d2e6-4fd2-a0b0-9aadbfb1ca6a
📒 Files selected for processing (3)
packages/app-store/zohocalendar/api/callback.tspackages/app-store/zohocalendar/lib/zoho-domains.test.tspackages/app-store/zohocalendar/lib/zoho-domains.ts
| export function getValidZohoDomain(location: string | undefined): string { | ||
| return LOCATION_TO_DOMAIN[location || "us"] || DEFAULT_DOMAIN; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current behavior with prototype-inherited key vs safe hasOwn check.
node <<'NODE'
const LOCATION_TO_DOMAIN = { us:"com", eu:"eu", in:"in", cn:"com.cn", jp:"jp", au:"com.au" };
const probe = "toString";
const current = LOCATION_TO_DOMAIN[probe] || "com";
const safe = Object.prototype.hasOwnProperty.call(LOCATION_TO_DOMAIN, probe)
? LOCATION_TO_DOMAIN[probe]
: "com";
console.log("current_type:", typeof current);
console.log("current_preview:", String(current).slice(0, 60));
console.log("safe_value:", safe);
NODERepository: calcom/cal.com
Length of output: 151
🏁 Script executed:
# Check if the file exists and read the relevant code
cat -n packages/app-store/zohocalendar/lib/zoho-domains.ts | head -30Repository: calcom/cal.com
Length of output: 572
🏁 Script executed:
# Search for usages of getValidZohoDomain to understand the threat surface
rg "getValidZohoDomain" --type ts --type tsx -B 2 -A 2Repository: calcom/cal.com
Length of output: 85
🏁 Script executed:
# Check if LOCATION_TO_DOMAIN is accessed elsewhere in unsafe ways
rg "LOCATION_TO_DOMAIN" --type ts --type tsx -B 2 -A 2Repository: calcom/cal.com
Length of output: 85
🏁 Script executed:
# Find usages of getValidZohoDomain with simpler rg command
rg "getValidZohoDomain" -A 2 -B 2Repository: calcom/cal.com
Length of output: 3081
🏁 Script executed:
# Also check if location parameter comes from user input (query params, request body, etc.)
rg "location" packages/app-store/zohocalendar --type-not json -A 1 -B 1 | head -50Repository: calcom/cal.com
Length of output: 3801
Prototype-key inputs can bypass unknown-location fallback.
In packages/app-store/zohocalendar/api/callback.ts, the location parameter comes directly from req.query. Prototype keys like "toString" will resolve inherited Object properties, allowing non-domain values to flow into URL construction. The current fallback LOCATION_TO_DOMAIN[location || "us"] || DEFAULT_DOMAIN does not filter these.
🛠️ Proposed fix
-const LOCATION_TO_DOMAIN: Record<string, string> = {
+const LOCATION_TO_DOMAIN = {
us: "com",
eu: "eu",
in: "in",
cn: "com.cn",
jp: "jp",
au: "com.au",
-};
+} as const;
+type ZohoLocation = keyof typeof LOCATION_TO_DOMAIN;
const DEFAULT_DOMAIN = "com";
+function isZohoLocation(location: string): location is ZohoLocation {
+ return Object.prototype.hasOwnProperty.call(LOCATION_TO_DOMAIN, location);
+}
+
export function getValidZohoDomain(location: string | undefined): string {
- return LOCATION_TO_DOMAIN[location || "us"] || DEFAULT_DOMAIN;
+ if (!location) return DEFAULT_DOMAIN;
+ return isZohoLocation(location) ? LOCATION_TO_DOMAIN[location] : DEFAULT_DOMAIN;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getValidZohoDomain(location: string | undefined): string { | |
| return LOCATION_TO_DOMAIN[location || "us"] || DEFAULT_DOMAIN; | |
| } | |
| const LOCATION_TO_DOMAIN = { | |
| us: "com", | |
| eu: "eu", | |
| in: "in", | |
| cn: "com.cn", | |
| jp: "jp", | |
| au: "com.au", | |
| } as const; | |
| type ZohoLocation = keyof typeof LOCATION_TO_DOMAIN; | |
| const DEFAULT_DOMAIN = "com"; | |
| function isZohoLocation(location: string): location is ZohoLocation { | |
| return Object.prototype.hasOwnProperty.call(LOCATION_TO_DOMAIN, location); | |
| } | |
| export function getValidZohoDomain(location: string | undefined): string { | |
| if (!location) return DEFAULT_DOMAIN; | |
| return isZohoLocation(location) ? LOCATION_TO_DOMAIN[location] : DEFAULT_DOMAIN; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/app-store/zohocalendar/lib/zoho-domains.ts` around lines 13 - 15,
The getValidZohoDomain function currently indexes LOCATION_TO_DOMAIN with the
raw location value which allows prototype keys like "toString" to resolve
inherited properties; update getValidZohoDomain to first ensure location is a
plain string and then check membership using a safe own-property check (e.g.,
Object.prototype.hasOwnProperty.call(LOCATION_TO_DOMAIN, location) or
Object.hasOwn) before returning LOCATION_TO_DOMAIN[location]; if the check fails
return DEFAULT_DOMAIN (still allow the existing "us" default when location is
undefined). Reference getValidZohoDomain, LOCATION_TO_DOMAIN, and DEFAULT_DOMAIN
when making the change.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been closed due to inactivity. Please feel free to reopen it if you'd like to continue the work. |
1 participant
What does this PR do?
Replaces the inline
location → domainmapping in the Zoho Calendar OAuth callback with a shared helper (getValidZohoDomain) backed by an explicit map of supported Zoho data centers. Falls back tocomfor unknown locations.Changes
packages/app-store/zohocalendar/lib/zoho-domains.tswithLOCATION_TO_DOMAINmapping covering the Zoho multi-DC domains (us, eu, in, cn, jp, au) and agetValidZohoDomain(location)helpercallback.tsuses the helper instead of the partialif (location === "us") ... else if (location === "au") ... else server_location = locationblock, which previously forwarded any string into the OAuth URLHow should this be tested?
Manual
Mandatory Tasks