OAuth credential sync and app integration enhancements

Golden recall: 100% Extra findings: 36

Golden Comments (5/5 found)

#	Comment	Severity	Status	FriendlyReviewer Detail
#1	The `parseRefreshTokenResponse` function incorrectly sets `refresh_token` to the hardcoded string `'refresh_token'` when it's missing from the OAuth refresh token response. This invalidates the token, breaking subsequent token refreshes and causing authentication failures.	HIGH	✓ Found	"When the sync response omits `refresh_token`, the function writes the literal string `"refresh_token"` into the data. Consumers persist this placeholder into the credential key (webex VideoApiAdapter.ts:105, zoomvideo VideoApiAdapter.ts:121), so if sharing is later disabled the credential tries to refresh with the literal `"refresh_token"` and fails. Keep the field absent and let consumers handle the sharing-mode case explicitly" (packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts L26)
#2	Invalid Zod schema syntax. Computed property keys like `[z.string().toString()]` are not valid in Zod object schemas and will cause runtime errors.	HIGH	✓ Found	"`minimumTokenResponseSchema` is broken: `z.string().toString()` does not create catch-all keys — both computed properties collapse to the literal key `"[object Object]"`, so the schema effectively validates only `access_token` and never enforces the numeric-expiry assumption. As a result, with sharing enabled the office365calendar refresh path (CalendarService.ts:262) fails validation every time and throws. Use `z.record(...)`/`.passthrough()` + `.superRefine` and require `access_token` and the expiry field explicitly" (packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts L8)
#3	`parseRefreshTokenResponse` returns a Zod `safeParse` result (`{ success, data, error }`), not the credential key object. Persisting that as key stores the wrapper instead of the token payload; we should store the parsed data or use schema parse.	HIGH	✓ Found	"The function returns the full `SafeParseReturnType` envelope (`{ success, data?, error? }`) while mutating `data`, diverging from the previous `.parse()` contract. Consumers now use it inconsistently: googlecalendar persists the whole envelope as the credential key (CalendarService.ts:98) instead of the token data, corrupting the stored Google key so the next `googleCredentialSchema.parse` throws and breaks all calendar operations. Align the return type with the `.parse()` contract (`return data` and throw on failure) and update all callers" (packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts L29)
#4	When `APP_CREDENTIAL_SHARING_ENABLED` and `CALCOM_CREDENTIAL_SYNC_ENDPOINT` are set, the `refreshFunction` helper returns the fetch `Response`, but several callers (for example `GoogleCalendarService.refreshAccessToken` expecting `res.data`, and `HubspotCalendarService.refreshAccessToken` expecting a `HubspotToken`) assume it returns the integration-specific token object. That mismatch will cause runtime errors in the sync-enabled path unless the return type or those call sites are adjusted.	HIGH	✓ Found	"`refreshOAuthTokens` returns a raw `fetch` `Response` in the sharing-enabled branch but the arbitrary `refreshFunction()` result in fallback mode. Every consumer expects an axios-like `.data` shape, so the sharing branch crashes and never refreshes: `token.access_token` throws TypeError in googlecalendar (CalendarService.ts:95), `tokenInfo.data.error` in zoho-bigin (CalendarService.ts:84), `zohoCrmTokenInfo.data.error` in zohocrm (CalendarService.ts:217), plus hubspot, lark, webex, and zoom. Normalize the return type so both branches produce the same shape (or let callers branch on the sync path)" (packages/app-store/_utils/oauth/refreshOAuthTokens.ts L8)
#5	When the sync endpoint path is used, `res` is a fetch `Response` and has no `.data`; `res?.data` will be undefined and `token.access_token` will throw at runtime. This relies on a consistent return shape from `refreshOAuthTokens`, which isn't guaranteed currently.	HIGH	✓ Found	"Same root cause as golden #4: in sync mode `res` is the raw fetch `Response`, `res?.data` is undefined, and `token.access_token` throws at runtime. The review also traces the HubSpot damage: the `Response` is treated as a `HubspotToken` — `expiresIn` is undefined so `expiryDate` becomes NaN, the `Response` is persisted as the credential key (serializes to `{}`, permanently destroying the stored token), and `setAccessToken(undefined)` is called. The sync response is also snake_case (`access_token`/`expires_in`) vs HubSpot's camelCase `accessToken`/`expiresIn`" (packages/app-store/_utils/oauth/refreshOAuthTokens.ts L8; packages/app-store/hubspot/lib/CalendarService.ts L177)

Supplementary Findings (36 findings)

