Skip to content

Commit

Permalink
feat: Add policy for detecting GitHub Secret-Scanning (#311)
Browse files Browse the repository at this point in the history
* * Add policy for detecting GitHub Secret-Scanning
* Add collection of 'Security and Analysis' settings for github repos

* Fix phrasing of policy desc

* add e2e testing

* change naming to advanced_security

* add public repo for secret scanning testing

* more detailed errors about secret_scanning permissions
  • Loading branch information
naphtalid-legit authored Jun 13, 2024
1 parent 34641ae commit daec3b4
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 11 deletions.
5 changes: 5 additions & 0 deletions e2e/github_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,9 @@ var testCasesGitHubRepository = []testCase{
failedEntity: "bad_repo",
passedEntity: "good_repo",
},
{
path: "data.repository.secret_scanning_not_enabled",
failedEntity: "bad_public_repo",
passedEntity: "good_public_repo",
},
}
8 changes: 8 additions & 0 deletions internal/analyzers/skippers/skippers.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ func NewSkipper(ctx context.Context) Skipper {
"enterprise": func(_ collectors.CollectedData) bool {
return !context_utils.GetIsCloud(ctx)
},
"advanced_security": func(data collectors.CollectedData) bool {
repositoryContext, ok := data.Context.(collectors.CollectedDataRepositoryContext)
if !ok {
log.Printf("invalid type %T", data.Context)
return false
}
return repositoryContext.HasGithubAdvancedSecurity()
},
},
}
}
Expand Down
24 changes: 19 additions & 5 deletions internal/clients/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ import (
"bytes"
"context"
"fmt"
"log"
"net/http"
"regexp"
"strings"
"sync"

"github.com/Legit-Labs/legitify/internal/clients/github/pagination"
"github.com/Legit-Labs/legitify/internal/clients/github/transport"
"github.com/Legit-Labs/legitify/internal/clients/github/types"
Expand All @@ -12,11 +18,6 @@ import (
"github.com/Legit-Labs/legitify/internal/common/slice_utils"
commontypes "github.com/Legit-Labs/legitify/internal/common/types"
"github.com/Legit-Labs/legitify/internal/screen"
"log"
"net/http"
"regexp"
"strings"
"sync"

githubcollected "github.com/Legit-Labs/legitify/internal/collected/github"
"github.com/Legit-Labs/legitify/internal/common/permissions"
Expand Down Expand Up @@ -656,3 +657,16 @@ func (c *Client) GetOrganizationSecrets(org string) (*gh.Secrets, error) {
}
return secrets, nil
}

func (c *Client) GetSecurityAndAnalysisForRepository(repo, owner string) (*gh.SecurityAndAnalysis, error) {
r, res, err := c.Client().Repositories.Get(c.context, owner, repo)

if err != nil {
return nil, err
}
if res.StatusCode != 200 {
return nil, fmt.Errorf("unexpected HTTP status: %d", res.StatusCode)
}

return r.SecurityAndAnalysis, nil
}
1 change: 1 addition & 0 deletions internal/collected/github/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type Repository struct {
DependencyGraphManifests *GitHubQLDependencyGraphManifests `json:"dependency_graph_manifests"`
RulesSet []*types.RepositoryRule `json:"rules_set,omitempty"`
RepoSecrets []*RepositorySecret `json:"repository_secrets,omitempty"`
SecurityAndAnalysis *github.SecurityAndAnalysis `json:"security_and_analysis,omitempty"`
}

type RepositorySecret struct {
Expand Down
1 change: 1 addition & 0 deletions internal/collectors/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type CollectedDataContext interface {
type CollectedDataRepositoryContext interface {
CollectedDataContext
HasBranchProtectionPermission() bool
HasGithubAdvancedSecurity() bool
}

type CollectedData struct {
Expand Down
48 changes: 43 additions & 5 deletions internal/collectors/github/repository_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ func (rc *repositoryCollector) collectSpecific(repositories []types.RepositoryWi

hasBp := hasBranchProtection(org, query.RepositoryOwner.Repository.IsPrivate)
collectionContext = newRepositoryContext([]permissions.Role{org.Role, query.RepositoryOwner.Repository.ViewerPermission},
hasBp, org.IsEnterprise(), false)
hasBp, org.IsEnterprise(), false, false)
} else {
hasBp := rc.hasBranchProtectionForUser(repo.Owner, query.RepositoryOwner.Repository.IsPrivate)
collectionContext = newRepositoryContext([]permissions.Role{query.RepositoryOwner.Repository.ViewerPermission},
hasBp, false, false)
hasBp, false, false, false)
}

rc.collectRepository(&query.RepositoryOwner.Repository, repo.Owner, collectionContext)
Expand Down Expand Up @@ -206,7 +206,7 @@ func (rc *repositoryCollector) collectRepositories(org *ghcollected.ExtendedOrg)
node := &(nodes[i])
extraGw.Do(func() {
collectionContext := newRepositoryContext([]permissions.Role{org.Role, node.ViewerPermission},
hasBranchProtection(org, node.IsPrivate), org.IsEnterprise(), false)
hasBranchProtection(org, node.IsPrivate), org.IsEnterprise(), false, false)
rc.collectRepository(node, org.Name(), collectionContext)
})
}
Expand All @@ -226,9 +226,10 @@ func (rc *repositoryCollector) collectRepositories(org *ghcollected.ExtendedOrg)
func (rc *repositoryCollector) collectRepository(repository *ghcollected.GitHubQLRepository, login string, collectionContext *repositoryContext) {
repo := rc.collectExtraData(login, repository, collectionContext.isBranchProtectionSupported)
entityName := collectors.FullRepoName(login, repo.Repository.Name)
missingPermissions := rc.checkMissingPermissions(repo, entityName)
missingPermissions := rc.checkMissingPermissions(repo, entityName, collectionContext)
rc.IssueMissingPermissions(missingPermissions...)
collectionContext.SetHasBranchProtectionPermission(!repo.NoBranchProtectionPermission)
collectionContext.SetHasGithubAdvancedSecurity(repo.SecurityAndAnalysis != nil)
rc.CollectDataWithContext(repo, repo.Repository.Url, collectionContext)
rc.CollectionChangeByOne()
}
Expand All @@ -255,6 +256,11 @@ func (rc *repositoryCollector) collectExtraData(login string,
log.Printf("error getting repository dependency manifests for %s: %s", collectors.FullRepoName(login, repo.Repository.Name), err)
}

