From 729df3d6f23cf2018bfd5d6702be7c8dfc5005ac Mon Sep 17 00:00:00 2001 From: Liam Galvin Date: Thu, 18 Aug 2022 10:31:39 +0100 Subject: [PATCH 1/5] fix(terraform): Fix bucket public access block referencing by implied id (#881) fix(terraform): Bucket public access block referencing by implied id --- .../terraform/aws/s3/public_access_block.go | 11 ----- pkg/terraform/block.go | 12 +----- pkg/terraform/presets.go | 40 +++++++++++++++++++ 3 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 pkg/terraform/presets.go diff --git a/internal/adapters/terraform/aws/s3/public_access_block.go b/internal/adapters/terraform/aws/s3/public_access_block.go index 6aae23620..c267381ad 100644 --- a/internal/adapters/terraform/aws/s3/public_access_block.go +++ b/internal/adapters/terraform/aws/s3/public_access_block.go @@ -18,17 +18,6 @@ func (a *adapter) adaptPublicAccessBlocks() { var bucketName string bucketAttr := b.GetAttribute("bucket") - - if bucketAttr.IsNotNil() { - if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, b); err == nil { - if bucket, ok := a.bucketMap[referencedBlock.ID()]; ok { - bucket.PublicAccessBlock = &pba - a.bucketMap[referencedBlock.ID()] = bucket - continue - } - } - } - if bucketAttr.IsString() { bucketName = bucketAttr.Value().AsString() for id, bucket := range a.bucketMap { diff --git a/pkg/terraform/block.go b/pkg/terraform/block.go index acb9d728d..f6a736314 100644 --- a/pkg/terraform/block.go +++ b/pkg/terraform/block.go @@ -435,19 +435,11 @@ func (b *Block) Attributes() map[string]*Attribute { } func (b *Block) Values() cty.Value { - values := make(map[string]cty.Value) - // here we set up common "id" values that are set by the provider - this ensures all blocks have a default - // referencable id/arn. this isn't perfect, but the only way to link blocks in certain circumstances. - values["id"] = cty.StringVal(b.ID()) - values["arn"] = cty.StringVal(b.ID()) - // workaround for weird iam feature - if b.TypeLabel() == "aws_iam_policy_document" { - values["json"] = cty.StringVal(b.ID()) - } + values := createPresetValues(b) for _, attribute := range b.GetAttributes() { values[attribute.Name()] = attribute.Value() } - return cty.ObjectVal(values) + return cty.ObjectVal(postProcessValues(b, values)) } func (b *Block) IsNil() bool { diff --git a/pkg/terraform/presets.go b/pkg/terraform/presets.go new file mode 100644 index 000000000..6810a81ae --- /dev/null +++ b/pkg/terraform/presets.go @@ -0,0 +1,40 @@ +package terraform + +import ( + "strings" + + "github.com/zclconf/go-cty/cty" +) + +func createPresetValues(b *Block) map[string]cty.Value { + presets := make(map[string]cty.Value) + + // here we set up common "id" values that are set by the provider - this ensures all blocks have a default + // referencable id/arn. this isn't perfect, but the only way to link blocks in certain circumstances. + presets["id"] = cty.StringVal(b.ID()) + + if strings.HasPrefix(b.TypeLabel(), "aws_") { + presets["arn"] = cty.StringVal(b.ID()) + } + + // workaround for weird iam feature + switch b.TypeLabel() { + case "aws_iam_policy_document": + presets["json"] = cty.StringVal(b.ID()) + } + + return presets + +} + +func postProcessValues(b *Block, input map[string]cty.Value) map[string]cty.Value { + + // alias id to "bucket" (bucket name) for s3 bucket resources + if strings.HasPrefix(b.TypeLabel(), "aws_s3_bucket") { + if bucket, ok := input["bucket"]; ok { + input["id"] = bucket + } + } + + return input +} From cf940096b26340ff0655c18906bd443104257d12 Mon Sep 17 00:00:00 2001 From: Liam Galvin Date: Thu, 18 Aug 2022 11:58:36 +0100 Subject: [PATCH 2/5] fix(terraform): Fix queue logging of azure storage account being detected when no queues are used (#883) Resolves #882 Signed-off-by: Liam Galvin --- .../adapters/terraform/azure/storage/adapt.go | 7 +++++++ .../storage/queue_services_logging_enabled.go | 2 +- .../queue_services_logging_enabled.tf.go | 5 +++++ .../queue_services_logging_enabled_test.go | 21 +++++++++++++++++++ pkg/providers/azure/storage/storage.go | 6 ++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/adapters/terraform/azure/storage/adapt.go b/internal/adapters/terraform/azure/storage/adapt.go index b15bd8b31..b9deabfb0 100644 --- a/internal/adapters/terraform/azure/storage/adapt.go +++ b/internal/adapters/terraform/azure/storage/adapt.go @@ -85,6 +85,13 @@ func adaptAccounts(modules terraform.Modules) ([]storage.Account, []string, []st accountedForNetworkRules = append(accountedForNetworkRules, networkRuleBlock.ID()) account.NetworkRules = append(account.NetworkRules, adaptNetworkRule(networkRuleBlock)) } + for _, queueBlock := range module.GetReferencingResources(resource, "azurerm_storage_queue", "storage_account_name") { + queue := storage.Queue{ + Metadata: queueBlock.GetMetadata(), + Name: queueBlock.GetAttribute("name").AsStringValueOrDefault("", queueBlock), + } + account.Queues = append(account.Queues, queue) + } accounts = append(accounts, account) } } diff --git a/internal/rules/azure/storage/queue_services_logging_enabled.go b/internal/rules/azure/storage/queue_services_logging_enabled.go index edf558a40..1c23405f6 100755 --- a/internal/rules/azure/storage/queue_services_logging_enabled.go +++ b/internal/rules/azure/storage/queue_services_logging_enabled.go @@ -35,7 +35,7 @@ Requests are logged on a best-effort basis.`, }, func(s *state.State) (results scan.Results) { for _, account := range s.Azure.Storage.Accounts { - if account.IsUnmanaged() { + if account.IsUnmanaged() || len(account.Queues) == 0 { continue } if account.QueueProperties.EnableLogging.IsFalse() { diff --git a/internal/rules/azure/storage/queue_services_logging_enabled.tf.go b/internal/rules/azure/storage/queue_services_logging_enabled.tf.go index 926b185ab..b2189c02b 100644 --- a/internal/rules/azure/storage/queue_services_logging_enabled.tf.go +++ b/internal/rules/azure/storage/queue_services_logging_enabled.tf.go @@ -32,6 +32,11 @@ var terraformQueueServicesLoggingEnabledBadExamples = []string{ queue_properties { } } + + resource "azurerm_storage_queue" "bad_example" { + name = "my-queue" + storage_account_name = azurerm_storage_account.bad_example.name + } `, } diff --git a/internal/rules/azure/storage/queue_services_logging_enabled_test.go b/internal/rules/azure/storage/queue_services_logging_enabled_test.go index 1cc23dfee..68f7e5a93 100644 --- a/internal/rules/azure/storage/queue_services_logging_enabled_test.go +++ b/internal/rules/azure/storage/queue_services_logging_enabled_test.go @@ -29,11 +29,32 @@ func TestCheckQueueServicesLoggingEnabled(t *testing.T) { Metadata: defsecTypes.NewTestMetadata(), EnableLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), }, + Queues: []storage.Queue{ + { + Metadata: defsecTypes.NewTestMetadata(), + Name: defsecTypes.String("my-queue", defsecTypes.NewTestMetadata()), + }, + }, }, }, }, expected: true, }, + { + name: "Storage account queue properties logging disabled with no queues", + input: storage.Storage{ + Accounts: []storage.Account{ + { + Metadata: defsecTypes.NewTestMetadata(), + QueueProperties: storage.QueueProperties{ + Metadata: defsecTypes.NewTestMetadata(), + EnableLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), + }, + }, + }, + }, + expected: false, + }, { name: "Storage account queue properties logging enabled", input: storage.Storage{ diff --git a/pkg/providers/azure/storage/storage.go b/pkg/providers/azure/storage/storage.go index 370cb7add..6d4e3d0b5 100755 --- a/pkg/providers/azure/storage/storage.go +++ b/pkg/providers/azure/storage/storage.go @@ -15,6 +15,12 @@ type Account struct { Containers []Container QueueProperties QueueProperties MinimumTLSVersion defsecTypes.StringValue + Queues []Queue +} + +type Queue struct { + defsecTypes.Metadata + Name defsecTypes.StringValue } type QueueProperties struct { From 146934fa43dd66bdf0ab879db8fb216ff5bc7b6a Mon Sep 17 00:00:00 2001 From: Reed Loden Date: Fri, 19 Aug 2022 01:00:30 -0700 Subject: [PATCH 3/5] feat: GitHub branch protection should require signed commits (#877) check: GitHub branch protection should require signed commits Fixes #876. Port over https://github.com/aquasecurity/tfsec/pull/1101 to defsec. --- .../AVD-GIT-0004/Terraform.md | 14 +++++ .../branch_protections/AVD-GIT-0004/docs.md | 19 ++++++ internal/adapters/terraform/github/adapt.go | 2 + .../github/branch_protections/adapt.go | 30 +++++++++ .../github/branch_protections/adapt_test.go | 59 ++++++++++++++++++ .../require_signed_commits.go | 49 +++++++++++++++ .../require_signed_commits.tf.go | 29 +++++++++ .../require_signed_commits_test.go | 61 +++++++++++++++++++ pkg/providers/github/branch_protections.go | 14 +++++ pkg/providers/github/github.go | 1 + pkg/rules/rules.go | 1 + 11 files changed, 279 insertions(+) create mode 100644 avd_docs/github/branch_protections/AVD-GIT-0004/Terraform.md create mode 100644 avd_docs/github/branch_protections/AVD-GIT-0004/docs.md create mode 100644 internal/adapters/terraform/github/branch_protections/adapt.go create mode 100644 internal/adapters/terraform/github/branch_protections/adapt_test.go create mode 100755 internal/rules/github/branch_protections/require_signed_commits.go create mode 100644 internal/rules/github/branch_protections/require_signed_commits.tf.go create mode 100644 internal/rules/github/branch_protections/require_signed_commits_test.go create mode 100755 pkg/providers/github/branch_protections.go diff --git a/avd_docs/github/branch_protections/AVD-GIT-0004/Terraform.md b/avd_docs/github/branch_protections/AVD-GIT-0004/Terraform.md new file mode 100644 index 000000000..54940cf5f --- /dev/null +++ b/avd_docs/github/branch_protections/AVD-GIT-0004/Terraform.md @@ -0,0 +1,14 @@ + +Require signed commits for a repository + +```hcl +resource "github_branch_protection" "good_example" { + repository_id = "example" + pattern = "main" + + require_signed_commits = true +} +``` + +#### Remediation Links + - https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits diff --git a/avd_docs/github/branch_protections/AVD-GIT-0004/docs.md b/avd_docs/github/branch_protections/AVD-GIT-0004/docs.md new file mode 100644 index 000000000..7ee3833b9 --- /dev/null +++ b/avd_docs/github/branch_protections/AVD-GIT-0004/docs.md @@ -0,0 +1,19 @@ + +GitHub branch protection should be set to require signed commits. + +You can do this by setting the require_signed_commits attribute to 'true'. + +### Impact +Commits may not be verified and signed as coming from a trusted developer + + +{{ remediationActions }} + +### Links +- https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits + +- https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification + +- https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-signed-commits + + diff --git a/internal/adapters/terraform/github/adapt.go b/internal/adapters/terraform/github/adapt.go index a03799289..36452f239 100644 --- a/internal/adapters/terraform/github/adapt.go +++ b/internal/adapters/terraform/github/adapt.go @@ -1,6 +1,7 @@ package github import ( + "github.com/aquasecurity/defsec/internal/adapters/terraform/github/branch_protections" "github.com/aquasecurity/defsec/internal/adapters/terraform/github/repositories" "github.com/aquasecurity/defsec/internal/adapters/terraform/github/secrets" "github.com/aquasecurity/defsec/pkg/providers/github" @@ -11,5 +12,6 @@ func Adapt(modules terraform.Modules) github.GitHub { return github.GitHub{ Repositories: repositories.Adapt(modules), EnvironmentSecrets: secrets.Adapt(modules), + BranchProtections: branch_protections.Adapt(modules), } } diff --git a/internal/adapters/terraform/github/branch_protections/adapt.go b/internal/adapters/terraform/github/branch_protections/adapt.go new file mode 100644 index 000000000..57af17a14 --- /dev/null +++ b/internal/adapters/terraform/github/branch_protections/adapt.go @@ -0,0 +1,30 @@ +package branch_protections + +import ( + "github.com/aquasecurity/defsec/pkg/providers/github" + "github.com/aquasecurity/defsec/pkg/terraform" +) + +func Adapt(modules terraform.Modules) []github.BranchProtection { + return adaptBranchProtections(modules) +} + +func adaptBranchProtections(modules terraform.Modules) []github.BranchProtection { + var branchProtections []github.BranchProtection + for _, module := range modules { + for _, resource := range module.GetResourcesByType("github_branch_protection") { + branchProtections = append(branchProtections, adaptBranchProtection(resource)) + } + } + return branchProtections +} + +func adaptBranchProtection(resource *terraform.Block) github.BranchProtection { + + branchProtection := github.BranchProtection{ + Metadata: resource.GetMetadata(), + RequireSignedCommits: resource.GetAttribute("require_signed_commits").AsBoolValueOrDefault(false, resource), + } + + return branchProtection +} diff --git a/internal/adapters/terraform/github/branch_protections/adapt_test.go b/internal/adapters/terraform/github/branch_protections/adapt_test.go new file mode 100644 index 000000000..44400ecd2 --- /dev/null +++ b/internal/adapters/terraform/github/branch_protections/adapt_test.go @@ -0,0 +1,59 @@ +package branch_protections + +import ( + "testing" + + "github.com/aquasecurity/defsec/internal/adapters/terraform/tftestutil" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_AdaptDefaults(t *testing.T) { + + src := ` +resource "github_branch_protection" "my-repo" { + +} +` + modules := tftestutil.CreateModulesFromSource(t, src, ".tf") + branchProtections := Adapt(modules) + require.Len(t, branchProtections, 1) + branchProtection := branchProtections[0] + + assert.True(t, branchProtection.RequireSignedCommits.IsFalse()) +} + +func Test_Adapt_RequireSignedCommitsEnabled(t *testing.T) { + + src := ` +resource "github_branch_protection" "my-repo" { + require_signed_commits = true +} +` + modules := tftestutil.CreateModulesFromSource(t, src, ".tf") + branchProtections := Adapt(modules) + require.Len(t, branchProtections, 1) + branchProtection := branchProtections[0] + + assert.True(t, branchProtection.RequireSignedCommits.IsTrue()) + assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetStartLine()) + assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetEndLine()) +} + +func Test_Adapt_RequireSignedCommitsDisabled(t *testing.T) { + + src := ` +resource "github_branch_protection" "my-repo" { + require_signed_commits = false +} +` + modules := tftestutil.CreateModulesFromSource(t, src, ".tf") + branchProtections := Adapt(modules) + require.Len(t, branchProtections, 1) + branchProtection := branchProtections[0] + + assert.False(t, branchProtection.RequireSignedCommits.IsTrue()) + assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetStartLine()) + assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetEndLine()) +} diff --git a/internal/rules/github/branch_protections/require_signed_commits.go b/internal/rules/github/branch_protections/require_signed_commits.go new file mode 100755 index 000000000..2719d4e2c --- /dev/null +++ b/internal/rules/github/branch_protections/require_signed_commits.go @@ -0,0 +1,49 @@ +package branch_protections + +import ( + "github.com/aquasecurity/defsec/internal/rules" + "github.com/aquasecurity/defsec/pkg/providers" + "github.com/aquasecurity/defsec/pkg/scan" + "github.com/aquasecurity/defsec/pkg/severity" + "github.com/aquasecurity/defsec/pkg/state" +) + +var CheckRequireSignedCommits = rules.Register( + scan.Rule{ + AVDID: "AVD-GIT-0004", + Provider: providers.GitHubProvider, + Service: "branch_protections", + ShortCode: "require_signed_commits", + Summary: "GitHub branch protection does not require signed commits.", + Impact: "Commits may not be verified and signed as coming from a trusted developer", + Resolution: "Require signed commits", + Explanation: `GitHub branch protection should be set to require signed commits. + +You can do this by setting the require_signed_commits attribute to 'true'.`, + Links: []string{ + "https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits", + "https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification", + "https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-signed-commits", + }, + Terraform: &scan.EngineMetadata{ + GoodExamples: terraformRequireSignedCommitsGoodExamples, + BadExamples: terraformRequireSignedCommitsBadExamples, + Links: terraformRequireSignedCommitsLinks, + RemediationMarkdown: terraformRequireSignedCommitsRemediationMarkdown, + }, + Severity: severity.High, + }, + func(s *state.State) (results scan.Results) { + for _, branchProtection := range s.GitHub.BranchProtections { + if branchProtection.RequireSignedCommits.IsFalse() { + results.Add( + "Branch protection does not require signed commits,", + branchProtection.RequireSignedCommits, + ) + } else { + results.AddPassed(branchProtection) + } + } + return + }, +) diff --git a/internal/rules/github/branch_protections/require_signed_commits.tf.go b/internal/rules/github/branch_protections/require_signed_commits.tf.go new file mode 100644 index 000000000..51aa2736b --- /dev/null +++ b/internal/rules/github/branch_protections/require_signed_commits.tf.go @@ -0,0 +1,29 @@ +package branch_protections + +var terraformRequireSignedCommitsGoodExamples = []string{ + ` + resource "github_branch_protection" "good_example" { + repository_id = "example" + pattern = "main" + + require_signed_commits = true + } + `, +} + +var terraformRequireSignedCommitsBadExamples = []string{ + ` + resource "github_branch_protection" "good_example" { + repository_id = "example" + pattern = "main" + + require_signed_commits = false + } + `, +} + +var terraformRequireSignedCommitsLinks = []string{ + `https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection`, +} + +var terraformRequireSignedCommitsRemediationMarkdown = `` diff --git a/internal/rules/github/branch_protections/require_signed_commits_test.go b/internal/rules/github/branch_protections/require_signed_commits_test.go new file mode 100644 index 000000000..c529c8ce3 --- /dev/null +++ b/internal/rules/github/branch_protections/require_signed_commits_test.go @@ -0,0 +1,61 @@ +package branch_protections + +import ( + "testing" + + defsecTypes "github.com/aquasecurity/defsec/pkg/types" + + "github.com/aquasecurity/defsec/pkg/state" + + "github.com/aquasecurity/defsec/pkg/providers/github" + "github.com/aquasecurity/defsec/pkg/scan" + + "github.com/stretchr/testify/assert" +) + +func TestCheckRequireSignedCommits(t *testing.T) { + tests := []struct { + name string + input []github.BranchProtection + expected bool + }{ + { + name: "Require signed commits enabled for branch", + input: []github.BranchProtection{ + { + Metadata: defsecTypes.NewTestMetadata(), + RequireSignedCommits: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()), + }, + }, + expected: false, + }, + { + name: "Require signed commits disabled for repository", + input: []github.BranchProtection{ + { + Metadata: defsecTypes.NewTestMetadata(), + RequireSignedCommits: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), + }, + }, + expected: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var testState state.State + testState.GitHub.BranchProtections = test.input + results := CheckRequireSignedCommits.Evaluate(&testState) + var found bool + for _, result := range results { + if result.Status() != scan.StatusPassed && result.Rule().LongID() == CheckRequireSignedCommits.Rule().LongID() { + found = true + } + } + if test.expected { + assert.True(t, found, "Rule should have been found") + } else { + assert.False(t, found, "Rule should not have been found") + } + }) + } +} diff --git a/pkg/providers/github/branch_protections.go b/pkg/providers/github/branch_protections.go new file mode 100755 index 000000000..e4f08fe49 --- /dev/null +++ b/pkg/providers/github/branch_protections.go @@ -0,0 +1,14 @@ +package github + +import ( + defsecTypes "github.com/aquasecurity/defsec/pkg/types" +) + +type BranchProtection struct { + defsecTypes.Metadata + RequireSignedCommits defsecTypes.BoolValue +} + +func (b BranchProtection) RequiresSignedCommits() bool { + return b.RequireSignedCommits.IsTrue() +} diff --git a/pkg/providers/github/github.go b/pkg/providers/github/github.go index aecbf3a90..449f94cec 100755 --- a/pkg/providers/github/github.go +++ b/pkg/providers/github/github.go @@ -3,4 +3,5 @@ package github type GitHub struct { Repositories []Repository EnvironmentSecrets []EnvironmentSecret + BranchProtections []BranchProtection } diff --git a/pkg/rules/rules.go b/pkg/rules/rules.go index e65465272..d55d70b9f 100644 --- a/pkg/rules/rules.go +++ b/pkg/rules/rules.go @@ -51,6 +51,7 @@ import ( _ "github.com/aquasecurity/defsec/internal/rules/digitalocean/compute" _ "github.com/aquasecurity/defsec/internal/rules/digitalocean/spaces" _ "github.com/aquasecurity/defsec/internal/rules/github/actions" + _ "github.com/aquasecurity/defsec/internal/rules/github/branch_protections" _ "github.com/aquasecurity/defsec/internal/rules/github/repositories" _ "github.com/aquasecurity/defsec/internal/rules/google/bigquery" _ "github.com/aquasecurity/defsec/internal/rules/google/compute" From 16d866536589cf8087f59c013c58ca3a95715428 Mon Sep 17 00:00:00 2001 From: askingcat Date: Fri, 19 Aug 2022 10:14:15 +0200 Subject: [PATCH 4/5] feat: distinguish failed and ignored for JUnit (#719) (#879) Previously all not passed scan results were treated as a failure by the JUnit formatter. This was a problem for e.g. tfsec, since with a JUnit result viewer there was no way to distinguish between ignored tests and actually failed ones. Now "ignored" scan results are marked as "skipped". Additionally, a test for passed scans has been added as well. It already worked as intended, but so far we had no test to ensure this. Co-authored-by: Liam Galvin --- pkg/formatters/junit.go | 37 +++++++++++++---- pkg/formatters/junit_test.go | 78 ++++++++++++++++++++++++++++++------ 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/pkg/formatters/junit.go b/pkg/formatters/junit.go index 62b06e29a..34aff2cde 100644 --- a/pkg/formatters/junit.go +++ b/pkg/formatters/junit.go @@ -18,6 +18,7 @@ type jUnitTestSuite struct { XMLName xml.Name `xml:"testsuite"` Name string `xml:"name,attr"` Failures string `xml:"failures,attr"` + Skipped string `xml:"skipped,attr,omitempty"` Tests string `xml:"tests,attr"` TestCases []jUnitTestCase `xml:"testcase"` } @@ -29,6 +30,7 @@ type jUnitTestCase struct { Name string `xml:"name,attr"` Time string `xml:"time,attr"` Failure *jUnitFailure `xml:"failure,omitempty"` + Skipped *jUnitSkipped `xml:"skipped,omitempty"` } // jUnitFailure contains data related to a failed test. @@ -38,14 +40,23 @@ type jUnitFailure struct { Contents string `xml:",chardata"` } -func outputJUnit(b ConfigurableFormatter, results scan.Results) error { +// jUnitSkipped defines a not executed test. +type jUnitSkipped struct { + Message string `xml:"message,attr,omitempty"` +} +func outputJUnit(b ConfigurableFormatter, results scan.Results) error { output := jUnitTestSuite{ Name: filepath.Base(os.Args[0]), - Failures: fmt.Sprintf("%d", len(results)-countPassedResults(results)), + Failures: fmt.Sprintf("%d", countWithStatus(results, scan.StatusFailed)), Tests: fmt.Sprintf("%d", len(results)), } + skipped := countWithStatus(results, scan.StatusIgnored) + if skipped > 0 { + output.Skipped = fmt.Sprintf("%d", skipped) + } + for _, res := range results { switch res.Status() { case scan.StatusIgnored: @@ -64,6 +75,7 @@ func outputJUnit(b ConfigurableFormatter, results scan.Results) error { Name: fmt.Sprintf("[%s][%s] - %s", res.Rule().LongID(), res.Severity(), res.Description()), Time: "0", Failure: buildFailure(b, res), + Skipped: buildSkipped(res), }, ) } @@ -94,7 +106,7 @@ func highlightCodeJunit(res scan.Result) string { } func buildFailure(b ConfigurableFormatter, res scan.Result) *jUnitFailure { - if res.Status() == scan.StatusPassed { + if res.Status() != scan.StatusFailed { return nil } @@ -120,14 +132,23 @@ func buildFailure(b ConfigurableFormatter, res scan.Result) *jUnitFailure { } } -func countPassedResults(results []scan.Result) int { - passed := 0 +func buildSkipped(res scan.Result) *jUnitSkipped { + if res.Status() != scan.StatusIgnored { + return nil + } + return &jUnitSkipped{ + Message: res.Description(), + } +} + +func countWithStatus(results []scan.Result, status scan.Status) int { + count := 0 for _, res := range results { - if res.Status() == scan.StatusPassed { - passed++ + if res.Status() == status { + count++ } } - return passed + return count } diff --git a/pkg/formatters/junit_test.go b/pkg/formatters/junit_test.go index 96b5b89b7..cc18b599b 100644 --- a/pkg/formatters/junit_test.go +++ b/pkg/formatters/junit_test.go @@ -20,6 +20,23 @@ import ( "github.com/stretchr/testify/require" ) +var ( + jUnitScanRule = scan.Rule{ + AVDID: "AVD-AA-9999", + ShortCode: "enable-at-rest-encryption", + Summary: "summary", + Explanation: "explanation", + Impact: "impact", + Resolution: "resolution", + Provider: providers.AWSProvider, + Service: "dynamodb", + Links: []string{ + "https://google.com", + }, + Severity: severity.High, + } +) + func Test_JUnit(t *testing.T) { want := fmt.Sprintf(` @@ -35,20 +52,55 @@ func Test_JUnit(t *testing.T) { Metadata: defsecTypes.NewTestMetadata(), Enabled: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), }) - results.SetRule(scan.Rule{ - AVDID: "AVD-AA-9999", - ShortCode: "enable-at-rest-encryption", - Summary: "summary", - Explanation: "explanation", - Impact: "impact", - Resolution: "resolution", - Provider: providers.AWSProvider, - Service: "dynamodb", - Links: []string{ - "https://google.com", + results.SetRule(jUnitScanRule) + require.NoError(t, formatter.Output(results)) + assert.Equal(t, want, buffer.String()) +} + +func Test_JUnit_skipped(t *testing.T) { + want := fmt.Sprintf(` + + + + +`, filepath.Base(os.Args[0])) + buffer := bytes.NewBuffer([]byte{}) + formatter := New().AsJUnit(). + WithWriter(buffer). + WithIncludeIgnored(true). + Build() + var results scan.Results + results.AddIgnored( + dynamodb.ServerSideEncryption{ + Metadata: defsecTypes.NewTestMetadata(), + Enabled: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), }, - Severity: severity.High, - }) + "Cluster encryption is not enabled.", + ) + results.SetRule(jUnitScanRule) + require.NoError(t, formatter.Output(results)) + assert.Equal(t, want, buffer.String()) +} + +func Test_JUnit_passed(t *testing.T) { + want := fmt.Sprintf(` + + +`, filepath.Base(os.Args[0])) + buffer := bytes.NewBuffer([]byte{}) + formatter := New().AsJUnit(). + WithWriter(buffer). + WithIncludePassed(true). + Build() + var results scan.Results + results.AddPassed( + dynamodb.ServerSideEncryption{ + Metadata: defsecTypes.NewTestMetadata(), + Enabled: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()), + }, + "Cluster encryption is not enabled.", + ) + results.SetRule(jUnitScanRule) require.NoError(t, formatter.Output(results)) assert.Equal(t, want, buffer.String()) } From bacbc997213caf158a451b23b991b0399b289318 Mon Sep 17 00:00:00 2001 From: Liam Galvin Date: Fri, 19 Aug 2022 09:44:51 +0100 Subject: [PATCH 5/5] fix(terraform): Fix remote module loading with ref (#886) --- pkg/scanners/terraform/parser/resolvers/remote.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/scanners/terraform/parser/resolvers/remote.go b/pkg/scanners/terraform/parser/resolvers/remote.go index a28400818..4c1a96437 100644 --- a/pkg/scanners/terraform/parser/resolvers/remote.go +++ b/pkg/scanners/terraform/parser/resolvers/remote.go @@ -6,7 +6,6 @@ import ( "io/fs" "os" "path/filepath" - "strings" "sync/atomic" "github.com/hashicorp/go-getter" @@ -39,12 +38,6 @@ func (r *remoteResolver) Resolve(ctx context.Context, _ fs.FS, opt Options) (fil return nil, "", "", false, nil } - if opt.RelativePath == "" && strings.LastIndex(opt.Source, "//") > strings.Index(opt.Source, "/") { - parts := strings.Split(opt.Source, "//") - opt.RelativePath = parts[len(parts)-1] - opt.Source = strings.TrimSuffix(opt.Source, "//"+opt.RelativePath) - } - key := cacheKey(opt.OriginalSource, opt.OriginalVersion, opt.RelativePath) opt.Debug("Storing with cache key %s", key)