Skip to content

Commit

Permalink
🐛 Add npm installs to Pinned-Dependencies score (ossf#2960)
Browse files Browse the repository at this point in the history 
* feat: Add npm install to pinned dependencies score

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Fix pinned dependencies evaluation tests

Considering the new npm installs dependencies in Pinned-Dependencies score, there are some changes. Now, all tests generate one more Info log for "npm installs are all pinned". Also, for "various wanrings" test, the total score has to weight now 6 scores instead of 5. The new score counts 10 for actionScore, 0 for dockerFromScore, 0 for dockerDownloadScore, 0 for scriptScore, 0 for pipScore and 10 for npm score, which gives us 20/6~=3.

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Fix pinned dependencies e2e tests

Considering the new npm installs dependencies in Pinned-Dependencies score, there are some changes. The repo being tested, ossf-tests/scorecard-check-pinned-dependencies-e2e, has third-party GitHub actions pinned, no npm installs, and all other dependencies types are unpinned. This gives us 8 for actionScore, 10 for npmScore and 0 for all other scores. Previously the total score was 8/5~=1, and now the total score is 18/6=3. Also, since there are no npm installs, there's one more Info log for "npm installs are pinned".

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Fix typo

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Unpinned npm install score

When having one unpinned npm install and all other dependencies pinned, the score should be 50/6~=8. Also, it should raise 1 warning for the unpinned npm install, 6 infos saying the other dependency types are pinned (2 for GHAs, 2 for dockerfile image and downdloads, 1 for script downdloads and 1 for pip installs), and 0 debug logs since the npm install dependency does not have an error message.

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Undefined npm install score

When an error happens to parse a npm install dependency, the error/debug message is saved in "Msg" field. In this case, we were not able to define if the npm install is pinned or not. This dependency is classified as pinned undefined. We treat such cases as pinned cases, so it logs as Info that npm installs are all pinned and counts the score as 10. Then, the final score makes it to 10 as well. Since it logs the error/debug message, the Debug log goes to 1.

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Fix typo

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Fix "validate various warnings and info" test

Considering the new npm installs dependencies in Pinned-Dependencies score, there are some changes. Now, all tests generate one more Info log for "npm installs are all pinned". Also, this test total score has to weight now 6 scores instead of 5. The new score counts 10 for actionScore, 0 for dockerFromScore, 0 for dockerDownloadScore, 0 for scriptScore, 0 for pipScore and 10 for npm score, which gives us 20/6~=3.

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* fix: npm dependencies pinned log

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* test: Remove test of error when parsing an npm dependency

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

---------

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>
Signed-off-by: Allen Shearin <allen.p.shearin@gmail.com>
  • Loading branch information
@gabibguti @ashearin
gabibguti authored and ashearin committed Nov 13, 2023
1 parent 204dbfe commit e351e3e
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 18 deletions.
18 changes: 17 additions & 1 deletion checks/evaluation/pinned_dependencies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,22 @@ func PinningDependencies(name string, c *checker.CheckRequest,
return checker.CreateRuntimeErrorResult(name, err)
}

// Npm installs.
npmScore, err := createReturnForIsNpmInstallPinned(pr, dl)
if err != nil {
return checker.CreateRuntimeErrorResult(name, err)
}

// Scores may be inconclusive.
actionScore = maxScore(0, actionScore)
dockerFromScore = maxScore(0, dockerFromScore)
dockerDownloadScore = maxScore(0, dockerDownloadScore)
scriptScore = maxScore(0, scriptScore)
pipScore = maxScore(0, pipScore)
npmScore = maxScore(0, npmScore)

score := checker.AggregateScores(actionScore, dockerFromScore,
dockerDownloadScore, scriptScore, pipScore)
dockerDownloadScore, scriptScore, pipScore, npmScore)

if score == checker.MaxResultScore {
return checker.CreateMaxScoreResult(name, "all dependencies are pinned")
Expand Down Expand Up @@ -260,6 +267,15 @@ func createReturnForIsPipInstallPinned(pr map[checker.DependencyUseType]pinnedRe
dl)
}

// Create the result for npm install commands.
func createReturnForIsNpmInstallPinned(pr map[checker.DependencyUseType]pinnedResult,
dl checker.DetailLogger,
) (int, error) {
return createReturnValues(pr, checker.DependencyUseTypeNpmCommand,
"npm installs are pinned",
dl)
}

func createReturnValues(pr map[checker.DependencyUseType]pinnedResult,
t checker.DependencyUseType, infoMsg string,
dl checker.DetailLogger,
Expand Down
38 changes: 27 additions & 11 deletions checks/evaluation/pinned_dependencies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 6,
NumberOfInfo: 7,
NumberOfDebug: 1,
},
},
Expand All @@ -132,12 +132,12 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 6,
NumberOfWarn: 1,
NumberOfInfo: 4,
NumberOfInfo: 5,
NumberOfDebug: 1,
},
},
{
name: "various wanrings",
name: "various warnings",
dependencies: []checker.Dependency{
{
Location: &checker.File{},
Expand All @@ -158,9 +158,9 @@ func Test_PinningDependencies(t *testing.T) {
},
expected: scut.TestReturn{
Error: nil,
Score: 2,
Score: 3,
NumberOfWarn: 3,
NumberOfInfo: 2,
NumberOfInfo: 3,
NumberOfDebug: 1,
},
},
Expand All @@ -176,7 +176,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 8,
NumberOfWarn: 1,
NumberOfInfo: 5,
NumberOfInfo: 6,
NumberOfDebug: 0,
},
},
Expand All @@ -193,7 +193,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 6,
NumberOfInfo: 7,
NumberOfDebug: 1,
},
},
Expand All @@ -203,12 +203,12 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 6,
NumberOfInfo: 7,
NumberOfDebug: 0,
},
},
{
name: "Validate various wanrings and info",
name: "Validate various warnings and info",
dependencies: []checker.Dependency{
{
Location: &checker.File{},
Expand All @@ -229,12 +229,28 @@ func Test_PinningDependencies(t *testing.T) {
},
expected: scut.TestReturn{
Error: nil,
Score: 2,
Score: 3,
NumberOfWarn: 3,
NumberOfInfo: 2,
NumberOfInfo: 3,
NumberOfDebug: 1,
},
},
{
name: "unpinned npm install",
dependencies: []checker.Dependency{
{
Location: &checker.File{},
Type: checker.DependencyUseTypeNpmCommand,
},
},
expected: scut.TestReturn{
Error: nil,
Score: 8,
NumberOfWarn: 1,
NumberOfInfo: 6,
NumberOfDebug: 0,
},
},
}

for _, tt := range tests {
Expand Down
12 changes: 6 additions & 6 deletions e2e/pinned_dependencies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
}
expected := scut.TestReturn{
Error: nil,
Score: 1,
Score: 3,
NumberOfWarn: 139,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand All @@ -74,9 +74,9 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
}
expected := scut.TestReturn{
Error: nil,
Score: 1,
Score: 3,
NumberOfWarn: 139,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand Down Expand Up @@ -110,9 +110,9 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
}
expected := scut.TestReturn{
Error: nil,
Score: 1,
Score: 3,
NumberOfWarn: 139,
NumberOfInfo: 1,
NumberOfInfo: 2,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand Down

0 comments on commit e351e3e

Please sign in to comment.