-
Notifications
You must be signed in to change notification settings - Fork 391
Import old Wasmtime security advisories #2254
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
For maintainers here, two questions:
|
IMO (as a recently joined maintainer):
We don't need a PR-per-advisory, I'm fine with adding many of these in the same PR or even in the same commit.
I think that's probably okay, as long as there's a decent title summarizing the issue. I'd suggest reducing the boilerplate:
Maybe like:
|
@alexcrichton you can submit several advisories in a single PR, and the ID assigner will make a single PR to assign them all incrementing IDs |
Ok thanks! When adding multiple advisories should I use Happy to include the renames, but I'm a bit hesitant to import too much if this will end up pinging many other contributors asking for licensing information. Is that something standard to require? Even for just a link to GHA? Is there something we can do to avoid the licensing back-and-forth? |
That sounds about right, let's try it?
The full guidance:
Are wasmtime vulnerabilities assigned a CVE? If so, using just the title + link should be fine assuming that the title is also used in the CVE. Otherwise, it looks like there are just 5 GHSA authors for wasmtime, so getting those releases should be fairly straightforward? (Also presumably if they were employed by a single organization at the time they submitted, it might be enough to get a copy right release from that organization?) |
It'll take me some time to get around to filling out old advisories. I'm happy to have this merged as-is and I can follow-up with a later batch, but if y'all would prefer this can hold off on merging until I've imported other old advisories. All Wasmtime vulnerabilities are assigned a CVE, and for example this advisory has a CVE listed in it which you can see on the GHSA as well. The CVE itself doesn't seem to be a 1:1 copy of what's on GHSA. Basically if licensing is going to be a drag on this I think we'll probably skip the step of manually publishing here and wait for the auto-importing tool to work (or something like that). If we can sidestep licensing issues by having the report be a URL that's fine though from our end, the goal here is to just get notifications out to users who exclusively use |
@alexcrichton you can also add a |
Ok found some time on my hands and I've uploaded other advisories we've had. I've also added |
Ah alas the |
We should probably make sure the underlying tools are aligned on their understanding of what constitutes a valid new advisory filename. I'll see if I can have a look at that sometime soon (or maybe you can make a PR to the rustsec repo?). |
To my knowledge it’s files named RUSTSEC-0000-0000.md |
Here's a proposal to improve the tooling to make it possible to have multiple advisories for the same crate in one PR: |
I tweaked CI a bit to test out |
This needs to wait for rustsec-admin 0.8.10, which is blocked on getting a new rustsec release out. |
@alexcrichton sorry for the delay -- the updated rustsec-admin should be integrated into the CI workflow here now, so can you rebase on top of current main? |
[Wasmtime] recently got a [request] to have our security advisories published on the RustSec database as well. We've got a few old advisories on here but we haven't been keeping up-to-date with later advisories. In lieu of automatic imports from GitHub to RustSec we figured we'd in the interim manually fill in some fields. In this PR I'm back-filling security advisories we've had in Wasmtime into the RustSec database here. The oldest advisory here is 3 years old and the goal is to have this serve as a template for importing future advisories that Wasmtime gets. It's not expected for this to cause any churn or undue warnings but instead is intended to bring RustSec up-to-date with the advisories we have for this crate. [Wasmtime]: https://crates.io/crates/wasmtime [request]: bytecodealliance/wasmtime#10344
6ddd7fb
to
4cdfb82
Compare
Sure thing, rebased and updated the commit message/PR title too |
Wasmtime recently got a request to have our security advisories
published on the RustSec database as well. We've got a few old
advisories on here but we haven't been keeping up-to-date with later
advisories. In lieu of automatic imports from GitHub to RustSec we
figured we'd in the interim manually fill in some fields.
In this PR I'm back-filling security advisories we've had in Wasmtime
into the RustSec database here. The oldest advisory here is 3 years old
and the goal is to have this serve as a template for importing future
advisories that Wasmtime gets. It's not expected for this to cause any
churn or undue warnings but instead is intended to bring RustSec
up-to-date with the advisories we have for this crate.