Chmod values are currently hard coded in includes/file.inc and modules/simpletest/tests/file.test. The defaults might work on many setups, but on a setgid system where apache is given write access using the setgid directory group the hard coded values cause serious breakage (they remove the necessary setgid bit so created sub dirs have the wrong group).

Attached is a patch which makes these options configurable by setting a few defines for the used modes, replacing the hard coded values with these defines and making these defines available via default.settings.php.

For the chmod values I've used the current values*, but I would suggest to set the last octal to 0 by default. A permission of 0775 just doesn't make sense. You either give write permission to the owner or the group, the world read permission is superfluos. (Even 0777 makes more sense if the web server runs as nobody and needs world write access. So depending on the setup directory write permissions are 0700, 0770, 02770 or 0777, but never 0775.)

* One exception, I did change the 0444 directory permission to 0555. This should not cause any breakage.

CommentFileSizeAuthor
#4 drupal.chmod_config.patch9.05 KBleonardjo
#3 drupal.chmod_config.patch9.2 KBleonardjo
drupal.chmod_config.patch8.33 KBleonardjo

Comments

lilou’s picture

lilou commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal.chmod_config.patch, failed testing.

leonardjo’s picture

leonardjo commented
Status: Needs work » Active
StatusFileSize
new drupal.chmod_config.patch9.2 KB

Patch applies fine if you move it inside the drupal directory and use
$patch -p1 < drupal.chmod_config.patch
(the patch uses drupal/ instead of drupal-7.0/ as the base directory.)

However, I realized that on old installations the defines are only added in the default.settings.php but not necessarily set in settings.php.

To avoid breakage if the defines aren't set on an old installation the chmod values should be defined conditionally in file.inc and file.test.

Attached patch uses drupal-7.0/ as the base path and adds the aforementioned conditional defines so old installations can safely apply it without having to make changes to the config.

Still I would suggest to change the last octal in every chmod value to 0 before the patch is applied.

Please let me know if you want a different path shift or just check the patch for the base path and change patch's strip level (-p #) if you can't get it applied.

leonardjo’s picture

leonardjo commented
StatusFileSize
new drupal.chmod_config.patch9.05 KB

I just figured out patches need to be made from inside the base directory. What a shame diff(1) doesn't work like that so you have to hand edit patches :S

Anyway, attached the last patch fixed for the base path.

leonardjo’s picture

leonardjo commented
Status: Active » Needs review

drupal.chmod_config.patch queued for re-testing.

leonardjo’s picture

leonardjo commented

#3: drupal.chmod_config.patch queued for re-testing.

leonardjo’s picture

leonardjo commented

#3: drupal.chmod_config.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal.chmod_config.patch, failed testing.

leonardjo’s picture

leonardjo commented
Status: Needs work » Needs review

I cannot reproduce the failures in Authorize API the test system finds on a fresh Drupal-7 with this patch applied.

Could someone have a look at this patch and the errors? Can you please point out what needs to be fixed?

bfroehle’s picture

bfroehle commented

Now try doing a command which involves the authorize.php file, for example install a new module using the admin interface. (Note you'll have to be in an environment where the httpd user doesn't have write access to the files to trigger authorize.php).

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Postponed (maintainer needs more info)

All of those permissions are configurable via variables. The only one that is hardcoded is the read-only 444 on .htaccess. I don't see the point in making this one configurable.

damien tournoud’s picture

damien tournoud commented
Title: [PATCH] Chmod values should be configurable » Chmod values should be configurable
leonardjo’s picture

leonardjo commented
Status: Postponed (maintainer needs more info) » Needs review

#4: drupal.chmod_config.patch queued for re-testing.

bfroehle’s picture

bfroehle commented

leonardjo: To change the defaults, you can already have put the following lines in your settings.php:

$conf['file_chmod_directory'] = 0775;
$conf['file_chmod_file'] = 0664;
coderintherye’s picture

coderintherye commented
Status: Needs review » Postponed (maintainer needs more info)

Assuming #11 kills the reasoning for the patch, I'd like to change this into a feature request for the documentation team to work on a handbook page explaining how to set these values. Like the OP I had trouble with permissions on a couple of hosts and had to play around with some of the chmod values directly at first because I was not sure where to set them.

leonardjo’s picture

leonardjo commented
Status: Postponed (maintainer needs more info) » Needs review

A bit of a doh moment there. I didn't realize that variable_get takes the values from the db and uses the second argument as a fall back. (If only I could find where to set them.) Still...

These kind of basic file system permission variables belong in a config file, not in the database. If I pick up the database and drop it on another machine all I should have to do is edit 2 lines in a config file not have to fiddle with the contents of the database. These are local configuration options that are somewhat independent of the actual installation so it's kind of counter intuitive to me to store them in the database. (A bit like storing the database credentials in the database ;) But I suppose that's more a matter of opinion so do as you see fit.

leonardjo’s picture

leonardjo commented

