chore: disable apps with missing required keys (#27012)
* ci(companion): add separate typecheck workflow to catch type errors This adds a new companion-typecheck.yml workflow that runs TypeScript type checking for the companion app. This would have caught the missing useEffect import issue in PR #26931. Changes: - Add new companion-typecheck.yml workflow file - Update pr.yml to call the new workflow when companion files change - Add typecheck-companion to the required jobs list Co-Authored-By: anik@cal.com <adhabal2002@gmail.com> * add lint checks * test * remove * update typecheck * address review * chore:- hide apps with missing required keys from app store * update * Delete packages/app-store/_utils/hasRequiredAppKeys.test.ts * Update validateAppKeys.ts * Update validateAppKeys.ts * Update validateAppKeys.test.ts --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
anik@cal.com
Devin AI
parent
da91152365
commit
ad137fbf9c
@@ -3,6 +3,7 @@ import type { NextRequest } from "next/server";
|
||||
import { NextResponse } from "next/server";
|
||||
|
||||
import { getAppWithMetadata } from "@calcom/app-store/_appRegistry";
|
||||
import { shouldEnableApp } from "@calcom/app-store/_utils/validateAppKeys";
|
||||
import logger from "@calcom/lib/logger";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
import type { AppCategories, Prisma } from "@calcom/prisma/client";
|
||||
@@ -48,6 +49,17 @@ async function postHandler(request: NextRequest) {
|
||||
updates["dirName"] = app.dirName ?? app.slug;
|
||||
}
|
||||
|
||||
// Ensure app is only enabled if it has valid keys (or doesn't require keys)
|
||||
const shouldBeEnabled = shouldEnableApp(dbApp.dirName, dbApp.keys);
|
||||
if (dbApp.enabled !== shouldBeEnabled) {
|
||||
updates["enabled"] = shouldBeEnabled;
|
||||
if (!shouldBeEnabled && dbApp.enabled) {
|
||||
log.warn(
|
||||
`⚠️ Disabling app ${dbApp.slug} - required keys are missing or invalid. Please configure keys in admin settings.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (Object.keys(updates).length > 0) {
|
||||
log.info(`🔨 Updating app ${dbApp.slug} with ${Object.keys(updates).join(", ")}`);
|
||||
if (!isDryRun) {
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { shouldEnableApp } from "./validateAppKeys";
|
||||
|
||||
describe("shouldEnableApp", () => {
|
||||
describe("Apps without key schemas", () => {
|
||||
it("should return true for apps that don't require keys", () => {
|
||||
// Use a dirName that doesn't have a schema (e.g., a non-existent app)
|
||||
const result = shouldEnableApp("non-existent-app", null);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true even if keys are null for apps without schemas", () => {
|
||||
const result = shouldEnableApp("non-existent-app", null);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true even if keys are empty object for apps without schemas", () => {
|
||||
const result = shouldEnableApp("non-existent-app", {});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Apps with key schemas", () => {
|
||||
it("should return false when keys are null", () => {
|
||||
// dailyvideo requires api_key and has scale_plan with default
|
||||
const result = shouldEnableApp("dailyvideo", null);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when keys are undefined", () => {
|
||||
const result = shouldEnableApp("dailyvideo", undefined);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when keys is an empty object", () => {
|
||||
// Empty object doesn't have required api_key field
|
||||
const result = shouldEnableApp("dailyvideo", {});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when keys is an array", () => {
|
||||
const result = shouldEnableApp("dailyvideo", [] as any);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when keys is a string", () => {
|
||||
const result = shouldEnableApp("dailyvideo", "invalid" as any);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when keys is a number", () => {
|
||||
const result = shouldEnableApp("dailyvideo", 123 as any);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when required keys are missing", () => {
|
||||
// Missing required api_key field
|
||||
const result = shouldEnableApp("dailyvideo", {
|
||||
scale_plan: "true",
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return false when required keys are empty strings", () => {
|
||||
// api_key is empty string, which violates .min(1)
|
||||
const result = shouldEnableApp("dailyvideo", {
|
||||
api_key: "",
|
||||
scale_plan: "false",
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return true when all required keys are present and valid", () => {
|
||||
const result = shouldEnableApp("dailyvideo", {
|
||||
api_key: "valid-api-key",
|
||||
scale_plan: "false",
|
||||
});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true when required keys are present and optional fields use defaults", () => {
|
||||
// scale_plan has a default, so we can omit it
|
||||
const result = shouldEnableApp("dailyvideo", {
|
||||
api_key: "valid-api-key",
|
||||
});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return false when keys have wrong types", () => {
|
||||
// api_key should be string, not number
|
||||
const result = shouldEnableApp("dailyvideo", {
|
||||
api_key: 123 as any,
|
||||
scale_plan: "false",
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Apps with multiple required fields", () => {
|
||||
it("should return false when any required field is missing", () => {
|
||||
// vital requires mode, region, api_key, and webhook_secret
|
||||
const result = shouldEnableApp("vital", {
|
||||
mode: "sandbox",
|
||||
region: "us",
|
||||
api_key: "test-key",
|
||||
// Missing webhook_secret
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("should return true when all required fields are present", () => {
|
||||
const result = shouldEnableApp("vital", {
|
||||
mode: "sandbox",
|
||||
region: "us",
|
||||
api_key: "test-key",
|
||||
webhook_secret: "test-secret",
|
||||
});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return false when any required field is empty", () => {
|
||||
const result = shouldEnableApp("vital", {
|
||||
mode: "",
|
||||
region: "us",
|
||||
api_key: "test-key",
|
||||
webhook_secret: "test-secret",
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Apps with empty key schemas (user-configured apps like PayPal, GTM)", () => {
|
||||
// These apps have `appKeysSchema = z.object({})` - they don't need server-side keys
|
||||
// Users configure them after installation, so they should always be enabled
|
||||
|
||||
it("should return true for PayPal when keys are null", () => {
|
||||
const result = shouldEnableApp("paypal", null);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true for PayPal when keys are undefined", () => {
|
||||
const result = shouldEnableApp("paypal", undefined);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true for PayPal when keys are empty object", () => {
|
||||
const result = shouldEnableApp("paypal", {});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true for GTM when keys are null", () => {
|
||||
const result = shouldEnableApp("gtm", null);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true for GTM when keys are undefined", () => {
|
||||
const result = shouldEnableApp("gtm", undefined);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should return true for GA4 when keys are null", () => {
|
||||
const result = shouldEnableApp("ga4", null);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,28 @@
|
||||
import { appKeysSchemas } from "@calcom/app-store/apps.keys-schemas.generated";
|
||||
import type { Prisma } from "@calcom/prisma/client";
|
||||
/**
|
||||
* Determines if an app should be enabled based on whether it has valid keys.
|
||||
* This is used by app registration scripts to prevent enabling apps without proper configuration.
|
||||
*
|
||||
* @param dirName - The directory name of the app
|
||||
* @param keys - The keys object (can be undefined/null)
|
||||
*/
|
||||
export function shouldEnableApp(dirName: string, keys?: Prisma.JsonValue | null): boolean {
|
||||
const keySchema = appKeysSchemas[dirName as keyof typeof appKeysSchemas];
|
||||
|
||||
// If no schema, the app doesn't require keys - can be enabled
|
||||
if (!keySchema) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// If keys is null/undefined, check if the schema accepts an empty object.
|
||||
if (keys === null || keys === undefined) {
|
||||
const emptyObjectResult = keySchema.safeParse({});
|
||||
return emptyObjectResult.success;
|
||||
}
|
||||
|
||||
|
||||
// Validate keys against schema
|
||||
const result = keySchema.safeParse(keys);
|
||||
return result.success;
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
import { captureException } from "@sentry/nextjs";
|
||||
|
||||
import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData";
|
||||
import { shouldEnableApp } from "@calcom/app-store/_utils/validateAppKeys";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
import type { Prisma } from "@calcom/prisma/client";
|
||||
|
||||
@@ -12,13 +13,15 @@ export class PrismaAppRepository {
|
||||
throw new Error(`App ${dirName} not found`);
|
||||
}
|
||||
|
||||
// Only enable if keys are valid (or app doesn't require keys)
|
||||
const enabled = shouldEnableApp(dirName, keys as Prisma.JsonValue);
|
||||
await prisma.app.create({
|
||||
data: {
|
||||
slug: appMetadata.slug,
|
||||
categories: appMetadata.categories,
|
||||
dirName: dirName,
|
||||
keys,
|
||||
enabled: true,
|
||||
enabled,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import dotEnv from "dotenv";
|
||||
import path from "node:path"
|
||||
|
||||
import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData";
|
||||
import { shouldEnableApp } from "@calcom/app-store/_utils/validateAppKeys";
|
||||
import prisma from "@calcom/prisma";
|
||||
import type { Prisma } from "@calcom/prisma/client";
|
||||
import { AppCategories } from "@calcom/prisma/enums";
|
||||
@@ -48,8 +49,9 @@ async function createApp(
|
||||
},
|
||||
});
|
||||
|
||||
// We need to enable seeded apps as they are used in tests.
|
||||
const data = { slug, dirName, categories, keys, enabled: true };
|
||||
// Only enable apps if they have valid keys (or don't require keys)
|
||||
const enabled = shouldEnableApp(dirName, keys as Prisma.JsonValue);
|
||||
const data = { slug, dirName, categories, keys, enabled };
|
||||
|
||||
if (!foundApp) {
|
||||
await prisma.app.create({
|
||||
|
||||
Reference in New Issue
Block a user