From f5343e7c1aea3877df023d53481d49912c458e7e Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 8 Jan 2022 16:23:26 +0100
Subject: [PATCH] config: make get always return a clone

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .changeset/clean-wolves-jog.md     |  5 ++++
 packages/config/src/reader.test.ts | 47 ++++++++++++++++++++++++++++++
 packages/config/src/reader.ts      |  9 ++----
 3 files changed, 55 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/clean-wolves-jog.md

diff --git a/.changeset/clean-wolves-jog.md b/.changeset/clean-wolves-jog.md
new file mode 100644
index 0000000000..70ab23715f
--- /dev/null
+++ b/.changeset/clean-wolves-jog.md
@@ -0,0 +1,5 @@
+---
+'@backstage/config': patch
+---
+
+The `ConfigReader#get` method now always returns a deep clone of the configuration data.
diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 2472222c2b..c0627a200b 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -673,4 +673,51 @@ describe('ConfigReader.get()', () => {
       },
     });
   });
+
+  it('should return deep clones of the backing data', () => {
+    const data1 = {
+      foo: {
+        bar: [],
+        baz: {},
+      },
+    };
+    const data2 = {
+      x: {
+        y: {
+          z: {},
+        },
+      },
+    };
+
+    const reader = ConfigReader.fromConfigs([
+      { data: data1, context: '1' },
+      { data: data2, context: '2' },
+    ]);
+
+    reader.get<any>().foo.bar.push(1);
+    reader.get<any>('foo').bar.push(1);
+    reader.get<any>('foo.bar').push(1);
+    reader.get<any>().foo.baz.x = 1;
+    reader.get<any>('foo').baz.x = 1;
+    reader.get<any>('foo.baz').x = 1;
+    reader.get<any>().x.y.z.w = 1;
+    reader.get<any>('x').y.z.w = 1;
+    reader.get<any>('x.y').z.w = 1;
+    reader.get<any>('x.y.z').w = 1;
+
+    const readerSingle = ConfigReader.fromConfigs([
+      { data: data1, context: '1' },
+    ]);
+
+    readerSingle.get<any>().foo.bar.push(1);
+    readerSingle.get<any>('foo').bar.push(1);
+    readerSingle.get<any>('foo.bar').push(1);
+    readerSingle.get<any>().foo.baz.x = 1;
+    readerSingle.get<any>('foo').baz.x = 1;
+    readerSingle.get<any>('foo.baz').x = 1;
+
+    expect(data1.foo.bar).toEqual([]);
+    expect(data1.foo.baz).toEqual({});
+    expect(data2.x.y.z).toEqual({});
+  });
 });
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index 8c0c4140a5..129a703784 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -126,7 +126,7 @@ export class ConfigReader implements Config {
 
   /** {@inheritdoc Config.getOptional} */
   getOptional<T = JsonValue>(key?: string): T | undefined {
-    const value = this.readValue(key);
+    const value = cloneDeep(this.readValue(key));
     const fallbackValue = this.fallback?.getOptional<T>(key);
 
     if (value === undefined) {
@@ -153,11 +153,8 @@ export class ConfigReader implements Config {
 
     // Avoid merging arrays and primitive values, since that's how merging works for other
     // methods for reading config.
-    return mergeWith(
-      {},
-      { value: cloneDeep(fallbackValue) },
-      { value },
-      (into, from) => (!isObject(from) || !isObject(into) ? from : undefined),
+    return mergeWith({}, { value: fallbackValue }, { value }, (into, from) =>
+      !isObject(from) || !isObject(into) ? from : undefined,
     ).value as T;
   }