processwire / processwire-requests

ProcessWire feature requests.
39 stars 0 forks source link

Set $config->moduleInstall settings false by default #519

Open teppokoivula opened 5 months ago

teppokoivula commented 5 months ago

This is loosely related to https://processwire.com/talk/topic/29541-cve-2023-24676/, and mostly about being extra careful that dangerous features are only enabled intentionally:

If someone is able to enable debug mode and needs to also upload modules via UI or from an URL, it is reasonable to assume that they are also able to change said settings to "debug".

Thanks for considering.

ryancramerdesign commented 4 months ago

@teppokoivula I have added this but kept the 'directory' option at 'debug' since that has a gatekeeper at least. Though you can easily talk me out of that one too, if you want to.

This is one of those things where I think making it easy to install modules may be kind of important towards attracting new users to PW. And PW is almost entirely module based, so I think it's good to provide some turn key options for module installation, at least at the start, when they are likely to be exploring the system in debug mode.

We're pretty clear during installation that debug mode is for development, while debug mode should always be off for live or production sites. And PW warns you about debug mode every time you login. The purpose of debug mode really is development, so that you can see your errors as you go. And in this context, I think the module installation options are good.

In any case, I always prefer things to be as locked down as possible in the defaults, and let people unlock things as needed, so your suggestion definitely appeals to me. Though others prefer the opposite, so 'debug' was kind of the middle ground compromise between being locked-down and easy-to-install, where things could be locked down for live/production sites and easy-to-install for development sites. I don't really know what the ideal balance for this settings is, but will divert to your suggestion.

adrianbj commented 4 months ago

@ryancramerdesign and @teppokoivula - I am wondering if there might be a new setting that could be introduced, perhaps $config->developmentMode that could determine the default state for many potentially dangerous things in the admin (in case a superuser account was breached).

It could be used to control all module installations, hanna code editing, Tracy Console access, and anything else that module authors think should be protected. The reason I am proposing this vs using $config->debug is that I don't ever want debug enabled on a production site in case errors are displayed publicly. I'd feel safer temporarily setting development mode to true while I make some changes, rather than having debug mode true.

I suppose we could be handle it ourselves with a conditional in config.php like:

$config->developmentMode = false; // toggle this as needed
if($config->developmentMode) {
    $config->moduleInstall('download', true);
    $config->moduleInstall('upload', true);
    $config->moduleInstall('directory', true);
    $config->HannaCodeEdit = true;
    $config->Tracy['enableConsole'] = true; // note that this option doesn't yet exist, but I can add it
}

But if $config->developmentMode was a core setting module authors would know they could rely on it to protect potentially dangerous features.

Just a note about Tracy - I am not sure how many devs leave it installed in production (in production mode), but I do because I find the error logging, notifications (email and Slack), and in particular the saving of Tracy's bluescreen HTML (viewable via the Tracy Exceptions panel) so incredibly useful in debugging production errors. All this to say that I think having a simple developmentMode toggle to lock down the Console panel while still leaving its other functionality intact would probably be a good thing.

ryancramerdesign commented 4 months ago

@adrianbj @teppokoivula Once a site gets out of my local dev environment, I like the $config->debugIf option for cases where I want debug mode, but not for the whole site. I'm not sure how well known this option is, but it can be pretty useful. Though another way to achieve something similar is just $config->debug = $_SERVER['REMOTE_ADDR'] === '123.123.123.123'; or some other condition as appropriate.

Since the purpose of config.debug is for development, I think it could be confusing to have a separate config.developmentMode. Though I think it's fine if one wants to add their own config.developmentMode in the config.php file, but I'm not sure it even needs to be a $config variable if just comparing its value in the /site/config.php file to turn on/off different things in the config variable. Still, wouldn't it have the same limitation as the existing config.debug since it's just contextual to a predefined boolean value in the same file? It seems like conditional config.debug (or config.debugIf function) has the most flexibility here?