Skip to content
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.

Sign up for GitHub

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

Jump to bottom

🐛 Dependency-Pinning: only score detected ecosystems #3436

Merged
merged 47 commits into from
Sep 25, 2023
Merged

🐛 Dependency-Pinning: only score detected ecosystems #3436

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
2b2dd82
feat: Define if dependency is pinned or unpinned
gabibguti Feb 14, 2023
9638a92
refactor: Convert diff var types to pointer
gabibguti Feb 16, 2023
e497a20
fix: Pinned Dependency field type
gabibguti Feb 16, 2023
3f3dd80
feat: Count pinned and unpinned deps
gabibguti Aug 28, 2023
309158a
feat: Flag not applicable ecossystems
gabibguti Aug 28, 2023
14f69b1
feat: Score only applicable ecossystems
gabibguti Aug 28, 2023
6ff571b
feat: If no dependencies then create inconclusive score
gabibguti Aug 28, 2023
7b15634
test: GitHub Actions score and logs
gabibguti Aug 29, 2023
104f969
test: Pinned dependencies score
gabibguti Aug 29, 2023
5f67732
test: Ecossystems score and logs
gabibguti Aug 29, 2023
5691e0d
test: Remove deleted maxScore function test
gabibguti Aug 29, 2023
84757ea
test: Adding GitHub Actions dependencies to result
gabibguti Aug 29, 2023
5509c47
test: Update GitHub Actions result
gabibguti Aug 29, 2023
9a787c4
test: Update pip installs result
gabibguti Aug 29, 2023
423cac8
fix: Handle if nuget dependency is pinned or unpinned
gabibguti Apr 18, 2023
9056875
tests: Fix check warnings for unpinned dependencies
gabibguti Aug 29, 2023
c1a81fe
fix: Linter errors
gabibguti Aug 29, 2023
c8ee14a
fix: GitHub Actions pinned log
gabibguti Aug 29, 2023
7f7b89d
test: Fix "ossf-tests/scorecard-check-pinned-dependencies-e2e"
gabibguti Aug 29, 2023
a1b1781
Revert rename `asPointer` to `asStringPointer`
gabibguti Aug 31, 2023
5adf329
fix: Handle deps with parsing error and undefined pinning
gabibguti Aug 31, 2023
1ad766b
test: Delete unecessary test
gabibguti Aug 31, 2023
7a2a3a4
test: Add missing dep Location cases
gabibguti Aug 31, 2023
4412f0e
fix: Simplify Dockerfile pinned as name logic
gabibguti Aug 31, 2023
512659c
fix: If ecossystem is not found show debug log
gabibguti Aug 31, 2023
9731d8a
test: Fix e2e tests and more unit tests
gabibguti Aug 31, 2023
08bd85e
feat: Iterate all dependency types for final score
gabibguti Aug 31, 2023
e52be0b
feat: Proportional score
gabibguti Sep 4, 2023
29be827
fix: GHA weights in proportional score
gabibguti Sep 4, 2023
5dc6464
test: Fix scores and logs checking
gabibguti Sep 4, 2023
e8667cc
test: Fix e2e test
gabibguti Sep 4, 2023
407773c
refactor: Rename to ProportionalScoreWeighted
gabibguti Sep 6, 2023
a68194b
refactor: Var declarations to create proportional score
gabibguti Sep 6, 2023
aa23b15
fix: Remove unnecessary pointer
gabibguti Sep 6, 2023
e478307
fix: Dependencies priority declaration
gabibguti Sep 6, 2023
99f4dc6
fix: Ecosystem spelling
gabibguti Sep 13, 2023
7000eb5
fix: Handle 0 weight and 0 total when creating proportional weighted …
gabibguti Sep 13, 2023
6390493
fix: Revert -d flag identification change
gabibguti Sep 13, 2023
3187362
fix: npm ci command is npm download and is pinned
gabibguti Sep 13, 2023
2484ddc
fix: Linter errors
gabibguti Sep 13, 2023
24f3a8f
fix: Unexport error variable to other packages
gabibguti Sep 15, 2023
f839462
refactor: Simplify no score groups condition
gabibguti Sep 15, 2023
ca725e0
feat: Log proportion of dependencies pinned
gabibguti Sep 19, 2023
f6ccf91
test: Fix unit tests to include info logs
gabibguti Sep 19, 2023
9baf547
test: Fix e2e tests to include info logs
gabibguti Sep 19, 2023
b986c65
fix: Linter error
gabibguti Sep 19, 2023
2e3e9a5
Merge branch 'main' into fix/count-existing-ecossystems-for-pinned-deps
spencerschrock Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions checker/check_result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ const (
DetailDebug
)

