From b593061da3446684cad0d11986b55f0747beff31 Mon Sep 17 00:00:00 2001
From: JP Dhabolt <jdhabolt@agero.com>
Date: Wed, 31 Mar 2021 15:05:53 +0000
Subject: [PATCH] Fix bug in substitution transform

Signed-off-by: JP Dhabolt john.p.dhabolt@gmail.com
Signed-off-by: JP Dhabolt <jdhabolt@agero.com>
---
 .changeset/fair-geese-yell.md                 | 15 +++----
 .../src/lib/transform/substitution.test.ts    |  5 ++-
 .../src/lib/transform/substitution.ts         |  9 ++++-
 packages/config-loader/src/loader.test.ts     | 40 +++++++++++++++++++
 4 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/.changeset/fair-geese-yell.md b/.changeset/fair-geese-yell.md
index 5ca86b8913..00c5b16854 100644
--- a/.changeset/fair-geese-yell.md
+++ b/.changeset/fair-geese-yell.md
@@ -2,13 +2,14 @@
 '@backstage/config-loader': minor
 ---
 
+Fix bug where `$${...}` was not being escaped to `${...}`
+
 Add support for environment variable substitution in `$include`, `$file` and
 `$env` transform values.
 
-This change allows for including dynamic paths, such as environment specific
-secrets by using the same environment variable substitution (`${..}`) already
-supported outside of the various include transforms.
-
-If you are currently using the syntax `${...}` in your include transform values,
-you will need to escape the substitution by using `$${...}` instead to maintain
-the same behavior.
+- This change allows for including dynamic paths, such as environment specific
+  secrets by using the same environment variable substitution (`${..}`) already
+  supported outside of the various include transforms.
+- If you are currently using the syntax `${...}` in your include transform values,
+  you will need to escape the substitution by using `$${...}` instead to maintain
+  the same behavior.
diff --git a/packages/config-loader/src/lib/transform/substitution.test.ts b/packages/config-loader/src/lib/transform/substitution.test.ts
index e2c0200bfd..ef557431dd 100644
--- a/packages/config-loader/src/lib/transform/substitution.test.ts
+++ b/packages/config-loader/src/lib/transform/substitution.test.ts
@@ -51,7 +51,7 @@ describe('substituteTransform', () => {
     });
     await expect(
       substituteTransform('${SECRET      } $${} ${TOKEN }', '/'),
-    ).resolves.toEqual({ applied: true, value: 'my-secret $${} my-token' });
+    ).resolves.toEqual({ applied: true, value: 'my-secret ${} my-token' });
     await expect(substituteTransform('foo ${MISSING}', '/')).resolves.toEqual({
       applied: true,
       value: undefined,
@@ -62,5 +62,8 @@ describe('substituteTransform', () => {
     await expect(
       substituteTransform('foo ${SECRET} ${SECRET}', '/'),
     ).resolves.toEqual({ applied: true, value: 'foo my-secret my-secret' });
+    await expect(
+      substituteTransform('foo ${SECRET} $$${ESCAPE_ME}', '/'),
+    ).resolves.toEqual({ applied: true, value: 'foo my-secret $${ESCAPE_ME}' });
   });
 });
diff --git a/packages/config-loader/src/lib/transform/substitution.ts b/packages/config-loader/src/lib/transform/substitution.ts
index 821cc8bc67..482f1221a9 100644
--- a/packages/config-loader/src/lib/transform/substitution.ts
+++ b/packages/config-loader/src/lib/transform/substitution.ts
@@ -29,8 +29,13 @@ export function createSubstitutionTransform(env: EnvFunc): TransformFunc {
     }
 
     const parts: (string | undefined)[] = input.split(/(?<!\$)\$\{([^{}]+)\}/);
-    for (let i = 1; i < parts.length; i += 2) {
-      parts[i] = await env(parts[i]!.trim());
+    for (let i = 0; i < parts.length; i++) {
+      if (i % 2 === 0) {
+        parts[i] = parts[i]!.replace(/\$\${(.*?)\}/, '${$1}');
+      }
+      if (i % 2 === 1) {
+        parts[i] = await env(parts[i]!.trim());
+      }
     }
     if (parts.some(part => part === undefined)) {
       return { applied: true, value: undefined };
diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts
index 8c7de557e3..27b1e20789 100644
--- a/packages/config-loader/src/loader.test.ts
+++ b/packages/config-loader/src/loader.test.ts
@@ -20,6 +20,7 @@ import mockFs from 'mock-fs';
 describe('loadConfig', () => {
   beforeAll(() => {
     process.env.MY_SECRET = 'is-secret';
+    process.env.SUBSTITUTE_ME = 'substituted';
 
     mockFs({
       '/root/app-config.yaml': `
@@ -27,6 +28,7 @@ describe('loadConfig', () => {
           title: Example App
           sessionKey:
             $file: secrets/session-key.txt
+          escaped: \$\${Escaped}
       `,
       '/root/app-config.development.yaml': `
         app:
@@ -45,6 +47,19 @@ describe('loadConfig', () => {
         foo:
           bar: token \${MY_SECRET}
       `,
+      '/root/app-config.substitute.yaml': `
+        app:
+          someConfig:
+            $include: \${SUBSTITUTE_ME}.yaml
+          noSubstitute:
+            $file: \$\${ESCAPE_ME}.txt
+      `,
+      '/root/substituted.yaml': `
+        secret:
+          $file: secrets/\${SUBSTITUTE_ME}.txt
+      `,
+      '/root/secrets/substituted.txt': '123abc',
+      '/root/${ESCAPE_ME}.txt': 'notSubstituted',
     });
   });
 
@@ -66,6 +81,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -86,6 +102,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -109,6 +126,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -130,4 +148,26 @@ describe('loadConfig', () => {
       },
     ]);
   });
+
+  it('loads deep substituted config', async () => {
+    await expect(
+      loadConfig({
+        configRoot: '/root',
+        configPaths: ['/root/app-config.substitute.yaml'],
+        env: 'development',
+      }),
+    ).resolves.toEqual([
+      {
+        context: 'app-config.substitute.yaml',
+        data: {
+          app: {
+            someConfig: {
+              secret: '123abc',
+            },
+            noSubstitute: 'notSubstituted',
+          },
+        },
+      },
+    ]);
+  });
 });