Dependency patch resolution: Only allow patches from trusted dependencies

[RobinHoutevelts]

trafficstars

Verification

• [X] I have updated Composer to the most recent stable release (composer self-update)
• [X] I have updated Composer Patches to the most recent stable release (composer update cweagans/composer-patches)
• [X] I am using one of the supported PHP versions (8.0+)
• [X] I have searched existing issues and discussions for my idea.

Is your feature request related to a problem?

I only allow patches from sources I trust. By default cweagans/composer-patches seems to apply patches from any of my dependencies.

Describe your proposed solution(s)

{
   // don't allow patches from anyone else
   "ignore-dependency-patches": ["*"],
   // list of repositories you trust to apply patches from
   "dependency-patches": ["my-org/drupal-patches"]
}

Describe alternatives

No response

Additional context

You will have less bug reports related to Dependency patch resolution if users are forced to know where they want to apply patches from.

However, I don't mind it -not my project- but I would need a way to opt-out and instead define the sources I trust myself.

[github-actions[bot]]

👋 Thanks for the idea! Please remember that this is an open source project - feature requests may or may not be implemented, and if they are, the timeline is unknown. If you need a guaranteed implementation or timeline, sponsorships are welcome!

[cweagans]

I'm on board. Maybe we can add another config value 'allow-dependency-patches' and have that act as an allow list if set. I don't want to have to set both. Having both set should probably be an error.

[RobinHoutevelts]

Thanks!

Having both set should probably be an error.

Yes, that would also make this much better to maintain 👍

[RobinHoutevelts]

If I want to disable dependency patch resolution altogether: Would setting allow-dependency-patches: [] (empty list) achieve that?

Edit: I guess I could cheat by doing allow-dependency-patches: ["drupal/core"] or something since it doesn't define patches 😛

[cweagans]

You can just disable the resolver - no need to try to make an allow list work for that :)

[RobinHoutevelts]

@cweagans I'd be happy to take a stab at adding allow-dependency-patches. But I'm currently busy for the next two weeks.

[cweagans]

No worries. Same thing here - I'll update here if I beat you to it!

[markfullmer]

Can we clarify the intention of the allow-dependency-patches definition in regards to the current behavior? With the introduction of this option, is it understood that if allow-dependency-patches is NOT defined, there will be no change in the processing of dependency patches, i.e., all will be allowed? In other words, this will be implemented in a way that allows implementers to opt out of a default "allow all dependency patches" behavior?

[cweagans]

Correct. If the option is NOT defined, all dependencies will be scanned for patches. If it IS defined, only the listed packages will be scanned.

[Natshah]

I'm so interested to having this option, something like allow-plugins but it could be called allowed-patches or allowed-patchers

as we do have allowed-packages for drupal-scaffold

      "allowed-packages": [
        "drupal/core", // Only listing to demo
        "vardot/varbase"
      ],

as

      "allowed-dependency-patches": [
        "drupal/core", // Only listing to demo
        "drupal/starshot-patches", // If the Drupal CMS (starshot) team selected a list of important patches.
        "vardot/varbase-patches"
      ]

Maybe in projects the support team could chose to add more patches. Not to keep coping them every time to 200 projects or more

      "allowed-dependency-patches": [
        "vardot/varbase-patches",
        "my-trusted-private-vendor/private-list-of-patches-by-the-support-team"
      ]

[cweagans]

@Natshah All composer patches config is nested inside of a extra.composer-patches to avoid naming conflicts like that.

As an aside, you might be interested in https://github.com/cweagans/composer-configurable-plugin/ as a replacement for this file: https://github.com/drupal/core-composer-scaffold/blob/11.x/ScaffoldOptions.php

It handles getting config from composer.json + the environment, merging it all down into one location, and ensures that it's the right data type. It probably wouldn't be backwards compatible, but it might be an interesting thing to explore!

[bradjones1]

Another use case here, again from Drupal-land. Now that Drupal CI is run on GitLab the template allows modules to express patches in their composer.json which will be applied during CI runs, e.g. if they require a patch to core. I'm in a situation where the patches in question conflict with others I have on my specific project, so when they are applied from the dependency they fail. Would be nice to be able to disable the patches to avoid this conflict.

[metalbote]

I think this is a very important feature, especially when i think about security, implemented this and PR

[regilero]

While thinking about securely supporting patches, I'd like for example to only allow patches added in a local folder, and present in the first level of my composer.json. I'm not sure what should be done of untrusted patches referenced by packages, is an error message enough? Should we make the build fail? But allowing any package of patching the other ones seems strange.

Edit: it seems on version2 no dependencies patches are running. So it fixes the problem for packages updating other packages. But still being able to fix the trusted sources of patches seems good. Edit again: I noticed the patch-import command on v2, and here the problems are back.