diff --git a/lib/util/settings.ts b/lib/util/settings.ts index 168f93aa3..4d91e90e2 100644 --- a/lib/util/settings.ts +++ b/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(); diff --git a/test/settings.test.ts b/test/settings.test.ts index e7df023f4..868d54640 100644 --- a/test/settings.test.ts +++ b/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");