pr comments

Signed-off-by: balteravishay <[email protected]>

probes/memorysafe/def.yml

@@ -38,4 +38,6 @@ ecosystem:
     - go
     - c#
   clients:
-    - github
+    - github  
+    - gitlab  
+    - localdir  

probes/memorysafe/impl.go

@@ -64,47 +64,49 @@ func init() {
 }
 
 func Run(raw *checker.CheckRequest) (found []finding.Finding, probeName string, err error) {
-	prominentLangs := getRepositoryLanguageChecks(raw)
+	prominentLangs, err := getLanguageChecks(raw)
+	if err != nil {
+		return nil, Probe, err
+	}
 	findings := []finding.Finding{}
-
 	for _, lang := range prominentLangs {
 		for _, langFunc := range lang.funcPointers {
-			if langFunc == nil {
-				raw.Dlogger.Warn(&checker.LogMessage{
-					Text: fmt.Sprintf("no function pointer found for language %s", lang.Desc),
-				})
-			}
 			langFindings, err := langFunc(raw)
 			if err != nil {
 				return nil, Probe, fmt.Errorf("error while running function for language %s: %w", lang.Desc, err)
 			}
 			findings = append(findings, langFindings...)
 		}
 	}
+	if len(findings) == 0 {
+		found, err := finding.NewWith(fs, Probe,
+			"All supported ecosystems follow memory safety best practices", nil, finding.OutcomeTrue)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *found)
+	}
 	return findings, Probe, nil
 }
 
-func getRepositoryLanguageChecks(raw *checker.CheckRequest) []languageMemoryCheckConfig {
+func getLanguageChecks(raw *checker.CheckRequest) ([]languageMemoryCheckConfig, error) {
 	langs, err := raw.RepoClient.ListProgrammingLanguages()
 	if err != nil {
-		raw.Dlogger.Warn(&checker.LogMessage{
-			Text: fmt.Sprintf("RepoClient retured error for ListProgrammingLanguages: %v", err),
-		})
-		return nil
+		return nil, fmt.Errorf("cannot get langs of repo: %w", err)
 	}
 	if len(langs) == 0 {
-		return []languageMemoryCheckConfig{}
+		return []languageMemoryCheckConfig{}, nil
 	}
 	if len(langs) == 1 && langs[0].Name == clients.All {
-		return getAllLanguages()
+		return getAllLanguages(), nil
 	}
 	ret := []languageMemoryCheckConfig{}
 	for _, language := range langs {
 		if lang, ok := languageMemorySafeSpecs[clients.LanguageName(strings.ToLower(string(language.Name)))]; ok {
 			ret = append(ret, lang)
 		}
 	}
-	return ret
+	return ret, nil
 }
 
 func getAllLanguages() []languageMemoryCheckConfig {
@@ -126,14 +128,7 @@ func checkGoUnsafePackage(client *checker.CheckRequest) ([]finding.Finding, erro
 	}, goCodeUsesUnsafePackage, &findings, client.Dlogger); err != nil {
 		return nil, err
 	}
-	if len(findings) == 0 {
-		found, err := finding.NewWith(fs, Probe,
-			"Golang code does not use the unsafe package", nil, finding.OutcomeTrue)
-		if err != nil {
-			return nil, fmt.Errorf("create finding: %w", err)
-		}
-		findings = append(findings, *found)
-	}
+
 	return findings, nil
 }
 
@@ -143,25 +138,25 @@ func goCodeUsesUnsafePackage(path string, content []byte, args ...interface{}) (
 		// panic if it is not correct type
 		panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
 	}