Finding	File	Severity	Legitimate?
Webhook secret compared with a plain `!==` (non-constant-time), enabling a timing side-channel on the only authorization of an endpoint that creates/overwrites credentials	app-credential.ts:25	HIGH	✓ Real — timing-unsafe secret comparison on a credential-creating endpoint
The forwarded POST to `CALCOM_CREDENTIAL_SYNC_ENDPOINT` sends only `calcomUserId` and `appSlug` — no webhook secret header and no identifier for the credential/refresh token	refreshOAuthTokens.ts:8	HIGH	✓ Real — the forward is unauthenticated and unresolvable by the receiver
With `state.teamId` present, the ownership check is only `members.some({ userId, accepted: true })` — any accepted team member (not an admin) can create a team-level credential	createOAuthAppCredential.ts:33	HIGH	✓ Real — privilege escalation via caller-controlled state
Lark's refresh keeps its own parsing instead of `parseRefreshTokenResponse`: the sync `Response` is fed to `handleLarkError` (requires `code === 0`, absent from sync bodies) so it always throws; the flat shape also mismatches Lark's `data.data` envelope	larkcalendar/CalendarService.ts:78	HIGH	✓ Real — sync mode is broken for Lark
Refactored `refreshAccessToken` replaces the entire stored credential `key` with the sync body — `refresh_token` (and `scope`/`token_type`/`email`) permanently lost; local MS Graph refresh breaks if sharing is later disabled	office365video/VideoApiAdapter.ts:93	HIGH	✓ Real — permanent credential field loss
New block unconditionally calls the Salesforce token endpoint and writes the DB on every `getClient()` (every event op), while the jsforce connection is still built from the stale `credentialKey.access_token` — redundant refresh on the hot path	salesforce/CalendarService.ts:106	HIGH	✓ Real — hot-path load with zero benefit
`parseRefreshTokenResponse` switches to `minimumTokenResponseSchema` whenever sharing is enabled, but Salesforce always refreshes against the real endpoint — validation would fail on every refresh and silently break Salesforce	salesforce/CalendarService.ts:87	HIGH	✓ Real — sharing flag wrongly gates real-endpoint validation
Stripe callback reads `returnTo` from the attacker-controlled `req.query.state` and redirects to it directly with no allowlist — an open redirect that can also forward the OAuth `code`; the sole callback not using `getSafeRedirectUrl`	stripepayment/callback.ts:51	HIGH	✓ Real — open redirect plus code forwarding
Existing Tandem credentials are deleted unconditionally before the `if (result.ok)` success check — a failed token exchange silently destroys previously-working credentials	tandemvideo/callback.ts:53	HIGH	✓ Real — silent data loss on exchange failure
Webex refresh validates with the strict `webexRefreshedTokenSchema` (requires `scope`/`token_type`/`refresh_token`/`refresh_token_expires_in`) while the sync endpoint returns only `access_token` + expiry — refresh broken for synced Webex credentials	webex/VideoApiAdapter.ts:90	HIGH	✓ Real — strict schema vs minimal sync shape
`getToken` parses the stored key with the strict `webexTokenSchema`; a credential created through the webhook (only `access_token` + `expiry_date`) is rejected and can't be used at all	webex/VideoApiAdapter.ts:116	HIGH	✓ Real — webhook-created Webex credentials unusable
`refreshOAuthTokens(refreshFunction, appSlug, userId)` is called with `credentialId` (the Credential primary key) instead of `credential.userId` — the sync endpoint cannot resolve the right credential	zoho-bigin/CalendarService.ts:92	HIGH	✓ Real — wrong identifier sent to the receiver
`accountsUrl` built directly from the caller-supplied `accounts-server` query param, then `axios.post` sends `client_id`, `client_secret` and the code to it — SSRF + secret/code exfiltration to an arbitrary host	zoho-bigin/callback.ts:35	HIGH	✓ Real — SSRF with secret exfiltration (pre-existing, but in scope)
Token-exchange URL built from the unvalidated `accounts-server` param and persisted verbatim, later used to POST `client_secret` and the victim's `refresh_token` — crafted callback URL exfiltrates both	zohocrm/callback.ts:33	HIGH	✓ Real — SSRF/secret exfiltration, pre-existing but in scope
Sync branch returns `expiry_date` (not `expires_in`), so `key.expiry_date = Math.round(Date.now() + newTokens.expires_in * 1000)` becomes NaN (serialized null) — token permanently expired, refresh from the sync endpoint on every meeting op	zoomvideo/VideoApiAdapter.ts:122	HIGH	✓ Real — expiry corrupted in sync mode
`appCredentialWebhookRequestBodySchema.parse(req.body)` throws an unhandled `ZodError` on invalid input — 500 response leaking Zod internals	app-credential.ts:31	MEDIUM	✓ Real — unhandled error on malformed payloads
`symmetricDecrypt(...)` then `JSON.parse(...)` can throw (wrong key, malformed ciphertext, non-JSON plaintext) with no try/catch — unhandled 500; falling back to `""` for the missing key turns a config error into a confusing cipher error	app-credential.ts:57	MEDIUM	✓ Real — bad ciphertext must not 500
Credential lookup + create/update is not atomic: concurrent webhook deliveries for the same (userId, appId) can both create duplicate `Credential` rows	app-credential.ts:62	MEDIUM	✓ Real — duplicate-insert race
`JSON.parse(req.query.state)` unguarded — malformed `state` throws an unhandled `SyntaxError` out of every callback (unauthenticated 500/DoS); parsed state never validated against `IntegrationOAuthCallbackState`, and `state.teamId` is used for ownership with no signature/origin binding	decodeOAuthState.ts:8	MEDIUM	✓ Real — unauthenticated 500/DoS plus unbound ownership
`encodeOAuthState` merely re-stringifies the parsed `req.query.state` (a no-op round-trip), does not build/encode any state, and throws the same unhandled `SyntaxError` on malformed input	encodeOAuthState.ts:8	MEDIUM	✓ Real — dead utility with the same unguarded parse
In the non-team path, `throwIfNotHaveAdminAccessToTeam({ teamId: state?.teamId ?? null, ... })` is dead code (teamId is always null there); the team branch throws a bare `Error` instead of `HttpError` — generic 500 instead of the intended 403	createOAuthAppCredential.ts:43	MEDIUM	✓ Real — misleading dead code, wrong error status
`APP_CREDENTIAL_SHARING_ENABLED` evaluates to the value of `CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY` (a secret used for AES-256 decryption) — inferred type `string \| undefined`, not boolean, and the exported constant carries the raw encryption-key value (latent leak if logged)	constants.ts:102	MEDIUM	✓ Real — a boolean flag that exposes a secret
Callback error path redirects to a caller-controlled `state?.returnTo` without sanitization — craft `?state={"returnTo":"https://evil.example"}` and force an error to get a 302 to the attacker host	larkcalendar/callback.ts:100	MEDIUM	✓ Real — open redirect on the catch path (pre-existing)
Office365 Calendar callback still creates its credential directly and ignores `teamId` from the OAuth state, unlike every other callback migrated to `createOAuthAppCredential` — team installs attach to the wrong owner and skip the admin-access check	office365calendar/callback.ts:72	MEDIUM	✓ Real — diverges from every other migrated callback
Callback deletes all user-level `office365_video` credentials, then the shared util creates a team-scoped credential when `state.teamId` is present — the user is left with zero personal credentials	office365video/callback.ts:95	MEDIUM	✓ Real — owner-scope mismatch
Salesforce refresh is a direct `fetch` and never goes through the shared `refreshOAuthTokens` — not gated on `APP_CREDENTIAL_SHARING_ENABLED`/`CALCOM_CREDENTIAL_SYNC_ENDPOINT`, inconsistent with every other app	salesforce/CalendarService.ts:72	MEDIUM	✓ Borderline: real inconsistency, but Salesforce may intentionally be excluded
When the token exchange fails (`result.ok === false`) the handler falls through with no `res.status(...).json(...)` and no redirect — the client request hangs until timeout	tandemvideo/callback.ts:63	MEDIUM	✓ Real — missing error response, hung request
Hand-rolled dedup deletes only user-scoped credentials before calling the shared `createOAuthAppCredential` — if the flow ever passes a `teamId`, duplicates remain and the user's own credential is deleted	tandemvideo/callback.ts:43	MEDIUM	✓ Borderline: requires a future teamId to trigger
`zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60)` adds 3600 seconds to a millisecond `Date.now()` — token appears to expire ~3.6s after issuance; `isTokenValid` comparison also inverted	zohocrm/CalendarService.ts:219	MEDIUM	✓ Real — unit mismatch makes every token instantly stale
Sync-endpoint `Response` fed through Zoom-specific `handleZoomResponse` — invalidation on `invalid_grant` won't fire in sync mode, and a non-JSON error body would throw a confusing parse error	zoomvideo/VideoApiAdapter.ts:93	MEDIUM	✓ Real — wrong shape into Zoom-specific parsing
Duplicate-avoidance `deleteMany` is scoped to `userId` only, but the shared util creates a team-scoped credential when OAuth state carries a `teamId` — re-authorizing creates a second team credential	zoomvideo/callback.ts:71	MEDIUM	✓ Real — no-duplicate guarantee broken for team installs
Token-refresh parse-failure path changed from best-effort (log Zod error + raw response, keep old token) to a bare throw with no diagnostic context — removes diagnosability for transient bad responses	office365calendar/CalendarService.ts:262	LOW	✓ Real — removes diagnosability, but only on failure
`if (response.statusText !== "OK")` relies on the HTTP reason phrase, unreliable across runtimes/HTTP2, and the non-OK body is never inspected	salesforce/CalendarService.ts:83	LOW	✓ Real — fragile check, minor
`if (code === undefined && typeof code !== "string")` only rejects `undefined` — an array `code` (`?code=a&code=b`) bypasses the guard and is passed as a string, producing a confusing failure	salesforce/callback.ts:12	LOW	✓ Real — should be `\|\|`; minor edge case
Stripe hand-rolls its own `returnTo` parsing (`getReturnToValueFromQueryState`) instead of the shared `decodeOAuthState` — swallows errors silently and returns an unvalidated bare string	stripepayment/callback.ts:13	LOW	✓ Borderline: consistency nit, same effect today
`if (code === undefined && typeof code !== "string")` is redundant and wrong for the array case — `?code=a&code=b` bypasses the guard and the array is passed to the token exchange	zohocrm/callback.ts:18	LOW	✓ Real — wrong operator; minor edge case

