OAuth credential sync and app integration enhancements

[CodingKylo]

Martian Code Review Benchmark PR (mirrored from source #8)

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 96/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🔴 Critical

Intent

Add a shared credential-sync mechanism that can refresh OAuth tokens via an external endpoint and ingest encrypted credential keys through a new webhook.

Summary

Behaviorally, this PR introduces a new webhook endpoint that authenticates via a shared secret header, decrypts an encrypted payload, and writes credential keys to the database. It also refactors multiple provider refresh flows to optionally route refresh through a shared refreshOAuthTokens helper, and adds new Zod parsing utilities for token/webhook payloads. The top risks are (1) contract/shape mismatches between refreshOAuthTokens and the existing provider code paths (likely runtime crashes or incorrect token persistence), and (2) webhook ingestion that can overwrite credential keys—so authorization, input validation, and encryption/key handling must be airtight. Reviewers must verify the refresh response contract end-to-end across providers and confirm the webhook cannot be abused to write arbitrary credential keys for other apps/users.

🎯 Review Focus

The webhook credential ingestion path and the refresh-token contract: verify that (1) the webhook cannot be abused to overwrite credential keys for arbitrary users/apps (stronger auth + strict decrypted payload validation), and (2) refreshOAuthTokens returns a normalized, validated token shape that every provider caller uses safely before dereferencing.

Key Findings

• 🚨 [apps/web/pages/api/webhook/app-credential.ts:L17-L55] CRITICAL: Authorization is only a shared-secret header comparison, with no replay protection and no request authenticity beyond a static secret. If the secret leaks (logs, misconfig, referrer, etc.), an attacker can decrypt/submit payloads and overwrite credential keys for any user/app slug they can reference. Fix: use a stronger scheme than static header equality—e.g., HMAC signature over the raw request body with a timestamp/nonce, and reject if timestamp is outside a small window or nonce was seen. Concretely: compute expected = HMAC_SHA256(secret, rawBody + timestamp) and compare using constant-time compare; require X-Signature and X-Timestamp headers; store/track nonces (or use timestamp-only with short TTL).
• 🚨 [apps/web/pages/api/webhook/app-credential.ts:L57-L93] CRITICAL: The webhook decrypts and then persists decrypted key material without validating the decrypted structure against a strict schema before writing to Prisma. This creates a direct data-integrity/credential-integrity risk: malformed or attacker-controlled decrypted JSON can be stored as credential keys, potentially breaking downstream OAuth flows or enabling credential corruption. Fix: after symmetricDecrypt, parse the decrypted JSON and validate it with a Zod schema that matches the exact credential key shape expected by the target app (or at minimum validate required fields and types). Only then persist. Also ensure you never persist arbitrary extra fields unless explicitly allowed by schema (use .strict() or .passthrough(false) semantics).
• ⚠️ [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L3-L18] WARNING: refreshOAuthTokens returns a raw fetch Response when credential-sync is enabled, but callers (e.g., Google) still assume a parsed { data: { access_token, expiry_date } } shape. This will cause runtime failures (res?.data undefined) or incorrect token persistence. Fix: make refreshOAuthTokens normalize the response contract. For example, always return a typed object like { access_token: string; expiry_date: number } (or { data: ... }) by doing const r = await fetch(...); const json = await r.json(); return json; and ensure the same shape is returned in both branches. Then update all callers to use the normalized return type.
• ⚠️ [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L3-L18] WARNING: The gating condition uses userId truthiness (&& userId) which will skip credential-sync for valid userId values like 0 (if ever possible) and also makes the behavior dependent on nullability semantics. Fix: change the condition to userId !== null && userId !== undefined and ensure the webhook schema and DB schema align with the same nullability expectations.

✅ Action Checklist

• [ ] CRITICAL [apps/web/pages/api/webhook/app-credential.ts:L17-L55] — Authorization is only a shared-secret header comparison, with no replay protection and no request authenticity beyond a static secret. If the secret leaks (logs, misconfig, referrer, etc.), an attacker can decrypt/submit payloads and overwrite credential keys for any user/app slug they can reference. Fix: use a stronger scheme than static header equality—e.g., HMAC signature over the raw request body with a timestamp/nonce, and reject if timestamp is outside a small window or nonce was seen. Concretely: compute expected = HMAC_SHA256(secret, rawBody + timestamp) and compare using constant-time compare; require X-Signature and X-Timestamp headers; store/track nonces (or use timestamp-only with short TTL).
• [ ] CRITICAL [apps/web/pages/api/webhook/app-credential.ts:L57-L93] — The webhook decrypts and then persists decrypted key material without validating the decrypted structure against a strict schema before writing to Prisma. This creates a direct data-integrity/credential-integrity risk: malformed or attacker-controlled decrypted JSON can be stored as credential keys, potentially breaking downstream OAuth flows or enabling credential corruption. Fix: after symmetricDecrypt, parse the decrypted JSON and validate it with a Zod schema that matches the exact credential key shape expected by the target app (or at minimum validate required fields and types). Only then persist. Also ensure you never persist arbitrary extra fields unless explicitly allowed by schema (use .strict() or .passthrough(false) semantics).
• [ ] WARNING [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L3-L18] — refreshOAuthTokens returns a raw fetch Response when credential-sync is enabled, but callers (e.g., Google) still assume a parsed { data: { access_token, expiry_date } } shape. This will cause runtime failures (res?.data undefined) or incorrect token persistence. Fix: make refreshOAuthTokens normalize the response contract. For example, always return a typed object like { access_token: string; expiry_date: number } (or { data: ... }) by doing const r = await fetch(...); const json = await r.json(); return json; and ensure the same shape is returned in both branches. Then update all callers to use the normalized return type.
• [ ] WARNING [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L3-L18] — The gating condition uses userId truthiness (&& userId) which will skip credential-sync for valid userId values like 0 (if ever possible) and also makes the behavior dependent on nullability semantics. Fix: change the condition to userId !== null && userId !== undefined and ensure the webhook schema and DB schema align with the same nullability expectations.
• [ ] SUGGESTION — [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L8-L22] Replace the broken dynamic-key Zod logic with an explicit refinement that checks for access_token and at least one numeric expiry field. Example: const minimum = z.object({ access_token: z.string() }).passthrough().superRefine((obj, ctx)=>{ const hasNumeric = Object.values(obj).some(v=>typeof v==='number'); if(!hasNumeric) ctx.addIssue({code:'custom', message:'Missing numeric expiry'}); });
• [ ] SUGGESTION — [packages/app-store/googlecalendar/lib/CalendarService.ts:L83-L99] Add a hard validation boundary for the refresh-sync response before mutating googleCredentials. Example: const token = parseRefreshTokenResponse(res, googleCredentialSchema).data; (or whatever the normalized contract is) and only then assign fields.
• [ ] SUGGESTION — [packages/app-store/office365calendar/lib/CalendarService.ts:L246] Verify the closure variable usage: ensure credential.userId is in scope and passed explicitly into the refresh callback. If the callback currently relies on an outer credential variable that may not exist in that scope, refactor the call site to pass userId as an argument to the callback.
• [ ] SUGGESTION — [apps/web/pages/api/webhook/app-credential.ts:L1-L93] Validate req.body types more strictly: appSlug should be constrained (e.g., .min(1).max(100) and regex for allowed characters) and keys should be validated as base64/hex (depending on symmetricDecrypt expectations) to avoid decrypt/parse exceptions and to reduce attack surface. Also ensure req.body is the raw body if signature/HMAC is implemented.
• [ ] SUGGESTION — [apps/web/pages/api/webhook/app-credential.ts:L17-L55] Use constant-time comparison for secrets (Node crypto.timingSafeEqual) and handle missing env vars explicitly at startup (fail fast). Example: if CALCOM_WEBHOOK_SECRET or CALCOM_WEBHOOK_HEADER_NAME is undefined, throw during module init rather than silently comparing against undefined.

Suggestions

• [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L8-L22] Replace the broken dynamic-key Zod logic with an explicit refinement that checks for access_token and at least one numeric expiry field. Example: const minimum = z.object({ access_token: z.string() }).passthrough().superRefine((obj, ctx)=>{ const hasNumeric = Object.values(obj).some(v=>typeof v==='number'); if(!hasNumeric) ctx.addIssue({code:'custom', message:'Missing numeric expiry'}); });
• [packages/app-store/googlecalendar/lib/CalendarService.ts:L83-L99] Add a hard validation boundary for the refresh-sync response before mutating googleCredentials. Example: const token = parseRefreshTokenResponse(res, googleCredentialSchema).data; (or whatever the normalized contract is) and only then assign fields.
• [packages/app-store/office365calendar/lib/CalendarService.ts:L246] Verify the closure variable usage: ensure credential.userId is in scope and passed explicitly into the refresh callback. If the callback currently relies on an outer credential variable that may not exist in that scope, refactor the call site to pass userId as an argument to the callback.
• [apps/web/pages/api/webhook/app-credential.ts:L1-L93] Validate req.body types more strictly: appSlug should be constrained (e.g., .min(1).max(100) and regex for allowed characters) and keys should be validated as base64/hex (depending on symmetricDecrypt expectations) to avoid decrypt/parse exceptions and to reduce attack surface. Also ensure req.body is the raw body if signature/HMAC is implemented.
• [apps/web/pages/api/webhook/app-credential.ts:L17-L55] Use constant-time comparison for secrets (Node crypto.timingSafeEqual) and handle missing env vars explicitly at startup (fail fast). Example: if CALCOM_WEBHOOK_SECRET or CALCOM_WEBHOOK_HEADER_NAME is undefined, throw during module init rather than silently comparing against undefined.

📝 This review includes 3 inline comments (3 warnings)

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

⚠️ WARNING

The dynamic Zod keys here do not create a catch-all validator; they become literal property names, so the schema still only validates a fixed set of keys. That means the intended “allow other properties” behavior is not actually implemented. The root cause is using object-literal computed keys for schema shape when this needs .passthrough()/.catchall() semantics instead.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

This change now routes token refreshes through refreshOAuthTokens, which can return the raw fetch response from the credential-sync endpoint. The code still assumes res.data exists and immediately dereferences token.access_token/token.expiry_date, so a nonconforming endpoint response will now fail at runtime. The underlying issue is that the new cross-service refresh path does not normalize the response shape before existing provider-specific parsing.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

refreshOAuthTokens is called with credential.userId, but this method still depends on credential being in scope inside the closure. If that variable is not guaranteed by the surrounding method, this will crash when the refresh path is entered. The root cause is that the new helper was threaded into provider code without making the credential dependency explicit at the call site.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 98/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🔴 Critical

Intent

Add a shared OAuth refresh + credential-sync mechanism (optionally via a webhook endpoint) and update multiple app calendar services to use it.

Summary

The PR introduces a new webhook endpoint to receive encrypted credential keys and a shared OAuth refresh helper that can optionally POST to a credential-sync endpoint instead of calling the provider directly. The biggest risks are (1) a security-critical webhook authentication/authorization gap (the handler only checks a shared secret header and then updates credentials for any userId/appSlug), and (2) correctness/security issues in token parsing and refresh-token handling (including mutating/synthesizing refresh_token values and overly-permissive token schema parsing). Before merging, verify the webhook is properly authenticated and authorized per user/app, and that token normalization never fabricates refresh tokens or accepts malformed token responses.

🎯 Review Focus

The webhook endpoint authorization model and the refresh-token normalization contract: verify the webhook cannot update arbitrary users/apps without strong per-request authentication, and verify refreshOAuthTokens returns a consistent, validated token shape (no fabricated refresh_token, no raw fetch Response).

Key Findings

• 🚨 [apps/web/pages/api/webhook/app-credential.ts:L1] CRITICAL: Webhook only checks a shared secret header, then trusts reqBody.userId/appSlug to look up and update records — enabling cross-user credential injection if the secret is leaked or guessed.

Evidence:

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);

