Address greptile comments

2026-09-22 13:39:35 +00:00 · 2025-09-16 10:22:34 -03:00
parent 5bfff46c16
commit d77726d9f3
23 changed files with 102 additions and 57 deletions
--- a/backend/src/db/migrations/20250910193000_pki-sync.ts
+++ b/backend/src/db/migrations/20250910193000_pki-sync.ts
@@ -19,7 +19,7 @@ export async function up(knex: Knex): Promise<void> {
      t.uuid("subscriberId");
      t.foreign("subscriberId").references("id").inTable(TableName.PkiSubscriber).onDelete("SET NULL");
      t.uuid("connectionId").notNullable();
-      t.foreign("connectionId").references("id").inTable(TableName.AppConnection);
+      t.foreign("connectionId").references("id").inTable(TableName.AppConnection).onDelete("CASCADE");
      t.timestamps(true, true, true);
      t.string("syncStatus");
      t.string("lastSyncJobId");
--- a/backend/src/ee/services/license/mocks/license-fns.ts
+++ b/backend/src/ee/services/license/mocks/license-fns.ts
@@ -31,6 +31,7 @@ export const getDefaultOnPremFeatures = () => {
    caCrl: false,
    sshHostGroups: false,
    enterpriseSecretSyncs: false,
+    enterpriseCertificateSyncs: false,
    enterpriseAppConnections: true,
    machineIdentityAuthTemplates: false
  };
--- a/backend/src/ee/services/license/license-fns.ts
+++ b/backend/src/ee/services/license/license-fns.ts
@@ -62,6 +62,7 @@ export const getDefaultOnPremFeatures = (): TFeatureSet => ({
  sshHostGroups: false,
  secretScanning: false,
  enterpriseSecretSyncs: false,
+  enterpriseCertificateSyncs: false,
  enterpriseAppConnections: false,
  fips: false,
  eventSubscriptions: false,
--- a/backend/src/ee/services/license/license-types.ts
+++ b/backend/src/ee/services/license/license-types.ts
@@ -75,6 +75,7 @@ export type TFeatureSet = {
  sshHostGroups: false;
  secretScanning: false;
  enterpriseSecretSyncs: false;
+  enterpriseCertificateSyncs: false;
  enterpriseAppConnections: false;
  machineIdentityAuthTemplates: false;
  fips: false;
--- a/backend/src/services/pki-sync/pki-sync-dal.ts
+++ b/backend/src/services/pki-sync/pki-sync-dal.ts
@@ -30,8 +30,7 @@ const basePkiSyncQuery = ({ filter, db, tx }: { db: TDbClient; filter?: PkiSyncF
      db
        .ref("isPlatformManagedCredentials")
        .withSchema(TableName.AppConnection)
-        .as("appConnectionIsPlatformManagedCredentials"),
-      db.ref("encryptedCredentials").withSchema(TableName.AppConnection).as("appConnectionEncryptedCredentials")
+        .as("appConnectionIsPlatformManagedCredentials")
    );

  if (filter) {
--- a/backend/src/services/pki-sync/pki-sync-fns.ts
+++ b/backend/src/services/pki-sync/pki-sync-fns.ts
@@ -18,7 +18,7 @@ export const enterprisePkiSyncCheck = async (
 ) => {
  const plan = await licenseService.getPlan(orgId);

-  if (!plan.enterpriseSecretSyncs && ENTERPRISE_PKI_SYNCS.includes(pkiSyncDestination)) {
+  if (!plan.enterpriseCertificateSyncs && ENTERPRISE_PKI_SYNCS.includes(pkiSyncDestination)) {
    throw new BadRequestError({
      message: errorMessage || "Failed to create PKI sync due to plan restriction. Upgrade plan to create PKI sync."
    });
--- a/backend/src/services/pki-sync/pki-sync-queue.ts
+++ b/backend/src/services/pki-sync/pki-sync-queue.ts
@@ -172,7 +172,8 @@ export const pkiSyncQueueFactory = ({
        // Validate certificate data
        if (!certData.certificate || certData.certificate.trim() === "") {
          logger.error(`Skipping certificate ${certData.name}: empty certificate data`);
-          return;
+          // eslint-disable-next-line no-continue
+          continue;
        }

        // Parse certificate to extract metadata
--- a/backend/src/services/pki-sync/pki-sync-schemas.ts
+++ b/backend/src/services/pki-sync/pki-sync-schemas.ts
@@ -1,19 +1,33 @@
 import { z } from "zod";

+import { AzureKeyVaultPkiSyncConfigSchema } from "./azure-key-vault/azure-key-vault-pki-sync-types";
 import { PkiSync } from "./pki-sync-enums";

+// Schema for PKI sync options configuration
+export const PkiSyncOptionsSchema = z.object({
+  canImportCertificates: z.boolean()
+});
+
+// Schema for destination-specific configurations
+export const PkiSyncDestinationConfigSchema = z.discriminatedUnion("destination", [
+  z.object({
+    destination: z.literal(PkiSync.AzureKeyVault),
+    config: AzureKeyVaultPkiSyncConfigSchema
+  })
+]);
+
 // Base PKI sync schema for API responses
 export const PkiSyncSchema = z.object({
-  id: z.string(),
-  name: z.string(),
+  id: z.string().uuid(),
+  name: z.string().max(255),
  description: z.string().nullable().optional(),
  destination: z.nativeEnum(PkiSync),
  isAutoSyncEnabled: z.boolean(),
  destinationConfig: z.record(z.unknown()),
  syncOptions: z.record(z.unknown()),
-  projectId: z.string(),
-  subscriberId: z.string().nullable().optional(),
-  connectionId: z.string(),
+  projectId: z.string().uuid(),
+  subscriberId: z.string().uuid().nullable().optional(),
+  connectionId: z.string().uuid(),
  createdAt: z.date(),
  updatedAt: z.date(),
  syncStatus: z.string().nullable().optional(),
@@ -22,14 +36,14 @@ export const PkiSyncSchema = z.object({

 // Schema for PKI sync list items (includes app connection info)
 export const PkiSyncListItemSchema = PkiSyncSchema.extend({
-  appConnectionName: z.string(),
-  appConnectionApp: z.string()
+  appConnectionName: z.string().max(255),
+  appConnectionApp: z.string().max(255)
 });

 // Schema for PKI sync details (includes app connection info)
 export const PkiSyncDetailsSchema = PkiSyncSchema.extend({
-  appConnectionName: z.string(),
-  appConnectionApp: z.string()
+  appConnectionName: z.string().max(255),
+  appConnectionApp: z.string().max(255)
 });

 export type TPkiSyncSchema = z.infer<typeof PkiSyncSchema>;
--- a/backend/src/services/pki-sync/pki-sync-service.ts
+++ b/backend/src/services/pki-sync/pki-sync-service.ts
@@ -29,6 +29,15 @@ import {
  TUpdatePkiSyncDTO
 } from "./pki-sync-types";

+const getDestinationAppType = (destination: PkiSync): AppConnection => {
+  switch (destination) {
+    case PkiSync.AzureKeyVault:
+      return AppConnection.AzureKeyVault;
+    default:
+      throw new BadRequestError({ message: "Unsupported PKI sync destination" });
+  }
+};
+
 type TPkiSyncServiceFactoryDep = {
  pkiSyncDAL: TPkiSyncDALFactory;
  pkiSubscriberDAL: Pick<TPkiSubscriberDALFactory, "findById">;
@@ -89,7 +98,7 @@ export const pkiSyncServiceFactory = ({
    }

    // Get the destination app type based on PKI sync destination
-    const destinationApp = destination === PkiSync.AzureKeyVault ? AppConnection.AzureKeyVault : destination;
+    const destinationApp = getDestinationAppType(destination);

    // Validates permission to connect and app is valid for sync destination
    await appConnectionService.connectAppConnectionById(destinationApp, connectionId, actor);
--- a/frontend/src/components/pki-syncs/PkiSyncSelect.tsx
+++ b/frontend/src/components/pki-syncs/PkiSyncSelect.tsx
@@ -65,10 +65,10 @@ export const PkiSyncSelect = ({ onSelect }: Props) => {
          const { image, name } = PKI_SYNC_MAP[destination];
          return (
            <button
-              key={name}
+              key={destination}
              type="button"
              onClick={() =>
-                enterprise && !subscription.enterpriseSecretSyncs
+                enterprise && !subscription.enterpriseCertificateSyncs
                  ? handlePopUpOpen("upgradePlan")
                  : onSelect(destination)
              }
--- a/frontend/src/components/pki-syncs/PkiSyncStatusBadge.tsx
+++ b/frontend/src/components/pki-syncs/PkiSyncStatusBadge.tsx
@@ -47,7 +47,7 @@ export const PkiSyncStatusBadge = ({ status }: Props) => {
    <Badge className="flex h-5 w-min items-center gap-1.5 whitespace-nowrap" variant={variant}>
      <FontAwesomeIcon
        icon={icon}
-        className={[PkiSyncStatus.Running].includes(status) ? "animate-spin" : ""}
+        className={status === PkiSyncStatus.Running ? "animate-spin" : ""}
      />
      <span>{text}</span>
    </Badge>
--- a/frontend/src/components/pki-syncs/forms/CreatePkiSyncForm.tsx
+++ b/frontend/src/components/pki-syncs/forms/CreatePkiSyncForm.tsx
@@ -10,7 +10,13 @@ import { createNotification } from "@app/components/notifications";
 import { Button, FormControl, Switch } from "@app/components/v2";
 import { useWorkspace } from "@app/context";
 import { PKI_SYNC_MAP } from "@app/helpers/pkiSyncs";
-import { PkiSync, TPkiSync, useCreatePkiSync, usePkiSyncOption } from "@app/hooks/api/pkiSyncs";
+import {
+  PkiSync,
+  TCreatePkiSyncDTO,
+  TPkiSync,
+  useCreatePkiSync,
+  usePkiSyncOption
+} from "@app/hooks/api/pkiSyncs";

 import { PkiSyncDestinationFields } from "./PkiSyncDestinationFields";
 import { PkiSyncDetailsFields } from "./PkiSyncDetailsFields";
@@ -57,12 +63,16 @@ export const CreatePkiSyncForm = ({ destination, onComplete, onCancel }: Props)
    reValidateMode: "onChange"
  });

-  const onSubmit = async ({ connection, ...formData }: TPkiSyncForm) => {
+  const onSubmit = async ({ connection, destinationConfig, ...formData }: TPkiSyncForm) => {
    try {
      const pkiSync = await createPkiSync.mutateAsync({
        ...formData,
        connectionId: connection.id,
-        projectId: currentWorkspace.id
+        projectId: currentWorkspace.id,
+        destinationConfig: {
+          destination,
+          config: destinationConfig
+        } as unknown as TCreatePkiSyncDTO["destinationConfig"]
      });

      createNotification({
@@ -70,12 +80,12 @@ export const CreatePkiSyncForm = ({ destination, onComplete, onCancel }: Props)
        type: "success"
      });
      onComplete(pkiSync);
-    } catch (err: any) {
+    } catch (err: Error | unknown) {
      console.error(err);
      setShowConfirmation(false);
      createNotification({
        title: `Failed to add ${destinationName} Certificate Sync`,
-        text: err.message,
+        text: err instanceof Error ? err.message : "An unknown error occurred",
        type: "error"
      });
    }
--- a/frontend/src/components/pki-syncs/forms/PkiSyncDestinationFields.tsx
+++ b/frontend/src/components/pki-syncs/forms/PkiSyncDestinationFields.tsx
@@ -14,6 +14,10 @@ export const PkiSyncDestinationFields = () => {
    case PkiSync.AzureKeyVault:
      return <AzureKeyVaultPkiSyncFields />;
    default:
-      throw new Error(`Unhandled Destination Config Field: ${destination}`);
+      return (
+        <div className="flex items-center justify-center rounded-md border border-red-500 bg-red-100 p-4 text-red-700">
+          <p>Unsupported destination: {destination}</p>
+        </div>
+      );
  }
 };
--- a/frontend/src/components/pki-syncs/forms/PkiSyncOptionsFields/PkiSyncOptionsFields.tsx
+++ b/frontend/src/components/pki-syncs/forms/PkiSyncOptionsFields/PkiSyncOptionsFields.tsx
@@ -61,17 +61,17 @@ export const PkiSyncOptionsFields = () => {
              isChecked={value}
            >
              <p>
-                Disable Certificate Removal{" "}
+                Enable Certificate Removal{" "}
                <Tooltip
                  className="max-w-md"
                  content={
                    <>
                      <p>
-                        When enabled, Infisical will <span className="font-semibold">not</span>{" "}
-                        remove certificates from the destination during a sync.
+                        When enabled, Infisical will remove certificates from the destination during
+                        a sync if they are no longer managed by Infisical.
                      </p>
                      <p className="mt-4">
-                        Enable this option if you intend to manage some certificates manually
+                        Disable this option if you intend to manage some certificates manually
                        outside of Infisical.
                      </p>
                    </>
--- a/frontend/src/components/pki-syncs/forms/schemas.ts
+++ b/frontend/src/components/pki-syncs/forms/schemas.ts
@@ -3,14 +3,18 @@ import { z } from "zod";
 import { PkiSync } from "@app/hooks/api/pkiSyncs";

 export const PkiSyncFormSchema = z.object({
-  name: z.string().trim().min(1, "Name is required"),
+  name: z
+    .string()
+    .trim()
+    .min(1, "Name is required")
+    .max(255, "Name must be less than 255 characters"),
  description: z.string().optional(),
  destination: z.nativeEnum(PkiSync),
  isAutoSyncEnabled: z.boolean().default(true),
  subscriberId: z.string().min(1, "PKI Subscriber is required"),
  connection: z.object({
-    id: z.string(),
-    name: z.string()
+    id: z.string().uuid("Invalid connection ID format"),
+    name: z.string().max(255, "Connection name must be less than 255 characters")
  }),
  destinationConfig: z.object({
    vaultBaseUrl: z.string().url("Valid URL is required")
@@ -25,11 +29,15 @@ export type TPkiSyncForm = z.infer<typeof PkiSyncFormSchema>;

 export const UpdatePkiSyncFormSchema = PkiSyncFormSchema.partial().merge(
  z.object({
-    name: z.string().trim().min(1, "Name is required"),
+    name: z
+      .string()
+      .trim()
+      .min(1, "Name is required")
+      .max(255, "Name must be less than 255 characters"),
    destination: z.nativeEnum(PkiSync),
    connection: z.object({
-      id: z.string(),
-      name: z.string()
+      id: z.string().uuid("Invalid connection ID format"),
+      name: z.string().max(255, "Connection name must be less than 255 characters")
    })
  })
 );
--- a/frontend/src/hooks/api/pkiSyncs/mutations.tsx
+++ b/frontend/src/hooks/api/pkiSyncs/mutations.tsx
@@ -39,7 +39,7 @@ export const useUpdatePkiSync = () => {
    },
    onSuccess: (_, { syncId, projectId }) => {
      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.list(projectId) });
-      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId) });
+      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId, projectId) });
    }
  });
 };
@@ -56,7 +56,7 @@ export const useDeletePkiSync = () => {
    },
    onSuccess: (_, { syncId, projectId }) => {
      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.list(projectId) });
-      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId) });
+      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId, projectId) });
    }
  });
 };
@@ -77,7 +77,7 @@ export const useTriggerPkiSyncSyncCertificates = () => {
    },
    onSuccess: (_, { syncId, projectId }) => {
      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.list(projectId) });
-      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId) });
+      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId, projectId) });
    }
  });
 };