Honest Analysis

FriendlyReviewer found all 5 golden comments of this PR — recall 100% — and three of them land in the same shared utility: the literal "refresh_token" placeholder (L26), the malformed Zod schema whose computed keys collapse to "[object Object]" (L8), and the SafeParseReturnType envelope persisted as a credential key (L29). Each golden is matched with its concrete consumer-level consequence: the placeholder ends up in the webex/zoom keys, the broken schema makes office365calendar fail validation on every refresh, and the envelope corrupts the stored Google key so googleCredentialSchema.parse throws.

The two remaining goldens target the sync-mode contract mismatch, and the review resolves them as one root cause with two victims. It identifies the raw fetch Response returned by refreshOAuthTokens in the sharing branch (L8) and enumerates the crashing consumers (google, zoho-bigin, zohocrm, hubspot, lark, webex, zoom); it then traces the worst damage in HubSpot (L177), where the Response is treated as a HubspotToken — expiryDate becomes NaN, the key is serialized to {}, and the token is permanently lost. That level of damage tracing is exactly what a review of a credential-sync feature needs.

The 36 supplementary findings (15 high, 16 medium, 5 low) form the strongest part of the review. The security cluster — the non-constant-time webhook secret comparison (app-credential.ts L25), the unauthenticated and unresolvable forward to the sync endpoint, the SSRF plus secret exfiltration via the unvalidated accounts-server parameter (zoho-bigin L35, zohocrm L33), and the Stripe open redirect — is arguably more critical than any single golden, and it is precisely the class of defect a credential-sync feature cannot afford. The per-app regression set (salesforce hot-path refresh, office365video key replacement, webex/zoom strict schemas vs the minimal sync shape) is precise and actionable.

