fix: Preserve secret references when persisting environment overrides (#33329)

lib/util/settings.ts

@@ -210,23 +210,6 @@ export function write(): void {
     // Read settings to check if we have to split devices/groups into separate file.
     const actual = yaml.read(CONFIG_FILE_PATH);
 
-    // In case the setting is defined in a separate file (e.g. !secret network_key) update it there.
-    for (const [ns, key] of [
-        ["mqtt", "server"],
-        ["mqtt", "user"],
-        ["mqtt", "password"],
-        ["advanced", "network_key"],
-        ["frontend", "auth_token"],
-    ]) {
-        if (actual[ns]?.[key]) {
-            const ref = parseValueRef(actual[ns][key]);
-            if (ref) {
-                yaml.updateIfChanged(data.joinPath(ref.filename), ref.key, toWrite[ns][key]);
-                toWrite[ns][key] = actual[ns][key];
-            }
-        }
-    }
-
     // Write devices/groups to separate file if required.
     const writeDevicesOrGroups = (type: "devices" | "groups"): void => {
         if (typeof actual[type] === "string" || (Array.isArray(actual[type]) && actual[type].length > 0)) {
@@ -255,6 +238,26 @@ export function write(): void {
 
     applyEnvironmentVariables(toWrite);
 
+    // In case the setting is defined in a separate file (e.g. !secret network_key) update it there.
+    for (const [ns, key] of [
+        ["mqtt", "server"],
+        ["mqtt", "user"],
+        ["mqtt", "password"],
+        ["advanced", "network_key"],
+        ["frontend", "auth_token"],
+    ]) {
+        if (actual[ns]?.[key]) {
+            const ref = parseValueRef(actual[ns][key]);
+            const value = toWrite[ns][key];
+            // An environment override may itself be a reference; never write it into another secret.
+            const valueIsReference = typeof value === "string" && parseValueRef(value) !== null;
+            if (ref && !valueIsReference) {
+                yaml.updateIfChanged(data.joinPath(ref.filename), ref.key, value);
+                toWrite[ns][key] = actual[ns][key];
+            }
+        }
+    }
+
     yaml.writeIfChanged(CONFIG_FILE_PATH, toWrite);
 
     _settings = read();

test/settings.test.ts

@@ -202,7 +202,7 @@ describe("Settings", () => {
         writeAndCheck();
     });
 
-    it("Should write environment variables as overrides to configuration.yaml, not in the ref file", () => {
+    it("Should preserve secret references when writing environment overrides", () => {
         write(secretFile, {password: "password-in-secret-file"}, false);
         write(configurationFile, {mqtt: {password: "!secret password", server: "server"}});
         process.env.ZIGBEE2MQTT_CONFIG_MQTT_PASSWORD = "password-in-env-var";
@@ -216,8 +216,8 @@ describe("Settings", () => {
             expected.mqtt.password = "password-in-env-var";
             expected.mqtt.server = "server";
             expect(s).toStrictEqual(expected);
-            expect(read(secretFile)).toMatchObject({password: "password-in-secret-file"});
-            expect(read(configurationFile)).toMatchObject({mqtt: {password: "password-in-env-var", server: "server"}});
+            expect(read(secretFile)).toMatchObject({password: "password-in-env-var"});
+            expect(read(configurationFile)).toMatchObject({mqtt: {password: "!secret password", server: "server"}});
         };
 
         // Write trice to ensure there are no side effects.
@@ -226,6 +226,19 @@ describe("Settings", () => {
         writeAndCheck();
     });
 
+    it("Should prefer a secret reference from the environment over the configuration reference", () => {
+        write(secretFile, {password: "original-password", override: "environment-password"}, false);
+        write(configurationFile, {mqtt: {password: "!secret password", server: "server"}});
+        process.env.ZIGBEE2MQTT_CONFIG_MQTT_PASSWORD = "!secret.yaml override";
+
+        for (let i = 0; i < 3; i++) {
+            settings.set(["advanced", "log_level"], "debug");
+            expect(settings.get().mqtt.password).toBe("environment-password");
+            expect(read(configurationFile)).toHaveProperty("mqtt.password", "!secret.yaml override");
+            expect(read(secretFile)).toStrictEqual({password: "original-password", override: "environment-password"});
+        }
+    });
+
     it("Should add devices", () => {
         write(configurationFile, {});
         settings.addDevice("0x12345678");