Skip to content

Add OSV information#232

Open
S4mB3ckS wants to merge 2 commits into
DrupalSecurityTeam:mainfrom
S4mB3ckS:add-OSV-information
Open

Add OSV information#232
S4mB3ckS wants to merge 2 commits into
DrupalSecurityTeam:mainfrom
S4mB3ckS:add-OSV-information

Conversation

@S4mB3ckS

Copy link
Copy Markdown

node_type = project.get('type').split('_', maxsplit=1)[-1]

# Exception for : SA-CONTRIB-2019-074, SA-CONTRIB-2024-022
if "for drupal " in node_type:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of these modules does include "for Drupal" which is a little odd. If this is not appropriate to have in the OSV summary then it seems OK to remove.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the problem. I hesitated for a long time about the format for the summary.

This resulted in the following :
BAT online reservations for Drupal module for Drupal - Access Bypass - SA-CONTRIB-2019-074

It might be more appropriate to use this type of formatting; it will avoid repetition :
BAT online reservations for Drupal (Module) - Access Bypass - SA-CONTRIB-2019-074

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh I think only the middle part is appropriate for the summary, as the other parts are already encoded in the advisory and would typically be displayed already as they're critical to doing anything with the advisory.

And then I'm not sure if the middle section is something that would stand well on its own...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The summary can also be seen as the title or subject of the advisory email, even if some elements are already in the description.

Like this :

Sélection_1083

But no problem, i understand your opinion.

@G-Rath

G-Rath commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

👋 thanks for the interest in improving our database!

I'm not sure how valuable it is having both the summary and details, though looking at the generated advisories locally with this I will say the result does seem more consistent than I was expecting given the transformations we're doing.

I'm also not sure if it's appropriate to use composer for the PURL for the similar reason to why we use the Packagist ecosystem with the optional suffix: I suspect composer may be referring specifically to packages hosted on packagist.org at least by default, which most Drupal packages are not.

If you're ok waiting a couple of weeks I might run this by the OSV team to check what their recommendations are

@S4mB3ckS

S4mB3ckS commented Jun 20, 2026

Copy link
Copy Markdown
Author

It may be more relevant to use the purl information only for packages that are in the ecosystem "Packagist" :

if composer_package_name in drupal_packages_available_on_packagist:
  purl = f'pkg:composer/{composer_package_name.lower()}'

@G-Rath

G-Rath commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Of which there's only like two that have vulns here and one of those is "deprecated" in favour of the Drupal repo version, so I don't think it'd be useful 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants