Security: Deprecate "enable-patching" option

[metalbote]

Correct me, if I'm wrong but if i enable "enable-patching" in my project, patches from all my required packages are applied.

Is there a possibility to restrict this behavior to a certain set of package, for examples i just want to get those patches apllied from myvendor/x, and not from all packages my project installs?

This would have several benefits, a) i have a project which could be used just as a quick starter, and all of patches, maintaining etc could be done in a required package. b) I do not have to worry about security by patches being applied from vendor/xyz, only those i explictly added in the allowed patching module.

[acha5066]

I also think this would be a good feature. I want to provide patches to certain modules in a utility module we use internally on a couple of projects. However, I don't want to allow every dependency to provide patches.

[mxr576]

I wanted to open a new issue but I am glad that I am not the only one who finds "enable-patching" option is a slippery sloop and a possible security issue, namely an open door for supply chain attacks.

I put together a small POC that demonstrates how one small dependency in a dependency tree by ANY vendor can sneak in code to your application when you have enable-patching: true; in your root composer.json.

The story that my dummy POC would like to tell that

• module1 was developed by our team
• module2 is an external dependency of our code
• which also depends on a 3rd party library that gone rouge and either ships a previously trusted patch with malicious content or just added a new patch with malicious code, it does not matter

(Please ignore that these are local packages, they could be any anyvendor/awesomepackage in vendor.)

I understand why developers would like to deliver/bundle patches with libraries and if we need a similar feature my proposed alternative would be something like Composer 2.2.0 introduced with allow-plugins. Let's deprecate "enable-patching" and replace with for example an apply-patches-from list that contains all those trusted packages that can apply patches on a project. Example:

{
    "name": "mxr576/my-project",
    "type": "project",
    "minimum-stability": "stable",
    "extra": {
        "apply-patches-from": {
            "mxr576/trusted_lib1",
            "anyvendor/awesomepackage"
        }
    }
}

@metalbote If you like my suggestion, can you rename this issue to Security: Deprecate "enable-patching" option?

[cweagans]

This is in line with what composer has done for enabling plugins. I've thought about this before and I'm not opposed to some kind of limitation or control or something here, but I'd also be open to just emitting a warning when a non-static patch reference is detected. I think a strongly worded warning is probably the low hanging fruit here and then we can take our time to figure out a more thorough path forward.

[acha5066]

I just had a look at the code to see how it could be tackled and I think adding the allow-patches-from option should be added to a new branch as I think the behavior specified in isPatchingEnabled on the 1.x branch could be improved.

Currently, it will only apply patches from dependencies if the parent composer.json does not define any patches but I think the parent composer.json should be able to define patches and allow dependencies to patch files too. I don't think it's possible to change that behaviour on 1.x without breaking BC.

[cweagans]

main doesn't resolve patches from dependencies anymore.