You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Several proxy and SSL paths could leave a site in an inconsistent state after a failure: a proxy cache rollback could leave nginx unable to load its config, a failed Let's Encrypt alias change reported success and revoked the certificate still being served, update --ssl=le saved le without a certificate, and deleting a site or turning SSL off left certificate files behind.
Fix
A proxy cache update that fails keeps a valid proxy config: default.conf is regenerated before nginx -t, the test is retried once, exactly the files the update wrote are restored (also when a write fails part-way), and alias changes reload the proxy instead of restarting it.
A failed Let's Encrypt alias change rolls back instead of reporting success: the certificate, ACME and redirect files are restored, the aliases stay unchanged and the command exits non-zero. ssl-renew <site> also exits non-zero when the renewal fails.
A site's certificate and ACME files (including those of its aliases and its www counterpart) are removed on delete and --ssl=off whenever they exist.
Sites without SSL stay HTTP-only when their aliases change.
Token-bearing ACME challenges of every type are kept when ordering a certificate; only token-less types EE can't solve are skipped, so newer ACME servers that offer them no longer break every order.
Certificates without a subject CN are parsed, using the first SAN as the subject.
SSL can be turned off on sites stored as wildcard, and the wildcard flag is kept.
The proxy is reloaded after a deleted site's www redirect is removed, so www. stops redirecting to it.
update --ssl=le fails when no certificate was issued.
Only certificates that an alias change actually replaced are revoked.
--ssl=off is refused while other sites inherit the certificate.
A failure in update --ssl= puts the site back HTTP-only and exits non-zero before anything is saved, including removing a copied custom certificate pair.
Removing an alias also removes its ACME state.
A failed update --ssl=custom keeps certificate files that already sit in the certs dir.
A self-signed site can turn SSL back on after --ssl=off.
update --ssl=self no longer needs --wildcard on a subdomain multisite or a site with a *.<site> alias after --ssl=off, and stores the wildcard flag as 1, since a self-signed certificate always covers *.<site>.
The re-run hint after a failed update --ssl= repeats --wildcard and the quoted key/cert paths.
--ssl=off and delete remove a wildcard Let's Encrypt site's ACME state.
Testing
Proxy cache: a broken include during an alias change, a transient failure that passes on retry, alias delete and proxy cache off, each followed by nginx -t, a proxy restart and HTTP checks.
SSL off, alias changes on sites without SSL, the inherit guard and delete cleanup on self-signed and plain sites.
Let's Encrypt flows (alias add and delete, failed issuance rollback, renewal, off, delete) on Pebble 2.9.0 and 2.10.1, including SAN-only certificates, on PHP 7.4 and 8.5.
Live on a host: 4 real Let's Encrypt certificates (create, alias add and delete, update --ssl=le, then off and delete) and the failure paths (failed alias changes rolled back with no revocation, a failed custom-cert update, off then back on, wildcard ACME cleanup), plus the Pebble CI lanes against develop.
update_proxy_cache() tested nginx before default.conf was regenerated and, on a failure, removed the site's cache zone plus whichever location file the alias loop had written last. The other locations kept `proxy_cache <zone>` with no zone, so every later `nginx -t` failed, and after a proxy restart nginx could not start at all.
- Regenerate default.conf before the test and retry it once after a second, since the proxy's docker-gen rewrites the file in place and a test racing it can read a partial file. reload_global_nginx_proxy() retries the same way when the failure is in default.conf.
- On a failure, put back exactly the files this call wrote (restoring earlier contents, removing new ones), so a rollback never leaves a location without its zone.
- A failed enable now exits non-zero and keeps proxy_cache unchanged; on alias changes and site create it warns instead, and create stores proxy_cache as off.
- Alias changes only write locations, so they reload the proxy instead of restarting it (which took every site down for ~0.3 s).
- Deleting an alias removes its proxy cache location, which would otherwise use this site's zone if the domain were ever served again.
…sued
When the Let's Encrypt order, validation or request failed during `--add-alias-domains`/`--delete-alias-domains`, init_le() only warned and cleared site_ssl. The command then printed "SSL renewal completed" and "Alias domains updated", revoked the certificate that was still being served, and saved the new aliases with SSL "Not Enabled", so later renewals failed with "Only Letsencrypt certificate renewal is supported.".
The certificate is now issued through reissue_le_certificate(), which throws when init_le() did not issue one, after putting back the site's certificate, ACME and redirect files. The alias change then takes the existing revert path: the site is refreshed from the unchanged DB, the failure hook fires, the new domains' authorization challenges are removed, and the command exits non-zero. The old certificate is only revoked after a successful reissue.
`ee site ssl-renew <site>` gets the same restore and now exits non-zero on a failed renewal instead of printing "SSL renewal completed."; under `--all` it warns and moves on to the next site.
`ee site delete` only removed the site's certificate and ACME files when site_ssl was set, and `--ssl=off` never removed them. Certificates left after SSL was turned off or lost stayed in nginx-proxy's certs forever, where its closest-name matching keeps serving them for the site and for similarly named sites.
Delete now removes `certs/<site>.{crt,key,chain.pem}`, `acme-conf/certs/<site>` and the `acme-conf/var` state of the site, its alias domains and its www counterpart (unless that name belongs to another site), whatever the SSL type. This also removes the empty alias directories and the www authorization challenge that deleting an LE site used to leave behind.
`--ssl=off` rewrites the site's www redirect without its HTTPS block, which loads the certificate, and then removes the same files.
update_alias_domains() dumped the compose file with `nohttps` only for LE sites, so adding or deleting an alias dropped `HTTPS_METHOD=nohttps` from a site without SSL until the next refresh. nginx-proxy then falls back to `redirect` and turns HTTPS on as soon as a certificate file name matches the host, e.g. the site's own leftover certificate or a parent site's.
…icate
acmephp 1.3's requestOrder() builds an AuthorizationChallenge from every challenge of every authorization and reads `token` from each. A challenge type without a token, such as the draft dns-persist-01 that Pebble 2.10 offers on every authorization and that Let's Encrypt has announced, makes the order throw, so every issuance failed with "It seems you're in local environment or using non-public domain".
EEAcmeClient now overrides requestOrder() and keeps only http-01 and dns-01 challenges that carry a token. An authorization left without any still reaches authorize(), which reports the domain as unsupported. No vendor code is patched.
acmephp's CertificateParser throws `Missing expected key "subject.cn"` for SAN-only certificates, which Let's Encrypt's tlsserver and shortlived profiles and Pebble's default profile issue. `ee site ssl-info` then printed "Could not parse certificate", and the renewal checks (isRenewalNecessary(), ssl_needs_creation()) threw.
EECertificateParser falls back to the first SAN as the subject, as the classic profile does, and is used everywhere site-command parses a certificate.
update_ssl() compared the stored wildcard flag with `--wildcard` for every change, including `--ssl=off`. Sites created with `--ssl=self` are stored with site_ssl_wildcard=1, as are wildcard LE sites, so `--ssl=off` failed with "Update from wildcard SSL to normal SSL is not supported yet." and `--ssl=off --wildcard` with "You cannot use --wildcard flag with --ssl=off": SSL could never be turned off on them. The wildcard checks now only apply when SSL is being enabled.
delete_site() removed `<site>-redirect.conf` without setting `$reload`, and the proxy had already reloaded when the site's containers went down, so nginx kept redirecting `www.<site>` to the deleted site until some later reload.
When the Let's Encrypt registration, order or validation failed, init_le() only warned and cleared site_ssl on the array copy, while update_ssl() saved `le` on the model and printed "Enabled SSL". The site was then recorded as an LE site without a certificate.
update_ssl() now checks the result, puts the site back to HTTP-only through disable_ssl() (which also removes the failed order's ACME state), and exits non-zero without saving.
The requestOrder() override kept only http-01 and dns-01. An authorization that is already valid through another challenge type lists only that challenge, so it came back empty and authorize() refused a domain that acmephp's own code accepted. Now only challenges acmephp can't represent (no token, type, status or url, e.g. dns-persist-01) are skipped; authorize() still picks the one its solver supports, as before.
A wildcard or DNS-01 LE site without Cloudflare credentials returns from init_le() with a "run ssl-verify" notice and no new certificate, so the alias change counted as a success and revoked the certificate still being served. Old certificates are now revoked only when a different one was stored for the same domain.
With `--ssl=off` allowed on wildcard sites, turning it off on a parent removed the certificate its `inherit` children load in their www redirects, and nginx could no longer load its config. The command now names those children and stops before any change.
Turning SSL off also keeps the stored wildcard flag, so the site's SSL (e.g. a subdom multisite's wildcard) can be enabled again with the same flags.
A registration or finalize exception skipped the new site_ssl check and went straight to the error, leaving the half-configured SSL state behind. Any failure while enabling SSL now runs disable_ssl() first, and the error says to re-run `ee site update <site> --ssl=<type>` instead of the `ssl-verify` hint, which can't work once the order is gone.
Removing an alias left acme-conf/var/<alias> (its authorization challenge) behind, and since the site no longer lists the alias, deleting the site later couldn't find it either (SSL-5). Found by the le.feature "no ACME state behind" scenario on Pebble.
…update
Since EasyEngine#496, update_alias_domains() resolves and validates le-mail for LE sites before the compose dump, and exits with an error if that fails, so the fallback before reissue_le_certificate() can't run. It also bypassed that validation.
…when enabling SSL fails
`ee site update <site> --ssl=custom --ssl-key=<certs dir>/<site>.key --ssl-crt=<certs dir>/<site>.crt` is allowed (custom_site_ssl() skips copying a file onto itself), so there the files in nginx-proxy's certs dir are the user's only copy. A later failure in `update --ssl=custom` ran disable_ssl(), which removed them, and the error then asked to re-run with files that no longer existed (SSL-16). The rollback now leaves the passed key and certificate in place; a pair copied from elsewhere is still removed.
Sites created with `--ssl=self` are stored with site_ssl_wildcard=1, because self-signed certificates always cover `*.<site>`. `--ssl=off` kept that flag, so the next `ee site update <site> --ssl=self`, the same flags the site was created with, and `--ssl=le` both failed with "Update from wildcard SSL to normal SSL is not supported yet." unless `--wildcard` was added, which for Let's Encrypt forces DNS-01. Turning off self-signed SSL now clears the flag, except on subdomain multisites, which need a wildcard certificate whatever the SSL type. Wildcard Let's Encrypt and custom sites keep it as before.
The error of a failed `ee site update <site> --ssl=<type>` suggested re-running with only `--ssl=<type>`. For `--ssl=<type> --wildcard` that command fails with "Update from wildcard SSL to normal SSL is not supported yet.", and for `--ssl=custom` with "Pass --ssl-key and --ssl-crt for custom SSL". The hint now repeats `--wildcard` and the key and certificate paths that were passed.
… the site
A wildcard Let's Encrypt order stores the authorization of `*.<site>` under `acme-conf/var/*.<site>`, whether or not `*.<site>` is also an alias domain. Site delete and `--ssl=off` only removed the state of the site, its alias domains and its www counterpart, so that directory stayed behind (SSL-5). It is now removed too, unless `*.<site>` is another site's alias domain.
The docblocks of get_site_ssl_file_paths() and reissue_le_certificate() read as if every ACME file an issuance touches were restored. Only the served certificate, the ACME key pair, certificates and DN, and the www redirect are; authorization challenges and orders are left as they are, because the next order revokes and replaces them.
…e wildcard
aa4766d cleared the stored wildcard flag when a self-signed site turned SSL off. That let `--ssl=self` and `--ssl=le` work again, but broke the flags the site may have been created with: `--ssl=self --wildcard` and `--ssl=le --wildcard` were then refused as "Update from normal SSL to wildcard SSL", and a `*.<site>` alias domain ended up with a flag that no longer matched it.
The flag is kept again on `--ssl=off`. Instead, "Update from wildcard SSL to normal SSL is not supported yet." is only raised when a normal certificate can't cover the site: a subdomain multisite, or a site with a `*.<site>` alias domain. Self-signed sites are stored as wildcard although they don't need it, so after `--ssl=off` they can enable SSL again with or without `--wildcard`.
mrrobot47
added a commit
to mrrobot47/site-command
that referenced
this pull request
Sep 29, 2026
Propagate docker-gen failures before testing stale configuration
src/helper/site-utils.php:828
The regeneration result is ignored, so if docker-gen fails while the previous default.conf is still valid, the following nginx -t succeeds against stale configuration and callers treat the update as successful. Propagate regeneration failures (and include them in the optional retry) instead of testing/reloading the old file.
Fail safely when existing backup files cannot be read
src/helper/site-utils.php:845
file_get_contents() can return false for an existing but unreadable or concurrently removed file. Recording that as backup content makes rollback write an empty value (or throw), potentially corrupting the previously valid proxy/certificate file. Fail before making changes when an existing file cannot be read.
backup_files() recorded a failed read as the file's content, so a later rollback would replace the file with an empty one. It now throws before the caller changes anything.
Unreadable backup files (site-utils.php:845): fixed in a9372b4. backup_files() now throws before the caller changes anything when an existing file can't be read, instead of recording false and later writing an empty file back.
Ignored docker-gen result (site-utils.php:828): leaving this as is in this PR. develop's reload_global_nginx_proxy() already ignores the regeneration result the same way; the PR only moves it into test_global_nginx_proxy_config(). When the regeneration fails, default.conf is still the last generated one the proxy is serving, and nginx -t still checks the conf.d/vhost.d files and certificates the update wrote. I'll handle it separately.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Self-signed SSL re-enablement can still be rejected or persisted with an incorrect wildcard flag.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Allow self-signed SSL updates for wildcard sites
src/helper/class-ee-site.php:1024
Self-signed certificates are always wildcard certificates, but this guard still rejects the documented update <site> --ssl=self flow for a subdomain-style site or a site with a *.<site> alias after SSL was turned off. Exempt self here; init_ssl() generates the required wildcard certificate without requiring the CLI flag.
This issue also appears on line 1063 of the same file.
Self-signed certificates always cover *.<site>, but update --ssl=self stored the wildcard flag from --wildcard, so it saved 0 without the flag, and the wildcard checks refused --ssl=self on a subdomain multisite or a site with a *.<site> alias after --ssl=off unless --wildcard was repeated. --ssl=self now skips both checks and stores the flag as 1, as create does.
Fixed in 5c34be8: after --ssl=off, --ssl=self no longer needs --wildcard on a subdomain multisite or a site with a *.<site> alias, and update --ssl=self stores the wildcard flag as 1, since a self-signed certificate always covers *.<site>.
Preserve SAN fallback when certificate subject CN is absent
src/helper/class-ee-site.php:2155
Using EECertificateParser does not fix ssl-info for a certificate without a subject CN: line 2168 re-parses the raw certificate and sets issued_to_CN to an empty string, discarding the parser's first-SAN fallback. Fall back to $subject when the raw CN is absent.
Handle docker-gen failures before reloading configuration
src/helper/site-utils.php:828
A failed docker-gen invocation is currently ignored. If the old default.conf is still valid, nginx -t returns success and callers reload/restart using stale configuration instead of rolling back the update. Treat regeneration failure as the test failure (and include it in the one retry) before reporting success.
Quote certificate paths in generated retry commands
src/helper/class-ee-site.php:1093
The generated retry command breaks when either validated certificate path contains spaces or shell metacharacters, because the paths are appended without quoting. Escape each path so the operator can copy and run the hint unchanged.
The backup was only restored after a failed nginx -t. If a write threw part-way (for example a full disk), the command exited with the zone or some locations already written and the site still recorded without proxy cache, so the next proxy reload applied a partial cache config. Restore the backup before rethrowing.
…s no CN
ssl-info re-read the subject CN from the raw certificate and showed an empty "issued to" for certificates without one, dropping the parser's first-SAN fallback.
ssl-info subject for certificates without a CN (class-ee-site.php): fixed in 4d549a5. "Issued to" now falls back to the parser's subject, which is the first SAN.
Quoting the paths in the --ssl=custom re-run hint: fixed in 0fbab4d, both paths are shell-quoted.
Ignored docker-gen result (site-utils.php): same as the earlier reply, this is pre-existing behaviour and stays out of this PR; it's tracked for a follow-up.
Ignored regeneration failure allows stale nginx config validation
src/helper/site-utils.php:828
The regeneration result is ignored, so if docker-gen fails before replacing default.conf, nginx -t can pass against the previous file and callers proceed as though the new proxy configuration was validated. Check the regeneration exit code (and include it in the one retry) before running or accepting nginx -t; otherwise a cache/alias update can be saved without ever producing the configuration it intended to serve.
Ignore docker-gen failure and validate stale configuration
src/helper/site-utils.php:819
The regeneration result is discarded, so a failed docker-gen can be followed by a successful nginx -t against the old default.conf. The helper then reports success and callers reload/restart without ever applying the new container/alias state. Treat a non-zero regeneration exit as failure (and include it in the one-retry path) before testing the generated file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several proxy and SSL paths could leave a site in an inconsistent state after a failure: a proxy cache rollback could leave nginx unable to load its config, a failed Let's Encrypt alias change reported success and revoked the certificate still being served,
update --ssl=lesavedlewithout a certificate, and deleting a site or turning SSL off left certificate files behind.Fix
nginx -t, the test is retried once, exactly the files the update wrote are restored (also when a write fails part-way), and alias changes reload the proxy instead of restarting it.ssl-renew <site>also exits non-zero when the renewal fails.--ssl=offwhenever they exist.www.stops redirecting to it.update --ssl=lefails when no certificate was issued.--ssl=offis refused while other sites inherit the certificate.update --ssl=puts the site back HTTP-only and exits non-zero before anything is saved, including removing a copied custom certificate pair.update --ssl=customkeeps certificate files that already sit in the certs dir.--ssl=off.update --ssl=selfno longer needs--wildcardon a subdomain multisite or a site with a*.<site>alias after--ssl=off, and stores the wildcard flag as 1, since a self-signed certificate always covers*.<site>.update --ssl=repeats--wildcardand the quoted key/cert paths.--ssl=offand delete remove a wildcard Let's Encrypt site's ACME state.Testing
nginx -t, a proxy restart and HTTP checks.update --ssl=le, then off and delete) and the failure paths (failed alias changes rolled back with no revocation, a failed custom-cert update, off then back on, wildcard ACME cleanup), plus the Pebble CI lanes against develop.Dependencies
Built on the merged SSL PRs (#485, #486, #487, #489, #490, #491, #492, #493, #495, #496, #497, #498, #499, #500), which touch the same functions.