Provide security-advisories API endpoint

This should be doable, I can probably retrieve my draft implementation from before https://git.drupalcode.org/project/project_composer/-/merge_requests/7#n..., which I interpreted as saying this API wasn’t necessary.

I'll leave out advertising the new API until its been tested a bit

https://git.drupalcode.org/issue/project_composer-3399877/-/commit/3c1d1... can be reverted when ready

naderman - The API is now live, but I'd like some verification before advertising it in http://packages.drupal.org/8/packages.json

Examples:

• https://packages.drupal.org/8/security-advisories
• https://packages.drupal.org/8/security-advisories?updatedSince=1700000000
• https://packages.drupal.org/8/security-advisories?packages[]=drupal/mollie
• https://packages.drupal.org/8/security-advisories?packages[]=drupal/moll...

Implementation looks fine to me as far as I can tell that without knowing all the surrounding code, and API looks to work as expected. Thanks very much for the speedy implementation here!

Thanks! This API is now advertised in https://packages.drupal.org/8/packages.json and looks like it is working well.

I’ve rolled back this deployment as it almost certainly caused https://github.com/composer/composer/issues/11767

We got the same error as #11 on several projects, all of which either errored on drupal/ctools or drupal/entity_browser. Hopefully those projects give some insight into what the problem may be :)

Without this endpoint reporting security advisories in p2 endpoints for packages not actually provided by packages.drupal.org is unreliable and may even be considered a bug and made impossible in future Composer versions

I will note support for this was intentionally added to Composer with the following two issues:
https://github.com/composer/composer/issues/11256 (Added into Composer 2.5.2).
https://github.com/composer/composer/issues/11435 (Added in Composer 2.6.0).

    api-url: "https://packagist.org/api/security-advisories/",

As noted in the linked MR I was the one who originally suggested they might want to use the p2 endpoints only since these are already cached by the D.O. CDN (and by composer) creating minimal additional infrastructure load. Obviously if @drumm feels the extra load is acceptable that original reason shouldn't be a blocker to implementing the api-url endpoint now if it is desired.

It is true that p2 files are going to be slightly larger when they need to be download, and they will require more GET requests rather than a single request to an API endpoint, however they also are not likely to be downloaded in full on every request when a 304 - Not Modified is returned.

acbramley - for this issue, more detail is needed than that. https://github.com/composer/composer/issues/11767 is a good example of a detailed, helpful bug report.

cmlara - load is probably a toss-up between the two approaches. Static files are great for caching, but a round-trip for each package to see if it is not modified is still an HTTP request for each. Given the reasons in the issue summary, I don’t see a problem implementing this API. Automatic updates does need static files, since they can be signed. I expect https://github.com/php-tuf/composer-integration can alter Composer’s behavior to only use the p2 files.

I suspect the issue might be that https://packages.drupal.org/8/security-advisories?packages[]=drupal/font... returns the advisory indexed with drupal/fontawesome. fontawesome_iconpicker_widget is a metapackage for that component module of fontawesome. If this is the issue, then we should either return nothing for that request, or return the advisory with the name mapped back to fontawesome_iconpicker_widget

@cmlara I had a conversation with Jordi about those tickets before creating the issue here. Generally I think there are some problems with how that was implemented and in hindside I don't think it should have happened that way.

So for now the way to fully get it working correctly is with the API as described here, and I think @drumm may be right about the reason for the issue that popped up.

That said I'd like to revisit if there is a way to avoid implementing the API endpoint with Jordi in January and see if maybe there's a way to get the p2 mechanism working fully, maybe through modifying what the changes feed contains, or providing some other static discoverability mechanism for advisories, that the p2 files currently lack. Think we also need to take another look at consistency of Composer behavior about loading p2 files for advisories from lower priority repositories despite dependency resolution intentionally skipping them.

I did implement ✨ Provide metadata/changes.json endpoint to track updates Needs review so any p2 metadata change will appear in https://packages.drupal.org/8/metadata/changes.json. So when a security advisory is updated, that will go through updating everything on our end and appear in changes.json

So a first update on this as we're going through the advisories functionality again to make sure that different 3rd party repo implementations don't cause problems:

https://github.com/composer/composer/commit/a29acbdd2ed087940ed12447fe7d...

The query-all option has been dropped, it was ignored anyways. The security advisories API is now a requirement if you don't provide a list of available-packages or an available-packages-pattern to restrict search space for advisories in p2 files. Since packages.drupal.org/8/packages.json specifies "available-package-patterns" this particular requirement does not require the API implementation for this repo right now.

I'll post again when/if there are more updates that may impact this repo.

BTW I had a closer look at https://github.com/composer/composer/issues/11767 thanks to what you wrote above and I tweaked the code now so that Composer would just warn about an invalid API instead of crashing.

That said, yes indeed @drumm the problem is that you returned package names which were not expected in https://packages.drupal.org/8/security-advisories?packages[]=drupal/font... - We use the keys in the advisories object to map the advisories to a package, so returning others messes stuff up and overall looks dodgy to me.

It would be nice if you could fix and re-publish the API IMO, so I took a closer look and the way I see it drupal/fontawesome_iconpicker_widget itself has no vuln so it should not return anything. As that package requires drupal/fontawesome, Composer will install that and will request it from the API via https://packages.drupal.org/8/security-advisories?packages[]=drupal/font... if it is interested in that package's advisories, but the repo shouldn't try to resolve dependencies and return advisories for dependents of the project we ask for.

Does that make sense? Or is there another reason you returned that package?

The mismatched result was just a bug from Drupal core’s dependency resolution being on modules/themes, and Drupal projects can contain multiple modules or themes. I used the function we had for “get the Composer namepace of this module/theme” instead of the project that exactly matches.

https://packages.drupal.org/8/security-advisories?packages[]=drupal/font... will never have an advisory since it is part of the drupal/fontawesome package.

It should be doable to fix this in the next week or so.

This API should be better-behaved now.

naderman / Seldaek - can you think of any other testing to do before putting this back in https://packages.drupal.org/8/packages.json, along with removing query-all?

I don't expect any other problems, but to be honest I would not have expected the last one either so I guess try it and see :)

I plan to deploy the https://packages.drupal.org/8/packages.json change in the next hour.

This is now deployed.

@drumm this triggered another error for us - traced it back to https://packages.drupal.org/8/security-advisories?packages[]=drupal/para... which returns drupal/paragraphs - so it looks like the problem still exists or at least some of it.

#27: AFAICT that's accurate, because paragraphs_library is a submodule of paragraphs.

i.e.:

• https://www.drupal.org/project/paragraphs → = the project
• https://git.drupalcode.org/project/paragraphs/-/blob/8.x-1.x/paragraphs.... = the main module, which must be in the root of the repo
• https://git.drupalcode.org/project/paragraphs/-/blob/8.x-1.x/modules/par... = a submodule, which must be in <root>/modules/<submodulename/

(There are more submodules, see https://git.drupalcode.org/project/paragraphs/-/tree/8.x-1.x/modules.)

IOW: even if there is no vulnerability in drupal/paragraphs-paragraphs_library itself; it is part of drupal/paragraphs. Hence this seems accurate. (Note I do not know how exactly Drupal submodules are mapped onto Composer semantics. But I do know that it's not without challenges/contention 😅 See 🌱 Change how modules and submodule dependencies are handled to have more consistent namepsaces [DRAFT]. Active for example.)

I see, yes that seems like a bit of a mess, but I can see how it makes sense.

However, it should be returned with the key set to drupal/paragraphs-paragraphs_library and packageName probably set the same as well in the object.

Yep — it's a mess, but we had to figure out some mechanism to not completely disrupt the Drupal ecosystem 😅 (I was not involved, so I don't know any of the details. I just have … empathy for how we got to this place.)

However, it should be returned with the key set to drupal/paragraphs-paragraphs_library and packageName probably set the same as well in the object.

I can see this going in either direction, there is an argument for consistency both ways. But it sounds like that'd be a hard requirement, because Composer does some basic sanity checks that would otherwise fail (and Drupal is likely the only weird exception)?

Right at the very least the key in the top level object should not be an unexpected name we did not request, as that looks very wrong. If the packageName is still set to the one in the advisory i.e. drupal/paragraphs that sounds kinda reasonable to me as it says you wanted advisories for X and here is one but it references another package so be it.

I think it's important to add too, that these advisories for different packages must use different advisoryIds, so you should not return different names for the same advisory depending on how some API request arrives. Instead you really do need to define one advisory with its own id for each package that has a security issue. You can refer to the report that's the basis for all of them by its shared identifier in the sources remoteId section. So for submodules, you probably want to at least append the submodule name or something to the advisory id if not define a new numbering system for these.

Oof and looking at sources in your response. The "name" there is not the name of the advisory at the source, but rather the name of the source. So something like "Drupal" if you are the authority administrating those remoteIds.

Great, I think the information in #31 + #32 + #33 is what he'd need to push this forward. (I merely tried to be helpful here to accelerate timezone-induced delays 😄)

I backed out the api-url from security-advisories again. So it only has metadata: true now.

Since composer require drupal/paragraphs-paragraphs_library installs drupal/paragraphs as a dependency, https://packages.drupal.org/8/security-advisories?packages[]=drupal/para... should be returning nothing. Composer audit will catch the advisory when drupal/paragraphs is checked.

It looks like there was another problem when I smoke tested this after deployment.

I installed an old, unsupported version of Drupal with composer create-project drupal/recommended-project:^9 test-drupal

With api-url removed, now composer audit returns the advisory for https://www.drupal.org/sa-core-2024-001 → . It was missed after deployment yesterday.

https://packages.drupal.org/8/security-advisories?packages[]=drupal/core isn't returning the advisories it should.

I spotted a much more reliable DB table to map Composer namespaces to Drupal.org projects. And I added the special case for Drupal Core. This still needs a bit more testing before putting it back into https://packages.drupal.org/8/packages.json, which I'll plan to do toward the start of my day sometime this week.

This is deployed again. I reviewed all the examples from this issue, and everything looks good, as far as I can tell. The new way of getting advisories from composer names is much more straightforward and foolproof.

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