feat(dashmate): add Let's Encrypt SSL provider support #3000
Conversation
Add Let's Encrypt as a new SSL certificate provider option using the goacme/lego Docker client. This enables IP-based certificate issuance with automatic renewal support for shortlived certificates. New files: - LegoCertificate.js - Certificate model for parsing PEM files - validateLetsEncryptCertificateFactory.js - Validation logic - obtainLetsEncryptCertificateTaskFactory.js - Main obtain task - scheduleRenewLetsEncryptCertificateFactory.js - Renewal scheduler Changes: - Add LETSENCRYPT to SSL_PROVIDERS constant - Add letsencrypt config schema and defaults - Add Let's Encrypt to setup wizard choices - Update ssl obtain command with --provider flag - Update helper.js with Let's Encrypt renewal scheduling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Let’s Encrypt as a new SSL provider: schema, constants, DI wiring, certificate class and validators, obtain/renew task factories, CLI/provider selection updates, and scheduling for automatic renewals. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Command
participant DI as DI Container
participant Task as Obtain Task
participant Docker as Docker/Lego
participant Validator as Validator
participant Config as Config Repository
User->>CLI: run obtain (--provider letsencrypt)
CLI->>DI: resolve obtainLetsEncryptCertificateTask
DI->>Task: instantiate task with dependencies
Task->>Validator: validate existing certificate (config, expirationDays)
Validator->>Task: validation result
alt Certificate missing/invalid
Task->>Docker: pull lego image & run lego container
Docker->>Task: produce cert and key files
end
Task->>Validator: parse/verify obtained cert
Validator->>Task: validity confirmation
Task->>Config: enable SSL, set provider, save certs
Config->>Task: persisted
Task->>CLI: complete
sequenceDiagram
participant Scheduler as Renewal Scheduler
participant CronJob as CronJob
participant Task as Obtain Task
participant DockerCompose as Docker Compose
participant Gateway as Gateway Service
Scheduler->>Scheduler: read existing lego cert files
Scheduler->>Scheduler: compute renewal time (expiry - EXPIRATION_LIMIT_DAYS)
Scheduler->>CronJob: schedule renewal
CronJob->>Task: trigger obtainLetsEncryptCertificateTask
Task->>Task: obtain/renew certificate
Task->>Scheduler: return success
Scheduler->>DockerCompose: execCommand SIGHUP to PID 1
DockerCompose->>Gateway: reload SSL configuration
Scheduler->>CronJob: stop job
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/dashmate/src/helper/scheduleRenewLetsEncryptCertificateFactory.js`:
- Around line 67-94: The current scheduleRenewLetsEncryptCertificate CronJob
immediately reschedules on any failure (see CronJob callback, job.stop(), and
process.nextTick(() => scheduleRenewLetsEncryptCertificate(config))) which can
create a tight retry loop; modify the error path in the catch block so that
instead of immediately calling process.nextTick to reschedule, it waits a fixed
backoff (e.g., setTimeout with a sensible delay like 1 hour) before invoking
scheduleRenewLetsEncryptCertificate(config), while leaving the success path
unchanged; update references around obtainLetsEncryptCertificateTask, job.stop,
and process.nextTick so only the failure branch uses the delayed reschedule.
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`:
- Around line 43-44: The code uses the logical OR operator when setting
expirationDays which treats 0 as falsy; change the assignment in the
obtainLetsEncryptCertificateTaskFactory to use the nullish coalescing operator
so an explicit ctx.expirationDays = 0 is honored (i.e. replace "const
expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS;"
with a nullish-coalescing version), then pass that expirationDays into
validateLetsEncryptCertificate unchanged.
In
`@packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js`:
- Around line 95-101: The current check in
validateLetsEncryptCertificateFactory.js only inspects
data.certificate.commonName and short-circuits when CN is null, so update
validate logic to inspect SubjectAltName (SAN) IP entries instead: extract
IP-type SANs from data.certificate.subjectAltName (or equivalent SAN list on
data.certificate), ensure at least one SAN IP equals data.externalIp, and return
ERRORS.CERTIFICATE_IP_MISMATCH if none match; also update the error message used
in obtainLetsEncryptCertificateTaskFactory.js (the message that currently prints
the null commonName) to reference the external IP and/or the SANs (not
commonName) so it never prints "null" as the certificate IP.
🧹 Nitpick comments (1)
packages/dashmate/src/config/configJsonSchema.js (1)
703-712: Consider adding email format validation to the schema.The
letsencrypt.emailfield accepts strings but has no format constraint. While the UI prompt validates email format, invalid emails could be loaded directly from config files bypassing the UI. Adding JSON Schema'sformat: 'email'would provide defense-in-depth validation.♻️ Suggested improvement
letsencrypt: { type: ['object'], properties: { email: { type: ['string', 'null'], + format: 'email', }, }, required: ['email'], additionalProperties: false, },Note: JSON Schema's
format: 'email'requires the validator to have format validation enabled.
| const job = new CronJob(renewAt, async () => { | ||
| try { | ||
| const tasks = obtainLetsEncryptCertificateTask(config); | ||
|
|
||
| await tasks.run({ | ||
| expirationDays: LegoCertificate.EXPIRATION_LIMIT_DAYS, | ||
| noRetry: true, | ||
| }); | ||
|
|
||
| // Write config files | ||
| configFileRepository.write(configFile); | ||
| writeConfigTemplates(config); | ||
|
|
||
| // Restart Gateway to catch up new SSL certificates | ||
| await dockerCompose.execCommand(config, 'gateway', 'kill -SIGHUP 1'); | ||
|
|
||
| // eslint-disable-next-line no-console | ||
| console.log("Let's Encrypt certificate renewed successfully"); | ||
| } catch (e) { | ||
| // eslint-disable-next-line no-console | ||
| console.error(`Failed to renew Let's Encrypt certificate: ${e.message}`); | ||
| } | ||
|
|
||
| job.stop(); | ||
| }, async () => { | ||
| // Schedule new cron task after completion | ||
| process.nextTick(() => scheduleRenewLetsEncryptCertificate(config)); | ||
| }); |
There was a problem hiding this comment.
Avoid a tight retry loop after renewal failure.
On failure, the flow immediately reschedules; with an expiring cert this retries every ~3s, which can thrash Docker/LE and hit rate limits. Add a fixed delay before rescheduling on error.
🔧 Proposed fix (delay reschedule on failure)
- const job = new CronJob(renewAt, async () => {
- try {
+ let hadError = false;
+ const job = new CronJob(renewAt, async () => {
+ try {
const tasks = obtainLetsEncryptCertificateTask(config);
await tasks.run({
expirationDays: LegoCertificate.EXPIRATION_LIMIT_DAYS,
noRetry: true,
});
@@
- } catch (e) {
+ } catch (e) {
+ hadError = true;
// eslint-disable-next-line no-console
console.error(`Failed to renew Let's Encrypt certificate: ${e.message}`);
}
job.stop();
}, async () => {
// Schedule new cron task after completion
- process.nextTick(() => scheduleRenewLetsEncryptCertificate(config));
+ const delayMs = hadError ? 60 * 60 * 1000 : 0;
+ setTimeout(() => scheduleRenewLetsEncryptCertificate(config), delayMs);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const job = new CronJob(renewAt, async () => { | |
| try { | |
| const tasks = obtainLetsEncryptCertificateTask(config); | |
| await tasks.run({ | |
| expirationDays: LegoCertificate.EXPIRATION_LIMIT_DAYS, | |
| noRetry: true, | |
| }); | |
| // Write config files | |
| configFileRepository.write(configFile); | |
| writeConfigTemplates(config); | |
| // Restart Gateway to catch up new SSL certificates | |
| await dockerCompose.execCommand(config, 'gateway', 'kill -SIGHUP 1'); | |
| // eslint-disable-next-line no-console | |
| console.log("Let's Encrypt certificate renewed successfully"); | |
| } catch (e) { | |
| // eslint-disable-next-line no-console | |
| console.error(`Failed to renew Let's Encrypt certificate: ${e.message}`); | |
| } | |
| job.stop(); | |
| }, async () => { | |
| // Schedule new cron task after completion | |
| process.nextTick(() => scheduleRenewLetsEncryptCertificate(config)); | |
| }); | |
| let hadError = false; | |
| const job = new CronJob(renewAt, async () => { | |
| try { | |
| const tasks = obtainLetsEncryptCertificateTask(config); | |
| await tasks.run({ | |
| expirationDays: LegoCertificate.EXPIRATION_LIMIT_DAYS, | |
| noRetry: true, | |
| }); | |
| // Write config files | |
| configFileRepository.write(configFile); | |
| writeConfigTemplates(config); | |
| // Restart Gateway to catch up new SSL certificates | |
| await dockerCompose.execCommand(config, 'gateway', 'kill -SIGHUP 1'); | |
| // eslint-disable-next-line no-console | |
| console.log("Let's Encrypt certificate renewed successfully"); | |
| } catch (e) { | |
| hadError = true; | |
| // eslint-disable-next-line no-console | |
| console.error(`Failed to renew Let's Encrypt certificate: ${e.message}`); | |
| } | |
| job.stop(); | |
| }, async () => { | |
| // Schedule new cron task after completion | |
| const delayMs = hadError ? 60 * 60 * 1000 : 0; | |
| setTimeout(() => scheduleRenewLetsEncryptCertificate(config), delayMs); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/dashmate/src/helper/scheduleRenewLetsEncryptCertificateFactory.js`
around lines 67 - 94, The current scheduleRenewLetsEncryptCertificate CronJob
immediately reschedules on any failure (see CronJob callback, job.stop(), and
process.nextTick(() => scheduleRenewLetsEncryptCertificate(config))) which can
create a tight retry loop; modify the error path in the catch block so that
instead of immediately calling process.nextTick to reschedule, it waits a fixed
backoff (e.g., setTimeout with a sensible delay like 1 hour) before invoking
scheduleRenewLetsEncryptCertificate(config), while leaving the success path
unchanged; update references around obtainLetsEncryptCertificateTask, job.stop,
and process.nextTick so only the failure branch uses the delayed reschedule.
| const expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS; | ||
| const { error, data } = await validateLetsEncryptCertificate(config, expirationDays); |
There was a problem hiding this comment.
Honor explicit expiration-days = 0.
||_toggle treats 0 as falsy, so the CLI can’t request “only renew if already expired.” Use nullish coalescing instead.
🔧 Proposed fix
- const expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS;
+ const expirationDays = ctx.expirationDays ?? LegoCertificate.EXPIRATION_LIMIT_DAYS;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS; | |
| const { error, data } = await validateLetsEncryptCertificate(config, expirationDays); | |
| const expirationDays = ctx.expirationDays ?? LegoCertificate.EXPIRATION_LIMIT_DAYS; | |
| const { error, data } = await validateLetsEncryptCertificate(config, expirationDays); |
🤖 Prompt for AI Agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`
around lines 43 - 44, The code uses the logical OR operator when setting
expirationDays which treats 0 as falsy; change the assignment in the
obtainLetsEncryptCertificateTaskFactory to use the nullish coalescing operator
so an explicit ctx.expirationDays = 0 is honored (i.e. replace "const
expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS;"
with a nullish-coalescing version), then pass that expirationDays into
validateLetsEncryptCertificate unchanged.
| // Check if certificate IP matches external IP | ||
| if (data.certificate.commonName && data.certificate.commonName !== data.externalIp) { | ||
| return { | ||
| error: ERRORS.CERTIFICATE_IP_MISMATCH, | ||
| data, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for --disable-cn usage
rg "\-\-disable-cn" --type js --type tsRepository: dashpay/platform
Length of output: 180
🏁 Script executed:
# Find the obtain flow for Let's Encrypt
fd ".*letsencrypt.*" -t f --include="*.js" --include="*.ts" | head -20Repository: dashpay/platform
Length of output: 280
🏁 Script executed:
# Look at the validateLetsEncryptCertificateFactory.js file
cat -n packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js | head -120Repository: dashpay/platform
Length of output: 4433
🏁 Script executed:
# Look at LegoCertificate.js
cat -n packages/dashmate/src/ssl/letsencrypt/LegoCertificate.jsRepository: dashpay/platform
Length of output: 2953
🏁 Script executed:
# Search for subjectAltName usage
rg "subjectAltName" --type js --type tsRepository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Check the obtain flow more carefully
cat -n packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js | head -100Repository: dashpay/platform
Length of output: 4761
🏁 Script executed:
# Search for forge extensions usage to understand how to extract SubjectAltName
rg "getExtension|altNames" --type js --type tsRepository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Check if there's any documentation or comments about the certificate validation
rg "commonName|CN|disable-cn" -B 2 -A 2 --type js --type ts packages/dashmate/src/ssl/Repository: dashpay/platform
Length of output: 3807
🏁 Script executed:
# Check if there are any tests or examples of certificate parsing
fd "*.test.js" -o "*.spec.js" | xargs grep -l "certificate\|LegoCertificate" 2>/dev/null | head -5Repository: dashpay/platform
Length of output: 197
🏁 Script executed:
# Look at lego task to see the full context of --disable-cn
cat -n packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js | grep -A 20 -B 5 "\-\-disable-cn"Repository: dashpay/platform
Length of output: 1125
🏁 Script executed:
# Search for any forge extension handling patterns
rg "extensions|getExtension" --type js --type ts -A 3Repository: dashpay/platform
Length of output: 50373
🏁 Script executed:
# Check package.json for node-forge version
cat packages/dashmate/package.json | grep -A 5 -B 5 "forge"Repository: dashpay/platform
Length of output: 353
🏁 Script executed:
# Check if there are any tests or examples related to certificate parsing
find packages/dashmate -name "*.js" -type f | xargs grep -l "certificate\|Certificate" | head -10Repository: dashpay/platform
Length of output: 698
🏁 Script executed:
# Look for any certificate-related tests
find . -path "*/node_modules" -prune -o -name "*.test.js" -o -name "*.spec.js" | grep -i certifRepository: dashpay/platform
Length of output: 143
🌐 Web query:
node-forge certificate getExtension subjectAltName altNames example
💡 Result:
Use pki.certificateFromPem(...) then read the subjectAltName extension's altNames array. Example:
Code:
const forge = require('node-forge');
const pki = forge.pki;
// parse PEM cert
const certPem = `-----BEGIN CERTIFICATE-----\n...base64...\n-----END CERTIFICATE-----`;
const cert = pki.certificateFromPem(certPem);
// find subjectAltName extension (OID 2.5.29.17) or by name
const sanExt = cert.extensions.find(e => e.name === 'subjectAltName' || e.id === '2.5.29.17');
if (sanExt && sanExt.altNames) {
sanExt.altNames.forEach(alt => {
// types: 1=email, 2=DNS, 6=URI, 7=IP
if (alt.type === 2) console.log('DNS:', alt.value);
else if (alt.type === 7) console.log('IP:', alt.ip);
else if (alt.type === 6) console.log('URI:', alt.value);
else console.log('other type', alt);
});
}
Notes:
- When creating certs, subjectAltName is set with { name: 'subjectAltName', altNames: [...] } where altNames entries use type and value/ip. [1]
- For more complex otherName parsing you may need to inspect ASN.1 fields (examples available). [2]
Sources:
- node-forge pki docs / examples for subjectAltName and altNames. [1]
- Example parsing otherName in subjectAltName (Stack Overflow). [2]
🏁 Script executed:
# Check if there are any existing certificate extension handling in the repo
rg "extensions\s*\." packages/dashmate --type js -B 2 -A 2Repository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Look for forge certificate parsing examples
rg "certificateFromPem\|pki\.certificate" packages/dashmate --type js -B 2 -A 5Repository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Check the test file for ZeroSSL to see certificate validation patterns
cat -n packages/dashmate/test/unit/ssl/zerossl/validateZeroSslCertificateFactory.spec.js | head -150Repository: dashpay/platform
Length of output: 6368
🏁 Script executed:
# Check the error message handling in obtainLetsEncryptCertificateTaskFactory
cat -n packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js | grep -A 2 "CERTIFICATE_IP_MISMATCH"Repository: dashpay/platform
Length of output: 360
🏁 Script executed:
# Let me verify the correct API for accessing certificate extensions in node-forge
# by looking at actual usage patterns in the codebase
rg "forge\.pki\|pki\.certificate" packages/dashmate --type js -A 10 | head -50Repository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Check if there are any comments or documentation about certificate structure
rg "commonName|CN|certificate" packages/dashmate/src/ssl --type js -B 1 -A 1 | grep -i "subject\|extension\|alt" | head -20Repository: dashpay/platform
Length of output: 241
🏁 Script executed:
# Check if forge's certificate object has getExtension method
node -e "
const forge = require('node-forge');
const pki = forge.pki;
// Try to see what methods are available on certificate objects
console.log('Checking forge pki API...');
try {
const testCert = pki.createCertificate();
console.log('Methods:', Object.getOwnPropertyNames(Object.getPrototypeOf(testCert)).filter(m => m.includes('Extension')));
console.log('Has getExtension:', typeof testCert.getExtension);
console.log('Has extensions property:', 'extensions' in testCert);
} catch (e) {
console.log('Error:', e.message);
}
" 2>&1 || echo "Node.js test failed, checking package.json instead"Repository: dashpay/platform
Length of output: 948
🏁 Script executed:
# Let me check how the ZeroSSL certificate validation works for comparison
cat -n packages/dashmate/src/ssl/zerossl/validateZeroSslCertificateFactory.js | head -80Repository: dashpay/platform
Length of output: 3185
🏁 Script executed:
# Check node-forge version and see if we can find documentation about the API
cat packages/dashmate/package.json | grep -A 2 -B 2 "node-forge"Repository: dashpay/platform
Length of output: 206
🏁 Script executed:
# Look for any other certificate parsing or validation code that might give hints
rg "forge\.pki\.certificate" packages/dashmate/src --type js -B 2 -A 10Repository: dashpay/platform
Length of output: 1564
🏁 Script executed:
# Search for certificate validation tests to understand the structure
cat -n packages/dashmate/test/unit/ssl/letsencrypt/validateLetsEncryptCertificateFactory.spec.js 2>/dev/null | head -100Repository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Search for letsencrypt test files
find packages/dashmate/test -name "*letsencrypt*" -type fRepository: dashpay/platform
Length of output: 42
🏁 Script executed:
# Look at the self-signed certificate creation to understand the certificate structure better
cat -n packages/dashmate/src/ssl/selfSigned/createSelfSignedCertificate.jsRepository: dashpay/platform
Length of output: 1115
🌐 Web query:
node-forge certificate extensions property altNames structure API
💡 Result:
Short summary — node-forge SubjectAltName (extensions.altNames) structure
-
Where: put an extension object in cert/csr extensions with name: 'subjectAltName' and altNames: an array. Example:
{ name: 'subjectAltName', altNames: [{ type: 2, value: 'example.com' }, { type: 7, ip: '127.0.0.1' }] }. [1][2] -
Per-altName fields:
- type: number (GeneralName type): common values:
- 0 = otherName (complex ASN.1 value)
- 1 = rfc822Name (email)
- 2 = dNSName (DNS string)
- 6 = uniformResourceIdentifier (URI)
- 7 = iPAddress (use ip property for human-readable IP)
- 8 = registeredID (OID; altName.oid available when parsed)
- value: string (used for types like 1,2,6; otherName uses a value array)
- ip: string (for type 7; node-forge converts bytes to human IP string)
- oid: string (for type 8 when parsing). [1][2]
- type: number (GeneralName type): common values:
-
Notes:
- For otherName (type 0) the value is an ASN.1 sequence (node-forge exposes the raw value; you must decode OID and inner value). Example parsing shown in community answers. [2][3]
Example (creating a cert/csr):
cert.setExtensions([
{ name: 'subjectAltName', altNames:[
{ type: 2, value: 'test.domain.com' },
{ type: 7, ip: '127.0.0.1' },
{ type: 6, value: 'http://example.org/webid#me' }
]}
]); [1][2]
Sources:
- node-forge pki docs / examples showing altNames schema. [1]
- node-forge x509 parsing code & examples (how altNames are parsed, types and ip/oid handling). [2]
- StackOverflow example for parsing otherName (type 0) in node-forge. [3]
IP mismatch check is ineffective when CN is disabled.
The obtain flow passes --disable-cn, so commonName is always null in the parsed certificate. The condition at line 96 uses AND logic (data.certificate.commonName &&), which short-circuits and skips the mismatch validation entirely—allowing stale certificates for different IPs to pass validation.
Additionally, the error message in obtainLetsEncryptCertificateTaskFactory.js (line 81) will display "Certificate IP null does not match..." when this error is thrown, since it references the null commonName.
Validate SubjectAltName entries (IP type) instead:
🔧 Proposed fix (validate SANs for IP)
// packages/dashmate/src/ssl/letsencrypt/LegoCertificate.js
@@
/**
* `@type` {string}
*/
commonName;
+
+ /**
+ * `@type` {string[]}
+ */
+ subjectAltNames;
@@
- * `@param` {string} data.commonName
+ * `@param` {string|null} data.commonName
+ * `@param` {string[]} data.subjectAltNames
*/
constructor(data) {
this.expires = data.expires;
this.created = data.created;
this.commonName = data.commonName;
+ this.subjectAltNames = data.subjectAltNames ?? [];
}
@@
const commonNameAttr = cert.subject.attributes.find(
(attr) => attr.shortName === 'CN',
);
+ const altNameExt = cert.extensions?.find(e => e.name === 'subjectAltName');
+ const subjectAltNames = (altNameExt?.altNames ?? [])
+ .map((alt) => alt.ip || alt.value)
+ .filter(Boolean);
return new LegoCertificate({
expires: cert.validity.notAfter,
created: cert.validity.notBefore,
commonName: commonNameAttr ? commonNameAttr.value : null,
+ subjectAltNames,
});// packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js
@@
- if (data.certificate.commonName && data.certificate.commonName !== data.externalIp) {
+ const matchesIp = data.certificate.subjectAltNames?.includes(data.externalIp)
+ || data.certificate.commonName === data.externalIp;
+ if (!matchesIp) {
return {
error: ERRORS.CERTIFICATE_IP_MISMATCH,
data,
};
}🤖 Prompt for AI Agents
In
`@packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js`
around lines 95 - 101, The current check in
validateLetsEncryptCertificateFactory.js only inspects
data.certificate.commonName and short-circuits when CN is null, so update
validate logic to inspect SubjectAltName (SAN) IP entries instead: extract
IP-type SANs from data.certificate.subjectAltName (or equivalent SAN list on
data.certificate), ensure at least one SAN IP equals data.externalIp, and return
ERRORS.CERTIFICATE_IP_MISMATCH if none match; also update the error message used
in obtainLetsEncryptCertificateTaskFactory.js (the message that currently prints
the null commonName) to reference the external IP and/or the SANs (not
commonName) so it never prints "null" as the certificate IP.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`:
- Around line 186-193: After verifying ctx.legoCertPath exists, also check that
ctx.legoKeyPath exists and throw an Error if the key file was not created
(similar message to the cert check); update the success path only after both
files exist (the block inside the obtainLetsEncryptCertificateTaskFactory task
where task.output is set) so downstream code that reads ctx.legoKeyPath will not
fail.
- Around line 80-82: The error thrown for ERRORS.CERTIFICATE_IP_MISMATCH
currently interpolates ctx.certificate.commonName which can be null when
--disable-cn is used; update the message in the throw inside the
ERRORS.CERTIFICATE_IP_MISMATCH branch to avoid printing "null" by preferring a
SAN value when available (e.g., use ctx.certificate.subjectAlternativeNames or
the first SAN) or by omitting the CN and phrasing the message generically (e.g.,
"Certificate IP does not match external IP <ctx.externalIp>"). Make the change
in the ERRORS.CERTIFICATE_IP_MISMATCH handler where ctx.certificate.commonName
and ctx.externalIp are referenced so the message never shows "null."
- Around line 145-156: The docker container creation sets User: `${uid}:${gid}`
when calling docker.createContainer (containerName, LEGO_IMAGE, legoArgs) which
will prevent binding to port 80 as a non-root user; change the container options
to either remove the User field or add the NET_BIND_SERVICE capability in
HostConfig (CapAdd: ['NET_BIND_SERVICE']) so the goacme/lego image can bind to
privileged ports, or alternatively switch the task to use a DNS challenge
instead of exposing 80/tcp; update the block that builds the container options
(the object passed to docker.createContainer) accordingly and ensure
ExposedPorts/PortBindings remain consistent with the chosen approach.
♻️ Duplicate comments (2)
packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js (1)
98-104: IP mismatch check is ineffective when CN is disabled.The obtain flow passes
--disable-cn, socommonNameis always null. The AND condition short-circuits, skipping validation entirely. This was flagged in the previous review—please address by validating SubjectAltName (SAN) IP entries instead.packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js (1)
43-44: Honor explicitexpiration-days = 0.Using
||treats 0 as falsy. This was flagged in the previous review—use nullish coalescing (??) instead.
| case ERRORS.CERTIFICATE_IP_MISMATCH: | ||
| throw new Error(`Certificate IP ${ctx.certificate.commonName} does not match external IP ${ctx.externalIp}.\n` | ||
| + 'Please change the external IP in config or use --force to obtain a new certificate.'); |
There was a problem hiding this comment.
Error message will display "null" when CN is disabled.
Since --disable-cn is used (line 119), ctx.certificate.commonName will be null. The error message should reference the expected IP from SANs or simply state the mismatch without the null value.
🔧 Proposed fix
case ERRORS.CERTIFICATE_IP_MISMATCH:
- throw new Error(`Certificate IP ${ctx.certificate.commonName} does not match external IP ${ctx.externalIp}.\n`
+ throw new Error(`Certificate does not match external IP ${ctx.externalIp}.\n`
+ 'Please change the external IP in config or use --force to obtain a new certificate.');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case ERRORS.CERTIFICATE_IP_MISMATCH: | |
| throw new Error(`Certificate IP ${ctx.certificate.commonName} does not match external IP ${ctx.externalIp}.\n` | |
| + 'Please change the external IP in config or use --force to obtain a new certificate.'); | |
| case ERRORS.CERTIFICATE_IP_MISMATCH: | |
| throw new Error(`Certificate does not match external IP ${ctx.externalIp}.\n` | |
| 'Please change the external IP in config or use --force to obtain a new certificate.'); |
🤖 Prompt for AI Agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`
around lines 80 - 82, The error thrown for ERRORS.CERTIFICATE_IP_MISMATCH
currently interpolates ctx.certificate.commonName which can be null when
--disable-cn is used; update the message in the throw inside the
ERRORS.CERTIFICATE_IP_MISMATCH branch to avoid printing "null" by preferring a
SAN value when available (e.g., use ctx.certificate.subjectAlternativeNames or
the first SAN) or by omitting the CN and phrasing the message generically (e.g.,
"Certificate IP does not match external IP <ctx.externalIp>"). Make the change
in the ERRORS.CERTIFICATE_IP_MISMATCH handler where ctx.certificate.commonName
and ctx.externalIp are referenced so the message never shows "null."
| const container = await docker.createContainer({ | ||
| name: containerName, | ||
| Image: LEGO_IMAGE, | ||
| Cmd: legoArgs, | ||
| User: `${uid}:${gid}`, | ||
| ExposedPorts: { '80/tcp': {} }, | ||
| HostConfig: { | ||
| AutoRemove: true, | ||
| Binds: [`${ctx.legoDir}:/data`], | ||
| PortBindings: { '80/tcp': [{ HostPort: '80' }] }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
goacme/lego docker image non-root user support documentation
💡 Result:
Short answer: The official goacme/lego Docker image does not document a non-root user or show a USER in its published metadata — it expects the default (root) runtime. See the image page and installation docs. [1][2]
If you need a rootless/root‑nonroot build, use a community rootless image (example: 11notes/lego) or build your own Dockerfile that adds a non‑root user and sets USER. [3][4]
Sources
- goacme/lego Docker Hub page. [1]
- lego installation (Docker) in official docs. [2]
- Example community rootless image (11notes/lego) on Docker Hub. [3]
- Public image metadata showing no USER configured for goacme/lego. [4]
🌐 Web query:
goacme lego GitHub dockerfile USER instruction
💡 Result:
Short answer: lego’s Docker image can run non-root, but the binary needs permission to bind ports 80/443 (or you must avoid binding those ports). Use the Dockerfile USER instruction to drop privileges after preparing files and ownership, or give the binary CAP_NET_BIND_SERVICE with setcap. See lego docs on "Running without root privileges" and the official image. [1][2]
Example patterns:
- Create a non-root user and run as it (common):
- create user, chown .lego (or other runtime dirs), then switch USER.
Dockerfile snippet:
FROM golang:1.20-alpine AS build
build steps...
FROM alpine:3.18
add lego binary and any certs
RUN addgroup -S lego && adduser -S -G lego lego
&& mkdir -p /home/lego/.lego
&& chown -R lego:lego /home/lego /etc/ssl
USER lego
WORKDIR /home/lego
ENTRYPOINT ["/usr/local/bin/lego"]
Use --http.port/--tls.port or DNS challenge if you cannot bind 80/443 as non-root. [2]
- Grant CAP_NET_BIND_SERVICE to allow binding 80/443 while running non-root:
- setcap must be applied to the binary (on the final image or during runtime build). Example:
RUN apk add --no-cache libcap && setcap 'cap_net_bind_service=+ep' /usr/local/bin/lego
then optionally drop to non-root USER as above.
Notes:
- If you run lego as non-root without capabilities you must either forward host ports 80/443 to higher ports (and run lego with --http.port/--tls.port) or use the DNS challenge. [2]
- Official goacme/lego image and docs: Docker Hub & project site. [1][2]
References:
[1] goacme/lego Docker image (official).
[2] lego — Running without root privileges / Port Usage (official docs).
The User option will cause a runtime failure with the official goacme/lego image.
The official goacme/lego Docker image runs as root by default and does not support non-root execution without additional configuration. Setting User: ${uid}:${gid} combined with PortBindings: { '80/tcp': [{ HostPort: '80' }] } will fail—non-root users cannot bind to privileged ports (< 1024) without the CAP_NET_BIND_SERVICE capability.
To fix this, either:
- Remove the
Useroption and accept running as root - Add
CAP_NET_BIND_SERVICEto the container (usingCapAdd: ['NET_BIND_SERVICE']) - Switch to a DNS challenge instead of HTTP challenge to avoid binding port 80
- Use a community rootless image variant instead of the official goacme/lego image
🤖 Prompt for AI Agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`
around lines 145 - 156, The docker container creation sets User: `${uid}:${gid}`
when calling docker.createContainer (containerName, LEGO_IMAGE, legoArgs) which
will prevent binding to port 80 as a non-root user; change the container options
to either remove the User field or add the NET_BIND_SERVICE capability in
HostConfig (CapAdd: ['NET_BIND_SERVICE']) so the goacme/lego image can bind to
privileged ports, or alternatively switch the task to use a DNS challenge
instead of exposing 80/tcp; update the block that builds the container options
(the object passed to docker.createContainer) accordingly and ensure
ExposedPorts/PortBindings remain consistent with the chosen approach.
There was a problem hiding this comment.
Hmmm... unsure about this, it appears to work when I tested it as the dashmate user?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Docker container non-root user bind port 80 inside container network namespace
💡 Result:
Short answer — give the process the CAP_NET_BIND_SERVICE capability (or give that capability to the binary) or bind on a high port and map host port 80 -> container port.
Options and commands (pick one):
- Add capability to the container process at runtime
- docker run --cap-add=NET_BIND_SERVICE --user 1000 ... image
This grants the process the capability to bind <1024. Note: some runtimes/contexts (rootless Docker, certain security settings) may still prevent it unless ambient/inheritable caps are set correctly. [1][2]
- Give the application binary the capability inside the image (recommended for least privilege)
- In Dockerfile:
- install libcap (libcap2-bin / libcap)
- setcap 'cap_net_bind_service=+ep' /path/to/executable
- switch to non-root user and run
This makes that specific executable able to bind port 80 without running as UID 0. Example pattern: setcap on /usr/bin/python3 or your app binary. [3][4]
- Use an explicit bind + port mapping (no special capabilities)
- Run the app on an unprivileged port (e.g. 8080) as non-root and expose/map host port 80:
docker run -p 80:8080 image
This keeps the container non-root and avoids capabilities but requires clients to use the mapped host port or internal services to use the higher port. [4]
Special/rootless cases and caveats
- Rootless Docker/podman: privileged port bind may be blocked by rootless network stack or kernel sysctl net.ipv4.ip_unprivileged_port_start (default 1024). Workarounds include adjusting that sysctl or granting capabilities to rootless helper (rootlesskit) or using setcap on the binary — behaviour varies by runtime. [5][1]
- Kubernetes: adding NET_BIND_SERVICE in securityContext is common, but ensure capability ends up in the container's effective/ambient set; otherwise binding still fails. Some setups need setcap on the executable or allowPrivilegeEscalation=false and correct ambient support. [2][5]
References
- How to bind non-root process to privileged port inside Docker (example Dockerfile + setcap). [1]
- Using CAP_NET_BIND_SERVICE via securityContext / runtime caveats (Kubernetes issues, ambient capabilities). [2]
- Practical guide: setcap in Dockerfile to grant single binary ability to bind <1024. [3]
- Explanation and history of capability issues and workarounds (blog / troubleshooting). [4]
- Rootless Docker notes: rootlesskit and net.unprivileged_port_start implications. [5]
If you want, tell me which runtime (Docker rootful vs rootless, Kubernetes, podman) and your app language/binary and I’ll give a concrete Dockerfile and docker run example.
🌐 Web query:
goacme lego docker image capabilities NET_BIND_SERVICE setcap
💡 Result:
Short answer: lego supports running without root by either using setcap on the lego binary (cap_net_bind_service), running on non‑privileged ports, or using DNS challenges — and for containers you normally either grant the capability to the container (docker run --cap-add=NET_BIND_SERVICE / compose cap_add) or set the capability on the binary during image build. See notes and links below. [1][2][3][4]
Key points and practical options
- lego docs: lego can bind low ports without sudo by (a) setcap 'cap_net_bind_service=+ep' /path/to/lego, (b) change --http.port/--tls.port, or (c) use DNS challenge. Use whichever fits your deployment. [1]
- Granting capability to the container: give the container CAP_NET_BIND_SERVICE at runtime (docker run --cap-add=NET_BIND_SERVICE or docker-compose cap_add / Kubernetes securityContext add: ["NET_BIND_SERVICE"]). This lets a non‑root process bind ports <1024. [3]
- Using setcap in an image: you can run setcap in your Dockerfile (install libcap and run setcap on the lego binary) so the binary has cap_net_bind_service. However, be aware of environment caveats: some runtimes or host security policies (e.g., platforms that drop file capabilities, certain container runtimes, or mounting behavior) can make setcap ineffective at runtime — granting the capability to the container is more robust. [3][2]
- Docker image notes: Docker’s capability model and filesystem handling mean capability bits on files in images may not behave the same across runtimes; audit with getcap/getpcaps and prefer runtime cap_add when possible for portability. [2]
References
- lego docs — running without root / options (setcap, ports, DNS). [1]
- Docker / capabilities explanation (file capability limits, getcap, and runtime cap_add examples). [2]
- Practical examples: using setcap in Dockerfile and adding NET_BIND_SERVICE in Kubernetes / Docker. [3]
- goacme/lego Docker Hub page (image tags/usage). [4]
If you want, I can:
- show a minimal Dockerfile that installs libcap and runs setcap on lego, or
- show docker run / docker-compose / Kubernetes snippets to give CAP_NET_BIND_SERVICE to the container. Which would you prefer?
@ktechmidas Thanks for testing! That's interesting that it worked in your environment. Let me clarify what might be happening:
When Docker does port mapping with PortBindings: { '80/tcp': [{ HostPort: '80' }] }, the Docker daemon (running as root) handles the host-side port 80 binding. However, inside the container, the lego process still needs to bind to port 80 in the container's network namespace, which typically requires either root privileges or the CAP_NET_BIND_SERVICE capability.
Your test might have succeeded due to:
- Recent kernel versions or Docker configurations that handle capabilities differently
- The
net.ipv4.ip_unprivileged_port_startsysctl being set below 80 on your system - Some other environment-specific configuration
Recommendation: To ensure this works reliably across different environments and production setups, I suggest explicitly adding the capability:
HostConfig: {
AutoRemove: true,
Binds: [`${ctx.legoDir}:/data`],
PortBindings: { '80/tcp': [{ HostPort: '80' }] },
CapAdd: ['NET_BIND_SERVICE'],
},This makes the requirement explicit and ensures consistent behavior regardless of the host's kernel configuration. Could you verify whether your test environment has any special Docker/kernel settings, or would you prefer to add the explicit capability for safety?
🧠 Learnings used
Learnt from: shumkov
Repo: dashpay/platform PR: 2297
File: packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js:148-150
Timestamp: 2024-10-31T14:41:42.158Z
Learning: In the `obtainZeroSSLCertificateTaskFactory` function in `packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js`, the application already fails appropriately if `configFileRepository.write(configFile)` fails, so additional error handling around this operation is unnecessary.
Learnt from: shumkov
Repo: dashpay/platform PR: 2298
File: packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js:27-48
Timestamp: 2024-10-31T15:53:06.937Z
Learning: In `cleanupZeroSSLCertificatesTask` located in `packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js`, if the certificate listing API call fails, we prefer the entire command to fail without additional error handling.
Learnt from: shumkov
Repo: dashpay/platform PR: 2298
File: packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js:74-74
Timestamp: 2024-10-31T15:54:57.897Z
Learning: In the `cleanupZeroSSLCertificatesTask` function located in `packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js`, the team prefers to include a small fixed delay between all requests to avoid overloading ZeroSSL, rather than implementing exponential backoff.
Learnt from: shumkov
Repo: dashpay/platform PR: 2298
File: packages/dashmate/src/commands/ssl/cleanup.js:33-50
Timestamp: 2024-10-31T15:37:28.986Z
Learning: In `packages/dashmate/src/commands/ssl/cleanup.js`, the `cleanupZeroSSLCertificatesTask` function already includes progress reporting within its nested `Listr` tasks, so additional progress reporting in the `CleanupCommand` is unnecessary.
Learnt from: shumkov
Repo: dashpay/platform PR: 2365
File: packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js:206-213
Timestamp: 2024-12-03T05:47:30.524Z
Learning: In `obtainZeroSSLCertificateTaskFactory.js`, when handling ZeroSSL API errors during domain verification, the error detail structure is consistent when an error occurs, so additional null checks on `e.details[ctx.externalIp]` may not be necessary.
Learnt from: shumkov
Repo: dashpay/platform PR: 2107
File: packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js:116-124
Timestamp: 2024-11-22T10:28:33.588Z
Learning: In `configureCoreTaskFactory` in `packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js`, the `local_seed` configuration is already established earlier in the code, so adding error handling for its absence in the 'Generating funds to use as a collateral for masternodes' task is unnecessary.
| // Verify certificate was created | ||
| if (!fs.existsSync(ctx.legoCertPath)) { | ||
| throw new Error('Certificate file was not created by lego'); | ||
| } | ||
|
|
||
| // eslint-disable-next-line no-param-reassign | ||
| task.output = 'Certificate obtained successfully'; | ||
| }, |
There was a problem hiding this comment.
Also verify the key file was created.
Only the certificate file existence is verified, but the key file is also required and read on line 201.
🔧 Proposed fix
// Verify certificate was created
if (!fs.existsSync(ctx.legoCertPath)) {
throw new Error('Certificate file was not created by lego');
}
+
+ if (!fs.existsSync(ctx.legoKeyPath)) {
+ throw new Error('Private key file was not created by lego');
+ }🤖 Prompt for AI Agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`
around lines 186 - 193, After verifying ctx.legoCertPath exists, also check that
ctx.legoKeyPath exists and throw an Error if the key file was not created
(similar message to the cert check); update the success path only after both
files exist (the block inside the obtainLetsEncryptCertificateTaskFactory task
where task.output is set) so downstream code that reads ctx.legoKeyPath will not
fail.
Add Let's Encrypt as a new SSL certificate provider option using the goacme/lego Docker client. This enables IP-based certificate issuance with automatic renewal support for shortlived certificates.
New files:
Changes:
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.