Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows

📌 Limit trusted Composer plugins to a known list, allow user to add more Fixed is no longer postponed, so decreasing postponedness.

@kunal.sachdev or @Wim Leers re #4 I am not sure why this issue is postponed on 📌 Allow Validators/Excluders to store info on an individual stage use.. Closed: outdated

Does this validator need to store metadata in the stage? If so what?

Relating 📌 Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed because if we didn't allow other uses besides drupal core then this issue would only have to worry about what files drupal core puts in the base folder, probably my inspecting a composer.json directly rather than worry what any package might use the scaffold for.

#7: Yes, @kunal.sachdev should've documented the reason when he postponed it in #4… Almost a month later, it's no longer clear to me either.

I suspect it's due to some need for first gathering information during PreCreateEvent (i.e. "active") and passing that on to a later event just before applying during PreApplyEvent (i.e. "stage"), to ensure differences in scaffolded files between "active" and "stage" are taken into account, and the superset of the two is allowed — to ensure scaffolded files can be added or deleted?

(Otherwise a file scaffolded pre-update could not be deleted post-update, because it'd be excluded.)

@kunal.sachdev: Can you please confirm/deny my theory? 🙏🤓

I think the more explicit title makes the connection with 📌 Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed clearer and sets clearer expectations.

Also: AFAICT this can happen independently of 📌 Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed .

I had postponed this issue on 📌 Allow Validators/Excluders to store info on an individual stage use.. Closed: outdated because it was needed in 🐛 GitExcluder should not ignore .git directories that belong to packages installed by Composer Fixed so I thought we might need it in our issue as this is similar to GitExcluder. But now I think I can continue to work on this issue and will postponed if needed.

👍 Great! 😊

Postponed on 🐛 The 'fake_site' fixture cannot be using with `composer show` because the packages are not installed Fixed because we need to test that case where project root and web root are different. We don't want to make another `fake_site` fixture until we fix the current one

Looks like some essential files are being excluded … like composer.lock 😳😁

I have pushed up a start to how I think we can test a project with different web and project root. see the comments in \Drupal\Tests\package_manager\Kernel\PathExcluder\UnknownPathExcluderTest::createTestProjectForTemplate

At least 1 test should fail now until it is implemented

Looking great — this needs docs improvements and additional test cases, but it looks superb otherwise! 🤩

Disabling UnknownPathExcluder yields:

/opt/homebrew/bin/php /private/var/folders/x1/mlh998q15c70vs7_y8s1mgqm0000gp/T/ide-phpunit.php --configuration /Users/wim.leers/core/core/phpunit.xml --filter Drupal\\Tests\\package_manager\\Kernel\\PathExcluder\\UnknownPathExcluderTest --test-suffix UnknownPathExcluderTest.php /Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder
Testing started at 4:22 PM ...
PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

Testing /Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder
.F.F                                                                4 / 4 (100%)

Time: 00:05.197, Memory: 10.00 MB

There were 2 failures:

1) Drupal\Tests\package_manager\Kernel\PathExcluder\UnknownPathExcluderTest::testUnknownPath with data set "unknown file where web and project root different" (true, null, array('unknown_file.txt'))
Failed asserting that file "/private/tmp/package_manager_testing_roottest88114527/stage/gpovbxs9gsHOEM5lDu9rZm4cy0wRpOUBcVv2Y8PDzyI/unknown_file.txt" does not exist.

/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder/UnknownPathExcluderTest.php:159
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

2) Drupal\Tests\package_manager\Kernel\PathExcluder\UnknownPathExcluderTest::testUnknownPath with data set "unknown directory where web and project root different" (true, 'unknown_dir', array('unknown_dir/unknown_dir.README.md', 'unknown_dir/unknown_file.txt'))
Failed asserting that file "/private/tmp/package_manager_testing_roottest68717519/stage/_rMZ7beLyeq8uyvirzOGCp0Nbn-xABMUPvwZzWEgYOU/unknown_dir/unknown_dir.README.md" does not exist.

/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder/UnknownPathExcluderTest.php:159
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

FAILURES!
Tests: 4, Assertions: 21, Failures: 2.
Process finished with exit code 1

→ test coverage works as expected! 👍

I left one documentation nit suggestion that I cannot apply because GitLab is once again buggy 🤷‍♂️ @tedbow or @phenaproxima can apply it prior to commit. I suggest @phenaproxima commits this one because @tedbow has many other things on his plate today.

I think this generally makes sense but there are a few things we could change, IMHO, to increase the clarity of the code and the comments. Nothing major, though.

@phenaproxima thanks for the review. The feedback was pretty straight forward so I addressed. There were a couple parts we could just remove so we didn't need update those comments.

going to self RTBC because the feedback was straight forward

@phenaproxima hopefully I addressed your feedback correctly. I pretty much copied your suggested improvements

Yay, this unpostponed #3323461: #3323461-19: Hosting environment (e.g. cPanel) may add additional files (including symlinks) to the project, which breaks AU → .

The commits after I RTBC'd all look like solid improvements 👍

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