Remove DNS provider credentials from disk after certbot runs

The credentials file written for a DNS-01 challenge was only cleaned up when
certbot failed - the unlink sat in a catch block. On success the file stayed in
/etc/letsencrypt/credentials for the entire life of the certificate, holding a
live DNS provider API token in plaintext.

The file cannot simply be deleted at issuance, because certbot records its path
in the renewal config and reads it back on every `certbot renew`. So the renew
path now writes the file itself immediately before invoking certbot, and both
paths remove it in a finally block.

Net effect: the credentials exist on disk for the duration of a certbot run
rather than permanently. The value still lives in the certificates table, which
is unavoidable - it has to come from somewhere to be written at all.

renewLetsEncryptSslWithDnsChallenge reads the row directly from the model
because renew() sources its certificate from internalCertificate.get(), which
strips meta.dns_provider_credentials via omissions().

backend/internal/certificate.js

@@ -881,10 +881,20 @@ const internalCertificate = {
 			const result = await utils.execFile(certbotCommand, args, adds.opts);
 			logger.info(result);
 			return result;
-		} catch (err) {
-			// Don't fail if file does not exist, so no need for action in the callback
+		} finally {
+			// Remove the credentials file whether certbot succeeded or failed.
+			//
+			// This cleanup used to sit in a catch block, so it only ran when issuance FAILED.
+			// A certificate that issued successfully left its DNS provider API credentials in
+			// /etc/letsencrypt/credentials for the entire life of that certificate. Nothing
+			// reads the file between certbot runs, so there is no reason to keep it:
+			// renewLetsEncryptSslWithDnsChallenge() writes it again immediately before each
+			// renewal.
+			//
+			// unlink is fire-and-forget with an empty callback. If the file is already gone
+			// that is the end state we wanted anyway, and a missing file must never turn a
+			// successful issuance into a failure.
 			fs.unlink(credentialsLocation, () => {});
-			throw err;
 		}
 	},
 
@@ -981,6 +991,43 @@ const internalCertificate = {
 			`Renewing LetsEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(", ")}`,
 		);
 
+		// certbot reads the DNS credentials back from the path recorded in the renewal config
+		// it wrote at issuance time, for example:
+		//
+		//     authenticator = dns-cloudflare
+		//     dns_cloudflare_credentials = /etc/letsencrypt/credentials/credentials-27
+		//
+		// so the file has to be present for the duration of this run. Write it here and remove
+		// it again below rather than leaving it on disk between renewals.
+		//
+		// Leaving it is an avoidable exposure. Anything running as root - a compromised
+		// process, a script, malware - can read the token and use it to issue valid Let's
+		// Encrypt certificates for the domain. Those certificates are genuinely trusted, so
+		// traffic presented with them passes TLS inspection, IDS/IPS and DLP that would
+		// otherwise flag it, and an exfiltration path built on them looks like ordinary
+		// HTTPS. The exposure window should be one certbot run, not the life of the
+		// certificate.
+		//
+		// The value is not on the certificate object we were handed: renew() sources that from
+		// internalCertificate.get(), which pipes the row through utils.omitRow(omissions()) so
+		// meta.dns_provider_credentials can never travel out over the API. Read the row from
+		// the model directly to get at it.
+		const row = await certificateModel.query().where("id", certificate.id).first();
+		const credentials = row?.meta?.dns_provider_credentials;
+		const credentialsLocation = `/etc/letsencrypt/credentials/credentials-${certificate.id}`;
+
+		if (credentials) {
+			fs.mkdirSync("/etc/letsencrypt/credentials", { recursive: true });
+			fs.writeFileSync(credentialsLocation, credentials, { mode: 0o600 });
+		} else {
+			// Nothing stored to write. A certificate issued under the previous behaviour may
+			// still have its file on disk; leave it be and let certbot decide. Throwing here
+			// would break a renewal that would otherwise have succeeded.
+			logger.warn(
+				`No stored DNS credentials for Cert #${certificate.id}; relying on any existing ${credentialsLocation}`,
+			);
+		}
+
 		const args = [
 			"renew",
 			"--force-renewal",
@@ -1008,9 +1055,19 @@ const internalCertificate = {
 
 		logger.info(`Command: ${certbotCommand} ${args ? args.join(" ") : ""}`);
 
-		const result = await utils.execFile(certbotCommand, args, adds.opts);
-		logger.info(result);
-		return result;
+		try {
+			const result = await utils.execFile(certbotCommand, args, adds.opts);
+			logger.info(result);
+			return result;
+		} finally {
+			// Only clean up a file we put there ourselves. If `credentials` came back empty we
+			// wrote nothing, and an older file left on disk by the previous behaviour is the
+			// only thing keeping that certificate renewable - deleting it would break the next
+			// run for no gain.
+			if (credentials) {
+				fs.unlink(credentialsLocation, () => {});
+			}
+		}
 	},
 
 	/**