Skip to content

Commit

Permalink
Decouple Trusty engine from Trusty SDK structs.
Browse files Browse the repository at this point in the history 
This change aims to limit the impact of changes deriving from Trusty
SDK to fewer lines of code. Specifically, business logic implemented
in Trusty evaluation engine looks into the response as returned by the
SDK itself, making it harder to move to Trusty API v2.

Note: this change is meant to be bug-compatible with the current
Trusty evaluation engine.
  • Loading branch information
@blkt
blkt committed Nov 20, 2024
1 parent 1fa93ef commit fd4bd48
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 333 deletions.
95 changes: 38 additions & 57 deletions internal/engine/eval/trusty/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ import (
"slices"
"strings"
template "text/template"
"unicode"

"github.com/google/go-github/v63/github"
"github.com/rs/zerolog"
trustytypes "github.com/stacklok/trusty-sdk-go/pkg/v1/types"

"github.com/mindersec/minder/internal/constants"
"github.com/mindersec/minder/internal/engine/eval/pr_actions"
Expand Down Expand Up @@ -194,7 +192,7 @@ type dependencyAlternatives struct {
BlockPR bool

// trustyReply is the complete response from trusty for this package
trustyReply *trustytypes.Reply
trustyReply *trustyReport
}

// summaryPrHandler is a prStatusHandler that adds a summary text to the PR as a comment.
Expand Down Expand Up @@ -276,8 +274,8 @@ func (sph *summaryPrHandler) generateSummary() (string, error) {
for _, alternative := range sph.trackedAlternatives {
if _, ok := lowScorePackages[alternative.Dependency.Name]; !ok {
var score float64
if alternative.trustyReply.Summary.Score != nil {
score = *alternative.trustyReply.Summary.Score
if alternative.trustyReply.Score != nil {
score = *alternative.trustyReply.Score
}

packageUIURL, err := url.JoinPath(
Expand All @@ -300,37 +298,42 @@ func (sph *summaryPrHandler) generateSummary() (string, error) {
if slices.Contains(alternative.Reasons, TRUSTY_MALICIOUS_PKG) {
malicious = append(malicious, maliciousTemplateData{
templatePackageData: packageData,
Summary: alternative.trustyReply.PackageData.Malicious.Summary,
Details: preprocessDetails(alternative.trustyReply.PackageData.Malicious.Details),
Summary: alternative.trustyReply.Malicious.Summary,
Details: preprocessDetails(alternative.trustyReply.Malicious.Details),
})
continue
}

lowScorePackages[alternative.Dependency.Name] = templatePackage{
templatePackageData: packageData,
Deprecated: alternative.trustyReply.PackageData.Deprecated,
Archived: alternative.trustyReply.PackageData.Archived,
ScoreComponents: buildScoreMatrix(alternative),
Deprecated: alternative.trustyReply.IsDeprecated,
Archived: alternative.trustyReply.IsArchived,
ScoreComponents: buildScoreMatrix(alternative.trustyReply.ScoreComponents),
Alternatives: []templateAlternative{},
Provenance: buildProvenanceStruct(alternative.trustyReply),
}
}

for _, altData := range alternative.trustyReply.Alternatives.Packages {
if altData.Score <= lowScorePackages[alternative.Dependency.Name].Score {
for _, altData := range alternative.trustyReply.Alternatives {
// Note: now that the score is deprecated and
// effectively `nil` for all packages, this
// loop will always discard all alternatives,
// rendering the whole block dead code.
//
// Since (1) we don't have score anymore, and
// (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 {
continue
}

altPackageData := templateAlternative{
templatePackageData: templatePackageData{
Ecosystem: alternative.Dependency.Ecosystem.AsString(),
Ecosystem: altData.PackageType,
PackageName: altData.PackageName,
TrustyURL: fmt.Sprintf(
"%s%s/%s", constants.TrustyHttpURL,
strings.ToLower(alternative.Dependency.Ecosystem.AsString()),
url.PathEscape(altData.PackageName),
),
Score: altData.Score,
TrustyURL: altData.TrustyURL,
Score: *altData.Score,
},
}

Expand All @@ -344,27 +347,27 @@ func (sph *summaryPrHandler) generateSummary() (string, error) {
}

// buildProvenanceStruct builds the provenance data structure for the PR template
func buildProvenanceStruct(r *trustytypes.Reply) *templateProvenance {
func buildProvenanceStruct(r *trustyReport) *templateProvenance {
if r == nil || r.Provenance == nil {
return nil
}
var provenance *templateProvenance
if r.Provenance != nil {
provenance = &templateProvenance{}
if r.Provenance.Description.Historical.Overlap != 0 {
if r.Provenance.Historical != nil && r.Provenance.Historical.Overlap != 0 {
provenance.Historical = &templateHistoricalProvenance{
NumVersions: int(r.Provenance.Description.Historical.Versions),
NumTags: int(r.Provenance.Description.Historical.Tags),
MatchedVersions: int(r.Provenance.Description.Historical.Common),
NumVersions: int(r.Provenance.Historical.Versions),
NumTags: int(r.Provenance.Historical.Tags),
MatchedVersions: int(r.Provenance.Historical.Common),
}
}

if r.Provenance.Description.Sigstore.Issuer != "" {
if r.Provenance.Sigstore != nil && r.Provenance.Sigstore.Issuer != "" {
provenance.Sigstore = &templateSigstoreProvenance{
SourceRepository: r.Provenance.Description.Sigstore.SourceRepository,
Workflow: r.Provenance.Description.Sigstore.Workflow,
Issuer: r.Provenance.Description.Sigstore.Issuer,
RekorURI: r.Provenance.Description.Sigstore.Transparency,
SourceRepository: r.Provenance.Sigstore.SourceRepository,
Workflow: r.Provenance.Sigstore.Workflow,
Issuer: r.Provenance.Sigstore.Issuer,
RekorURI: r.Provenance.Sigstore.RekorURI,
}
}

Expand All @@ -377,35 +380,13 @@ func buildProvenanceStruct(r *trustytypes.Reply) *templateProvenance {

// buildScoreMatrix builds the score components matrix that populates
// the score table in the PR comment template
func buildScoreMatrix(alternative dependencyAlternatives) []templateScoreComponent {
func buildScoreMatrix(components []scoreComponent) []templateScoreComponent {
scoreComp := []templateScoreComponent{}
if alternative.trustyReply.Summary.Description != nil {
for l, v := range alternative.trustyReply.Summary.Description {
switch l {
case "activity":
l = "Package activity"
case "activity_user":
l = "User activity"
case "provenance":
l = "Provenance"
case "typosquatting":
if v.(float64) > 5.00 {
continue
}
v = "⚠️ Dependency may be trying to impersonate a well known package"
l = "Typosquatting"
case "activity_repo":
l = "Repository activity"
default:
if len(l) > 1 {
l = string(unicode.ToUpper([]rune(l)[0])) + l[1:]
}
}
scoreComp = append(scoreComp, templateScoreComponent{
Label: l,
Value: v,
})
}
for _, component := range components {
scoreComp = append(scoreComp, templateScoreComponent{

Check failure on line 386 in internal/engine/eval/trusty/actions.go

View workflow job for this annotation

GitHub Actions / lint / Run golangci-lint 

S1016: should convert component (type scoreComponent) to templateScoreComponent instead of using struct literal (gosimple)
Label: component.Label,
Value: component.Value,
})
}
return scoreComp
}
Expand Down
68 changes: 29 additions & 39 deletions internal/engine/eval/trusty/actions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package trusty
import (
"testing"

trustytypes "github.com/stacklok/trusty-sdk-go/pkg/v1/types"
"github.com/stretchr/testify/require"

v1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
Expand All @@ -26,28 +25,25 @@ func TestBuildProvenanceStruct(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
sut *trustytypes.Reply
sut *trustyReport
mustNil bool
expected *templateProvenance
}{
{
name: "full-response",
sut: &trustytypes.Reply{
Provenance: &trustytypes.Provenance{
Score: 8.0,
Description: trustytypes.ProvenanceDescription{
Historical: trustytypes.HistoricalProvenance{
Tags: 10,
Common: 8,
Overlap: 80,
Versions: 10,
},
Sigstore: trustytypes.SigstoreProvenance{
Issuer: "CN=sigstore-intermediate,O=sigstore.dev",
Workflow: ".github/workflows/build_and_deploy.yml",
SourceRepository: "https://github.com/vercel/next.js",
Transparency: "https://search.sigstore.dev/?logIndex=88381843",
},
sut: &trustyReport{
Provenance: &provenance{
Historical: &historicalProvenance{
Tags: 10,
Common: 8,
Overlap: 80,
Versions: 10,
},
Sigstore: &sigstoreProvenance{
Issuer: "CN=sigstore-intermediate,O=sigstore.dev",
Workflow: ".github/workflows/build_and_deploy.yml",
SourceRepository: "https://github.com/vercel/next.js",
RekorURI: "https://search.sigstore.dev/?logIndex=88381843",
},
},
},
Expand All @@ -68,16 +64,13 @@ func TestBuildProvenanceStruct(t *testing.T) {
},
{
name: "only-historical",
sut: &trustytypes.Reply{
Provenance: &trustytypes.Provenance{
Score: 8.0,
Description: trustytypes.ProvenanceDescription{
Historical: trustytypes.HistoricalProvenance{
Tags: 10,
Common: 8,
Overlap: 80,
Versions: 10,
},
sut: &trustyReport{
Provenance: &provenance{
Historical: &historicalProvenance{
Tags: 10,
Common: 8,
Overlap: 80,
Versions: 10,
},
},
},
Expand All @@ -92,16 +85,13 @@ func TestBuildProvenanceStruct(t *testing.T) {
},
{
name: "only-sigstore",
sut: &trustytypes.Reply{
Provenance: &trustytypes.Provenance{
Score: 8.0,
Description: trustytypes.ProvenanceDescription{
Sigstore: trustytypes.SigstoreProvenance{
Issuer: "CN=sigstore-intermediate,O=sigstore.dev",
Workflow: ".github/workflows/build_and_deploy.yml",
SourceRepository: "https://github.com/vercel/next.js",
Transparency: "https://search.sigstore.dev/?logIndex=88381843",
},
sut: &trustyReport{
Provenance: &provenance{
Sigstore: &sigstoreProvenance{
Issuer: "CN=sigstore-intermediate,O=sigstore.dev",
Workflow: ".github/workflows/build_and_deploy.yml",
SourceRepository: "https://github.com/vercel/next.js",
RekorURI: "https://search.sigstore.dev/?logIndex=88381843",
},
},
},
Expand All @@ -122,7 +112,7 @@ func TestBuildProvenanceStruct(t *testing.T) {
},
{
name: "no-provenance",
sut: &trustytypes.Reply{},
sut: &trustyReport{},
mustNil: true,
},
} {
Expand Down
Loading

0 comments on commit fd4bd48

Please sign in to comment.