CRM-21565: Change mkdir to use correct and more secure mode numbers #120
Conversation
The original setting of `777` is not a valid mode number for mkdir, and should have been `0777` as it is an octal number. This has been changed to `0755` to be in keeping with the wordpress permission scheme, which is readable by all but only writeable by the current user. As these are for backups a more strict setting may be preferable, but this is a reasonable compromise
|
@TBSliver this looks good. I've tested and it works on my system. Before mergeing we should add a change on https://github.com/civicrm/civicrm-wordpress/blob/master/wp-cli/civicrm.php#L382 to set the permissions there to 0755 as well. Can you make this change on your currebt branch so that this gets added to this PR? I would like more testing on this due to the multiple hosting environments we encounter. I am not sure why we set to 777 in the first place, so I do want to have this changed. Thanks for the work on this. |
|
@kcristiano done and done: https://issues.civicrm.org/jira/browse/CRM-21565 |
|
@kcristiano is this something that just needs editing on the original PR text, or is it something that gets put in on the merge notes when you accept and close the PR? |
|
@TBSliver I know that if the PR title includes the CRM-xxxx it helps the release notes get done. If you can edit and add that in it would be great. My hope is to get this in 4.7.30-RC as soon as possible |
|
Thanks @kcristiano @TBSliver: if you could please edit the title of this pull request to read
that would be perfect! |
TBSliver
changed the title
|
@kcristiano modified the title a while ago, hope this can be merged soon :) |
|
@totten would like to see if this can be merged. Somewhere along the line we set loose permission in WP and never caught it. |
|
@eileenmcnaughton Any chance you can take a look here. It's been merge ready and would be good to get in. |
|
@Monish please review and merge if ready |
|
Merging based on @kcristiano review and also because it seems more correct & consistent to me |
The original setting of
777is not a valid mode number for mkdir, andshould have been
0777as it is an octal number. This has been changedto
0755to be in keeping with the wordpress permission scheme, whichis readable by all but only writeable by the current user. As these are
for backups a more strict setting may be preferable, but this is a
reasonable compromise