Merge pull request #4164 from Infisical/fix/listSecretsThrowOnReadPermissionError

Throw when user does not have permission to read the referenced secret value

backend/src/services/secret-import/secret-import-fns.ts

@@ -174,6 +174,7 @@ export const fnSecretsV2FromImports = async ({
     skipMultilineEncoding?: boolean | null;
     secretPath: string;
     environment: string;
+    secretKey: string;
   }) => Promise<string | undefined>;
   hasSecretAccess: (environment: string, secretPath: string, secretName: string, secretTagSlugs: string[]) => boolean;
 }) => {
@@ -293,7 +294,8 @@ export const fnSecretsV2FromImports = async ({
               value: decryptedSecret.secretValue,
               secretPath: processedImport.secretPath,
               environment: processedImport.environment,
-              skipMultilineEncoding: decryptedSecret.skipMultilineEncoding
+              skipMultilineEncoding: decryptedSecret.skipMultilineEncoding,
+              secretKey: decryptedSecret.secretKey
             });
             // eslint-disable-next-line no-param-reassign
             processedImport.secrets[index].secretValue = expandedSecretValue || "";

backend/src/services/secret-sync/secret-sync-queue.ts

@@ -231,7 +231,8 @@ export const secretSyncQueueFactory = ({
           environment: environment.slug,
           secretPath: folder.path,
           skipMultilineEncoding: secret.skipMultilineEncoding,
-          value: secretValue
+          value: secretValue,
+          secretKey
         });
         secretMap[secretKey] = { value: expandedSecretValue || "" };
 

backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts

@@ -614,6 +614,7 @@ export const expandSecretReferencesFactory = ({
     secretPath: string;
     environment: string;
     shouldStackTrace?: boolean;
+    secretKey: string;
   }) => {
     const stackTrace = { ...dto, key: "root", children: [] } as TSecretReferenceTraceNode;
 
@@ -656,7 +657,7 @@ export const expandSecretReferencesFactory = ({
             const referredValue = await fetchSecret(environment, secretPath, secretKey);
             if (!canExpandValue(environment, secretPath, secretKey, referredValue.tags))
               throw new ForbiddenRequestError({
-                message: `You are attempting to reference secret named ${secretKey} from environment ${environment} in path ${secretPath} which you do not have access to read value on.`
+                message: `You do not have permission to read secret '${secretKey}' in environment '${environment}' at path '${secretPath}', which is referenced by secret '${dto.secretKey}' in environment '${dto.environment}' at path '${dto.secretPath}'.`
               });
 
             const cacheKey = getCacheUniqueKey(environment, secretPath);
@@ -675,7 +676,7 @@ export const expandSecretReferencesFactory = ({
             const referedValue = await fetchSecret(secretReferenceEnvironment, secretReferencePath, secretReferenceKey);
             if (!canExpandValue(secretReferenceEnvironment, secretReferencePath, secretReferenceKey, referedValue.tags))
               throw new ForbiddenRequestError({
-                message: `You are attempting to reference secret named ${secretReferenceKey} from environment ${secretReferenceEnvironment} in path ${secretReferencePath} which you do not have access to read value on.`
+                message: `You do not have permission to read secret '${secretReferenceKey}' in environment '${secretReferenceEnvironment}' at path '${secretReferencePath}', which is referenced by secret '${dto.secretKey}' in environment '${dto.environment}' at path '${dto.secretPath}'.`
               });
 
             const cacheKey = getCacheUniqueKey(secretReferenceEnvironment, secretReferencePath);
@@ -692,6 +693,7 @@ export const expandSecretReferencesFactory = ({
             secretPath: referencedSecretPath,
             environment: referencedSecretEnvironmentSlug,
             depth: depth + 1,
+            secretKey: referencedSecretKey,
             trace
           };
 
@@ -726,6 +728,7 @@ export const expandSecretReferencesFactory = ({
     skipMultilineEncoding?: boolean | null;
     secretPath: string;
     environment: string;
+    secretKey: string;
   }) => {
     if (!inputSecret.value) return inputSecret.value;
 
@@ -741,6 +744,7 @@ export const expandSecretReferencesFactory = ({
     value?: string;
     secretPath: string;
     environment: string;
+    secretKey: string;
   }) => {
     const { stackTrace, expandedValue } = await recursivelyExpandSecret({ ...inputSecret, shouldStackTrace: true });
     return { stackTrace, expandedValue };

backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts

@@ -1105,7 +1105,7 @@ export const secretV2BridgeServiceFactory = ({
 
     if (shouldExpandSecretReferences) {
       const secretsGroupByPath = groupBy(decryptedSecrets, (i) => i.secretPath);
-      await Promise.allSettled(
+      const settledPromises = await Promise.allSettled(
         Object.keys(secretsGroupByPath).map((groupedPath) =>
           Promise.allSettled(
             secretsGroupByPath[groupedPath].map(async (decryptedSecret, index) => {
@@ -1113,7 +1113,8 @@ export const secretV2BridgeServiceFactory = ({
                 value: decryptedSecret.secretValue,
                 secretPath: groupedPath,
                 environment,
-                skipMultilineEncoding: decryptedSecret.skipMultilineEncoding
+                skipMultilineEncoding: decryptedSecret.skipMultilineEncoding,
+                secretKey: decryptedSecret.secretKey
               });
               // eslint-disable-next-line no-param-reassign
               secretsGroupByPath[groupedPath][index].secretValue = expandedSecretValue || "";
@@ -1121,6 +1122,35 @@ export const secretV2BridgeServiceFactory = ({
           )
         )
       );
+      const errors: { path: string; error: string }[] = [];
+
+      settledPromises.forEach((outerResult: PromiseSettledResult<PromiseSettledResult<void>[]>, outerIndex) => {
+        const groupedPath = Object.keys(secretsGroupByPath)[outerIndex];
+
+        if (outerResult.status === "rejected") {
+          errors.push({
+            path: groupedPath,
+            error: `Failed to process secret group: ${outerResult.reason}`
+          });
+        } else {
+          // Check inner promise results
+          outerResult.value.forEach((innerResult: PromiseSettledResult<void>) => {
+            if (innerResult.status === "rejected") {
+              const reason = innerResult.reason as ForbiddenRequestError;
+              errors.push({
+                path: groupedPath,
+                error: reason.message
+              });
+            }
+          });
+        }
+      });
+      if (errors.length > 0) {
+        throw new ForbiddenRequestError({
+          message: "Failed to expand one or more secret references",
+          details: errors.map((err) => err.error)
+        });
+      }
     }
 
     if (!includeImports) {
@@ -1424,7 +1454,8 @@ export const secretV2BridgeServiceFactory = ({
         environment,
         secretPath: path,
         value: secretValue,
-        skipMultilineEncoding: secret.skipMultilineEncoding
+        skipMultilineEncoding: secret.skipMultilineEncoding,
+        secretKey: secret.key
       });
 
       secretValue = expandedSecretValue || "";
@@ -2722,7 +2753,8 @@ export const secretV2BridgeServiceFactory = ({
     const { expandedValue, stackTrace } = await getExpandedSecretStackTrace({
       environment,
       secretPath,
-      value: decryptedSecretValue
+      value: decryptedSecretValue,
+      secretKey: secretName
     });
 
     return { tree: stackTrace, value: expandedValue };

backend/src/services/secret/secret-queue.ts

@@ -426,7 +426,8 @@ export const secretQueueFactory = ({
           environment: dto.environment,
           secretPath: dto.secretPath,
           skipMultilineEncoding: secret.skipMultilineEncoding,
-          value: secretValue
+          value: secretValue,
+          secretKey
         });
         content[secretKey] = { value: expandedSecretValue || "" };