const user = await prisma.user.findUnique({ where: { id: reqBody.userId } });
...
const app = await prisma.app.findUnique({ where: { slug: reqBody.appSlug }, select: { slug: true } });

Why it’s a problem: This endpoint performs a privileged state change (credential key decryption + persistence, per the rest of the handler) based solely on a static shared secret and caller-provided userId/appSlug. If the webhook secret is compromised (or misconfigured), an attacker can inject credentials for arbitrary users/apps. Even without compromise, there is no additional authorization binding (e.g., verifying the sender is allowed to act for that user/app).
Fix: Bind the webhook request to the specific user/app and enforce authorization at the DB layer. Concretely: (a) include a signed payload (HMAC/JWT) that covers userId + appSlug + a nonce/timestamp, and verify signature server-side; (b) additionally verify that the appSlug corresponds to an integration that the user actually has (or that the credential row exists for that user/app) before updating. Example approach:

• Change request schema to include timestamp, nonce, and signature.
• Verify signature = HMAC(secret, JSON.stringify({userId, appSlug, timestamp, nonce})).
• Before updating, require prisma.credential.findFirst({ where: { userId, appSlug }}) (or equivalent) and only update that row.
• 🚨 [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L1] WARNING: Token response validation is overly permissive and can accept malformed/hostile shapes (e.g., any numeric property name becomes an expiry candidate), increasing the chance of persisting incorrect token data.