Ah right, in the config file itself. That indeed solves my problem.

And yes, that might be documented a bit more prominently :-) .

Status: Needs review » Needs work

The last submitted patch, drupal.chmod_config.patch, failed testing.

bfroehle’s picture

bfroehle commented
Title: Chmod values should be configurable » Document setting default chmod permissions
Component: base system » documentation
Category: feature » task
Status: Needs work » Active
Issue tags: +Needs documentation

From post #15:

Assuming #11 kills the reasoning for the patch, I'd like to change this into a feature request for the documentation team to work on a handbook page explaining how to set these values. Like the OP I had trouble with permissions on a couple of hosts and had to play around with some of the chmod values directly at first because I was not sure where to set them.

leonardjo’s picture

leonardjo commented
Title: Document setting default chmod permissions » Chmod values should be configurable
Component: documentation » base system
Category: task » feature
Status: Active » Closed (works as designed)
Issue tags: -Needs documentation

Could you give a simple scenario (which files/dirs to make inaccessible and what action to perform)? I still don't see how this patch could cause breakage there (as all it does is add some defines and use those instead of hard coded values).

Sorry for requeuing the test. I overlooked your comments (perhaps Firefox loads from cache when recreating a session and they just weren't there) so I thought that maybe the tests "got fixed" (as I see 2 failures in drupal_render() on a pristine installation which I didn't see on the testing machine I assumed these tests are in as much flux as HEAD is).

Just a thought, it might be useful if people could obsolote their own patches in these bug reports. I put in version #2, then realized that patches need to be -p0 from inside the installation root so I added #3, only to have #2 go through testing as there was no way to indicate only the latest version needed to be tested.

Closing out this report. Can any of you open an issue for the documentation or do you want me to do that?

bfroehle’s picture

bfroehle commented
Title: Chmod values should be configurable » Document setting default chmod permissions
Component: base system » documentation
Category: feature » task
Status: Closed (works as designed) » Active

This issue will become the documentation request for configuring

$conf['file_chmod_directory'] = 0775;
$conf['file_chmod_file'] = 0664;

in settings.php

leonardjo’s picture

leonardjo commented

Just put

/* File system permissions for writable files */
#$conf['file_chmod_directory'] = 0770;
#$conf['file_chmod_file'] = 0660;

in settings.default.php. That should make it much more obvious. Any mention in the documentation is an added bonus.

coderintherye’s picture

coderintherye commented

Right I agree, this should have a comment in settings.php as well documentation in the handbook. I could write a documentation page if no one else wants to.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Do we document every possible setting that someone could put in settings.php inside settings.default.php, or are they generally documented in the on-line drupal.org handbook only? I do not know the answer to that question.

If we only have some possible settings in settings.php then I am not sure that this is commonly enough needed to put there, and maybe it should just go into the Handbook.

Any thoughts on that?

leonardjo’s picture

leonardjo commented

File permissions for writable files are a basic configuration option that you _need_ to set at installation time. They are about as basic as database credentials, so imo those options should be put in settings.default.php.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Right. My question is more about how often people need to overwrite the default chmod settings. If it's a rare thing to need to override, and we aren't documenting every possible thing you can override in default.settings.php, then we should put this along with all the other stuff we're documenting on a hypothetical doc page on drupal.org (if such a thing exists) and add a note in default.settings.php pointing to that page (if there is one). We should also consider moving other rarely-needed settings out of default.settings.php and into that doc page.

If our goal is to document all possiblities in default.settings.php, then that's a different story.

I still don't know what the answer to that question is.

bfroehle’s picture

bfroehle commented

I think that a note in the default.settings.php file with a link to a documentation page on drupal.org of advanced settings would be appropriate. Users who require some massaging of default file permissions are going to know this and will likely read the settings file to find out how it is possible.

coderintherye’s picture

coderintherye commented

I think that a note in the default.settings.php file with a link to a documentation page on drupal.org of advanced settings would be appropriate. Users who require some massaging of default file permissions are going to know this and will likely read the settings file to find out how it is possible

Yes, this is the same sentiment I have. Regarding the previous question of how common this need is, I don't think we could really tell unless we did a proactive study of all the major hosts to see if permissions needed to be changed to install on them (as I don't think relying on self-reporting from users would be enough to establish the need).

mlncn’s picture

mlncn commented

Subscribe and bump. Not sure what to propose as language etc. this just looks like the kind of thing that will bite me at some point, so tracking here until it gets documented in settings.php ;-)

niklp’s picture

niklp commented

Bumping this because I just spent two days trying to get a decent permissions setup only to find that setgid isn't honoured out of the box, and I still don't know where to look to see if this config will work on various versions of Drupal!

erikwebb’s picture

erikwebb commented
Version: 7.0 » 8.x-dev
Component: documentation » file system

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.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Issue summary: View changes
Status: Active » Closed (outdated)

Closing as outdated. Please re-open if this is still relevant.