-
-
Notifications
You must be signed in to change notification settings - Fork 130
CII-Best-Practices for Nodejs: Gold level #956
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
Open
UlisesGascon
wants to merge
25
commits into
nodejs:main
Choose a base branch
from
UlisesGascon:feat/best-practices-gold
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
5d3fba7
feat: copied passing criterial Questions and Answers
UlisesGascon d936238
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 92588d0
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon ba689e7
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 871db48
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 58a0411
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 19aab9f
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon e0fd2db
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon d36c6e0
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 5a25aa4
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 702f6d3
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon af5c87c
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 58b4364
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 5e75801
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 0d60f62
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 676d31e
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 1feeaa8
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 4934f98
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 66c2f34
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon da57efe
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon 0cfdca9
Update tools/ossf_best_practices/gold_criteria.md
UlisesGascon f639ecb
docs: added questions context and consolidate the current responses
UlisesGascon 3f496a8
docs: add commit hash reference for the context links
UlisesGascon 91f35e7
docs: update responses and references
UlisesGascon 10f6219
docs: update OpenSSF Best practices Gold criteria
UlisesGascon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
docs: update responses and references
- Loading branch information
commit 91f35e74f87b95f1ce0f36f21bd660154a64831c
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,7 @@ Context: | |
|
||
> The project MUST have a "bus factor" of 2 or more. (URL required) | ||
|
||
**Met** | ||
**Met. https://github.com/nodejs/node/blob/main/GOVERNANCE.md** | ||
|
||
Context: | ||
- [CII Best Practices: Basics](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-basics-1) | ||
|
@@ -42,7 +42,7 @@ Context: | |
|
||
> The project MUST have at least two unassociated significant contributors. | ||
|
||
**Met** | ||
**Met. https://github.com/nodejs/node/blob/main/GOVERNANCE.md** | ||
|
||
Context: | ||
- [CII Best Practices: Basics](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-basics-1) | ||
|
@@ -53,12 +53,13 @@ Context: | |
|
||
> The project MUST include a copyright statement in each source file, identifying the copyright holder (e.g., the [project name] contributors). | ||
|
||
**Meet** | ||
**Unmet** | ||
|
||
Context: | ||
- [CII Best Practices: Basics](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-basics-1) | ||
- [CII Best Practices: Copyright Per File](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#copyright_per_file) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307391551) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1187) | ||
|
||
> The project MUST include a license statement in each source file. This MAY be done by including the following inside a comment near the beginning of each file: SPDX-License-Identifier: [SPDX license expression for project](https://spdx.dev/ids/#how) | ||
|
||
|
@@ -67,6 +68,7 @@ Context: | |
Context: | ||
- [CII Best Practices: Basics](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-basics-1) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307392811) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1187) | ||
|
||
# Change Control | ||
|
||
|
@@ -90,7 +92,7 @@ Context: | |
|
||
> The project MUST require two-factor authentication (2FA) for developers for changing a central repository or accessing sensitive data (such as private vulnerability reports). This 2FA mechanism MAY use mechanisms without cryptographic mechanisms such as SMS, though that is not recommended. | ||
|
||
**Met** | ||
**Met. Node.js org has enabled 2FA for all the members, see: https://github.com/openjs-foundation/security-collab-space/issues/94#issuecomment-1874627417** | ||
|
||
Context: | ||
- [CII Best Practices: Change Control](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-change-control-1) | ||
|
@@ -119,7 +121,7 @@ Context: | |
|
||
> The project MUST have at least 50% of all proposed modifications reviewed before release by a person other than the author, to determine if it is a worthwhile modification and free of known issues which would argue against its inclusion | ||
|
||
**Met. Currently the repo is monitored against the OSSF Scorecard, where this is checked every 2 weeks in the Security Team regular meetings. See: https://github.com/nodejs/security-wg/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/tools/ossf_scorecard/report.md** | ||
**Met. The process is documented: https://github.com/nodejs/node/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/doc/contributing/pull-requests.md#reviewing-pull-requests and we use additional tools like the OSSF Scorecard to monitor it** | ||
|
||
Context: | ||
- [CII Best Practices: Quality](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-quality-1) | ||
|
@@ -157,21 +159,24 @@ Context: | |
|
||
> The project MUST have FLOSS automated test suite(s) that provide at least 90% statement coverage if there is at least one FLOSS tool that can measure this criterion in the selected language. | ||
|
||
**Met. This is part of the CI Checks in place** | ||
**Met. Report available in https://app.codecov.io/gh/nodejs/node** | ||
|
||
Context: | ||
- [CII Best Practices: Quality](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-quality-1) | ||
- [CII Best Practices: Test Statement Coverage 90%](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#test_statement_coverage90) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307405014) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1187) | ||
|
||
|
||
> The project MUST have FLOSS automated test suite(s) that provide at least 80% branch coverage if there is at least one FLOSS tool that can measure this criterion in the selected language. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to check it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created this issue to follow up with the discussion. #1188 |
||
|
||
**Met. This is part of the CI Checks in place** | ||
**Met. Report available in https://app.codecov.io/gh/nodejs/node** | ||
|
||
Context: | ||
- [CII Best Practices: Quality](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-quality-1) | ||
- [CII Best Practices: Test Branch Coverage 80%](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#test_branch_coverage80) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307405888) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1188) | ||
|
||
|
||
# Security | ||
|
@@ -188,6 +193,8 @@ Context: | |
- [CII Best Practices: Security](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-security-1) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307415866) | ||
- [See related question in Silver Criteria](/tools/ossf_best_practices/silver_criteria.md) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1189) | ||
|
||
|
||
> The software produced by the project MUST, if it supports or uses TLS, support at least TLS version 1.2. Note that the predecessor of TLS was called SSL. If the software does not use TLS, select "not applicable" (N/A). | ||
|
||
|
@@ -208,6 +215,7 @@ Context: | |
Context: | ||
- [CII Best Practices: Security](https://github.com/coreinfrastructure/best-practices-badge/blob/a51ed45fdcd8e2959781a86929f561521ac2e0e0/docs/other.md#upgrade-security-1) | ||
- [Team Discussion](https://github.com/nodejs/security-wg/pull/956#discussion_r1307413951) | ||
- [Issue to follow up](https://github.com/nodejs/security-wg/issues/1190) | ||
|
||
## Other security issues | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we met the 90% percentage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've no idea if https://app.codecov.io/gh/nodejs/node is able to show statement coverage (it's showing line coverage).
https://github.com/nodejs/node/actions/workflows/coverage-linux.yml?query=branch%3Amain is reporting 95.5% statement coverage in the most recent run for JS code (via c8, see the "Report JS" twisty) but unfortunately no summary/easily readable numbers for the C++ code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created this issue to follow up with the discussion. #1188