search

keycdn / cache-enabler

A lightweight caching plugin for WordPress that makes your website faster by generating static HTML files.
https://wordpress.org/plugins/cache-enabler/
123 stars 46 forks source link

Explicitly chmod the cache and settings directories #194

Closed stevegrunwell closed 3 years ago

stevegrunwell commented 3 years ago

The wp_mkdir_p() function will create new directories recursively based on the permissions of the parent directory; in the case of this plugin, wp-content/.

Unfortunately, that function doesn't expose a way to explicitly specify permissions, so a locked-down wp-content/ directory can result in the directories being created with permissions that don't allow the web server to serve the files.

This PR introduces a new Cache_Enabler_Disk::mkdir_p() method, which recursively creates the directories and ensures that the newly-created directory has 0755 permissions, meaning /cache, /cache/cache-enabler, /settings, and /settings/cache-enabler will all explicitly be given 0755.

Fixes #193.

coreykn commented 3 years ago

@stevegrunwell I have a quick question regarding the Cache_Enabler_Disk::mkdir_p() method that you created. After going over this again recently I believe I found an issue but wanted to get your feedback in case I'm misunderstanding something and am wrong.

I understand that this method assumes that the directory and its parent should have 0755 permissions and will attempt to update any existing directories. However, the following code would have it return true if the correct permissions aren't set, right?

https://github.com/keycdn/cache-enabler/blob/d01cf5de486c7ea62e995df19e4965ae8882809b/inc/cache_enabler_disk.class.php#L1003-L1013

For example, if the permissions were 0600 both if statements would be skipped and true would be returned. Are both of these if statements supposed to be !== instead of ===?

stevegrunwell commented 3 years ago

@coreykn Looking at it again, you're absolutely correct: those should be !==, not ===.

(Something something Monday details). I'll get an updated PR together.

coreykn commented 3 years ago

@stevegrunwell Looks like a small issue has arisen from this addition: https://wordpress.org/support/topic/error-on-1-7-0-update/

https://github.com/keycdn/cache-enabler/blob/7316ce83231f4db11eef6df62a005db9a980bba3/inc/cache_enabler_disk.class.php#L886-L891

Thought I'd get your feedback again since you are the author. 🙂 Were the $wp_error variables on line 889 supposed to be $wp_filesystem->errors, which would get the first error code available, or are we trying to target a specific error in this object for this exception?

Also, is $wp_filesystem->get_error_message on line 888 supposed to be $wp_filesystem->errors->get_error_message as $wp_filesystem->errors is the WordPress Error class?

stevegrunwell commented 3 years ago

You're absolutely correct, it should be referencing $wp_filesystem->errors. I've opened #221 to update it.

To your first question, it's intentionally grabbing the first error in the WP_Error object.

jcatello commented 2 years ago

@stevegrunwell Can you think of any scenario where this is some how going back way to far in terms of parent directory? We use cache enabler on hundreds of sites and almost daily well find 5-10 random sites where there are several endlessly running PHP processes trying to chown the entire filesystem, this eventually fills up PHP error logs.

Example of what we see when running an strace on the PHP processes:

chmod("/sys/dev/char/1:3/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/full/subsystem/zero/subsystem/urandom/subsystem/random/subsystem/random/subsystem/full/subsystem/zero/subsystem/null/subsystem/urandom/subsystem/full/dev", 0755) = -1 EPERM (Operation not permitted)

followed by endless output to logs:

[11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173 [11-Jan-2022 22:58:25 UTC] PHP Warning: chmod(): Operation not permitted in /home/nginx/domains/domain.com/public/wp-admin/includes/class-wp-filesystem-direct.php on line 173

stevegrunwell commented 2 years ago

@jcatello Nothing immediately comes to mind; the Cache_Enabler_Disk::mkdir_p() method intentionally limits its scope to the passed path and its children, and only gets called as new cache files are getting created.

Is it possible a CACHE_ENABLER_* constant is misconfigured on those sites, causing Cache Enabler to try to store files outside of its typical locations?