Evidence:

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(),
});

Why it’s a problem: This schema effectively says “any string key maps to a number”, which is not what OAuth token responses look like. It can cause safeParse to succeed for unexpected payloads and then callers may read token.expiry_date / other fields that aren’t actually present or are wrong. This is a correctness + security boundary issue because provider responses are untrusted external input.
Fix: Replace with a strict schema that validates the exact fields you use (e.g., access_token, expires_in or expiry_date, and optionally refresh_token). Example:

const minimumTokenResponseSchema = z.object({
  access_token: z.string(),
  expires_in: z.number().int().positive().optional(),
  expiry_date: z.number().optional(),
  refresh_token: z.string().optional(),
}).passthrough();

Then in callers, compute expiry_date deterministically from expires_in if needed, rather than relying on “any numeric property”.

• 🚨 [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] CRITICAL: Shared refresh helper returns the raw fetch Response without parsing JSON, but callers treat it as if it were the provider SDK return shape (res?.data). This will break refresh flows at runtime.

Evidence:

const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
  method: "POST",
  body: new URLSearchParams({
    calcomUserId: userId.toString(),
    appSlug,
  }),
});
return response;

And in callers (example Google):

const res = await refreshOAuthTokens(...);
const token = res?.data;
googleCredentials.access_token = token.access_token;

Why it’s a problem: fetch() returns a Response object, which does not have .data. This will make token undefined and then token.access_token will throw (guaranteed crash) or silently set undefined values depending on optional chaining usage elsewhere.
Fix: Make refreshOAuthTokens normalize the return type. For example:

const response = await fetch(...);
if (!response.ok) throw new Error(`Credential sync failed: ${response.status}`);
const json = await response.json();
return json; // ensure it matches expected shape

Then update callers to use the normalized shape (or change refreshOAuthTokens to return { data: token } consistently).

• ⚠️ [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] WARNING: Sync gating uses userId truthiness, which will skip syncing for valid userId=0 (and generally relies on truthy semantics rather than explicit null/undefined checks).

Evidence:

if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {

Why it’s a problem: If userId can ever be 0 (or if types drift), syncing will be incorrectly disabled. It’s a latent correctness bug.
Fix: Use explicit null/undefined check:

if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId !== null && userId !== undefined) {

✅ Action Checklist

• [ ] CRITICAL [apps/web/pages/api/webhook/app-credential.ts:L1] — Webhook only checks a shared secret header, then trusts reqBody.userId/appSlug to look up and update records — enabling cross-user credential injection if the secret is leaked or guessed.

Evidence:

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);

const user = await prisma.user.findUnique({ where: { id: reqBody.userId } });
...
const app = await prisma.app.findUnique({ where: { slug: reqBody.appSlug }, select: { slug: true } });

