Improve Gitlab sync destination check and show projectId on duplicate destination

2026-09-22 13:39:35 +00:00 · 2025-10-03 01:00:30 -03:00
parent 76976ab85a
commit fb1d5ca257
7 changed files with 112 additions and 28 deletions
--- a/backend/src/server/routes/v1/secret-sync-routers/secret-sync-endpoints.ts
+++ b/backend/src/server/routes/v1/secret-sync-routers/secret-sync-endpoints.ts
@@ -440,7 +440,10 @@ export const registerSyncSecretsEndpoints = <T extends TSecretSync, I extends TS
        projectId: z.string().uuid()
      }),
      response: {
-        200: z.object({ hasDuplicate: z.boolean() })
+        200: z.object({
+          hasDuplicate: z.boolean(),
+          duplicateProjectId: z.string().uuid().optional()
+        })
      }
    },
    onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]),
--- a/backend/src/services/secret-sync/secret-sync-service.ts
+++ b/backend/src/services/secret-sync/secret-sync-service.ts
@@ -713,7 +713,16 @@ export const secretSyncServiceFactory = ({
      case SecretSync.DigitalOceanAppPlatform:
        return ["appName"];
      case SecretSync.GitLab:
-        return ["projectName", "shouldProtectSecrets", "shouldMaskSecrets", "shouldHideSecrets"];
+        return [
+          "projectName",
+          "shouldProtectSecrets",
+          "shouldMaskSecrets",
+          "shouldHideSecrets",
+          "targetEnvironment",
+          "groupName",
+          "groupId",
+          "projectId"
+        ];
      case SecretSync.Heroku:
        return ["appName"];
      case SecretSync.Netlify:
@@ -733,6 +742,34 @@ export const secretSyncServiceFactory = ({
    }
  };

+  const handleSpecialCaseDuplicateCheck = (
+    destination: SecretSync,
+    existingConfig: Record<string, unknown>,
+    newConfig: Record<string, unknown>
+  ): boolean => {
+    switch (destination) {
+      case SecretSync.GitLab: {
+        const existingTargetEnv = existingConfig.targetEnvironment as string | undefined;
+        const newTargetEnv = newConfig.targetEnvironment as string | undefined;
+
+        // If either has wildcard '*', it conflicts with any targetEnvironment
+        if (existingTargetEnv === "*" || newTargetEnv === "*") {
+          return true;
+        }
+
+        return (
+          existingTargetEnv === newTargetEnv &&
+          ((newConfig.scope as string) === "group"
+            ? existingConfig.groupId === newConfig.groupId
+            : existingConfig.projectId === newConfig.projectId)
+        );
+      }
+      default:
+        // For other sync types, no special handling needed
+        return true;
+    }
+  };
+
  const checkDuplicateDestination = async (
    { destination, destinationConfig, excludeSyncId, projectId }: TCheckDuplicateDestinationDTO,
    actor: OrgServiceActor
@@ -753,7 +790,7 @@ export const secretSyncServiceFactory = ({
    );

    if (!destinationConfig || Object.keys(destinationConfig).length === 0) {
-      return { hasDuplicate: false };
+      return { hasDuplicate: false, duplicateProjectId: undefined };
    }

    try {
@@ -765,15 +802,27 @@ export const secretSyncServiceFactory = ({
        }

        try {
-          return deepEqualSkipFields(sync.destinationConfig, destinationConfig, skipFields);
+          const baseFieldsMatch = deepEqualSkipFields(sync.destinationConfig, destinationConfig, skipFields);
+          if (baseFieldsMatch) {
+            return handleSpecialCaseDuplicateCheck(
+              destination,
+              sync.destinationConfig as Record<string, unknown>,
+              destinationConfig
+            );
+          }
+          return false;
        } catch {
          return false;
        }
      });

-      return { hasDuplicate: duplicates.length > 0 };
+      const hasDuplicate = duplicates.length > 0;
+      return {
+        hasDuplicate,
+        duplicateProjectId: hasDuplicate ? duplicates[0].projectId : undefined
+      };
    } catch (error) {
-      return { hasDuplicate: false };
+      return { hasDuplicate: false, duplicateProjectId: undefined };
    }
  };

--- a/frontend/src/components/secret-syncs/forms/DuplicateDestinationConfirmationModal.tsx
+++ b/frontend/src/components/secret-syncs/forms/DuplicateDestinationConfirmationModal.tsx
@@ -5,13 +5,15 @@ type Props = {
  onOpenChange: (isOpen: boolean) => void;
  onConfirm: () => void;
  isLoading?: boolean;
+  duplicateProjectId?: string;
 };

 export const DuplicateDestinationConfirmationModal = ({
  isOpen,
  onOpenChange,
  onConfirm,
-  isLoading
+  isLoading,
+  duplicateProjectId
 }: Props) => {
  return (
    <Modal isOpen={isOpen} onOpenChange={onOpenChange}>
@@ -21,6 +23,14 @@ export const DuplicateDestinationConfirmationModal = ({
            Another secret sync in your organization is already configured with the same
            destination. Proceeding may cause conflicts or overwrite existing data.
          </p>
+          {duplicateProjectId && (
+            <p className="mt-2 text-xs text-mineshaft-400">
+              Duplicate found in project ID:{" "}
+              <code className="rounded bg-mineshaft-600 px-1 py-0.5 text-mineshaft-200">
+                {duplicateProjectId}
+              </code>
+            </p>
+          )}
          <p className="mt-2">Are you sure you want to continue?</p>
        </div>

--- a/frontend/src/components/secret-syncs/forms/EditSecretSyncForm.tsx
+++ b/frontend/src/components/secret-syncs/forms/EditSecretSyncForm.tsx
@@ -44,8 +44,9 @@ export const EditSecretSyncForm = ({ secretSync, fields, onComplete }: Props) =>

  const [destinationConfigToCheck, setDestinationConfigToCheck] = useState<unknown>(null);
  const [checkDuplicateEnabled, setCheckDuplicateEnabled] = useState(false);
+  const [storedDuplicateProjectId, setStoredDuplicateProjectId] = useState<string | undefined>();

-  const { data: hasDuplicate, isLoading: isCheckingDuplicate } = useCheckDuplicateDestination(
+  const { data: duplicateData, isLoading: isCheckingDuplicate } = useCheckDuplicateDestination(
    secretSync.destination,
    destinationConfigToCheck,
    secretSync.projectId,
@@ -84,7 +85,8 @@ export const EditSecretSyncForm = ({ secretSync, fields, onComplete }: Props) =>

  useEffect(() => {
    if (checkDuplicateEnabled && !isCheckingDuplicate && destinationConfigToCheck) {
-      if (hasDuplicate) {
+      if (duplicateData?.hasDuplicate) {
+        setStoredDuplicateProjectId(duplicateData.duplicateProjectId);
        setShowDuplicateConfirmation(true);
      } else if (pendingFormData) {
        performUpdate(pendingFormData);
@@ -96,7 +98,8 @@ export const EditSecretSyncForm = ({ secretSync, fields, onComplete }: Props) =>
  }, [
    checkDuplicateEnabled,
    isCheckingDuplicate,
-    hasDuplicate,
+    duplicateData?.hasDuplicate,
+    duplicateData?.duplicateProjectId,
    destinationConfigToCheck,
    pendingFormData,
    performUpdate
@@ -197,9 +200,15 @@ export const EditSecretSyncForm = ({ secretSync, fields, onComplete }: Props) =>

      <DuplicateDestinationConfirmationModal
        isOpen={showDuplicateConfirmation}
-        onOpenChange={setShowDuplicateConfirmation}
+        onOpenChange={(open) => {
+          setShowDuplicateConfirmation(open);
+          if (!open) {
+            setStoredDuplicateProjectId(undefined);
+          }
+        }}
        onConfirm={handleConfirmDuplicate}
        isLoading={updateSecretSync.isPending}
+        duplicateProjectId={storedDuplicateProjectId}
      />
    </>
  );
--- a/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/SecretSyncReviewFields.tsx
+++ b/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/SecretSyncReviewFields.tsx
@@ -67,7 +67,7 @@ export const SecretSyncReviewFields = () => {

  const destinationName = SECRET_SYNC_MAP[destination].name;

-  const { hasDuplicate, isChecking } = useDuplicateDestinationCheck({
+  const { hasDuplicate, duplicateProjectId, isChecking } = useDuplicateDestinationCheck({
    destination,
    projectId: currentProject?.id || "",
    enabled: true,
@@ -192,10 +192,17 @@ export const SecretSyncReviewFields = () => {
          <div className="mb-2 flex items-start rounded-md border border-yellow-600 bg-yellow-900/20 px-3 py-2">
            <div className="flex text-sm text-yellow-100">
              <FontAwesomeIcon icon={faWarning} className="mr-2 mt-1 text-yellow-600" />
-              <p>
-                Another secret sync in your organization is already configured with the same
-                destination. This may lead to conflicts or unexpected behavior.
-              </p>
+              <div>
+                <p>
+                  Another secret sync in your organization is already configured with the same
+                  destination. This may lead to conflicts or unexpected behavior.
+                </p>
+                {duplicateProjectId && (
+                  <p className="mt-1 text-xs text-yellow-200">
+                    Duplicate found in project ID: <code className="rounded bg-yellow-800/50 px-1 py-0.5">{duplicateProjectId}</code>
+                  </p>
+                )}
+              </div>
            </div>
          </div>
        )}
--- a/frontend/src/hooks/api/secretSyncs/queries.tsx
+++ b/frontend/src/hooks/api/secretSyncs/queries.tsx
@@ -103,23 +103,28 @@ export const useCheckDuplicateDestination = (
  projectId: string,
  excludeSyncId?: string,
  options?: Omit<
-    UseQueryOptions<boolean, unknown, boolean, ReturnType<typeof secretSyncKeys.duplicateCheck>>,
+    UseQueryOptions<
+      { hasDuplicate: boolean; duplicateProjectId?: string },
+      unknown,
+      { hasDuplicate: boolean; duplicateProjectId?: string },
+      ReturnType<typeof secretSyncKeys.duplicateCheck>
+    >,
    "queryKey" | "queryFn"
  >
 ) => {
  return useQuery({
    queryKey: secretSyncKeys.duplicateCheck(destination, destinationConfig, excludeSyncId),
    queryFn: async () => {
-      const { data } = await apiRequest.post<{ hasDuplicate: boolean }>(
-        `/api/v1/secret-syncs/${destination}/check-destination`,
-        {
-          destinationConfig,
-          excludeSyncId,
-          projectId
-        }
-      );
+      const { data } = await apiRequest.post<{
+        hasDuplicate: boolean;
+        duplicateProjectId?: string;
+      }>(`/api/v1/secret-syncs/${destination}/check-destination`, {
+        destinationConfig,
+        excludeSyncId,
+        projectId
+      });

-      return data.hasDuplicate;
+      return data;
    },
    enabled: Boolean(destinationConfig) && Object.keys(destinationConfig || {}).length > 0,
    ...options
--- a/frontend/src/hooks/api/secretSyncs/useDuplicateDestinationCheck.ts
+++ b/frontend/src/hooks/api/secretSyncs/useDuplicateDestinationCheck.ts
@@ -30,7 +30,7 @@ export const useDuplicateDestinationCheck = ({
  const shouldCheck = enabled && hasValidConfig;

  const {
-    data: hasDuplicate,
+    data: duplicateData,
    isLoading,
    error,
    refetch
@@ -41,7 +41,8 @@ export const useDuplicateDestinationCheck = ({
  });

  return {
-    hasDuplicate: shouldCheck ? Boolean(hasDuplicate) : false,
+    hasDuplicate: shouldCheck ? Boolean(duplicateData?.hasDuplicate) : false,
+    duplicateProjectId: duplicateData?.duplicateProjectId,
    isChecking: shouldCheck && isLoading,
    hasError: Boolean(error),
    hasValidConfig,