// ErrSuccessTotal indicates a runtime error because number of success cases should
// errSuccessTotal indicates a runtime error because number of success cases should
// be smaller than the total cases to create a proportional score.
var ErrSuccessTotal = errors.New("unexpected number of success is higher than total")
var errSuccessTotal = errors.New("unexpected number of success is higher than total")

// CheckResult captures result from a check run.
//
Expand Down Expand Up @@ -119,14 +119,12 @@ func CreateProportionalScoreWeighted(scores ...ProportionalScoreWeighted) (int,
noScoreGroups := true
for _, score := range scores {
if score.Success > score.Total {
return InconclusiveResultScore, fmt.Errorf("%w: %d, %d", ErrSuccessTotal, score.Success, score.Total)
return InconclusiveResultScore, fmt.Errorf("%w: %d, %d", errSuccessTotal, score.Success, score.Total)
}
if score.Total != 0 {
noScoreGroups = false
} else {
// Group with 0 total, does not count for score
continue
if score.Total == 0 {
continue // Group with 0 total, does not count for score
}
noScoreGroups = false
if score.Weight != 0 {
allWeightsZero = false
}
Expand Down
22 changes: 17 additions & 5 deletions checks/evaluation/pinned_dependencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,12 @@ func PinningDependencies(name string, c *checker.CheckRequest,
var scores []checker.ProportionalScoreWeighted
// Go through all dependency types
// GitHub Actions need to be handled separately since they are not in pr
scores = append(scores, createScoreForGitHubActionsWorkflow(&wp)...)
scores = append(scores, createScoreForGitHubActionsWorkflow(&wp, dl)...)
// Only exisiting dependencies will be found in pr
// We will only score the ecosystem if there are dependencies
// This results in only existing ecosystems being included in the final score
for t := range pr {
logPinnedResult(dl, pr[t], string(t))
scores = append(scores, checker.ProportionalScoreWeighted{
Success: pr[t].pinned,
Total: pr[t].total,
Expand Down Expand Up @@ -180,17 +181,17 @@ func generateText(rr *checker.Dependency) string {
// Check if we are dealing with a GitHub action or a third-party one.
gitHubOwned := fileparser.IsGitHubOwnedAction(rr.Location.Snippet)
owner := generateOwnerToDisplay(gitHubOwned)
return fmt.Sprintf("%s %s not pinned by hash", owner, rr.Type)
return fmt.Sprintf("%s not pinned by hash", owner)
}

return fmt.Sprintf("%s not pinned by hash", rr.Type)
}

func generateOwnerToDisplay(gitHubOwned bool) string {
if gitHubOwned {
return "GitHub-owned"
return fmt.Sprintf("GitHub-owned %s", checker.DependencyUseTypeGHAction)
}
return "third-party"
return fmt.Sprintf("third-party %s", checker.DependencyUseTypeGHAction)
}

func addPinnedResult(rr *checker.Dependency, r *pinnedResult) {
Expand All @@ -208,11 +209,20 @@ func addWorkflowPinnedResult(rr *checker.Dependency, w *worklowPinningResult, is
}
}

func createScoreForGitHubActionsWorkflow(wp *worklowPinningResult) []checker.ProportionalScoreWeighted {
func logPinnedResult(dl checker.DetailLogger, p pinnedResult, name string) {
dl.Info(&checker.LogMessage{
Text: fmt.Sprintf("%3d out of %3d %s dependencies pinned", p.pinned, p.total, name),
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
})
}

func createScoreForGitHubActionsWorkflow(wp *worklowPinningResult, dl checker.DetailLogger,
) []checker.ProportionalScoreWeighted {
if wp.gitHubOwned.total == 0 && wp.thirdParties.total == 0 {
return []checker.ProportionalScoreWeighted{}
}
if wp.gitHubOwned.total != 0 && wp.thirdParties.total != 0 {
logPinnedResult(dl, wp.gitHubOwned, generateOwnerToDisplay(true))
logPinnedResult(dl, wp.thirdParties, generateOwnerToDisplay(false))
return []checker.ProportionalScoreWeighted{
{
Success: wp.gitHubOwned.pinned,
Expand All @@ -227,6 +237,7 @@ func createScoreForGitHubActionsWorkflow(wp *worklowPinningResult) []checker.Pro
}
}
if wp.gitHubOwned.total != 0 {
logPinnedResult(dl, wp.gitHubOwned, generateOwnerToDisplay(true))
return []checker.ProportionalScoreWeighted{
{
Success: wp.gitHubOwned.pinned,
Expand All @@ -235,6 +246,7 @@ func createScoreForGitHubActionsWorkflow(wp *worklowPinningResult) []checker.Pro
},
}
}
logPinnedResult(dl, wp.thirdParties, generateOwnerToDisplay(false))
return []checker.ProportionalScoreWeighted{
{
Success: wp.thirdParties.pinned,
Expand Down
49 changes: 25 additions & 24 deletions checks/evaluation/pinned_dependencies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ func Test_createScoreForGitHubActionsWorkflow(t *testing.T) {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
actual := createScoreForGitHubActionsWorkflow(&tt.r)
dl := scut.TestDetailLogger{}
actual := createScoreForGitHubActionsWorkflow(&tt.r, &dl)
diff := cmp.Diff(tt.scores, actual)
if diff != "" {
t.Errorf("createScoreForGitHubActionsWorkflow (-want,+got) %+v", diff)
Expand Down Expand Up @@ -289,7 +290,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfInfo: 7,
NumberOfDebug: 0,
},
},
Expand Down Expand Up @@ -340,7 +341,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 7,
NumberOfInfo: 0,
NumberOfInfo: 7,
NumberOfDebug: 0,
},
},
Expand All @@ -362,7 +363,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 5,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -384,7 +385,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 5,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand Down Expand Up @@ -412,7 +413,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -429,7 +430,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand Down Expand Up @@ -503,7 +504,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -520,7 +521,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -537,7 +538,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -554,7 +555,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -571,7 +572,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -588,7 +589,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -605,7 +606,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -627,7 +628,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -649,7 +650,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -668,7 +669,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -687,7 +688,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
Expand All @@ -713,7 +714,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 10,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -739,7 +740,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 0,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -765,7 +766,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 2,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand All @@ -791,7 +792,7 @@ func Test_PinningDependencies(t *testing.T) {
Error: nil,
Score: 8,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
},
Expand Down Expand Up @@ -826,12 +827,12 @@ func Test_generateOwnerToDisplay(t *testing.T) {
{
name: "returns GitHub if gitHubOwned is true",
gitHubOwned: true,
want: "GitHub-owned",
want: "GitHub-owned GitHubAction",
},
{
name: "returns GitHub if gitHubOwned is false",
gitHubOwned: false,
want: "third-party",
want: "third-party GitHubAction",
},
}
for _, tt := range tests {
Expand Down
6 changes: 3 additions & 3 deletions e2e/pinned_dependencies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
Error: nil,
Score: 2,
NumberOfWarn: 139,
NumberOfInfo: 0,
NumberOfInfo: 5,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand All @@ -76,7 +76,7 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
Error: nil,
Score: 2,
NumberOfWarn: 139,
NumberOfInfo: 0,
NumberOfInfo: 5,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand Down Expand Up @@ -112,7 +112,7 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
Error: nil,
Score: 2,
NumberOfWarn: 139,
NumberOfInfo: 0,
NumberOfInfo: 5,
NumberOfDebug: 0,
}
result := checks.PinningDependencies(&req)
Expand Down
Loading