Why it’s a problem: This endpoint performs a privileged state change (credential key decryption + persistence, per the rest of the handler) based solely on a static shared secret and caller-provided userId/appSlug. If the webhook secret is compromised (or misconfigured), an attacker can inject credentials for arbitrary users/apps. Even without compromise, there is no additional authorization binding (e.g., verifying the sender is allowed to act for that user/app).
Fix: Bind the webhook request to the specific user/app and enforce authorization at the DB layer. Concretely: (a) include a signed payload (HMAC/JWT) that covers userId + appSlug + a nonce/timestamp, and verify signature server-side; (b) additionally verify that the appSlug corresponds to an integration that the user actually has (or that the credential row exists for that user/app) before updating. Example approach:

• Change request schema to include timestamp, nonce, and signature.
• Verify signature = HMAC(secret, JSON.stringify({userId, appSlug, timestamp, nonce})).
• Before updating, require prisma.credential.findFirst({ where: { userId, appSlug }}) (or equivalent) and only update that row.
• [ ] CRITICAL [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] — Shared refresh helper returns the raw fetch Response without parsing JSON, but callers treat it as if it were the provider SDK return shape (res?.data). This will break refresh flows at runtime.

Evidence:

const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {
  method: "POST",
  body: new URLSearchParams({
    calcomUserId: userId.toString(),
    appSlug,
  }),
});
return response;

And in callers (example Google):

const res = await refreshOAuthTokens(...);
const token = res?.data;
googleCredentials.access_token = token.access_token;

Why it’s a problem: fetch() returns a Response object, which does not have .data. This will make token undefined and then token.access_token will throw (guaranteed crash) or silently set undefined values depending on optional chaining usage elsewhere.
Fix: Make refreshOAuthTokens normalize the return type. For example:

const response = await fetch(...);
if (!response.ok) throw new Error(`Credential sync failed: ${response.status}`);
const json = await response.json();
return json; // ensure it matches expected shape

Then update callers to use the normalized shape (or change refreshOAuthTokens to return { data: token } consistently).

• [ ] WARNING [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L1] — Token response validation is overly permissive and can accept malformed/hostile shapes (e.g., any numeric property name becomes an expiry candidate), increasing the chance of persisting incorrect token data.

Evidence:

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(),
});

Why it’s a problem: This schema effectively says “any string key maps to a number”, which is not what OAuth token responses look like. It can cause safeParse to succeed for unexpected payloads and then callers may read token.expiry_date / other fields that aren’t actually present or are wrong. This is a correctness + security boundary issue because provider responses are untrusted external input.
Fix: Replace with a strict schema that validates the exact fields you use (e.g., access_token, expires_in or expiry_date, and optionally refresh_token). Example:

const minimumTokenResponseSchema = z.object({
  access_token: z.string(),
  expires_in: z.number().int().positive().optional(),
  expiry_date: z.number().optional(),
  refresh_token: z.string().optional(),
}).passthrough();

Then in callers, compute expiry_date deterministically from expires_in if needed, rather than relying on “any numeric property”.

• [ ] WARNING [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] — Sync gating uses userId truthiness, which will skip syncing for valid userId=0 (and generally relies on truthy semantics rather than explicit null/undefined checks).

Evidence:

