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

RFC - Security Process Update #13

Merged
merged 5 commits into from
Aug 20, 2019
Merged

Conversation

j01tz
Copy link
Member

@j01tz j01tz commented Jul 20, 2019

Initial WIP for an RFC describing changes to improve Grin's Security Process by adopting a community vulnerability disclosure standard.

Rendered link to RFC document
Tracking issue: mimblewimble/grin-pm#178

@lehnberg lehnberg added the core Related to core team label Jul 20, 2019
@lehnberg
Copy link
Contributor

Thanks for submitting! I can be shepherd for this one. Does it make sense to then assign it to me?

Also: @j01tz, is this ready for a first review by the community, or are you still working on it?

@j01tz
Copy link
Member Author

j01tz commented Jul 20, 2019

@lehnberg that sounds good, thanks. I'd like to maybe take one more pass today then I'll mark as ready for review.

@j01tz
Copy link
Member Author

j01tz commented Jul 20, 2019

I went back and forth on adding the canaries section. It may be overkill but I thought it was at least worth mentioning for consideration. Set as ready for review now, thanks!

@j01tz j01tz marked this pull request as ready for review July 20, 2019 18:47
@lehnberg lehnberg self-assigned this Jul 22, 2019
@lehnberg lehnberg changed the title [WIP] RFC - Security Process Update RFC - Security Process Update Jul 22, 2019
Copy link
Member

@quentinlesceller quentinlesceller left a comment

Choose a reason for hiding this comment

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

Overall 👍. Minor comments/questions.

text/0000-security-process.md Outdated Show resolved Hide resolved
text/0000-security-process.md Outdated Show resolved Hide resolved
Copy link
Contributor

@lehnberg lehnberg left a comment

Choose a reason for hiding this comment

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

👍 in general, nice work on this. See my minor comments and suggestions.

text/0000-security-process.md Outdated Show resolved Hide resolved
text/0000-security-process.md Outdated Show resolved Hide resolved

Likewise 'Chain Splits' is not defined well enough and would probably be better
tracked and managed outside of the formal security policy, or with a sub-team,
at least until the language and tooling are better defined and stable.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually not sure we have even started work on that Chain Split monitoring tool (@antiochp, @yeastplume do you know?). For sure it doesn't belong in here in any case, and might be better to just say it's moved into a Github Issue for /grin repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated with a statement that the section would be moved to a github issue but I have not yet created and linked the issue. I'll wait to hear from others before I add more details here.

The tool would be nice to have but perhaps challenging with current resource level. A github issue sounds fine to me for now if it is something that makes sense to keep on the radar.

text/0000-security-process.md Outdated Show resolved Hide resolved
@j01tz
Copy link
Member Author

j01tz commented Aug 1, 2019

I refactored a lot of the Reference-level explanation section to attempt to add more clarity and added a bunch of relevant links to sections in the standard.

The feedback so far is very much appreciated, please keep it coming!

Copy link
Contributor

@lehnberg lehnberg left a comment

Choose a reason for hiding this comment

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

Great work @j01tz, thanks for incorporating my feedback, this looks good to me. 👍

@lehnberg lehnberg merged commit 07f01a9 into mimblewimble:master Aug 20, 2019
@lehnberg
Copy link
Contributor

lehnberg commented Aug 20, 2019

🎉 Wohooo! This RFC has now been merged! 🤸‍♀️
Tracking issue: mimblewimble/grin-pm#178

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants