Merge pull request #4942 from Infisical/agent-docs-2

feat(dynamic-secrets): retry lease revocation when failing

backend/src/ee/services/dynamic-secret-lease/dynamic-secret-lease-queue.ts

@@ -1,10 +1,18 @@
+import { ProjectMembershipRole } from "@app/db/schemas";
 import { DisableRotationErrors } from "@app/ee/services/secret-rotation/secret-rotation-queue";
+import { getConfig } from "@app/lib/config/env";
+import { applyJitter } from "@app/lib/delay";
 import { NotFoundError } from "@app/lib/errors";
 import { logger } from "@app/lib/logger";
 import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue";
+import { TIdentityDALFactory } from "@app/services/identity/identity-dal";
 import { TKmsServiceFactory } from "@app/services/kms/kms-service";
 import { KmsDataKey } from "@app/services/kms/kms-types";
+import { TProjectDALFactory } from "@app/services/project/project-dal";
+import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal";
 import { TSecretFolderDALFactory } from "@app/services/secret-folder/secret-folder-dal";
+import { SmtpTemplates, TSmtpService } from "@app/services/smtp/smtp-service";
+import { TUserDALFactory } from "@app/services/user/user-dal";
 
 import { TDynamicSecretDALFactory } from "../dynamic-secret/dynamic-secret-dal";
 import { DynamicSecretStatus } from "../dynamic-secret/dynamic-secret-types";
