Skip to content
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

Documentation: Add notices about reporting security flaws #15260

Merged
merged 2 commits into from Jan 9, 2024

Conversation

amotl
Copy link
Member

@amotl amotl commented Dec 22, 2023

About

This patch follows the suggestion by @Tu0Laj1 at 1, in order to improve the guidance about reporting security flaws. The SECURITY.rst file has been derived from 2. The SECURITY.md file has been derived from 3, as suggested on the review of the patch.

Footnotes

  1. https://community.cratedb.com/t/mitigations-for-reported-vulnerability/1676/7

  2. https://github.com/dec0dOS/amazing-github-template/blob/main/%7B%7Bcookiecutter.repo_slug%7D%7D/docs/SECURITY.md

  3. https://github.com/electron/electron/blob/main/SECURITY.md

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

Thank you! I think it looks good, not approving for now, I'll wait for the opinion of others.

Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

Why not use and mention Github's Security Advisories functionality?
I think the policy file must be in the markdown format then, using a .md suffix.
I'd prefer mentioning this as the first option and contacting us via mail as the second one.
Even that I totally agree on responsible disclosure, the current phrasing sounds a bit harsh to me, could be understand like we want to hide reported vulnerability issues.
I think a mix between this text and e.g. https://github.com/electron/electron/blob/main/SECURITY.md would be great.

README.rst Outdated Show resolved Hide resolved
SECURITY.rst Outdated Show resolved Hide resolved
@amotl
Copy link
Member Author

amotl commented Dec 22, 2023

Thanks for your feedback and suggestions. I will adjust the files and content as you suggested. If you want to bring it in early, feel free to take over.

@amotl amotl force-pushed the amo/security-reporting branch 2 times, most recently from bbb7858 to fca7a20 Compare December 22, 2023 11:41
Copy link
Member

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

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

I'd add the SECURITY.md to https://github.com/crate/.github to have it enabled in all repositories.

And I'd also prefer the wording of https://github.com/electron/electron/blob/main/SECURITY.md

@amotl
Copy link
Member Author

amotl commented Dec 22, 2023

Hi again. I've updated the patch according to your suggestions, and will also prepare a similar one for https://github.com/crate/.github. Let me know about any kinds of wording adjustments you would like to see. 🙇

@amotl
Copy link
Member Author

amotl commented Dec 22, 2023

Other than the SECURITY.md, I haven't been able to discover the spot where to adjust the reporting template text. When comparing the "Description" texts below, I also like the template from Electron much better.

Ours: https://github.com/crate/crate-python/security/advisories/new
Electron: https://github.com/electron/electron/security/advisories/new

@mfussenegger mfussenegger added the ready-to-merge Let Mergify merge the PR once approved and checks pass label Jan 9, 2024
@mergify mergify bot merged commit 0039090 into master Jan 9, 2024
13 checks passed
@mergify mergify bot deleted the amo/security-reporting branch January 9, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Let Mergify merge the PR once approved and checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants