Code review guidelines, responsible disclosure and contributing guidelines

This commit is contained in:
Francisco Gindre 2021-09-24 17:38:19 -03:00
parent 0e9751dd2c
commit 8a0aff141b
3 changed files with 210 additions and 0 deletions

48
CODE_REVIEW_GUIDELINES.md Normal file
View File

@ -0,0 +1,48 @@
# Overall:
- Does the contribution referece an existing Issue?
- Are the requirements well defined?
# Specification:
- Are the requirements for the change well specified? Where are they documented? Bugfixes need a clear specification of the bug's cause and fix. (Small PRs might specify in a github ticket. Large changes may require stand-alone docs, or ZIPs.)
- Is there a documented test plan, which describes how to manually verify the change works on testnet prior to a release?
# Documentation:
- What is the "changelog" entry for this change? (All changes should include this.)
- Are there any changes which require updates to user-facing documentation? If so, does the new documentation make sense? (documentation should be placed in [docs/](/docs) folder of the repo)
# Testing:
- For non-minor PRs (up to the code reviewers and PR creator if this is needed), we require authors to perform a thorough self-review and self-test of the resulting code base, including use cases that might not be visibly affected by the introduced changes. Reviewers are not expected to run the changes locally, but are definitely encouraged to do so at their best judgement.
- When introducing modifications that affect the UI, screenshots might be provided in a BEFORE/AFTER fashion to speed up UI/UX requirement validation.
- Are there new tests that check all of the requirements? If it's a bugfix does it include new tests testing the bug triggering condition? (Do they fail before the fix and pass after the fix?)
- Do tests include edge cases, error conditions, and "negative case" tests to ensure the software is robust? Example: a function for verifying transaction signatures should include a bunch of tests for invalid signature cases.
- Do tests include all "logical test coverage" in addition to requirements-focused tests? Logical test coverage verifies behavior of the code that isn't obvious or implied by the requirements themselves.
- Have all of the automated tests been executed by CI? Read over the CI report to verify this. (Note: our CI system may not currently provide test results prior to review. We should change this! In the meantime, the reviewer should run the tests.)
- Is it clear that the test plan does actually test the new change? Does the test plan include any edge case / negative case / error conditions?
# Code Review:
- Does the code structure make sense? Does it follow existing conventions / frameworks of existing code, or is it introducing new abstractions / structure?
- Is the code style consistent with the [Swift Style Guide](SWIFTLINT.md)?
- Does every change make sense? Make sure to ask questions, even or especially "basic" coding questions ("what does this mean in Swift?") and domain-newbie questions ("what is the priority system, and why does this line change the priority?").
In summary, here's a checklist that summarizes what reviewers should be looking for when doing a Code Review [1]
- The code is well-designed.
- The functionality is good for the users of the code.
- Any UI changes are sensible and look good.
- Any parallel programming is done safely.
- The code isnt more complex than it needs to be.
- The developer isnt implementing things they might need in the future but dont know they need now or aren't using now.
- Code has appropriate unit tests.
- Tests are well-designed.
- The developer used clear names for everything.
- Comments are clear and useful, and mostly explain why instead of what.
- Code is appropriately documented (generally in g3doc).
- The code conforms to our style guides.
- Make sure to review every line of code youve been asked to review, look at the context, make sure youre improving code health, and compliment developers on good things that they do.
[[1] What to look for in a code review](https://google.github.io/eng-practices/review/reviewer/looking-for.html)

98
CONTRIBUTING.md Normal file
View File

@ -0,0 +1,98 @@
# Contributing Guidelines
This document contains information and guidelines about contributing to this project.
Please read it before you start participating.
**Topics**
* [Asking Questions](#asking-questions)
* [Reporting Security Issues](#reporting-security-issues)
* [Reporting Non Security Issues](#reporting-other-issues)
* [Developers Certificate of Origin](#developers-certificate-of-origin)
## Asking Questions
Questions are welcome! We encourage you to ask questions through GitHub issues.
Before doing so, please check that the project issues database doesn't already
include an answer to your question. Then open a new Issue and use the "Question"
label.
## Reporting Security Issues
If you have discovered an issue with this code that could present a security hazard or wish to discuss a sensitive issue with our security team, please contact security@z.cash [security.asc](https://z.cash/gpg-pubkeys/security.asc). Key fingerprint = AF85 0445 546C 18B7 86F9 2C62 88FB 8B86 D8B5 A68C
## Reporting Non Security Issues
A great way to contribute to the project
is to send a detailed issue when you encounter a problem.
We always appreciate a well-written, thorough bug report.
Check that the project issues database
doesn't already include that problem or suggestion before submitting an issue.
If you find a match, add a quick "+1" or "I have this problem too."
Doing this helps prioritize the most common problems and requests.
When reporting issues, please include the following:
* The Android API you're using
* The device you're targeting
* The full output of any stack trace or compiler error
* A code snippet that reproduces the described behavior, if applicable
* Any other details that would be useful in understanding the problem
This information will help us review and fix your issue faster.
## Pull Requests
We **love** pull requests!
All contributions _will_ be licensed under the MIT license.
Code/comments should adhere to the following rules:
* Every Pull request must have an Issue associated to it. PRs with not
associated with an Issue will be closed
* Code build and Code Lint must pass.
* Names should be descriptive and concise.
* Although they are not mandatory, PRs that include significant testing will be
prioritized.
* All enhancements and bug fixes need to be documented in the CHANGELOG.
* When writing comments, use properly constructed sentences, including
punctuation.
* When documenting APIs and/or source code, don't make assumptions or make
implications about race, gender, religion, political orientation or anything
else that isn't relevant to the project.
* Remember that source code usually gets written once and read often: ensure
the reader doesn't have to make guesses. Make sure that the purpose and inner
logic are either obvious to a reasonably skilled professional, or add a
comment that explains it.
## Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
- (a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
- (b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
- (c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
- (d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
This contribution guide is inspired on great projects like [AlamoFire](https://github.com/Alamofire/Foundation/blob/master/CONTRIBUTING.md) and [CocoaPods](https://github.com/CocoaPods/CocoaPods/blob/master/CONTRIBUTING.md)

64
RESPONSIBLE_DISCLOSURE.md Normal file
View File

@ -0,0 +1,64 @@
This page is copyright The Electric Coin Company, 2021. It is posted in order to conform to this standard: https://github.com/RD-Crypto-Spec/Responsible-Disclosure/tree/d47a5a3dafa5942c8849a93441745fdd186731e6
# Security Disclosures
## Receiving Disclosures
The Electric Coin Company is committed to working with researchers who submit security vulnerability notifications to us to resolve those issues on an appropriate timeline and perform a coordinated release, giving credit to the reporter if they would like.
Please submit issues to security@z.cash, using the following PGP key:
```
-----BEGIN PGP PUBLIC KEY BLOCK-----
mQENBFaegcoBCAC+G82ZBYYm1GoVn4dKa0WiLYD/Q+BuU89PS1X7A4eOOy8g9yS4
wJKMzB0AxFsH/t85P7pPZwHw3i2gmiJKeIqEGhEBL08D3id2u6ZyCnwDuWs0i6My
MXWTwK5shvE61ZI/KPbjemoOG6MPF5QdrouNqei2Vk+4RjbRCyyS0A59GQi2dNZX
BMwTnHnUZ5qi6T0RFelqJ3dE5Nc/UwJPdAcg71c3b3dMOHjaDBMPB6+fTLBeidV6
5B72nGO3eIYkMUNj+qCQmM/esRkmGmlDH/9WGMBOKCq7Yw3LyEoPOi5cba1m8SN2
xFlNzkUGrlVrwZMF+1UdjvN7BGDypA3Dr/STABEBAAG0JVpjYXNoIFNlY3VyaXR5
IFRlYW0gPHNlY3VyaXR5QHouY2FzaD6JATcEEwEIACEFAlaegcoCGwMFCwkIBwIG
FQgJCgsCBBYCAwECHgECF4AACgkQiPuLhti1poxlRAf/bZ6fhUby5bAbViAO4TzQ
yfbD0ksGeF8MHicPz7HqOYuXAE9GrKnVAOFptwRo94O+iRC5aXhW8OAP+38IWorv
gsAuag7Y8k0nlfNdrJRqqJpjyxtiuv+cd2o5dre8E9PVNE5IPv9qEJA4Zag3snmC
a+O4HAqKeXYddunFq2drLkTRlOwuFkGXJzwi3VSNVYCuuGyezFDuaD45ltmsXgid
jZSdnnc6L1BrEd9LVzahvFV0+fT4bNKQHQDk+f0RTnHed+m9NqAoC9K8ftPTQ/i4
5+W/dXJATztWDrJ7ZevHXGR+RAhMNsT1psvnQsJzMkUz1GMdQOtk4PfuZLGSIiTM
ErkBDQRWnoHKAQgAp+w+xsPJWFdadE6Ok1aZC0Lk1J9xU/cqX1aBlkwi5SynwOkV
Eg1xNHLJMelp13bgDjLRsvaMbsseaCVk3goNln4atNbZpqz6FoM/f8pJx52LFD0j
CFFOVUlGEF0h+KdFr+3JHI+mg+3ifXTD4Dajj4lpu8kR/FQjftcxyttByz01wLRO
sK5BDC856WzHXAJuX6TpX4sGJujzKoLXR5V0SkUopqn9g4aJGnWuNh4kyOQI6fd7
YZyPZhWDrXdgInCKAKAgq8r6hgSDMYFvmflp6+reCfeOe1VFF8q3Foio02YPIrQW
WjjH0w6nOvOKCEtxistz1sP6ZoYq4gR41LOwIQARAQABiQEfBBgBCAAJBQJWnoHK
AhsMAAoJEIj7i4bYtaaMiA4IALIy4xP/Btu86yT3b53t8cfYZddFSO8Nlg+y3EMu
1LchdSOpWgpXCvQ7d4ndWGsuBSmQ+jaRwU2UIkq2iIxf5cg63dJz9grAcF6MXCrO
t5BSowFC4m3RFaEaG6G6SjDVIA0ZEdEMFd9Gzc1ikqbVLyNuJXKmzz0evvbAJgVO
D0nht5ifwLjQxM4olvYHUwtT0wPhniH69ghFo8LQiMgncjaukDzbgANiuj07QYy/
DlzhQUsp1qZvqZnVKqUJ3lFb86b6zoqoRNiUnvP9JB2v3kLG0T39UlcXUFnZJ/4H
CDHCrwSovQRMHtoOWZijBNobO2y1d0FkUpzNlNw44Ssw0Vo=
=6GYS
-----END PGP PUBLIC KEY BLOCK-----
```
## Sending Disclosures
In the case where we become aware of security issues affecting other projects that has never affected Zcash, our intention is to inform those projects of security issues on a best effort basis.
In the case where we fix a security issue in Zcash that also affects the following neighboring projects, our intention is to engage in responsible disclosures with them as described in https://github.com/RD-Crypto-Spec/Responsible-Disclosure, subject to the deviations described in the section at the bottom of this document.
## Bilateral Responsible Disclosure Agreements
We have set up agreements with the following neighboring projects to share vulnerability information, subject to the deviations described in the next section.
Specifically, we have agreed to engage in responsible disclosures for security issues affecting this repository and others we maintain for the same platform with the following contacts:
- Unstoppable wallet via PGP email to security.hs@protonmail.com
- Nighthawk wallet via their disclosure policy @ https://github.com/nighthawk-apps/nighthawk-wallet-android/blob/master/README.md
## Deviations from the Standard
Zcash is a technology that provides strong privacy. Notes are encrypted to their destination, and then the monetary base is kept via zero-knowledge proofs intended to only be creatable by the real holder of Zcash. If this fails, and a counterfeiting bug results, that counterfeiting bug might be exploited without any way for blockchain analyzers to identify the perpetrator or which data in the blockchain has been used to exploit the bug. Rollbacks before that point, such as have been executed in some other projects in such cases, are therefore impossible.
The standard describes reporters of vulnerabilities including full details of an issue, in order to reproduce it. This is necessary for instance in the case of an external researcher both demonstrating and proving that there really is a security issue, and that security issue really has the impact that they say it has - allowing the development team to accurately prioritize and resolve the issue.
In the case of a counterfeiting bug, however, just like in CVE-2019-7167, we might decide not to include those details with our reports to partners ahead of coordinated release, so long as we are sure that they are vulnerable.