Support directories other than vendor in the Hardening Plugin

The vendor hardening plugin does, indeed, assume that the package is in the vendor directory. https://git.drupalcode.org/project/drupal/blob/8.8.x/composer/Plugin/Ven...

If the package is symlinked then that works to remove the cleanup directories regardless where the real directory lives.

I'm not entirely sure we can get the new package location from composer/installers, but it should be possible.

Also, in the case of drupal/core, there's this issue: #2385395: Make Drupal core folder agnostic and allow it to be placed in vendor/drupal/core

OK, given that cleanup happens in post-comand and post-update/install events, composer/installers has always already modified the installation path. This means we can use the installation manager to get that path.

Here's a patch which does this.

Edit:

+++ b/composer/Plugin/VendorHardening/VendorHardeningPlugin.php
@@ -257,20 +263,23 @@ public function cleanAllPackages($vendor_dir) {
+    /* @var $packages_to_be_cleaned \Composer\Plugin\PluginInterface[] */
+    $packages_to_be_cleaned = array_intersect_key($cleanup_paths, $installed_packages);

Need to fix this. The inline doc is wrong. It's an array of strings.

Also need to review this in light of #3103918: [policy + docs] Decide on backwards compatibility policy for Composer plugins in Drupal 8

Marked that one duplicate.

Updated docblocks, fixed inline documentation.

Updates the readme.

This issue is targeted for 8.9.x, but it would be great to have it in 8.8.3+. This is a new behavior, but it's not exactly a new feature. We can consult findings in #3103918: [policy + docs] Decide on backwards compatibility policy for Composer plugins in Drupal 8 to figure out when this gets unleashed.

Added a CR: https://www.drupal.org/node/3112213

Adding needs docs update tag because we'll need to change that once we know which versions of core support this behavior. We'll probably also need to update the CR with this version information if we get a backport to 8.8.x.

Looking at docs it looks totally fine https://www.drupal.org/docs/develop/using-composer/using-drupals-vendor-...

PS: guess only CR will need version update

Great many thanks @Mile23.

For anyone that wishes to apply this patch using composer and cweagans/composer-patches note that you will need to edit the patch first. Search and replace /composer/Plugin/VendorHardening with blank and delete the test change at the end.

I would support backporting to 8 as this removes a significant limitation in the existing code. I am using it to remove Core tests and the demo/testing profiles.

re-roll for 9.1.x

another re-roll

I couldn't apply last patch using cweagans/composer-patches cause it includes changes in core tests which are part of a different composer package.

Therefore, I'm attaching a new patch which just removes test changes so that we can apply it with cweagans/composer-patches by adding

"patchLevel": {
  "drupal/core": "-p2",
  "drupal/core-vendor-hardening": "-p4"
},

This came up in discussion again recently. I am a weak -1 on this enhancement. If we supported removal of dev files from non-vendor locations, then modules might also use this feature for security, which would mean that sites based on drupal/recommended-project would have to start using the vendor hardening tool as well.

A better solution would be for modules to mode their dev bits into a Composer dependency installed in the vendor directory.

I do realize that a lot of modules want to work in both Composer and non-Composer environments, so this might be an additional burden for the non-Composer case. However, requiring Composer for development only doesn't seem too bad to me.

Drupal core ships with 4 MB demo_umami installation profile. Given it's a demo profile why should any Drupal site have it on production

@andypost in slack: In a long run it would be great to decouple tests our of core, but package 10 extra megabytes of umami to live sites also needs work

At least, it‘s worthy allowing to remove demo_umami profile optionally.

https://github.com/skilld-labs/drupal-cleanup, @andypost shared the composer plugin in slack, which is able to remove "profiles/demo_umami"

A better solution would be for modules to mode their dev bits into a Composer dependency installed in the vendor directory.

True but it depends on how the plug-in is being used. Here are two ways of using it where that won't work.

1. I use the hardening plug-in to remove "junk" files from modules that I don't maintain. Do you see this as a valid usage? Is there a better way?
2. #3067979: Exclude test files from release packages Suggests an option to use the plug-in to remove test files from core, and it's unlikely that core would split the codebase.

#24.1: That's reasonable. I still feel that module maintainers should try to move away from shipping dev components, and rely on require-dev instead.

#24.2: Long term, all of core should move to vendor. This will happen eventually, but I don't know how long it will take.

If we supported removal of dev files from non-vendor locations, then modules might also use this feature for security.

+1 thank you, sounds great. :-)

I'm currently using oomphinc/composer-installers-extender to build a libraries/ directory on a D7 site. I'd love to be able to remove test directories from those packages.

which would mean that sites based on drupal/recommended-project would have to start using the vendor hardening tool as well.

I'm not sure how that follows. You could optionally add vendor-hardening after you've created your project if you want to.

A better solution would be for modules to mode their dev bits into a Composer dependency installed in the vendor directory.

If we're going there, then the solution is for extensions to be able to live in vendor/.

Yeah I guess we need this, because it will take a while before core and extensions live in vendor, and we probably won't get half solutions very quickly either.

I guess that leads us back to #18 needing review. #19 is bespoke for people to patch their core.

Tangentially related is #3165183: Do not remove vendor/bin/composer. Discussion on #drupal with @andypost and @Mile23 suggest we might want to allow modifying the removal list, both to add and remove cleaning targets, not just add more.

This issue is about 'hardening' any installed package, regardless of whether it's under vendor/.

Changing the way the plugin is configured would be out of scope, so add a follow-up for that.

Good point. Done: #3165346: Make the hardening plugin more configurable.

I confirm that I used this patch and it worked for me. I don't understand composer enough to comment on the details of the code in the patch - however the maintainer/commiter will have that knowledge. It seems fair to set RTBC on this basis, let's see if we can get this useful patch in.

#18 isn't passing - custom commands failed.

Hi,

Creating a patch as suggested in #35 comment.

Please review the patch.

Thanks.

Not quite sure on what patch #36 was based on, #18 or #19.

@Pooja Ganjage: An interdiff would help with that.

Anyway, since #36 didn't pass and #35 mentions #18 here's a patch based on that one which should pass Custom Commands

Note: I couldn't find the test-file VendorHardeningPluginTest actually being ran on TestBot: https://dispatcher.drupalci.org/job/drupal_patches/72210/consoleText

Thanks @Spokje Looks good to me

This changes a public method on the vendor hardening plugin, but it's explicitly marked @internal so that's OK.

I have some of the same concerns as @greg.1.anderson here, however the changes required to support this are very minimal, so I guess I'm +0.1 or something.

Committed 216dc5c and pushed to 9.2.x. Thanks!