From 7b9c5a427365648e3db0b73eaaf1525659c91093 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Wed, 10 Jun 2026 09:11:52 +0200 Subject: [PATCH] fix(security): prevent unauthorized secret expansion in socket payloads --- js/app.js | 2 ++ js/node_helper.js | 25 ++++++++++++++++- js/server_functions.js | 28 +++++++++++++------ tests/unit/functions/server_functions_spec.js | 27 ++++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/js/app.js b/js/app.js index af8310992a..c79d40e44e 100644 --- a/js/app.js +++ b/js/app.js @@ -187,6 +187,8 @@ function App () { try { const configObj = Utils.loadConfig(); global.config = configObj.fullConf; + // Keep a copy of the redacted config to later verify module secret permissions + global.configRedacted = configObj.redactedConf; const config = global.config; Utils.checkConfigFile(configObj); diff --git a/js/node_helper.js b/js/node_helper.js index 833c90aa73..1883bd6b66 100644 --- a/js/node_helper.js +++ b/js/node_helper.js @@ -2,6 +2,26 @@ const express = require("express"); const Log = require("logger"); const { replaceSecretPlaceholder } = require("#server_functions"); +/** + * Determine which secrets a module is allowed to restore. A module may only + * restore the `**SECRET_***` placeholders that appear in its own config — the + * exact inverse of how the config is redacted before it is sent to the browser. + * @param {string} moduleName - Name of the module. + * @returns {Set} The secret names the module may restore. + */ +function getAllowedSecrets (moduleName) { + const modules = global.configRedacted?.modules || []; + const moduleConfig = modules.find((m) => m.module === moduleName); + const allowed = new Set(); + if (moduleConfig) { + // Stringify the config to easily find all expected **SECRET_*** placeholders + for (const [, secretName] of JSON.stringify(moduleConfig).matchAll(/\*\*(SECRET_[^*]+)\*\*/g)) { + allowed.add(secretName); + } + } + return allowed; +} + class NodeHelper { init () { Log.log("Initializing new module helper ..."); @@ -90,7 +110,10 @@ class NodeHelper { socket.onAny((notification, payload) => { if (config?.hideConfigSecrets && payload && typeof payload === "object") { try { - const payloadStr = replaceSecretPlaceholder(JSON.stringify(payload)); + // Calculate exactly which secrets this module is allowed to receive + const allowedSecrets = getAllowedSecrets(this.name); + // Expand only these safe, module-specific secrets in the payload + const payloadStr = replaceSecretPlaceholder(JSON.stringify(payload), allowedSecrets); this.socketNotificationReceived(notification, JSON.parse(payloadStr)); } catch (e) { Log.error("Error substituting variables in payload: ", e); diff --git a/js/server_functions.js b/js/server_functions.js index 2d55bc3b44..55f9b81ca5 100644 --- a/js/server_functions.js +++ b/js/server_functions.js @@ -17,21 +17,31 @@ function getStartup (req, res) { } /** - * A method that replaces the secret placeholders `**SECRET_ABC**` with the environment variable SECRET_ABC - * @param {string} input - the input string - * @returns {string} the input with real variable content + * Replace `**SECRET_ABC**` placeholders with the value of `process.env.SECRET_ABC`. + * + * If `allowedSecrets` is given, only those secret names are restored and every + * other placeholder is left untouched. Without it, all secrets are restored + * (used by the CORS proxy, which only runs on the trusted server side). + * @param {string} input - String that may contain `**SECRET_***` placeholders. + * @param {Set} [allowedSecrets] - Secret names that may be restored. + * @returns {string} The input with the allowed placeholders replaced. */ -function replaceSecretPlaceholder (input) { - if (global.config.cors !== "allowAll") { - return input.replaceAll(/\*\*(SECRET_[^*]+)\*\*/g, (match, group) => { - return process.env[group]; - }); - } else { +function replaceSecretPlaceholder (input, allowedSecrets) { + if (global.config.cors === "allowAll") { if (input.includes("**SECRET_")) { Log.error("Replacing secrets doesn't work with CORS `allowAll`, you need to set `cors` to `disabled` or `allowWhitelist` in `config.js`"); } return input; } + return input.replaceAll(/\*\*(SECRET_[^*]+)\*\*/g, (placeholder, secretName) => { + // Block replacing secrets that are not explicitly allowed. + if (allowedSecrets && !allowedSecrets.has(secretName)) { + return placeholder; + } + + // Load the real value from the environment. Fallback to placeholder if missing. + return process.env[secretName] || placeholder; + }); } /** diff --git a/tests/unit/functions/server_functions_spec.js b/tests/unit/functions/server_functions_spec.js index 8c38cf9f0a..cd43eaf2bd 100644 --- a/tests/unit/functions/server_functions_spec.js +++ b/tests/unit/functions/server_functions_spec.js @@ -47,6 +47,33 @@ describe("server_functions tests", () => { }); }); + describe("The replaceSecretPlaceholder method with an allowedSecrets set", () => { + beforeEach(() => { + global.config = { cors: "allowWhitelist" }; + process.env.SECRET_ALLOWED = "allowed-value"; + process.env.SECRET_DENIED = "denied-value"; + }); + + it("Restores only allowed secrets and keeps denied placeholders untouched", () => { + const teststring = "allowed=**SECRET_ALLOWED** denied=**SECRET_DENIED**"; + const result = replaceSecretPlaceholder(teststring, new Set(["SECRET_ALLOWED"])); + expect(result).toBe("allowed=allowed-value denied=**SECRET_DENIED**"); + expect(result).not.toContain("denied-value"); + }); + + it("Does not restore any placeholder when the set is empty", () => { + const teststring = "value=**SECRET_ALLOWED**"; + const result = replaceSecretPlaceholder(teststring, new Set()); + expect(result).toBe(teststring); + }); + + it("Falls back to the placeholder if the allowed secret doesn't exist in environment", () => { + const teststring = "value=**SECRET_MISSING**"; + const result = replaceSecretPlaceholder(teststring, new Set(["SECRET_MISSING"])); + expect(result).toBe(teststring); + }); + }); + describe("The cors method", () => { let fetchSpy; let fetchResponseHeadersGet;