Only allow force deactivated plugins to be activated with SQ1_DEBUG

[defunctl]

What does this do/fix?

If you're utilizing WP_ENVIRONMENT_TYPE defines on some hosts, setting that to development would lead to plugins being allowed active that should never. For example for clients using Jetpack or anything that publishes out to a 3rd party, those plugins would remain active if WP_DEBUG wasn't defined in the host's wp-config.php and lead to some unexpected surprises.

QA

• Items in the $force_inactive[] array should not be able to be activated when WP_ENVIRONMENT_TYPE is not set to production.
• Defining define( 'SQ1_DEBUG', true ); in your wp-config.php will allow those plugins to be activated, and if they were previously activated in the database, will now be active.

Tests

Does this have tests?

• Yes
• No, this doesn't need tests because...well...some tests could be good for this.
• No, I need help figuring out how to write the tests.

[nickpelton]

Why add another define we have to track? Why not key off the WP_ENVIROMENT_TYPE instead? You noted we shoudn't activate some plugins in development, and WP_ENVIROMENT_TYPE can provide that, can't it?

[defunctl]

Why add another define we have to track? Why not key off the WP_ENVIROMENT_TYPE instead? You noted we shoudn't activate some plugins in development, and WP_ENVIROMENT_TYPE can provide that, can't it?

Because WP_ENVIROMENT_TYPE automatically enables WP_DEBUG if it's not defined.

Take this example:

1. You have a plugin that auto creates a post and tweets every 10 minutes.
2. Your host utilizes WP_ENVIROMENT_TYPE or you automatically set that to the correct environment via some scripting method.
3. The host does not have WP_DEBUG defined in wp-config.php.
4. You've added this auto tweet plugin to $this->force_inactive[], with the expectation this plugin would never be enabled on anything other than production.

In this scenario with the logic before this PR, that plugin would be auto tweeting if it was ever previously activated (e.g. when you did a database clone from production to dev, for example).

The SQ1_DEBUG define is there for when a developer on their local or on a server would actually like to force enable it in a non production environment to do troubleshooting, so explicitly defining it (since it doesn't have to exist) and setting it to true, minimizing the risk a plugin gets activated without knowledge.

Without this, you'd have to switch the environment type to production to test this, therefore unexpectedly disabling Glomar's protection to the site and enabling other 3rd party plugins that automatically deactivate some features when the environment type is not production.

We've seen the consequences of that in the past when a dev server gets picked up by Google unexpectedly.

I added the define to the sample only for visibility, so developers know it's there, but it can be removed or just commented out or perhaps add more context for it in a comment as well.

Hope this makes sense.

[defunctl]

I should add, if the host already has WP_DEBUG set to true, which is often the case on a development environment, the plugin would not be force deactivated with the logic in place before the PR

[ChrisMKindred]

I might not be completely understanding but why not just check the WP_ENVIRONMENT_TYPE instead of WP_DEBUG?

[nickpelton]

After reading it over again, wouldn't it be simpler to just ensure WP_DEBUG is properly set? It this situation we now have a bool we set to make sure if another bool isn't set we fix the issue. Setting the original bool properly seems like the simpler choice.

[defunctl]

I might not be completely understanding but why not just check the WP_ENVIRONMENT_TYPE instead of WP_DEBUG?

Because developers need a way to explicitly enable force disabled plugins for debugging, in a non-unexpected way. If you wanted to debug some plugin you force deactivated in a situation where it's only happening on a remote server, without a secondary check you would set define( 'WP_ENVIRONMENT_TYPE', 'production' ); and now you disabled Glomar and the dev server is wide open to the public. Also, you just accidentally enabled Jetpack, and the client is using Publicize. You start debugging by creating a whole bunch of posts and you just sent out debug posts to the client's Facebook, Twitter, LinkedIn social sites (this is a true story).

After reading it over again, wouldn't it be simpler to just ensure WP_DEBUG is properly set? It this situation we now have a bool we set to make sure if another bool isn't set we fix the issue. Setting the original bool properly seems like the simpler choice.

I'm not sure what the big deal about one more define is. SQ1_DEBUG doesn't need to be added or enabled by default. It does not need to be included in any deploy processes and its only purpose is that a developer would define that to true if they were 100% sure they wanted to enable force deactivated plugins for debugging purposes. If you look at the code the default is that it's not defined at all to force deactivate plugins that the developer expects.

Scenarios:

1. WP Engine doesn't define WP_DEBUG out of the box.
2. Developer adds a system that detects the current environment (like I did on another project) that automatically sets WP_ENVIRONMENT_TYPE based on the install name, so WP_ENVIRONMENT_TYPE matches WP Engine's development, staging and production environments.
3. Developer explicitly defines a number of plugins the client complained sent out 1000 tweets last time to be force deactivated. Expecting this plugin to never be enabled unless the environment is set to production (or in this caseSQ1_DEBUG === true).
4. Upon deploy to develop 1000 tweets go out, whoops, developer forgot to set define( 'WP_DEBUG', false ); because WP Engine doesn't define it at all and WordPress automatically sets WP_DEBUG to true when the environment is set to development and the client is angry.
5. Developer now corrects their mistake, SFTP's in to update wp-config.php with define( 'WP_DEBUG', false ); and everything is good (ignoring the fact though we actually want this enabled on a development server to catch unrelated bugs, see below).

A week later...

1. Another developer is trying to debug something completely unrelated that is only happening on WP Engine's servers, they SFTP in and update define( 'WP_DEBUG', true ); to see if any specific errors will help with the situation.
2. 1000 tweets go out to a public account from the dev server.
3. That developer now realizes they cannot use WP_DEBUG on a development environment and sets it back to false.

Everything's fine for now but then a month later...

QA asks for production data on the develop server to test a different feature, someone has WP Engine clone the production environment but has now forgotten to check wp-config.php and the define( 'WP_DEBUG', false ); was removed because it never defined on production either and that was cloned down in addition to the fact that cloning production means that you cloned a system where those plugins are enabled in the database, so even if the developer had previously deactivated the plugins manually on the development environment, the clone enables them again.

1000 tweets go out.

[ChrisMKindred]

I don't have an issue with another define, my concern is with not intuitively knowing what this one does differently than the others. In your example, it would be better to create a define called ACTIVATE_AUTO_TWEET or something like that and then customizing the force activation for that specific use case.

[defunctl]

I don't have an issue with another define, my concern is with not intuitively knowing what this one does differently than the others. In your example, it would be better to create a define called ACTIVATE_AUTO_TWEET or something like that and then customizing the force activation for that specific use case.

Can you give me a code example of what you mean here and let me know how it prevents the scenarios I outlined above?

[nickpelton]

Thanks for the longer explanation. I think it's fine but I would rename the SQ1_DEBUG to SQ1_FORCE_INACTIVE_PLUGINS so it's 100% clear what it does. SQ1_DEBUG for the unaware is not very clear.

[defunctl]

Thanks for the longer explanation. I think it's fine but I would rename the SQ1_DEBUG to SQ1_FORCE_INACTIVE_PLUGINS so it's 100% clear what it does. SQ1_DEBUG for the unaware is not very clear.

Fair. I named it that because I was thinking it could be used for other things. Thinking about that again, it would create the same cross dependency where one should only specifically activate force deactivated plugins when they're absolutely sure it's safe to do so and the define should only do just that one thing.

I'll update after lunch.

[tarecord]

Has this gone stale? Or is it still relevant @defunctl (I assume so)