Caveats: 41 findings on one PR is a very large surface, and several items are explicitly pre-existing (the zoho SSRF, the lark catch-path redirect, the salesforce guards) — the review marks them as such rather than claiming this PR introduced them. The volume also risks burying the three contract-level defects that alone justify blocking the merge. The review's own summary is the honest bottom line: with the sharing branch returning a raw Response, an unauthenticated forward, and a schema that never validates expiry, the feature is non-functional in exactly the environment it targets — and the changes-required verdict is correct.

FriendlyReviewer's full review

As posted on the PR — the complete output from FriendlyReviewer.

friendly-reviewer bot commented now

Hi there 👋

🌥️ Tech	🌥️ Feat
19 high, 17 medium, 5 low	3 issues, 2 mitigated

This MR enables self-hosters to sync 3rd-party app credentials with Cal.com via a new webhook receiver (api/webhook/app-credential), a set of shared OAuth utilities under packages/app-store/_utils/oauth/, and a migration of ~12 OAuth-based calendar/video apps onto those utilities. The overall architecture is sound (shared-secret gate, env-flag branching, consistent import relocation, turbo/env wiring) and the import/rename refactor is verified clean with no dangling references.

However, the implementation is not merge-ready. The shared utilities have contract-level and security defects that break the feature in exactly the environment it targets, and several app call sites introduce regressions to existing (non-synced) functionality:

Security: the webhook secret is compared non-constant-time; the refresh forward to the sync endpoint carries no secret header/credential identifier; open-redirects remain in the stripe and lark callbacks; zoho-bigin/zohocrm build the token endpoint from an unvalidated accounts-server query param (SSRF + client_secret/refresh_token exfiltration); APP_CREDENTIAL_SHARING_ENABLED carries the raw encryption key instead of a boolean; decodeOAuthState/encodeOAuthState raise unhandled exceptions.
Shared-util contracts: refreshOAuthTokens returns a raw fetch Response in sync mode but an arbitrary function result in fallback mode, crashing at every consumer (google, hubspot, zoho-bigin, zohocrm, lark, webex, zoom); parseRefreshTokenResponse returns a SafeParseReturnType envelope (corrupting the persisted Google credential key), mutates refresh_token to a literal placeholder, and validates against a malformed minimumTokenResponseSchema so expiry is never actually checked; webhook find-then-create is non-atomic.
Per-app regressions: sync-mode refresh corrupts stored expiry (zoom, zohocrm, hubspot always-expired); salesforce/office365video refresh paths overwrite or drop credentials and run unconditially on the hot path; several callbacks have data-loss, missing-response, or team/user dedup issues.

All sub-reviews independently recommended changes. The MR should not be merged until the shared OAuth utilities are fixed and the app call sites are made consistent with their contracts, and security issues (constant-time compare, SSRF, open redirects, secret handling) are resolved.

These might need a close look

🔴 apps/web/pages/api/webhook/app-credential.ts (L25)
Webhook secret is compared with a plain !== (non-constant-time), enabling a timing side-channel to brute-force the shared secret that is the only authorization on an endpoint which creates/overwrites credentials. Use crypto.timingSafeEqual on equal-length buffers (and guard against an unset CALCOM_WEBHOOK_SECRET).
🔴 packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts (L8)
minimumTokenResponseSchema is broken: z.string().toString() does not create catch-all keys — both computed properties collapse to the literal key "[object Object]", so the schema effectively validates only access_token and never enforces the numeric-expiry assumption. As a result, with sharing enabled the office365calendar refresh path (CalendarService.ts:262) fails validation every time and throws. Use z.record(...)/.passthrough() + .superRefine and require access_token and the expiry field explicitly.
🔴 packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts (L29)
The function returns the full SafeParseReturnType envelope ({ success, data?, error? }) while mutating data, diverging from the previous .parse() contract. Consumers now use it inconsistently: googlecalendar persists the whole envelope as the credential key (CalendarService.ts:98) instead of the token data, corrupting the stored Google key so the next googleCredentialSchema.parse throws and breaks all calendar operations. Align the return type with the .parse() contract (return data and throw on failure) and update all callers.
🔴 packages/app-store/_utils/oauth/refreshOAuthTokens.ts (L8)
refreshOAuthTokens returns a raw fetch Response in the sharing-enabled branch but the arbitrary refreshFunction() result in fallback mode. Every consumer expects an axios-like .data shape, so the sharing branch crashes and never refreshes: token.access_token throws TypeError in googlecalendar (CalendarService.ts:95), tokenInfo.data.error in zoho-bigin (CalendarService.ts:84), zohoCrmTokenInfo.data.error in zohocrm (CalendarService.ts:217), plus hubspot, lark, webex, and zoom. Normalize the return type so both branches produce the same shape (or let callers branch on the sync path).
🔴 packages/app-store/_utils/oauth/refreshOAuthTokens.ts (L8)
The forwarded POST to CALCOM_CREDENTIAL_SYNC_ENDPOINT sends only calcomUserId and appSlug via URLSearchParams — no webhook secret header and no identifier for the specific credential/refresh token. A self-hoster receiver cannot authenticate the request (the webhook in this MR rejects requests without the secret) nor resolve which credential to refresh; left unauthenticated this is a spoofing risk. Send the same secret header and the refresh token/credential id.
🔴 packages/app-store/_utils/oauth/createOAuthAppCredential.ts (L33)
When state.teamId is present, the ownership check is only members.some({ userId, accepted: true }) — any accepted team member (not an admin) can create a team-level credential, and since state is caller-controlled this lets a member attach a credential to their team. Reuse getUserAdminTeams/throwIfNotHaveAdminAccessToTeam with the resolved state.teamId.
🔴 packages/app-store/hubspot/lib/CalendarService.ts (L177)
In the sharing branch refreshOAuthTokens returns a raw Response, which HubSpot treats as a HubspotToken: expiresIn is undefined so expiryDate becomes NaN, the Response is persisted as the credential key (serializes to {} — permanently destroying the stored token, unrecoverable without re-auth), and setAccessToken(undefined) is called. The sync response is also snake_case (access_token/expires_in) vs HubSpot's camelCase accessToken/expiresIn. Guard against undefined, normalize the shape, and surface/log failures instead of persisting a malformed key. Also note isTokenValid at line 173 compares expiresIn seconds against epoch-ms Date.now(), so the token is always considered expired and a refresh (hitting the sync endpoint when enabled) runs on every meeting op.
🔴 packages/app-store/larkcalendar/lib/CalendarService.ts (L78)
Lark's refresh wraps the call in refreshOAuthTokens but keeps Lark-specific downstream parsing instead of using parseRefreshTokenResponse. In the sharing branch the sync Response is fed to handleLarkError, which requires code === 0 (the sync body has no code) so it always throws; even if it didn't, the flat {access_token, expiry_date} shape doesn't match Lark's data.data.{refresh_token, refresh_expires_in, access_token, expires_in} envelope, so a broken credential would be written. Route the sharing response through parseRefreshTokenResponse/shape it into LarkAuthCredentials.
🔴 packages/app-store/office365video/lib/VideoApiAdapter.ts (L93)
The refactored refreshAccessToken replaces the entire stored credential key with responseBody. In the sync branch the body contains only access_token + expiry, so refresh_token (and scope/token_type/email) is permanently lost — if sharing is later disabled the local MS Graph refresh breaks. This diverges from the office365calendar sibling which merges fields. Merge instead of replace ({ ...o365AuthCredentials, ...responseBody }). Also reconcile expiry_date computed from expires_in vs the sync endpoint's absolute expiry: if no expires_in is returned, expiry_date becomes Date.now() and the token is immediately expired.
🔴 packages/app-store/salesforce/lib/CalendarService.ts (L106)
The new block unconditionally calls the Salesforce token endpoint and writes the DB on every getClient() (every event create/update/delete/availability), whereas previously the connection was built lazily. Worse, the freshly refreshed token is never used — the jsforce connection is built from the stale credentialKey.access_token, so the explicit refresh is redundant and load-inducing. Build the connection from the refreshed token.
🔴 packages/app-store/salesforce/lib/CalendarService.ts (L87)
parseRefreshTokenResponse switches to minimumTokenResponseSchema whenever sharing is enabled, but Salesforce always refreshes against the real endpoint (which returns id, issued_at, instance_url, signature, etc.). Enabling sharing would make validation fail on every refresh and silently break Salesforce. The refresh validation must not depend on the sharing flag when the sync endpoint is not used.
🔴 packages/app-store/stripepayment/api/callback.ts (L51)
The Stripe callback reads returnTo from the attacker-controlled req.query.state and redirects to it directly with no allowlist/sanitization — an open redirect that can also forward the OAuth code to an attacker origin. Every other OAuth callback in this MR guards this with getSafeRedirectUrl; Stripe is the sole exception. Use res.redirect(getSafeRedirectUrl(state?.returnTo) ?? getInstalledAppPath({ variant: "payment", slug: "stripe" })).
🔴 packages/app-store/tandemvideo/api/callback.ts (L53)
Existing Tandem credentials are deleted unconditionally before the if (result.ok) token-exchange success check. If the exchange fails, the user's previously-working credential(s) are already deleted with no replacement and no error surfaced — silent data loss. Move the deletion inside the success branch.
🔴 packages/app-store/webex/lib/VideoApiAdapter.ts (L90)
Webex refresh validates the response with webexRefreshedTokenSchema.safeParse(...) directly instead of the shared parseRefreshTokenResponse. The strict schema requires scope/token_type/refresh_token/refresh_token_expires_in, but the sync endpoint returns only access_token + expiry, so refresh (and hence every meeting operation) is broken for synced Webex credentials. Use parseRefreshTokenResponse(responseBody, webexRefreshedTokenSchema).
🔴 packages/app-store/webex/lib/VideoApiAdapter.ts (L116)
getToken parses the stored key with webexTokenSchema, which requires scope/token_type/refresh_token/refresh_token_expires_in. A credential created through the app-credential webhook contains only access_token + expiry_date, so webexTokenSchema.parse rejects and the credential can't be used at all. If Webex is meant to be syncable, the read schema must tolerate the minimal sync shape (or Webex should be excluded from sync).
🔴 packages/app-store/zoho-bigin/lib/CalendarService.ts (L92)
refreshOAuthTokens(refreshFunction, appSlug, userId) is called with credentialId (the Credential primary key) instead of the user id. Every other app passes credential.userId. The sharing branch is entered but the sync endpoint receives the wrong identifier, so the self-hosted platform can neither resolve nor refresh the right credential. Pass credential.userId.
🔴 packages/app-store/zoho-bigin/api/callback.ts (L35)
accountsUrl = `${accountsServer}/oauth/v2/token` is built directly from the caller-supplied accounts-server query param, then axios.post sends the app client_id, client_secret and authorization code to it — SSRF + secret/code exfiltration to an arbitrary host. Restrict accounts-server to a known allowlist (e.g. accounts.zoho.com and regional hosts). (Pre-existing, but the callback is under review and the shared credential path now centralizes creation.)
🔴 packages/app-store/zohocrm/api/callback.ts (L33)
The token-exchange URL is built from the unvalidated accounts-server query param and persisted verbatim (accountServer), later used to POST client_id, client_secret and the victim's refresh_token. A crafted callback URL (?accounts-server=https://attacker.example) exfiltrates the OAuth secret and refresh token. Validate against a fixed allowlist of Zoho account hosts before use and before persistence.
🔴 packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (L122)
In the sync branch the endpoint returns expiry_date (not expires_in), so key.expiry_date = Math.round(Date.now() + newTokens.expires_in * 1000) becomes NaN (serialized to null), and isTokenValid treats the token as permanently expired — refreshing from the sync endpoint on every meeting op. Derive expiry from the returned expiry_date (guard expires_in as fallback).

