silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 820 forks source link

Installation issues with suPHP due to file permissions #8632

Open DorsetDigital opened 6 years ago

DorsetDigital commented 6 years ago

Affected Version All current v4

Description

This has come up a lot just lately, especially with users running cPanel.

When installing on a system which uses suPHP (as cPanel does now), the installer script fails due to the setting of permissions which are too loose on the generated files (eg. index.php). The installer currently sets the permissions as 755 (https://github.com/silverstripe/silverstripe-framework/blob/1d9e40ba264aa6752037bbcb364646e6947fd891/src/Dev/Install/Installer.php#L454) - this causes suPHP to throw an error and prevent installation due to the group-write permission.

It would make sense to me to use 0644 here instead. Is there a circumstance where this would cause issues? The webserver user is (should) be the only one that needs to write to that file, and it would cure a lot of installation issues for users.

DorsetDigital commented 5 years ago

Just a follow up to this, if there is a case for the current permissions, an alternative would be to add an 'advanced' field to the installer to allow a user to set more restricted permissions (with a suitable note about only using it if you know what you're doing, etc.). Selecting the option, or checking a box, or similar would change the permissions in use, and would still allow suPHP users to use the installer.

chillu commented 5 years ago

Yeah it feels like we're finally approaching a time where most devs actually run CLI and web as the same user, e.g. because they're using virtual machines and know about the wonders of su. Although it makes me sad that CPanel is still a thing ;)

There's already graceful errors in the installer if files can't be written. @DorsetDigital Do you want to review our installation instructions, and ensure we guide people through workarounds in the appropriate places? If that's the case, I'm happy with a PR changing that installer default.

In terms of making this a choice in the installer itself, I'm not keen to increase the complexity of both user choices and our implementation maintenance. Its a one-off process, where users are likely to run into problems after they've run the installer (and try to e.g. edit those files through their IDE with a different user). In which case we should ensure there's troubleshooting info

DorsetDigital commented 5 years ago

I've had a read through, and I think for the most part the instructions are fine.
I'll submit a PR on the installer permissions. I don't believe it's specific to cPanel, but rather anywhere suPHP is being used. (It just happened that there were a couple of users with the same problem in quick succession and they were both using cPanel). Ironically, my gut feeling is that a lot more of the commodity hosting is now better configured for this kind of thing than a lot of the 'roll-your-own' setups.

I think at the same time I'll do a docs PR on the installation page to make a note about using the public directory, since it's not mentioned anywhere and it seems to come up a lot on the Slack channel and community forum.

chillu commented 5 years ago

@DorsetDigital Any update on this?

DorsetDigital commented 5 years ago

Hi, Sorry.. haven't got round to this one yet. I'll pull my finger out and get the PR sorted.