Draft: Add guidelines for library reviews
What does this MR do?
Our code review guidelines do not provide instructions on how to review code libraries (Gems, Node.js modules etc.). This MR closes this gap.
GitLab Renovate Bot does remind reviewers that Node.js modules need to be reviewed for Security, Performance, and Stability. However, it's up to the individual review to decide how to review these criteria.
The goal of this MR is to give reviewers clear guidelines on how to review libraries for security. By following these guidelines, we hope that reviews can be performed more efficiently and are performed to a common standard, without introducing undue friction to the review process.
Related issues are:
- https://gitlab.com/gitlab-com/gl-security/security-research/sec-research/-/issues/15
- https://gitlab.com/gitlab-com/gl-security/engineering-and-research/automation-team/automation/-/issues/188
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team