From 3c7119cb2a2d3a3f9e3df2282a0dba8f11152375 Mon Sep 17 00:00:00 2001 From: saubyk <39208279+saubyk@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:29:32 -0700 Subject: [PATCH] Harden redaction helpers and secret restore paths --- backend/controllers/shared/RTLConf.js | 4 +- backend/utils/common.js | 53 +++++++---- backend/utils/config.js | 4 +- server/controllers/shared/RTLConf.ts | 4 +- server/utils/common.ts | 51 +++++++---- server/utils/config.ts | 4 +- test/backend/common.test.mjs | 36 ++++++++ test/backend/rtlconf.test.mjs | 121 ++++++++++++++++++++++++++ 8 files changed, 234 insertions(+), 43 deletions(-) diff --git a/backend/controllers/shared/RTLConf.js b/backend/controllers/shared/RTLConf.js index ac5c9251..a7b4b342 100644 --- a/backend/controllers/shared/RTLConf.js +++ b/backend/controllers/shared/RTLConf.js @@ -309,7 +309,9 @@ export const updateApplicationSettings = (req, res, next) => { }); fs.writeFileSync(RTLConfFile, JSON.stringify(fileConfig, null, 2), 'utf-8'); const newConfig = JSON.parse(JSON.stringify(common.appConfig)); - logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.maskPasswords(newConfig) }); + // removeSecureData clones, so the runtime config is untouched; it strips rtlPass, + // the TOTP seed, the SSO cookie and all per-node credentials symmetrically. + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.removeSecureData(newConfig) }); res.status(201).json(common.removeSecureData(newConfig)); } catch (errRes) { diff --git a/backend/utils/common.js b/backend/utils/common.js index ceff3000..86c6dde6 100644 --- a/backend/utils/common.js +++ b/backend/utils/common.js @@ -26,14 +26,17 @@ export class CommonService { const length = keys.length; if (length !== 0) { for (let i = 0; i < length; i++) { - if (typeof obj[keys[i]] === 'object') { - keys[keys[i]] = this.maskPasswords(obj[keys[i]]); + // Truthiness guard: null is 'object' too, and the recursive call mutates in + // place — assigning into keys[] here clobbered the key list for numeric keys. + if (obj[keys[i]] && typeof obj[keys[i]] === 'object') { + this.maskPasswords(obj[keys[i]]); } if (typeof keys[i] === 'string' && ((keys[i].toLowerCase().includes('password') && keys[i] !== 'allowPasswordUpdate') || keys[i].toLowerCase().includes('multipass') || keys[i].toLowerCase().includes('rpcpass') || keys[i].toLowerCase().includes('rpcpassword') || keys[i].toLowerCase().includes('rpcuser') || keys[i].toLowerCase().includes('secret2fa') || - keys[i].toLowerCase().includes('cookievalue'))) { + keys[i].toLowerCase().includes('cookievalue') || keys[i].toLowerCase().includes('rtlpass') || + keys[i].toLowerCase().includes('runevalue'))) { obj[keys[i]] = '*'.repeat(20); } } @@ -51,30 +54,41 @@ export class CommonService { return node; }; this.removeSecureData = (config) => { - delete config.rtlConfFilePath; - delete config.rtlPass; - delete config.multiPass; - delete config.multiPassHashed; - delete config.secret2FA; + // Clone before deleting: cookieValue is runtime-only, so mutating a caller's live + // appConfig would destroy SSO state with no way to restore it. + const sanitized = JSON.parse(JSON.stringify(config)); + delete sanitized.rtlConfFilePath; + delete sanitized.rtlPass; + delete sanitized.multiPass; + delete sanitized.multiPassHashed; + delete sanitized.secret2FA; // The SSO cookie is a live bearer credential; it must never leave the server. - if (config.SSO) { - delete config.SSO.cookieValue; + if (sanitized.SSO) { + delete sanitized.SSO.cookieValue; } - config.nodes?.forEach((node) => this.removeAuthSecureData(node)); - return config; + sanitized.nodes?.forEach((node) => this.removeAuthSecureData(node)); + return sanitized; }; this.addSecureData = (config) => { config.rtlConfFilePath = this.appConfig.rtlConfFilePath; config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; - config.SSO.rtlCookiePath = this.appConfig.SSO.rtlCookiePath; - // cookieValue is stripped from client responses, so a settings save can never echo it; - // restore the server-held value or the save would silently wipe the live SSO cookie. - config.SSO.cookieValue = this.appConfig.SSO.cookieValue; + // Merge rather than replace: sanitized client responses never carry cookieValue, and + // a trimmed or missing SSO object must not silently wipe server-held SSO state. The + // cookie is always pinned to the server-held value. + config.SSO = { + ...(this.appConfig.SSO || {}), + ...(config.SSO || {}), + rtlCookiePath: this.appConfig.SSO?.rtlCookiePath, + cookieValue: this.appConfig.SSO?.cookieValue + }; if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } - if (config.secret2FA === this.appConfig.secret2FA) { + // Restore the TOTP seed only when the client omits it (sanitized responses never + // include it, so an echo would otherwise wipe it). An explicit value is the settings + // UI's enable/disable flow and is honored. + if (config.secret2FA === undefined) { config.secret2FA = this.appConfig.secret2FA; } const appConfigNodes = new Map(this.appConfig.nodes?.map((node) => [node.index, node]) || []); @@ -353,10 +367,11 @@ export class CommonService { this.logger.log({ selectedNode: selectedNode, level: 'ERROR', fileName: fileName, msg: errMsg, error: (typeof err === 'object' ? JSON.stringify(err) : err) }); let newErrorObj = { statusCode: 500, message: '', error: '' }; if (err.code && err.code === 'ENOENT') { + // The absolute path stays in the server log above but is not echoed to clients. newErrorObj = { statusCode: 500, - message: 'No such file or directory ' + (err.path ? err.path : ''), - error: 'No such file or directory ' + (err.path ? err.path : '') + message: 'No such file or directory', + error: 'No such file or directory' }; } else { diff --git a/backend/utils/config.js b/backend/utils/config.js index 318e080b..da7f7d47 100644 --- a/backend/utils/config.js +++ b/backend/utils/config.js @@ -302,8 +302,8 @@ export class ConfigService { this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'Config', msg: 'Something went wrong while creating the backup directory: \n' + err }); } this.common.nodes[idx].settings.logFile = config.rtlConfFilePath + '/logs/RTL-Node-' + node.index + '.log'; - // maskPasswords keeps paths visible for debugging while redacting lnApiPassword - // and any other credential fields before they reach the log file. + // maskPasswords keeps paths visible for debugging while redacting credential + // fields such as lnApiPassword before they reach the log file. this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.maskPasswords(JSON.parse(JSON.stringify(this.common.nodes[idx])))) }); const log_file = this.common.nodes[idx].settings.logFile; if (fs.existsSync(log_file || '')) { diff --git a/server/controllers/shared/RTLConf.ts b/server/controllers/shared/RTLConf.ts index e84513ce..b3c05fd9 100644 --- a/server/controllers/shared/RTLConf.ts +++ b/server/controllers/shared/RTLConf.ts @@ -307,7 +307,9 @@ export const updateApplicationSettings = (req, res, next) => { }); fs.writeFileSync(RTLConfFile, JSON.stringify(fileConfig, null, 2), 'utf-8'); const newConfig = JSON.parse(JSON.stringify(common.appConfig)); - logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.maskPasswords(newConfig) }); + // removeSecureData clones, so the runtime config is untouched; it strips rtlPass, + // the TOTP seed, the SSO cookie and all per-node credentials symmetrically. + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.removeSecureData(newConfig) }); res.status(201).json(common.removeSecureData(newConfig)); } catch (errRes) { const errMsg = 'Update Default Node Error'; diff --git a/server/utils/common.ts b/server/utils/common.ts index c9145f90..d154acab 100644 --- a/server/utils/common.ts +++ b/server/utils/common.ts @@ -31,14 +31,17 @@ export class CommonService { const length = keys.length; if (length !== 0) { for (let i = 0; i < length; i++) { - if (typeof obj[keys[i]] === 'object') { - keys[keys[i]] = this.maskPasswords(obj[keys[i]]); + // Truthiness guard: null is 'object' too, and the recursive call mutates in + // place — assigning into keys[] here clobbered the key list for numeric keys. + if (obj[keys[i]] && typeof obj[keys[i]] === 'object') { + this.maskPasswords(obj[keys[i]]); } if (typeof keys[i] === 'string' && ((keys[i].toLowerCase().includes('password') && keys[i] !== 'allowPasswordUpdate') || keys[i].toLowerCase().includes('multipass') || keys[i].toLowerCase().includes('rpcpass') || keys[i].toLowerCase().includes('rpcpassword') || keys[i].toLowerCase().includes('rpcuser') || keys[i].toLowerCase().includes('secret2fa') || - keys[i].toLowerCase().includes('cookievalue')) + keys[i].toLowerCase().includes('cookievalue') || keys[i].toLowerCase().includes('rtlpass') || + keys[i].toLowerCase().includes('runevalue')) ) { obj[keys[i]] = '*'.repeat(20); } @@ -59,29 +62,40 @@ export class CommonService { }; public removeSecureData = (config: ApplicationConfig) => { - delete config.rtlConfFilePath; - delete config.rtlPass; - delete config.multiPass; - delete config.multiPassHashed; - delete config.secret2FA; + // Clone before deleting: cookieValue is runtime-only, so mutating a caller's live + // appConfig would destroy SSO state with no way to restore it. + const sanitized = JSON.parse(JSON.stringify(config)); + delete sanitized.rtlConfFilePath; + delete sanitized.rtlPass; + delete sanitized.multiPass; + delete sanitized.multiPassHashed; + delete sanitized.secret2FA; // The SSO cookie is a live bearer credential; it must never leave the server. - if (config.SSO) { delete config.SSO.cookieValue; } - config.nodes?.forEach((node) => this.removeAuthSecureData(node)); - return config; + if (sanitized.SSO) { delete sanitized.SSO.cookieValue; } + sanitized.nodes?.forEach((node) => this.removeAuthSecureData(node)); + return sanitized; }; public addSecureData = (config: ApplicationConfig) => { config.rtlConfFilePath = this.appConfig.rtlConfFilePath; config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; - config.SSO.rtlCookiePath = this.appConfig.SSO.rtlCookiePath; - // cookieValue is stripped from client responses, so a settings save can never echo it; - // restore the server-held value or the save would silently wipe the live SSO cookie. - config.SSO.cookieValue = this.appConfig.SSO.cookieValue; + // Merge rather than replace: sanitized client responses never carry cookieValue, and + // a trimmed or missing SSO object must not silently wipe server-held SSO state. The + // cookie is always pinned to the server-held value. + config.SSO = { + ...(this.appConfig.SSO || {}), + ...(config.SSO || {}), + rtlCookiePath: this.appConfig.SSO?.rtlCookiePath, + cookieValue: this.appConfig.SSO?.cookieValue + }; if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } - if (config.secret2FA === this.appConfig.secret2FA) { + // Restore the TOTP seed only when the client omits it (sanitized responses never + // include it, so an echo would otherwise wipe it). An explicit value is the settings + // UI's enable/disable flow and is honored. + if (config.secret2FA === undefined) { config.secret2FA = this.appConfig.secret2FA; } const appConfigNodes = new Map(this.appConfig.nodes?.map((node) => [node.index, node]) || []); @@ -370,10 +384,11 @@ export class CommonService { this.logger.log({ selectedNode: selectedNode, level: 'ERROR', fileName: fileName, msg: errMsg, error: (typeof err === 'object' ? JSON.stringify(err) : err) }); let newErrorObj = { statusCode: 500, message: '', error: '' }; if (err.code && err.code === 'ENOENT') { + // The absolute path stays in the server log above but is not echoed to clients. newErrorObj = { statusCode: 500, - message: 'No such file or directory ' + (err.path ? err.path : ''), - error: 'No such file or directory ' + (err.path ? err.path : '') + message: 'No such file or directory', + error: 'No such file or directory' }; } else { newErrorObj = { diff --git a/server/utils/config.ts b/server/utils/config.ts index ec3b8b85..62cbb78d 100644 --- a/server/utils/config.ts +++ b/server/utils/config.ts @@ -284,8 +284,8 @@ export class ConfigService { this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'Config', msg: 'Something went wrong while creating the backup directory: \n' + err }); } this.common.nodes[idx].settings.logFile = config.rtlConfFilePath + '/logs/RTL-Node-' + node.index + '.log'; - // maskPasswords keeps paths visible for debugging while redacting lnApiPassword - // and any other credential fields before they reach the log file. + // maskPasswords keeps paths visible for debugging while redacting credential + // fields such as lnApiPassword before they reach the log file. this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.maskPasswords(JSON.parse(JSON.stringify(this.common.nodes[idx])))) }); const log_file = this.common.nodes[idx].settings.logFile; if (fs.existsSync(log_file || '')) { diff --git a/test/backend/common.test.mjs b/test/backend/common.test.mjs index 6fe47d7d..a0c5cab3 100644 --- a/test/backend/common.test.mjs +++ b/test/backend/common.test.mjs @@ -39,3 +39,39 @@ test('removeSecureData strips the SSO cookie along with the other secrets', () = assert.equal(cleaned.SSO.rtlCookiePath, '/cookie-path'); assert.equal(cleaned.nodes[0].authentication.macaroonPath, undefined); }); + +test('removeSecureData does not mutate its input', () => { + // cookieValue is runtime-only: if a caller ever passes the live appConfig, an in-place + // delete would wipe SSO state with no way to restore it. The function must clone. + const config = { rtlPass: 'password-hash', SSO: { cookieValue: 'live-sso-cookie' }, nodes: [] }; + Common.removeSecureData(config); + assert.equal(config.rtlPass, 'password-hash'); + assert.equal(config.SSO.cookieValue, 'live-sso-cookie'); +}); + +test('maskPasswords masks rtlPass and runeValue', () => { + const config = { + rtlPass: 'login-hash', + nodes: [{ index: 1, authentication: { runeValue: 'cln-rune' } }] + }; + const masked = Common.maskPasswords(config); + assert.equal(masked.rtlPass, '*'.repeat(20)); + assert.equal(masked.nodes[0].authentication.runeValue, '*'.repeat(20)); +}); + +test('maskPasswords tolerates null values and numeric keys without skipping secrets', () => { + // Integer-like keys order first; the recursion must not clobber its own key list, and + // typeof null === 'object' must not send it into Object.keys(null). + const config = { '1': { nested: 'value' }, lnApiPassword: 'eclair-pass', nothing: null }; + const masked = Common.maskPasswords(config); + assert.equal(masked.lnApiPassword, '*'.repeat(20)); + assert.equal(masked.nothing, null); + assert.deepEqual(masked['1'], { nested: 'value' }); +}); + +test('handleError does not echo the absolute file path to the caller', () => { + // The path belongs in the server log, not in the API response. + const err = Common.handleError({ code: 'ENOENT', path: '/secret/dir/RTL-Config.json' }, 'Test', 'Reading Config Error', { lnImplementation: 'LND', settings: {} }); + assert.equal(err.error.includes('/secret/dir'), false); + assert.equal(err.message.includes('/secret/dir'), false); +}); diff --git a/test/backend/rtlconf.test.mjs b/test/backend/rtlconf.test.mjs index 8bf42826..e33bb5e3 100644 --- a/test/backend/rtlconf.test.mjs +++ b/test/backend/rtlconf.test.mjs @@ -208,3 +208,124 @@ test('updateApplicationSettings keeps the SSO cookie server-side without exposin rmSync(tempDir, { force: true, recursive: true }); } }); + +test('updateApplicationSettings restores omitted secret2FA and merges a trimmed SSO object', () => { + const tempDir = mkdtempSync(join(tmpdir(), 'rtlconf-secrets-')); + const oldConfig = { + defaultNodeIndex: 0, + dbDirectoryPath: '/db', + SSO: { rtlSSO: 0, rtlCookiePath: '/cookie-path', logoutRedirectLink: 'https://logout.example' }, + nodes: [ + { + index: 0, + lnNode: 'lnd-main', + lnImplementation: 'LND', + authentication: { macaroonPath: '/lnd/admin' }, + settings: { userPersona: 'OPERATOR', themeMode: 'DAY' } + } + ] + }; + const runtimeConfig = clone({ + ...oldConfig, + selectedNodeIndex: 0, + enable2FA: true, + allowPasswordUpdate: true, + rtlConfFilePath: tempDir, + rtlPass: 'hashed-password', + secret2FA: 'live-totp-seed', + SSO: { rtlSSO: 0, rtlCookiePath: '/cookie-path', logoutRedirectLink: 'https://logout.example', cookieValue: 'live-sso-cookie' } + }); + // Sanitized responses carry neither secret2FA nor cookieValue, so an echoing client + // omits both; a trimmed SSO object also lacks logoutRedirectLink. All three must + // survive the save server-side. + const requestBody = { + ...clone(oldConfig), + selectedNodeIndex: 0, + enable2FA: true, + allowPasswordUpdate: true, + SSO: { rtlSSO: 0 } + }; + + try { + Common.appConfig = clone(runtimeConfig); + Common.nodes = clone(runtimeConfig.nodes); + Common.selectedNode = Common.nodes[0]; + writeFileSync(join(tempDir, 'RTL-Config.json'), JSON.stringify(oldConfig, null, 2), 'utf-8'); + + let responseStatus; + updateApplicationSettings( + { body: clone(requestBody), session: { selectedNode: Common.selectedNode } }, + { + status: (status) => { + responseStatus = status; + return { json: () => {} }; + } + }, + null + ); + + assert.equal(responseStatus, 201); + assert.equal(Common.appConfig.secret2FA, 'live-totp-seed'); + assert.equal(Common.appConfig.enable2FA, true); + assert.equal(Common.appConfig.SSO.cookieValue, 'live-sso-cookie'); + assert.equal(Common.appConfig.SSO.logoutRedirectLink, 'https://logout.example'); + assert.equal(Common.appConfig.SSO.rtlSSO, 0); + const fileConfig = JSON.parse(readFileSync(join(tempDir, 'RTL-Config.json'), 'utf-8')); + assert.equal(fileConfig.SSO.cookieValue, undefined); + assert.equal(fileConfig.secret2FA, 'live-totp-seed'); + } finally { + clearInterval(WSServer.pingInterval); + rmSync(tempDir, { force: true, recursive: true }); + } +}); + +test('updateApplicationSettings tolerates a request body without an SSO object', () => { + const tempDir = mkdtempSync(join(tmpdir(), 'rtlconf-nosso-')); + const oldConfig = { + defaultNodeIndex: 0, + dbDirectoryPath: '/db', + SSO: { rtlSSO: 0, rtlCookiePath: '', logoutRedirectLink: '' }, + nodes: [ + { + index: 0, + lnNode: 'lnd-main', + lnImplementation: 'LND', + authentication: { macaroonPath: '/lnd/admin' }, + settings: { userPersona: 'OPERATOR', themeMode: 'DAY' } + } + ] + }; + const requestBody = clone(oldConfig); + delete requestBody.SSO; + + try { + Common.appConfig = clone({ + ...oldConfig, + selectedNodeIndex: 0, + rtlConfFilePath: tempDir, + rtlPass: 'hashed-password', + SSO: { rtlSSO: 0, rtlCookiePath: '', logoutRedirectLink: '', cookieValue: '' } + }); + Common.nodes = clone(oldConfig.nodes); + Common.selectedNode = Common.nodes[0]; + writeFileSync(join(tempDir, 'RTL-Config.json'), JSON.stringify(oldConfig, null, 2), 'utf-8'); + + let responseStatus; + updateApplicationSettings( + { body: requestBody, session: { selectedNode: Common.selectedNode } }, + { + status: (status) => { + responseStatus = status; + return { json: () => {} }; + } + }, + null + ); + + assert.equal(responseStatus, 201); + assert.equal(typeof Common.appConfig.SSO, 'object'); + } finally { + clearInterval(WSServer.pingInterval); + rmSync(tempDir, { force: true, recursive: true }); + } +});