Site Health makes downright wrong and dangerous suggestions

[peterhoegsg]

1. "Background updates are not working as expected"

We absolutely do not want background updates to run. We want to carefully manage updates by performing them in a test environment before we then deploy the changes to staging and finally production. Too much breakage otherwise. White screen of death anyone?

2. "Some files are not writable by WordPress:"

The wordpress files are served from a read-only store and having the web application have access to modify itself is a *terrible* recommendation from a security point of view.

3. "Inactive plugins are tempting targets for attackers.".

How is an inactive plugin special in terms of attackability? Surely "Code that runs on an internal accessible is a tempting target for attackers".

Whether they are active or not, plugins *in general* should be kept to a minimum to minimize the attack surface. Also, how is an inactivate plugin a target in the first place? If it's deactivated, surely it doesn't run. If there is a way to execute code in a deactivated plugin surely *that* needs to be addressed.

4. "You should remove inactive themes"

Same as with plugins.

5. Some plugins will also detect that auto-updates are disabled and add to the noise

One example is "MonsterInsights" that reports "Automatic updates are disabled". See item 1.

[joyously]

1. for background updates, what would you suggest is wrong with the statement? From the WP point of view, I guess the expected should be whatever the define is set to, not what the default is set to.

2. writable files: there is no recommendation in that statement of fact.

3. inactive plugins: PHP files in the plugins folder can be executed by addressing them directly. Depending on the file, a hack could be accomplished.
4. same

5. Do you mean plugins that report separately or add in to the Site Health page? Either way, that's something for that plugin to change.

[peterhoegsg]

We might going slightly off-topic (changes to Site Health vs general security), so if there is a better way to address that, please let me know.

1. for background updates, what would you suggest is wrong with the statement?

I would suggest having an official set of security guidelines that explains how to handle updates:

1. never directly in production, but instead offline and then subsequently deployed after testing (like you would for any other web application)
2. *not* having auto-updates enabled

I agree, it's better to have a possibly broken site than one that is vulnerable, but that should only ever be the recommendation if it cannot be done properly.

2. writable files: there is no recommendation in that statement of fact.

Correct, but it's shown as an "issue", which for the purpose of auto-updates is correct, but from the point of keeping things secure, it very much isn't.

3. inactive plugins: PHP files in the plugins folder can be executed by addressing them directly.
4. same

The takeaway here should be to force plugins and themes to include guard statements in every file so they do nothing if not enabled. For plugins distributed via wordpress.org/plugins, surely there's a CI step that approves new versions where this could be enforced.

5. Do you mean plugins that report separately or add in to the Site Health page? Either way, that's something for that plugin to change.

I mean the latter and you are of course correct - this is a plugin issue. However, wordpress should be providing the infrastructure that makes it easy for plugins to handle properly.

The 2 main issues with "Site Health" as I see it are as follows:

1. some of the recommendations made fly against industry best practice and thus makes things unnecessarily insecure

2. customers (here I'm talking companies for whom the sites are built, not the end-users) *will* see this and *will* raise issues. "It says my site has critical issues - fiiiiix iiiiiit".

[galbaras]

+1. Well maintained live sites should not update ANYTHING automatically, except minor WP releases and emergency (security) fixes.

Specifically for plugin messages, it's tough to control, but once the official stance is changed and announced, it's more likely to be adopted. Using the above example, I can't imagine the MonsterInsights team not paying attention to Core Team suggestions.

[peterhoegsg]

Well maintained live sites should not update ANYTHING automatically

I would argue that the site shouldn't have permissions to change anything thus making auto-upgrades impossible.

It's interesting to think about that on your average VM running WordPress, literally the only piece of software that can change itself through its own files is WordPress.

I understand that WordPress has been hugely successful in making things easy for the non-tech inclined people, but there is a massive difference between doing that and then completely disregarding years of hard-(l)earned industry best practice.

[galbaras]

One idea might be to have a setting for the level of maintenance professionalism applied to the site, e.g. "hobbyist", "professional", "agency", etc, and have that drive the assumptions used for reports.

In general, I find that Site Health is aimed at people who know very little, and can produce rather scary output, which is also sometimes not the best advice, as per this ticket. For someone who maintains sites for a living (like me, among other things), this output is annoying.

Frankly, I'm also annoyed by errors being sent to me by email, which aren't recorded in the log files, but I digress.

[Presskopp]

related #49923

[peterhoegsg]

Having the option to turn off warnings in IMHO orthogonal to promoting unsafe practices.

[galbaras]

Replying to peterhoegsg:

Having the option to turn off warnings in IMHO orthogonal to promoting unsafe practices.

Agreed, but while we're putting things in real-world perspective, we might as well deal with everything.

[peterhoegsg]

Absolutely - the question is just if this should be split into multiple tickets:

1. a general security recommendation/guide (possible with differences in recommendations based on end-user type)
2. what Site Health shows
3. a mechanism to either target Site Health based on end-user type or a way to silence/acknowledge warnings

[galbaras]

@peterhoegsg My suggestion was to put things in the context of who needs to use them, thinking that this can help clarify and simplify things. Either way, this is your ticket and you can drive the scope.

[Clorith]

The checks are aimed at non-technical users, if you have set up your site to be version controlled, then it is fair to assume that you are aware that you can ignore, or filter out, checks that are then irrelevant to you, at least that is the thought behind it.

That said, I will happily address some of your concerns with the thought behind them, they can all have nuances, but they are aimed at the majority of users.

1. Background updates are tied into automated security updates, a feature WordPress core will continue to recommend, it is one of the most important tools we have right now to help keep the internet as safe as we can (given WordPress' current market share, this is incredibly important).

2. Having files not be writable by WordPress may impact the automated updates (if it can't write the new files, you may end up with broken updates). Writing to files is also used by other elements of WordPress in some way (for example the theme and plugin editors), but the focus and intent here is on the security aspect, and the ability to write updates without breaking a site.

3. (and 4) As noted, inactive plugins or themes can still be queried directly, and depending on the code in these, may be vectors for security incidents of varying degrees. If you have suggestions for better wording to relay the potential risk of leaving unused code laying around in a strong enough manner to incite action, but not sound like fear mongering, I'm very open to hearing that though.

I'm not sure if "Set what kind of user you are" is something that would make sense from a core perspective, but that would be a whole other ticket, and not related to the Site Health component. As noted, there are already filters for managing what checks and information is available, alongside multiple plugins that provide interfaces for managing these screens to help as well.

[galbaras]

As noted, there are already filters for managing what checks and information is available, alongside multiple plugins that provide interfaces for managing these screens to help as well.

I can see the ​site_status_tests filter provides a way to add, remove or change site health checks, and professionals can use it on their sites, e.g. via a custom plugin.

@Clorith Which plugins were you referring to? I can't find any that allow tweaking of the tests.

As for automatic updates to plugins and themes, I've seen them introduce problems, and find them more dangerous than they're worth, because they can crash sites without the owner being aware of it. I strongly object to enabling automatic major updates on anything, not even WordPress (other than on dedicated dev/test sites), but even disabling minor updates should be noted using mind language that doesn't assume anything about the user's actual level of risk from it.

This is particularly important when the plugin or theme developer doesn't adhere to the version naming convention and includes significant changes with minor release numbers. Alas, this is all too common.

With the assumption that Site Health is aimed at site owners who aren't WordPress expects, we should be extra careful about prompting them to do things they shouldn't, and even about making them worry, when they shouldn't.

I wonder if the majority of those who look at Site Health reports are of this type, though. Maybe that's the case on platforms, like wordpress.com, but is it the case on self-hosted sites too? I would expect those to be mostly built and/or maintained professionally.

[pbiron]

I won't comment on the wrong and dangerous suggestions aspect of this ticket.

I will say, however, that I think that if the site admin has disabled all updates (e.g., by define( 'AUTOMATIC_UPDATER_DISABLED', true );) then the Background updates are not working as expected wording would only be "correct" if background updates actually happened!

In such a case, I think wording such as Background updates have been disabled would be more appropriate for the test label and amend the test description to add an explanation of why admins _might_ want to enable them.

I'm fine with the test still reporting it as a critical test failure...it's the "not working as expected" part that always bothers me. Since core explicitly provides several means for site admins to disable updates, then the fact that a site admin uses one of those methods to disable updates shouldn't be listed "not working as expected".