fix: don't generate advisories for Drupal distributions

[G-Rath]

Distributions are not sourced through Composer so these advisories currently don't map to an actual packages and are getting flagged by the osv-linter.

Some of these advisories do seem to name specific modules which might be available via Composer, so these could come back in future but that'll require having the specific module represented in a machine-readable way

[G-Rath]

I'm pretty sure this is the right move at least for the short-term, but I'm not that familiar with distributions so would appreciate some guidance from @Unifex & @greggles - if either of you think this is the right way to go, I'll take this out of draft

[Unifex]

Looking at the projects for a number of the SAs removed, and checking a few of them on Packagist, filtering on project_distribution does look like a legit filter.

I would wait for the +1 from @greggles as he likely has more insight into the ingestion process for Packagist.

[drumm]

I recommend limiting to the known good types instead of finding ones to filter out one-by-one. Packages.drupal.org/8 distributes project_module & project_theme. Packages.drupal.org is also the authoritative source for advisories on project_general & project_core for the drupal/core packages distributed via Packagist.org. Any type except for those 4 should be discarded along with distributions if the OSV database does not want them.

[G-Rath]

I recommend limiting to the known good types instead of finding ones to filter out one-by-one

I would rather we default to "excluding" rather than "including" until it proves to be untenable since that should ensure the most coverage and the osv-linter makes it very easy to catch these - so far this has resulted in 19 incorrect records out of over 300+, which is a very good ratio.

Any type except for those 4 should be discarded along with distributions if the OSV database does not want them.

It's not a case that osv.dev doesn't want them, it's that the advisories should map to a concrete "thing" which is what needs to be changed to address the vulnerability, and for the Packagist ecosystem "thing" is "a package that is available via a Packagist-like repository".

My understanding is that Drupal distributions are not provided as Packagist packages, so we shouldn't use that ecosystem, and there's no other way to represent them currently (i.e. in theory we could have a "drupal distribution" ecosystem that'd let us publish advisories for these)

[drumm]

My understanding is that Drupal distributions are not provided as Packagist packages

That's correct, and the only type that will be available are project_module, project_theme, project_general and Drupal core as I describe above. There are not plans to introduce more types, everything new falls under project_general. For example, recipes were recently introduced and are project_general.

[star-szr]

Setting aside the include/exclude question, the current code seems ready to go from our perspective (myself, @sensespidey, and @ergonlogic reviewed).

We also reproduced the linting errors that were reported with osv-linter, and can confirm that those are no longer output with the new set of advisories. Noting that we did see other linting errors, and are happy to contribute to getting that list of errors down to zero.

Example linting error that is no longer being output:

../../../drupal-advisory-database/advisories/social/DRUPAL-CONTRIB-2022-062.json:
	 * [PKG:001]: package "drupal/social" not found in "Packagist"
	 * [PKG:002]: unable to retrieve JSON for "drupal/social": unable to validate package: fail: "https://repo.packagist.org/p2/drupal/social.json": bad response: 404

As an FYI, we had to delete the existing ./advisories directory to actually have the effect of these JSON files being removed from the repo and that was a bit of a surprise. Our expectation was that we would see the deletions immediately after re-running scripts/generate_osv_advisories.py.

Would it make sense to build in more of a workflow where we delete ./advisories in certain circumstances (or maybe always, before re-generating, because re-generating seems to be very fast)? The current situation seems like we're sometimes relying on a human to remember to rm -rf ./advisories. Happy to work through this on a separate issue if the current maintainers think it's worthwhile.

[G-Rath]

Noting that we did see other linting errors, and are happy to contribute to getting that list of errors down to zero.

Appreciate the offer, but I've already got the majority of them addressed, I just need to push them up which should be up shortly 😅

As an FYI, we had to delete the existing ./advisories directory to actually have the effect of these JSON files being removed from the repo and that was a bit of a surprise

Yeah I have been thinking about that myself - I settled on it not being worth it because it's not expected that advisories will be removed outside of changes like this (which in turn should be increasingly rare, else we have other problems 😅) and our generate workflow does remove them nope it doesn't, it should do that - I've opened #151 for that.

(also because my preference would be to remove specific advisories rather than nuke the whole directory, which'd require more logic)

Definitely something to keep an eye on though in case it's a more frequent occurrence than we expect

After having to manually remove more advisories, I was convinced to just do this now: #153