Problem/Motivation

In Drupal\Core\Composer\Composer::vendorTestCodeCleanup() we attempt to remove test directories from the vendor directory as security mitigation.

The test directory for twig/twig is listed as test, which is accurate up through 1.42.2: https://github.com/twigphp/Twig/tree/v1.42.2

But then in 1.42.3 it's changed to tests: https://github.com/twigphp/Twig/tree/v1.42.3

This leaves behind the tests for twig/twig after an update.

This same issue is present in 8.8.x's vendor hardening plugin: https://git.drupalcode.org/project/drupal/blob/8.8.x/composer/Plugin/Ven...

Proposed resolution

Add tests to the list of directories to remove, so that we attempt to remove both test and tests.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3082145-10-8.7.x.patch517 bytesvoleger
#5 3082145-5.patch1.21 KBvoleger
#3 3082145-3-8.8.x.patch623 bytesvoleger
#3 3082145-3-8.7.x.patch517 bytesvoleger
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Status: Active » Needs review
FileSize
3082145-3-8.7.x.patch517 bytes
3082145-3-8.8.x.patch623 bytes

Here the patches for both branches

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Version: 8.7.x-dev » 8.8.x-dev
Status: Needs review » Needs work
Issue tags: +needs backport to 8.7.x

Thanks, @voleger. :-)

Let's just do 8.8.x now and then get a backport, because they're different.

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -90,7 +90,7 @@ class Composer {
+    'twig/twig' => ['doc', 'ext', 'test', 'tests'],

We need this same change in composer/Plugin/VendorHardening/Config.php.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Status: Needs work » Needs review
FileSize
3082145-5.patch1.21 KB

Sure

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

LGTM. :-)

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Title: Test directories left behind for twig/twig » Vendor cleanup fail for twig/twig

Updated title to be clearer.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 69fc0df and pushed to 8.8.x. Thanks!

  • larowlan committed 69fc0df on 8.8.x 
    Issue #3082145 by voleger, Mile23: Vendor cleanup fail for twig/twig
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Status: Patch (to be ported) » Needs review
Issue tags: -needs backport to 8.7.x
FileSize
3082145-10-8.7.x.patch517 bytes

Patch for 8.7.x

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

LGTM.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Committed 3f3c757 and pushed to 8.7.x. Thanks!

  • larowlan committed 3f3c757 on 8.7.x 
    Issue #3082145 by voleger, Mile23: Vendor cleanup fail for twig/twig
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Issue tags: +Composer initiative

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.