-
Notifications
You must be signed in to change notification settings - Fork 104
feat: add support for Drupal advisory database #372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3a85418 to
5bbc56b
Compare
docs/schema.md
Outdated
| <li>How to contribute: <code>TBD</code></li> | ||
| <li>Source URL: <code>TBD</code></li> | ||
| <li>OSV Formatted URL: <code>TBD</code></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the ETA on having this info pulled together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <li>How to contribute: <code>TBD</code></li> | |
| <li>Source URL: <code>TBD</code></li> | |
| <li>OSV Formatted URL: <code>TBD</code></li> | |
| <li>How to contribute: <a href="https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquette/reporting-a-security-issue">https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquette/reporting-a-security-issue</a></li> | |
| <li>Source URL: <code>https://www.drupal.org/security/</code></li> | |
| <li>OSV Formatted URL: <code>https://github.com/ackama/drupal-advisory-database/blob/main/advisories/>module</>ID<.json</code></li> |
It looks like the Source URL could also be listed as https://www.drupal.org/<ID> except that a user must remove the leading D from any ID, i.e. DSA-CONTRIB-2023-012 => sa-contrib-2023-012. Is there a better source URL with direct ID matching?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I've used is currently the most direct source URL available - I think it would be good in the long-run to explore aligning everything but right now it's a bit of a chicken and egg situation since Drupal advisories cannot be represented correctly in OSV format until this is landed.
Also fwiw the "D" is a placeholder for the final database prefix since we cannot use the prefix until its part of the spec, so going forward the advisories in our database will be i.e. DRUPAL-CONTRIB-2023-012 (or possibly DRUPAL-2023-012, but I'm leaning towards the former to make it easier for consumers since we can just say "replace the DRUPAL with SA")
I've opened DrupalSecurityTeam/drupal-advisory-database#114 doing that rename
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the ETA on having this info pulled together?
it looks like you've beaten me to it already, so thanks for that 😄
docs/schema.md
Outdated
| | `CRAN` | The R package ecosystem. The `name` is an R package name. | | ||
| | `crates.io` | The crates.io ecosystem for Rust; the `name` field is a crate name. | | ||
| | `Debian` | The Debian package ecosystem; the `name` is the name of the source package. The ecosystem string might optionally have a `:<RELEASE>` suffix to scope the package to a particular Debian release. `<RELEASE>` is a numeric version specified in the [Debian distro-info-data](https://debian.pages.debian.net/distro-info-data/debian.csv). For example, the ecosystem string "Debian:7" refers to the Debian 7 (wheezy) release. | | ||
| | `Drupal` | The Drupal CMS ecosystem, for packages sourced from the Drupal composer repository. The ecosystem implies https://packages.drupal.org/8 as the source repository, unless a :7 suffix is present in which case the repository is https://packages.drupal.org/7 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is https://packages.drupal.org the canonical URL for the most recent package repo? I.e. does it currently imply https://packages.drupal.org/8 but in the future could imply https://packages.drupal.org/9, etc.? If so, I would suggest changing the language here to make it more future proof by not assuming 8 as the latest.
I'd also suggest a few tweaks to make it more closely align with the language from other ecosystems that support suffixing.
For example:
| | `Drupal` | The Drupal CMS ecosystem, for packages sourced from the Drupal composer repository. The ecosystem implies https://packages.drupal.org/8 as the source repository, unless a :7 suffix is present in which case the repository is https://packages.drupal.org/7 | | |
| | `Drupal` | The Drupal CMS ecosystem, for packages sourced from the [Drupal composer repository](https://packages.drupal.org). An optional numeric `:<RELEASE>` suffix may be used to scope the package to a specific repository. For example, `Drupal:7` refers to the [https://packages.drupal.org/7](https://packages.drupal.org/7) repository.| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going forward there will not be a 9+ as Drupal as transitioning to using semver for their modules (https://www.drupal.org/docs/develop/git/git-for-drupal-project-maintainers/release-naming-conventions#semver-transition) and in our discussions so far no one has seemed particularly worried about this happening in future which doesn't surprise me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks for explaining that.
ecosystems.json
Outdated
| "CRAN": "The R package ecosystem. The `name` is an R package name.", | ||
| "crates.io": "The crates.io ecosystem for Rust; the `name` field is a crate name.", | ||
| "Debian": "The Debian package ecosystem; the `name` is the name of the source package. The ecosystem string might optionally have a `:<RELEASE>` suffix to scope the package to a particular Debian release. `<RELEASE>` is a numeric version specified in the [Debian distro-info-data](https://debian.pages.debian.net/distro-info-data/debian.csv). For example, the ecosystem string \"Debian:7\" refers to the Debian 7 (wheezy) release.", | ||
| "Drupal": "The Drupal CMS ecosystem, for packages sourced from the Drupal composer repository. The ecosystem implies https://packages.drupal.org/8 as the source repository, unless a :7 suffix is present in which case the repository is https://packages.drupal.org/7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same suggestion as above
|
Thanks for all the work and discussion here! Similar to #378 we should discuss the URL before merging this (it looks like there might be some other open questions as well). |
5b8f278 to
66875e0
Compare
Signed-off-by: Gareth Jones <[email protected]>
|
@chrisbloom7 @another-rex this should be good to proceed now |
Signed-off-by: Gareth Jones <[email protected]>
Signed-off-by: Gareth Jones <[email protected]>
| <li>OSV Formatted URL: <code>https://github.com/DrupalSecurityTeam/drupal-advisory-database/blob/main/advisories/>module</>ID<.json</code></li> | ||
| </ul> | ||
| </td> | ||
| </tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what all needs to be here, but this content looks good to me. Thanks!
This is the prefix we're going with per ossf/osv-schema#372
~Pending ossf/osv-schema#372 and friends~
The Drupal Advisory Database is a database maintained by Ackama on behalf of the Drupal community that provides Drupal security advisories in OSV format to allow them to be ingested by osv.dev.
While Drupal packages are installed and managed using Composer, they are (mostly) sourced from a dedicated repository rather than the Packagist repository; since these packages are all within the
drupal/namespace which is owned by the Drupal community, it's been agreed that it is fine to still use the Packagist rather than introduce a new one.This means that existing tools like
osv-scannerand libraries likeosv-scalibrshould "just work" with the advisories in this database.For the database prefix, the community are decided to use
DRUPALas that is straightforward and matches what other has been proposed in other tools likedependency-track, which replaces the "SA" prefix used by advisories published on drupal.org so advisory ids can be easily mapped to their original advisory by just replacingDRUPALwithSA.Discussion on this can be found here and here