Recursive WP_Filesystem_Direct::chgrp() and ::chown() fail to descend into subdirectories

[westonruter]

WP_Filesystem_Direct::chgrp() and WP_Filesystem_Direct::chown() are broken when called recursively on a directory: they never descend into the directory's contents, so no descendant's group or owner is actually changed, and PHP emits an "Array to string conversion" warning for each entry. Both methods still return true, reporting a false success.

Cause

WP_Filesystem_Base::dirlist() returns array<string, FileListing> — an array keyed by filename whose values are the per-file info arrays. The recursive branch of chgrp()/chown() iterates only the values:

<?php
foreach ( $filelist as $filename ) {
        $this->chgrp( $file . $filename, $group, $recursive );
}

Here $filename is bound to the FileListing array (the value), not the filename (the key). Concatenating $file . $filename therefore coerces the array to the string "Array", producing a non-existent path such as /path/to/dir/Array. Every recursive call then bails at the exists() check, so nothing is changed.

Comparison with chmod()

WP_Filesystem_Direct::chmod() is not affected because it iterates keys and values and uses the key as the filename:

<?php
foreach ( (array) $filelist as $filename => $filemeta ) {
        $this->chmod( $file . $filename, $mode, $recursive );
}

This is the same fix that was applied to chmod() in WordPress 3.0 — see [12997] for #11261 — but it was never ported to chgrp() and chown().

Fix

Iterate the listing and build the path from the name element:

<?php
foreach ( $filelist as $file_listing ) {
        $this->chgrp( $file . $file_listing['name'], $group, $recursive );
}

WP_Filesystem_Direct::chown() has the identical defect and the same fix: use the listing entry's name when building each child path.

👉 This shows the great value of implementing strict PHPStan rule level adherence: bugs are identified. See also #64898.

This was fixed incidentally in ​PR #11593 (Filesystem type-hardening, #65409), where static analysis surfaced the array-to-string concatenation. This ticket tracks the underlying bug on its own so it can be evaluated and backported independently.

Since this issue hasn't been reported in 16 years since it was first fixed for chmod(), it may be that the chgrp() and chown() methods simply are not used in practice.

Claude Opus 4.8 was used in the creation of this ticket.

[westonruter]

In 62636:

Filesystem: Fix subdirectory recursion in WP_Filesystem_Direct::chgrp() and ::chown().

This applies the equivalent fix which had previously been done to ::chmod(). Tests are added for all three methods.

Developed in subset of ​https://github.com/WordPress/wordpress-develop/pull/11593.
Follow-up to r12997.

Fixes #65584.
See #65409, #11261.

[westonruter]

In 62637:

Filesystem API: Improve type safety across the transport classes.

Change the optional constructor argument of WP_Filesystem_FTPext, WP_Filesystem_ftpsockets, and WP_Filesystem_SSH2 from an empty string default to an empty array, matching how the argument is actually consumed, and improve the associated DocBlocks.

These classes were also brought to adherence with PHPStan rule level 10:

• Add FileListing and Options array shapes, and initialize each transport's $options to a complete default array before any early return.
• Correct several inaccurate @return descriptions, including the group() methods that had been describing the owner.
• Allow WP_Filesystem_SSH2::connect() to be retried after a failed connection attempt.
• Stop WP_Filesystem_FTPext::parselisting() from leaking its intermediate date-parsing keys into the returned listing.
• Add ext-ftp and ext-ssh2 to the suggested extensions in composer.json.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/11593.
Follow-up to r62635, r62636.

Props soean, westonruter, mukesh27.
See #65584, #64898.
Fixes #65409.