Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#56713 new defect (bug)

Check ACL permission before upgrading

Reported by: cartman34's profile Cartman34 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: reporter-feedback dev-feedback
Focuses: Cc:

Description

I am using ACL to define permissions of files and folder of my wordpress installation, but when I upgrade my wordpress installation using web ui tool, I am getting the following error:

Warning: chmod(): Operation not permitted in /home/my-website/wp-admin/includes/class-wp-filesystem-direct.php on line 173

Then the upgrade is stuck in an invalid state and I have to upgrade it manually.

Wordpress upgrade program should check all its abilities before trying to upgrade and it should handle the case of using ACL for permissions. All permissions are good, it's just using ACL instead of MOD.

The is the third upgrade I am experiencing this issue.

Change History (5)

#2 @desrosj
2 years ago

  • Component changed from General to Upgrade/Install

#3 @costdev
2 years ago

  • Keywords reporter-feedback added

Hi @Cartman34, thanks for opening this ticket!

Can you please clarify what "web ui tool" refers to? Is this through the WordPress Dashboard > Updates page?

If not, what happens when you visit that page and click the Update button?

Regarding your ACL, I assume that you used setfacl --recursive -m u:www-data:rwx website, is that correct?

#4 @costdev
2 years ago

#56510 was marked as a duplicate.

#5 @costdev
2 years ago

  • Keywords dev-feedback added

Notes

  • This is the chmod() call that leads to the first warning. There are others later in the trace.
  • In WP_Filesystem_Direct::chmod(), this is the line that actually triggers the first warning.
  • Site Health > Info > Filesystem Permissions shows all directories as writable.
  • Changing file ownership via chown -R www-data:www-data <directory> does resolve the issue, but defeats the purpose of adding u:www-data:rwx to the (F)ACL.
  • chmod() can only be used by the file owner, or root.

When Testing

For my tests, I used the following commands:

# Set the owner to root.
sudo chown -R root:root <directory>

# Give www-data permissions via (F)ACL.
setfacl -Rm u:www-data:rwx <directory>

When trying to update through Dashboard > Updates, Plugins > Installed Plugins > Update now, or Themes > Update now, the "Connection Information" dialog to enter FTP credentials may be displayed.

This is caused by a mismatch in file ownership vs temp file ownership in this condition.

define( 'FS_METHOD', 'direct' ); in wp-config.php resolves this.


Findings

This seems to be an issue with calling $wp_filesystem->chmod() when the file is not owned by www-data. The files are writable, but chmod() can only be used by the actual file owner.

Unfortunately, I don't believe we can rely on fileowner() or $wp_filesystem->owner() checks, as www-data is a default username and will be different based on environment or customizations.

Therefore, wrapping each of these calls to $wp_filesystem->chmod() with an ! $wp_filesystem->is_writable() check should make sure that chmod() is only run when the path is not writable. This is done in most other places in Core, but not all.

Wrapping various chmod() calls with if ( ! $wp_filesystem->is_writable() ) {} and defining FS_METHOD as direct seems to update just fine for me.

The chmod() calls I found to be relevant to this issue are:

Problem 1
This change may (read: will) have unintended consequences.

Problem 2
This file is loaded from the files of the WordPress version being installed, not the files of the current WordPress version.

This means any patch will not be fully functional unless:

  1. We backport the is_writable() checks to older branches (not desirable), OR
  2. Users with such a setup continue to manually install until upgrading to the version after the one that patches this issue (a minor hassle, but not asking anything extra of users). That is, if WordPress 6.2 patches this issue, the patch won't be fully functional until WordPress 6.3 is released.

Conclusion

This may be resolvable in Core, but needs feedback and discussion on the above before it can move forward.

Last edited 2 years ago by costdev (previous) (diff)
Note: See TracTickets for help on using tickets.