Automated Test: oauth-security-enhanced #371
Conversation
…11059) * Add credential sync .env variables * Add webhook to send app credentials * Upsert credentials when webhook called * Refresh oauth token from a specific endpoint * Pass appSlug * Add credential encryption * Move oauth helps into a folder * Create parse token response wrapper * Add OAuth helpers to apps * Clean up * Refactor `appDirName` to `appSlug` * Address feedback * Change to safe parse * Remove console.log --------- Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Co-authored-by: Omar López <zomars@me.com>
📝 WalkthroughWalkthroughThis PR implements OAuth credential sharing infrastructure by introducing environment configuration, a webhook endpoint for credential synchronization, reorganizing OAuth utilities into a dedicated directory, creating centralized token refresh and parsing helpers, and updating token refresh flows across multiple integrations. Changes
Sequence DiagramsequenceDiagram
participant External as External System
participant Webhook as Webhook Handler
participant DB as Database
participant Crypto as Encryption Service
External->>Webhook: POST /api/webhook/app-credential<br/>(with CALCOM_WEBHOOK_SECRET header)
Webhook->>Webhook: Validate feature flag<br/>(APP_CREDENTIAL_SHARING_ENABLED)
Webhook->>Webhook: Verify webhook secret header
Webhook->>Webhook: Parse request body schema<br/>(userId, appSlug, keys)
Webhook->>DB: Verify user exists
Webhook->>DB: Verify app exists
Webhook->>Crypto: Decrypt provided keys
Webhook->>DB: Check for existing<br/>user+app credential
alt Credential exists
Webhook->>DB: Update existing credential<br/>with decrypted keys
else Credential not found
Webhook->>DB: Create new credential<br/>with userId, appId, keys
end
DB-->>Webhook: Success
Webhook-->>External: 200 OK / 404 / 403<br/>(with descriptive message)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app-store/salesforce/lib/CalendarService.ts (1)
90-108:⚠️ Potential issue | 🔴 CriticalUse refreshed tokens when creating the jsforce connection.
The connection is created with stale tokens from the original credential (
credentialKey) instead of using the newly refreshed tokens from the API response. Although the refreshed tokens are correctly saved to the database (line 96-99), the jsforce Connection instantiated on line 101-108 uses the expiredaccess_tokenand oldinstance_url, so the refresh won't apply to the current request.🛠️ Proposed fix
await prisma.credential.update({ where: { id: credential.id }, data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } }, }); - return new jsforce.Connection({ + const refreshedKey = accessTokenParsed.data; + return new jsforce.Connection({ clientId: consumer_key, clientSecret: consumer_secret, redirectUri: WEBAPP_URL + "/api/integrations/salesforce/callback", - instanceUrl: credentialKey.instance_url, - accessToken: credentialKey.access_token, - refreshToken: credentialKey.refresh_token, + instanceUrl: refreshedKey.instance_url ?? credentialKey.instance_url, + accessToken: refreshedKey.access_token, + refreshToken: refreshedKey.refresh_token ?? credentialKey.refresh_token, });packages/app-store/googlecalendar/lib/CalendarService.ts (1)
95-101:⚠️ Potential issue | 🟠 MajorPersist the parsed token payload, not the SafeParse wrapper.
parseRefreshTokenResponsereturns a Zod SafeParse result with structure{ success, data, error }. Storing it directly persists the wrapper object instead of the credential fields, corrupting the credential shape and breaking downstream token usage.🛠️ Proposed fix
- const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema); + const key = parseRefreshTokenResponse(googleCredentials, googleCredentialSchema).data;
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 233-244: The dotenv-linter warnings come from quoted empty values
and key ordering; update the CALCOM_* block by removing the surrounding double
quotes for the empty values (e.g., CALCOM_WEBHOOK_SECRET=,
CALCOM_WEBHOOK_HEADER_NAME=calcom-webhook-secret,
CALCOM_CREDENTIAL_SYNC_ENDPOINT=, CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY=) and
reorder the keys into the expected alphabetical order
(CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY, CALCOM_CREDENTIAL_SYNC_ENDPOINT,
CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET) so QuoteCharacter and
UnorderedKey warnings are resolved.
In `@apps/web/pages/api/webhook/app-credential.ts`:
- Around line 56-59: The code currently calls symmetricDecrypt and JSON.parse
without validating the encryption key or catching errors; update the handler to
first verify process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is present and
non-empty, then wrap the symmetricDecrypt(reqBody.keys, ...) and JSON.parse(...)
sequence in a try/catch; on failure log the error (including the exception
message) and return a client error (e.g., 400/422) rather than letting an
unhandled 500 propagate. Reference symmetricDecrypt, JSON.parse, reqBody.keys
and CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY when making the checks and error
handling.
- Around line 17-32: The handler currently lacks an HTTP method guard and uses
parse() which throws on invalid JSON/body; update the exported handler to
early-return 405 for non-POST methods (check req.method === 'POST') and replace
appCredentialWebhookRequestBodySchema.parse(req.body) with safeParse and return
a 400 with validation errors when safeParse fails; additionally wrap any
explicit JSON.parse usage in the handler (the JSON.parse call you added) in a
try-catch and return a 400 with a clear message on parse errors instead of
letting exceptions bubble.
In `@packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts`:
- Around line 25-27: In parseRefreshTokenResponse, stop assigning the literal
placeholder "refresh_token" to refreshTokenResponse.data.refresh_token; this
corrupts stored credentials. Instead, remove that assignment so the field
remains undefined when absent, or if a fallback is available use a
caller-supplied existing stored credential value (pass it into
parseRefreshTokenResponse and apply it only when present). Update references to
refreshTokenResponse.data.refresh_token accordingly so callers handle the
optional/absent case rather than relying on a hardcoded string.
- Around line 5-11: minimumTokenResponseSchema currently uses invalid computed
keys (evaluating to "[object Object]") and should be replaced with a proper Zod
schema that explicitly validates known OAuth fields and permits other
properties; update minimumTokenResponseSchema to require access_token:
z.string(), expires_in: z.number().optional() (and token_type:
z.string().optional() if desired) and allow unknown keys via .passthrough() or
.catchall(z.unknown()) so real token responses are accepted. Also remove the
placeholder assignment that sets the refresh token to the literal string
"refresh_token" when missing (the code that assigns that fallback in
parseRefreshTokenResponse); instead return or throw an error / handle the
missing refresh token explicitly so no invalid token value persists.
In `@packages/app-store/salesforce/lib/CalendarService.ts`:
- Around line 96-99: The file uses the Prisma client (await
prisma.credential.update(...)) but never imports it; add an import for the
Prisma client at the top of CalendarService.ts matching how other calendar
services import it (the module that exports the shared prisma instance used by
office365calendar/zohocrm), e.g. import { prisma } from the shared Prisma client
module, and ensure the imported symbol name is prisma so the existing call to
prisma.credential.update resolves.
- Around line 75-87: In CalendarService.ts replace the current response check
that uses response.statusText with a check using response.ok (e.g., after the
fetch that sets const response) so successful HTTP/2 or non-200 2xx responses
are treated as success; if !response.ok, throw an HttpError that incorporates
response.status and response.statusText (or response.statusText fallback) to
preserve useful diagnostics while relying on response.ok for correctness.
In `@packages/app-store/zoho-bigin/lib/CalendarService.ts`:
- Around line 85-94: Call refreshOAuthTokens with the Cal.com user id
(credential.userId) instead of credentialId and normalize the returned token
payload so both AxiosResponse and fetch Response work: pass credential.userId
into the refreshOAuthTokens call, then after the call extract the token data by
checking for tokenInfo.data first, and if absent await tokenInfo.json() (and
handle a null/undefined result) before using the tokens; reference the
refreshOAuthTokens call site and the tokenInfo variable when making these
changes.
In `@packages/lib/constants.ts`:
- Around line 103-104: APP_CREDENTIAL_SHARING_ENABLED currently evaluates to the
raw encryption key string when both env vars exist; change it to return a
boolean instead by coercing the check for process.env.CALCOM_WEBHOOK_SECRET and
process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY (e.g., use Boolean(...) or
double-bang) so APP_CREDENTIAL_SHARING_ENABLED is true/false and never exposes
secret values.
| # - APP CREDENTIAL SYNC *********************************************************************************** | ||
| # Used for self-hosters that are implementing Cal.com into their applications that already have certain integrations | ||
| # Under settings/admin/apps ensure that all app secrets are set the same as the parent application | ||
| # You can use: `openssl rand -base64 32` to generate one | ||
| CALCOM_WEBHOOK_SECRET="" | ||
| # This is the header name that will be used to verify the webhook secret. Should be in lowercase | ||
| CALCOM_WEBHOOK_HEADER_NAME="calcom-webhook-secret" | ||
| CALCOM_CREDENTIAL_SYNC_ENDPOINT="" | ||
| # Key should match on Cal.com and your application | ||
| # must be 32 bytes for AES256 encryption algorithm | ||
| # You can use: `openssl rand -base64 24` to generate one | ||
| CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY="" |
There was a problem hiding this comment.
Fix dotenv-linter warnings for the new CALCOM_ entries.*
The linter flags QuoteCharacter and UnorderedKey for this block; reordering and removing quotes resolves it.
🛠️ Proposed fix
-CALCOM_WEBHOOK_SECRET=""
-# This is the header name that will be used to verify the webhook secret. Should be in lowercase
-CALCOM_WEBHOOK_HEADER_NAME="calcom-webhook-secret"
-CALCOM_CREDENTIAL_SYNC_ENDPOINT=""
-# Key should match on Cal.com and your application
-# must be 32 bytes for AES256 encryption algorithm
-# You can use: `openssl rand -base64 24` to generate one
-CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY=""
+# Key should match on Cal.com and your application
+# must be 32 bytes for AES256 encryption algorithm
+# You can use: `openssl rand -base64 24` to generate one
+CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY=
+CALCOM_CREDENTIAL_SYNC_ENDPOINT=
+# This is the header name that will be used to verify the webhook secret. Should be in lowercase
+CALCOM_WEBHOOK_HEADER_NAME=calcom-webhook-secret
+CALCOM_WEBHOOK_SECRET=📝 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.
| # - APP CREDENTIAL SYNC *********************************************************************************** | |
| # Used for self-hosters that are implementing Cal.com into their applications that already have certain integrations | |
| # Under settings/admin/apps ensure that all app secrets are set the same as the parent application | |
| # You can use: `openssl rand -base64 32` to generate one | |
| CALCOM_WEBHOOK_SECRET="" | |
| # This is the header name that will be used to verify the webhook secret. Should be in lowercase | |
| CALCOM_WEBHOOK_HEADER_NAME="calcom-webhook-secret" | |
| CALCOM_CREDENTIAL_SYNC_ENDPOINT="" | |
| # Key should match on Cal.com and your application | |
| # must be 32 bytes for AES256 encryption algorithm | |
| # You can use: `openssl rand -base64 24` to generate one | |
| CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY="" | |
| # - APP CREDENTIAL SYNC *********************************************************************************** | |
| # Used for self-hosters that are implementing Cal.com into their applications that already have certain integrations | |
| # Under settings/admin/apps ensure that all app secrets are set the same as the parent application | |
| # You can use: `openssl rand -base64 32` to generate one | |
| # Key should match on Cal.com and your application | |
| # must be 32 bytes for AES256 encryption algorithm | |
| # You can use: `openssl rand -base64 24` to generate one | |
| CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY= | |
| CALCOM_CREDENTIAL_SYNC_ENDPOINT= | |
| # This is the header name that will be used to verify the webhook secret. Should be in lowercase | |
| CALCOM_WEBHOOK_HEADER_NAME=calcom-webhook-secret | |
| CALCOM_WEBHOOK_SECRET= |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 237-237: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 239-239: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 239-239: [UnorderedKey] The CALCOM_WEBHOOK_HEADER_NAME key should go before the CALCOM_WEBHOOK_SECRET key
(UnorderedKey)
[warning] 240-240: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 240-240: [UnorderedKey] The CALCOM_CREDENTIAL_SYNC_ENDPOINT key should go before the CALCOM_WEBHOOK_HEADER_NAME key
(UnorderedKey)
[warning] 244-244: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 244-244: [UnorderedKey] The CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY key should go before the CALCOM_CREDENTIAL_SYNC_ENDPOINT key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.example around lines 233 - 244, The dotenv-linter warnings come from
quoted empty values and key ordering; update the CALCOM_* block by removing the
surrounding double quotes for the empty values (e.g., CALCOM_WEBHOOK_SECRET=,
CALCOM_WEBHOOK_HEADER_NAME=calcom-webhook-secret,
CALCOM_CREDENTIAL_SYNC_ENDPOINT=, CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY=) and
reorder the keys into the expected alphabetical order
(CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY, CALCOM_CREDENTIAL_SYNC_ENDPOINT,
CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET) so QuoteCharacter and
UnorderedKey warnings are resolved.
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | ||
| // Check that credential sharing is enabled | ||
| if (!APP_CREDENTIAL_SHARING_ENABLED) { | ||
| return res.status(403).json({ message: "Credential sharing is not enabled" }); | ||
| } | ||
|
|
||
| // Check that the webhook secret matches | ||
| if ( | ||
| req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] !== | ||
| process.env.CALCOM_WEBHOOK_SECRET | ||
| ) { | ||
| return res.status(403).json({ message: "Invalid webhook secret" }); | ||
| } | ||
|
|
||
| const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/pages/api/webhook/app-credential.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3523
🏁 Script executed:
# Check for other webhook handlers to see if method guards are common
find apps/web/pages/api/webhook -name "*.ts" -type f | head -10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 134
🏁 Script executed:
# Look at a few other webhook handlers to see patterns
head -35 apps/web/pages/api/webhook/*.ts | grep -E "(method|POST|safeParse|parse)" | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 162
🏁 Script executed:
# Check if there's a pattern of using safeParse vs parse in the codebase
rg "\.safeParse|\.parse\(" apps/web/pages/api --type ts -A 2 | head -40Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2696
🏁 Script executed:
# Check if there's a global error handler or middleware in the API routes
find apps/web/pages/api -name "_*.ts" -o -name "middleware.ts" | xargs cat 2>/dev/null | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Verify Zod parse behavior - check if parse throws
rg "parse\(" apps/web/pages/api --type ts -B 2 -A 3 | grep -E "(try|catch|parse)" | head -30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2955
🏁 Script executed:
# Check if other webhook handlers have method guards
fd "webhook" apps/web/pages/api --type f -name "*.ts" -exec grep -l "req.method" {} \;Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 320
🏁 Script executed:
# Check if webhook handlers have method guards
grep -r "req.method" apps/web/pages/api/webhook/ --include="*.ts"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Check the actual error handling in Next.js API routes
# Look at recorded-daily-video.ts which uses safeParse
cat -n apps/web/pages/api/recorded-daily-video.ts | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2222
🏁 Script executed:
# Verify JSON.parse error handling pattern
rg "JSON\.parse" apps/web/pages/api --type ts -B 2 -A 2 | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2559
Add method guard and handle validation errors without throwing.
Invalid request bodies or JSON parsing failures will throw unhandled exceptions and surface as 500s. Add a method guard and use safeParse to ensure webhook handlers return deterministic error responses.
🛠️ Proposed fix
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+ if (req.method !== "POST") {
+ return res.status(405).json({ message: "Method not allowed" });
+ }
// Check that credential sharing is enabled
if (!APP_CREDENTIAL_SHARING_ENABLED) {
return res.status(403).json({ message: "Credential sharing is not enabled" });
}
@@
- const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);
+ const parsed = appCredentialWebhookRequestBodySchema.safeParse(req.body);
+ if (!parsed.success) {
+ return res.status(400).json({ message: "Invalid request body" });
+ }
+ const reqBody = parsed.data;
// Check that the user exists
const user = await prisma.user.findUnique({ where: { id: reqBody.userId } });Also wrap the JSON.parse on line 57-59 in a try-catch:
- const keys = JSON.parse(
- symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
- );
+ let keys;
+ try {
+ keys = JSON.parse(
+ symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
+ );
+ } catch {
+ return res.status(400).json({ message: "Failed to decrypt credentials" });
+ }🤖 Prompt for AI Agents
In `@apps/web/pages/api/webhook/app-credential.ts` around lines 17 - 32, The
handler currently lacks an HTTP method guard and uses parse() which throws on
invalid JSON/body; update the exported handler to early-return 405 for non-POST
methods (check req.method === 'POST') and replace
appCredentialWebhookRequestBodySchema.parse(req.body) with safeParse and return
a 400 with validation errors when safeParse fails; additionally wrap any
explicit JSON.parse usage in the handler (the JSON.parse call you added) in a
try-catch and return a 400 with a clear message on parse errors instead of
letting exceptions bubble.
| // Decrypt the keys | ||
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/pages/api/webhook/app-credential.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3523
Add error handling for decryption and validate the encryption key.
symmetricDecrypt and JSON.parse can throw on invalid payloads or missing configuration, causing unhandled 500 errors. The encryption key must be explicitly validated before use.
🛠️ Proposed fix
- const keys = JSON.parse(
- symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "")
- );
+ const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
+ if (!encryptionKey) {
+ return res.status(500).json({ message: "Encryption key not configured" });
+ }
+ let keys: Record<string, unknown>;
+ try {
+ keys = JSON.parse(symmetricDecrypt(reqBody.keys, encryptionKey));
+ } catch {
+ return res.status(400).json({ message: "Invalid encrypted keys payload" });
+ }📝 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.
| // Decrypt the keys | |
| const keys = JSON.parse( | |
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | |
| ); | |
| // Decrypt the keys | |
| const encryptionKey = process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; | |
| if (!encryptionKey) { | |
| return res.status(500).json({ message: "Encryption key not configured" }); | |
| } | |
| let keys: Record<string, unknown>; | |
| try { | |
| keys = JSON.parse(symmetricDecrypt(reqBody.keys, encryptionKey)); | |
| } catch { | |
| return res.status(400).json({ message: "Invalid encrypted keys payload" }); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/pages/api/webhook/app-credential.ts` around lines 56 - 59, The code
currently calls symmetricDecrypt and JSON.parse without validating the
encryption key or catching errors; update the handler to first verify
process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY is present and non-empty, then
wrap the symmetricDecrypt(reqBody.keys, ...) and JSON.parse(...) sequence in a
try/catch; on failure log the error (including the exception message) and return
a client error (e.g., 400/422) rather than letting an unhandled 500 propagate.
Reference symmetricDecrypt, JSON.parse, reqBody.keys and
CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY when making the checks and error handling.
| const minimumTokenResponseSchema = z.object({ | ||
| access_token: z.string(), | ||
| // Assume that any property with a number is the expiry | ||
| [z.string().toString()]: z.number(), | ||
| // Allow other properties in the token response | ||
| [z.string().optional().toString()]: z.unknown().optional(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "parseRefreshTokenResponse.ts" | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 152
🏁 Script executed:
cat -n ./packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1344
🏁 Script executed:
rg -A 5 -B 5 "minimumTokenResponseSchema" --type tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2138
🏁 Script executed:
# Let's check what z.string().toString() actually returns
# First, let's look for any tests related to this schema
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "minimumTokenResponseSchema\|parseRefreshTokenResponse" 2>/dev/null | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Let's understand what z.string().toString() returns
# Check if there are any similar patterns in the codebase
rg "z\.string\(\)\.toString\(\)" --type tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 189
🏁 Script executed:
cd packages/app-store/_utils/oauth && node -e "
const { z } = require('zod');
console.log('z.string().toString():', z.string().toString());
console.log('z.string().optional().toString():', z.string().optional().toString());
// Test the schema
const minimumTokenResponseSchema = z.object({
access_token: z.string(),
[z.string().toString()]: z.number(),
[z.string().optional().toString()]: z.unknown().optional(),
});
console.log('\nSchema keys:', Object.keys(minimumTokenResponseSchema.shape));
// Test with a real token response
const testResponse = {
access_token: 'token123',
expires_in: 3600,
refresh_token: 'refresh123'
};
console.log('\nTesting with real response:', testResponse);
const result = minimumTokenResponseSchema.safeParse(testResponse);
console.log('Parse result:', result);
"
</parameter>Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1092
🏁 Script executed:
# Let me check what toString() returns for Zod objects
# First, let's look at the package.json to understand dependencies
cat packages/package.json | head -30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 143
🏁 Script executed:
# Find package.json files
find . -name "package.json" -type f | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 218
🏁 Script executed:
# Let me search for Zod documentation or examples in the codebase
rg "z\.object" --type ts -A 3 | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3415
🏁 Script executed:
# Let me search for how token responses look in the codebase
rg "access_token\|refresh_token\|expires_in" --type ts -B 2 -A 2 | head -80Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Let's verify what z.string().toString() actually produces
python3 << 'EOF'
# Simulate what happens with z.string().toString()
# In JavaScript, calling toString() on an object typically returns something like "[object Object]"
class MockZodSchema:
def __str__(self):
return "[object Object]"
def __repr__(self):
return "[object Object]"
mock_schema = MockZodSchema()
print("What z.string().toString() likely returns:", str(mock_schema))
# Now let's think about what the schema would look like:
schema_keys = {
"access_token": "z.string()",
"[object Object]": "z.number()", # This is the computed key from z.string().toString()
"[object Object]": "z.unknown().optional()" # This would overwrite the previous
}
print("\nSchema keys after evaluation:", list(schema_keys.keys()))
print("This means the schema expects these literal property names:")
print("1. 'access_token'")
print("2. '[object Object]'")
# A real token response would have:
real_token = {
"access_token": "token123",
"expires_in": 3600,
"token_type": "Bearer"
}
print("\nReal token response keys:", list(real_token.keys()))
print("\nWould schema match real token?", set(real_token.keys()) == set(schema_keys.keys()))
EOFRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 424
🏁 Script executed:
# Let's check if there are any tests or usages that show what the schema is supposed to do
rg "APP_CREDENTIAL_SHARING_ENABLED" --type ts -B 3 -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3992
minimumTokenResponseSchema uses invalid computed keys that won't match real token responses.
The expressions [z.string().toString()] and [z.string().optional().toString()] evaluate to literal property names "[object Object]" at runtime, so the schema will reject all valid OAuth token responses. Real responses have properties like access_token, expires_in, and token_type, which don't match these literal keys.
Additionally, line 26 assigns the placeholder string "refresh_token" when the refresh token is missing, which creates an invalid token that persists through the system.
🛠️ Proposed fix
-const minimumTokenResponseSchema = z.object({
- access_token: z.string(),
- // Assume that any property with a number is the expiry
- [z.string().toString()]: z.number(),
- // Allow other properties in the token response
- [z.string().optional().toString()]: z.unknown().optional(),
-});
+const minimumTokenResponseSchema = z
+ .object({ access_token: z.string() })
+ .catchall(z.unknown());And remove the placeholder fallback or return an error instead:
if (!refreshTokenResponse.data.refresh_token) {
- refreshTokenResponse.data.refresh_token = "refresh_token";
+ throw new Error("Missing refresh_token in response");
}🤖 Prompt for AI Agents
In `@packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts` around lines 5
- 11, minimumTokenResponseSchema currently uses invalid computed keys
(evaluating to "[object Object]") and should be replaced with a proper Zod
schema that explicitly validates known OAuth fields and permits other
properties; update minimumTokenResponseSchema to require access_token:
z.string(), expires_in: z.number().optional() (and token_type:
z.string().optional() if desired) and allow unknown keys via .passthrough() or
.catchall(z.unknown()) so real token responses are accepted. Also remove the
placeholder assignment that sets the refresh token to the literal string
"refresh_token" when missing (the code that assigns that fallback in
parseRefreshTokenResponse); instead return or throw an error / handle the
missing refresh token explicitly so no invalid token value persists.
| if (!refreshTokenResponse.data.refresh_token) { | ||
| refreshTokenResponse.data.refresh_token = "refresh_token"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the file
fd "parseRefreshTokenResponse" --type fRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 150
🏁 Script executed:
# Read the file to see the actual code
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1344
🏁 Script executed:
# Search for usages of this function to understand the impact
rg "parseRefreshTokenResponse" --type ts --type tsx -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 117
🏁 Script executed:
# Search for usages of parseRefreshTokenResponse without file type restriction
rg "parseRefreshTokenResponse" -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 5206
🏁 Script executed:
# Check if refresh_token is used in other places and how
rg "refresh_token" packages/app-store/_utils/oauth/ -B 1 -A 1Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 451
🏁 Script executed:
# Search for where the return value of parseRefreshTokenResponse is used
rg "parseRefreshTokenResponse" --type ts -lRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 372
🏁 Script executed:
# Let's examine how the parsed token is used in the calling code
# Look at the office365calendar usage since it showed the usage pattern
sed -n '1,80p' packages/app-store/office365calendar/lib/CalendarService.ts | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1523
🏁 Script executed:
# Get more context around the parseRefreshTokenResponse call in office365calendar
rg -A 10 "parseRefreshTokenResponse\(responseJson" packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 508
🏁 Script executed:
# Check what happens with the parsed token - specifically the refresh_token field
rg -A 20 "const tokenResponse = parseRefreshTokenResponse" packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 806
🏁 Script executed:
# Let's also check the zoom usage to see the pattern
rg -A 15 "const parsedToken = parseRefreshTokenResponse" packages/app-store/zoomvideo/lib/VideoApiAdapter.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 907
🏁 Script executed:
# Let's check the schema definitions to see if refresh_token is expected
rg "refreshTokenResponseSchema\|zoomRefreshedTokenSchema\|salesforceTokenSchema" packages/app-store/ -B 2 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Check if refresh_token is defined in minimumTokenResponseSchema
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1344
🏁 Script executed:
# Look for where these schemas are defined
find packages/app-store -name "*.ts" -type f | xargs grep -l "refreshTokenResponseSchema = \|zoomRefreshedTokenSchema = " | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 201
🏁 Script executed:
# Check office365 schema definition
rg "refreshTokenResponseSchema" packages/app-store/office365calendar/ -B 5 -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 4348
🏁 Script executed:
# Check the zoom and google schema definitions as well
rg "zoomRefreshedTokenSchema\|googleCredentialSchema\|salesforceTokenSchema" packages/app-store/ -B 2 -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Let's trace through a specific case - how is the fake token used if refresh_token is needed?
# Check if refresh_token is actually used after being set
rg "refresh_token" packages/app-store/office365calendar/ -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1599
🏁 Script executed:
# Check what happens in the zoom case with the fake token
rg "refresh_token" packages/app-store/zoomvideo/ -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2271
🏁 Script executed:
# Verify the context around where the fake token is used - check the refresh flow
rg -B 5 -A 5 "grant_type.*refresh_token" packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 523
Do not set missing refresh_token to a placeholder string.
This corrupts credentials stored in the database. The literal "refresh_token" string will be sent to OAuth providers (Microsoft, Zoom, etc.) on the next token refresh attempt, causing the request to fail with an invalid grant error and breaking the refresh flow.
Either use a caller-supplied fallback (from an existing stored credential) or leave refresh_token unset—schemas already mark it as optional where appropriate.
🛠️ Proposed fix
-const parseRefreshTokenResponse = (response: any, schema: z.ZodTypeAny) => {
+const parseRefreshTokenResponse = (
+ response: any,
+ schema: z.ZodTypeAny,
+ opts?: { fallbackRefreshToken?: string }
+) => {
@@
- if (!refreshTokenResponse.data.refresh_token) {
- refreshTokenResponse.data.refresh_token = "refresh_token";
- }
+ if (!refreshTokenResponse.data.refresh_token && opts?.fallbackRefreshToken) {
+ refreshTokenResponse.data.refresh_token = opts.fallbackRefreshToken;
+ }🤖 Prompt for AI Agents
In `@packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts` around lines 25
- 27, In parseRefreshTokenResponse, stop assigning the literal placeholder
"refresh_token" to refreshTokenResponse.data.refresh_token; this corrupts stored
credentials. Instead, remove that assignment so the field remains undefined when
absent, or if a fallback is available use a caller-supplied existing stored
credential value (pass it into parseRefreshTokenResponse and apply it only when
present). Update references to refreshTokenResponse.data.refresh_token
accordingly so callers handle the optional/absent case rather than relying on a
hardcoded string.
| const response = await fetch("https://login.salesforce.com/services/oauth2/token", { | ||
| method: "POST", | ||
| body: new URLSearchParams({ | ||
| grant_type: "refresh_token", | ||
| client_id: consumer_key, | ||
| client_secret: consumer_secret, | ||
| refresh_token: credentialKey.refresh_token, | ||
| format: "json", | ||
| }), | ||
| }); | ||
|
|
||
| if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText }); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/app-store/salesforce/lib/CalendarService.ts | sed -n '70,90p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1048
🌐 Web query:
Fetch API response.statusText different status codes behavior HTTP 200 201 202
💡 Result:
In the Fetch API, response.statusText is the HTTP “status message” / reason phrase associated with response.status (e.g., "OK" for 200). However, it’s not reliable across protocols and environments. [1][2]
Typical values (when a reason phrase is available, e.g., HTTP/1.1)
200→"OK"[3]201→"Created"[4]202→"Accepted"[5]
Why you may see "" (empty string)
- The Fetch standard defines a response’s status message default as empty, and
statusTextreturns that status message. [2] - HTTP/2 does not support reason phrases, so responses over HTTP/2 will have an empty status message (so
response.statusText === ""even ifresponse.statusis200/201/202). [1][2]
Practical guidance
Use response.status (number) and/or response.ok instead of statusText for logic; treat statusText as best-effort/debug-only. [2][1]
Sources: [1] MDN Response.statusText • [2] Fetch Standard • [3] MDN 200 OK • [4] MDN 201 Created • [5] MDN 202 Accepted
Use response.ok instead of statusText for error detection.
The statusText value is unreliable across protocols: HTTP/2 does not include reason phrases (resulting in empty string), and other 2xx responses have different phrases (e.g., "Created" for 201). The current code will incorrectly throw errors for successful responses over HTTP/2 or non-200 success codes.
🛠️ Proposed fix
- if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText });
+ if (!response.ok) {
+ const errorText = await response.text();
+ throw new HttpError({ statusCode: response.status, message: errorText || response.statusText });
+ }📝 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.
| const response = await fetch("https://login.salesforce.com/services/oauth2/token", { | |
| method: "POST", | |
| body: new URLSearchParams({ | |
| grant_type: "refresh_token", | |
| client_id: consumer_key, | |
| client_secret: consumer_secret, | |
| refresh_token: credentialKey.refresh_token, | |
| format: "json", | |
| }), | |
| }); | |
| if (response.statusText !== "OK") throw new HttpError({ statusCode: 400, message: response.statusText }); | |
| const response = await fetch("https://login.salesforce.com/services/oauth2/token", { | |
| method: "POST", | |
| body: new URLSearchParams({ | |
| grant_type: "refresh_token", | |
| client_id: consumer_key, | |
| client_secret: consumer_secret, | |
| refresh_token: credentialKey.refresh_token, | |
| format: "json", | |
| }), | |
| }); | |
| if (!response.ok) { | |
| const errorText = await response.text(); | |
| throw new HttpError({ statusCode: response.status, message: errorText || response.statusText }); | |
| } | |
🤖 Prompt for AI Agents
In `@packages/app-store/salesforce/lib/CalendarService.ts` around lines 75 - 87,
In CalendarService.ts replace the current response check that uses
response.statusText with a check using response.ok (e.g., after the fetch that
sets const response) so successful HTTP/2 or non-200 2xx responses are treated
as success; if !response.ok, throw an HttpError that incorporates
response.status and response.statusText (or response.statusText fallback) to
preserve useful diagnostics while relying on response.ok for correctness.
| await prisma.credential.update({ | ||
| where: { id: credential.id }, | ||
| data: { key: { ...accessTokenParsed.data, refresh_token: credentialKey.refresh_token } }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/app-store/salesforce/lib && wc -l CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 112
🏁 Script executed:
cd packages/app-store/salesforce/lib && head -30 CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 940
🏁 Script executed:
cd packages/app-store/salesforce/lib && rg "import.*prisma" CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
cd packages/app-store/salesforce/lib && sed -n '90,105p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 715
🏁 Script executed:
cd packages/app-store/salesforce/lib && rg "prisma" CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 126
🏁 Script executed:
cd packages/app-store/salesforce/lib && grep -n "import.*from.*prisma" CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
cd packages/app-store && find . -name "*.ts" -type f -exec grep -l "import prisma from.*@calcom/prisma" {} \; | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2438
🏁 Script executed:
cd packages/app-store && rg "import prisma from" --type ts | head -10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 754
🏁 Script executed:
cd packages/app-store && rg "CalendarService.ts" --type ts | grep -E "(import|salesforce|office365)" | head -10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
cd packages/app-store/office365calendar/lib && head -30 CalendarService.ts | grep -E "^import"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 953
🏁 Script executed:
cd packages/app-store/zohocrm/lib && head -30 CalendarService.ts | grep -E "^import"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 544
Add the missing Prisma import for the credential update.
The prisma object is used at lines 96-99 but not imported in this file, which will fail compilation. Other CalendarService implementations in the codebase (e.g., office365calendar, zohocrm) include this import.
Proposed fix
import { RRule } from "rrule";
import { z } from "zod";
+import prisma from "@calcom/prisma";🤖 Prompt for AI Agents
In `@packages/app-store/salesforce/lib/CalendarService.ts` around lines 96 - 99,
The file uses the Prisma client (await prisma.credential.update(...)) but never
imports it; add an import for the Prisma client at the top of CalendarService.ts
matching how other calendar services import it (the module that exports the
shared prisma instance used by office365calendar/zohocrm), e.g. import { prisma
} from the shared Prisma client module, and ensure the imported symbol name is
prisma so the existing call to prisma.credential.update resolves.
| const tokenInfo = await refreshOAuthTokens( | ||
| async () => | ||
| await axios.post(accountsUrl, qs.stringify(formData), { | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded;charset=utf-8", | ||
| }, | ||
| }), | ||
| "zoho-bigin", | ||
| credentialId | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its structure
find . -type f -name "CalendarService.ts" | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 378
🏁 Script executed:
# Search for refreshOAuthTokens definition
rg -n "refreshOAuthTokens" --type ts --type tsx -A 5 | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 117
🏁 Script executed:
# Read the specific file around the mentioned lines
fd "CalendarService.ts" --exec cat -n {} \; | sed -n '75,110p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1491
🏁 Script executed:
# Find the zoho-bigin CalendarService.ts file
find . -path "*zoho-bigin*" -name "CalendarService.ts" -type fRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 144
🏁 Script executed:
# Also search for refreshOAuthTokens to understand its definition
rg -n "export.*refreshOAuthTokens|function refreshOAuthTokens|const refreshOAuthTokens" -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1432
🏁 Script executed:
# Search for the zoho-bigin directory structure
find . -path "*zoho-bigin*" -type f | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 681
🏁 Script executed:
cat -n ./packages/app-store/zoho-bigin/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 12695
🏁 Script executed:
cat -n ./packages/app-store/_utils/oauth/refreshOAuthTokens.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1094
🏁 Script executed:
# Verify that CredentialPayload has userId field
rg "type CredentialPayload|interface CredentialPayload" -A 15Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 659
🏁 Script executed:
# Check if credential.userId is used elsewhere in zoho-bigin or similar services
rg "credential\.userId" packages/app-store/ | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1153
🏁 Script executed:
# Check how zohocrm and other similar services pass userId to refreshOAuthTokens
rg -B 5 -A 5 "refreshOAuthTokens.*credential\.userId" packages/app-store/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Also verify the CredentialPayload structure from the Prisma select
cat packages/prisma/selects/credential.ts | grep -A 50 "credentialForCalendarServiceSelect"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 637
🏁 Script executed:
# Search for how zohocrm and other services call refreshOAuthTokens
rg "refreshOAuthTokens" packages/app-store/zohocrm/ -B 3 -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1289
🏁 Script executed:
# Also check hubspot and googlecalendar
rg "refreshOAuthTokens" packages/app-store/hubspot/ packages/app-store/googlecalendar/ -B 3 -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2834
🏁 Script executed:
# Get more context on zohocrm, hubspot and googlecalendar calls to refreshOAuthTokens
rg "refreshOAuthTokens\(" packages/app-store/zohocrm/lib/CalendarService.ts -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 456
🏁 Script executed:
rg "refreshOAuthTokens\(" packages/app-store/hubspot/lib/CalendarService.ts -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 496
🏁 Script executed:
rg "refreshOAuthTokens\(" packages/app-store/googlecalendar/lib/CalendarService.ts -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 527
🏁 Script executed:
# Get the full refreshOAuthTokens call in zohocrm to see what it passes as third param
rg "refreshOAuthTokens\(" packages/app-store/zohocrm/lib/CalendarService.ts -A 15 | head -25Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 676
🏁 Script executed:
# Get the full refreshOAuthTokens call in hubspot to see what it passes as third param
rg "refreshOAuthTokens\(" packages/app-store/hubspot/lib/CalendarService.ts -A 15 | head -25Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 696
🏁 Script executed:
# Check how googlecalendar handles the response
rg -B 2 -A 5 "const token = res\?\.data" packages/app-store/googlecalendar/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 908
🏁 Script executed:
# Check hubspot response handling more carefully - why does it access properties directly?
cat -n packages/app-store/hubspot/lib/CalendarService.ts | sed -n '50,100p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2170
🏁 Script executed:
# Look at the hubspot HubspotToken type definition
rg "type HubspotToken" packages/app-store/hubspot/ -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Look for HubspotToken definition in callback.ts
cat -n packages/app-store/hubspot/api/callback.ts | head -100Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2665
🏁 Script executed:
# Also check how hubspot's refreshAccessToken is being called and what it returns
rg -B 10 "const hubspotRefreshToken" packages/app-store/hubspot/lib/CalendarService.ts | head -30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 513
🏁 Script executed:
# Confirm the issue by checking what happens when refreshOAuthTokens returns a fetch Response
# Let me see if there's any response normalization in other services
rg "parseRefreshTokenResponse|\.json\(\)" packages/app-store/googlecalendar/lib/CalendarService.ts -B 3 -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 822
🏁 Script executed:
# Check if there's any other handling of fetch Response in the codebase
rg "response\.json\(\)" packages/app-store/_utils/oauth/ -B 3 -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Check the APP_CREDENTIAL_SHARING_ENABLED constant
rg "APP_CREDENTIAL_SHARING_ENABLED" packages/lib/constants.ts -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 305
🏁 Script executed:
# Look at parseRefreshTokenResponse to understand the pattern
cat -n packages/app-store/_utils/oauth/parseRefreshTokenResponse.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1344
🏁 Script executed:
# Final verification: confirm that credential.userId exists in zoho-bigin's constructor
cat -n packages/app-store/zoho-bigin/lib/CalendarService.ts | sed -n '43,65p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 999
🏁 Script executed:
# Verify that the proposed fix logic for response normalization is sound
# by checking what a fetch Response looks like vs AxiosResponse
rg "const tokenData.*data.*json" packages/app-store/ --type ts -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
Pass credential.userId and normalize response handling to support credential sharing.
refreshOAuthTokens expects a Cal.com userId, but credentialId is passed instead. More critically, when APP_CREDENTIAL_SHARING_ENABLED is true, the helper returns a fetch Response object (not an AxiosResponse), so accessing tokenInfo.data will fail with a TypeError. The code must handle both response types.
🔧 Proposed fix
private biginAuth(credential: CredentialPayload) {
const credentialKey = credential.key as unknown as BiginToken;
const credentialId = credential.id;
+ const credentialUserId = credential.userId ?? null;
@@
- : this.refreshAccessToken(credentialId, credentialKey),
+ : this.refreshAccessToken(credentialId, credentialKey, credentialUserId),
};
}
@@
- private async refreshAccessToken(credentialId: number, credentialKey: BiginToken) {
+ private async refreshAccessToken(
+ credentialId: number,
+ credentialKey: BiginToken,
+ credentialUserId: number | null
+ ) {
@@
- const tokenInfo = await refreshOAuthTokens(
+ const tokenInfo = await refreshOAuthTokens(
async () =>
await axios.post(accountsUrl, qs.stringify(formData), {
headers: {
"Content-Type": "application/x-www-form-urlencoded;charset=utf-8",
},
}),
"zoho-bigin",
- credentialId
+ credentialUserId
);
- if (!tokenInfo.data.error) {
+ const tokenData = "data" in tokenInfo ? tokenInfo.data : await tokenInfo.json();
+ if (!tokenData.error) {
// set expiry date as offset from current time.
- tokenInfo.data.expiryDate = Math.round(Date.now() + tokenInfo.data.expires_in);
- tokenInfo.data.accountServer = credentialKey.accountServer;
- tokenInfo.data.refresh_token = credentialKey.refresh_token;
+ tokenData.expiryDate = Math.round(Date.now() + tokenData.expires_in);
+ tokenData.accountServer = credentialKey.accountServer;
+ tokenData.refresh_token = credentialKey.refresh_token;
@@
- data: {
- key: tokenInfo.data as BiginToken,
- },
+ data: { key: tokenData as BiginToken },
});
- this.log.debug("Fetched token", tokenInfo.data.access_token);
+ this.log.debug("Fetched token", tokenData.access_token);
} else {
- this.log.error(tokenInfo.data);
+ this.log.error(tokenData);
}
- return tokenInfo.data as BiginToken;
+ return tokenData as BiginToken;
}🤖 Prompt for AI Agents
In `@packages/app-store/zoho-bigin/lib/CalendarService.ts` around lines 85 - 94,
Call refreshOAuthTokens with the Cal.com user id (credential.userId) instead of
credentialId and normalize the returned token payload so both AxiosResponse and
fetch Response work: pass credential.userId into the refreshOAuthTokens call,
then after the call extract the token data by checking for tokenInfo.data first,
and if absent await tokenInfo.json() (and handle a null/undefined result) before
using the tokens; reference the refreshOAuthTokens call site and the tokenInfo
variable when making these changes.
| export const APP_CREDENTIAL_SHARING_ENABLED = | ||
| process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; |
There was a problem hiding this comment.
Coerce to boolean to avoid exporting a secret value.
Line 103-104 currently resolves to the actual encryption key string when enabled, which risks accidental exposure if this constant is ever serialized or bundled client-side. Please coerce to a boolean.
🔒 Proposed fix
-export const APP_CREDENTIAL_SHARING_ENABLED =
- process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY;
+export const APP_CREDENTIAL_SHARING_ENABLED = Boolean(
+ process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY
+);📝 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 const APP_CREDENTIAL_SHARING_ENABLED = | |
| process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY; | |
| export const APP_CREDENTIAL_SHARING_ENABLED = Boolean( | |
| process.env.CALCOM_WEBHOOK_SECRET && process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY | |
| ); |
🤖 Prompt for AI Agents
In `@packages/lib/constants.ts` around lines 103 - 104,
APP_CREDENTIAL_SHARING_ENABLED currently evaluates to the raw encryption key
string when both env vars exist; change it to return a boolean instead by
coercing the check for process.env.CALCOM_WEBHOOK_SECRET and
process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY (e.g., use Boolean(...) or
double-bang) so APP_CREDENTIAL_SHARING_ENABLED is true/false and never exposes
secret values.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit