diff --git a/backend/controllers/shared/RTLConf.js b/backend/controllers/shared/RTLConf.js index a7b4b342..9a0b5e5a 100644 --- a/backend/controllers/shared/RTLConf.js +++ b/backend/controllers/shared/RTLConf.js @@ -89,7 +89,8 @@ export const getFile = (req, res, next) => { return res.status(err.statusCode).json({ message: err.error, error: err.error }); } else { - logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'File Data Received', data: data }); + // File contents can carry node credentials; never write them to the log. + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'File Data Received' }); res.status(200).json(data); } }); @@ -109,7 +110,6 @@ export const getApplicationSettings = (req, res, next) => { delete appConfData.SSO.rtlCookiePath; delete appConfData.SSO.cookieValue; delete appConfData.SSO.logoutRedirectLink; - appConfData.secret2FA = ''; appConfData.dbDirectoryPath = ''; appConfData.nodes[selNodeIdx].authentication = new Authentication(); delete appConfData.nodes[selNodeIdx].settings.bitcoindConfigPath; @@ -281,7 +281,7 @@ export const updateApplicationSettings = (req, res, next) => { const newOnlyNodes = [...newNodesMap.values()].map((newNode) => JSON.parse(JSON.stringify(newNode))); runtimeConfig.nodes = [...updatedAndExistingNodes, ...newOnlyNodes]; } - common.appConfig = JSON.parse(JSON.stringify({ + const newAppConfig = JSON.parse(JSON.stringify({ ...runtimeConfig, selectedNodeIndex: config.selectedNodeIndex !== undefined ? config.selectedNodeIndex : common.appConfig.selectedNodeIndex, @@ -292,7 +292,7 @@ export const updateApplicationSettings = (req, res, next) => { rtlConfFilePath: common.appConfig.rtlConfFilePath, rtlPass: common.appConfig.rtlPass })); - const fileConfig = JSON.parse(JSON.stringify(common.appConfig)); + const fileConfig = JSON.parse(JSON.stringify(newAppConfig)); delete fileConfig.selectedNodeIndex; delete fileConfig.enable2FA; delete fileConfig.allowPasswordUpdate; @@ -307,12 +307,14 @@ export const updateApplicationSettings = (req, res, next) => { delete node.authentication?.options; delete node.authentication?.runeValue; }); + // Persist first and only then adopt the new runtime config, so a failed write + // (read-only volume, ENOSPC) cannot leave the process diverged from the file. fs.writeFileSync(RTLConfFile, JSON.stringify(fileConfig, null, 2), 'utf-8'); - const newConfig = JSON.parse(JSON.stringify(common.appConfig)); + common.appConfig = newAppConfig; // 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)); + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.removeSecureData(newAppConfig) }); + res.status(201).json(common.removeSecureData(newAppConfig)); } catch (errRes) { const errMsg = 'Update Default Node Error'; diff --git a/backend/utils/common.js b/backend/utils/common.js index 86c6dde6..8d87cb68 100644 --- a/backend/utils/common.js +++ b/backend/utils/common.js @@ -26,9 +26,15 @@ export class CommonService { const length = keys.length; if (length !== 0) { for (let i = 0; i < length; 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') { + // Header maps always carry credentials in this codebase (macaroon, rune, basic + // auth). Key-substring matching cannot catch them without also hiding the *Path + // fields the settings UI legitimately shows, so mask the whole map. + if (keys[i] === 'headers' && obj[keys[i]] && typeof obj[keys[i]] === 'object') { + Object.keys(obj[keys[i]]).forEach((headerKey) => { obj[keys[i]][headerKey] = '*'.repeat(20); }); + } + else if (obj[keys[i]] && typeof obj[keys[i]] === 'object') { + // Truthiness guard: null is 'object' too, and the recursive call mutates in + // place — assigning into keys[] here clobbered the key list for numeric keys. this.maskPasswords(obj[keys[i]]); } if (typeof keys[i] === 'string' && @@ -73,24 +79,25 @@ export class CommonService { config.rtlConfFilePath = this.appConfig.rtlConfFilePath; config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; - // 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 - }; + // Deployment-level switches are pinned to server-held values: the settings API must + // not flip the authentication mode (disableAuth, SSO) or move SSO fields, and no UI + // flow writes them. Pinning the whole object also means a trimmed or missing SSO + // object can never wipe server state. + config.disableAuth = this.appConfig.disableAuth; + config.SSO = JSON.parse(JSON.stringify(this.appConfig.SSO || {})); if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } - // 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) { + // Restore the TOTP seed when the client omits it — and when it sends an empty seed + // while still claiming 2FA is on (the pre-login config response carries that shape). + // An explicit non-empty seed is the settings UI's enable flow and is honored; an + // empty seed with enable2FA false is its disable flow and is honored too. + if (config.secret2FA === undefined || (config.secret2FA === '' && config.enable2FA)) { config.secret2FA = this.appConfig.secret2FA; } + // enable2FA derives from the seed, matching the boot-time derivation in config.ts, + // so the two fields can never diverge after a save. + config.enable2FA = !!config.secret2FA; const appConfigNodes = new Map(this.appConfig.nodes?.map((node) => [node.index, node]) || []); config.nodes?.forEach((node) => { const appConfigNode = appConfigNodes.get(node.index); diff --git a/server/controllers/shared/RTLConf.ts b/server/controllers/shared/RTLConf.ts index b3c05fd9..32625f11 100644 --- a/server/controllers/shared/RTLConf.ts +++ b/server/controllers/shared/RTLConf.ts @@ -91,7 +91,8 @@ export const getFile = (req, res, next) => { const err = common.handleError({ statusCode: 500, message: errMsg, error: errRes }, 'RTLConf', errMsg, req.session.selectedNode); return res.status(err.statusCode).json({ message: err.error, error: err.error }); } else { - logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'File Data Received', data: data }); + // File contents can carry node credentials; never write them to the log. + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'File Data Received' }); res.status(200).json(data); } }); @@ -112,7 +113,6 @@ export const getApplicationSettings = (req, res, next) => { delete appConfData.SSO.rtlCookiePath; delete appConfData.SSO.cookieValue; delete appConfData.SSO.logoutRedirectLink; - appConfData.secret2FA = ''; appConfData.dbDirectoryPath = ''; appConfData.nodes[selNodeIdx].authentication = new Authentication(); delete appConfData.nodes[selNodeIdx].settings.bitcoindConfigPath; @@ -281,7 +281,7 @@ export const updateApplicationSettings = (req, res, next) => { const newOnlyNodes = [...newNodesMap.values()].map((newNode) => JSON.parse(JSON.stringify(newNode))); runtimeConfig.nodes = [...updatedAndExistingNodes, ...newOnlyNodes]; } - common.appConfig = JSON.parse(JSON.stringify({ + const newAppConfig = JSON.parse(JSON.stringify({ ...runtimeConfig, selectedNodeIndex: config.selectedNodeIndex !== undefined ? config.selectedNodeIndex : common.appConfig.selectedNodeIndex, @@ -292,7 +292,7 @@ export const updateApplicationSettings = (req, res, next) => { rtlConfFilePath: common.appConfig.rtlConfFilePath, rtlPass: common.appConfig.rtlPass })); - const fileConfig = JSON.parse(JSON.stringify(common.appConfig)); + const fileConfig = JSON.parse(JSON.stringify(newAppConfig)); delete fileConfig.selectedNodeIndex; delete fileConfig.enable2FA; delete fileConfig.allowPasswordUpdate; @@ -305,12 +305,14 @@ export const updateApplicationSettings = (req, res, next) => { delete node.authentication?.options; delete node.authentication?.runeValue; }); + // Persist first and only then adopt the new runtime config, so a failed write + // (read-only volume, ENOSPC) cannot leave the process diverged from the file. fs.writeFileSync(RTLConfFile, JSON.stringify(fileConfig, null, 2), 'utf-8'); - const newConfig = JSON.parse(JSON.stringify(common.appConfig)); + common.appConfig = newAppConfig; // 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)); + logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'RTLConf', msg: 'Application Settings Updated', data: common.removeSecureData(newAppConfig) }); + res.status(201).json(common.removeSecureData(newAppConfig)); } catch (errRes) { const errMsg = 'Update Default Node Error'; const err = common.handleError({ statusCode: 500, message: errMsg, error: errRes }, 'RTLConf', errMsg, req.session.selectedNode); diff --git a/server/utils/common.ts b/server/utils/common.ts index d154acab..55769c09 100644 --- a/server/utils/common.ts +++ b/server/utils/common.ts @@ -31,9 +31,14 @@ export class CommonService { const length = keys.length; if (length !== 0) { for (let i = 0; i < length; 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') { + // Header maps always carry credentials in this codebase (macaroon, rune, basic + // auth). Key-substring matching cannot catch them without also hiding the *Path + // fields the settings UI legitimately shows, so mask the whole map. + if (keys[i] === 'headers' && obj[keys[i]] && typeof obj[keys[i]] === 'object') { + Object.keys(obj[keys[i]]).forEach((headerKey) => { obj[keys[i]][headerKey] = '*'.repeat(20); }); + } else if (obj[keys[i]] && typeof obj[keys[i]] === 'object') { + // Truthiness guard: null is 'object' too, and the recursive call mutates in + // place — assigning into keys[] here clobbered the key list for numeric keys. this.maskPasswords(obj[keys[i]]); } if (typeof keys[i] === 'string' && @@ -80,24 +85,25 @@ export class CommonService { config.rtlConfFilePath = this.appConfig.rtlConfFilePath; config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; - // 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 - }; + // Deployment-level switches are pinned to server-held values: the settings API must + // not flip the authentication mode (disableAuth, SSO) or move SSO fields, and no UI + // flow writes them. Pinning the whole object also means a trimmed or missing SSO + // object can never wipe server state. + config.disableAuth = this.appConfig.disableAuth; + config.SSO = JSON.parse(JSON.stringify(this.appConfig.SSO || {})); if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } - // 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) { + // Restore the TOTP seed when the client omits it — and when it sends an empty seed + // while still claiming 2FA is on (the pre-login config response carries that shape). + // An explicit non-empty seed is the settings UI's enable flow and is honored; an + // empty seed with enable2FA false is its disable flow and is honored too. + if (config.secret2FA === undefined || (config.secret2FA === '' && config.enable2FA)) { config.secret2FA = this.appConfig.secret2FA; } + // enable2FA derives from the seed, matching the boot-time derivation in config.ts, + // so the two fields can never diverge after a save. + config.enable2FA = !!config.secret2FA; const appConfigNodes = new Map(this.appConfig.nodes?.map((node) => [node.index, node]) || []); config.nodes?.forEach((node) => { const appConfigNode = appConfigNodes.get(node.index); diff --git a/test/backend/common.test.mjs b/test/backend/common.test.mjs index a0c5cab3..1054b530 100644 --- a/test/backend/common.test.mjs +++ b/test/backend/common.test.mjs @@ -75,3 +75,89 @@ test('handleError does not echo the absolute file path to the caller', () => { assert.equal(err.error.includes('/secret/dir'), false); assert.equal(err.message.includes('/secret/dir'), false); }); + +test('handleError keeps the absolute path out of controller-wrapped errors too', () => { + // RTLConf handlers pass { statusCode, message, error: errRes } wrappers; the response + // must resolve to the caller's generic message, never the wrapped fs error's path. + const err = Common.handleError( + { statusCode: 500, message: 'Reading File Error', error: { code: 'ENOENT', path: '/secret/dir/x.bak' } }, + 'Test', 'Reading File Error', { lnImplementation: 'LND', settings: {} } + ); + assert.equal(err.error.includes('/secret/dir'), false); + assert.equal(err.message.includes('/secret/dir'), false); +}); + +test('maskPasswords masks every value under a headers key', () => { + // Header values are always credential carriers here (macaroon, rune, basic auth), and + // key-substring matching cannot catch them without also hiding *Path fields. + const config = { + authentication: { + macaroonPath: '/visible/path', + options: { headers: { 'Grpc-Metadata-macaroon': 'deadbeef', rune: 'cln-rune', authorization: 'Basic xyz' } } + } + }; + const masked = Common.maskPasswords(config); + assert.equal(masked.authentication.options.headers['Grpc-Metadata-macaroon'], '*'.repeat(20)); + assert.equal(masked.authentication.options.headers.rune, '*'.repeat(20)); + assert.equal(masked.authentication.options.headers.authorization, '*'.repeat(20)); + assert.equal(masked.authentication.macaroonPath, '/visible/path'); +}); + +const seedAppConfig = () => { + Common.appConfig = { + defaultNodeIndex: 0, + selectedNodeIndex: 0, + rtlConfFilePath: '/conf', + dbDirectoryPath: '/db', + rtlPass: 'server-hash', + allowPasswordUpdate: true, + enable2FA: true, + secret2FA: 'server-seed', + disableAuth: false, + SSO: { rtlSSO: 0, rtlCookiePath: '/server-cookie', logoutRedirectLink: 'https://server-logout', cookieValue: 'server-cookie' }, + nodes: [] + }; + Common.selectedNode = null; + Common.nodes = []; +}; + +test('addSecureData pins disableAuth and the SSO object to server-held values', () => { + // The settings API must not be able to flip the authentication mode or move SSO fields; + // client-supplied values for these are deployment-level switches, not settings. + seedAppConfig(); + const config = Common.addSecureData({ + disableAuth: true, + SSO: { rtlSSO: 1, rtlCookiePath: '/client-path', logoutRedirectLink: 'https://client', cookieValue: 'client-cookie' }, + secret2FA: 'client-seed', + nodes: [] + }); + assert.equal(config.disableAuth, false); + assert.deepEqual(config.SSO, { rtlSSO: 0, rtlCookiePath: '/server-cookie', logoutRedirectLink: 'https://server-logout', cookieValue: 'server-cookie' }); + // An explicit non-empty seed is the settings UI's enable flow and is honored. + assert.equal(config.secret2FA, 'client-seed'); + assert.equal(config.enable2FA, true); +}); + +test('addSecureData restores an omitted TOTP seed and derives enable2FA from the seed', () => { + seedAppConfig(); + const config = Common.addSecureData({ nodes: [] }); + assert.equal(config.secret2FA, 'server-seed'); + assert.equal(config.enable2FA, true); +}); + +test('addSecureData treats an empty seed with 2FA claimed on as an omission', () => { + // The pre-login config response shape carries secret2FA: ''; echoing it must not wipe + // the seed while enable2FA stays on. + seedAppConfig(); + const config = Common.addSecureData({ secret2FA: '', enable2FA: true, nodes: [] }); + assert.equal(config.secret2FA, 'server-seed'); + assert.equal(config.enable2FA, true); +}); + +test('addSecureData honors an explicit seed wipe only when 2FA is disabled', () => { + // The settings UI's disable flow sends secret2FA: '' together with enable2FA: false. + seedAppConfig(); + const config = Common.addSecureData({ secret2FA: '', enable2FA: false, nodes: [] }); + assert.equal(config.secret2FA, ''); + assert.equal(config.enable2FA, false); +});