Opened 2 years ago
Last modified 2 years ago
#56713 new defect (bug)
Check ACL permission before upgrading
Reported by: | 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)
#3
@
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?
#5
@
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 addingu:www-data:rwx
to the (F)ACL. chmod()
can only be used by the file owner, orroot
.
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:
- wp-admin/includes/class-core-upgrader.php.
- wp-admin/includes/class-wp-filesystem-direct.php - Problem 1.
- wp-admin/includes/update-core.php - Problem 2.
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:
- We backport the
is_writable()
checks to older branches (not desirable), OR - 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.
I forgot to tell this is related to topic https://wordpress.org/support/topic/issue-updating-wordpress-using-acl-permissions/