From 5432bcd36579be6d34808df9cf126803377aa305 Mon Sep 17 00:00:00 2001 From: Michelangelo Mori Date: Wed, 20 Nov 2024 18:33:52 +0100 Subject: [PATCH] Migrate trusty eval engine to Trusty v2 API. This change migrates `trusty` evaluation engine to use Trusty v2 API. Most of the changes apply to the intermediate representation we recently added to decouple Trusty from Minder, but some additional changes were required due to some fields becoming optional and nullable. Note: this change is again meant to be bug-compatible with the current evaluation engine, so we treat the lack of `"score"` as a score of `0`, thus triggering false positives as done by the current engine. The idea is to build on top of this to fix known issues of the engine before migrating it to Data Sources. Fixes stacklok/minder-stories#77 --- go.mod | 2 +- go.sum | 2 + internal/engine/eval/trusty/actions.go | 16 +- internal/engine/eval/trusty/trusty.go | 231 +++++++++++++-------- internal/engine/eval/trusty/trusty_test.go | 31 +-- 5 files changed, 171 insertions(+), 111 deletions(-) diff --git a/go.mod b/go.mod index a423ed075d..eda4766bef 100644 --- a/go.mod +++ b/go.mod @@ -70,7 +70,7 @@ require ( github.com/spf13/viper v1.19.0 github.com/sqlc-dev/pqtype v0.3.0 github.com/stacklok/frizbee v0.1.4 - github.com/stacklok/trusty-sdk-go v0.2.2 + github.com/stacklok/trusty-sdk-go v0.2.3-0.20241120172703-3643fb488d5e github.com/stretchr/testify v1.9.0 github.com/styrainc/regal v0.29.2 github.com/thomaspoignant/go-feature-flag v1.38.0 diff --git a/go.sum b/go.sum index 366473b651..fcb9436831 100644 --- a/go.sum +++ b/go.sum @@ -1021,6 +1021,8 @@ github.com/stacklok/frizbee v0.1.4 h1:00v6/2HBmwzNdOyVAP4e1isOeUAIWTlb5eggoNUpHm github.com/stacklok/frizbee v0.1.4/go.mod h1:rFA90VkGFYLb7qCiUniAihmkgXfZAj2BnfF6jR8Csro= github.com/stacklok/trusty-sdk-go v0.2.2 h1:55B2DrneLYZXxBaNEeyRMGac7mj+pFbrKomx/hSxUyI= github.com/stacklok/trusty-sdk-go v0.2.2/go.mod h1:BbXNVVT32meuxyJuO4pmXRzVPjc/9AXob2sBbOPiKpk= +github.com/stacklok/trusty-sdk-go v0.2.3-0.20241120172703-3643fb488d5e h1:4fTGYQPNLex5VT4c4S7AifMJqUiM/r1B+J5qXUJMSFI= +github.com/stacklok/trusty-sdk-go v0.2.3-0.20241120172703-3643fb488d5e/go.mod h1:QR01jLW/yfwcXY38dwDpgeEjVc2MAR1LycH1fXtoSXs= github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/engine/eval/trusty/actions.go b/internal/engine/eval/trusty/actions.go index 48969503a8..bf92abf91f 100644 --- a/internal/engine/eval/trusty/actions.go +++ b/internal/engine/eval/trusty/actions.go @@ -299,7 +299,7 @@ func (sph *summaryPrHandler) generateSummary() (string, error) { malicious = append(malicious, maliciousTemplateData{ templatePackageData: packageData, Summary: alternative.trustyReply.Malicious.Summary, - Details: preprocessDetails(alternative.trustyReply.Malicious.Details), + Details: alternative.trustyReply.Malicious.Details, }) continue } @@ -324,7 +324,7 @@ func (sph *summaryPrHandler) generateSummary() (string, error) { // (2) we don't suggest malicious packages, I // suggest getting rid of this check // altogether. - if altData.Score != nil && *altData.Score <= lowScorePackages[alternative.Dependency.Name].Score { + if altData.Score != nil && *altData.Score != 0 && *altData.Score <= lowScorePackages[alternative.Dependency.Name].Score { continue } @@ -333,9 +333,11 @@ func (sph *summaryPrHandler) generateSummary() (string, error) { Ecosystem: altData.PackageType, PackageName: altData.PackageName, TrustyURL: altData.TrustyURL, - Score: *altData.Score, }, } + if altData.Score != nil { + altPackageData.templatePackageData.Score = *altData.Score + } dep := lowScorePackages[alternative.Dependency.Name] dep.Alternatives = append(dep.Alternatives, altPackageData) @@ -428,8 +430,12 @@ func newSummaryPrHandler( }, nil } -func preprocessDetails(s string) string { - scanner := bufio.NewScanner(strings.NewReader(s)) +func preprocessDetails(s *string) string { + if s == nil { + return "" + } + + scanner := bufio.NewScanner(strings.NewReader(*s)) text := "" for scanner.Scan() { if strings.HasPrefix(scanner.Text(), "#") { diff --git a/internal/engine/eval/trusty/trusty.go b/internal/engine/eval/trusty/trusty.go index aa62b192fb..7cec843bb2 100644 --- a/internal/engine/eval/trusty/trusty.go +++ b/internal/engine/eval/trusty/trusty.go @@ -12,10 +12,8 @@ import ( "strings" "github.com/rs/zerolog" - trusty "github.com/stacklok/trusty-sdk-go/pkg/v1/client" - trustytypes "github.com/stacklok/trusty-sdk-go/pkg/v1/types" - "golang.org/x/text/cases" - "golang.org/x/text/language" + trusty "github.com/stacklok/trusty-sdk-go/pkg/v2/client" + trustytypes "github.com/stacklok/trusty-sdk-go/pkg/v2/types" "google.golang.org/protobuf/reflect/protoreflect" "github.com/mindersec/minder/internal/constants" @@ -39,7 +37,7 @@ const ( type Evaluator struct { cli provifv1.GitHub endpoint string - client *trusty.Trusty + client trusty.Trusty } // NewTrustyEvaluator creates a new trusty evaluator @@ -296,105 +294,158 @@ type alternative struct { func getDependencyScore( ctx context.Context, - trustyClient *trusty.Trusty, + trustyClient trusty.Trusty, dep *pbinternal.PrDependencies_ContextualDependency, ) (*trustyReport, error) { // Call the Trusty API - resp, err := trustyClient.Report(ctx, &trustytypes.Dependency{ - Name: dep.Dep.Name, - Version: dep.Dep.Version, - Ecosystem: trustytypes.Ecosystem(dep.Dep.Ecosystem), - }) + packageType := dep.Dep.Ecosystem.AsString() + input := &trustytypes.Dependency{ + PackageName: dep.Dep.Name, + PackageVersion: &dep.Dep.Version, + PackageType: &packageType, + } + + respSummary, err := trustyClient.Summary(ctx, input) + if err != nil { + return nil, fmt.Errorf("failed getting summary: %w", err) + } + + respPkg, err := trustyClient.PackageMetadata(ctx, input) + if err != nil { + return nil, fmt.Errorf("failed getting package metadata: %w", err) + } + + respAlternatives, err := trustyClient.Alternatives(ctx, input) if err != nil { - return nil, fmt.Errorf("failed to send request: %w", err) + return nil, fmt.Errorf("failed getting alternatives: %w", err) } - res := makeTrustyReport(dep, resp) + respProvenance, err := trustyClient.Provenance(ctx, input) + if err != nil { + return nil, fmt.Errorf("failed getting provenance: %w", err) + } + + res := makeTrustyReport(dep, + respSummary, + respPkg, + respAlternatives, + respProvenance, + ) return res, nil } func makeTrustyReport( dep *pbinternal.PrDependencies_ContextualDependency, - resp *trustytypes.Reply, + respSummary *trustytypes.PackageSummaryAnnotation, + respPkg *trustytypes.TrustyPackageData, + respAlternatives *trustytypes.PackageAlternatives, + respProvenance *trustytypes.Provenance, ) *trustyReport { res := &trustyReport{ - PackageName: dep.Dep.Name, - PackageVersion: dep.Dep.Version, - PackageType: dep.Dep.Ecosystem.AsString(), - TrustyURL: makeTrustyURL(dep.Dep.Name, strings.ToLower(dep.Dep.Ecosystem.AsString())), - Score: resp.Summary.Score, - IsDeprecated: resp.PackageData.Deprecated, - IsArchived: resp.PackageData.Archived, - ActivityScore: getValueFromMap[float64](resp.Summary.Description, "activity"), - ProvenanceScore: getValueFromMap[float64](resp.Summary.Description, "provenance"), - } - - res.ScoreComponents = makeScoreComponents(resp.Summary.Description) - res.Alternatives = makeAlternatives(dep.Dep.Ecosystem.AsString(), resp.Alternatives.Packages) - - if getValueFromMap[bool](resp.Summary.Description, "malicious") { - res.Malicious = &malicious{ - Summary: resp.PackageData.Malicious.Summary, - Details: preprocessDetails(resp.PackageData.Malicious.Details), - } + PackageName: dep.Dep.Name, + PackageVersion: dep.Dep.Version, + PackageType: dep.Dep.Ecosystem.AsString(), + TrustyURL: makeTrustyURL(dep.Dep.Name, strings.ToLower(dep.Dep.Ecosystem.AsString())), } - res.Provenance = makeProvenance(resp.Provenance) + addSummaryDetails(res, respSummary) + addMetadataDetails(res, respPkg) + + res.ScoreComponents = makeScoreComponents(respSummary.Description) + res.Alternatives = makeAlternatives(dep.Dep.Ecosystem.AsString(), respAlternatives.Packages) + + if respSummary.Description.Malicious { + res.Malicious = makeMaliciousDetails(respPkg.Malicious) + } + + res.Provenance = makeProvenance(respProvenance) return res } -func makeScoreComponents(descr map[string]any) []scoreComponent { +func addSummaryDetails(res *trustyReport, resp *trustytypes.PackageSummaryAnnotation) { + if resp == nil { + return + } + + res.Score = resp.Score + res.ActivityScore = resp.Description.Activity + res.ProvenanceScore = resp.Description.Provenance +} + +func addMetadataDetails(res *trustyReport, resp *trustytypes.TrustyPackageData) { + if resp == nil { + return + } + + res.IsDeprecated = resp.IsDeprecated != nil && *resp.IsDeprecated + res.IsArchived = resp.Archived != nil && *resp.Archived +} + +func makeScoreComponents(resp trustytypes.SummaryDescription) []scoreComponent { scoreComponents := make([]scoreComponent, 0) - if descr == nil { - return scoreComponents - } - - caser := cases.Title(language.Und, cases.NoLower) - for l, v := range descr { - switch l { - case "activity": - l = "Package activity" - case "activity_repo": - l = "Repository activity" - case "activity_user": - l = "User activity" - case "provenance_type": - l = "Provenance" - case "typosquatting": - if f, ok := v.(float64); ok && f > 5.0 { - // skip typosquatting entry - continue - } - l = "Typosquatting" - v = "⚠️ Dependency may be trying to impersonate a well known package" - } + // activity scores + if resp.Activity != 0 { + scoreComponents = append(scoreComponents, scoreComponent{ + Label: "Package activity", + Value: resp.Activity, + }) + } + if resp.ActivityRepo != 0 { + scoreComponents = append(scoreComponents, scoreComponent{ + Label: "Repository activity", + Value: resp.ActivityRepo, + }) + } + if resp.ActivityUser != 0 { + scoreComponents = append(scoreComponents, scoreComponent{ + Label: "User activity", + Value: resp.ActivityUser, + }) + } - // Note: if none of the cases above match, we still - // add the value to the list along with its - // capitalized label. + // provenance information + if resp.ProvenanceType != nil { + scoreComponents = append(scoreComponents, scoreComponent{ + Label: "Provenance", + Value: string(*resp.ProvenanceType), + }) + } + // typosquatting information + if resp.TypoSquatting != 0 && resp.TypoSquatting <= 5.0 { scoreComponents = append(scoreComponents, scoreComponent{ - Label: fmt.Sprintf("%s%s", caser.String(l[0:1]), l[1:]), - Value: v, + Label: "Typosquatting", + Value: "⚠️ Dependency may be trying to impersonate a well known package", }) } + // Note: in the previous implementation based on Trusty v1 + // API, if new fields were added to the `"description"` field + // of a package they were implicitly added to the table of + // score components. + // + // This was possible because the `Description` field of the go + // struct was defined as `map[string]any`. + // + // This is not the case with v2 API, so we need to keep track + // of new measures being added to the API. + return scoreComponents } func makeAlternatives( ecosystem string, - trustyAlternatives []trustytypes.Alternative, + trustyAlternatives []*trustytypes.PackageBasicInfo, ) []alternative { alternatives := []alternative{} for _, alt := range trustyAlternatives { alternatives = append(alternatives, alternative{ PackageName: alt.PackageName, PackageType: ecosystem, - Score: &alt.Score, + Score: alt.Score, TrustyURL: makeTrustyURL(alt.PackageName, ecosystem), }) } @@ -402,29 +453,42 @@ func makeAlternatives( return alternatives } +func makeMaliciousDetails( + maliciousInfo *trustytypes.PackageMaliciousPayload, +) *malicious { + if maliciousInfo == nil { + return nil + } + + return &malicious{ + Summary: maliciousInfo.Summary, + Details: preprocessDetails(maliciousInfo.Details), + } +} + func makeProvenance( - trustyProvenance *trustytypes.Provenance, + resp *trustytypes.Provenance, ) *provenance { - if trustyProvenance == nil { + if resp == nil { return nil } prov := &provenance{} - if trustyProvenance.Description.Historical.Overlap != 0 { + if resp.Historical.Overlap != 0 { prov.Historical = &historicalProvenance{ - Versions: int(trustyProvenance.Description.Historical.Versions), - Tags: int(trustyProvenance.Description.Historical.Tags), - Common: int(trustyProvenance.Description.Historical.Common), - Overlap: trustyProvenance.Description.Historical.Overlap, + Versions: int(resp.Historical.Versions), + Tags: int(resp.Historical.Tags), + Common: int(resp.Historical.Common), + Overlap: resp.Historical.Overlap, } } - if trustyProvenance.Description.Sigstore.Issuer != "" { + if resp.Sigstore.Issuer != "" { prov.Sigstore = &sigstoreProvenance{ - SourceRepository: trustyProvenance.Description.Sigstore.SourceRepository, - Workflow: trustyProvenance.Description.Sigstore.Workflow, - Issuer: trustyProvenance.Description.Sigstore.Issuer, - RekorURI: trustyProvenance.Description.Sigstore.Transparency, + SourceRepository: resp.Sigstore.SourceRepo, + Workflow: resp.Sigstore.Workflow, + Issuer: resp.Sigstore.Issuer, + RekorURI: resp.Sigstore.Transparency, } } @@ -440,19 +504,6 @@ func makeTrustyURL(packageName string, ecosystem string) string { return trustyURL } -func getValueFromMap[T any](coll map[string]any, field string) T { - var t T - v, ok := coll[field] - if !ok { - return t - } - res, ok := v.(T) - if !ok { - return t - } - return res -} - // classifyDependency checks the dependencies from the PR for maliciousness or // low scores and adds them to the summary if needed func classifyDependency( diff --git a/internal/engine/eval/trusty/trusty_test.go b/internal/engine/eval/trusty/trusty_test.go index f1bd3f9a09..55217934a0 100644 --- a/internal/engine/eval/trusty/trusty_test.go +++ b/internal/engine/eval/trusty/trusty_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/rs/zerolog" + trustytypes "github.com/stacklok/trusty-sdk-go/pkg/v2/types" "github.com/stretchr/testify/require" evalerrors "github.com/mindersec/minder/internal/engine/errors" @@ -343,39 +344,39 @@ func TestMakeScoreComponents(t *testing.T) { t.Parallel() for _, tc := range []struct { name string - sut map[string]any + sut trustytypes.SummaryDescription expected []scoreComponent }{ { name: "no-description", - sut: nil, + sut: trustytypes.SummaryDescription{}, }, { name: "normal-response", - sut: map[string]any{ - "activity": "a", - "activity_user": "b", - "provenance": "c", - "activity_repo": "d", + sut: trustytypes.SummaryDescription{ + Activity: 1.0, + ActivityUser: 2.0, + ActivityRepo: 3.0, + ProvenanceType: &trustytypes.ProvenanceTypeVerified, }, expected: []scoreComponent{ - {Label: "Package activity", Value: "a"}, - {Label: "User activity", Value: "b"}, - {Label: "Provenance", Value: "c"}, - {Label: "Repository activity", Value: "d"}, + {Label: "Package activity", Value: 1.0}, + {Label: "User activity", Value: 2.0}, + {Label: "Repository activity", Value: 3.0}, + {Label: "Provenance", Value: "verified_provenance_match"}, }, }, { name: "typosquatting-low", - sut: map[string]any{ - "typosquatting": float64(10), + sut: trustytypes.SummaryDescription{ + TypoSquatting: 10.0, }, expected: []scoreComponent{}, }, { name: "typosquatting-high", - sut: map[string]any{ - "typosquatting": float64(1), + sut: trustytypes.SummaryDescription{ + TypoSquatting: 1, }, expected: []scoreComponent{ {Label: "Typosquatting", Value: "⚠️ Dependency may be trying to impersonate a well known package"},