+	dl, ok := args[1].(checker.DetailLogger)
+	if !ok {
+		// panic if it is not correct type
+		panic(fmt.Sprintf("expected type checker.DetailLogger, got %v", reflect.TypeOf(args[1])))
+	}
 	fset := token.NewFileSet()
 	f, err := parser.ParseFile(fset, "", content, parser.ImportsOnly)
 	if err != nil {
-		dl, ok := args[1].(checker.DetailLogger)
-		if !ok {
-			// panic if it is not correct type
-			panic(fmt.Sprintf("expected type checker.DetailLogger, got %v", reflect.TypeOf(args[1])))
-		}
-
 		dl.Warn(&checker.LogMessage{
 			Text: fmt.Sprintf("malformed golang file: %v", err),
 		})
 		return true, nil
 	}
 	for _, i := range f.Imports {
 		if i.Path.Value == `"unsafe"` {
+			lineStart := uint(fset.Position(i.Pos()).Line)
 			found, err := finding.NewWith(fs, Probe,
 				"Golang code uses the unsafe package", &finding.Location{
-					Path: path,
+					Path: path, LineStart: &lineStart,
 				}, finding.OutcomeFalse)
 			if err != nil {
 				return false, fmt.Errorf("create finding: %w", err)
@@ -184,14 +179,6 @@ func checkDotnetAllowUnsafeBlocks(client *checker.CheckRequest) ([]finding.Findi
 	}, csProjAllosUnsafeBlocks, &findings, client.Dlogger); err != nil {
 		return nil, err
 	}
-	if len(findings) == 0 {
-		found, err := finding.NewWith(fs, Probe,
-			"C# code does not allow unsafe blocks", nil, finding.OutcomeTrue)
-		if err != nil {
-			return nil, fmt.Errorf("create finding: %w", err)
-		}
-		findings = append(findings, *found)
-	}
 	return findings, nil
 }
 
@@ -201,14 +188,14 @@ func csProjAllosUnsafeBlocks(path string, content []byte, args ...interface{}) (
 		// panic if it is not correct type
 		panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
 	}
+	dl, ok := args[1].(checker.DetailLogger)
+	if !ok {
+		// panic if it is not correct type
+		panic(fmt.Sprintf("expected type checker.DetailLogger, got %v", reflect.TypeOf(args[1])))
+	}
+
 	unsafe, err := csproj.IsAllowUnsafeBlocksEnabled(content)
 	if err != nil {
-		dl, ok := args[1].(checker.DetailLogger)
-		if !ok {
-			// panic if it is not correct type
-			panic(fmt.Sprintf("expected type checker.DetailLogger, got %v", reflect.TypeOf(args[1])))
-		}
-
 		dl.Warn(&checker.LogMessage{
 			Text: fmt.Sprintf("malformed csproj file: %v", err),
 		})

probes/memorysafe/impl_test.go

@@ -45,8 +45,14 @@ func Test_Run(t *testing.T) {
 			name:          "no languages",
 			repoLanguages: []clients.Language{},
 			filenames:     []string{},
-			expected:      []finding.Finding{},
-			err:           nil,
+			expected: []finding.Finding{
+				{
+					Probe:   Probe,
+					Message: "All supported ecosystems follow memory safety best practices",
+					Outcome: finding.OutcomeTrue,
+				},
+			},
+			err: nil,
 		},
 		// unimplemented languages
 		{
@@ -55,8 +61,14 @@ func Test_Run(t *testing.T) {
 				{Name: clients.Erlang, NumLines: 0},
 			},
 			filenames: []string{},
-			expected:  []finding.Finding{},
-			err:       nil,
+			expected: []finding.Finding{
+				{
+					Probe:   Probe,
+					Message: "All supported ecosystems follow memory safety best practices",
+					Outcome: finding.OutcomeTrue,
+				},
+			},
+			err: nil,
 		},
 		// golang
 		{
@@ -68,7 +80,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -85,7 +97,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -102,7 +114,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -125,7 +137,7 @@ func Test_Run(t *testing.T) {
 						Text:   "Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.\nGuidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)\nGuidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)",
 						Effort: 2,
 					},
-					Location: &finding.Location{Path: "testdata/unsafe.go"},
+					Location: &finding.Location{Path: "testdata/unsafe.go", LineStart: toUintPointer(4)},
 				},
 			},
 			err: nil,
@@ -148,7 +160,7 @@ func Test_Run(t *testing.T) {
 						Text:   "Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.\nGuidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)\nGuidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)",
 						Effort: 2,
 					},
-					Location: &finding.Location{Path: "testdata/unsafe.go"},
+					Location: &finding.Location{Path: "testdata/unsafe.go", LineStart: toUintPointer(4)},
 				},
 			},
 			err: nil,
@@ -171,7 +183,7 @@ func Test_Run(t *testing.T) {
 						Text:   "Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.\nGuidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)\nGuidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)",
 						Effort: 2,
 					},
-					Location: &finding.Location{Path: "testdata/unsafe.go"},
+					Location: &finding.Location{Path: "testdata/unsafe.go", LineStart: toUintPointer(4)},
 				},
 			},
 			err: nil,
@@ -186,7 +198,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -203,7 +215,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -220,7 +232,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -305,12 +317,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
-					Outcome: finding.OutcomeTrue,
-				},
-				{
-					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -328,12 +335,7 @@ func Test_Run(t *testing.T) {
 			expected: []finding.Finding{
 				{
 					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
-					Outcome: finding.OutcomeTrue,
-				},
-				{
-					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
+					Message: "All supported ecosystems follow memory safety best practices",
 					Outcome: finding.OutcomeTrue,
 				},
 			},
@@ -349,11 +351,6 @@ func Test_Run(t *testing.T) {
 				"testdata/unsafe.csproj",
 			},
 			expected: []finding.Finding{
-				{
-					Probe:   Probe,
-					Message: "Golang code does not use the unsafe package",
-					Outcome: finding.OutcomeTrue,
-				},
 				{
 					Probe:   Probe,
 					Message: "C# code allows the use of unsafe blocks",
@@ -385,12 +382,7 @@ func Test_Run(t *testing.T) {
 						Text:   "Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.\nGuidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)\nGuidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)",
 						Effort: 2,
 					},
-					Location: &finding.Location{Path: "testdata/unsafe.go"},
-				},
-				{
-					Probe:   Probe,
-					Message: "C# code does not allow unsafe blocks",
-					Outcome: finding.OutcomeTrue,
+					Location: &finding.Location{Path: "testdata/unsafe.go", LineStart: toUintPointer(4)},
 				},
 			},
 			err: nil,
@@ -413,7 +405,7 @@ func Test_Run(t *testing.T) {
 						Text:   "Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.\nGuidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)\nGuidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)",
 						Effort: 2,
 					},
-					Location: &finding.Location{Path: "testdata/unsafe.go"},
+					Location: &finding.Location{Path: "testdata/unsafe.go", LineStart: toUintPointer(4)},
 				},
 				{
 					Probe:   Probe,
@@ -469,12 +461,9 @@ func Test_Run_Error_ListProgrammingLanguages(t *testing.T) {
 	}).AnyTimes()
 	raw.RepoClient = mockRepoClient
 	raw.Dlogger = checker.NewLogger()
-	findings, _, err := Run(raw)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	if diff := cmp.Diff(findings, []finding.Finding{}, cmpopts.IgnoreUnexported(finding.Finding{})); diff != "" {
-		t.Error(diff)
+	_, _, err := Run(raw)
+	if err == nil {
+		t.Fatalf("expected error: %v", err)
 	}
 }
 
@@ -519,3 +508,7 @@ func Test_Run_Error_OnMatchingFileContentDo(t *testing.T) {
 		})
 	}
 }
+
+func toUintPointer(num uint) *uint {
+	return &num
+}