@@ -15,7 +23,12 @@ import { TDynamicSecretLeaseConfig } from "./dynamic-secret-lease-types";
 type TDynamicSecretLeaseQueueServiceFactoryDep = {
   queueService: TQueueServiceFactory;
   dynamicSecretLeaseDAL: Pick<TDynamicSecretLeaseDALFactory, "findById" | "deleteById" | "find" | "updateById">;
-  dynamicSecretDAL: Pick<TDynamicSecretDALFactory, "findById" | "deleteById" | "updateById">;
+  smtpService: Pick<TSmtpService, "sendMail">;
+  userDAL: Pick<TUserDALFactory, "findById">;
+  identityDAL: TIdentityDALFactory;
+  dynamicSecretDAL: Pick<TDynamicSecretDALFactory, "findById" | "deleteById" | "updateById" | "findOne">;
+  projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findAllProjectMembers">;
+  projectDAL: Pick<TProjectDALFactory, "findById">;
   dynamicSecretProviders: Record<DynamicSecretProviders, TDynamicProviderFns>;
   kmsService: Pick<TKmsServiceFactory, "createCipherPairWithDataKey">;
   folderDAL: Pick<TSecretFolderDALFactory, "findById">;
@@ -23,18 +36,24 @@ type TDynamicSecretLeaseQueueServiceFactoryDep = {
 
 export type TDynamicSecretLeaseQueueServiceFactory = {
   pruneDynamicSecret: (dynamicSecretCfgId: string) => Promise<void>;
-  setLeaseRevocation: (leaseId: string, expiryAt: Date) => Promise<void>;
+  setLeaseRevocation: (leaseId: string, dynamicSecretId: string, expiryAt: Date) => Promise<void>;
   unsetLeaseRevocation: (leaseId: string) => Promise<void>;
+  queueFailedRevocation: (leaseId: string, dynamicSecretId: string) => Promise<void>;
   init: () => Promise<void>;
 };
 
+const MAX_REVOCATION_RETRY_COUNT = 10;
+
 export const dynamicSecretLeaseQueueServiceFactory = ({
   queueService,
   dynamicSecretDAL,
   dynamicSecretProviders,
   dynamicSecretLeaseDAL,
   kmsService,
-  folderDAL
+  folderDAL,
+  projectMembershipDAL,
+  projectDAL,
+  smtpService
 }: TDynamicSecretLeaseQueueServiceFactoryDep): TDynamicSecretLeaseQueueServiceFactory => {
   const pruneDynamicSecret = async (dynamicSecretCfgId: string) => {
     await queueService.queuePg<QueueName.DynamicSecretRevocation>(
@@ -48,10 +67,10 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
     );
   };
 
-  const setLeaseRevocation = async (leaseId: string, expiryAt: Date) => {
+  const setLeaseRevocation = async (leaseId: string, dynamicSecretId: string, expiryAt: Date) => {
     await queueService.queuePg<QueueName.DynamicSecretRevocation>(
       QueueJobs.DynamicSecretRevocation,
-      { leaseId },
+      { leaseId, dynamicSecretId },
       {
         id: leaseId,
         singletonKey: leaseId,
@@ -68,10 +87,53 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
     await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, leaseId);
   };
 
+  const queueFailedRevocation = async (leaseId: string, dynamicSecretId: string) => {
+    const appConfig = getConfig();
+
+    const retryDelaySeconds = appConfig.isDevelopmentMode ? 1 : Math.floor(applyJitter(3_600_000 * 4) / 1000); // retry every 4 hours with 20% +- jitter (convert ms to seconds for pgboss)
+
+    await queueService.queuePg<QueueName.DynamicSecretRevocation>(
+      QueueJobs.DynamicSecretRevocation,
+      { leaseId, isRetry: true, dynamicSecretId },
+      {
+        singletonKey: `${leaseId}-retry`, // avoid conflicts with scheduled revocation
+        retryDelay: retryDelaySeconds,
+        retryLimit: MAX_REVOCATION_RETRY_COUNT, // we dont want it to ever hit the limit, we want the expireInHours to take effect.
+        expireInHours: 23 // if we set it to 24 hours, pgboss will complain that the expireIn is too high
+      }
+    );
+  };
+
+  const $queueDynamicSecretLeaseRevocationFailedEmail = async (leaseId: string, dynamicSecretId: string) => {
+    const appConfig = getConfig();
+
+    const delay = appConfig.isDevelopmentMode ? 1_000 * 60 : 1_000 * 60 * 15; // 1 minute in development, 15 minutes in production
+
+    await queueService.queue(
+      QueueName.DynamicSecretLeaseRevocationFailedEmail,
+      QueueJobs.DynamicSecretLeaseRevocationFailedEmail,
+      {
+        leaseId
+      },
+      {
+        jobId: `dynamic-secret-lease-revocation-failed-email-${dynamicSecretId}`,
+        delay,
+        attempts: 3,
+        backoff: {
+          type: "exponential",
+          delay: 1000 * 60 // 1 minute
+        },
+        removeOnComplete: true,
+        removeOnFail: true
+      }
+    );
+  };
+
   const $dynamicSecretQueueJob = async (
     jobName: string,
     jobId: string,
-    data: { leaseId: string } | { dynamicSecretCfgId: string }
+    data: { leaseId: string; dynamicSecretId: string; isRetry?: boolean } | { dynamicSecretCfgId: string },
+    retryCount?: number
   ): Promise<void> => {
     try {
       if (jobName === QueueJobs.DynamicSecretRevocation) {
@@ -79,7 +141,9 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
         logger.info("Dynamic secret lease revocation started: ", leaseId, jobId);
 
         const dynamicSecretLease = await dynamicSecretLeaseDAL.findById(leaseId);
-        if (!dynamicSecretLease) throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
+        if (!dynamicSecretLease) {
+          throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
+        }
 
         const folder = await folderDAL.findById(dynamicSecretLease.dynamicSecret.folderId);
         if (!folder)
@@ -150,7 +214,7 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
       }
       logger.info("Finished dynamic secret job", jobId);
     } catch (error) {
-      logger.error(error);
+      logger.error(error, "Failed to delete dynamic secret");
 
       if (jobName === QueueJobs.DynamicSecretPruning) {
         const { dynamicSecretCfgId } = data as { dynamicSecretCfgId: string };
@@ -161,20 +225,97 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
       }
 
       if (jobName === QueueJobs.DynamicSecretRevocation) {
-        const { leaseId } = data as { leaseId: string };
+        const { leaseId, isRetry, dynamicSecretId } = data as {
+          leaseId: string;
+          isRetry?: boolean;
+          dynamicSecretId: string;
+        };
         await dynamicSecretLeaseDAL.updateById(leaseId, {
           status: DynamicSecretStatus.FailedDeletion,
-          statusDetails: (error as Error)?.message?.slice(0, 255)
+          statusDetails: `${(error as Error)?.message?.slice(0, 255)} - Retrying automatically`
         });
+
+        // only add to retry queue if this is not a retry, and if the error is not a DisableRotationErrors error
+        if (!isRetry && !(error instanceof DisableRotationErrors)) {
+          // if revocation fails, we should stop the job and queue a new job to retry the revocation at a later time.
+          await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, jobId);
+          await queueService.stopRepeatableJobByJobId(QueueName.DynamicSecretRevocation, jobId);
+          await queueFailedRevocation(leaseId, dynamicSecretId);
+
+          // if its the last attempt, and the error isn't a DisableRotationErrors error, send an email to the project admins (debounced)
+        } else if (isRetry && !(error instanceof DisableRotationErrors)) {
+          if (retryCount && retryCount === MAX_REVOCATION_RETRY_COUNT) {
+            // if all retries fail, we should also stop the automatic revocation job.
+            // the ID of the revocation job is set to the leaseId, so we can use that to stop the job
+
+            // we dont have to stop the retry job, because if we hit this point, its the last attempt and the retry job will be stopped by pgboss itself after this point,
+            await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, leaseId);
+            await queueService.stopRepeatableJobByJobId(QueueName.DynamicSecretRevocation, leaseId);
+
+            await $queueDynamicSecretLeaseRevocationFailedEmail(leaseId, dynamicSecretId);
+          }
+        }
       }
       if (error instanceof DisableRotationErrors) {
         if (jobId) {
           await queueService.stopRepeatableJobByJobId(QueueName.DynamicSecretRevocation, jobId);
           await queueService.stopJobByIdPg(QueueName.DynamicSecretRevocation, jobId);
         }
+      } else {
+        // propagate to next part
+        throw error;
+      }
+    }
+  };
+
+  // send alert email once all revocation attempts have failed
+  const $dynamicSecretLeaseRevocationFailedEmailJob = async (jobId: string, data: { leaseId: string }) => {
+    try {
+      const appCfg = getConfig();
+
+      const { leaseId } = data;
+      logger.info(
+        { leaseId, jobId },
+        "Dynamic secret revocation failed. Notifying project admins about failed revocation."
+      );
+
+      const lease = await dynamicSecretLeaseDAL.findById(leaseId);
+      if (!lease) {
+        throw new DisableRotationErrors({ message: "Dynamic secret lease not found" });
+      }
+
+      const folder = await folderDAL.findById(lease.dynamicSecret.folderId);
+      if (!folder) throw new NotFoundError({ message: `Failed to find folder with ${lease.dynamicSecret.folderId}` });
+
+      const project = await projectDAL.findById(folder.projectId);
+      const projectMembers = await projectMembershipDAL.findAllProjectMembers(project.id);
+
+      const projectAdmins = projectMembers.filter((member) =>
+        member.roles.some((role) => role.role === ProjectMembershipRole.Admin)
+      );
+
+      await smtpService.sendMail({
+        recipients: projectAdmins.map((member) => member.user.email!).filter(Boolean),
+        template: SmtpTemplates.DynamicSecretLeaseRevocationFailed,
+        subjectLine: "Dynamic Secret Lease Revocation Failed",
+        substitutions: {
+          dynamicSecretLeaseUrl: `${appCfg.SITE_URL}/organizations/${project.orgId}/projects/secret-management/${project.id}/secrets/${folder.environment.envSlug}?dynamicSecretId=${lease.dynamicSecret.id}&filterBy=dynamic&search=${lease.dynamicSecret.name}`,
+          dynamicSecretName: lease.dynamicSecret.name,
+          projectName: project.name,
+          environmentSlug: folder.environment.envSlug,
+          errorMessage: lease.statusDetails || "An unknown error occurred"
+        }
+      });
+    } catch (error) {
+      logger.error(error, "Failed to send dynamic secret lease revocation failed email");
+      if (error instanceof DisableRotationErrors) {
+        if (jobId) {
+          await queueService.stopRepeatableJobByJobId(QueueName.DynamicSecretLeaseRevocationFailedEmail, jobId);
+          await queueService.stopJobById(QueueName.DynamicSecretLeaseRevocationFailedEmail, jobId);
+        }
+      } else {
+        throw error;
       }
-      // propogate to next part
-      throw error;
     }
   };
 
@@ -182,14 +323,21 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
     await $dynamicSecretQueueJob(job.name, job.id as string, job.data);
   });
 
+  // we use redis for sending the email because:
+  // 1. we are insensitive to losing the jobs in queue in case of a disaster event
+  // 2. pgboss does not support exclusive job keys on v0.10.x, and upgrading to v0.11.x which supports exclusive jobs comes with a lot of breaking changes, and we would need to manually migrate our existing jobs to the new version
+  queueService.start(QueueName.DynamicSecretLeaseRevocationFailedEmail, async (job) => {
+    await $dynamicSecretLeaseRevocationFailedEmailJob(job.id as string, job.data);
+  });
+
   const init = async () => {
     await queueService.startPg<QueueName.DynamicSecretRevocation>(
       QueueJobs.DynamicSecretRevocation,
       async ([job]) => {
-        await $dynamicSecretQueueJob(job.name, job.id, job.data);
+        await $dynamicSecretQueueJob(job.name, job.id, job.data, job.retryCount);
       },
       {
-        workerCount: 5,
+        workerCount: 10,
         pollingIntervalSeconds: 1
       }
     );
@@ -210,6 +358,7 @@ export const dynamicSecretLeaseQueueServiceFactory = ({
     pruneDynamicSecret,
     setLeaseRevocation,
     unsetLeaseRevocation,
+    queueFailedRevocation,
     init
   };
 };

backend/src/ee/services/dynamic-secret-lease/dynamic-secret-lease-service.ts

@@ -178,7 +178,7 @@ export const dynamicSecretLeaseServiceFactory = ({
       config
     });
 
-    await dynamicSecretQueueService.setLeaseRevocation(dynamicSecretLease.id, expireAt);
+    await dynamicSecretQueueService.setLeaseRevocation(dynamicSecretLease.id, dynamicSecretCfg.id, expireAt);
     return { lease: dynamicSecretLease, dynamicSecret: dynamicSecretCfg, data };
   };
 
@@ -272,7 +272,7 @@ export const dynamicSecretLeaseServiceFactory = ({
     );
 
     await dynamicSecretQueueService.unsetLeaseRevocation(dynamicSecretLease.id);
-    await dynamicSecretQueueService.setLeaseRevocation(dynamicSecretLease.id, expireAt);
+    await dynamicSecretQueueService.setLeaseRevocation(dynamicSecretLease.id, dynamicSecretCfg.id, expireAt);
     const updatedDynamicSecretLease = await dynamicSecretLeaseDAL.updateById(dynamicSecretLease.id, {
       expireAt,
       externalEntityId: entityId
@@ -358,11 +358,13 @@ export const dynamicSecretLeaseServiceFactory = ({
     if ((revokeResponse as { error?: Error })?.error) {
       const { error } = revokeResponse as { error?: Error };
       logger.error(error?.message, "Failed to revoke lease");
-      const deletedDynamicSecretLease = await dynamicSecretLeaseDAL.updateById(dynamicSecretLease.id, {
+      const updatedDynamicSecretLease = await dynamicSecretLeaseDAL.updateById(dynamicSecretLease.id, {
         status: DynamicSecretLeaseStatus.FailedDeletion,
         statusDetails: error?.message?.slice(0, 255)
       });
-      return deletedDynamicSecretLease;
+      // queue a job to retry the revocation at a later time
+      await dynamicSecretQueueService.queueFailedRevocation(dynamicSecretLease.id, dynamicSecretCfg.id);
+      return updatedDynamicSecretLease;
     }
 
     await dynamicSecretQueueService.unsetLeaseRevocation(dynamicSecretLease.id);

backend/src/lib/delay/index.ts

@@ -2,3 +2,13 @@ export const delay = (ms: number) =>
   new Promise<void>((resolve) => {
     setTimeout(resolve, ms);
   });
+
+export const applyJitter = (delayMs: number) => {
+  const jitterFactor = 0.2;
+
+  // generates random value in [-0.2, +0.2] range
+  const randomFactor = (Math.random() * 2 - 1) * jitterFactor;
+  const jitterAmount = randomFactor * delayMs;
+
+  return delayMs + jitterAmount;
+};

backend/src/queue/queue-service.ts

@@ -61,6 +61,7 @@ export enum QueueName {
   SecretPushEventScan = "secret-push-event-scan",
   UpgradeProjectToGhost = "upgrade-project-to-ghost",
   DynamicSecretRevocation = "dynamic-secret-revocation",
+  DynamicSecretLeaseRevocationFailedEmail = "dynamic-secret-lease-revocation-failed-email",
   CaCrlRotation = "ca-crl-rotation",
   CaLifecycle = "ca-lifecycle", // parent queue to ca-order-certificate-for-subscriber
   SecretReplication = "secret-replication",
@@ -120,6 +121,7 @@ export enum QueueJobs {
   SecretRotationV2RotateSecrets = "secret-rotation-v2-rotate-secrets",
   SecretRotationV2SendNotification = "secret-rotation-v2-send-notification",
   CreateFolderTreeCheckpoint = "create-folder-tree-checkpoint",
+  DynamicSecretLeaseRevocationFailedEmail = "dynamic-secret-lease-revocation-failed-email",
   InvalidateCache = "invalidate-cache",
   SecretScanningV2FullScan = "secret-scanning-v2-full-scan",
   SecretScanningV2DiffScan = "secret-scanning-v2-diff-scan",
@@ -219,11 +221,19 @@ export type TQueueJobTypes = {
     name: QueueJobs.TelemetryInstanceStats;
     payload: undefined;
   };
+  [QueueName.DynamicSecretLeaseRevocationFailedEmail]: {
+    name: QueueJobs.DynamicSecretLeaseRevocationFailedEmail;
+    payload: {
+      leaseId: string;
+    };
+  };
   [QueueName.DynamicSecretRevocation]:
     | {
         name: QueueJobs.DynamicSecretRevocation;
         payload: {
+          isRetry?: boolean;
           leaseId: string;
+          dynamicSecretId: string;
         };
       }
     | {

backend/src/server/routes/index.ts

@@ -1874,7 +1874,12 @@ export const registerRoutes = async (
     dynamicSecretProviders,
     dynamicSecretDAL,
     folderDAL,
-    kmsService
+    kmsService,
+    smtpService,
+    userDAL,
+    identityDAL,
+    projectMembershipDAL,
+    projectDAL
   });
   const dynamicSecretService = dynamicSecretServiceFactory({
     projectDAL,

backend/src/services/smtp/emails/DynamicSecretLeaseRevocationFailedTemplate.tsx

@@ -0,0 +1,68 @@
+import { Heading, Section, Text } from "@react-email/components";
+
+import { BaseButton } from "./BaseButton";
+import { BaseEmailWrapper, BaseEmailWrapperProps } from "./BaseEmailWrapper";
+
+interface DynamicSecretLeaseRevocationFailedTemplateProps
+  extends Omit<BaseEmailWrapperProps, "title" | "preview" | "children"> {
+  siteUrl: string;
+  dynamicSecretLeaseUrl: string;
+  dynamicSecretName: string;
+  projectName: string;
+  environmentSlug: string;
+  errorMessage: string;
+}
+
+export const DynamicSecretLeaseRevocationFailedTemplate = ({
+  siteUrl,
+  dynamicSecretLeaseUrl,
+  dynamicSecretName,
+  projectName,
+  environmentSlug,
+  errorMessage
+}: DynamicSecretLeaseRevocationFailedTemplateProps) => {
+  return (
+    <BaseEmailWrapper
+      title="Dynamic Secret Lease Revocation Failed"
+      preview={`Dynamic secret lease revocation failed for dynamic secret ${dynamicSecretName}`}
+      siteUrl={siteUrl}
+    >
+      <Heading className="text-black text-[18px] leading-[28px] text-center font-normal p-0 mx-0">
+        Dynamic Secret Lease Revocation Failed
+      </Heading>
+      <Section className="px-[24px] mt-[36px] pt-[12px] pb-[8px] border border-solid border-gray-200 rounded-md bg-gray-50">
+        <Text className="text-black text-[14px] leading-[24px]">
+          One or more leases for the dynamic secret <strong>{dynamicSecretName}</strong> in project{" "}
+          <strong>{projectName}</strong> and environment <strong>{environmentSlug}</strong> have failed to revoke after
+          multiple attempts.
+        </Text>
+        <Text className="text-black text-[14px] leading-[24px]">
+          Please review the dynamic secret leases and attempt to revoke them again.
+        </Text>
+      </Section>
+
+      <Section className="mt-[24px] bg-gray-50 pt-[2px] mb-[25px] pb-[16px] border border-solid border-gray-200 px-[24px] rounded-md text-gray-800">
+        <Text className="mb-[0px]">
+          <strong>Latest error message</strong>
+        </Text>
+        <Text className="leading-[24px] text-[14px] text-red-600 mt-[4px]">{errorMessage}</Text>
+      </Section>
+
+      <Section className="text-center">
+        <BaseButton href={dynamicSecretLeaseUrl}>View Dynamic Secret Leases</BaseButton>
+      </Section>
+    </BaseEmailWrapper>
+  );
+};
+
+export default DynamicSecretLeaseRevocationFailedTemplate;
+
+DynamicSecretLeaseRevocationFailedTemplate.PreviewProps = {
+  errorMessage: 'REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM "[REDACTED]" - tuple concurrently updated.',
+  dynamicSecretLeaseUrl: "https://infisical.com/test",
+  leaseId: "717d5013-7194-49d9-b6ac-6192328c2914",
+  dynamicSecretName: "postgres-prod-db",
+  projectName: "Development Team",
+  environmentSlug: "dev",
+  siteUrl: "https://infisical.com"
+} as DynamicSecretLeaseRevocationFailedTemplateProps;

backend/src/services/smtp/smtp-service.ts

@@ -43,6 +43,7 @@ import {
   SubOrganizationInvitationTemplate,
   UnlockAccountTemplate
 } from "./emails";
+import DynamicSecretLeaseRevocationFailedTemplate from "./emails/DynamicSecretLeaseRevocationFailedTemplate";
 
 export type TSmtpConfig = SMTPTransport.Options;
 export type TSmtpSendMail = {
@@ -89,7 +90,8 @@ export enum SmtpTemplates {
   SecretScanningV2ScanFailed = "secretScanningV2ScanFailed",
   SecretScanningV2SecretsDetected = "secretScanningV2SecretsDetected",
   AccountDeletionConfirmation = "accountDeletionConfirmation",
-  HealthAlert = "healthAlert"
+  HealthAlert = "healthAlert",
+  DynamicSecretLeaseRevocationFailed = "dynamicSecretLeaseRevocationFailed"
 }
 
 export enum SmtpHost {
@@ -137,7 +139,8 @@ const EmailTemplateMap: Record<SmtpTemplates, React.FC<any>> = {
   [SmtpTemplates.SecretScanningV2ScanFailed]: SecretScanningScanFailedTemplate,
   [SmtpTemplates.SecretScanningV2SecretsDetected]: SecretScanningSecretsDetectedTemplate,
   [SmtpTemplates.AccountDeletionConfirmation]: AccountDeletionConfirmationTemplate,
-  [SmtpTemplates.HealthAlert]: HealthAlertTemplate
+  [SmtpTemplates.HealthAlert]: HealthAlertTemplate,
+  [SmtpTemplates.DynamicSecretLeaseRevocationFailed]: DynamicSecretLeaseRevocationFailedTemplate
 };
 
 export const smtpServiceFactory = (cfg: TSmtpConfig) => {