Worth checking

🟡 apps/web/pages/api/webhook/app-credential.ts (L31)
appCredentialWebhookRequestBodySchema.parse(req.body) throws an unhandled ZodError on invalid/malformed input, returning 500 and leaking Zod internals. Use .safeParse() and return 400 on failure.
🟡 apps/web/pages/api/webhook/app-credential.ts (L57)
symmetricDecrypt(...) followed by JSON.parse(...) can throw (wrong key, malformed ciphertext, non-JSON plaintext) with no try/catch, producing an unhandled 500. Wrap both and return 400. Also, falling back to "" for the missing encryption key turns a config error into a confusing cipher error.
🟡 apps/web/pages/api/webhook/app-credential.ts (L62)
Credential lookup + create/update is not atomic: concurrent webhook deliveries for the same (userId, appId) can both create duplicate Credential rows. Use prisma.credential.upsert on a unique constraint (and confirm a DB unique constraint on (userId, appId)/(teamId, appId)) or wrap lookup+write in a transaction.
🟡 packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts (L26)
When the sync response omits refresh_token, the function writes the literal string "refresh_token" into the data. Consumers persist this placeholder into the credential key (webex VideoApiAdapter.ts:105, zoomvideo VideoApiAdapter.ts:121), so if sharing is later disabled the credential tries to refresh with the literal "refresh_token" and fails. Keep the field absent and let consumers handle the sharing-mode case explicitly.
🟡 packages/app-store/_utils/oauth/decodeOAuthState.ts (L8)
JSON.parse(req.query.state) is unguarded: malformed state throws an unhandled SyntaxError out of every callback (unauthenticated 500/DoS). Wrap in try/catch. Additionally, the parsed state is never validated against IntegrationOAuthCallbackState and state.teamId is used for credential ownership without any signature/origin binding, so a caller who controls state in the callback URL can influence teamId for credential creation.
🟡 packages/app-store/_utils/oauth/encodeOAuthState.ts (L8)
encodeOAuthState merely re-stringifies the parsed req.query.state (a no-op round-trip), does not build/encode any state, and throws the same unhandled SyntaxError on malformed input. If meant to build the OAuth state, it should construct/validate the payload and encode it; at minimum guard the JSON.parse.
🟡 packages/app-store/_utils/oauth/createOAuthAppCredential.ts (L43)
Two issues: (1) in the non-team path, throwIfNotHaveAdminAccessToTeam({ teamId: state?.teamId ?? null, ... }) is dead code (teamId is always null there) and misleading; (2) the team branch throws a bare Error("User does not belong to the team") instead of HttpError, so the app-store error handler returns a generic 500 instead of the intended 403.
🟡 packages/lib/constants.ts (L102)
APP_CREDENTIAL_SHARING_ENABLED evaluates to the value of CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY (a secret used for AES-256 decryption), so the inferred type is string | undefined, not boolean, and the exported constant carries the raw encryption-key value (latent leak if ever logged/serialized). Coerce with !!(...) — the same truthiness semantics without exposing the key.
🟡 packages/app-store/larkcalendar/api/callback.ts (L100)
The callback error path redirects to a caller-controlled state?.returnTo without sanitization (res.redirect(state?.returnTo ?? url)), an open-redirect vector: craft ?state={"returnTo":"https://evil.example"} and force an error to get a 302 to the attacker host. The success path already uses getSafeRedirectUrl; apply it to the catch path too. (Pre-existing, but in the OAuth-state path under review.)
🟡 packages/app-store/office365calendar/api/callback.ts (L72)
The office365 Calendar callback still creates its credential directly and ignores teamId from the OAuth state, unlike every other callback in this PR which migrates to the shared createOAuthAppCredential (which applies team/admin handling). Team installs would attach the credential to the wrong owner and skip the admin-access check. Migrate to createOAuthAppCredential for consistency or document the omission.
🟡 packages/app-store/office365video/api/callback.ts (L95)
The callback performs single-credential-per-user cleanup (deletes all user-level office365_video credentials) and then calls the shared createOAuthAppCredential, which creates a team-scoped credential (no userId) when state.teamId is present. In that team path the user is left with zero personal credentials while a team one is created. Scope the delete to the intended owner (user vs team) based on the decoded state before calling the shared util.
🟡 packages/app-store/salesforce/lib/CalendarService.ts (L72)
Salesforce refresh is a direct fetch and never goes through the shared refreshOAuthTokens, so it is not gated on APP_CREDENTIAL_SHARING_ENABLED/CALCOM_CREDENTIAL_SYNC_ENDPOINT — inconsistent with every other app in this MR. Route through the shared util or intentionally document why Salesforce is excluded.
🟡 packages/app-store/tandemvideo/api/callback.ts (L63)
When the token exchange fails (result.ok === false) the handler falls through with no res.status(...).json(...) and no redirect, so the client request hangs until timeout. Add an else branch returning a proper error.
🟡 packages/app-store/tandemvideo/api/callback.ts (L43)
The hand-rolled dedup deletes only user-scoped credentials and then calls the shared createOAuthAppCredential. If the flow is ever extended to pass a teamId, the shared util creates a team credential while this cleanup only deletes the user's personal credentials — leaving duplicates and deleting the user's own credential. Centralize dedup with creation (or scope the delete to the credential being replaced).
🟡 packages/app-store/zohocrm/lib/CalendarService.ts (L219)
zohoCrmTokenInfo.data.expiryDate = Math.round(Date.now() + 60 * 60) adds 3600 seconds to a millisecond Date.now() — the token appears to expire ~3.6s after issuance. Also isTokenValid uses token.expiryDate < Date.now() (inverted), so nearly every operation triggers a full OAuth refresh. Fix the unit (60 * 60 * 1000) and the validity comparison.
🟡 packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (L93)
The sync-endpoint Response is fed through Zoom-specific handleZoomResponse, which assumes a Zoom body. In sync mode credential invalidation on invalid_grant won't fire (the sync endpoint won't emit Zoom's error shape), and a non-JSON error body would throw a confusing parse error. Normalize the sync response to a known shape (and map sync failure to invalidation) before it reaches handleZoomResponse.
🟡 packages/app-store/zoomvideo/api/callback.ts (L71)
The duplicate-avoidance deleteMany is scoped to userId only, but the newly introduced createOAuthAppCredential creates a team-scoped credential when OAuth state carries a teamId. For a team install, re-authorizing Zoom creates a second team credential without removing the first — the no-duplicate guarantee no longer holds. Account for team-scoped credentials in the cleanup.

Small things (take or leave)

🔵 packages/app-store/office365calendar/lib/CalendarService.ts (L262)
The token-refresh parse-failure path was changed from best-effort (log the Zod error + raw response and keep the old token) to a bare throw with no diagnostic context ("Invalid refreshed tokens were returned"). This removes all diagnosability for transient bad responses. Surface the parse error/response body (or retain a console.error) instead of a silent hard-fail.
🔵 packages/app-store/salesforce/lib/CalendarService.ts (L83)
if (response.statusText !== "OK") relies on the HTTP reason phrase, which is unreliable across runtimes/HTTP2, and the non-OK body is never inspected. Prefer if (!response.ok) and log/parse the error body so real refresh failures (400 invalid_grant, 401) are diagnosable.
🔵 packages/app-store/salesforce/api/callback.ts (L12)
if (code === undefined && typeof code !== "string") only rejects undefined; an array code (?code=a&code=b) bypasses the guard and is passed as a string to requestToken, producing a confusing failure. Should be || instead of &&.
🔵 packages/app-store/stripepayment/api/callback.ts (L13)
Stripe hand-rolls its own returnTo parsing (getReturnToValueFromQueryState) instead of the shared decodeOAuthState that this PR centralizes and every other callback now uses. The custom parser swallows errors silently and returns an unvalidated bare string. Adopt the shared decodeOAuthState(req) for consistency and typed state.
🔵 packages/app-store/zohocrm/api/callback.ts (L18)
if (code === undefined && typeof code !== "string") is redundant and wrong for the array case: ?code=a&code=b bypasses the guard and the array is passed to the token exchange. Use if (typeof code !== "string").

✅ Feature-level checklist

Not there yet

❌ Self-hosters with their own app store can sync their users' 3rd-party credentials with Cal.com; the webhook receives the correct webhook secret + encrypted payload and the new credentials are created in the DB (app store as source of truth).
The webhook cannot yet be relied on: secret comparison is timing-unsafe, invalid/decryptable payloads produce unhandled 500s, credential creation is non-atomic (duplicate-insert race), team credential creation only checks membership (not admin), and the refresh callback to the sync endpoint is unauthenticated. Referenced by technical_findings 1-4, 9, 12, 13.
❌ For affected apps, forcing a token refresh hits the self-hoster endpoint and successfully refreshes the synced credentials (the endpoint simulates what a self-hoster would have made).
The sharing branch returns a raw fetch Response whose shape is incompatible with all consumers, the sync forward lacks the secret header/credential identifier, and the malformed minimumTokenResponseSchema causes validation to fail. The feature is non-functional in the exact environment it targets across google, hubspot, zoho-bigin, zohocrm, lark, webex, zoom, and office365calendar. Referenced by technical_findings 5, 8, 9, 15-17, 32, 39, 40.
❌ Checklist: 'I haven't added tests that prove my fix is effective or that my feature works' — tests are required to validate the new webhook and refresh behavior.
No automated tests were added for the webhook, the shared OAuth utils, or the per-app refresh integration. Given the cross-cutting contract defects, tests covering the sharing and non-sharing refresh paths are needed to prevent regressions.

Partially covered

⚠️ Forcing a token refresh without enabling app-credential syncing must keep existing functionality working (no regressions in the non-sync path).
Non-sync paths for most apps are preserved, but the MR introduces regressions: parseRefreshTokenResponse persisting the SafeParseReturnType envelope corrupts the Google credential key, salesforce refresh now runs unconditionally on the hot path with a stale token, and office365video/zohocrm refresh paths overwrite/lose credential fields. Referenced by technical_findings 7, 20, 22, 38.
⚠️ The MR is a breaking change and 'this change requires a documentation update' (new environment variables for feature configuration).
The four new env vars were added to .env.example and turbo.json globalEnv (verified consistent and safe defaults), but the PR body does not document the endpoint contract (webhook header/encryption, sync endpoint response shape, refresh handshake) and the documented sync contract is not actually honored by the code. Referenced by technical_finding 14 and finding 9.