From ada86e5821f501043de5eb3d2619cc107210b06a Mon Sep 17 00:00:00 2001 From: tal-legit Date: Wed, 24 Apr 2024 16:22:22 +0300 Subject: [PATCH 01/12] added repo secrets collection and policy --- internal/clients/github/client.go | 23 ++++++++---- internal/collected/github/repository.go | 1 + .../collectors/github/repository_collector.go | 17 +++++++-- policies/github/repository.rego | 35 +++++++++++++++++++ 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/internal/clients/github/client.go b/internal/clients/github/client.go index 94931248..b165412f 100644 --- a/internal/clients/github/client.go +++ b/internal/clients/github/client.go @@ -4,12 +4,6 @@ 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" @@ -18,6 +12,11 @@ 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" @@ -83,7 +82,6 @@ func (c *Client) initClients(ctx context.Context, token string) error { var ghClient *gh.Client var graphQLClient *githubv4.Client - rawClient, graphQLRawClient, err := newHttpClients(ctx, token) if err != nil { return err @@ -636,3 +634,14 @@ func (c *Client) GetSecurityAndAnalysisForEnterprise(enterprise string) (*types. } return &p, nil } + +func (c *Client) GetRepositorySecrets(repo, owner string) (*gh.Secrets, error) { + secrets, res, err := c.client.Actions.ListRepoSecrets(c.context, repo, owner, nil) + if err != nil { + return nil, err + } + if res.StatusCode != 200 { + return nil, fmt.Errorf("unexpected HTTP status: %d", res.StatusCode) + } + return secrets, nil +} diff --git a/internal/collected/github/repository.go b/internal/collected/github/repository.go index 970181ff..17485aca 100644 --- a/internal/collected/github/repository.go +++ b/internal/collected/github/repository.go @@ -71,6 +71,7 @@ type Repository struct { ActionsTokenPermissions *types.TokenPermissions `json:"actions_token_permissions"` DependencyGraphManifests *GitHubQLDependencyGraphManifests `json:"dependency_graph_manifests"` RulesSet []*types.RepositoryRule `json:"rules_set,omitempty"` + RepoSecrets *github.Secrets `json:"secret_update_date,omitempty"` } func (r Repository) ViolationEntityType() string { diff --git a/internal/collectors/github/repository_collector.go b/internal/collectors/github/repository_collector.go index 227c914f..21b106f9 100644 --- a/internal/collectors/github/repository_collector.go +++ b/internal/collectors/github/repository_collector.go @@ -3,13 +3,12 @@ package github import ( "context" "fmt" - "log" - "net/http" - "github.com/Legit-Labs/legitify/internal/collectors" "github.com/Legit-Labs/legitify/internal/common/types" "github.com/Legit-Labs/legitify/internal/context_utils" "github.com/Legit-Labs/legitify/internal/scorecard" + "log" + "net/http" "github.com/Legit-Labs/legitify/internal/common/group_waiter" "github.com/Legit-Labs/legitify/internal/common/permissions" @@ -246,6 +245,8 @@ func (rc *repositoryCollector) collectExtraData(login string, repo = rc.withRepositoryHooks(repo, login) repo = rc.withRepoCollaborators(repo, login) repo = rc.withActionsSettings(repo, login) + repo = rc.withSecrets(repo, login) + fmt.Println(repo) repo, err = rc.withDependencyGraphManifestsCount(repo, login) if err != nil { @@ -388,6 +389,16 @@ func (rc *repositoryCollector) withRulesSet(repository ghcollected.Repository, o return repository, nil } +func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, login string) ghcollected.Repository { + secrets, err := rc.Client.GetRepositorySecrets(repository.Name(), login) + if err != nil { + //add prerequisite / scope / error + return repository + } + repository.RepoSecrets = secrets + return repository +} + // 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) { diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 47b31948..93b37f3d 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -538,4 +538,39 @@ users_allowed_to_bypass_ruleset := false { some index rule := input.rules_set[index] count(rule.ruleset.bypass_actors) == 0 +} + +# METADATA +# scope: rule +# title: Repository Has Stale Secrets +# description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. +# custom: +# remediationSteps: +# -Enter your repository's landing page +# -Go to the settings tab +# -Enter your repository's landing page +# -Go to the settings tab +# -Enter your repository's landing page +# -Go to the settings tab +# -Enter your repository's landing page +# -Go to the settings tab +# severity: HIGH +# requiredScopes: [repo] +# threat: There may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. In addition, sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. +repository_secret_is_stale[stale] := true{ + some index + secret := input.RepoSecrets.Secrets[index] + is_stale(secret.UpdatedAt) + stale :={ + "name" : secret.Name, + "update date" : secret.UpdatedAt, + } + +} + +is_stale(date) { + ns_per_year := 365 * 24 * 60 * 60 * 1000000000 + event_time_ns := time.parse_rfc3339_ns(date) + diff_ns := time.now_ns() - event_time_ns + diff_ns > ns_per_year } \ No newline at end of file From 99dcdb8c599233c90b3ede0810292761ea527ff5 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Wed, 24 Apr 2024 16:39:13 +0300 Subject: [PATCH 02/12] finished description --- internal/collected/github/repository.go | 2 +- policies/github/repository.rego | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/internal/collected/github/repository.go b/internal/collected/github/repository.go index 17485aca..1d5652e0 100644 --- a/internal/collected/github/repository.go +++ b/internal/collected/github/repository.go @@ -71,7 +71,7 @@ type Repository struct { ActionsTokenPermissions *types.TokenPermissions `json:"actions_token_permissions"` DependencyGraphManifests *GitHubQLDependencyGraphManifests `json:"dependency_graph_manifests"` RulesSet []*types.RepositoryRule `json:"rules_set,omitempty"` - RepoSecrets *github.Secrets `json:"secret_update_date,omitempty"` + RepoSecrets *github.Secrets `json:"repository_secrets,omitempty"` } func (r Repository) ViolationEntityType() string { diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 93b37f3d..0fe81807 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -548,18 +548,16 @@ users_allowed_to_bypass_ruleset := false { # remediationSteps: # -Enter your repository's landing page # -Go to the settings tab -# -Enter your repository's landing page -# -Go to the settings tab -# -Enter your repository's landing page -# -Go to the settings tab -# -Enter your repository's landing page -# -Go to the settings tab -# severity: HIGH +# -Under the 'Security' title on the left, choose 'Secrets and variables' +# -Click 'Actions' +# -Sort secrets by 'Last Updated' +# -Regenerate every secret older than one year and add the new value to GitHub's secret manager +# severity: MEDIUM # requiredScopes: [repo] # threat: There may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. In addition, sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. repository_secret_is_stale[stale] := true{ some index - secret := input.RepoSecrets.Secrets[index] + secret := input.repository_secrets.Secrets[index] is_stale(secret.UpdatedAt) stale :={ "name" : secret.Name, From 94a11fbc4b00cab1df3172222f6bbfefc88f7605 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Thu, 25 Apr 2024 15:59:03 +0300 Subject: [PATCH 03/12] edits for policy --- internal/clients/github/client.go | 2 +- internal/collected/github/repository.go | 7 ++++++- internal/collectors/github/repository_collector.go | 7 +++++-- policies/github/repository.rego | 11 +++++------ 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/internal/clients/github/client.go b/internal/clients/github/client.go index b165412f..e37c12cb 100644 --- a/internal/clients/github/client.go +++ b/internal/clients/github/client.go @@ -636,7 +636,7 @@ func (c *Client) GetSecurityAndAnalysisForEnterprise(enterprise string) (*types. } func (c *Client) GetRepositorySecrets(repo, owner string) (*gh.Secrets, error) { - secrets, res, err := c.client.Actions.ListRepoSecrets(c.context, repo, owner, nil) + secrets, res, err := c.client.Actions.ListRepoSecrets(c.context, owner, repo, nil) if err != nil { return nil, err } diff --git a/internal/collected/github/repository.go b/internal/collected/github/repository.go index 1d5652e0..35bfe728 100644 --- a/internal/collected/github/repository.go +++ b/internal/collected/github/repository.go @@ -71,7 +71,12 @@ type Repository struct { ActionsTokenPermissions *types.TokenPermissions `json:"actions_token_permissions"` DependencyGraphManifests *GitHubQLDependencyGraphManifests `json:"dependency_graph_manifests"` RulesSet []*types.RepositoryRule `json:"rules_set,omitempty"` - RepoSecrets *github.Secrets `json:"repository_secrets,omitempty"` + RepoSecrets []*RepositorySecret `json:"repository_secrets,omitempty"` +} + +type RepositorySecret struct { + Name string `json:"name"` + UpdatedAt int `json:"updated_at"` } func (r Repository) ViolationEntityType() string { diff --git a/internal/collectors/github/repository_collector.go b/internal/collectors/github/repository_collector.go index 21b106f9..0697f74d 100644 --- a/internal/collectors/github/repository_collector.go +++ b/internal/collectors/github/repository_collector.go @@ -246,7 +246,6 @@ func (rc *repositoryCollector) collectExtraData(login string, repo = rc.withRepoCollaborators(repo, login) repo = rc.withActionsSettings(repo, login) repo = rc.withSecrets(repo, login) - fmt.Println(repo) repo, err = rc.withDependencyGraphManifestsCount(repo, login) if err != nil { @@ -395,7 +394,11 @@ func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, lo //add prerequisite / scope / error return repository } - repository.RepoSecrets = secrets + var repoSecrets []*ghcollected.RepositorySecret + for i := 0; i < len(secrets.Secrets); i++ { + repoSecrets = append(repoSecrets, &ghcollected.RepositorySecret{secrets.Secrets[i].Name, int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) + } + repository.RepoSecrets = repoSecrets return repository } diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 0fe81807..35bbb047 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -557,18 +557,17 @@ users_allowed_to_bypass_ruleset := false { # threat: There may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. In addition, sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. repository_secret_is_stale[stale] := true{ some index - secret := input.repository_secrets.Secrets[index] - is_stale(secret.UpdatedAt) + secret := input.repository_secrets[index] + is_stale(secret.updated_at) stale :={ - "name" : secret.Name, - "update date" : secret.UpdatedAt, + "name" : secret.name, + "update date" : secret.updated_at, } } is_stale(date) { ns_per_year := 365 * 24 * 60 * 60 * 1000000000 - event_time_ns := time.parse_rfc3339_ns(date) - diff_ns := time.now_ns() - event_time_ns + diff_ns := time.now_ns() - date diff_ns > ns_per_year } \ No newline at end of file From d25aa24de8cd3dfbb8de87ed644b6660f10fb9d6 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Thu, 25 Apr 2024 16:49:31 +0300 Subject: [PATCH 04/12] remediation steps --- policies/github/repository.rego | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 35bbb047..1511a341 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -546,20 +546,20 @@ users_allowed_to_bypass_ruleset := false { # description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. # custom: # remediationSteps: -# -Enter your repository's landing page -# -Go to the settings tab -# -Under the 'Security' title on the left, choose 'Secrets and variables' -# -Click 'Actions' -# -Sort secrets by 'Last Updated' -# -Regenerate every secret older than one year and add the new value to GitHub's secret manager +# - Enter your repository's landing page +# - Go to the settings tab +# - Under the 'Security' title on the left, choose 'Secrets and variables' +# - Click 'Actions' +# - Sort secrets by 'Last Updated' +# - Regenerate every secret older than one year and add the new value to GitHub's secret manager # severity: MEDIUM # requiredScopes: [repo] -# threat: There may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. In addition, sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. +# threat: Sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. In addition, there may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. repository_secret_is_stale[stale] := true{ some index secret := input.repository_secrets[index] is_stale(secret.updated_at) - stale :={ + stale := { "name" : secret.name, "update date" : secret.updated_at, } From 32d843dfa5463acaed3b07b20cdcd5fccc98a05b Mon Sep 17 00:00:00 2001 From: tal-legit Date: Wed, 8 May 2024 18:35:30 +0300 Subject: [PATCH 05/12] added org stale polciy, enricher, and tests --- internal/clients/github/client.go | 11 ++++ internal/collected/github/organization.go | 6 ++ .../github/organization_collector.go | 16 +++++ internal/enricher/enricher_manager.go | 1 + .../enrichers/secrets_list_enricher.go | 61 ++++++++++++++++++ policies/github/organization.rego | 33 ++++++++++ policies/github/repository.rego | 7 ++- test/organization_test.go | 57 +++++++++++++++++ test/repository_test.go | 62 +++++++++++++++++++ 9 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 internal/enricher/enrichers/secrets_list_enricher.go diff --git a/internal/clients/github/client.go b/internal/clients/github/client.go index e37c12cb..0724c81d 100644 --- a/internal/clients/github/client.go +++ b/internal/clients/github/client.go @@ -645,3 +645,14 @@ func (c *Client) GetRepositorySecrets(repo, owner string) (*gh.Secrets, error) { } return secrets, nil } + +func (c *Client) GetOrganizationSecrets(org string) (*gh.Secrets, error) { + secrets, res, err := c.client.Actions.ListOrgSecrets(c.context, org, nil) + if err != nil { + return nil, err + } + if res.StatusCode != 200 { + return nil, fmt.Errorf("unexpected HTTP status: %d", res.StatusCode) + } + return secrets, nil +} diff --git a/internal/collected/github/organization.go b/internal/collected/github/organization.go index 97a13868..b0d46174 100644 --- a/internal/collected/github/organization.go +++ b/internal/collected/github/organization.go @@ -45,6 +45,12 @@ type Organization struct { SamlEnabled *bool `json:"saml_enabled,omitempty"` Hooks []*github.Hook `json:"hooks"` UserRole permissions.OrganizationRole + OrgSecrets []*OrganizationSecret `json:"organization_secrets,omitempty"` +} + +type OrganizationSecret struct { + Name string `json:"name"` + UpdatedAt int `json:"updated_at"` } func (o Organization) ViolationEntityType() string { diff --git a/internal/collectors/github/organization_collector.go b/internal/collectors/github/organization_collector.go index 4844ca5e..db9b69f3 100644 --- a/internal/collectors/github/organization_collector.go +++ b/internal/collectors/github/organization_collector.go @@ -91,10 +91,13 @@ func (c *organizationCollector) collectExtraData(org *ghcollected.ExtendedOrg) g c.IssueMissingPermissions(perm) } + secrets := c.collectOrgSecrets(org.Name()) + return ghcollected.Organization{ Organization: org, SamlEnabled: samlEnabled, Hooks: hooks, + OrgSecrets: secrets, } } @@ -128,3 +131,16 @@ func (c *organizationCollector) collectOrgSamlData(org string) (*bool, error) { return &samlEnabled, nil } + +func (c *organizationCollector) collectOrgSecrets(org string) []*ghcollected.OrganizationSecret { + secrets, err := c.Client.GetOrganizationSecrets(org) + if err != nil { + return nil + } + var orgSecrets []*ghcollected.OrganizationSecret + for i := 0; i < len(secrets.Secrets); i++ { + orgSecrets = append(orgSecrets, &ghcollected.OrganizationSecret{secrets.Secrets[i].Name, int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) + } + + return orgSecrets +} diff --git a/internal/enricher/enricher_manager.go b/internal/enricher/enricher_manager.go index 2e1d49ef..a4f35090 100644 --- a/internal/enricher/enricher_manager.go +++ b/internal/enricher/enricher_manager.go @@ -50,6 +50,7 @@ var mapping = map[string]enrichers.Enricher{ enrichers.Scorecard: enrichers.NewScorecardEnricher(), enrichers.MembersList: enrichers.NewMembersListEnricher(), enrichers.HooksList: enrichers.NewHooksListEnricher(), + enrichers.SecretsList: enrichers.NewSecretsListEnricher(), } func NewEnricherManager() EnricherManager { diff --git a/internal/enricher/enrichers/secrets_list_enricher.go b/internal/enricher/enrichers/secrets_list_enricher.go new file mode 100644 index 00000000..56c3b35a --- /dev/null +++ b/internal/enricher/enrichers/secrets_list_enricher.go @@ -0,0 +1,61 @@ +package enrichers + +import ( + "encoding/json" + "fmt" + "github.com/Legit-Labs/legitify/internal/analyzers" + "github.com/Legit-Labs/legitify/internal/common/map_utils" + "github.com/iancoleman/orderedmap" + "golang.org/x/net/context" + "log" +) + +const SecretsList = "secretsList" + +func NewSecretsListEnricher() secretsListEnricher { + return secretsListEnricher{} +} + +type secretsListEnricher struct { +} + +func (e secretsListEnricher) Enrich(_ context.Context, data analyzers.AnalyzedData) (Enrichment, bool) { + result, err := createSecretListEnrichment(data.ExtraData) + if err != nil { + log.Printf("failed to enrich secrets list: %v", err) + return nil, false + } + return result, true +} + +func (e secretsListEnricher) Parse(data interface{}) (Enrichment, error) { + return NewGenericListEnrichmentFromInterface(data) +} + +func createSecretListEnrichment(extraData interface{}) (GenericListEnrichment, error) { + asMap, ok := extraData.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("invalid hookslist extra data") + } + + result := []orderedmap.OrderedMap{} + for k := range asMap { + var secretsEnrichment map[string]string + + err := json.Unmarshal([]byte(k), &secretsEnrichment) + if err != nil { + return nil, err + } + + result = append(result, *map_utils.ToKeySortedMap(secretsEnrichment)) + } + + // order by url to maintain a determenistic order + //sort.Slice(result, func(i, j int) bool { + // urlI := map_utils.UnsafeGet[string](&result[i], "url") + // urlJ := map_utils.UnsafeGet[string](&result[j], "url") + // return strings.Compare(urlI, urlJ) < 0 + //}) + + return result, nil +} diff --git a/policies/github/organization.rego b/policies/github/organization.rego index 5029e804..70b35247 100644 --- a/policies/github/organization.rego +++ b/policies/github/organization.rego @@ -110,3 +110,36 @@ default organization_not_using_single_sign_on := true organization_not_using_single_sign_on := false { input.saml_enabled } + +# METADATA +# scope: rule +# title: Organization Has Stale Secrets +# description: Some of the organizations secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. +# custom: +# requiredEnrichers: [secretsList] +# remediationSteps: +# - Enter your organization's landing page +# - Go to the settings tab +# - Under the 'Security' title on the left, choose 'Secrets and variables' +# - Click 'Actions' +# - Sort secrets by 'Last Updated' +# - Regenerate every secret older than one year and add the new value to GitHub's secret manager +# severity: MEDIUM +# requiredScopes: [repo] +# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. +organization_secret_is_stale[stale] := true{ + some index + secret := input.organization_secrets[index] + is_stale(secret.updated_at) + stale := { + "name" : secret.name, + "update date" : time.format(secret.updated_at), + } + +} + +is_stale(date) { + ns_per_year := 365 * 24 * 60 * 60 * 1000000000 + diff_ns := time.now_ns() - date + diff_ns > ns_per_year +} diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 1511a341..20d3353c 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -545,6 +545,7 @@ users_allowed_to_bypass_ruleset := false { # title: Repository Has Stale Secrets # description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. # custom: +# requiredEnrichers: [secretsList] # remediationSteps: # - Enter your repository's landing page # - Go to the settings tab @@ -554,14 +555,14 @@ users_allowed_to_bypass_ruleset := false { # - Regenerate every secret older than one year and add the new value to GitHub's secret manager # severity: MEDIUM # requiredScopes: [repo] -# threat: Sensitive data may have been inadvertently been made public in the past, and an attacker that hold this data may gain access to your currents CI and services. In addition, there may be unused unnecessary tokens that have not been inspected and embody a possible attack surface. +# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. repository_secret_is_stale[stale] := true{ some index secret := input.repository_secrets[index] is_stale(secret.updated_at) stale := { - "name" : secret.name, - "update date" : secret.updated_at, + "name" : secret.name, + "update date" : time.format(secret.updated_at), } } diff --git a/test/organization_test.go b/test/organization_test.go index 9529ad59..ab968d4b 100644 --- a/test/organization_test.go +++ b/test/organization_test.go @@ -4,6 +4,7 @@ import ( "github.com/Legit-Labs/legitify/internal/common/scm_type" "github.com/google/go-github/v53/github" "testing" + "time" githubcollected "github.com/Legit-Labs/legitify/internal/collected/github" "github.com/Legit-Labs/legitify/internal/common/namespace" @@ -14,6 +15,7 @@ type organizationMockConfiguration struct { ssoEnabled *bool name string url string + secrets []*githubcollected.OrganizationSecret } func newOrganizationMock(config organizationMockConfiguration) githubcollected.Organization { @@ -30,11 +32,16 @@ func newOrganizationMock(config organizationMockConfiguration) githubcollected.O } hooks = append(hooks, &hook) } + var orgSecrets []*githubcollected.OrganizationSecret = nil + if config.secrets != nil { + orgSecrets = append(orgSecrets, config.secrets...) + } return githubcollected.Organization{ Organization: nil, SamlEnabled: &samlEnabledMockResult, Hooks: hooks, + OrgSecrets: orgSecrets, } } @@ -113,6 +120,56 @@ func TestOrganization(t *testing.T) { ssoEnabled: &boolTrue, }, }, + { + name: "Organization has no stale secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: false, + args: organizationMockConfiguration{ + secrets: []*githubcollected.OrganizationSecret{ + { + Name: "test1", + UpdatedAt: int(time.Now().UnixNano()) - 2628000000000000, // one month + }, + { + Name: "test2", + UpdatedAt: int(time.Now().UnixNano()) - (3 * 2628000000000000), // three months + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()) - (6 * 2628000000000000), // six month + }, + }, + }, + }, + { + name: "Organization has stale secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: true, + args: organizationMockConfiguration{ + secrets: []*githubcollected.OrganizationSecret{ + { + Name: "test1", + UpdatedAt: 1652020546000000000, //08.05.2022 + }, + { + Name: "test2", + UpdatedAt: 957796546000000000, //08.05.2000 + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()), + }, + }, + }, + }, + { + name: "Organization has no secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: false, + args: organizationMockConfiguration{ + secrets: nil, + }, + }, } for _, test := range tests { diff --git a/test/repository_test.go b/test/repository_test.go index a74bfc96..2266dfb8 100644 --- a/test/repository_test.go +++ b/test/repository_test.go @@ -309,6 +309,68 @@ func TestRepositoryActionsSettingsActionsCanApprovePullRequests(t *testing.T) { } } +func TestRepositoryWithNoStaleSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: []*githubcollected.RepositorySecret{ + { + Name: "test1", + UpdatedAt: int(time.Now().UnixNano()) - 2628000000000000, // one month + }, + { + Name: "test2", + UpdatedAt: int(time.Now().UnixNano()) - (3 * 2628000000000000), // three months + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()) - (6 * 2628000000000000), // six month + }, + }, + } + } + expectFailure := false + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + +func TestRepositoryWithStaleSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: []*githubcollected.RepositorySecret{ + { + Name: "test1", + UpdatedAt: 1652020546000000000, //08.05.2022 + }, + { + Name: "test2", + UpdatedAt: 957796546000000000, //08.05.2000 + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()), + }, + }, + } + } + expectFailure := true + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + +func TestRepositoryWithNoSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: nil, + } + } + expectFailure := false + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + func TestGitlabRepositoryTooManyAdmins(t *testing.T) { name := "Project Has Too Many Owners" testedPolicyName := "project_has_too_many_admins" From 21ac85b205f4dabe171858f776faf0b92c855340 Mon Sep 17 00:00:00 2001 From: Tal-Legit <154063186+Tal-Legit@users.noreply.github.com> Date: Wed, 8 May 2024 18:46:34 +0300 Subject: [PATCH 06/12] Update secrets_list_enricher.go --- internal/enricher/enrichers/secrets_list_enricher.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/enricher/enrichers/secrets_list_enricher.go b/internal/enricher/enrichers/secrets_list_enricher.go index 56c3b35a..44b0b1d5 100644 --- a/internal/enricher/enrichers/secrets_list_enricher.go +++ b/internal/enricher/enrichers/secrets_list_enricher.go @@ -50,12 +50,5 @@ func createSecretListEnrichment(extraData interface{}) (GenericListEnrichment, e result = append(result, *map_utils.ToKeySortedMap(secretsEnrichment)) } - // order by url to maintain a determenistic order - //sort.Slice(result, func(i, j int) bool { - // urlI := map_utils.UnsafeGet[string](&result[i], "url") - // urlJ := map_utils.UnsafeGet[string](&result[j], "url") - // return strings.Compare(urlI, urlJ) < 0 - //}) - return result, nil } From 6a2c2f657028dec5c1d562ae8358f15f54e42353 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Wed, 8 May 2024 21:38:57 +0300 Subject: [PATCH 07/12] lint --- internal/collectors/github/organization_collector.go | 4 +++- internal/collectors/github/repository_collector.go | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/collectors/github/organization_collector.go b/internal/collectors/github/organization_collector.go index db9b69f3..3628712d 100644 --- a/internal/collectors/github/organization_collector.go +++ b/internal/collectors/github/organization_collector.go @@ -139,7 +139,9 @@ func (c *organizationCollector) collectOrgSecrets(org string) []*ghcollected.Org } var orgSecrets []*ghcollected.OrganizationSecret for i := 0; i < len(secrets.Secrets); i++ { - orgSecrets = append(orgSecrets, &ghcollected.OrganizationSecret{secrets.Secrets[i].Name, int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) + orgSecrets = append(orgSecrets, &ghcollected.OrganizationSecret{ + Name: secrets.Secrets[i].Name, + UpdatedAt: int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) } return orgSecrets diff --git a/internal/collectors/github/repository_collector.go b/internal/collectors/github/repository_collector.go index 0697f74d..6802bc5c 100644 --- a/internal/collectors/github/repository_collector.go +++ b/internal/collectors/github/repository_collector.go @@ -396,7 +396,10 @@ func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, lo } var repoSecrets []*ghcollected.RepositorySecret for i := 0; i < len(secrets.Secrets); i++ { - repoSecrets = append(repoSecrets, &ghcollected.RepositorySecret{secrets.Secrets[i].Name, int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) + repoSecrets = append(repoSecrets, &ghcollected.RepositorySecret{ + Name: secrets.Secrets[i].Name, + UpdatedAt: int(secrets.Secrets[i].UpdatedAt.Time.UnixNano()), + }) } repository.RepoSecrets = repoSecrets return repository From 22570bfa605ba4073b5d305a0fd8caf367d4be3a Mon Sep 17 00:00:00 2001 From: tal-legit Date: Sun, 12 May 2024 11:36:00 +0300 Subject: [PATCH 08/12] split rego into two files --- policies/github/repository.rego | 33 -------------------------- policies/github/repository_two.rego | 36 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 33 deletions(-) create mode 100644 policies/github/repository_two.rego diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 20d3353c..47b31948 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -538,37 +538,4 @@ users_allowed_to_bypass_ruleset := false { some index rule := input.rules_set[index] count(rule.ruleset.bypass_actors) == 0 -} - -# METADATA -# scope: rule -# title: Repository Has Stale Secrets -# description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. -# custom: -# requiredEnrichers: [secretsList] -# remediationSteps: -# - Enter your repository's landing page -# - Go to the settings tab -# - Under the 'Security' title on the left, choose 'Secrets and variables' -# - Click 'Actions' -# - Sort secrets by 'Last Updated' -# - Regenerate every secret older than one year and add the new value to GitHub's secret manager -# severity: MEDIUM -# requiredScopes: [repo] -# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. -repository_secret_is_stale[stale] := true{ - some index - secret := input.repository_secrets[index] - is_stale(secret.updated_at) - stale := { - "name" : secret.name, - "update date" : time.format(secret.updated_at), - } - -} - -is_stale(date) { - ns_per_year := 365 * 24 * 60 * 60 * 1000000000 - diff_ns := time.now_ns() - date - diff_ns > ns_per_year } \ No newline at end of file diff --git a/policies/github/repository_two.rego b/policies/github/repository_two.rego new file mode 100644 index 00000000..2aee2072 --- /dev/null +++ b/policies/github/repository_two.rego @@ -0,0 +1,36 @@ +package repository + +import data.common.webhooks as webhookUtils + +# METADATA +# scope: rule +# title: Repository Has Stale Secrets +# description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. +# custom: +# requiredEnrichers: [secretsList] +# remediationSteps: +# - Enter your repository's landing page +# - Go to the settings tab +# - Under the 'Security' title on the left, choose 'Secrets and variables' +# - Click 'Actions' +# - Sort secrets by 'Last Updated' +# - Regenerate every secret older than one year and add the new value to GitHub's secret manager +# severity: MEDIUM +# requiredScopes: [repo] +# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. +repository_secret_is_stale[stale] := true{ + some index + secret := input.repository_secrets[index] + is_stale(secret.updated_at) + stale := { + "name" : secret.name, + "update date" : time.format(secret.updated_at), + } + +} + +is_stale(date) { + ns_per_year := 365 * 24 * 60 * 60 * 1000000000 + diff_ns := time.now_ns() - date + diff_ns > ns_per_year +} \ No newline at end of file From 87c373d56fd2ba129df52b35802a0a87206c1a36 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Sun, 12 May 2024 11:36:56 +0300 Subject: [PATCH 09/12] lint --- policies/github/repository_two.rego | 2 -- 1 file changed, 2 deletions(-) diff --git a/policies/github/repository_two.rego b/policies/github/repository_two.rego index 2aee2072..7cd7bf82 100644 --- a/policies/github/repository_two.rego +++ b/policies/github/repository_two.rego @@ -1,7 +1,5 @@ package repository -import data.common.webhooks as webhookUtils - # METADATA # scope: rule # title: Repository Has Stale Secrets From 1216c7de77e05e51ae07b68ca7950b3318b7b1c9 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Sun, 12 May 2024 11:42:52 +0300 Subject: [PATCH 10/12] fit bundle tests --- policies/bundle_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policies/bundle_test.go b/policies/bundle_test.go index 670fb745..13ecc4f6 100644 --- a/policies/bundle_test.go +++ b/policies/bundle_test.go @@ -46,5 +46,5 @@ func TestPoliciesBundle(t *testing.T) { count, err := countBundles() require.Nilf(t, err, "counting files: %v", err) - require.Equal(t, count, 7, "Expecting 7 files in bundle") + require.Equal(t, count, 8, "Expecting 8 files in bundle") } From 06f756150124bf9e8da11821ad83edf602943318 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Mon, 13 May 2024 13:36:40 +0300 Subject: [PATCH 11/12] moved to functions to common, passing errors and fixed rego logic --- .../collectors/github/organization_collector.go | 12 ++++++++---- .../collectors/github/repository_collector.go | 12 +++++++----- policies/github/common/members.rego | 13 +++++++++++++ policies/github/common/secrets.rego | 6 ++++++ policies/github/member.rego | 15 ++++----------- policies/github/organization.rego | 12 +++--------- policies/github/repository.rego | 2 +- policies/github/repository_two.rego | 10 +++------- test/member_test.go | 2 +- 9 files changed, 46 insertions(+), 38 deletions(-) create mode 100644 policies/github/common/members.rego create mode 100644 policies/github/common/secrets.rego diff --git a/internal/collectors/github/organization_collector.go b/internal/collectors/github/organization_collector.go index 3628712d..3ef9337e 100644 --- a/internal/collectors/github/organization_collector.go +++ b/internal/collectors/github/organization_collector.go @@ -91,7 +91,11 @@ func (c *organizationCollector) collectExtraData(org *ghcollected.ExtendedOrg) g c.IssueMissingPermissions(perm) } - secrets := c.collectOrgSecrets(org.Name()) + secrets, err := c.collectOrgSecrets(org.Name()) + if err != nil { + secrets = nil + log.Printf("failed to collect secrets for %s, %s", org.Name(), err) + } return ghcollected.Organization{ Organization: org, @@ -132,10 +136,10 @@ func (c *organizationCollector) collectOrgSamlData(org string) (*bool, error) { } -func (c *organizationCollector) collectOrgSecrets(org string) []*ghcollected.OrganizationSecret { +func (c *organizationCollector) collectOrgSecrets(org string) ([]*ghcollected.OrganizationSecret, error) { secrets, err := c.Client.GetOrganizationSecrets(org) if err != nil { - return nil + return nil, err } var orgSecrets []*ghcollected.OrganizationSecret for i := 0; i < len(secrets.Secrets); i++ { @@ -144,5 +148,5 @@ func (c *organizationCollector) collectOrgSecrets(org string) []*ghcollected.Org UpdatedAt: int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) } - return orgSecrets + return orgSecrets, nil } diff --git a/internal/collectors/github/repository_collector.go b/internal/collectors/github/repository_collector.go index 6802bc5c..f3148e08 100644 --- a/internal/collectors/github/repository_collector.go +++ b/internal/collectors/github/repository_collector.go @@ -245,7 +245,10 @@ func (rc *repositoryCollector) collectExtraData(login string, repo = rc.withRepositoryHooks(repo, login) repo = rc.withRepoCollaborators(repo, login) repo = rc.withActionsSettings(repo, login) - repo = rc.withSecrets(repo, login) + repo, err = rc.withSecrets(repo, login) + if err != nil { + log.Printf("failed to collect repository secrets for %s: %s", repo.Repository.Name, err) + } repo, err = rc.withDependencyGraphManifestsCount(repo, login) if err != nil { @@ -388,11 +391,10 @@ func (rc *repositoryCollector) withRulesSet(repository ghcollected.Repository, o return repository, nil } -func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, login string) ghcollected.Repository { +func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, login string) (ghcollected.Repository, error) { secrets, err := rc.Client.GetRepositorySecrets(repository.Name(), login) if err != nil { - //add prerequisite / scope / error - return repository + return repository, err } var repoSecrets []*ghcollected.RepositorySecret for i := 0; i < len(secrets.Secrets); i++ { @@ -402,7 +404,7 @@ func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, lo }) } repository.RepoSecrets = repoSecrets - return repository + return repository, nil } // fixBranchProtectionInfo fixes the branch protection info for the repository, diff --git a/policies/github/common/members.rego b/policies/github/common/members.rego new file mode 100644 index 00000000..fadadd3c --- /dev/null +++ b/policies/github/common/members.rego @@ -0,0 +1,13 @@ +package common.members + +isStale(target_last_active, count_months) { + now := time.now_ns() + diff := time.diff(now, target_last_active) + # diff[0] the year index + diff[0] >= 1 + } else { + now := time.now_ns() + diff := time.diff(now, target_last_active) + # diff[1] the months index + diff[1] >= count_months +} diff --git a/policies/github/common/secrets.rego b/policies/github/common/secrets.rego new file mode 100644 index 00000000..86e9036b --- /dev/null +++ b/policies/github/common/secrets.rego @@ -0,0 +1,6 @@ +package common.secrets + +is_stale(date) { + diff := time.diff(time.now_ns(),date) + diff[0] >= 1 +} diff --git a/policies/github/member.rego b/policies/github/member.rego index ce126f5c..53803e55 100644 --- a/policies/github/member.rego +++ b/policies/github/member.rego @@ -1,5 +1,6 @@ package member +import data.common.members as memberUtils # METADATA # scope: rule # title: Organization Should Have Fewer Than Three Owners @@ -36,7 +37,7 @@ stale_member_found[mem] := true { mem := input.members[member] mem.is_admin == false mem.last_active != -1 - isStale(mem.last_active, 6) + memberUtils.isStale(mem.last_active, 6) } # METADATA @@ -57,13 +58,5 @@ stale_admin_found[mem] := true { mem := input.members[member] mem.is_admin == true mem.last_active != -1 - isStale(mem.last_active, 6) -} - -isStale(target_last_active, count_months) { - now := time.now_ns() - diff := time.diff(now, target_last_active) - - # diff[1] the months index - diff[1] >= count_months -} + memberUtils.isStale(mem.last_active, 6) +} \ No newline at end of file diff --git a/policies/github/organization.rego b/policies/github/organization.rego index 70b35247..968862fe 100644 --- a/policies/github/organization.rego +++ b/policies/github/organization.rego @@ -1,6 +1,7 @@ package organization import data.common.webhooks as webhookUtils +import data.common.secrets as secretUtils # METADATA # scope: rule @@ -125,21 +126,14 @@ organization_not_using_single_sign_on := false { # - Sort secrets by 'Last Updated' # - Regenerate every secret older than one year and add the new value to GitHub's secret manager # severity: MEDIUM -# requiredScopes: [repo] +# requiredScopes: [admin:org, repo] # threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. organization_secret_is_stale[stale] := true{ some index secret := input.organization_secrets[index] - is_stale(secret.updated_at) + secretUtils.is_stale(secret.updated_at) stale := { "name" : secret.name, "update date" : time.format(secret.updated_at), } - -} - -is_stale(date) { - ns_per_year := 365 * 24 * 60 * 60 * 1000000000 - diff_ns := time.now_ns() - date - diff_ns > ns_per_year } diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 47b31948..f1e914de 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -538,4 +538,4 @@ users_allowed_to_bypass_ruleset := false { some index rule := input.rules_set[index] count(rule.ruleset.bypass_actors) == 0 -} \ No newline at end of file +} diff --git a/policies/github/repository_two.rego b/policies/github/repository_two.rego index 7cd7bf82..1d04bc99 100644 --- a/policies/github/repository_two.rego +++ b/policies/github/repository_two.rego @@ -1,5 +1,7 @@ package repository +import data.common.secrets as secretUtils + # METADATA # scope: rule # title: Repository Has Stale Secrets @@ -19,16 +21,10 @@ package repository repository_secret_is_stale[stale] := true{ some index secret := input.repository_secrets[index] - is_stale(secret.updated_at) + secretUtils.is_stale(secret.updated_at) stale := { "name" : secret.name, "update date" : time.format(secret.updated_at), } } - -is_stale(date) { - ns_per_year := 365 * 24 * 60 * 60 * 1000000000 - diff_ns := time.now_ns() - date - diff_ns > ns_per_year -} \ No newline at end of file diff --git a/test/member_test.go b/test/member_test.go index 5246ad34..ce635f51 100644 --- a/test/member_test.go +++ b/test/member_test.go @@ -51,7 +51,7 @@ func TestMember(t *testing.T) { hasLastActive: true, members: []githubcollected.OrganizationMember{ { - LastActive: int(time.Now().AddDate(0, -9, 0).UnixNano()), + LastActive: int(time.Now().AddDate(-1, -2, 0).UnixNano()), IsAdmin: true, }, }, From cb77bfd3b6bb677d2324589139142c5976c884e3 Mon Sep 17 00:00:00 2001 From: tal-legit Date: Mon, 13 May 2024 13:47:17 +0300 Subject: [PATCH 12/12] fixed bundle test --- policies/bundle_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policies/bundle_test.go b/policies/bundle_test.go index 13ecc4f6..e73caea0 100644 --- a/policies/bundle_test.go +++ b/policies/bundle_test.go @@ -46,5 +46,5 @@ func TestPoliciesBundle(t *testing.T) { count, err := countBundles() require.Nilf(t, err, "counting files: %v", err) - require.Equal(t, count, 8, "Expecting 8 files in bundle") + require.Equal(t, count, 10, "Expecting 10 files in bundle") }