repo, err = rc.withSecurityAndAnalysis(repo, login)
if err != nil {
log.Printf("failed to collect repository Security and Analysis settings for %s: %s", repo.Repository.Name, err)
}

if isBranchProtectionSupported {
repo, err = rc.fixBranchProtectionInfo(repo, login)
if err != nil {
Expand Down Expand Up @@ -407,6 +413,17 @@ func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, lo
return repository, nil
}

func (rc *repositoryCollector) withSecurityAndAnalysis(repo ghcollected.Repository, login string) (ghcollected.Repository, error) {

securityAndAnalysis, err := rc.Client.GetSecurityAndAnalysisForRepository(repo.Name(), login)
if err != nil {
return repo, err
}

repo.SecurityAndAnalysis = securityAndAnalysis
return repo, nil
}

// fixBranchProtectionInfo fixes the branch protection info for the repository,
// to reflect whether there is no branch protection, or just no permission to fetch the info.
func (rc *repositoryCollector) fixBranchProtectionInfo(repository ghcollected.Repository, org string) (ghcollected.Repository, error) {
Expand Down Expand Up @@ -444,16 +461,37 @@ func (rc *repositoryCollector) fixBranchProtectionInfo(repository ghcollected.Re
return repository, nil
}

func (rc *repositoryCollector) checkMissingPermissions(repo ghcollected.Repository, entityName string) []collectors.MissingPermission {
func (rc *repositoryCollector) checkMissingPermissions(repo ghcollected.Repository, entityName string, repoContext *repositoryContext) []collectors.MissingPermission {
var missingPermissions []collectors.MissingPermission
if repo.NoBranchProtectionPermission {
effect := "Cannot read repository branch protection information"
perm := collectors.NewMissingPermission(permissions.RepoAdmin, entityName, effect, namespace.Repository)
missingPermissions = append(missingPermissions, perm)
}
if repo.SecurityAndAnalysis == nil {
var effect string
if !checkRepoAdminPermission(repoContext.roles) {
effect = "Cannot read repository Security and Analysis settings"
} else if repo.Repository.IsPrivate {
effect = "Your GitHub plan does not include a secret scanning feature."
}
perm := collectors.NewMissingPermission(permissions.RepoAdmin, entityName, effect, namespace.Repository)
missingPermissions = append(missingPermissions, perm)
}

return missingPermissions
}

func checkRepoAdminPermission(roles []permissions.RepositoryRole) bool{
for _, role := range roles {
if (permissions.IsRepositoryRole(role) && role == permissions.RepoRoleAdmin) ||
(permissions.IsOrgRole(role) && role == permissions.OrgRoleOwner) {
return true
}
}
return false
}

const (
orgIsFreeEffect = "Branch protection cannot be collected because the organization is in free plan"
)
12 changes: 11 additions & 1 deletion internal/collectors/github/repository_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type repositoryContext struct {
isEnterprise bool
isBranchProtectionSupported bool
hasBranchProtectionPermission bool
hasGithubAdvancedSecurity bool
}

func (rc *repositoryContext) Premium() bool {
Expand All @@ -31,11 +32,20 @@ func (rc *repositoryContext) HasBranchProtectionPermission() bool {
return rc.hasBranchProtectionPermission
}

func newRepositoryContext(roles []permissions.RepositoryRole, isBranchProtectionSupported bool, isEnterprise bool, hasBranchProtectionPermission bool) *repositoryContext {
func (rc *repositoryContext) SetHasGithubAdvancedSecurity(value bool) {
rc.hasGithubAdvancedSecurity = value
}

func (rc *repositoryContext) HasGithubAdvancedSecurity() bool {
return rc.hasGithubAdvancedSecurity
}

func newRepositoryContext(roles []permissions.RepositoryRole, isBranchProtectionSupported bool, isEnterprise bool, hasBranchProtectionPermission bool, hasGithubAdvancedSecurity bool) *repositoryContext {
return &repositoryContext{
roles: roles,
isEnterprise: isEnterprise,
isBranchProtectionSupported: isBranchProtectionSupported,
hasBranchProtectionPermission: hasBranchProtectionPermission,
hasGithubAdvancedSecurity: hasGithubAdvancedSecurity,
}
}
19 changes: 19 additions & 0 deletions policies/github/repository.rego
Original file line number Diff line number Diff line change
Expand Up @@ -716,4 +716,23 @@ repository_secret_is_stale[stale] := true{
"update date" : time.format(secret.updated_at),
}

}

# METADATA
# scope: rule
# title: Secret Scanning should be enabled
# description: Repository should have secret scanning enabled. Secret scanning helps prevent the exposure of sensitive information and ensures compliance.
# custom:
# remediationSteps:
# - 1. Go to the repository settings page
# - 2. Under the 'Security' title on the left, select 'Code security and analysis'
# - 3. Under 'Secret scanning', click 'Enable'
# severity: MEDIUM
# requiredScopes: [repo]
# prerequisites: [advanced_security]
# threat: Exposed secrets increases the risk of sensitive information such as API keys, passwords, and tokens being disclosed, leading to unauthorized access to systems and services, and data breaches.
default secret_scanning_not_enabled := true

secret_scanning_not_enabled := false{
input.security_and_analysis.secret_scanning.status == "enabled"
}
22 changes: 22 additions & 0 deletions test/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,28 @@ func TestRepositoryWithNoSecrets(t *testing.T) {
repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub)
}

func TestRepositorySecretScanning(t *testing.T) {
name := "repository secret scanning is disabled"
testedPolicyName := "secret_scanning_not_enabled"
makeMockData := func(flag string) githubcollected.Repository {
return githubcollected.Repository{
SecurityAndAnalysis: &github.SecurityAndAnalysis{
SecretScanning: &github.SecretScanning{Status: &flag},
},
}
}

options := map[bool]string{
false: "enabled",
true: "disabled",
}

for _, expectFailure := range bools {
flag := options[expectFailure]
repositoryTestTemplate(t, name, makeMockData(flag), testedPolicyName, expectFailure, scm_type.GitHub)
}
}

func TestGitlabRepositoryTooManyAdmins(t *testing.T) {
name := "Project Has Too Many Owners"
testedPolicyName := "project_has_too_many_admins"
Expand Down

0 comments on commit daec3b4

Please sign in to comment.