From b3d332f59f34fc8184e32832dd8d3dcec84daa65 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 11:44:26 +0530 Subject: [PATCH 1/5] fix(auth): validate the whole whitelist entry, including its CIDR prefix Only the address before `/` was checked, so entries like `192.0.2.1/99`, `/99` or `2001:db8::/200` were stored and written to the `_acl` files. nginx rejects such a file, so every later proxy reload was skipped, and a proxy restart failed. An entry must now be an IPv4 or IPv6 address nginx accepts, optionally with a prefix of 0-32 or 0-128 (digits only), and the error names the rejected entry. Invalid entries stored by older versions are skipped with a warning when the `_acl` files are written, and `ee auth delete --ip` still accepts them so they can be removed. The list is split on any whitespace and deduplicated, so a repeated IP no longer hits the UNIQUE(site_url, ip) constraint on create (AUTH-14). --- ...valid_whitelist_entries_from_acl_files.php | 68 +++++++++++++++++++ src/Auth_Command.php | 36 +++++++--- src/auth-utils.php | 60 ++++++++++++++-- 3 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php diff --git a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php new file mode 100644 index 0000000..af2fdaf --- /dev/null +++ b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php @@ -0,0 +1,68 @@ +is_first_execution ? [] : Whitelist::all(); + $invalid = array_filter( + $rows, + function ( $row ) { + return ! is_valid_whitelist_ip( (string) $row->ip ); + } + ); + + if ( empty( $invalid ) ) { + $this->skip_this_migration = true; + + return; + } + + $scopes = array_unique( array_column( $invalid, 'site_url' ) ); + // Global entries are merged into every site's file. + $this->site_urls = in_array( 'default', $scopes, true ) ? array_unique( array_column( $rows, 'site_url' ) ) : $scopes; + } + + /** + * Rewrites the `_acl` files that carry an invalid entry, which older versions stored unchecked: nginx rejects such a file, so proxy reloads and restarts fail. + * + * @throws EE\ExitException + */ + public function up() { + + if ( $this->skip_this_migration ) { + EE::debug( 'Skipping whitelist entries check as all entries are valid.' ); + + return; + } + + foreach ( $this->site_urls as $site_url ) { + try { + generate_site_whitelist( $site_url, 'default' === $site_url ? null : ( Site::find( $site_url ) ?: null ) ); + } catch ( \Throwable $e ) { + EE::warning( sprintf( 'Could not rewrite the whitelist files of %s: %s', $site_url, $e->getMessage() ) ); + } + } + + \EE\Site\Utils\reload_global_nginx_proxy(); + } + + /** + * Not reverted: the rewritten files only lack entries nginx rejects. + */ + public function down() { + } +} diff --git a/src/Auth_Command.php b/src/Auth_Command.php index 55588ad..cd06c9b 100644 --- a/src/Auth_Command.php +++ b/src/Auth_Command.php @@ -19,6 +19,7 @@ use Symfony\Component\Filesystem\Filesystem; use function EE\Auth\Utils\generate_site_auth_files; use function EE\Auth\Utils\generate_site_whitelist; +use function EE\Auth\Utils\is_valid_whitelist_ip; use function EE\Auth\Utils\verify_htpasswd_is_present; use function EE\Auth\Utils\write_htpasswd_file; use function EE\Site\Utils\auto_site_name; @@ -97,26 +98,41 @@ public function create( $args, $assoc_args ) { /** * Cleans and Validate IP addresses - * Converts input separated by comma, spaces and new-lines in array + * Converts input separated by comma, whitespace and new-lines in an array of unique entries * - * @param string $ips IPs to clean and validate + * @param string $ips IPs to clean and validate + * @param string $site_url If set, entries already stored for it are accepted, so ones saved by older versions can be deleted. * * @return array $user_ips Cleaned IP addresses. */ - private function clean_and_validate_ips( string $ips ) { + private function clean_and_validate_ips( string $ips, string $site_url = '' ) { - $user_ips = preg_split( '/[\ \n\,]+/', $ips ); + // Unique, as a repeated entry would hit the UNIQUE(site_url, ip) constraint. + $user_ips = array_values( array_unique( preg_split( '/[\s,]+/', trim( $ips ) ) ) ); foreach ( $user_ips as $ip ) { - // Remove subnet from ip if present. - if ( preg_match( '~^(.+?)/([^/]+)$~', $ip, $m ) ) { - $ip = $m[1]; + if ( is_valid_whitelist_ip( $ip ) ) { + continue; } - if ( ! filter_var( $ip, FILTER_VALIDATE_IP ) ) { - EE::error( 'Please check your list do not have any empty or wrong IP addresses.' ); + $stored = '' !== $site_url && '' !== $ip && Whitelist::where( + [ + 'site_url' => $site_url, + 'ip' => $ip, + ] + ); + + if ( $stored ) { + continue; } + + EE::error( + sprintf( + 'Please check your list do not have any empty or wrong IP addresses. Invalid entry: %s. Use an IPv4 or IPv6 address, optionally with a CIDR prefix of 0-32 (IPv4) or 0-128 (IPv6), e.g. 192.0.2.0/24 or 2001:db8::/32.', + '' === $ip ? 'an empty one' : "'$ip'" + ) + ); } return $user_ips; @@ -548,7 +564,7 @@ public function delete( $args, $assoc_args ) { $whitelist->delete(); } } else { - $user_ips = $this->clean_and_validate_ips( $ip ); + $user_ips = $this->clean_and_validate_ips( $ip, $site_url ); foreach ( $user_ips as $ip ) { $existing_ips = Whitelist::where( diff --git a/src/auth-utils.php b/src/auth-utils.php index 4ed908f..d88d2c4 100644 --- a/src/auth-utils.php +++ b/src/auth-utils.php @@ -364,25 +364,77 @@ function htpasswd_command( string $flags, string $name, string $username, string ); } +/** + * Checks that a whitelist entry is an IPv4 or IPv6 address, optionally with a CIDR prefix, that nginx's `allow` accepts. + * + * @param string $entry Whitelist entry. + * + * @return bool + */ +function is_valid_whitelist_ip( string $entry ): bool { + + $parts = explode( '/', $entry ); + + if ( count( $parts ) > 2 ) { + return false; + } + + $is_ipv4 = false !== filter_var( $parts[0], FILTER_VALIDATE_IP, FILTER_FLAG_IPV4 ); + + if ( ! $is_ipv4 && false === filter_var( $parts[0], FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 ) ) { + return false; + } + + // nginx reads 255.255.255.255 as its INADDR_NONE error value, also when embedded in an IPv6 address. + if ( preg_match( '/(^|:)255\.255\.255\.255\z/', $parts[0] ) ) { + return false; + } + + if ( ! isset( $parts[1] ) ) { + return true; + } + + // Digits only, without leading zeros, so no sign, space or `;` reaches the nginx config. + return 1 === preg_match( '/^(0|[1-9][0-9]{0,2})\z/', $parts[1] ) && (int) $parts[1] <= ( $is_ipv4 ? 32 : 128 ); +} + /** * Gets the IPs to whitelist on a site: global and site entries, or none when the site has no own entries. * + * Invalid entries stored by older versions are skipped with a warning, as nginx would reject the whole file. + * * @param string $site_url URL of site, `default` for global. * * @return array */ function get_site_whitelist_ips( string $site_url ): array { + static $warned = []; + $site_ips = Whitelist::where( 'site_url', $site_url ); if ( empty( $site_ips ) ) { return []; } - return array_column( - 'default' === $site_url ? $site_ips : array_merge( Whitelist::get_global_ips(), $site_ips ), - 'ip' - ); + $ips = []; + foreach ( 'default' === $site_url ? $site_ips : array_merge( Whitelist::get_global_ips(), $site_ips ) as $row ) { + if ( is_valid_whitelist_ip( (string) $row->ip ) ) { + $ips[] = $row->ip; + continue; + } + + $scope = 'default' === $row->site_url ? 'global' : $row->site_url; + // Global entries are merged into every site's file: warn once. + if ( empty( $warned[ $scope ][ $row->ip ] ) ) { + $warned[ $scope ][ $row->ip ] = true; + // `--ip` splits on whitespace and commas, so such an entry can only go with the whole list. + $hint = preg_match( '/[\s,]/', $row->ip ) ? "`ee auth delete $scope --ip` and add the valid ones again" : sprintf( '`ee auth delete %s --ip=%s`', $scope, escapeshellarg( $row->ip ) ); + EE::warning( sprintf( "Skipping the invalid whitelist entry '%s' of %s: nginx would reject it. Remove it with %s.", $row->ip, $scope, $hint ) ); + } + } + + return $ips; } /** From d60240106b4f723947148caee47a10b8f5b982e3 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 12:08:51 +0530 Subject: [PATCH 2/5] fix(migration): drop invalid whitelist entries from existing _acl files Hosts upgraded from a version that stored unchecked entries may have an `_acl` file nginx rejects, which makes the proxy fail when the upgrade recreates it. The migration removes only those `allow` lines from the existing files, in place, so no new file is written while an older nginx-proxy runs, and warns with the removed entries. The rows stay in the database. --- ...valid_whitelist_entries_from_acl_files.php | 72 +++++++++++++------ 1 file changed, 49 insertions(+), 23 deletions(-) diff --git a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php index af2fdaf..055d506 100644 --- a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php +++ b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php @@ -4,65 +4,91 @@ use EE; use EE\Migration\Base; -use EE\Model\Site; -use EE\Model\Whitelist; -use function EE\Auth\Utils\generate_site_whitelist; use function EE\Auth\Utils\is_valid_whitelist_ip; class DropInvalidWhitelistEntriesFromAclFiles extends Base { - private $site_urls = []; + /** + * @var array ACL files with an entry nginx rejects, mapped to those entries. + */ + private $files = []; public function __construct() { parent::__construct(); - $rows = $this->is_first_execution ? [] : Whitelist::all(); - $invalid = array_filter( - $rows, - function ( $row ) { - return ! is_valid_whitelist_ip( (string) $row->ip ); - } - ); - - if ( empty( $invalid ) ) { + if ( $this->is_first_execution ) { $this->skip_this_migration = true; return; } - $scopes = array_unique( array_column( $invalid, 'site_url' ) ); - // Global entries are merged into every site's file. - $this->site_urls = in_array( 'default', $scopes, true ) ? array_unique( array_column( $rows, 'site_url' ) ) : $scopes; + foreach ( glob( EE_ROOT_DIR . '/services/nginx-proxy/vhost.d/*_acl' ) ?: [] as $file ) { + $invalid = array_filter( + self::allowed_entries( (string) file_get_contents( $file ) ), + function ( $entry ) { + return ! is_valid_whitelist_ip( $entry ); + } + ); + if ( $invalid ) { + $this->files[ $file ] = $invalid; + } + } + + $this->skip_this_migration = empty( $this->files ); } /** - * Rewrites the `_acl` files that carry an invalid entry, which older versions stored unchecked: nginx rejects such a file, so proxy reloads and restarts fail. + * Removes the entries nginx rejects from the `_acl` files: older versions wrote them unchecked, and such a file fails every proxy reload and restart. + * + * The files are edited in place instead of regenerated, so no new file is written while an older nginx-proxy runs. * * @throws EE\ExitException */ public function up() { if ( $this->skip_this_migration ) { - EE::debug( 'Skipping whitelist entries check as all entries are valid.' ); + EE::debug( 'Skipping the whitelist entries check: every _acl file is valid.' ); return; } - foreach ( $this->site_urls as $site_url ) { + foreach ( $this->files as $file => $invalid ) { + $lines = array_filter( + explode( "\n", (string) file_get_contents( $file ) ), + function ( $line ) { + return ! preg_match( '/^allow (.*);\r?$/', $line, $m ) || is_valid_whitelist_ip( $m[1] ); + } + ); + try { - generate_site_whitelist( $site_url, 'default' === $site_url ? null : ( Site::find( $site_url ) ?: null ) ); - } catch ( \Throwable $e ) { - EE::warning( sprintf( 'Could not rewrite the whitelist files of %s: %s', $site_url, $e->getMessage() ) ); + $this->fs->dumpFile( $file, implode( "\n", $lines ) ); + } catch ( \Exception $e ) { + EE::warning( sprintf( 'Could not rewrite %s: %s', $file, $e->getMessage() ) ); + continue; } + + EE::warning( sprintf( "Removed the invalid whitelist entries '%s' from %s, as nginx rejects them. They are still stored: delete them with `ee auth delete --ip=`.", implode( "', '", $invalid ), $file ) ); } \EE\Site\Utils\reload_global_nginx_proxy(); } /** - * Not reverted: the rewritten files only lack entries nginx rejects. + * Not reverted: the removed entries made the file invalid. */ public function down() { } + + /** + * @param string $content ACL file content. + * + * @return array Entries of its `allow` lines. + */ + private static function allowed_entries( string $content ): array { + + preg_match_all( '/^allow (.*);\r?$/m', $content, $m ); + + return $m[1]; + } } From 93a1b46ad1f44a3c63206e9dbb82b026382ae825 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 12:39:52 +0530 Subject: [PATCH 3/5] fix(auth): reject IPv6 entries nginx refuses and keep working stored entries - An IPv6 address of seven groups and a trailing `::` (e.g. `1:2:3:4:5:6:7::`) passed `filter_var`, but nginx's `allow` rejects it, so it still broke `nginx -t` for every site. It is now refused on input, skipped when stored, and removed by the migration. - Stored entries that nginx accepts, with a prefix leading zero (`10.0.0.0/08`) or surrounding whitespace such as a CR, are normalized instead of dropped, so the upgrade no longer revokes a working whitelist. - The migration stops the upgrade when an `_acl` file can't be rewritten, instead of recording itself and letting the image migration recreate the proxy on the invalid file. - The migration reads each file once, warns once per set of removed entries, and names the delete command of each stored entry, global ones included, with the same warning as the `_acl` generation. --- ...valid_whitelist_entries_from_acl_files.php | 80 ++++++++++--------- src/auth-utils.php | 62 ++++++++++---- 2 files changed, 91 insertions(+), 51 deletions(-) diff --git a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php index 055d506..cd6678d 100644 --- a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php +++ b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php @@ -4,12 +4,15 @@ use EE; use EE\Migration\Base; +use EE\Model\Whitelist; use function EE\Auth\Utils\is_valid_whitelist_ip; +use function EE\Auth\Utils\normalize_stored_whitelist_ip; +use function EE\Auth\Utils\warn_invalid_stored_whitelist_ip; class DropInvalidWhitelistEntriesFromAclFiles extends Base { /** - * @var array ACL files with an entry nginx rejects, mapped to those entries. + * @var array ACL files to fix, mapped to their new content and the entries removed from them. */ private $files = []; @@ -24,14 +27,28 @@ public function __construct() { } foreach ( glob( EE_ROOT_DIR . '/services/nginx-proxy/vhost.d/*_acl' ) ?: [] as $file ) { - $invalid = array_filter( - self::allowed_entries( (string) file_get_contents( $file ) ), - function ( $entry ) { - return ! is_valid_whitelist_ip( $entry ); + $content = file_get_contents( $file ); + if ( false === $content ) { + continue; + } + $removed = []; + $lines = []; + foreach ( explode( "\n", $content ) as $line ) { + if ( ! preg_match( '/^allow (.*);(\r?)$/', $line, $m ) || is_valid_whitelist_ip( $m[1] ) ) { + $lines[] = $line; + continue; + } + // Entries nginx accepts, e.g. with a prefix leading zero, are kept in a valid form. + $entry = normalize_stored_whitelist_ip( $m[1] ); + if ( is_valid_whitelist_ip( $entry ) ) { + $lines[] = "allow $entry;$m[2]"; + } else { + $removed[] = $m[1]; } - ); - if ( $invalid ) { - $this->files[ $file ] = $invalid; + } + $new_content = implode( "\n", $lines ); + if ( $new_content !== $content ) { + $this->files[ $file ] = [ $new_content, $removed ]; } } @@ -39,11 +56,11 @@ function ( $entry ) { } /** - * Removes the entries nginx rejects from the `_acl` files: older versions wrote them unchecked, and such a file fails every proxy reload and restart. + * Removes the invalid entries from the `_acl` files: older versions wrote them unchecked, and nginx fails every reload and restart on most of them. * * The files are edited in place instead of regenerated, so no new file is written while an older nginx-proxy runs. * - * @throws EE\ExitException + * @throws \Exception When a file can't be rewritten, so the upgrade stops before the image migration recreates the proxy on it. */ public function up() { @@ -53,22 +70,25 @@ public function up() { return; } - foreach ( $this->files as $file => $invalid ) { - $lines = array_filter( - explode( "\n", (string) file_get_contents( $file ) ), - function ( $line ) { - return ! preg_match( '/^allow (.*);\r?$/', $line, $m ) || is_valid_whitelist_ip( $m[1] ); - } - ); - - try { - $this->fs->dumpFile( $file, implode( "\n", $lines ) ); - } catch ( \Exception $e ) { - EE::warning( sprintf( 'Could not rewrite %s: %s', $file, $e->getMessage() ) ); - continue; + $removed = []; + foreach ( $this->files as $file => list( $content, $entries ) ) { + $this->fs->dumpFile( $file, $content ); + if ( $entries ) { + $removed[ implode( "', '", $entries ) ][] = basename( $file ); + } else { + EE::debug( "Normalized the whitelist entries of $file" ); } + } + + foreach ( $removed as $entries => $files ) { + EE::warning( sprintf( "Removed the invalid whitelist entries '%s' from %s in %s.", $entries, implode( ', ', $files ), EE_ROOT_DIR . '/services/nginx-proxy/vhost.d' ) ); + } - EE::warning( sprintf( "Removed the invalid whitelist entries '%s' from %s, as nginx rejects them. They are still stored: delete them with `ee auth delete --ip=`.", implode( "', '", $invalid ), $file ) ); + // The rows stay stored; name the command that removes each one. + foreach ( Whitelist::all() as $row ) { + if ( ! is_valid_whitelist_ip( normalize_stored_whitelist_ip( (string) $row->ip ) ) ) { + warn_invalid_stored_whitelist_ip( $row->site_url, $row->ip ); + } } \EE\Site\Utils\reload_global_nginx_proxy(); @@ -79,16 +99,4 @@ function ( $line ) { */ public function down() { } - - /** - * @param string $content ACL file content. - * - * @return array Entries of its `allow` lines. - */ - private static function allowed_entries( string $content ): array { - - preg_match_all( '/^allow (.*);\r?$/m', $content, $m ); - - return $m[1]; - } } diff --git a/src/auth-utils.php b/src/auth-utils.php index d88d2c4..7bca116 100644 --- a/src/auth-utils.php +++ b/src/auth-utils.php @@ -390,14 +390,54 @@ function is_valid_whitelist_ip( string $entry ): bool { return false; } + // nginx rejects a `::` that stands for no group, e.g. `1:2:3:4:5:6:7::`. + if ( preg_match( '/^(?:[0-9a-f]{1,4}:){7}:\z/i', $parts[0] ) ) { + return false; + } + if ( ! isset( $parts[1] ) ) { return true; } - // Digits only, without leading zeros, so no sign, space or `;` reaches the nginx config. + // Digits only, so no sign, space or `;` reaches the nginx config; leading zeros are refused as ambiguous. return 1 === preg_match( '/^(0|[1-9][0-9]{0,2})\z/', $parts[1] ) && (int) $parts[1] <= ( $is_ipv4 ? 32 : 128 ); } +/** + * Normalizes a whitelist entry stored by an older version that nginx accepts as is, e.g. `10.0.0.0/08` or a trailing CR. + * + * @param string $entry Stored whitelist entry. + * + * @return string The entry without surrounding whitespace and prefix leading zeros, or unchanged when it's still invalid. + */ +function normalize_stored_whitelist_ip( string $entry ): string { + + $normalized = preg_replace( '~/0+(?=[0-9])~', '/', trim( $entry ) ); + + return is_valid_whitelist_ip( $normalized ) ? $normalized : $entry; +} + +/** + * Warns once per scope and entry about an invalid stored whitelist entry, with the command that removes it. + * + * @param string $site_url Site URL of the entry, `default` for global. + * @param string $ip Whitelist entry. + */ +function warn_invalid_stored_whitelist_ip( string $site_url, string $ip ) { + + static $warned = []; + + $scope = 'default' === $site_url ? 'global' : $site_url; + if ( ! empty( $warned[ $scope ][ $ip ] ) ) { + return; + } + $warned[ $scope ][ $ip ] = true; + + // `--ip` splits on whitespace and commas, so such an entry can only go with the whole list. + $hint = preg_match( '/[\s,]/', $ip ) ? "`ee auth delete $scope --ip` and add the valid ones again" : sprintf( '`ee auth delete %s --ip=%s`', $scope, escapeshellarg( $ip ) ); + EE::warning( sprintf( "Skipping the invalid whitelist entry '%s' of %s: nginx would reject it. Remove it with %s.", $ip, $scope, $hint ) ); +} + /** * Gets the IPs to whitelist on a site: global and site entries, or none when the site has no own entries. * @@ -409,8 +449,6 @@ function is_valid_whitelist_ip( string $entry ): bool { */ function get_site_whitelist_ips( string $site_url ): array { - static $warned = []; - $site_ips = Whitelist::where( 'site_url', $site_url ); if ( empty( $site_ips ) ) { @@ -419,18 +457,12 @@ function get_site_whitelist_ips( string $site_url ): array { $ips = []; foreach ( 'default' === $site_url ? $site_ips : array_merge( Whitelist::get_global_ips(), $site_ips ) as $row ) { - if ( is_valid_whitelist_ip( (string) $row->ip ) ) { - $ips[] = $row->ip; - continue; - } - - $scope = 'default' === $row->site_url ? 'global' : $row->site_url; - // Global entries are merged into every site's file: warn once. - if ( empty( $warned[ $scope ][ $row->ip ] ) ) { - $warned[ $scope ][ $row->ip ] = true; - // `--ip` splits on whitespace and commas, so such an entry can only go with the whole list. - $hint = preg_match( '/[\s,]/', $row->ip ) ? "`ee auth delete $scope --ip` and add the valid ones again" : sprintf( '`ee auth delete %s --ip=%s`', $scope, escapeshellarg( $row->ip ) ); - EE::warning( sprintf( "Skipping the invalid whitelist entry '%s' of %s: nginx would reject it. Remove it with %s.", $row->ip, $scope, $hint ) ); + $ip = normalize_stored_whitelist_ip( (string) $row->ip ); + if ( is_valid_whitelist_ip( $ip ) ) { + $ips[] = $ip; + } else { + // Global entries are merged into every site's file, so this warns once. + warn_invalid_stored_whitelist_ip( $row->site_url, $row->ip ); } } From b68d0ad1e243b3cd8e431ce1327eda5200827692 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 12:57:04 +0530 Subject: [PATCH 4/5] fix(auth): trim only nginx's whitespace from stored entries and report removals after a failed write - Stored entries are trimmed of spaces, tabs, CR and LF only: nginx doesn't separate on a vertical tab or NUL, so an entry with one never took effect and must stay invalid. - The migration's removal warnings are printed even when a later file can't be rewritten, since a retry no longer sees the files already fixed. --- ...valid_whitelist_entries_from_acl_files.php | 23 +++++++++++-------- src/auth-utils.php | 3 ++- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php index cd6678d..71bc65e 100644 --- a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php +++ b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php @@ -71,17 +71,20 @@ public function up() { } $removed = []; - foreach ( $this->files as $file => list( $content, $entries ) ) { - $this->fs->dumpFile( $file, $content ); - if ( $entries ) { - $removed[ implode( "', '", $entries ) ][] = basename( $file ); - } else { - EE::debug( "Normalized the whitelist entries of $file" ); + try { + foreach ( $this->files as $file => list( $content, $entries ) ) { + $this->fs->dumpFile( $file, $content ); + if ( $entries ) { + $removed[ implode( "', '", $entries ) ][] = basename( $file ); + } else { + EE::debug( "Normalized the whitelist entries of $file" ); + } + } + } finally { + // Also after a failed write: a retry no longer sees the files already fixed. + foreach ( $removed as $entries => $files ) { + EE::warning( sprintf( "Removed the invalid whitelist entries '%s' from %s in %s.", $entries, implode( ', ', $files ), EE_ROOT_DIR . '/services/nginx-proxy/vhost.d' ) ); } - } - - foreach ( $removed as $entries => $files ) { - EE::warning( sprintf( "Removed the invalid whitelist entries '%s' from %s in %s.", $entries, implode( ', ', $files ), EE_ROOT_DIR . '/services/nginx-proxy/vhost.d' ) ); } // The rows stay stored; name the command that removes each one. diff --git a/src/auth-utils.php b/src/auth-utils.php index 7bca116..8d22d0a 100644 --- a/src/auth-utils.php +++ b/src/auth-utils.php @@ -412,7 +412,8 @@ function is_valid_whitelist_ip( string $entry ): bool { */ function normalize_stored_whitelist_ip( string $entry ): string { - $normalized = preg_replace( '~/0+(?=[0-9])~', '/', trim( $entry ) ); + // Only the whitespace nginx itself separates on. + $normalized = preg_replace( '~/0+(?=[0-9])~', '/', trim( $entry, " \t\r\n" ) ); return is_valid_whitelist_ip( $normalized ) ? $normalized : $entry; } From cca2b2bd567c720a750c6dcf1d685cb07a2aa58c Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 15:34:40 +0530 Subject: [PATCH 5/5] fix(migration): keep nginx's other allow arguments and stop on an unreadable ACL file `allow all;` and `allow unix:;` are valid nginx, so a hand-edited file keeps them. A file that can't be read may still hold an invalid entry, so the migration now fails after fixing the others and stays pending, like a failed write. --- ...invalid_whitelist_entries_from_acl_files.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php index 71bc65e..358d090 100644 --- a/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php +++ b/migrations/container/20260927120000_auth-command_drop_invalid_whitelist_entries_from_acl_files.php @@ -16,6 +16,11 @@ class DropInvalidWhitelistEntriesFromAclFiles extends Base { */ private $files = []; + /** + * @var array ACL files that couldn't be read. + */ + private $unreadable = []; + public function __construct() { parent::__construct(); @@ -29,12 +34,15 @@ public function __construct() { foreach ( glob( EE_ROOT_DIR . '/services/nginx-proxy/vhost.d/*_acl' ) ?: [] as $file ) { $content = file_get_contents( $file ); if ( false === $content ) { + // It may hold an invalid entry, so up() fails and the migration stays pending. + $this->unreadable[] = $file; continue; } $removed = []; $lines = []; foreach ( explode( "\n", $content ) as $line ) { - if ( ! preg_match( '/^allow (.*);(\r?)$/', $line, $m ) || is_valid_whitelist_ip( $m[1] ) ) { + // nginx also accepts `all` and `unix:`, which EE doesn't write but a hand edit may add. + if ( ! preg_match( '/^allow (.*);(\r?)$/', $line, $m ) || is_valid_whitelist_ip( $m[1] ) || in_array( $m[1], [ 'all', 'unix:' ], true ) ) { $lines[] = $line; continue; } @@ -52,7 +60,7 @@ public function __construct() { } } - $this->skip_this_migration = empty( $this->files ); + $this->skip_this_migration = empty( $this->files ) && empty( $this->unreadable ); } /** @@ -60,7 +68,7 @@ public function __construct() { * * The files are edited in place instead of regenerated, so no new file is written while an older nginx-proxy runs. * - * @throws \Exception When a file can't be rewritten, so the upgrade stops before the image migration recreates the proxy on it. + * @throws \Exception When a file can't be read or rewritten, so the upgrade stops before the image migration recreates the proxy on it. */ public function up() { @@ -80,6 +88,9 @@ public function up() { EE::debug( "Normalized the whitelist entries of $file" ); } } + if ( $this->unreadable ) { + throw new \Exception( 'Could not read ' . implode( ', ', $this->unreadable ) ); + } } finally { // Also after a failed write: a retry no longer sees the files already fixed. foreach ( $removed as $entries => $files ) {