Consider disallowed plugin a fatal error

[damz]

Since July 1st, all plugins are now silently disabled if allow-plugins is not explicitely set in composer.json.

Unfortunately in that case, composer install still returns success. And in most cases this just results into a completely broken application, because composer plugins are used to install modules in the right place for Drupal, Wordpress, etc.

See #10912

I suggest we consider a disallowed plugin as a fatal error.

(Unfortunately, this doesn't seem to be breaking any test)

[Seldaek]

IMO this should rather be done here throwing instead of simply warning:

composer/src/Composer/Plugin/PluginManager.php

Lines 755 to 756 in 435e0f1

	$this->io->writeError('<warning>'.$package.($isGlobalPlugin ? ' (installed globally)' : '').' contains a Composer plugin which is blocked by your allow-plugins config. You may add it to the list if you consider it safe. See https://getcomposer.org/allow-plugins</warning>');
	$this->io->writeError('<warning>You can run "composer '.($isGlobalPlugin ? 'global ' : '').'config --no-plugins allow-plugins.'.$package.' [true|false]" to enable it (true) or keep it disabled and suppress this warning (false)</warning>');

So if you are running interactively, you will get prompted, if not it throws. And if you explicitly disable a plugin we don't want to throw either as that's a clear signal, we only want to throw for people without an allow-plugins configured.

[damz]

@Seldaek That makes sense, I updated the PR, and confirmed that it works as expected:

Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Package operations: 99 installs, 0 updates, 0 removals
  - Installing composer/installers (v1.12.0): Extracting archive
Plugin initialization failed (composer/installers contains a Composer plugin which is blocked by your allow-plugins config. You may add it to the list if you consider it safe. See https://getcomposer.org/allow-plugins), uninstalling plugin
  - Removing composer/installers (v1.12.0)
    Install of composer/installers failed

In PluginManager.php line 759:
                                                                                                                        
  composer/installers contains a Composer plugin which is blocked by your allow-plugins config. You may add it to the   
  list if you consider it safe. See https://getcomposer.org/allow-plugins                                               
      

[chx]

The solution is to version composer.json

Old versions behave as if composer config allow-plugins true -n were run.

New versions get security hardened. composer create-project creates such.

I believe currently composer.json is not versioned, a default takes easy care of that.

This allows future such changes not to break everyone's builds silently on a long weekend , one of the largest holidays in the United States...

[Seldaek]

The versioning is a good point and a good idea. The composer.lock has some notion of version thanks to the composer-plugin-api being stored in there.

So I think what I'll add to this PR tomorrow (feel free to leave as is @damz) is that if the lock file predates Composer 2.2, it will keep the warning and default to true (during installs), and for all updates or installs with 2.2+ lock we keep the current PR throwing hard.

I absolutely see the point that if you haven't run any Composer update in the last 6 months and suddenly an install with a newer Composer fails this is not great. The timing for the US is unfortunate, but we planned/coded this late last year (probably around Thanksgiving..) and July 1st seemed like a decent far away date without much special context to it to me.

I hope this soften the blow even further, unfortunately we cannot fix any existing version out there but it should at least reduce problem for things running automated on the latest version.

Regarding BC in general, I believe we do have a pretty decent track record of BC keeping. Heck Composer 2.2 still supports PHP 5.3 after 11 years and even Composer 2.0 did not break all that much for most users. But note that if you want to version your composer.json and play it extra safe you can require composer nowadays. Please don't do it in libraries ever tho, but for private projects why not I suppose, it may cause some pain but it is more explicit.

[chx]

I absolutely see the point that if you haven't run any Composer update in the last 6 months

composer 2.2 was released on 2021-12-22 you gave us six months for the composer upgrade while not having any reason whatsoever to upgrade it. My local simply wasn't, the version my provider runs was.

This was all the worse because the time bomb was hidden and the plugin warnings did not include any warnings that by July it will break so the community wasn't abuzz with warnings.

[Seldaek]

composer 2.2 was released on 2021-12-22 you gave us six months for the composer upgrade while not having any reason whatsoever to upgrade it. My local simply wasn't, the version my provider runs was.

Yes, as I wrote above versioning could have helped and will help once I implement it, but I guess I now have to argue with someone on the internet ™️ first, as somehow the plan above does not seem to be good enough and you had to go on a rant.

There was 6 months to upgrade, and IMO plenty of reasons (see https://blog.packagist.com/composer-2-2/ and https://blog.packagist.com/composer-2-3/) to upgrade. If new features and huge perf boosts aren't enough motivation for you to upgrade, we also fixed a CVE back in April in https://github.com/composer/composer/releases/tag/2.2.12. Generally speaking upgrading software regularly is a good idea, and if you don't do it regularly you are bound to run into issues when doing huge version jumps.

This was all the worse because the time bomb was hidden and the plugin warnings did not include any warnings that by July it will break so the community wasn't abuzz with warnings.

That is just plain wrong. You must have had warnings printed out in every Composer run on your hosting provider:

composer/src/Composer/Plugin/PluginManager.php

Lines 688 to 692 in c422fef

	if (!isset($warned['all'])) {
	$this->io->writeError('<warning>For additional security you should declare the allow-plugins config with a list of packages names that are allowed to run code. See https://getcomposer.org/allow-plugins</warning>');
	$this->io->writeError('<warning>You have until July 2022 to add the setting. Composer will then switch the default behavior to disallow all plugins.</warning>');
	$warned['all'] = true;
	}

This printed out:

You have until July 2022 to add the setting. Composer will then switch the default behavior to disallow all plugins.

If that isn't a clear enough warning, with a date, I don't know what is.

The problem is that some people never read warnings, and never act on them, so eventually we have to break things anyway, no matter how long we wait someone will always act surprised and come ranting.

[damz]

@Seldaek What is the behavior of this when there is no composer.lock?

It is unfortunate, but despite our best efforts, I am quite sure that a non-insignificant number of our customers do not have a composer.lock committed to their repository.

[Seldaek]

It silently allows plugins only if there is a lock file and that lock file has a composer-plugin-api < 2.2 (or no composer-plugin-api defined, which could be the case).

So if there is no lock file or a "modern" lock file it throws:

[Seldaek]

Note that I also made it throw very early in the prepare step so that it should hopefully throw before touching the vendor dir at all, avoiding a partially-updated vendor dir (tho for platform.sh I doubt this matters as it's always a new fresh dir I assume).