Skip to content

✨ Factor private vulnerability reporting into Security-Policy #5073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bmendonca3 wants to merge 1 commit into from
Closed

✨ Factor private vulnerability reporting into Security-Policy #5073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion checker/raw_result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ type SASTWorkflow struct {
// SecurityPolicyData contains the raw results
// for the Security-Policy check.
type SecurityPolicyData struct {
PolicyFiles []SecurityPolicyFile
PolicyFiles []SecurityPolicyFile
PrivateVulnerabilityReportingEnabled *bool
}

// BinaryArtifactData contains the raw results
Expand Down
13 changes: 12 additions & 1 deletion checks/evaluation/security_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,18 @@ import (
"github.com/ossf/scorecard/v5/probes/securityPolicyContainsText"
"github.com/ossf/scorecard/v5/probes/securityPolicyContainsVulnerabilityDisclosure"
"github.com/ossf/scorecard/v5/probes/securityPolicyPresent"
"github.com/ossf/scorecard/v5/probes/securityPolicyPrivateVulnerabilityReportingEnabled"
)

// SecurityPolicy applies the score policy for the Security-Policy check.
func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLogger) checker.CheckResult {
// We have 4 unique probes, each should have a finding.
// We have 5 unique probes, each should have a finding.
expectedProbes := []string{
securityPolicyContainsVulnerabilityDisclosure.Probe,
securityPolicyContainsLinks.Probe,
securityPolicyContainsText.Probe,
securityPolicyPresent.Probe,
securityPolicyPrivateVulnerabilityReportingEnabled.Probe,
}
if !finding.UniqueProbesEqual(findings, expectedProbes) {
e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
Expand All @@ -40,6 +42,7 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo

score := 0
m := make(map[string]bool)
privateVulnerabilityReportingEnabled := false
var logLevel checker.DetailType
for i := range findings {
f := &findings[i]
Expand All @@ -56,6 +59,8 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo
score += scoreProbeOnce(f.Probe, m, 3)
case securityPolicyPresent.Probe:
m[f.Probe] = true
case securityPolicyPrivateVulnerabilityReportingEnabled.Probe:
privateVulnerabilityReportingEnabled = true
default:
e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results")
return checker.CreateRuntimeErrorResult(name, e)
Expand All @@ -73,8 +78,14 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo
e := sce.WithMessage(sce.ErrScorecardInternal, "score calculation problem")
return checker.CreateRuntimeErrorResult(name, e)
}
if privateVulnerabilityReportingEnabled {
return checker.CreateResultWithScore(name, "private vulnerability reporting enabled", 8)
}
return checker.CreateMinScoreResult(name, "security policy file not detected")
}
if privateVulnerabilityReportingEnabled && score < 8 {
score = 8
}
return checker.CreateResultWithScore(name, "security policy file detected", score)
}

Expand Down
106 changes: 95 additions & 11 deletions checks/evaluation/security_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ func TestSecurityPolicy(t *testing.T) {
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyInvalidProbeName",
Outcome: finding.OutcomeFalse,
Expand Down Expand Up @@ -99,11 +103,16 @@ func TestSecurityPolicy(t *testing.T) {
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeTrue,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeNotApplicable,
},
},
result: scut.TestReturn{
Score: checker.MinResultScore,
NumberOfInfo: 1,
NumberOfWarn: 3,
Score: checker.MinResultScore,
NumberOfInfo: 1,
NumberOfWarn: 3,
NumberOfDebug: 1,
},
},
{
Expand All @@ -125,12 +134,47 @@ func TestSecurityPolicy(t *testing.T) {
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeNotApplicable,
},
},
result: scut.TestReturn{
Score: checker.InconclusiveResultScore,
Error: sce.ErrScorecardInternal,
NumberOfWarn: 1,
NumberOfInfo: 3,
Score: checker.InconclusiveResultScore,
Error: sce.ErrScorecardInternal,
NumberOfWarn: 1,
NumberOfInfo: 3,
NumberOfDebug: 1,
},
},
{
name: "file not found with private vulnerability reporting enabled",
findings: []finding.Finding{
{
Probe: "securityPolicyContainsVulnerabilityDisclosure",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyContainsLinks",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyContainsText",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeTrue,
},
},
result: scut.TestReturn{
Score: 8,
NumberOfInfo: 1,
NumberOfWarn: 4,
},
},
{
Expand All @@ -152,11 +196,46 @@ func TestSecurityPolicy(t *testing.T) {
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeTrue,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeNotApplicable,
},
},
result: scut.TestReturn{
Score: 6,
NumberOfInfo: 2,
NumberOfWarn: 2,
NumberOfDebug: 1,
},
},
{
name: "file found with private vulnerability reporting score floor",
findings: []finding.Finding{
{
Probe: "securityPolicyContainsVulnerabilityDisclosure",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyContainsLinks",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyContainsText",
Outcome: finding.OutcomeFalse,
},
{
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeTrue,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeTrue,
},
},
result: scut.TestReturn{
Score: 6,
Score: 8,
NumberOfInfo: 2,
NumberOfWarn: 2,
NumberOfWarn: 3,
},
},
{
Expand All @@ -178,10 +257,15 @@ func TestSecurityPolicy(t *testing.T) {
Probe: "securityPolicyPresent",
Outcome: finding.OutcomeTrue,
},
{
Probe: "securityPolicyPrivateVulnerabilityReportingEnabled",
Outcome: finding.OutcomeNotApplicable,
},
},
result: scut.TestReturn{
Score: checker.MaxResultScore,
NumberOfInfo: 4,
Score: checker.MaxResultScore,
NumberOfInfo: 4,
NumberOfDebug: 1,
},
},
}
Expand Down
35 changes: 32 additions & 3 deletions checks/raw/security_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,21 @@ type securityPolicyFilesWithURI struct {
files []checker.SecurityPolicyFile
}

