From 260f937cb4e17f275f88e43335671edb378289d5 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 19 Jul 2018 17:46:34 -0400 Subject: [PATCH 1/2] doc: add req for sec wg review in some cases Some recent dicsussions point to it being a good idea to require reviews from the security-wg for security related PRs. See https://github.com/nodejs/node/issues/21766 Add this requirement to the collaborator guide. --- COLLABORATOR_GUIDE.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index 818fd7cc967443..b447d8f1dd0498 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -20,6 +20,7 @@ - [Breaking Changes to Internal Elements](#breaking-changes-to-internal-elements) - [When Breaking Changes Actually Break Things](#when-breaking-changes-actually-break-things) - [Reverting commits](#reverting-commits) + - [Additions to the Cryptography and Security APIs](#additions-to-the-cryptography-and-security-apis) - [Introducing New Modules](#introducing-new-modules) - [Deprecations](#deprecations) - [Involving the TSC](#involving-the-tsc) @@ -378,6 +379,16 @@ multiple commits. Commit metadata and the reason for the revert should be appended. Commit message rules about line length and subsystem can be ignored. A Pull Request should be raised and approved like any other change. +### Additions to the Cryptography and Security APIs + +Semver-minor commits that add or change cryptograpy/security APIs should be +treated with extra care. Due to the potential impact, it is important that +these APIs be constructed to reduce the potential for incorrect +usage. + +These commits must have an approval from at least one member from the +[security working group](https://github.com/nodejs/security-wg). + ### Introducing New Modules Semver-minor commits that introduce new core modules should be treated with From a4850f41f822e10b3db72eb9930438adef7bc6b1 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Wed, 29 Aug 2018 14:58:23 -0400 Subject: [PATCH 2/2] squash: address comments --- COLLABORATOR_GUIDE.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index b447d8f1dd0498..4272521a566b11 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -381,13 +381,21 @@ A Pull Request should be raised and approved like any other change. ### Additions to the Cryptography and Security APIs -Semver-minor commits that add or change cryptograpy/security APIs should be -treated with extra care. Due to the potential impact, it is important that -these APIs be constructed to reduce the potential for incorrect -usage. +Semver-minor commits that add or change cryptograpy APIs and Security APIs +should be treated with extra care. Due to the potential impact, it is +important that these APIs be constructed to reduce the potential for +incorrect usage. -These commits must have an approval from at least one member from the -[security working group](https://github.com/nodejs/security-wg). +Semver-minor commits changing or adding cryptography or security APIs +should be made visible to the crypto team and the security working group +through an @nodejs/security-wg and @nodejs/crypto mention in the PR. + +For Semver-minor commits changing cryptography APIs, they must have +an approval from at least one member from the crypto team. + +For Semver-minor commits changing Security API's(other than those +related to cryptography) must have an approval from at least +one member from the [security working group](https://github.com/nodejs/security-wg). ### Introducing New Modules