if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId) {

Why it’s a problem: If userId can ever be 0 (or if types drift), syncing will be incorrectly disabled. It’s a latent correctness bug.
Fix: Use explicit null/undefined check:

if (APP_CREDENTIAL_SHARING_ENABLED && process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT && userId !== null && userId !== undefined) {

• [ ] SUGGESTION — Webhook handler should validate and fail closed on missing/invalid env vars and header name. In apps/web/pages/api/webhook/app-credential.ts, you do req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] but if CALCOM_WEBHOOK_SECRET is undefined you’ll compare against undefined and may accidentally accept requests depending on header presence. Add explicit checks at startup or early in handler: if either env var is missing, return 500 with a clear message and log once. [apps/web/pages/api/webhook/app-credential.ts:L1]
• [ ] SUGGESTION — Add replay protection to the webhook. Even with a shared secret, attackers can replay old payloads to re-encrypt/overwrite credentials. Include timestamp + nonce in the request body and store nonce hashes for a short TTL. [apps/web/pages/api/webhook/app-credential.ts:L1]
• [ ] SUGGESTION — Ensure refreshOAuthTokens has a typed contract and update all callers accordingly. Right now callers assume provider SDK shape (res.data) but the sync path returns a raw Response. Define a shared type like type TokenRefreshResult = { data: { access_token: string; expiry_date: number; refresh_token?: string } } and make both branches return it. [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1, packages/app-store/googlecalendar/lib/CalendarService.ts:L83]
• [ ] SUGGESTION — Repo-wide: search for any remaining imports of old OAuth state/type paths mentioned in the cross-file analysis and ensure compilation passes. A mismatch here can lead to subtle runtime state-shape bugs in OAuth callbacks. [packages/app-store/_utils/oauth/encodeOAuthState.ts, packages/app-store/_utils/oauth/decodeOAuthState.ts]

Suggestions

• Webhook handler should validate and fail closed on missing/invalid env vars and header name. In apps/web/pages/api/webhook/app-credential.ts, you do req.headers[process.env.CALCOM_WEBHOOK_HEADER_NAME || "calcom-webhook-secret"] but if CALCOM_WEBHOOK_SECRET is undefined you’ll compare against undefined and may accidentally accept requests depending on header presence. Add explicit checks at startup or early in handler: if either env var is missing, return 500 with a clear message and log once. [apps/web/pages/api/webhook/app-credential.ts:L1]
• Add replay protection to the webhook. Even with a shared secret, attackers can replay old payloads to re-encrypt/overwrite credentials. Include timestamp + nonce in the request body and store nonce hashes for a short TTL. [apps/web/pages/api/webhook/app-credential.ts:L1]
• Ensure refreshOAuthTokens has a typed contract and update all callers accordingly. Right now callers assume provider SDK shape (res.data) but the sync path returns a raw Response. Define a shared type like type TokenRefreshResult = { data: { access_token: string; expiry_date: number; refresh_token?: string } } and make both branches return it. [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1, packages/app-store/googlecalendar/lib/CalendarService.ts:L83]
• Repo-wide: search for any remaining imports of old OAuth state/type paths mentioned in the cross-file analysis and ensure compilation passes. A mismatch here can lead to subtle runtime state-shape bugs in OAuth callbacks. [packages/app-store/_utils/oauth/encodeOAuthState.ts, packages/app-store/_utils/oauth/decodeOAuthState.ts]

📝 This review includes 2 inline comments (2 warnings)

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

⚠️ WARNING

Valid concern: when credential sharing is enabled, this helper mutates the parsed token object and injects a sentinel refresh_token string if the provider response omits one. That hides a real contract mismatch between the sync endpoint and the existing credential schema, and downstream code will persist a fake refresh token instead of failing fast. The root cause is that the new shared refresh path is trying to satisfy legacy schema expectations by fabricating data rather than normalizing the schema or handling the missing field explicitly.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

This is a real concurrency risk, but the issue is broader than the import itself: client_id and client_secret remain module-level mutable state while refreshes now route through a shared helper that can execute per-request. In a multi-tenant or concurrent request scenario, one request can overwrite the globals while another is still refreshing, causing tokens to be minted with the wrong app credentials. The fix is to remove shared mutable state and pass credentials through the call chain.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 95/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add a shared OAuth credential refresh/sync mechanism and a new webhook endpoint to decrypt and persist app credential keys when credential sharing is enabled.

Summary

Behaviorally, this PR introduces a new credential-sync webhook path that can bypass the normal provider refresh flow and instead fetch tokens from a shared endpoint, plus a new API webhook that decrypts and writes credential keys into the database. The highest risks are (1) credential integrity/data corruption due to weak or incorrect parsing/normalization contracts (including a placeholder refresh_token mutation) and (2) runtime crashes or silent misbehavior from inconsistent return shapes and missing validation around decrypted payloads. You must verify the end-to-end contract between refreshOAuthTokens -> parseRefreshTokenResponse -> credential persistence, and ensure the webhook handler validates decrypted content and fails safely with 4xx rather than 500s. Also confirm the webhook secret/header checks are robust against missing env vars and that the decrypted keys schema matches what downstream code expects.

🎯 Review Focus

Verify the cross-file OAuth token/credential contract: refreshOAuthTokens must return a consistent, typed shape that parseRefreshTokenResponse validates without mutation, and the webhook decrypted keys must be schema-validated before persisting to prisma.credential.

Key Findings

• 🚨 [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] CRITICAL: refreshOAuthTokens returns a raw fetch(...) Response when the sync endpoint is enabled, but callers (e.g., GoogleCalendar) treat the result as { data: ... } (const token = res?.data), which will be undefined and can throw or persist invalid tokens; fix by making refreshOAuthTokens normalize the return shape (e.g., parse JSON and return { data: { access_token, expiry_date, refresh_token? } }) and update all call sites to use the same typed contract.

✅ Action Checklist

• [ ] CRITICAL [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1] — refreshOAuthTokens returns a raw fetch(...) Response when the sync endpoint is enabled, but callers (e.g., GoogleCalendar) treat the result as { data: ... } (const token = res?.data), which will be undefined and can throw or persist invalid tokens; fix by making refreshOAuthTokens normalize the return shape (e.g., parse JSON and return { data: { access_token, expiry_date, refresh_token? } }) and update all call sites to use the same typed contract.
• [ ] SUGGESTION — Harden the webhook handler to avoid 500s and log flooding: in apps/web/pages/api/webhook/app-credential.ts, replace appCredentialWebhookRequestBodySchema.parse(req.body) with safeParse and return 400 on failure (same pattern as your other services). Also wrap symmetricDecrypt(...) + JSON.parse(...) in try/catch and return 400 for invalid ciphertext/JSON.
• [ ] SUGGESTION — Make the webhook secret check robust to missing env vars: in apps/web/pages/api/webhook/app-credential.ts, explicitly validate process.env.CALCOM_WEBHOOK_SECRET and process.env.CALCOM_WEBHOOK_HEADER_NAME at startup (or early in handler) and fail closed with a clear 500/503 if misconfigured; currently the comparison can become undefined !== undefined-style behavior depending on env values.
• [ ] SUGGESTION — Define and reuse a shared Zod schema/type for the persisted credential key payload: create a single schema used by both the webhook handler and the credential creation/sync utilities (e.g., createOAuthAppCredential and any sync code) so decrypted keys match what downstream expects; otherwise you risk runtime breakage when the decrypted JSON shape drifts.
• [ ] SUGGESTION — Fix the contract mismatch in GoogleCalendar (and other apps) by aligning refreshOAuthTokens return type with what parseRefreshTokenResponse expects: either have refreshOAuthTokens return the raw provider response object that parseRefreshTokenResponse can validate, or have it return the already-parsed token payload and update parseRefreshTokenResponse accordingly. Add TypeScript types so this can’t compile if shapes diverge.

Suggestions

• Harden the webhook handler to avoid 500s and log flooding: in apps/web/pages/api/webhook/app-credential.ts, replace appCredentialWebhookRequestBodySchema.parse(req.body) with safeParse and return 400 on failure (same pattern as your other services). Also wrap symmetricDecrypt(...) + JSON.parse(...) in try/catch and return 400 for invalid ciphertext/JSON.
• Make the webhook secret check robust to missing env vars: in apps/web/pages/api/webhook/app-credential.ts, explicitly validate process.env.CALCOM_WEBHOOK_SECRET and process.env.CALCOM_WEBHOOK_HEADER_NAME at startup (or early in handler) and fail closed with a clear 500/503 if misconfigured; currently the comparison can become undefined !== undefined-style behavior depending on env values.
• Define and reuse a shared Zod schema/type for the persisted credential key payload: create a single schema used by both the webhook handler and the credential creation/sync utilities (e.g., createOAuthAppCredential and any sync code) so decrypted keys match what downstream expects; otherwise you risk runtime breakage when the decrypted JSON shape drifts.
• Fix the contract mismatch in GoogleCalendar (and other apps) by aligning refreshOAuthTokens return type with what parseRefreshTokenResponse expects: either have refreshOAuthTokens return the raw provider response object that parseRefreshTokenResponse can validate, or have it return the already-parsed token payload and update parseRefreshTokenResponse accordingly. Add TypeScript types so this can’t compile if shapes diverge.

📝 This review includes 4 inline comments (2 critical, 2 warnings)

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

🚨 CRITICAL

Quote: if (!refreshTokenResponse.data.refresh_token) { refreshTokenResponse.data.refresh_token = "refresh_token"; }

Issue: The code mutates refreshTokenResponse.data after parsing. This can be surprising and may cause incorrect behavior if callers expect the parsed data to reflect the actual token response. Additionally, setting refresh_token to the literal string "refresh_token" looks like a placeholder/bug: it does not preserve the real refresh token and will likely break subsequent refreshes.

Fix: If the intent is to normalize a missing refresh_token, you should either:

1. Require refresh_token in the schema when needed, or
2. Accept a caller-provided fallback refresh token, or
3. Do not overwrite with a sentinel string; instead, return parsed data without refresh_token and let the caller handle it.

Example (no mutation):

if (!refreshTokenResponse.data.refresh_token) {
  return refreshTokenResponse;
}

Example (caller-provided fallback):

const parseRefreshTokenResponse = (response: any, schema: z.ZodTypeAny, fallbackRefreshToken?: string) => {
  ...
  if (!refreshTokenResponse.data.refresh_token && fallbackRefreshToken) {
    refreshTokenResponse.data.refresh_token = fallbackRefreshToken;
  }
  return refreshTokenResponse;
};
``` (see also L24) (same pattern in packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L11)

---
_[re-entry.ai](https://re-entry.ai)_

[re-entry-ai]

🚨 CRITICAL

Quote: const res = await refreshOAuthTokens( async () => { const fetchTokens = await myGoogleAuth.refreshToken(googleCredentials.refresh_token); return fetchTokens.res; }, "google-calendar", credential.userId );

Issue: The code assumes refreshOAuthTokens(...) returns an object compatible with the subsequent usage const token = res?.data;. If refreshOAuthTokens returns the inner fetchTokens.res directly (or returns a different shape), res?.data may be undefined and token.access_token / token.expiry_date will throw at runtime.

Fix: Make the return contract explicit by typing/normalizing the result, e.g.:

const res = await refreshOAuthTokens(...);
const token = res?.data;
if (!token?.access_token || !token?.expiry_date) {
  throw new Error("Failed to refresh Google access token");
}

Or adjust to the actual return shape from refreshOAuthTokens (e.g., const { res } = ... if it returns { res }). (same pattern in packages/app-store/larkcalendar/lib/CalendarService.ts:L14, packages/app-store/webex/lib/VideoApiAdapter.ts:L64)

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

Quote: keys: z.string(),

Issue: The request schema only checks that keys is a string. After decryption, the code persists whatever JSON is provided (no schema for the decrypted object). This risks storing unexpected shapes/types in credential.key, potentially breaking downstream consumers or enabling data integrity issues.

Fix: Define a Zod schema for the decrypted keys payload and validate before writing:

const decryptedKeysSchema = z.record(z.any()); // or a stricter schema
const keys = decryptedKeysSchema.parse(parsed);

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

Quote: const reqBody = appCredentialWebhookRequestBodySchema.parse(req.body);

Issue: parse will throw on invalid input, and there is no try/catch to convert that into a controlled 400 response. This can lead to noisy 500s and potential log flooding/DoS via repeated invalid requests.

Fix: Use safeParse and return 400 on failure:

const parsed = appCredentialWebhookRequestBodySchema.safeParse(req.body);
if (!parsed.success) return res.status(400).json({ message: "Invalid request body" });
const reqBody = parsed.data;
``` (same pattern in packages/app-store/salesforce/lib/CalendarService.ts:L98)

---
_[re-entry.ai](https://re-entry.ai)_

[re-entry-ai]

🛡️ re-entry.ai Code Review

🚨 Risk Score: 94/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add an optional shared OAuth credential-sync mechanism via a webhook that decrypts and persists credential keys, and refactor multiple app integrations to route token refresh through a shared helper when enabled.

Summary

This PR introduces a new webhook endpoint that authenticates using a shared secret header, decrypts an encrypted payload, and writes credential keys to the database for a provided userId + appSlug. It also adds shared OAuth refresh utilities that can optionally bypass provider refresh flows by POSTing to a credential-sync endpoint, and then parses token responses using a permissive Zod schema. The highest risks are (1) a security-critical authorization gap in the webhook (secret header alone does not authorize writes to arbitrary userId/appSlug) and (2) credential integrity bugs in token parsing/fallback logic (including overwriting missing refresh_token with a literal string). Before merging, verify the webhook authorization model, tighten token parsing contracts, and ensure the sync routing conditions and error handling are correct and observable.

🎯 Review Focus

The security-critical webhook authorization model: verify that the webhook caller cannot write credential keys for arbitrary userId/appSlug just by knowing the shared secret, and that decrypted keys are validated against the expected schema for the resolved app type before persisting.

✅ Action Checklist

• SUGGESTION — Harden the webhook trust boundary: in [apps/web/pages/api/webhook/app-credential.ts:L1-L93], after const appMetadata = ..., derive the expected credential key schema from appMetadata.type (or a mapping) and validate the decrypted payload before writing. Concretely: replace const keys = JSON.parse(symmetricDecrypt(...)) with const decrypted = symmetricDecrypt(...); const parsed = keysSchema.parse(JSON.parse(decrypted)); and persist only parsed.
• SUGGESTION — Fix webhook error handling and observability: in [apps/web/pages/api/webhook/app-credential.ts:L1-L93], wrap the handler body in try/catch and return a consistent error response; also add structured logs/metrics for each failure reason (invalid secret, user/app not found, decrypt/parse failure, prisma failure) including userId and appSlug.
• SUGGESTION — Tighten token parsing contracts: in [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L1-L32], remove the permissive numeric-key heuristic ([z.string().toString()]: z.number()) and instead validate the exact fields returned by CALCOM_CREDENTIAL_SYNC_ENDPOINT (e.g., access_token, refresh_token, expiry_date/expires_in). Return a typed result (success vs structured error) so call sites can handle partial failures safely.
• SUGGESTION — Make the sync HTTP call robust: in [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1-L22], add an AbortController timeout and check response.ok before returning; if non-OK, parse the error payload (if any) and throw a typed error with appSlug/userId context.
• SUGGESTION — Add a startup/runtime self-check for the sync contract: since both the webhook and refresh helper depend on env wiring (CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET, CALCOM_CREDENTIAL_SYNC_ENDPOINT, CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY), add a single shared module that validates required env vars are present and consistent at boot (and optionally logs a warning if credential sharing is enabled but endpoint/keys are missing).

Suggestions

• Harden the webhook trust boundary: in [apps/web/pages/api/webhook/app-credential.ts:L1-L93], after const appMetadata = ..., derive the expected credential key schema from appMetadata.type (or a mapping) and validate the decrypted payload before writing. Concretely: replace const keys = JSON.parse(symmetricDecrypt(...)) with const decrypted = symmetricDecrypt(...); const parsed = keysSchema.parse(JSON.parse(decrypted)); and persist only parsed.
• Fix webhook error handling and observability: in [apps/web/pages/api/webhook/app-credential.ts:L1-L93], wrap the handler body in try/catch and return a consistent error response; also add structured logs/metrics for each failure reason (invalid secret, user/app not found, decrypt/parse failure, prisma failure) including userId and appSlug.
• Tighten token parsing contracts: in [packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts:L1-L32], remove the permissive numeric-key heuristic ([z.string().toString()]: z.number()) and instead validate the exact fields returned by CALCOM_CREDENTIAL_SYNC_ENDPOINT (e.g., access_token, refresh_token, expiry_date/expires_in). Return a typed result (success vs structured error) so call sites can handle partial failures safely.
• Make the sync HTTP call robust: in [packages/app-store/_utils/oauth/refreshOAuthTokens.ts:L1-L22], add an AbortController timeout and check response.ok before returning; if non-OK, parse the error payload (if any) and throw a typed error with appSlug/userId context.
• Add a startup/runtime self-check for the sync contract: since both the webhook and refresh helper depend on env wiring (CALCOM_WEBHOOK_HEADER_NAME, CALCOM_WEBHOOK_SECRET, CALCOM_CREDENTIAL_SYNC_ENDPOINT, CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY), add a single shared module that validates required env vars are present and consistent at boot (and optionally logs a warning if credential sharing is enabled but endpoint/keys are missing).

📝 This review includes 21 inline comments (3 critical, 11 warnings, 7 notes)

ℹ️ 14 additional lower-priority findings were hidden to keep this review focused. Adjust your team's review cap if you want them all.

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

🚨 CRITICAL

Quote: const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, {

Issue (security): The URL is taken directly from an environment variable and used without validation. If misconfigured (or compromised), this becomes an SSRF vector from the server to arbitrary internal/external destinations.

Fix: Validate the endpoint against an allowlist (scheme/host) before calling fetch.

Example:

const endpoint = process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT;
if (!endpoint) throw new Error("Missing endpoint");
const url = new URL(endpoint);
if (!['https'].includes(url.protocol) || !['your-allowed-host.com'].includes(url.host)) {
  throw new Error("Invalid credential sync endpoint");
}
const resp = await fetch(url.toString(), ...);
``` (see also L7, L4)

[re-entry-ai]

🚨 CRITICAL

Quote: if (!refreshTokenResponse.data.refresh_token) { refreshTokenResponse.data.refresh_token = "refresh_token"; }

Issue: When refresh_token is missing/falsey, the code assigns the literal string "refresh_token" instead of deriving a real token value. This will corrupt persisted auth state and can cause repeated refresh failures or unauthorized API calls depending on how the token is stored/used.

Fix: Do not invent a token value. Either:

• throw when refresh_token is missing, or
• keep the previous refresh token (requires passing it in), or
• only set a default when the caller explicitly intends a placeholder.

Example (fail fast):

if (!refreshTokenResponse.data.refresh_token) {
  throw new Error('refresh_token missing from token response');
}

[re-entry-ai]

Quote: import { z } from "zod";

Issue: z is used, but the function signature accepts schema: z.ZodTypeAny and then uses schema.safeParse(response). If callers pass a schema that does not include refresh_token, the code later assumes refreshTokenResponse.data.refresh_token exists and mutates it. This can lead to runtime behavior that silently diverges from the intended schema contract.

Fix: After parsing, validate that refresh_token exists (or is optional but handled explicitly) before mutating. For example:

if (!('refresh_token' in refreshTokenResponse.data) || typeof refreshTokenResponse.data.refresh_token !== 'string') {
  throw new Error('refresh_token missing or invalid in token response');
}

Or constrain the generic schema type to one that guarantees refresh_token?: string.

[re-entry-ai]

Quote: import prisma from "@calcom/prisma";

Issue: If @calcom/prisma exports a PrismaClient instance as a named export (common pattern is import prisma from ... vs import { prisma } from ...), this could be a build/runtime failure. This cannot be confirmed from the diff alone, but the import style is a potential build failure point.

Fix: Verify the export shape in @calcom/prisma and adjust to the correct import form (e.g., import { prisma } from "@calcom/prisma"; or import prisma from ...).

[re-entry-ai]

📈 Risk score updated: 93 → 94 (CRITICAL)

Review above has been updated with the latest assessment.

[re-entry-ai]

Re-entry follow-up — 7 new findings on this push.

[re-entry-ai]

🚨 CRITICAL

Authorization gap / missing auth: the webhook handler updates/creates credentials for an arbitrary userId and appSlug based only on a shared webhook secret header, without verifying that the caller is authorized to act on that specific user/app. Mitigate by binding the webhook payload to an authenticated principal (e.g., signed claims) and/or enforcing additional authorization checks (e.g., verify the app/user relationship server-side beyond existence).

[re-entry-ai]

🚨 CRITICAL

Wrong error handling / incorrect fallback: when refreshTokenResponse.data.refresh_token is missing, the code sets it to the literal string "refresh_token", which is almost certainly not the actual refresh token and will propagate incorrect credentials. Fix by either requiring refresh_token in the schema for the sync path or by preserving the original refresh token from the caller rather than overwriting with a placeholder.

[re-entry-ai]

🚨 CRITICAL

Domain concept: credential webhook ingestion authorization. Intent/implementation gap: the new webhook endpoint authenticates only via a shared secret header and then upserts prisma.credential.key for userId + appSlug, with no additional actor/role verification (e.g., admin/team ownership) and no check that the caller is allowed to write for that userId. Fix: add authorization tying the webhook caller to the target user/app (or require an additional signed claim/credentialId), and validate that the decrypted keys match the expected credential schema for appMetadata.type before writing.

[re-entry-ai]

Missing input validation at trust boundary: keys is decrypted and then JSON.parse(...) is called without schema validation of the resulting object before persisting to prisma.credential.key. Mitigate by validating the decrypted structure with a Zod schema per appMetadata.type (or at least ensuring it is an object with expected fields) before storing.

[re-entry-ai]

Type/logic mismatch: the function checks userId truthiness (userId is number | null) and only syncs when userId is truthy, which will skip syncing for valid userId values like 0 (and generally relies on truthiness rather than null-check). Fix by checking userId !== null (or typeof userId === 'number') instead of userId.

[re-entry-ai]

Domain concept: credential-sync refresh routing. Intent/implementation gap: refreshOAuthTokens only routes to CALCOM_CREDENTIAL_SYNC_ENDPOINT when userId is truthy, but several call sites pass credential.userId (which may be 0 or otherwise falsy) and the helper’s condition uses userId rather than an explicit null/undefined check, potentially skipping the sync path and persisting tokens via the provider refresh instead of the shared sync mechanism. Fix: change the condition to userId !== null && userId !== undefined (or accept number | null and check userId != null).

[re-entry-ai]

Domain concept: refreshed token parsing contract. Intent/implementation gap: parseRefreshTokenResponse uses minimumTokenResponseSchema with a broad numeric-property rule ([z.string().toString()]: z.number()), which can reject valid provider responses (or accept unintended numeric fields) depending on the sync endpoint’s payload shape; this can cause token persistence to fail or write incorrect token fields. Fix: tighten the schema to the exact expected fields from the sync endpoint (e.g., access_token, refresh_token, expiry_date), and map/transform explicitly rather than relying on “any numeric property”.