Merge pull request #24196 from backstage/rugvip/trim

config: trim strings when reading config

.changeset/chatty-parrots-trade.md

@@ -0,0 +1,13 @@
+---
+'@backstage/config-loader': minor
+---
+
+The default environment variable substitution function will now trim whitespace characters from the substituted value. This alleviates bugs where whitespace characters are mistakenly included in environment variables.
+
+If you depend on the old behavior, you can override the default substitution function with your own, for example:
+
+```ts
+ConfigSources.default({
+  substitutionFunc: async name => process.env[name],
+});
+```

packages/config-loader/api-report.md

@@ -25,7 +25,6 @@ export interface BaseConfigSourcesOptions {
   remote?: Pick<RemoteConfigSourceOptions, 'reloadInterval'>;
   // (undocumented)
   rootDir?: string;
-  // (undocumented)
   substitutionFunc?: EnvFunc;
   // (undocumented)
   watch?: boolean;

packages/config-loader/src/sources/ConfigSources.ts

@@ -74,6 +74,14 @@ export interface BaseConfigSourcesOptions {
   watch?: boolean;
   rootDir?: string;
   remote?: Pick<RemoteConfigSourceOptions, 'reloadInterval'>;
+
+  /**
+   * A custom substitution function that overrides the default one.
+   *
+   * @remarks
+   * The substitution function handles syntax like `${MY_ENV_VAR}` in configuration values.
+   * The default substitution will read the value from the environment and trim whitespace.
+   */
   substitutionFunc?: SubstitutionFunc;
 }
 

packages/config-loader/src/sources/transform/apply.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { applyConfigTransforms } from './apply';
+import { applyConfigTransforms, createConfigTransformer } from './apply';
 
 describe('applyConfigTransforms', () => {
   it('should apply no transforms to input', async () => {
@@ -84,3 +84,32 @@ describe('applyConfigTransforms', () => {
     });
   });
 });
+
+describe('createConfigTransformer', () => {
+  const origEnv = process.env;
+  process.env = {
+    ...process.env,
+    SECRET: 'my-secret',
+    PADDED_SECRET: ' \nmy-space \t',
+  };
+
+  afterAll(() => {
+    process.env = origEnv;
+  });
+
+  it('should substitute environment variables', async () => {
+    const transformer = createConfigTransformer({});
+
+    await expect(
+      transformer({
+        testAlone: '${SECRET}',
+        testMiddle: 'hello ${SECRET}!',
+        testSpace: 'hello ${PADDED_SECRET}!',
+      }),
+    ).resolves.toEqual({
+      testAlone: 'my-secret',
+      testMiddle: 'hello my-secret!',
+      testSpace: 'hello my-space!',
+    });
+  });
+});

packages/config-loader/src/sources/transform/apply.ts

@@ -105,8 +105,10 @@ export function createConfigTransformer(options: {
   substitutionFunc?: SubstitutionFunc;
   readFile?(path: string): Promise<string>;
 }): ConfigTransformer {
-  const { substitutionFunc = async name => process.env[name], readFile } =
-    options;
+  const {
+    substitutionFunc = async name => process.env[name]?.trim(),
+    readFile,
+  } = options;
   const substitutionTransform = createSubstitutionTransform(substitutionFunc);
   const transforms = [substitutionTransform];
   if (readFile) {