WP_Upgrader::maintenance_mode() trusts an unconnected $wp_filesystem, causing a fatal TypeError during automatic updates

[hatterops]

WP_Upgrader::maintenance_mode() only initializes the filesystem if $wp_filesystem isn't already set:

if ( ! $wp_filesystem ) {
    WP_Filesystem();
}

This assumes that if the global is already an object, it must be properly connected. That assumption doesn't hold: if something earlier in the same request leaves a WP_Filesystem_FTPext instance in that global without connect() ever having been called on it (or after a connection that's since gone stale), maintenance_mode() uses it anyway. The next filesystem call that needs a live connection — in our case WP_Filesystem_Base::find_folder() → WP_Filesystem_FTPext::exists() — passes the unset $this->link straight into ftp_nlist(), which fatals under PHP 8's strict internal-function typing:

PHP Fatal error:  Uncaught TypeError: ftp_nlist(): Argument #1 ($ftp) must be of type FTP\Connection, null given in wp-admin/includes/class-wp-filesystem-ftpext.php:438
Stack trace:
#0 wp-admin/includes/class-wp-filesystem-ftpext.php(438): ftp_nlist()
#1 wp-admin/includes/class-wp-filesystem-base.php(231): WP_Filesystem_FTPext->exists()
#2 wp-admin/includes/class-wp-filesystem-base.php(58): WP_Filesystem_Base->find_folder()
#3 wp-admin/includes/class-wp-upgrader.php(1025): WP_Filesystem_Base->abspath()
#4 wp-admin/includes/class-wp-automatic-updater.php(474): WP_Upgrader->maintenance_mode()
#5 wp-admin/includes/class-wp-automatic-updater.php(676): WP_Automatic_Updater->update()
#6 wp-includes/update.php(890): WP_Automatic_Updater->run()
#7 (via wp_maybe_auto_update → wp_version_check, triggered under real WP-Cron)
  thrown in wp-admin/includes/class-wp-filesystem-ftpext.php on line 438

Because maintenance_mode() is called once per WP_Automatic_Updater::update() invocation — not per attempted file operation — this single uncaught fatal aborts the entire automatic-update run for every item scheduled in that cycle (core, every opted-in plugin, every opted-in theme), not just the one that happened to trigger it.

Confirmed NOT the cause (ruled out during investigation, worth noting so triage doesn't re-tread this): FTP credentials (verified valid via direct FTP login with the exact stored values), disable_functions, the ftp extension not being loaded, and DNS/IPv6 resolution ambiguity on the configured FTP host (php_network_connect_socket_to_host() in main/network.c already iterates and falls back across resolved addresses correctly).

A parallel, manually-constructed WP_Filesystem_FTPext object using the site's real credentials connects successfully every time it's tested — including one second before an observed real crash in the same request — confirming the defect is specifically about whether connect() gets called on the object that ends up being used, not whether a connection can succeed.

What's not yet isolated: the exact condition that leaves the earlier $wp_filesystem unconnected in the first place (it's intermittent, not deterministic). request_filesystem_credentials() (called from WP_Upgrader::fs_connect(), which does always call WP_Filesystem()) seems like the most likely place to look, but this hasn't been confirmed.

Suggested fix

maintenance_mode()'s check should verify the existing $wp_filesystem is actually usable, not just non-null — e.g. checking connection state where the concrete class exposes one, or simply always calling WP_Filesystem() here rather than conditionally skipping it (since a fresh connect is cheap relative to the cost of silently aborting the whole update run).

Workaround in use (mu-plugin, not a core patch):

add_action('wp_maybe_auto_update', function () {
    global $wp_filesystem;
    if ($wp_filesystem instanceof WP_Filesystem_FTPext) {
        $needs_reconnect = empty($wp_filesystem->link) || @ftp_pwd($wp_filesystem->link) === false;
        if ($needs_reconnect) {
            $wp_filesystem = null; // forces maintenance_mode()'s check to do a fresh, real connect
        }
    }
}, 1);

This has been running on the previously failing site and confirmed to allow automatic updates to complete successfully on a previously-reliably-crashing site.

