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

github: improve security policy documentation #4130

Merged
merged 3 commits into from
Dec 22, 2020

Conversation

howardjohn
Copy link
Contributor

Fixes #4128

This is largely inspired by https://github.com/envoyproxy/envoy. For an
example of the end result and how it shows up in the Github UI, check
out https://github.com/envoyproxy/envoy/issues/new/choose (comes from
the ISSUE_TEMPLATE change) and
https://github.com/envoyproxy/envoy/security/policy.

Fixes grpc#4128

This is largely inspired by https://github.com/envoyproxy/envoy. For an
example of the end result and how it shows up in the Github UI, check
out https://github.com/envoyproxy/envoy/issues/new/choose (comes from
the ISSUE_TEMPLATE change) and
https://github.com/envoyproxy/envoy/security/policy.
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Interesting. Envoy also has this in their bug template; perhaps we should do something similar:

If you are reporting any crash or any potential security issue, do not open an issue in this repo. Please report the issue via emailing envoy-security@googlegroups.com where the issue will be triaged appropriately.

@@ -0,0 +1,3 @@
# Security Policy
Copy link
Member

Choose a reason for hiding this comment

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

Did you intend to put this in the root directory instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can go in either and still work (see https://github.com/istio/istio/security/policy). So whatever you prefer

Copy link
Member

Choose a reason for hiding this comment

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

Aha. Let's put it in the root directory so it's more visible. Thank you.

@howardjohn
Copy link
Contributor Author

Interesting. Envoy also has this in their bug template; perhaps we should do something similar:

One other thing, Envoy has a very clear stance that ALL crashes should be privately reported. I wasn't sure of gRPCs stance on that so I left out the language, but can add it back if desired.

@dfawley
Copy link
Member

dfawley commented Dec 22, 2020

One other thing, Envoy has a very clear stance that ALL crashes should be privately reported. I wasn't sure of gRPCs stance on that so I left out the language, but can add it back if desired.

I'm not sure our stance, either. 😄 I'll ask around. For now we can leave that language out -- I was just referring to the general idea of a comment in the issue template explaining that potential security issues shouldn't be filed as github issues.

@dfawley dfawley added no release notes Type: Documentation Documentation or examples labels Dec 22, 2020
@dfawley
Copy link
Member

dfawley commented Dec 22, 2020

IIUC, this will result in two new buttons under "new issue": one saying "Report a Security Vulnerability" and one saying "Potentional security vulnerability". I am thinking we shouldn't include the ISSUE_TEMPLATE/config.yml change in that case.

@dfawley dfawley changed the title Improve security policy documentation github: improve security policy documentation Dec 22, 2020
@howardjohn
Copy link
Contributor Author

IIUC, this will result in two new buttons under "new issue": one saying "Report a Security Vulnerability" and one saying "Potentional security vulnerability". I am thinking we shouldn't include the ISSUE_TEMPLATE/config.yml change in that case.

Yes, I think that is right. I think the "Report a Security Vulnerability" one is new, I don't recall seeing that previously. In any event, looks like just SECURITY.md is sufficient

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Yes, I think that is right. I think the "Report a Security Vulnerability" one is new, I don't recall seeing that previously. In any event, looks like just SECURITY.md is sufficient

Sounds good, thank you for investigating this and for the PR.

@dfawley dfawley merged commit 66c1393 into grpc:master Dec 22, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Documentation Documentation or examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a clear security/vulnerability disclosure process
2 participants