From 6dac821e862c8fab96f0db8f1b710eaf7e3cbde3 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 26 Oct 2026 09:54:25 +0530 Subject: [PATCH 1/4] fix(migrations): read and write custom.ini through checked handles in the upload progress migration The site's php container can swap custom.ini, conf.d or php/ in its config volume for a symlink, and the migration followed it: it read, rewrote, created or (on rollback) deleted custom.ini wherever the link pointed. It now uses site-command's checked helpers (get_container_file_contents() and put_container_file_contents()), and with an older site-command that lacks them it checks that custom.ini is a regular file and that no directory above it is a symlink. A missing custom.ini is no longer created, only warned about, as the site-command re-check migration does. --- ...hp_disable_php_session_upload_progress.php | 175 +++++++++++------- 1 file changed, 112 insertions(+), 63 deletions(-) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 24442ac..0e93609 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -14,6 +14,9 @@ class DisablePhpSessionUploadProgress extends Base { const SITE_TYPE = 'php'; + /** Largest custom.ini read. */ + const MAX_SIZE = 1048576; + const SETTING_LINES = [ '; Session upload progress is off for security; set it to On only if your app needs it.', 'session.upload_progress.enabled = Off', @@ -22,7 +25,7 @@ class DisablePhpSessionUploadProgress extends Base { /** @var Site[] */ private $sites = []; - /** @var array site_url => [ 'site' => Site, 'file' => string, 'backup' => string|null ] for every custom.ini changed by up(). */ + /** @var array site_url => [ 'site' => Site, 'file' => string, 'dir' => string, 'backup' => string ] for every custom.ini changed by up(). */ private $changed = []; /** @var array site_url => php container ID seen by up(), for running sites. */ @@ -225,26 +228,25 @@ private static function global_part( string $content ) { */ private function update_site( $site ) { - $conf_dir = $site->site_fs_path . '/config/php/php/conf.d'; - $file = $conf_dir . '/custom.ini'; - $manual = "Add 'session.upload_progress.enabled = Off' to $file and run: ee site reload $site->site_url"; + $file = $site->site_fs_path . '/config/php/php/conf.d/custom.ini'; + $manual = "Add 'session.upload_progress.enabled = Off' to $file and run: ee site reload $site->site_url"; - if ( is_link( $file ) || ( file_exists( $file ) && ! is_file( $file ) ) ) { - EE::warning( "Skipped disabling session.upload_progress for $site->site_url: custom.ini is not a regular file. $manual" ); + // Not created when missing: PHP can't create a file safely in a directory the site's php container can swap. + if ( ! file_exists( $file ) && ! is_link( $file ) ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: custom.ini is missing. $manual" ); return; } - $created = ! file_exists( $file ); - if ( $created && ! is_dir( $conf_dir ) ) { - EE::warning( "Skipped disabling session.upload_progress for $site->site_url: $conf_dir is missing. $manual" ); - - return; - } - - $content = $created ? '' : @file_get_contents( $file ); - if ( false === $content ) { - EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not read custom.ini. $manual" ); + try { + $base = realpath( $site->site_fs_path . '/config/php' ); + if ( false === $base ) { + throw new \Exception( 'could not resolve config/php' ); + } + $conf_dir = $base . '/php/conf.d'; + $content = self::read_custom_ini( $file, $conf_dir ); + } catch ( \Exception $e ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: {$e->getMessage()}. $manual" ); return; } @@ -262,39 +264,38 @@ private function update_site( $site ) { return; } - $backup = null; - if ( ! $created ) { - $backup = $this->backup_root . '/' . $site->site_url . '/custom.ini'; - try { - $this->fs->mkdir( dirname( $backup ), 0700 ); - $this->fs->copy( $file, $backup, true ); - $this->fs->chmod( $backup, fileperms( $file ) & 0777 ); - $this->fs->touch( $backup, filemtime( $file ) ); - } catch ( \Throwable $e ) { - EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not back up custom.ini ({$e->getMessage()}). $manual" ); - - return; + $backup = $this->backup_root . '/' . $site->site_url . '/custom.ini'; + try { + $stat = @lstat( $file ); + $this->fs->mkdir( dirname( $backup ), 0700 ); + $this->fs->dumpFile( $backup, $content ); + if ( $stat ) { + $this->fs->chmod( $backup, $stat['mode'] & 0777 ); + $this->fs->touch( $backup, $stat['mtime'] ); } + } catch ( \Throwable $e ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not back up custom.ini ({$e->getMessage()}). $manual" ); + + return; + } + + // In place, so owner, mode and inode stay (macOS bind-mounts this single file); a failed write puts the old content back itself. + try { + self::write_custom_ini( $file, $conf_dir, $new_content, $content ); + } catch ( \Exception $e ) { + $this->fs->remove( dirname( $backup ) ); + EE::warning( "Could not disable session.upload_progress for $site->site_url: {$e->getMessage()}. $manual" ); + + return; } $this->changed[ $site->site_url ] = [ 'site' => $site, 'file' => $file, + 'dir' => $conf_dir, 'backup' => $backup, ]; - // Written in place: keeps owner, mode and inode (macOS bind-mounts this single file). - if ( ! $this->write_in_place( $file, $new_content ) ) { - $this->restore_file( $site->site_url ); - EE::warning( "Could not disable session.upload_progress for $site->site_url: could not write custom.ini. $manual" ); - - return; - } - - if ( $created ) { - $this->match_owner( $file, $conf_dir ); - } - $id = $site->site_enabled ? $this->get_php_container( $site ) : ''; if ( null !== $id && '' !== $id ) { $this->containers[ $site->site_url ] = $id; @@ -327,50 +328,98 @@ private function restore_file( string $site_url ) { $entry = $this->changed[ $site_url ]; unset( $this->changed[ $site_url ] ); - if ( null === $entry['backup'] ) { - if ( ! @unlink( $entry['file'] ) && file_exists( $entry['file'] ) ) { - EE::warning( "Could not remove {$entry['file']} added by the session.upload_progress migration." ); + try { + $content = @file_get_contents( $entry['backup'] ); + if ( false === $content ) { + throw new \Exception( 'could not read the backup' ); } + self::write_custom_ini( $entry['file'], $entry['dir'], $content ); + } catch ( \Exception $e ) { + EE::warning( "Could not restore {$entry['file']} ({$e->getMessage()}); its original is {$entry['backup']}" ); return; } + $this->fs->remove( dirname( $entry['backup'] ) ); + EE::debug( "$site_url: restored custom.ini." ); + } - $content = @file_get_contents( $entry['backup'] ); - // Skip the write when it never happened, e.g. on an immutable file. - if ( false === $content || ( @file_get_contents( $entry['file'] ) !== $content && ! $this->write_in_place( $entry['file'], $content ) ) ) { - EE::warning( "Could not restore {$entry['file']}; its original is {$entry['backup']}" ); + /** + * Read custom.ini, which the site's php container can swap for a symlink, as can every directory above it in its config volume. + * + * site-command's checked handle refuses anything but the regular file in $dir; an older site-command lacks it, so only the path is checked then. + * + * @param string $file custom.ini. + * @param string $dir Real path of its directory. + * + * @return string + * @throws \Exception + */ + private static function read_custom_ini( string $file, string $dir ) { - return; + if ( function_exists( '\EE\Site\Utils\get_container_file_contents' ) ) { + return \EE\Site\Utils\get_container_file_contents( $file, $dir, self::MAX_SIZE ); } - $this->fs->remove( dirname( $entry['backup'] ) ); - EE::debug( "$site_url: restored custom.ini." ); + + self::check_custom_ini_path( $file, $dir ); + $content = @file_get_contents( $file, false, null, 0, self::MAX_SIZE + 1 ); + if ( false === $content || strlen( $content ) > self::MAX_SIZE ) { + throw new \Exception( 'could not read custom.ini' ); + } + + return $content; } /** - * @param string $file File to overwrite without replacing it. - * @param string $content New content. + * Overwrite custom.ini in place, checked like read_custom_ini(). + * + * @param string $file custom.ini. + * @param string $dir Real path of its directory. + * @param string $content New content. + * @param string|null $expected Content it must still hold; null for any. * - * @return bool Whether the file now holds exactly $content. + * @throws \Exception When it isn't written; the old content is put back after a partial write. */ - private function write_in_place( string $file, string $content ) { + private static function write_custom_ini( string $file, string $dir, string $content, ?string $expected = null ) { + if ( function_exists( '\EE\Site\Utils\put_container_file_contents' ) ) { + \EE\Site\Utils\put_container_file_contents( $file, $dir, $content, $expected, self::MAX_SIZE ); + + return; + } + + $current = self::read_custom_ini( $file, $dir ); + if ( null !== $expected && $current !== $expected ) { + throw new \Exception( 'custom.ini changed while it was checked' ); + } + // Skip a write that isn't needed, e.g. on an immutable file. + if ( $current === $content ) { + return; + } + self::check_custom_ini_path( $file, $dir ); $written = @file_put_contents( $file, $content ); clearstatcache( true, $file ); - - return strlen( $content ) === $written && @file_get_contents( $file ) === $content; + if ( strlen( $content ) !== $written || @file_get_contents( $file ) !== $content ) { + @file_put_contents( $file, $current ); + throw new \Exception( 'could not write custom.ini' ); + } } /** - * Give a new file the owner and group of its directory. + * @param string $file custom.ini. + * @param string $dir Real path of its directory. * - * @param string $file File. - * @param string $dir Directory. + * @throws \Exception When it isn't a regular file or a directory above it is a symlink. */ - private function match_owner( string $file, string $dir ) { + private static function check_custom_ini_path( string $file, string $dir ) { - @chmod( $file, 0644 ); - @chown( $file, fileowner( $dir ) ); - @chgrp( $file, filegroup( $dir ) ); + clearstatcache( true ); + $stat = @lstat( $file ); + if ( realpath( dirname( $file ) ) !== $dir ) { + throw new \Exception( 'a directory above custom.ini is a symlink' ); + } + if ( ! $stat || 0100000 !== ( $stat['mode'] & 0170000 ) ) { + throw new \Exception( 'custom.ini is not a regular file' ); + } } /** From 5c171e2ff8c6393fbabd98996ba2c38e33d41f69 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 26 Oct 2026 10:38:26 +0530 Subject: [PATCH 2/4] fix(migrations): keep the custom.ini backup when a write fails If putting the old content back failed too, the backup is the only copy, so it is kept and named in the warning; the fallback for an older site-command now reports a failed put-back. --- ..._site-type-php_disable_php_session_upload_progress.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 0e93609..7eecc0a 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -283,8 +283,8 @@ private function update_site( $site ) { try { self::write_custom_ini( $file, $conf_dir, $new_content, $content ); } catch ( \Exception $e ) { - $this->fs->remove( dirname( $backup ) ); - EE::warning( "Could not disable session.upload_progress for $site->site_url: {$e->getMessage()}. $manual" ); + // Kept: if putting the old content back failed too, it is the only copy. + EE::warning( "Could not disable session.upload_progress for $site->site_url: {$e->getMessage()}. The original custom.ini is $backup. $manual" ); return; } @@ -399,8 +399,8 @@ private static function write_custom_ini( string $file, string $dir, string $con $written = @file_put_contents( $file, $content ); clearstatcache( true, $file ); if ( strlen( $content ) !== $written || @file_get_contents( $file ) !== $content ) { - @file_put_contents( $file, $current ); - throw new \Exception( 'could not write custom.ini' ); + $restored = strlen( $current ) === @file_put_contents( $file, $current ); + throw new \Exception( 'could not write custom.ini' . ( $restored ? '' : ', nor put the old content back' ) ); } } From 10050fdbf1512dc3e16aa58d5b14d1046d98fc5c Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 26 Oct 2026 11:02:42 +0530 Subject: [PATCH 3/4] fix(migrations): skip a custom.ini that the setting would push over the size limit Rollback reads custom.ini back with the same limit, so content larger than it after the two lines are added is skipped with a warning instead of written. --- ...0_site-type-php_disable_php_session_upload_progress.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 7eecc0a..1584efa 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -264,6 +264,13 @@ private function update_site( $site ) { return; } + // Rollback reads the file back with the same limit. + if ( strlen( $new_content ) > self::MAX_SIZE ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: custom.ini would be larger than " . self::MAX_SIZE . " bytes. $manual" ); + + return; + } + $backup = $this->backup_root . '/' . $site->site_url . '/custom.ini'; try { $stat = @lstat( $file ); From 5b0ebb31b403bd0cd06766d29ab4d5b4528ec169 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 26 Oct 2026 11:20:07 +0530 Subject: [PATCH 4/4] fix(migrations): check the path again before putting custom.ini back With an older site-command, a failed write put the old content back by path without checking it again, so a symlink swapped in meanwhile was followed; the backup is kept and named in the warning instead. --- ..._site-type-php_disable_php_session_upload_progress.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 1584efa..782b813 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -406,7 +406,13 @@ private static function write_custom_ini( string $file, string $dir, string $con $written = @file_put_contents( $file, $content ); clearstatcache( true, $file ); if ( strlen( $content ) !== $written || @file_get_contents( $file ) !== $content ) { - $restored = strlen( $current ) === @file_put_contents( $file, $current ); + // Checked again: the path may have been swapped since the write. + try { + self::check_custom_ini_path( $file, $dir ); + $restored = strlen( $current ) === @file_put_contents( $file, $current ); + } catch ( \Exception $e ) { + $restored = false; + } throw new \Exception( 'could not write custom.ini' . ( $restored ? '' : ', nor put the old content back' ) ); } }