After writing changes in existing config, Drupal try to modify it's chmod. Not sure this is right, because, if Drupal successfully write changes to config, nothing else it should do.

Problem source

I install Drupal with 'drush si' in command line with my user: 'andyceo'. After that, all active configs have the chmod: 'rw r r' and owner=andyceo and group=andyceo.

I use ACL to allow both 'www-data' (apache2 user in ubuntu) and 'andyceo' change Drupal's 'files' folder:

$ getfacl files/
# file: files/
# owner: andyceo
# group: andyceo
user::rwx
user:www-data:rwx
user:andyceo:rwx
group::rwx
mask::rwx
other::r-x
default:user::rwx
default:user:www-data:rwx
default:user:andyceo:rwx
default:group::rwx
default:mask::rwx
default:other::rwx

Comments

andypost’s picture

Issue tags: +Configuration system
moshe weitzman’s picture

The File API always does a chmod after updating any Drupal file. It has always been that way. I don't know if it is a good idea or not. I personally would be fine with dropping it ... Note that you can control what the chmod does using new(ish) settings in settings.php

andyceo’s picture

In Linux, you can not execute chmod on files you don't own, even files has 0777 permissions. And even you add the user that try to execute chmod to the group of the file.

For the normal access rights this is a design decision. You need richacls: WRITE_ACL and maybe WRITE_OWNER.

Further info is available here: http://unix.stackexchange.com/questions/75893/can-i-allow-users-to-chmod...

So, if we want a simple solution for ability to work fine both with drush in console and with Drupal web interface without any annoying warnings in dblog, just let get this in :)

andypost’s picture

unneccessary_chmod.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, unneccessary_chmod.patch, failed testing.

tayzlor’s picture

Status: Needs work » Needs review
StatusFileSize
new890 bytes

Re-rolling last failing patch

Status: Needs review » Needs work

The last submitted patch, 6: 2232051-unecessary-chmod.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 2232051-unecessary-chmod.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

There has been no activity here for 8 years.

Is this issue still a problem?

Since we need more information to move forward with this issue, I am setting the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

andypost’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new557 bytes

I still think this chmod() is useless otherwise it needs comment explaining reason behind it

Technically setting 644 permission on newly written file could have security consideration but ensureStorage() already used to care about permissions on folder and writing .htaccess (which may not work if config stored in web-root)

andypost’s picture

StatusFileSize
new709 bytes

Also no reason to call getFilePath() in exception because it already called

andypost’s picture

Issue tags: +Needs followup

Moreover there's https://www.php.net/manual/en/function.umask.php which allows to set default permissions for every file to be written within request. FleSystem's chmod is very resource intensive operation and pointless in that case

Thinking a bit more - since PHP 8.1 acl works without issues so core could start to care about it

It needs follow-up to explore other places of core according to comment #2

smustgrave’s picture

Should this go back to needs work since it needs a follow up or can it be RTBC

andypost’s picture

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks I'll mark RTBC then

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: 2232051-23.patch, failed testing. View results

andypost’s picture

Status: Needs work » Reviewed & tested by the community

re-queued

alexpott’s picture

Version: 9.4.x-dev » 9.5.x-dev
Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed

The essential point of #2 has not been answered... I'm pretty sure we added the chmod to duplicate what's being done \Drupal\Core\File\FileSystem::move() in.

Here are some thoughts that try to answer that...

  • When this code was originally written FileStorage was being used for all configuration including active storage. I think this made being the same as the normal file system more important
  • Changing this shouldn't have much impact because file storage is only used to write for export operations.

Therefore I think making this change is fine.

Committed and pushed 581f751582 to 10.1.x and ed275b385e to 10.0.x and 3959be8d83 to 9.5.x. Thanks!

I'm going to treat this as a task and therefore only backport to 9.5.x.

  • alexpott committed 581f751 on 10.1.x
    Issue #2232051 by andypost, andyceo, tayzlor, smustgrave, moshe weitzman...

  • alexpott committed ed275b3 on 10.0.x
    Issue #2232051 by andypost, andyceo, tayzlor, smustgrave, moshe weitzman...

  • alexpott committed 3959be8 on 9.5.x
    Issue #2232051 by andypost, andyceo, tayzlor, smustgrave, moshe weitzman...

Status: Fixed » Closed (fixed)

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