diff --git a/backend/e2e-test/routes/v3/secret-reference.spec.ts b/backend/e2e-test/routes/v3/secret-reference.spec.ts index ad5699c877d..7c22b9c843f 100644 --- a/backend/e2e-test/routes/v3/secret-reference.spec.ts +++ b/backend/e2e-test/routes/v3/secret-reference.spec.ts @@ -141,6 +141,100 @@ describe("Secret expansion", () => { await Promise.all(secrets.map((el) => deleteSecretV2(el))); }); + test("Local secret reference to non-existent secret keeps literal reference", async () => { + const secrets = [ + { + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken, + key: "TEST", + // eslint-disable-next-line + value: "hello ${NON_EXISTENT_SECRET}" + } + ]; + + for (const secret of secrets) { + // eslint-disable-next-line no-await-in-loop + await createSecretV2(secret); + } + + const expandedSecret = await getSecretByNameV2({ + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken, + key: "TEST" + }); + // eslint-disable-next-line + expect(expandedSecret.secretValue).toBe("hello ${NON_EXISTENT_SECRET}"); + + const listSecrets = await getSecretsV2({ + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken + }); + expect(listSecrets.secrets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + secretKey: "TEST", + // eslint-disable-next-line + secretValue: "hello ${NON_EXISTENT_SECRET}" + }) + ]) + ); + + await Promise.all(secrets.map((el) => deleteSecretV2(el))); + }); + + test("Local secret reference with repeated non-existent secret keeps literal references", async () => { + const secrets = [ + { + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken, + key: "TEST", + // eslint-disable-next-line + value: "${MISSING} ${MISSING}" + } + ]; + + for (const secret of secrets) { + // eslint-disable-next-line no-await-in-loop + await createSecretV2(secret); + } + + const expandedSecret = await getSecretByNameV2({ + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken, + key: "TEST" + }); + // eslint-disable-next-line + expect(expandedSecret.secretValue).toBe("${MISSING} ${MISSING}"); + + const listSecrets = await getSecretsV2({ + environmentSlug: seedData1.environment.slug, + workspaceId: projectId, + secretPath: "/", + authToken: jwtAuthToken + }); + expect(listSecrets.secrets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + secretKey: "TEST", + // eslint-disable-next-line + secretValue: "${MISSING} ${MISSING}" + }) + ]) + ); + + await Promise.all(secrets.map((el) => deleteSecretV2(el))); + }); + test("Cross environment secret reference", async () => { const secrets = [ { diff --git a/backend/src/services/secret-v2-bridge/secret-reference-fns.ts b/backend/src/services/secret-v2-bridge/secret-reference-fns.ts index 332db86ee3a..35c56333170 100644 --- a/backend/src/services/secret-v2-bridge/secret-reference-fns.ts +++ b/backend/src/services/secret-v2-bridge/secret-reference-fns.ts @@ -87,45 +87,57 @@ export const expandSecretReferencesFactory = ({ canExpandValue, userId }: TInterpolateSecretArg) => { - const secretCache: Record> = {}; + const secretCache: Record> = {}; const getCacheUniqueKey = (environment: string, secretPath: string) => `${environment}-${secretPath}`; - const fetchSecret = async (environment: string, secretPath: string, secretKey: string) => { + const fetchSecret = async ( + environment: string, + secretPath: string, + secretKey: string + ): Promise<{ value: string; tags: string[]; exists: boolean }> => { const cacheKey = getCacheUniqueKey(environment, secretPath); if (secretCache?.[cacheKey]) { - return secretCache[cacheKey][secretKey] || { value: "", tags: [] }; + const cachedSecret = secretCache[cacheKey][secretKey]; + if (cachedSecret) return { ...cachedSecret }; + return { value: "", tags: [], exists: false }; } try { const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); - if (!folder) return { value: "", tags: [] }; + if (!folder) return { value: "", tags: [], exists: false }; // When userId is provided, findByFolderId returns both shared and personal secrets. // Personal overrides will take precedence over shared secrets in the reduce below. const secrets = await secretDAL.findByFolderId({ folderId: folder.id, userId }); - const decryptedSecret = secrets.reduce>((prev, secret) => { - // When userId is set, personal overrides (userId !== null) should take precedence - // over shared secrets for the same key. We skip overwriting if a personal override - // is already stored and the current secret is a shared one. - if (userId && prev[secret.key] && !secret.userId) { - return prev; - } + const decryptedSecret = secrets.reduce>( + (prev, secret) => { + // When userId is set, personal overrides (userId !== null) should take precedence + // over shared secrets for the same key. We skip overwriting if a personal override + // is already stored and the current secret is a shared one. + if (userId && prev[secret.key] && !secret.userId) { + return prev; + } - // eslint-disable-next-line no-param-reassign - prev[secret.key] = { - value: decryptSecret(secret.encryptedValue) || "", - tags: secret.tags?.map((el) => el.slug) - }; - return prev; - }, {}); + // eslint-disable-next-line no-param-reassign + prev[secret.key] = { + value: decryptSecret(secret.encryptedValue) || "", + tags: secret.tags?.map((el) => el.slug), + exists: true + }; + return prev; + }, + {} + ); secretCache[cacheKey] = decryptedSecret; - return secretCache[cacheKey][secretKey] || { value: "", tags: [] }; + const fetchedSecret = secretCache[cacheKey][secretKey]; + if (fetchedSecret) return { ...fetchedSecret }; + return { value: "", tags: [], exists: false }; } catch (error) { secretCache[cacheKey] = {}; - return { value: "", tags: [] }; + return { value: "", tags: [], exists: false }; } }; @@ -174,6 +186,7 @@ export const expandSecretReferencesFactory = ({ let referencedSecretKey = ""; let referencedSecretEnvironmentSlug = ""; let referencedSecretValue = ""; + let referencedSecretExists = false; if (entities.length === 1) { const [secretKey] = entities; @@ -190,6 +203,7 @@ export const expandSecretReferencesFactory = ({ secretCache[cacheKey][secretKey] = referredValue; referencedSecretValue = referredValue.value; + referencedSecretExists = referredValue.exists; referencedSecretKey = secretKey; referencedSecretPath = secretPath; referencedSecretEnvironmentSlug = environment; @@ -210,6 +224,7 @@ export const expandSecretReferencesFactory = ({ secretCache[cacheKey][secretReferenceKey] = referedValue; referencedSecretValue = referedValue.value; + referencedSecretExists = referedValue.exists; referencedSecretKey = secretReferenceKey; referencedSecretPath = secretReferencePath; referencedSecretEnvironmentSlug = secretReferenceEnvironment; @@ -247,10 +262,13 @@ export const expandSecretReferencesFactory = ({ stack.push({ ...node, visitedSecrets: newVisitedSecrets }); } - expandedValue = expandedValue.replaceAll( - interpolationSyntax, - () => referencedSecretValue // prevents special characters from triggering replacement patterns - ); + if (referencedSecretExists) { + expandedValue = expandedValue.replaceAll( + interpolationSyntax, + () => referencedSecretValue // prevents special characters from triggering replacement patterns + ); + } + // when the referenced secret does not exist, leave the literal ${REF} untouched } } }