file check is problematic "green" when not test with chmod

I commit a first code which makes the test "real" with

chmod($directory, 0755);

.
But depending on the webserver this can cause problems related to group and others permissions. Maybe we can read the permissions first and just change the user permission. And like good hackers we could clean up by ourself with resetting the permissions as before.

I added a file perm read and chmod reset to the code.

On my test system (D 10.2, nginx, fpm) I changed the file ownership to secured user and the chmod part was just ignored.

So this module is lacking tests but this seems like a change we should have coverage for as this is one of the more important checks

Just found my old issue about this: ✨ "Safe file system permissions" should test the ownership of files and directories Closed: outdated

@smustgrave OK, we need a test which creates an unsave situation to check if the file permission tests is still working correctly? Or is it only to make sure the test will not cause problems?

On my old concept I was only thinking about checking if webserver is the owner of the code. This can also be a good test additionally to the already working search on writeable folder. But this should be handle in different issues.

I'm taking my previous statement for tests back, sorta. 🌱 Add better test coverage per check Active is open. Don't think I should hold a fix for something the module is missing as a whole.

Think this change I'm only going to include in 3.1.x for now.

smustgrave → closed merge request !69

@smustgrave Thanks for committing. If I find some time I plan to check for some more scenarios on the file permissions. And in any case it is still a good idea to get test coverage.

100% all our plugins need it

Automatically closed - issue fixed for 2 weeks with no activity.