From 0d5514834dbe5f3e20d7d2d4db397e87f5f1570e Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Mon, 17 Jun 2024 13:14:28 +0200 Subject: [PATCH 1/5] Fix: Redundancies --- backend/src/services/secret/secret-queue.ts | 168 ++++++++++---------- 1 file changed, 82 insertions(+), 86 deletions(-) diff --git a/backend/src/services/secret/secret-queue.ts b/backend/src/services/secret/secret-queue.ts index 42e13b4456..fbd406a4c6 100644 --- a/backend/src/services/secret/secret-queue.ts +++ b/backend/src/services/secret/secret-queue.ts @@ -421,94 +421,88 @@ export const secretQueueFactory = ({ const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); if (!folder) { - logger.error(new Error("Secret path not found")); - return; + throw new Error("Secret path not found"); } - // start syncing all linked imports also - if (depth < MAX_SYNC_SECRET_DEPTH) { - // find all imports made with the given environment and secret path - const linkSourceDto = { - projectId, - importEnv: folder.environment.id, - importPath: secretPath, - isReplication: false - }; - const imports = await secretImportDAL.find(linkSourceDto); + // find all imports made with the given environment and secret path + const linkSourceDto = { + projectId, + importEnv: folder.environment.id, + importPath: secretPath, + isReplication: false + }; + const imports = await secretImportDAL.find(linkSourceDto); - if (imports.length) { - // keep calling sync secret for all the imports made - const importedFolderIds = unique(imports, (i) => i.folderId).map(({ folderId }) => folderId); - const importedFolders = await folderDAL.findSecretPathByFolderIds(projectId, importedFolderIds); - const foldersGroupedById = groupBy(importedFolders.filter(Boolean), (i) => i?.id as string); - logger.info( - `getIntegrationSecrets: Syncing secret due to link change [jobId=${job.id}] [projectId=${job.data.projectId}] [environment=${job.data.environment}] [secretPath=${job.data.secretPath}] [depth=${depth}]` - ); - await Promise.all( - imports - .filter(({ folderId }) => Boolean(foldersGroupedById[folderId][0]?.path as string)) - // filter out already synced ones - .filter( - ({ folderId }) => - !deDupeQueue[ - uniqueSecretQueueKey( - foldersGroupedById[folderId][0]?.environmentSlug as string, - foldersGroupedById[folderId][0]?.path as string - ) - ] - ) - .map(({ folderId }) => - syncSecrets({ - projectId, - secretPath: foldersGroupedById[folderId][0]?.path as string, - environmentSlug: foldersGroupedById[folderId][0]?.environmentSlug as string, - _deDupeQueue: deDupeQueue, - _depth: depth + 1, - excludeReplication: true - }) - ) - ); - } - - const secretReferences = await secretDAL.findReferencedSecretReferences( - projectId, - folder.environment.slug, - secretPath + if (imports.length) { + // keep calling sync secret for all the imports made + const importedFolderIds = unique(imports, (i) => i.folderId).map(({ folderId }) => folderId); + const importedFolders = await folderDAL.findSecretPathByFolderIds(projectId, importedFolderIds); + const foldersGroupedById = groupBy(importedFolders.filter(Boolean), (i) => i?.id as string); + logger.info( + `getIntegrationSecrets: Syncing secret due to link change [jobId=${job.id}] [projectId=${job.data.projectId}] [environment=${job.data.environment}] [secretPath=${job.data.secretPath}] [depth=${depth}]` + ); + await Promise.all( + imports + .filter(({ folderId }) => Boolean(foldersGroupedById[folderId][0]?.path as string)) + // filter out already synced ones + .filter( + ({ folderId }) => + !deDupeQueue[ + uniqueSecretQueueKey( + foldersGroupedById[folderId][0]?.environmentSlug as string, + foldersGroupedById[folderId][0]?.path as string + ) + ] + ) + .map(({ folderId }) => + syncSecrets({ + projectId, + secretPath: foldersGroupedById[folderId][0]?.path as string, + environmentSlug: foldersGroupedById[folderId][0]?.environmentSlug as string, + _deDupeQueue: deDupeQueue, + _depth: depth + 1, + excludeReplication: true + }) + ) + ); + } + + const secretReferences = await secretDAL.findReferencedSecretReferences( + projectId, + folder.environment.slug, + secretPath + ); + if (secretReferences.length) { + const referencedFolderIds = unique(secretReferences, (i) => i.folderId).map(({ folderId }) => folderId); + const referencedFolders = await folderDAL.findSecretPathByFolderIds(projectId, referencedFolderIds); + const referencedFoldersGroupedById = groupBy(referencedFolders.filter(Boolean), (i) => i?.id as string); + logger.info( + `getIntegrationSecrets: Syncing secret due to reference change [jobId=${job.id}] [projectId=${job.data.projectId}] [environment=${job.data.environment}] [secretPath=${job.data.secretPath}] [depth=${depth}]` + ); + await Promise.all( + secretReferences + .filter(({ folderId }) => Boolean(referencedFoldersGroupedById[folderId][0]?.path)) + // filter out already synced ones + .filter( + ({ folderId }) => + !deDupeQueue[ + uniqueSecretQueueKey( + referencedFoldersGroupedById[folderId][0]?.environmentSlug as string, + referencedFoldersGroupedById[folderId][0]?.path as string + ) + ] + ) + .map(({ folderId }) => + syncSecrets({ + projectId, + secretPath: referencedFoldersGroupedById[folderId][0]?.path as string, + environmentSlug: referencedFoldersGroupedById[folderId][0]?.environmentSlug as string, + _deDupeQueue: deDupeQueue, + _depth: depth + 1, + excludeReplication: true + }) + ) ); - if (secretReferences.length) { - const referencedFolderIds = unique(secretReferences, (i) => i.folderId).map(({ folderId }) => folderId); - const referencedFolders = await folderDAL.findSecretPathByFolderIds(projectId, referencedFolderIds); - const referencedFoldersGroupedById = groupBy(referencedFolders.filter(Boolean), (i) => i?.id as string); - logger.info( - `getIntegrationSecrets: Syncing secret due to reference change [jobId=${job.id}] [projectId=${job.data.projectId}] [environment=${job.data.environment}] [secretPath=${job.data.secretPath}] [depth=${depth}]` - ); - await Promise.all( - secretReferences - .filter(({ folderId }) => Boolean(referencedFoldersGroupedById[folderId][0]?.path)) - // filter out already synced ones - .filter( - ({ folderId }) => - !deDupeQueue[ - uniqueSecretQueueKey( - referencedFoldersGroupedById[folderId][0]?.environmentSlug as string, - referencedFoldersGroupedById[folderId][0]?.path as string - ) - ] - ) - .map(({ folderId }) => - syncSecrets({ - projectId, - secretPath: referencedFoldersGroupedById[folderId][0]?.path as string, - environmentSlug: referencedFoldersGroupedById[folderId][0]?.environmentSlug as string, - _deDupeQueue: deDupeQueue, - _depth: depth + 1, - excludeReplication: true - }) - ) - ); - } - } else { - logger.info(`getIntegrationSecrets: Secret depth exceeded for [projectId=${projectId}] [folderId=${folder.id}]`); } const integrations = await integrationDAL.findByProjectIdV2(projectId, environment); // note: returns array of integrations + integration auths in this environment @@ -571,10 +565,12 @@ export const secretQueueFactory = ({ syncMessage: "", isSynced: true }); - } catch (err: unknown) { + } catch (err) { logger.info("Secret integration sync error: %o", err); + const message = - err instanceof AxiosError ? JSON.stringify((err as AxiosError)?.response?.data) : (err as Error)?.message; + (err instanceof AxiosError ? JSON.stringify(err?.response?.data) : (err as Error)?.message) || + "Unknown error occurred."; await integrationDAL.updateById(integration.id, { lastSyncJobId: job.id, From f13930bc6b5796b49d59536017a1c96f1a9b4127 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Mon, 17 Jun 2024 13:14:46 +0200 Subject: [PATCH 2/5] Fix: Silent integration errors --- .../integration-sync-secret.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 6351b4d824..559ce838f2 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -557,6 +557,8 @@ const syncSecretsAWSParameterStore = async ({ `AWS Parameter Store Error [integration=${integration.id}]: double check AWS account permissions (refer to the Infisical docs)` ); } + + throw err; } } } @@ -603,7 +605,9 @@ const syncSecretsAWSSecretManager = async ({ }) => { const metadata = z.record(z.any()).parse(integration.metadata || {}); - if (!accessId) return; + if (!accessId) { + throw new Error("AWS access ID is required"); + } const secretsManager = new SecretsManagerClient({ region: integration.region as string, @@ -722,7 +726,7 @@ const syncSecretsAWSSecretManager = async ({ } } } catch (err) { - // case when AWS manager can't find the specified secret + // case 1: when AWS manager can't find the specified secret if (err instanceof ResourceNotFoundException && secretsManager) { await secretsManager.send( new CreateSecretCommand({ @@ -734,6 +738,9 @@ const syncSecretsAWSSecretManager = async ({ : [] }) ); + // case 2: something unexpected went wrong, so we'll throw the error to reflect the error in the integration sync status + } else { + throw err; } } }; @@ -753,14 +760,12 @@ const syncSecretsAWSSecretManager = async ({ const syncSecretsHeroku = async ({ createManySecretsRawFn, updateManySecretsRawFn, - integrationDAL, integration, secrets, accessToken }: { createManySecretsRawFn: (params: TCreateManySecretsRawFn) => Promise>; updateManySecretsRawFn: (params: TUpdateManySecretsRawFn) => Promise>; - integrationDAL: Pick; integration: TIntegrations & { projectId: string; environment: { @@ -862,10 +867,6 @@ const syncSecretsHeroku = async ({ } } ); - - await integrationDAL.updateById(integration.id, { - lastUsed: new Date() - }); }; /** @@ -2656,7 +2657,9 @@ const syncSecretsHashiCorpVault = async ({ accessId: string | null; accessToken: string; }) => { - if (!accessId) return; + if (!accessId) { + throw new Error("Access ID is required"); + } interface LoginAppRoleRes { auth: { @@ -3521,7 +3524,6 @@ export const syncIntegrationSecrets = async ({ await syncSecretsHeroku({ createManySecretsRawFn, updateManySecretsRawFn, - integrationDAL, integration, secrets, accessToken From a5cf6f40c75de1a80569b75757998580809cc411 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Mon, 17 Jun 2024 22:44:35 +0200 Subject: [PATCH 3/5] Update integration-sync-secret.ts --- .../src/services/integration-auth/integration-sync-secret.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 559ce838f2..cc20c0c019 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -556,9 +556,8 @@ const syncSecretsAWSParameterStore = async ({ logger.error( `AWS Parameter Store Error [integration=${integration.id}]: double check AWS account permissions (refer to the Infisical docs)` ); + throw err; } - - throw err; } } } From 719d0ea30f0ebccddf0f3793378e337b85dd4ac2 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Tue, 18 Jun 2024 12:33:36 +0200 Subject: [PATCH 4/5] Optional response --- .../integration-sync-secret.ts | 20 +++++++++++++++---- backend/src/services/secret/secret-queue.ts | 6 +++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index cc20c0c019..287722d950 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -18,7 +18,7 @@ import { UpdateSecretCommand } from "@aws-sdk/client-secrets-manager"; import { Octokit } from "@octokit/rest"; -import AWS from "aws-sdk"; +import AWS, { AWSError } from "aws-sdk"; import { AxiosError } from "axios"; import sodium from "libsodium-wrappers"; import isEqual from "lodash.isequal"; @@ -452,7 +452,9 @@ const syncSecretsAWSParameterStore = async ({ accessId: string | null; accessToken: string; }) => { - if (!accessId) return; + let response: { isSynced: boolean; syncMessage: string } | null = null; + + if (!accessId) return null; const config = new AWS.Config({ region: integration.region as string, @@ -556,8 +558,12 @@ const syncSecretsAWSParameterStore = async ({ logger.error( `AWS Parameter Store Error [integration=${integration.id}]: double check AWS account permissions (refer to the Infisical docs)` ); - throw err; } + + response = { + isSynced: false, + syncMessage: (err as AWSError)?.message || "Error syncing with AWS Parameter Store" + }; } } } @@ -586,6 +592,8 @@ const syncSecretsAWSParameterStore = async ({ } } } + + return response; }; /** @@ -3488,6 +3496,8 @@ export const syncIntegrationSecrets = async ({ accessToken: string; appendices?: { prefix: string; suffix: string }; }) => { + let response: { isSynced: boolean; syncMessage: string } | null = null; + switch (integration.integration) { case Integrations.GCP_SECRET_MANAGER: await syncSecretsGCPSecretManager({ @@ -3504,7 +3514,7 @@ export const syncIntegrationSecrets = async ({ }); break; case Integrations.AWS_PARAMETER_STORE: - await syncSecretsAWSParameterStore({ + response = await syncSecretsAWSParameterStore({ integration, secrets, accessId, @@ -3728,4 +3738,6 @@ export const syncIntegrationSecrets = async ({ default: throw new BadRequestError({ message: "Invalid integration" }); } + + return response; }; diff --git a/backend/src/services/secret/secret-queue.ts b/backend/src/services/secret/secret-queue.ts index fbd406a4c6..ba09994949 100644 --- a/backend/src/services/secret/secret-queue.ts +++ b/backend/src/services/secret/secret-queue.ts @@ -544,7 +544,7 @@ export const secretQueueFactory = ({ } try { - await syncIntegrationSecrets({ + const response = await syncIntegrationSecrets({ createManySecretsRawFn, updateManySecretsRawFn, integrationDAL, @@ -562,8 +562,8 @@ export const secretQueueFactory = ({ await integrationDAL.updateById(integration.id, { lastSyncJobId: job.id, lastUsed: new Date(), - syncMessage: "", - isSynced: true + syncMessage: response?.syncMessage || "", + isSynced: response?.isSynced || true }); } catch (err) { logger.info("Secret integration sync error: %o", err); From 9506b60d0219ee51271c8b7097ee015b28315722 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard <62331820+DanielHougaard@users.noreply.github.com> Date: Tue, 18 Jun 2024 14:11:19 +0200 Subject: [PATCH 5/5] Feat: Silent integration errors --- .../src/services/integration-auth/integration-sync-secret.ts | 4 +++- backend/src/services/secret/secret-queue.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 287722d950..70e3435a2a 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -454,7 +454,9 @@ const syncSecretsAWSParameterStore = async ({ }) => { let response: { isSynced: boolean; syncMessage: string } | null = null; - if (!accessId) return null; + if (!accessId) { + throw new Error("AWS access ID is required"); + } const config = new AWS.Config({ region: integration.region as string, diff --git a/backend/src/services/secret/secret-queue.ts b/backend/src/services/secret/secret-queue.ts index ba09994949..ac27d912f7 100644 --- a/backend/src/services/secret/secret-queue.ts +++ b/backend/src/services/secret/secret-queue.ts @@ -562,8 +562,8 @@ export const secretQueueFactory = ({ await integrationDAL.updateById(integration.id, { lastSyncJobId: job.id, lastUsed: new Date(), - syncMessage: response?.syncMessage || "", - isSynced: response?.isSynced || true + syncMessage: response?.syncMessage ?? "", + isSynced: response?.isSynced ?? true }); } catch (err) { logger.info("Secret integration sync error: %o", err);