Skip to content

fix(ssl): include ACME error message in operator-facing warnings - #486

Open
mrrobot47 wants to merge 3 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-surface-acme-errors
Open

mrrobot47 wants to merge 3 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-surface-acme-errors

Conversation

@mrrobot47

@mrrobot47 mrrobot47 commented Jun 30, 2026 •

Copy link
Copy Markdown
Member

Problem

SSL issuance/renewal failures showed operators only a generic EE::warning(); the real exception went to EE::debug(), i.e. only into ee.log, never to stderr. Renewal runs unattended (cron) without --debug, so the actual ACME error (rate limit, rejectedIdentifier, badNonce, a finalize/storage failure, etc.) never reached the operator. Worse, the new-order catch in authorize(), where Let's Encrypt rate limits and rejected identifiers surface on both issuance and renewal, didn't log the exception at all, so its "please check logs" pointed at nothing.

Separately, both executeRenewal() catch blocks emitted a hard-coded "Challenge Authorization failed. Check logs and check if your domain is pointed correctly to this server.". These catches fire for any exception/throwable (finalize failure, cert-store write error, etc.), so that line frequently misdirected operators toward a DNS/challenge problem that wasn't the cause.

Fix

  • Append $e->getMessage() to the operator-facing warning in check(), in both executeRenewal() catches, and in authorize()'s new-order catch (Let's Encrypt order request failed (<reason>). It seems you're …).
  • Drop the misleading generic "Challenge Authorization failed…" line from the executeRenewal() catches.
  • The "re-run ssl-verify" hint in executeRenewal() now always names the site (it used $domains[0], which is undefined when loading the stored cert/key fails, and printed an empty name plus PHP warnings).
  • Fix the "occured" typo.

Notes

The exception text surfaces at warning level. ACME error strings contain no account-key material or JWS tokens (the ACME server never echoes those); the only incremental disclosure is absolute server paths on filesystem errors, acceptable for operator-run CLI tooling. (The pre-existing EE::debug(print_r($e)) in executeRenewal() is a separate problem: with PHP's default zend.exception_ignore_args=0 it can write the domain private key to ee.log; tracked as a follow-up.)

Testing

Manual: ee site create x.test --ssl=le now shows Let's Encrypt order request failed ([rejectedIdentifier] … Cannot issue for "x.test" …); a renewal that fails after authorization (e.g. an unreadable domain key, ssl-renew --force) shows A critical error occurred during certificate renewal: <reason> with no "Challenge Authorization failed" line and the site name in the re-run hint.

Tested on Ubuntu 26.04 with EasyEngine 4.12.0, including one real Let's Encrypt certificate for the renewal-failure case.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

When renewal fails before the order is loaded (e.g. the stored certificate or key pair can't be read), $domains is not yet set, so the "re-run ee site ssl-verify" hint printed an empty site name plus PHP undefined-variable warnings. Use $domain, which is always set and equals $domains[0].
authorize() swallowed the requestOrder() exception without logging it anywhere, so new-order failures (rate limits, rejected identifiers, account errors), which Let's Encrypt reports at order creation, still showed only the generic "local environment" warning. Append the exception message as the other SSL warnings now do.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants