Fix remote execution bug where email address can contain malicious code

also convert almost all cmd execs for certificates to properly escape arguments
2025-08-23 09:20:23 +00:00 · 2025-08-20 09:53:13 +10:00
parent 54d463ac36
commit 8c9d2745e2
9 changed files with 305 additions and 222 deletions
--- a/backend/internal/access-list.js
+++ b/backend/internal/access-list.js
@@ -1,5 +1,5 @@
 const _                     = require('lodash');
-const fs                    = require('fs');
+const fs                    = require('node:fs');
 const batchflow             = require('batchflow');
 const logger                = require('../logger').access;
 const error                 = require('../lib/error');
@@ -38,7 +38,7 @@ const internalAccessList = {
 			.then((row) => {
 				data.id = row.id;

-				let promises = [];
+				const promises = [];

 				// Now add the items
 				data.items.map((item) => {
@@ -116,7 +116,7 @@ const internalAccessList = {
 			.then((row) => {
 				if (row.id !== data.id) {
 					// Sanity check that something crazy hasn't happened
-					throw new error.InternalValidationError('Access List could not be updated, IDs do not match: ' + row.id + ' !== ' + data.id);
+					throw new error.InternalValidationError(`Access List could not be updated, IDs do not match: ${row.id} !== ${data.id}`);
 				}
 			})
 			.then(() => {
@@ -135,10 +135,10 @@ const internalAccessList = {
 			.then(() => {
 				// Check for items and add/update/remove them
 				if (typeof data.items !== 'undefined' && data.items) {
-					let promises      = [];
-					let items_to_keep = [];
+					const promises      = [];
+					const items_to_keep = [];

-					data.items.map(function (item) {
+					data.items.map((item) => {
 						if (item.password) {
 							promises.push(accessListAuthModel
 								.query()
@@ -154,7 +154,7 @@ const internalAccessList = {
 						}
 					});

-					let query = accessListAuthModel
+					const query = accessListAuthModel
 						.query()
 						.delete()
 						.where('access_list_id', data.id);
@@ -175,9 +175,9 @@ const internalAccessList = {
 			.then(() => {
 				// Check for clients and add/update/remove them
 				if (typeof data.clients !== 'undefined' && data.clients) {
-					let promises = [];
+					const promises = [];

-					data.clients.map(function (client) {
+					data.clients.map((client) => {
 						if (client.address) {
 							promises.push(accessListClientModel
 								.query()
@@ -190,7 +190,7 @@ const internalAccessList = {
 						}
 					});

-					let query = accessListClientModel
+					const query = accessListClientModel
 						.query()
 						.delete()
 						.where('access_list_id', data.id);
@@ -249,7 +249,7 @@ const internalAccessList = {

 		return access.can('access_lists:get', data.id)
 			.then((access_data) => {
-				let query = accessListModel
+				const query = accessListModel
 					.query()
 					.select('access_list.*', accessListModel.raw('COUNT(proxy_host.id) as proxy_host_count'))
 					.leftJoin('proxy_host', function() {
@@ -267,7 +267,7 @@ const internalAccessList = {
 				}

 				if (typeof data.expand !== 'undefined' && data.expand !== null) {
-					query.withGraphFetched('[' + data.expand.join(', ') + ']');
+					query.withGraphFetched(`[${data.expand.join(', ')}]`);
 				}

 				return query.then(utils.omitRow(omissions()));
@@ -327,7 +327,7 @@ const internalAccessList = {
 									// 3. reconfigure those hosts, then reload nginx

 									// set the access_list_id to zero for these items
-									row.proxy_hosts.map(function (val, idx) {
+									row.proxy_hosts.map((_val, idx) => {
 										row.proxy_hosts[idx].access_list_id = 0;
 									});

@@ -340,11 +340,11 @@ const internalAccessList = {
 					})
 					.then(() => {
 						// delete the htpasswd file
-						let htpasswd_file = internalAccessList.getFilename(row);
+						const htpasswd_file = internalAccessList.getFilename(row);

 						try {
 							fs.unlinkSync(htpasswd_file);
-						} catch (err) {
+						} catch (_err) {
 							// do nothing
 						}
 					})
@@ -374,7 +374,7 @@ const internalAccessList = {
 	getAll: (access, expand, search_query) => {
 		return access.can('access_lists:list')
 			.then((access_data) => {
-				let query = accessListModel
+				const query = accessListModel
 					.query()
 					.select('access_list.*', accessListModel.raw('COUNT(proxy_host.id) as proxy_host_count'))
 					.leftJoin('proxy_host', function() {
@@ -393,19 +393,19 @@ const internalAccessList = {
 				// Query is used for searching
 				if (typeof search_query === 'string') {
 					query.where(function () {
-						this.where('name', 'like', '%' + search_query + '%');
+						this.where('name', 'like', `%${search_query}%`);
 					});
 				}

 				if (typeof expand !== 'undefined' && expand !== null) {
-					query.withGraphFetched('[' + expand.join(', ') + ']');
+					query.withGraphFetched(`[${expand.join(', ')}]`);
 				}

 				return query.then(utils.omitRows(omissions()));
 			})
 			.then((rows) => {
 				if (rows) {
-					rows.map(function (row, idx) {
+					rows.map((row, idx) => {
 						if (typeof row.items !== 'undefined' && row.items) {
 							rows[idx] = internalAccessList.maskItems(row);
 						}
@@ -424,7 +424,7 @@ const internalAccessList = {
 	 * @returns {Promise}
 	 */
 	getCount: (user_id, visibility) => {
-		let query = accessListModel
+		const query = accessListModel
 			.query()
 			.count('id as count')
 			.where('is_deleted', 0);
@@ -445,7 +445,7 @@ const internalAccessList = {
 	 */
 	maskItems: (list) => {
 		if (list && typeof list.items !== 'undefined') {
-			list.items.map(function (val, idx) {
+			list.items.map((val, idx) => {
 				let repeat_for = 8;
 				let first_char = '*';

@@ -468,7 +468,7 @@ const internalAccessList = {
 	 * @returns {String}
 	 */
 	getFilename: (list) => {
-		return '/data/access/' + list.id;
+		return `/data/access/${list.id}`;
 	},

 	/**
@@ -479,15 +479,15 @@ const internalAccessList = {
 	 * @returns {Promise}
 	 */
 	build: (list) => {
-		logger.info('Building Access file #' + list.id + ' for: ' + list.name);
+		logger.info(`Building Access file #${list.id} for: ${list.name}`);

 		return new Promise((resolve, reject) => {
-			let htpasswd_file = internalAccessList.getFilename(list);
+			const htpasswd_file = internalAccessList.getFilename(list);

 			// 1. remove any existing access file
 			try {
 				fs.unlinkSync(htpasswd_file);
-			} catch (err) {
+			} catch (_err) {
 				// do nothing
 			}

@@ -504,14 +504,14 @@ const internalAccessList = {
 				if (list.items.length) {
 					return new Promise((resolve, reject) => {
 						batchflow(list.items).sequential()
-							.each((i, item, next) => {
+							.each((_i, item, next) => {
 								if (typeof item.password !== 'undefined' && item.password.length) {
-									logger.info('Adding: ' + item.username);
+									logger.info(`Adding: ${item.username}`);

 									utils.execFile('openssl', ['passwd', '-apr1', item.password])
 										.then((res) => {
 											try {
-												fs.appendFileSync(htpasswd_file, item.username + ':' + res + '\n', {encoding: 'utf8'});
+												fs.appendFileSync(htpasswd_file, `${item.username}:${res}\n`, {encoding: 'utf8'});
 											} catch (err) {
 												reject(err);
 											}
@@ -528,7 +528,7 @@ const internalAccessList = {
 								reject(err);
 							})
 							.end((results) => {
-								logger.success('Built Access file #' + list.id + ' for: ' + list.name);
+								logger.success(`Built Access file #${list.id} for: ${list.name}`);
 								resolve(results);
 							});
 					});
--- a/backend/internal/certificate.js
+++ b/backend/internal/certificate.js
@@ -1,6 +1,6 @@
 const _                = require('lodash');
-const fs               = require('fs');
-const https            = require('https');
+const fs               = require('node:fs');
+const https            = require('node:https');
 const tempWrite        = require('temp-write');
 const moment           = require('moment');
 const archiver         = require('archiver');
@@ -49,7 +49,7 @@ const internalCertificate = {
 	processExpiringHosts: () => {
 		if (!internalCertificate.intervalProcessing) {
 			internalCertificate.intervalProcessing = true;
-			logger.info('Renewing SSL certs expiring within ' + internalCertificate.renewBeforeExpirationBy[0] + ' ' + internalCertificate.renewBeforeExpirationBy[1] + ' ...');
+			logger.info(`Renewing SSL certs expiring within ${internalCertificate.renewBeforeExpirationBy[0]} ${internalCertificate.renewBeforeExpirationBy[1]} ...`);

 			const expirationThreshold = moment().add(internalCertificate.renewBeforeExpirationBy[0], internalCertificate.renewBeforeExpirationBy[1]).format('YYYY-MM-DD HH:mm:ss');

@@ -70,7 +70,7 @@ const internalCertificate = {
 					 */
 					let sequence = Promise.resolve();

-					certificates.forEach(function (certificate) {
+					certificates.forEach((certificate) => {
 						sequence = sequence.then(() =>
 							internalCertificate
 								.renew(
@@ -202,7 +202,7 @@ const internalCertificate = {
 						.then(() => {
 							// At this point, the letsencrypt cert should exist on disk.
 							// Lets get the expiry date from the file and update the row silently
-							return internalCertificate.getCertificateInfoFromFile('/etc/letsencrypt/live/npm-' + certificate.id + '/fullchain.pem')
+							return internalCertificate.getCertificateInfoFromFile(`${internalCertificate.getLiveCertPath(certificate.id)}/fullchain.pem`)
 								.then((cert_info) => {
 									return certificateModel
 										.query()
@@ -263,7 +263,7 @@ const internalCertificate = {
 			.then((row) => {
 				if (row.id !== data.id) {
 					// Sanity check that something crazy hasn't happened
-					throw new error.InternalValidationError('Certificate could not be updated, IDs do not match: ' + row.id + ' !== ' + data.id);
+					throw new error.InternalValidationError(`Certificate could not be updated, IDs do not match: ${row.id} !== ${data.id}`);
 				}

 				return certificateModel
@@ -308,7 +308,7 @@ const internalCertificate = {

 		return access.can('certificates:get', data.id)
 			.then((access_data) => {
-				let query = certificateModel
+				const query = certificateModel
 					.query()
 					.where('is_deleted', 0)
 					.andWhere('id', data.id)
@@ -323,7 +323,7 @@ const internalCertificate = {
 				}

 				if (typeof data.expand !== 'undefined' && data.expand !== null) {
-					query.withGraphFetched('[' + data.expand.join(', ') + ']');
+					query.withGraphFetched(`[${data.expand.join(', ')}]`);
 				}

 				return query.then(utils.omitRow(omissions()));
@@ -354,17 +354,17 @@ const internalCertificate = {
 				})
 				.then((certificate) => {
 					if (certificate.provider === 'letsencrypt') {
-						const zipDirectory = '/etc/letsencrypt/live/npm-' + data.id;
+						const zipDirectory = internalCertificate.getLiveCertPath(data.id);

 						if (!fs.existsSync(zipDirectory)) {
-							throw new error.ItemNotFoundError('Certificate ' + certificate.nice_name + ' does not exists');
+							throw new error.ItemNotFoundError(`Certificate ${certificate.nice_name} does not exists`);
 						}

-						let certFiles      = fs.readdirSync(zipDirectory)
+						const certFiles    = fs.readdirSync(zipDirectory)
 							.filter((fn) => fn.endsWith('.pem'))
 							.map((fn) => fs.realpathSync(path.join(zipDirectory, fn)));
-						const downloadName = 'npm-' + data.id + '-' + `${Date.now()}.zip`;
-						const opName       = '/tmp/' + downloadName;
+						const downloadName = `npm-${data.id}-${Date.now()}.zip`;
+						const opName       = `/tmp/${downloadName}`;
 						internalCertificate.zipFiles(certFiles, opName)
 							.then(() => {
 								logger.debug('zip completed : ', opName);
@@ -392,7 +392,7 @@ const internalCertificate = {
 		return new Promise((resolve, reject) => {
 			source
 				.map((fl) => {
-					let fileName = path.basename(fl);
+					const fileName = path.basename(fl);
 					logger.debug(fl, 'added to certificate zip');
 					archive.file(fl, { name: fileName });
 				});
@@ -462,7 +462,7 @@ const internalCertificate = {
 	getAll: (access, expand, search_query) => {
 		return access.can('certificates:list')
 			.then((access_data) => {
-				let query = certificateModel
+				const query = certificateModel
 					.query()
 					.where('is_deleted', 0)
 					.groupBy('id')
@@ -479,12 +479,12 @@ const internalCertificate = {
 				// Query is used for searching
 				if (typeof search_query === 'string') {
 					query.where(function () {
-						this.where('nice_name', 'like', '%' + search_query + '%');
+						this.where('nice_name', 'like', `%${search_query}%`);
 					});
 				}

 				if (typeof expand !== 'undefined' && expand !== null) {
-					query.withGraphFetched('[' + expand.join(', ') + ']');
+					query.withGraphFetched(`[${expand.join(', ')}]`);
 				}

 				return query.then(utils.omitRows(omissions()));
@@ -499,7 +499,7 @@ const internalCertificate = {
 	 * @returns {Promise}
 	 */
 	getCount: (user_id, visibility) => {
-		let query = certificateModel
+		const query = certificateModel
 			.query()
 			.count('id as count')
 			.where('is_deleted', 0);
@@ -521,7 +521,7 @@ const internalCertificate = {
 	writeCustomCert: (certificate) => {
 		logger.info('Writing Custom Certificate:', certificate);

-		const dir = '/data/custom_ssl/npm-' + certificate.id;
+		const dir = `/data/custom_ssl/npm-${certificate.id}`;

 		return new Promise((resolve, reject) => {
 			if (certificate.provider === 'letsencrypt') {
@@ -531,7 +531,7 @@ const internalCertificate = {

 			let certData = certificate.meta.certificate;
 			if (typeof certificate.meta.intermediate_certificate !== 'undefined') {
-				certData = certData + '\n' + certificate.meta.intermediate_certificate;
+				certData = `${certData}\n${certificate.meta.intermediate_certificate}`;
 			}

 			try {
@@ -543,7 +543,7 @@ const internalCertificate = {
 				return;
 			}

-			fs.writeFile(dir + '/fullchain.pem', certData, function (err) {
+			fs.writeFile(`${dir}/fullchain.pem`, certData, (err) => {
 				if (err) {
 					reject(err);
 				} else {
@@ -553,7 +553,7 @@ const internalCertificate = {
 		})
 			.then(() => {
 				return new Promise((resolve, reject) => {
-					fs.writeFile(dir + '/privkey.pem', certificate.meta.certificate_key, function (err) {
+					fs.writeFile(`${dir}/privkey.pem`, certificate.meta.certificate_key, (err) => {
 						if (err) {
 							reject(err);
 						} else {
@@ -591,7 +591,7 @@ const internalCertificate = {
 	validate: (data) => {
 		return new Promise((resolve) => {
 			// Put file contents into an object
-			let files = {};
+			const files = {};
 			_.map(data.files, (file, name) => {
 				if (internalCertificate.allowedSslFiles.indexOf(name) !== -1) {
 					files[name] = file.data.toString();
@@ -603,7 +603,7 @@ const internalCertificate = {
 			.then((files) => {
 				// For each file, create a temp file and write the contents to it
 				// Then test it depending on the file type
-				let promises = [];
+				const promises = [];
 				_.map(files, (content, type) => {
 					promises.push(new Promise((resolve) => {
 						if (type === 'certificate_key') {
@@ -688,11 +688,11 @@ const internalCertificate = {
 						reject(new error.ValidationError('Result Validation Error: Validation timed out. This could be due to the key being passphrase-protected.'));
 					}, 10000);
 					utils
-						.exec('openssl pkey -in ' + filepath + ' -check -noout 2>&1 ')
+						.exec(`openssl pkey -in ${filepath} -check -noout 2>&1 `)
 						.then((result) => {
 							clearTimeout(failTimeout);
 							if (!result.toLowerCase().includes('key is valid')) {
-								reject(new error.ValidationError('Result Validation Error: ' + result));
+								reject(new error.ValidationError(`Result Validation Error: ${result}`));
 							}
 							fs.unlinkSync(filepath);
 							resolve(true);
@@ -700,7 +700,7 @@ const internalCertificate = {
 						.catch((err) => {
 							clearTimeout(failTimeout);
 							fs.unlinkSync(filepath);
-							reject(new error.ValidationError('Certificate Key is not valid (' + err.message + ')', err));
+							reject(new error.ValidationError(`Certificate Key is not valid (${err.message})`, err));
 						});
 				});
 			});
@@ -735,9 +735,9 @@ const internalCertificate = {
 	 * @param {Boolean} [throw_expired]  Throw when the certificate is out of date
 	 */
 	getCertificateInfoFromFile: (certificate_file, throw_expired) => {
-		let certData = {};
+		const certData = {};

-		return utils.exec('openssl x509 -in ' + certificate_file + ' -subject -noout')
+		return utils.execFile('openssl', ['x509', '-in', certificate_file, '-subject', '-noout'])
 			.then((result) => {
 				// Examples:
 				// subject=CN = *.jc21.com
@@ -745,11 +745,11 @@ const internalCertificate = {
 				const regex = /(?:subject=)?[^=]+=\s+(\S+)/gim;
 				const match = regex.exec(result);
 				if (match && typeof match[1] !== 'undefined') {
-					certData['cn'] = match[1];
+					certData.cn = match[1];
 				}
 			})
 			.then(() => {
-				return utils.exec('openssl x509 -in ' + certificate_file + ' -issuer -noout');
+				return utils.execFile('openssl', ['x509', '-in', certificate_file, '-issuer', '-noout']);
 			})

 			.then((result) => {
@@ -760,11 +760,11 @@ const internalCertificate = {
 				const regex = /^(?:issuer=)?(.*)$/gim;
 				const match = regex.exec(result);
 				if (match && typeof match[1] !== 'undefined') {
-					certData['issuer'] = match[1];
+					certData.issuer = match[1];
 				}
 			})
 			.then(() => {
-				return utils.exec('openssl x509 -in ' + certificate_file + ' -dates -noout');
+				return utils.execFile('openssl', ['x509', '-in', certificate_file, '-dates', '-noout']);
 			})
 			.then((result) => {
 				// notBefore=Jul 14 04:04:29 2018 GMT
@@ -773,7 +773,7 @@ const internalCertificate = {
 				let validTo   = null;

 				const lines = result.split('\n');
-				lines.map(function (str) {
+				lines.map((str) => {
 					const regex = /^(\S+)=(.*)$/gim;
 					const match = regex.exec(str.trim());

@@ -789,21 +789,21 @@ const internalCertificate = {
 				});

 				if (!validFrom || !validTo) {
-					throw new error.ValidationError('Could not determine dates from certificate: ' + result);
+					throw new error.ValidationError(`Could not determine dates from certificate: ${result}`);
 				}

 				if (throw_expired && validTo < parseInt(moment().format('X'), 10)) {
 					throw new error.ValidationError('Certificate has expired');
 				}

-				certData['dates'] = {
+				certData.dates = {
 					from: validFrom,
 					to:   validTo
 				};

 				return certData;
 			}).catch((err) => {
-				throw new error.ValidationError('Certificate is not valid (' + err.message + ')', err);
+				throw new error.ValidationError(`Certificate is not valid (${err.message})`, err);
 			});
 	},

@@ -814,7 +814,7 @@ const internalCertificate = {
 	 * @param   {Boolean} [remove]
 	 * @returns {Object}
 	 */
-	cleanMeta: function (meta, remove) {
+	cleanMeta: (meta, remove) => {
 		internalCertificate.allowedSslFiles.map((key) => {
 			if (typeof meta[key] !== 'undefined' && meta[key]) {
 				if (remove) {
@@ -834,24 +834,35 @@ const internalCertificate = {
 	 * @returns {Promise}
 	 */
 	requestLetsEncryptSsl: (certificate) => {
-		logger.info('Requesting Let\'sEncrypt certificates for Cert #' + certificate.id + ': ' + certificate.domain_names.join(', '));
+		logger.info(`Requesting LetsEncrypt certificates for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);

-		const cmd = `${certbotCommand} certonly ` +
-			`--config '${letsencryptConfig}' ` +
-			'--work-dir "/tmp/letsencrypt-lib" ' +
-			'--logs-dir "/tmp/letsencrypt-log" ' +
-			`--cert-name "npm-${certificate.id}" ` +
-			'--agree-tos ' +
-			'--authenticator webroot ' +
-			`--email '${certificate.meta.letsencrypt_email}' ` +
-			'--preferred-challenges "dns,http" ' +
-			`--domains "${certificate.domain_names.join(',')}" ` +
-			(letsencryptServer !== null ? `--server '${letsencryptServer}' ` : '') +
-			(letsencryptStaging && letsencryptServer === null ? '--staging ' : '');
+		const args = [
+			'certonly',
+			'--config',
+			letsencryptConfig,
+			'--work-dir',
+			'/tmp/letsencrypt-lib',
+			'--logs-dir',
+			'/tmp/letsencrypt-log',
+			'--cert-name',
+			`npm-${certificate.id}`,
+			'--agree-tos',
+			'--authenticator',
+			'webroot',
+			'--email',
+			certificate.meta.letsencrypt_email,
+			'--preferred-challenges',
+			'dns,http',
+			'--domains',
+			certificate.domain_names.join(','),
+		];

-		logger.info('Command:', cmd);
+		const adds = internalCertificate.getAdditionalCertbotArgs(certificate.id);
+		args.push(...adds.args);

-		return utils.exec(cmd)
+		logger.info(`Command: ${certbotCommand} ${args ? args.join(' ') : ''}`);
+
+		return utils.execFile(certbotCommand, args, adds.opts)
 			.then((result) => {
 				logger.success(result);
 				return result;
@@ -868,50 +879,48 @@ const internalCertificate = {
 	requestLetsEncryptSslWithDnsChallenge: async (certificate) => {
 		await certbot.installPlugin(certificate.meta.dns_provider);
 		const dnsPlugin = dnsPlugins[certificate.meta.dns_provider];
-		logger.info(`Requesting Let'sEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);
+		logger.info(`Requesting LetsEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);

-		const credentialsLocation = '/etc/letsencrypt/credentials/credentials-' + certificate.id;
+		const credentialsLocation = `/etc/letsencrypt/credentials/credentials-${certificate.id}`;
 		fs.mkdirSync('/etc/letsencrypt/credentials', { recursive: true });
 		fs.writeFileSync(credentialsLocation, certificate.meta.dns_provider_credentials, {mode: 0o600});

 		// Whether the plugin has a --<name>-credentials argument
 		const hasConfigArg = certificate.meta.dns_provider !== 'route53';

-		let mainCmd = certbotCommand + ' certonly ' +
-			`--config '${letsencryptConfig}' ` +
-			'--work-dir "/tmp/letsencrypt-lib" ' +
-			'--logs-dir "/tmp/letsencrypt-log" ' +
-			`--cert-name 'npm-${certificate.id}' ` +
-			'--agree-tos ' +
-			`--email '${certificate.meta.letsencrypt_email}' ` +
-			`--domains '${certificate.domain_names.join(',')}' ` +
-			`--authenticator '${dnsPlugin.full_plugin_name}' ` +
-			(
-				hasConfigArg
-					? `--${dnsPlugin.full_plugin_name}-credentials '${credentialsLocation}' `
-					: ''
-			) +
-			(
-				certificate.meta.propagation_seconds !== undefined
-					? `--${dnsPlugin.full_plugin_name}-propagation-seconds '${certificate.meta.propagation_seconds}' `
-					: ''
-			) +
-			(letsencryptServer !== null ? `--server '${letsencryptServer}' ` : '') +
-			(letsencryptStaging && letsencryptServer === null ? '--staging ' : '');
+		const args = [
+			'certonly',
+			'--config',
+			letsencryptConfig,
+			'--work-dir',
+			'/tmp/letsencrypt-lib',
+			'--logs-dir',
+			'/tmp/letsencrypt-log',
+			'--cert-name',
+			`npm-${certificate.id}`,
+			'--agree-tos',
+			'--email',
+			certificate.meta.letsencrypt_email,
+			'--domains',
+			certificate.domain_names.join(','),
+			'--authenticator',
+			dnsPlugin.full_plugin_name,
+		];

-		// Prepend the path to the credentials file as an environment variable
-		if (certificate.meta.dns_provider === 'route53') {
-			mainCmd = 'AWS_CONFIG_FILE=\'' + credentialsLocation + '\' ' + mainCmd;
+		if (hasConfigArg) {
+			args.push(`--${dnsPlugin.full_plugin_name}-credentials`, credentialsLocation);
+		}
+		if (certificate.meta.propagation_seconds !== undefined) {
+			args.push(`--${dnsPlugin.full_plugin_name}-propagation-seconds`, certificate.meta.propagation_seconds.toString());
 		}

-		if (certificate.meta.dns_provider === 'duckdns') {
-			mainCmd = mainCmd + ' --dns-duckdns-no-txt-restore';
-		}
+		const adds = internalCertificate.getAdditionalCertbotArgs(certificate.id, certificate.meta.dns_provider);
+		args.push(...adds.args);

-		logger.info('Command:', mainCmd);
+		logger.info(`Command: ${certbotCommand} ${args ? args.join(' ') : ''}`);

 		try {
-			const result = await utils.exec(mainCmd);
+			const result = await utils.execFile(certbotCommand, args, adds.opts);
 			logger.info(result);
 			return result;
 		} catch (err) {
@@ -939,7 +948,7 @@ const internalCertificate = {

 					return renewMethod(certificate)
 						.then(() => {
-							return internalCertificate.getCertificateInfoFromFile('/etc/letsencrypt/live/npm-' + certificate.id + '/fullchain.pem');
+							return internalCertificate.getCertificateInfoFromFile(`${internalCertificate.getLiveCertPath(certificate.id)}/fullchain.pem`);
 						})
 						.then((cert_info) => {
 							return certificateModel
@@ -971,22 +980,31 @@ const internalCertificate = {
 	 * @returns {Promise}
 	 */
 	renewLetsEncryptSsl: (certificate) => {
-		logger.info('Renewing Let\'sEncrypt certificates for Cert #' + certificate.id + ': ' + certificate.domain_names.join(', '));
+		logger.info(`Renewing LetsEncrypt certificates for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);

-		const cmd = certbotCommand + ' renew --force-renewal ' +
-			`--config '${letsencryptConfig}' ` +
-			'--work-dir "/tmp/letsencrypt-lib" ' +
-			'--logs-dir "/tmp/letsencrypt-log" ' +
-			`--cert-name 'npm-${certificate.id}' ` +
-			'--preferred-challenges "dns,http" ' +
-			'--no-random-sleep-on-renew ' +
-			'--disable-hook-validation ' +
-			(letsencryptServer !== null ? `--server '${letsencryptServer}' ` : '') +
-			(letsencryptStaging && letsencryptServer === null ? '--staging ' : '');
+		const args = [
+			'renew',
+			'--force-renewal',
+			'--config',
+			letsencryptConfig,
+			'--work-dir',
+			'/tmp/letsencrypt-lib',
+			'--logs-dir',
+			'/tmp/letsencrypt-log',
+			'--cert-name',
+			`npm-${certificate.id}`,
+			'--preferred-challenges',
+			'dns,http',
+			'--no-random-sleep-on-renew',
+			'--disable-hook-validation',
+		];

-		logger.info('Command:', cmd);
+		const adds = internalCertificate.getAdditionalCertbotArgs(certificate.id, certificate.meta.dns_provider);
+		args.push(...adds.args);

-		return utils.exec(cmd)
+		logger.info(`Command: ${certbotCommand} ${args ? args.join(' ') : ''}`);
+
+		return utils.execFile(certbotCommand, args, adds.opts)
 			.then((result) => {
 				logger.info(result);
 				return result;
@@ -1004,27 +1022,29 @@ const internalCertificate = {
 			throw Error(`Unknown DNS provider '${certificate.meta.dns_provider}'`);
 		}

-		logger.info(`Renewing Let'sEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);
+		logger.info(`Renewing LetsEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);

-		let mainCmd = certbotCommand + ' renew --force-renewal ' +
-			`--config "${letsencryptConfig}" ` +
-			'--work-dir "/tmp/letsencrypt-lib" ' +
-			'--logs-dir "/tmp/letsencrypt-log" ' +
-			`--cert-name 'npm-${certificate.id}' ` +
-			'--disable-hook-validation ' +
-			'--no-random-sleep-on-renew ' +
-			(letsencryptServer !== null ? `--server '${letsencryptServer}' ` : '') +
-			(letsencryptStaging && letsencryptServer === null ? '--staging ' : '');
+		const args = [
+			'renew',
+			'--force-renewal',
+			'--config',
+			letsencryptConfig,
+			'--work-dir',
+			'/tmp/letsencrypt-lib',
+			'--logs-dir',
+			'/tmp/letsencrypt-log',
+			'--cert-name',
+			`npm-${certificate.id}`,
+			'--disable-hook-validation',
+			'--no-random-sleep-on-renew',
+		];

-		// Prepend the path to the credentials file as an environment variable
-		if (certificate.meta.dns_provider === 'route53') {
-			const credentialsLocation = '/etc/letsencrypt/credentials/credentials-' + certificate.id;
-			mainCmd                   = 'AWS_CONFIG_FILE=\'' + credentialsLocation + '\' ' + mainCmd;
-		}
+		const adds = internalCertificate.getAdditionalCertbotArgs(certificate.id, certificate.meta.dns_provider);
+		args.push(...adds.args);

-		logger.info('Command:', mainCmd);
+		logger.info(`Command: ${certbotCommand} ${args ? args.join(' ') : ''}`);

-		return utils.exec(mainCmd)
+		return utils.execFile(certbotCommand, args, adds.opts)
 			.then(async (result) => {
 				logger.info(result);
 				return result;
@@ -1037,25 +1057,29 @@ const internalCertificate = {
 	 * @returns {Promise}
 	 */
 	revokeLetsEncryptSsl: (certificate, throw_errors) => {
-		logger.info('Revoking Let\'sEncrypt certificates for Cert #' + certificate.id + ': ' + certificate.domain_names.join(', '));
+		logger.info(`Revoking LetsEncrypt certificates for Cert #${certificate.id}: ${certificate.domain_names.join(', ')}`);

-		const mainCmd = certbotCommand + ' revoke ' +
-			`--config '${letsencryptConfig}' ` +
-			'--work-dir "/tmp/letsencrypt-lib" ' +
-			'--logs-dir "/tmp/letsencrypt-log" ' +
-			`--cert-path '/etc/letsencrypt/live/npm-${certificate.id}/fullchain.pem' ` +
-			'--delete-after-revoke ' +
-			(letsencryptServer !== null ? `--server '${letsencryptServer}' ` : '') +
-			(letsencryptStaging && letsencryptServer === null ? '--staging ' : '');
+		const args = [
+			'revoke',
+			'--config',
+			letsencryptConfig,
+			'--work-dir',
+			'/tmp/letsencrypt-lib',
+			'--logs-dir',
+			'/tmp/letsencrypt-log',
+			'--cert-path',
+			`${internalCertificate.getLiveCertPath(certificate.id)}/fullchain.pem`,
+			'--delete-after-revoke',
+		];

-		// Don't fail command if file does not exist
-		const delete_credentialsCmd = `rm -f '/etc/letsencrypt/credentials/credentials-${certificate.id}' || true`;
+		const adds = internalCertificate.getAdditionalCertbotArgs(certificate.id);
+		args.push(...adds.args);

-		logger.info('Command:', mainCmd + '; ' + delete_credentialsCmd);
+		logger.info(`Command: ${certbotCommand} ${args ? args.join(' ') : ''}`);

-		return utils.exec(mainCmd)
+		return utils.execFile(certbotCommand, args, adds.opts)
 			.then(async (result) => {
-				await utils.exec(delete_credentialsCmd);
+				await utils.exec(`rm -f '/etc/letsencrypt/credentials/credentials-${certificate.id}' || true`);
 				logger.info(result);
 				return result;
 			})
@@ -1073,9 +1097,8 @@ const internalCertificate = {
 	 * @returns {Boolean}
 	 */
 	hasLetsEncryptSslCerts: (certificate) => {
-		const letsencryptPath = '/etc/letsencrypt/live/npm-' + certificate.id;
-
-		return fs.existsSync(letsencryptPath + '/fullchain.pem') && fs.existsSync(letsencryptPath + '/privkey.pem');
+		const letsencryptPath = internalCertificate.getLiveCertPath(certificate.id);
+		return fs.existsSync(`${letsencryptPath}/fullchain.pem`) && fs.existsSync(`${letsencryptPath}/privkey.pem`);
 	},

 	/**
@@ -1087,7 +1110,7 @@ const internalCertificate = {
 	 */
 	disableInUseHosts: (in_use_result) => {
 		if (in_use_result.total_count) {
-			let promises = [];
+			const promises = [];

 			if (in_use_result.proxy_hosts.length) {
 				promises.push(internalNginx.bulkDeleteConfigs('proxy_host', in_use_result.proxy_hosts));
@@ -1117,7 +1140,7 @@ const internalCertificate = {
 	 */
 	enableInUseHosts: (in_use_result) => {
 		if (in_use_result.total_count) {
-			let promises = [];
+			const promises = [];

 			if (in_use_result.proxy_hosts.length) {
 				promises.push(internalNginx.bulkGenerateConfigs('proxy_host', in_use_result.proxy_hosts));
@@ -1150,12 +1173,12 @@ const internalCertificate = {

 		// Create a test challenge file
 		const testChallengeDir  = '/data/letsencrypt-acme-challenge/.well-known/acme-challenge';
-		const testChallengeFile = testChallengeDir + '/test-challenge';
+		const testChallengeFile = `${testChallengeDir}/test-challenge`;
 		fs.mkdirSync(testChallengeDir, {recursive: true});
 		fs.writeFileSync(testChallengeFile, 'Success', {encoding: 'utf8'});

 		async function performTestForDomain (domain) {
-			logger.info('Testing http challenge for ' + domain);
+			logger.info(`Testing http challenge for ${domain}`);
 			const url      = `http://${domain}/.well-known/acme-challenge/test-challenge`;
 			const formBody = `method=G&url=${encodeURI(url)}&bodytype=T&requestbody=&headername=User-Agent&headervalue=None&locationid=1&ch=false&cc=false`;
 			const options  = {
@@ -1169,13 +1192,16 @@ const internalCertificate = {

 			const result = await new Promise((resolve) => {

-				const req = https.request('https://www.site24x7.com/tools/restapi-tester', options, function (res) {
+				const req = https.request('https://www.site24x7.com/tools/restapi-tester', options, (res) => {
 					let responseBody = '';

-					res.on('data', (chunk) => responseBody = responseBody + chunk);
-					res.on('end', function () {
+					res.on('data', (chunk) => {
+						responseBody = responseBody + chunk;
+					});
+
+					res.on('end', () => {
 						try {
-							const parsedBody = JSON.parse(responseBody + '');
+							const parsedBody = JSON.parse(`${responseBody}`);
 							if (res.statusCode !== 200) {
 								logger.warn(`Failed to test HTTP challenge for domain ${domain} because HTTP status code ${res.statusCode} was returned: ${parsedBody.message}`);
 								resolve(undefined);
@@ -1196,7 +1222,7 @@ const internalCertificate = {
 				// Make sure to write the request body.
 				req.write(formBody);
 				req.end();
-				req.on('error', function (e) { logger.warn(`Failed to test HTTP challenge for domain ${domain}`, e);
+				req.on('error', (e) => { logger.warn(`Failed to test HTTP challenge for domain ${domain}`, e);
 					resolve(undefined); });
 			});

@@ -1238,6 +1264,34 @@ const internalCertificate = {
 		fs.unlinkSync(testChallengeFile);

 		return results;
+	},
+
+	getAdditionalCertbotArgs: (certificate_id, dns_provider) => {
+		const args = [];
+		if (letsencryptServer !== null) {
+			args.push('--server', letsencryptServer);
+		}
+		if (letsencryptStaging && letsencryptServer === null) {
+			args.push('--staging');
+		}
+
+		// For route53, add the credentials file as an environment variable,
+		// inheriting the process env
+		const opts = {};
+		if (certificate_id && dns_provider === 'route53') {
+			opts.env                 = process.env;
+			opts.env.AWS_CONFIG_FILE = `/etc/letsencrypt/credentials/credentials-${certificate_id}`;
+		}
+
+		if (dns_provider === 'duckdns') {
+			args.push('--dns-duckdns-no-txt-restore');
+		}
+
+		return {args: args, opts: opts};
+	},
+
+	getLiveCertPath: (certificate_id) => {
+		return `/etc/letsencrypt/live/npm-${certificate_id}`;
 	}
 };

--- a/backend/internal/nginx.js
+++ b/backend/internal/nginx.js
@@ -1,5 +1,5 @@
 const _      = require('lodash');
-const fs     = require('fs');
+const fs     = require('node:fs');
 const logger = require('../logger').nginx;
 const config = require('../lib/config');
 const utils  = require('../lib/utils');
@@ -57,9 +57,9 @@ const internalNginx = {
 						// It will always look like this:
 						//   nginx: [alert] could not open error log file: open() "/var/log/nginx/error.log" failed (6: No such device or address)

-						let valid_lines = [];
-						let err_lines   = err.message.split('\n');
-						err_lines.map(function (line) {
+						const valid_lines = [];
+						const err_lines   = err.message.split('\n');
+						err_lines.map((line) => {
 							if (line.indexOf('/var/log/nginx/error.log') === -1) {
 								valid_lines.push(line);
 							}
@@ -105,7 +105,7 @@ const internalNginx = {
 			logger.info('Testing Nginx configuration');
 		}

-		return utils.exec('/usr/sbin/nginx -t -g "error_log off;"');
+		return utils.execFile('/usr/sbin/nginx', ['-t', '-g', 'error_log off;']);
 	},

 	/**
@@ -115,7 +115,7 @@ const internalNginx = {
 		return internalNginx.test()
 			.then(() => {
 				logger.info('Reloading Nginx');
-				return utils.exec('/usr/sbin/nginx -s reload');
+				return utils.execFile('/usr/sbin/nginx', ['-s', 'reload']);
 			});
 	},

@@ -128,7 +128,7 @@ const internalNginx = {
 		if (host_type === 'default') {
 			return '/data/nginx/default_host/site.conf';
 		}
-		return '/data/nginx/' + internalNginx.getFileFriendlyHostType(host_type) + '/' + host_id + '.conf';
+		return `/data/nginx/${internalNginx.getFileFriendlyHostType(host_type)}/${host_id}.conf`;
 	},

 	/**
@@ -141,7 +141,7 @@ const internalNginx = {
 			let template;

 			try {
-				template = fs.readFileSync(__dirname + '/../templates/_location.conf', {encoding: 'utf8'});
+				template = fs.readFileSync(`${__dirname}/../templates/_location.conf`, {encoding: 'utf8'});
 			} catch (err) {
 				reject(new error.ConfigurationError(err.message));
 				return;
@@ -152,7 +152,7 @@ const internalNginx = {

 			const locationRendering = async () => {
 				for (let i = 0; i < host.locations.length; i++) {
-					let locationCopy = Object.assign({}, {access_list_id: host.access_list_id}, {certificate_id: host.certificate_id},
+					const locationCopy = Object.assign({}, {access_list_id: host.access_list_id}, {certificate_id: host.certificate_id},
 						{ssl_forced: host.ssl_forced}, {caching_enabled: host.caching_enabled}, {block_exploits: host.block_exploits},
 						{allow_websocket_upgrade: host.allow_websocket_upgrade}, {http2_support: host.http2_support},
 						{hsts_enabled: host.hsts_enabled}, {hsts_subdomains: host.hsts_subdomains}, {access_list: host.access_list},
@@ -183,21 +183,21 @@ const internalNginx = {
 	 */
 	generateConfig: (host_type, host_row) => {
 		// Prevent modifying the original object:
-		let host             = JSON.parse(JSON.stringify(host_row));
+		const host           = JSON.parse(JSON.stringify(host_row));
 		const nice_host_type = internalNginx.getFileFriendlyHostType(host_type);

 		if (config.debug()) {
-			logger.info('Generating ' + nice_host_type + ' Config:', JSON.stringify(host, null, 2));
+			logger.info(`Generating ${nice_host_type} Config:`, JSON.stringify(host, null, 2));
 		}

 		const renderEngine = utils.getRenderEngine();

 		return new Promise((resolve, reject) => {
-			let template = null;
-			let filename = internalNginx.getConfigName(nice_host_type, host.id);
+			let template   = null;
+			const filename = internalNginx.getConfigName(nice_host_type, host.id);

 			try {
-				template = fs.readFileSync(__dirname + '/../templates/' + nice_host_type + '.conf', {encoding: 'utf8'});
+				template = fs.readFileSync(`${__dirname}/../templates/${nice_host_type}.conf`, {encoding: 'utf8'});
 			} catch (err) {
 				reject(new error.ConfigurationError(err.message));
 				return;
@@ -252,7 +252,7 @@ const internalNginx = {
 					})
 					.catch((err) => {
 						if (config.debug()) {
-							logger.warn('Could not write ' + filename + ':', err.message);
+							logger.warn(`Could not write ${filename}:`, err.message);
 						}

 						reject(new error.ConfigurationError(err.message));
@@ -277,11 +277,11 @@ const internalNginx = {
 		const renderEngine = utils.getRenderEngine();

 		return new Promise((resolve, reject) => {
-			let template = null;
-			let filename = '/data/nginx/temp/letsencrypt_' + certificate.id + '.conf';
+			let template   = null;
+			const filename = `/data/nginx/temp/letsencrypt_${certificate.id}.conf`;

 			try {
-				template = fs.readFileSync(__dirname + '/../templates/letsencrypt-request.conf', {encoding: 'utf8'});
+				template = fs.readFileSync(`${__dirname}/../templates/letsencrypt-request.conf`, {encoding: 'utf8'});
 			} catch (err) {
 				reject(new error.ConfigurationError(err.message));
 				return;
@@ -302,7 +302,7 @@ const internalNginx = {
 				})
 				.catch((err) => {
 					if (config.debug()) {
-						logger.warn('Could not write ' + filename + ':', err.message);
+						logger.warn(`Could not write ${filename}:`, err.message);
 					}

 					reject(new error.ConfigurationError(err.message));
@@ -316,7 +316,7 @@ const internalNginx = {
 	 * @param   {String}  filename
 	 */
 	deleteFile: (filename) => {
-		logger.debug('Deleting file: ' + filename);
+		logger.debug(`Deleting file: ${filename}`);
 		try {
 			fs.unlinkSync(filename);
 		} catch (err) {
@@ -330,7 +330,7 @@ const internalNginx = {
 	 * @returns String
 	 */
 	getFileFriendlyHostType: (host_type) => {
-		return host_type.replace(new RegExp('-', 'g'), '_');
+		return host_type.replace(/-/g, '_');
 	},

 	/**
@@ -340,7 +340,7 @@ const internalNginx = {
 	 * @returns {Promise}
 	 */
 	deleteLetsEncryptRequestConfig: (certificate) => {
-		const config_file = '/data/nginx/temp/letsencrypt_' + certificate.id + '.conf';
+		const config_file = `/data/nginx/temp/letsencrypt_${certificate.id}.conf`;
 		return new Promise((resolve/*, reject*/) => {
 			internalNginx.deleteFile(config_file);
 			resolve();
@@ -355,7 +355,7 @@ const internalNginx = {
 	 */
 	deleteConfig: (host_type, host, delete_err_file) => {
 		const config_file     = internalNginx.getConfigName(internalNginx.getFileFriendlyHostType(host_type), typeof host === 'undefined' ? 0 : host.id);
-		const config_file_err = config_file + '.err';
+		const config_file_err = `${config_file}.err`;

 		return new Promise((resolve/*, reject*/) => {
 			internalNginx.deleteFile(config_file);
@@ -373,7 +373,7 @@ const internalNginx = {
 	 */
 	renameConfigAsError: (host_type, host) => {
 		const config_file     = internalNginx.getConfigName(internalNginx.getFileFriendlyHostType(host_type), typeof host === 'undefined' ? 0 : host.id);
-		const config_file_err = config_file + '.err';
+		const config_file_err = `${config_file}.err`;

 		return new Promise((resolve/*, reject*/) => {
 			fs.unlink(config_file, () => {
@@ -392,8 +392,8 @@ const internalNginx = {
 	 * @returns {Promise}
 	 */
 	bulkGenerateConfigs: (host_type, hosts) => {
-		let promises = [];
-		hosts.map(function (host) {
+		const promises = [];
+		hosts.map((host) => {
 			promises.push(internalNginx.generateConfig(host_type, host));
 		});

@@ -406,8 +406,8 @@ const internalNginx = {
 	 * @returns {Promise}
 	 */
 	bulkDeleteConfigs: (host_type, hosts) => {
-		let promises = [];
-		hosts.map(function (host) {
+		const promises = [];
+		hosts.map((host) => {
 			promises.push(internalNginx.deleteConfig(host_type, host, true));
 		});

@@ -418,14 +418,12 @@ const internalNginx = {
 	 * @param   {string}  config
 	 * @returns {boolean}
 	 */
-	advancedConfigHasDefaultLocation: function (cfg) {
-		return !!cfg.match(/^(?:.*;)?\s*?location\s*?\/\s*?{/im);
-	},
+	advancedConfigHasDefaultLocation: (cfg) => !!cfg.match(/^(?:.*;)?\s*?location\s*?\/\s*?{/im),

 	/**
 	 * @returns {boolean}
 	 */
-	ipv6Enabled: function () {
+	ipv6Enabled: () => {
 		if (typeof process.env.DISABLE_IPV6 !== 'undefined') {
 			const disabled = process.env.DISABLE_IPV6.toLowerCase();
 			return !(disabled === 'on' || disabled === 'true' || disabled === '1' || disabled === 'yes');