type privateVulnerabilityReportingClient interface {
IsPrivateVulnerabilityReportingEnabled() (bool, error)
}

// SecurityPolicy checks for presence of security policy
// and applicable content discovered by checkSecurityPolicyFileContent().
func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error) {
data := securityPolicyFilesWithURI{
uri: "", files: make([]checker.SecurityPolicyFile, 0),
}
err := fileparser.OnAllFilesDo(c.RepoClient, isSecurityPolicyFile, &data)
privateVulnerabilityReportingEnabled, err := privateVulnerabilityReportingStatus(c.RepoClient)
if err != nil {
return checker.SecurityPolicyData{}, err
}
err = fileparser.OnAllFilesDo(c.RepoClient, isSecurityPolicyFile, &data)
if err != nil {
return checker.SecurityPolicyData{}, err
}
Expand All @@ -55,7 +63,10 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
return checker.SecurityPolicyData{}, err
}
}
return checker.SecurityPolicyData{PolicyFiles: data.files}, nil
return checker.SecurityPolicyData{
PolicyFiles: data.files,
PrivateVulnerabilityReportingEnabled: privateVulnerabilityReportingEnabled,
}, nil
}

// Check if present in parent org.
Expand Down Expand Up @@ -95,7 +106,25 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
}
}
}
return checker.SecurityPolicyData{PolicyFiles: data.files}, nil
return checker.SecurityPolicyData{
PolicyFiles: data.files,
PrivateVulnerabilityReportingEnabled: privateVulnerabilityReportingEnabled,
}, nil
}

func privateVulnerabilityReportingStatus(repoClient clients.RepoClient) (*bool, error) {
client, ok := repoClient.(privateVulnerabilityReportingClient)
if !ok {
return nil, nil
}
enabled, err := client.IsPrivateVulnerabilityReportingEnabled()
if err != nil {
if errors.Is(err, clients.ErrUnsupportedFeature) {
return nil, nil
}
return nil, err
}
return &enabled, nil
}

// Check repository for repository-specific policy.
Expand Down
67 changes: 67 additions & 0 deletions checks/raw/security_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package raw

import (
"errors"
"io"
"os"
"strings"
Expand All @@ -23,10 +24,25 @@ import (
"go.uber.org/mock/gomock"

"github.com/ossf/scorecard/v5/checker"
"github.com/ossf/scorecard/v5/clients"
mockrepo "github.com/ossf/scorecard/v5/clients/mockclients"
scut "github.com/ossf/scorecard/v5/utests"
)

type privateVulnerabilityReportingRepoClient struct {
clients.RepoClient
enabled bool
err error
}

func (c privateVulnerabilityReportingRepoClient) IsPrivateVulnerabilityReportingEnabled() (bool, error) {
return c.enabled, c.err
}

func boolPtr(v bool) *bool {
return &v
}

func Test_isSecurityPolicyFilename(t *testing.T) {
t.Parallel()
tests := []struct {
Expand Down Expand Up @@ -60,6 +76,57 @@ func Test_isSecurityPolicyFilename(t *testing.T) {
}
}

func TestPrivateVulnerabilityReportingStatus(t *testing.T) {
t.Parallel()
errUnexpected := errors.New("unexpected")
tests := []struct {
name string
client privateVulnerabilityReportingRepoClient
want *bool
wantErr error
}{
{
name: "enabled",
client: privateVulnerabilityReportingRepoClient{enabled: true},
want: boolPtr(true),
},
{
name: "disabled",
client: privateVulnerabilityReportingRepoClient{enabled: false},
want: boolPtr(false),
},
{
name: "error",
client: privateVulnerabilityReportingRepoClient{err: errUnexpected},
wantErr: errUnexpected,
},
{
name: "unsupported",
client: privateVulnerabilityReportingRepoClient{err: clients.ErrUnsupportedFeature},
want: nil,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := privateVulnerabilityReportingStatus(tt.client)
if !errors.Is(err, tt.wantErr) {
t.Fatalf("privateVulnerabilityReportingStatus error = %v, want %v", err, tt.wantErr)
}
if tt.want == nil {
if got != nil {
t.Fatalf("privateVulnerabilityReportingStatus = %v, want nil", *got)
}
return
}
if got == nil || *got != *tt.want {
t.Fatalf("privateVulnerabilityReportingStatus = %v, want %v", got, tt.want)
}
})
}
}

// TestSecurityPolicy tests the security policy.
func TestSecurityPolicy(t *testing.T) {
t.Parallel()
Expand Down
Loading