-
Notifications
You must be signed in to change notification settings - Fork 60.5k
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
Reorder sections to make it more clear to the reader that requesting a CVE number is optional #10210
Reorder sections to make it more clear to the reader that requesting a CVE number is optional #10210
Conversation
…a CVE number is optional At the moment the publication doc leads with the topic of requesting a CVE number. This PR moves things around and adds language to imply that a CVE number is not required in order to publish a GHSA. The impetus for this PR comes from an out of band conversation with the rustsec community and some confusion around GHSA publication.
Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
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.
This is awesome, I left one suggestion though it might be a bit wordy. I think this PR could go out as is. Thanks @darakian
content/code-security/security-advisories/publishing-a-security-advisory.md
Outdated
Show resolved
Hide resolved
…y-advisory.md Co-authored-by: Robert Schultheis <rschultheis@github.com>
@darakian |
@@ -92,6 +76,22 @@ Publishing a security advisory deletes the temporary private fork for the securi | |||
|
|||
{% data reusables.repositories.github-reviews-security-advisories %} | |||
|
|||
## Requesting a CVE identification number (Optional) | |||
|
|||
Github can provide a CVE number if needed and if the software is not under the scope of another CVE Numbering Authority (CNA). Anyone with admin permissions to a security advisory can request a CVE identification number for the security advisory. If the advisory already has a CVE number, then that can be input in the advisory form. |
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.
👋 @darakian, thanks heaps for this PR! I agree that if the step is optional, then it should be moved lower and marked as such.
🤔 Most of the added content in this paragraph is already provided in the next paragraph in the {% data reusables.repositories.request-security-advisory-cve-id %}
reusable. You can see the content of that resuable file here. You can see a built stage of this branch here.
I would recommend sticking with the current wording in the reusable, or modifying the reusable directly.
Github can provide a CVE number if needed and if the software is not under the scope of another CVE Numbering Authority (CNA). Anyone with admin permissions to a security advisory can request a CVE identification number for the security advisory. If the advisory already has a CVE number, then that can be input in the advisory form. | |
Anyone with admin permissions to a security advisory can request a CVE identification number for the security advisory. |
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.
Thanks for the tip. I was unaware of what those {%...%}
patterns were. I've pushed a commit which removes the leading text from the base doc and alters the reusable. Let me know what you think @lucascosti
@rschultheis can I get a second review on the updated content?
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.
this all looks good to me, though I defer to @lucascosti and docs team in general. I think this does make it more clear that a CVE is optional which is the goal.
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.
Thanks, @darakian! I've added a few wording suggestions.
🤔 I'm also having second thoughts about moving the "Requesting a CVE identification number" section below the "Publishing" one. If someone wants to have a CVE, does a CVE need to be assigned before an advisory is published?
If so, we'd probably want the articles and sections to be in the order of the workflow. e.g. Draft -> request CVE -> publish.
I think adding the (Optional)
to the section title now clearly indicates that it is not required to publish an advisory.
data/reusables/repositories/request-security-advisory-cve-id.md
Outdated
Show resolved
Hide resolved
data/reusables/repositories/request-security-advisory-cve-id.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Lucas Costi <lucascosti@users.noreply.github.com>
Co-authored-by: Lucas Costi <lucascosti@users.noreply.github.com>
No. A CVE can be requested for the advisory either during the initial advisory publication step or later on.
Those look good to 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.
No. A CVE can be requested for the advisory either during the initial advisory publication step or later on.
Awesome, thanks for confirming! I'll merge this in 🚀
…re-clearly-optional
Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues ⚡ |
At the moment the publication doc leads with the topic of requesting a CVE number.
This PR moves things around and adds language to imply that a CVE number is not required in order to publish a GHSA.
The impetus for this PR comes from an out of band conversation with the rustsec community and some confusion around GHSA publication.
Why:
Closes #10209
What's being changed:
Minor reordering of sections and more additional language.
Check off the following:
Writer impact (This section is for GitHub staff members only):