search

selwynpolit / d9book

Drupal at your Fingertips: A developers quick reference for Drupal 9 and 10
https://selwynpolit.github.io/d9book/
Creative Commons Attribution 4.0 International
150 stars 103 forks source link

Patch from a merge request #112

Closed chx closed 4 weeks ago

chx commented 1 month ago

https://github.com/selwynpolit/d9book/blob/gh-pages/book/composer.md#patches-from-a-gitlab-merge-request

this is incredibly dangerous you are basically merging ... anything ... into your code base.

penyaskito commented 1 month ago

Yes, this need to be changed to say to download that locally and apply it after careful review.

selwynpolit commented 1 month ago

Thanks folks. I added a big red danger note.

penyaskito commented 1 month ago

Sorry, I would do a PR but that would take me quite longer.

I don't think it's enough.

It should clearly state that they should download the MR patch after review, and then have it applied with cweagans/composer-patches with a local reference like: patches/core-1234567-33.patch.

Ideally, this should be done with any patch, but it's really important with MRs.

For MRs: the .patch url thingy will include whatever is on the MR at the request time. If there were any new commits since you reviewed, you are getting those. That's dangerous, and that's what chx was referring to. First, maybe there are commits you didn't test. Second and most important, a malicious user could have committed something with a bad intent, and now you are giving them the keys to your castle voluntarily.

For drupal issue patches: even if not a security issue, your composer install (or deployment) now depends on drupal.org. If you are building your site and drupal.org has a temp outage, you can't complete it. That's undesired. EDIT: also recently knew that when new d.org is released, issues will be moved to gitlab. Those file urls will stop working at some point, and if they do your project won't build. Be future proof, use local patches.

selwynpolit commented 1 month ago

Thanks for the clarification. I updated the description to make those points. btw @penyaskito It is super easy to suggest edits. Just click the Edit this page on Github at the bottom of the page.