maintenance_mode() guards initialization with ! $wp_filesystem, which treats any object in the global as usable. WP_Filesystem() assigns that global at file.php:2213 before connecting and returns false at :2226 on a constructor error, so a failed call earlier in the request leaves an unconnected object behind. maintenance_mode() then calls abspath() on it, and under PHP 8 the transport fatals on a null link. It is called once per WP_Automatic_Updater::update(), so the whole update run aborts rather than the one item.

This is the branch [60107] did not cover on the same function, and the check mirrors fs_connect() in this class.

Design decision worth flagging: the guard treats any recorded error as unusable. WP_Filesystem_Base::$errors is set in the constructor and never cleared, and WP_Filesystem_FTPext::connect() deliberately tolerates connect and auth codes so a failed attempt can be retried on the same instance. An instance that failed once and then connected successfully still carries those codes, so this guard will discard it and reinitialize. That costs a reconnect in a rare case, and I preferred it to inspecting transport-specific link properties from the caller. fs_connect() is stricter still and hard-fails on the same condition.

## Testing

Set FS_METHOD to ftpext, call WP_Filesystem() once with incomplete credentials, then run maintenance_mode( true ). Before the change that fatals in the transport; after it, the existing "Could not access filesystem." notice path from [60107] handles it.

Thanks for picking it up and submitting a patch. Wanted to add some diagnostic detail from the investigation.

the crashing $wp_filesystem object in the system had:

is_object( $wp_filesystem ) → true
$wp_filesystem->errors->has_errors() → false (zero recorded errors — confirmed via print_r( $ftp->errors->errors, true ) on a freshly-constructed instance using the exact same credentials, which came back empty)
$wp_filesystem->link → undefined (not false, not a resource — genuinely never set), confirmed via var_export( $wp_filesystem->link ?? 'no-link-property', true ) at the moment of the real crash

Ruled out bad/missing FTP credentials specifically (valid FTP_HOST/FTP_USER/FTP_PASS, clean manual FTP login, OK LOGIN in vsftpd's log immediately before the crash) before landing on this. So in the case, connect() genuinely appears to have never been called at all on the object sitting in the global — not called-and-failed (which would leave errors populated, or link as false), just never invoked.

Given that, the proposed guard:

if ( ! is_object( $wp_filesystem )
|| ( is_wp_error( $wp_filesystem->errors ) && $wp_filesystem->errors->has_errors() )
) { ... }

would evaluate ! true

false → false for the object I observed, so maintenance_mode() would still reuse it and still fatal the same way. This patch looks correct and worth having for the case it targets (a constructor that recorded a real error — bad config, missing extension, etc.), but it looks like a different failure mode from the one in the reported ticket.

The link reading and the errors reading came from different objects, and that is where this turns.

var_export( $wp_filesystem->link ?? ... ) was on the crashing global. print_r( $ftp->errors->errors ) was on a freshly constructed instance using the same credentials. A fresh instance built with complete credentials always has empty errors, so that reading cannot separate the two explanations.

What the code allows: file.php:2213 is the only line in core that constructs a transport or assigns $wp_filesystem, and a grep of src/ returns that single hit. Between it and connect() at :2230 there is one early return, :2226, which fires only when the constructor recorded errors. connect() itself also bails before assigning $this->link when errors are present and none of them are connect or auth.

So a null link means connect() never reached its assignment, and both routes to that require errors to be populated. The null link is evidence for the constructor-error path rather than against it.

Could you run print_r( $wp_filesystem->errors->errors, true ) on the global at the same point you captured link? Populated, and this patch covers your case. Genuinely empty, and something outside core put that object in the global, because core has no path that does. Then I would want to know what: a mu-plugin, a host drop-in. No caller-side guard helps there and it belongs at the transport layer, #48689 and #58541.

@jigneshbhavani It seems like you may be using AI for authoring this PR and comments. If so, please add the missing AI Disclosure section to the description to capture what tools/models you are using:

​https://github.com/WordPress/wordpress-develop/blob/75683ef645d2216f5731b571fd153343833be5bb/.github/pull_request_template.md?plain=1#L22-L33

See ​https://make.wordpress.org/ai/handbook/ai-guidelines/

Added to the description. It was missing here and should not have been, the same section is on all my other core PRs.