From 93ae40f8c981984d2359410baaf813f8120f37c0 Mon Sep 17 00:00:00 2001 From: balteravishay Date: Wed, 5 Feb 2025 21:19:02 +0000 Subject: [PATCH] pr comments Signed-off-by: balteravishay --- probes/memorysafe/def.yml | 4 +- probes/memorysafe/impl.go | 75 +++++++++++++------------------- probes/memorysafe/impl_test.go | 79 ++++++++++++++++------------------ 3 files changed, 70 insertions(+), 88 deletions(-) diff --git a/probes/memorysafe/def.yml b/probes/memorysafe/def.yml index 24d6bba82ac..f8f5c17596b 100644 --- a/probes/memorysafe/def.yml +++ b/probes/memorysafe/def.yml @@ -38,4 +38,6 @@ ecosystem: - go - c# clients: - - github + - github + - gitlab + - localdir diff --git a/probes/memorysafe/impl.go b/probes/memorysafe/impl.go index 5b2d6f20735..bb8593542f7 100644 --- a/probes/memorysafe/impl.go +++ b/probes/memorysafe/impl.go @@ -64,16 +64,13 @@ 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) @@ -81,22 +78,27 @@ func Run(raw *checker.CheckRequest) (found []finding.Finding, probeName string, 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 { @@ -104,7 +106,7 @@ func getRepositoryLanguageChecks(raw *checker.CheckRequest) []languageMemoryChec 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,15 +138,14 @@ 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), }) @@ -159,9 +153,10 @@ func goCodeUsesUnsafePackage(path string, content []byte, args ...interface{}) ( } 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), }) diff --git a/probes/memorysafe/impl_test.go b/probes/memorysafe/impl_test.go index 691d1cb759f..26f3911f8b7 100644 --- a/probes/memorysafe/impl_test.go +++ b/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 +}