🌱 Add probe registration mechanism (#3876)

* add basic probe registration function Signed-off-by: Spencer Schrock <[email protected]> * ignore probes which call init to register the probe Signed-off-by: Spencer Schrock <[email protected]> * redefine probeimpl to avoid circular imports Signed-off-by: Spencer Schrock <[email protected]> * register all probes Signed-off-by: Spencer Schrock <[email protected]> * experiment with a probe struct Signed-off-by: Spencer Schrock <[email protected]> * make check name constants Signed-off-by: Spencer Schrock <[email protected]> * convert branch protection probes Signed-off-by: Spencer Schrock <[email protected]> * convert binary artifact probes Signed-off-by: Spencer Schrock <[email protected]> * convert cii probe Signed-off-by: Spencer Schrock <[email protected]> * convert ci test probe Signed-off-by: Spencer Schrock <[email protected]> * convert code review probes Signed-off-by: Spencer Schrock <[email protected]> * convert contributor probe Signed-off-by: Spencer Schrock <[email protected]> * convert dangerous workflow probe Signed-off-by: Spencer Schrock <[email protected]> * convert dep update tool probes Signed-off-by: Spencer Schrock <[email protected]> * convert fuzzing probes Signed-off-by: Spencer Schrock <[email protected]> * convert license probes Signed-off-by: Spencer Schrock <[email protected]> * convert maintained probes Signed-off-by: Spencer Schrock <[email protected]> * convert packaging probe Signed-off-by: Spencer Schrock <[email protected]> * convert sast probes Signed-off-by: Spencer Schrock <[email protected]> * convert security policy probes Signed-off-by: Spencer Schrock <[email protected]> * convert signed releases probes Signed-off-by: Spencer Schrock <[email protected]> * convert vuln probe Signed-off-by: Spencer Schrock <[email protected]> * try using probe registration data Signed-off-by: Spencer Schrock <[email protected]> * blank import unused probe Signed-off-by: Spencer Schrock <[email protected]> * add uncategorized group Signed-off-by: Spencer Schrock <[email protected]> * ensure All list is up-to-date Signed-off-by: Spencer Schrock <[email protected]> * add reason behind uncategorized group Signed-off-by: Spencer Schrock <[email protected]> * fix linter yaml parse error Signed-off-by: Spencer Schrock <[email protected]> * fix linter Signed-off-by: Spencer Schrock <[email protected]> * add webhook data Signed-off-by: Spencer Schrock <[email protected]> * convert probe registration to Must pattern Signed-off-by: Spencer Schrock <[email protected]> * add registration for new probes Signed-off-by: Spencer Schrock <[email protected]> * add missing license header Signed-off-by: Spencer Schrock <[email protected]> * revert changing wrapcheck linter config Signed-off-by: Spencer Schrock <[email protected]> * use error func which doesnt need wrapped Signed-off-by: Spencer Schrock <[email protected]> * add test for probe registration Signed-off-by: Spencer Schrock <[email protected]> * restore trailing newline Signed-off-by: Spencer Schrock <[email protected]> * order probe category list Signed-off-by: Spencer Schrock <[email protected]> --------- Signed-off-by: Spencer Schrock <[email protected]>
ossf · Mar 19, 2024 · b3ad602 · b3ad602
1 parent 2c02d4b
commit b3ad602
Show file tree

Hide file tree

Showing 50 changed files with 575 additions and 100 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -21,6 +21,10 @@ issues:
         - goerr113
         - lll
         - wrapcheck
+  # probes must register via init
+    - path: 'probes/.+/impl.go'
+      linters:
+        - gochecknoinits
 skip-files:
   - cron/data/request.pb.go # autogenerated
 linters:

diff --git a/internal/probes/probes.go b/internal/probes/probes.go
@@ -0,0 +1,93 @@
+// Copyright 2024 OpenSSF Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package probes
+
+import (
+	"fmt"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+type CheckName string
+
+// Redefining check names here to avoid circular imports.
+const (
+	BinaryArtifacts      CheckName = "Binary-Artifacts"
+	BranchProtection     CheckName = "Branch-Protection"
+	CIIBestPractices     CheckName = "CII-Best-Practices"
+	CITests              CheckName = "CI-Tests"
+	CodeReview           CheckName = "Code-Review"
+	Contributors         CheckName = "Contributors"
+	DangerousWorkflow    CheckName = "Dangerous-Workflow"
+	DependencyUpdateTool CheckName = "Dependency-Update-Tool"
+	Fuzzing              CheckName = "Fuzzing"
+	License              CheckName = "License"
+	Maintained           CheckName = "Maintained"
+	Packaging            CheckName = "Packaging"
+	PinnedDependencies   CheckName = "Pinned-Dependencies"
+	SAST                 CheckName = "SAST"
+	SecurityPolicy       CheckName = "Security-Policy"
+	SignedReleases       CheckName = "Signed-Releases"
+	TokenPermissions     CheckName = "Token-Permissions"
+	Vulnerabilities      CheckName = "Vulnerabilities"
+	Webhooks             CheckName = "Webhooks"
+)
+
+type Probe struct {
+	Name            string
+	Implementation  ProbeImpl
+	RequiredRawData []CheckName
+}
+
+type ProbeImpl func(*checker.RawResults) ([]finding.Finding, string, error)
+
+// registered is the mapping of all registered probes.
+var registered = map[string]Probe{}
+
+func MustRegister(name string, impl ProbeImpl, requiredRawData []CheckName) {
+	err := register(Probe{
+		Name:            name,
+		Implementation:  impl,
+		RequiredRawData: requiredRawData,
+	})
+	if err != nil {
+		panic(err)
+	}
+}
+
+func register(p Probe) error {
+	if p.Name == "" {
+		return errors.WithMessage(errors.ErrScorecardInternal, "name cannot be empty")
+	}
+	if p.Implementation == nil {
+		return errors.WithMessage(errors.ErrScorecardInternal, "implementation cannot be nil")
+	}
+	if len(p.RequiredRawData) == 0 {
+		return errors.WithMessage(errors.ErrScorecardInternal, "probes need some raw data")
+	}
+	registered[p.Name] = p
+	return nil
+}
+
+func Get(name string) (Probe, error) {
+	p, ok := registered[name]
+	if !ok {
+		msg := fmt.Sprintf("probe %q not found", name)
+		return Probe{}, errors.WithMessage(errors.ErrScorecardInternal, msg)
+	}
+	return p, nil
+}
diff --git a/internal/probes/probes_test.go b/internal/probes/probes_test.go
@@ -0,0 +1,144 @@
+// Copyright 2024 OpenSSF Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package probes
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+func emptyImpl(r *checker.RawResults) ([]finding.Finding, string, error) {
+	return nil, "", nil
+}
+
+var (
+	p1 = Probe{
+		Name:            "someProbe1",
+		Implementation:  emptyImpl,
+		RequiredRawData: []CheckName{BinaryArtifacts},
+	}
+
+	p2 = Probe{
+		Name:            "someProbe2",
+		Implementation:  emptyImpl,
+		RequiredRawData: []CheckName{BranchProtection},
+	}
+)
+
+//nolint:paralleltest // registration isn't safe for concurrent use
+func Test_register(t *testing.T) {
+	tests := []struct {
+		name    string
+		probe   Probe
+		wantErr bool
+	}{
+		{
+			name: "name is required",
+			probe: Probe{
+				Name:            "",
+				Implementation:  emptyImpl,
+				RequiredRawData: []CheckName{BinaryArtifacts},
+			},
+			wantErr: true,
+		},
+		{
+			name: "implementation is required",
+			probe: Probe{
+				Name:            "foo",
+				Implementation:  nil,
+				RequiredRawData: []CheckName{BinaryArtifacts},
+			},
+			wantErr: true,
+		},
+		{
+			name: "raw check data is required",
+			probe: Probe{
+				Name:            "foo",
+				Implementation:  emptyImpl,
+				RequiredRawData: []CheckName{},
+			},
+			wantErr: true,
+		},
+		{
+			name: "valid registration",
+			probe: Probe{
+				Name:            "foo",
+				Implementation:  emptyImpl,
+				RequiredRawData: []CheckName{BinaryArtifacts},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			err := register(tt.probe)
+			if err != nil != tt.wantErr {
+				t.Fatalf("got err: %v, wanted err: %t", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func setupControlledProbes(t *testing.T) {
+	t.Helper()
+	err := register(p1)
+	if err != nil {
+		t.Fatalf("unable to register someProbe1")
+	}
+	err = register(p2)
+	if err != nil {
+		t.Fatalf("unable to register someProbe2")
+	}
+}
+
+//nolint:paralleltest // registration isn't safe for concurrent use
+func TestGet(t *testing.T) {
+	tests := []struct {
+		name      string
+		probeName string
+		expected  Probe
+		wantErr   bool
+	}{
+		{
+			name:      "probe is found",
+			probeName: p1.Name,
+			expected:  p1,
+			wantErr:   false,
+		},
+		{
+			name:      "probe not found",
+			probeName: "noProbeCalledThis",
+			wantErr:   true,
+		},
+	}
+	setupControlledProbes(t)
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			p, err := Get(tt.probeName)
+			if err != nil != tt.wantErr {
+				t.Fatalf("got err: %v, wanted err: %t", err, tt.wantErr)
+			}
+			if diff := cmp.Diff(p.Name, tt.expected.Name); diff != "" {
+				t.Error("probes didn't match: " + diff)
+			}
+		})
+	}
+}
diff --git a/pkg/scorecard.go b/pkg/scorecard.go
@@ -30,6 +30,7 @@ import (
 	"github.com/ossf/scorecard/v4/clients"
 	sce "github.com/ossf/scorecard/v4/errors"
 	"github.com/ossf/scorecard/v4/finding"
+	proberegistration "github.com/ossf/scorecard/v4/internal/probes"
 	"github.com/ossf/scorecard/v4/options"
 	"github.com/ossf/scorecard/v4/probes"
 	"github.com/ossf/scorecard/v4/probes/zrunner"
@@ -198,14 +199,12 @@ func runEnabledProbes(request *checker.CheckRequest,
 
 	probeFindings := make([]finding.Finding, 0)
 	for _, probeName := range probesToRun {
-		// Get the probe Run func
-		probeRunner, err := probes.GetProbeRunner(probeName)
+		probe, err := proberegistration.Get(probeName)
 		if err != nil {
-			msg := fmt.Sprintf("could not find probe: %s", probeName)
-			return sce.WithMessage(sce.ErrScorecardInternal, msg)
+			return fmt.Errorf("getting probe %q: %w", probeName, err)
 		}
 		// Run probe
-		findings, _, err := probeRunner(&ret.RawResults)
+		findings, _, err := probe.Implementation(&ret.RawResults)
 		if err != nil {
 			return sce.WithMessage(sce.ErrScorecardInternal, "ending run")
 		}