@@ -98,7 +98,7 @@ export const useTriggerPkiSyncImportCertificates = () => {
    },
    onSuccess: (_, { syncId, projectId }) => {
      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.list(projectId) });
-      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId) });
+      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId, projectId) });
    }
  });
 };
@@ -119,7 +119,7 @@ export const useTriggerPkiSyncRemoveCertificates = () => {
    },
    onSuccess: (_, { syncId, projectId }) => {
      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.list(projectId) });
-      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId) });
+      queryClient.invalidateQueries({ queryKey: pkiSyncKeys.byId(syncId, projectId) });
    }
  });
 };
--- a/frontend/src/hooks/api/pkiSyncs/queries.tsx
+++ b/frontend/src/hooks/api/pkiSyncs/queries.tsx
@@ -13,7 +13,8 @@ export const pkiSyncKeys = {
  all: ["pki-sync"] as const,
  options: () => [...pkiSyncKeys.all, "options"] as const,
  list: (projectId: string) => [...pkiSyncKeys.all, "list", projectId] as const,
-  byId: (syncId: string) => [...pkiSyncKeys.all, "by-id", syncId] as const
+  byId: (syncId: string, projectId: string) =>
+    [...pkiSyncKeys.all, "by-id", syncId, projectId] as const
 };

 export const usePkiSyncOptions = (
@@ -75,7 +76,7 @@ export const useGetPkiSync = (
  >
 ) => {
  return useQuery({
-    queryKey: pkiSyncKeys.byId(syncId),
+    queryKey: pkiSyncKeys.byId(syncId, projectId),
    queryFn: async () => {
      const { data } = await apiRequest.get<TPkiSyncResponse>(`/api/v1/pki-syncs/${syncId}`, {
        params: { projectId }
--- a/frontend/src/hooks/api/subscriptions/types.ts
+++ b/frontend/src/hooks/api/subscriptions/types.ts
@@ -53,6 +53,7 @@ export type SubscriptionPlan = {
  kmip: boolean;
  secretScanning: boolean;
  enterpriseSecretSyncs: boolean;
+  enterpriseCertificateSyncs: boolean;
  enterpriseAppConnections: boolean;
  cardDeclined?: boolean;
  cardDeclinedReason?: string;
--- a/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncTable/PkiSyncRow.tsx
+++ b/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncTable/PkiSyncRow.tsx
@@ -124,7 +124,7 @@ export const PkiSyncRow = ({
  return (
    <Tr
      onClick={() => {
-        console.log("PKI Sync navigation:", { syncId: id, projectId });
+        // console.log("PKI Sync navigation:", { syncId: id, projectId });
        navigate({
          to: ROUTE_PATHS.CertManager.PkiSyncDetailsByIDPage.path,
          params: {
--- a/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncTable/PkiSyncsTable.tsx
+++ b/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncTable/PkiSyncsTable.tsx
@@ -146,7 +146,9 @@ export const PkiSyncsTable = ({ pkiSyncs }: Props) => {
          const destinationValues = getPkiSyncDestinationColValues(pkiSync);

          return (
-            PKI_SYNC_MAP[destination].name.toLowerCase().includes(searchValue) ||
+            (PKI_SYNC_MAP[destination]?.name || "Unknown Service")
+              .toLowerCase()
+              .includes(searchValue) ||
            name.toLowerCase().includes(searchValue) ||
            (pkiSync.appConnectionName &&
              pkiSync.appConnectionName.toLowerCase().includes(searchValue)) ||
@@ -209,7 +211,7 @@ export const PkiSyncsTable = ({ pkiSyncs }: Props) => {
  const getColSortIcon = (col: PkiSyncsOrderBy) =>
    orderDirection === OrderByDirection.DESC && orderBy === col ? faArrowUp : faArrowDown;

-  const isTableFiltered = Boolean(filters.destinations.length);
+  const isTableFiltered = Boolean(filters.destinations.length || filters.status.length);

  const handleDelete = (pkiSync: PkiSyncData) => handlePopUpOpen("deleteSync", pkiSync);

--- a/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncsTab.tsx
+++ b/frontend/src/pages/cert-manager/IntegrationsListPage/components/PkiSyncsTab/PkiSyncsTab.tsx
@@ -88,7 +88,7 @@ export const PkiSyncsTab = () => {
            {(isAllowed) => (
              <Button
                colorSchema="secondary"
-                type="submit"
+                type="button"
                leftIcon={<FontAwesomeIcon icon={faPlus} />}
                onClick={() => handlePopUpOpen("addSync")}
                isDisabled={!isAllowed}
--- a/frontend/src/pages/cert-manager/PkiSyncDetailsByIDPage/components/PkiSyncAuditLogsSection.tsx
+++ b/frontend/src/pages/cert-manager/PkiSyncDetailsByIDPage/components/PkiSyncAuditLogsSection.tsx
@@ -54,25 +54,18 @@ export const PkiSyncAuditLogsSection = ({ pkiSync }: Props) => {
              Please{" "}
              {subscription && subscription.slug !== null ? (
                <Link to="/organization/billing" target="_blank" rel="noopener noreferrer">
-                  <a
-                    className="cursor-pointer underline transition-all hover:text-white"
-                    target="_blank"
-                  >
+                  <span className="cursor-pointer underline transition-all hover:text-white">
                    upgrade your subscription
-                  </a>
+                  </span>
                </Link>
              ) : (
                <a
                  href="https://infisical.com/scheduledemo"
                  target="_blank"
                  rel="noopener noreferrer"
+                  className="cursor-pointer underline transition-all hover:text-white"
                >
-                  <a
-                    className="cursor-pointer underline transition-all hover:text-white"
-                    target="_blank"
-                  >
-                    upgrade your subscription
-                  </a>
+                  upgrade your subscription
                </a>
              )}{" "}
              to view sync logs.
--- a/frontend/src/pages/cert-manager/PkiSyncDetailsByIDPage/components/PkiSyncDetailsSection.tsx
+++ b/frontend/src/pages/cert-manager/PkiSyncDetailsByIDPage/components/PkiSyncDetailsSection.tsx
@@ -86,7 +86,7 @@ export const PkiSyncDetailsSection = ({ pkiSync, onEditDetails }: Props) => {
          )}
          {lastSyncedAt && (
            <GenericFieldLabel label="Last Synced">
-              {format(new Date(lastSyncedAt), "yyyy-MM-dd, hh:mm aaa")}
+              {format(new Date(lastSyncedAt), "yyyy-MM-dd, h:mm aaa")}
            </GenericFieldLabel>
          )}
          {syncStatus === PkiSyncStatus.Failed && failureMessage && (