Repository: tfsec/tfsec
Pull request #841 (2021-07-09 09:25:02)

Add more checks for digital ocean spaces
- check force_destroy isn't activated on bucket
- check that versioning is enabled
- update the template for new checks
- correct default severity



Patch:
diff --git a/cmd/tfsec-skeleton/templates.go b/cmd/tfsec-skeleton/templates.go
index 9a13c7fe0..12258b2d2 100644
--- a/cmd/tfsec-skeleton/templates.go
+++ b/cmd/tfsec-skeleton/templates.go
@@ -44,12 +44,11 @@ func init() {
 		Provider:       provider.{{.ProviderLongName}}Provider,
 		RequiredTypes:  []string{{.RequiredTypes}},
 		RequiredLabels: []string{{.RequiredLabels}},
-		DefaultSeverity: severity.Warning, //TODO set the default severity
+		DefaultSeverity: severity.Warning, 
 		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context){
 				
 			// function contents here
 
-			return
 		},
 	})
 }
diff --git a/internal/app/tfsec/rules/dig004.go b/internal/app/tfsec/rules/dig004.go
index c19eaffb3..0712a693e 100644
--- a/internal/app/tfsec/rules/dig004.go
+++ b/internal/app/tfsec/rules/dig004.go
@@ -72,7 +72,7 @@ func init() {
 		Provider:        provider.DigitalOceanProvider,
 		RequiredTypes:   []string{"resource"},
 		RequiredLabels:  []string{"digitalocean_loadbalancer"},
-		DefaultSeverity: severity.Warning,
+		DefaultSeverity: severity.Error,
 		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
 
 			if resourceBlock.MissingChild("forwarding_rule") {
diff --git a/internal/app/tfsec/rules/dig005.go b/internal/app/tfsec/rules/dig005.go
index d41c3a27c..16d519915 100644
--- a/internal/app/tfsec/rules/dig005.go
+++ b/internal/app/tfsec/rules/dig005.go
@@ -78,7 +78,7 @@ func init() {
 				if aclAttr.Equals("public-read", block.IgnoreCase) {
 					set.Add(result.New(resourceBlock).
 						WithDescription(fmt.Sprintf("Resource '%s' has a publicly readable acl.", resourceBlock.FullName())).
-						WithSeverity(severity.Error).
+						WithSeverity(severity.Warning).
 						WithAttributeAnnotation(aclAttr).
 						WithRange(aclAttr.Range()))
 				}
diff --git a/internal/app/tfsec/rules/dig006.go b/internal/app/tfsec/rules/dig006.go
new file mode 100644
index 000000000..c543e238b
--- /dev/null
+++ b/internal/app/tfsec/rules/dig006.go
@@ -0,0 +1,92 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/block"
+	"github.com/tfsec/tfsec/internal/app/tfsec/hclcontext"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/tfsec/tfsec/pkg/provider"
+	"github.com/tfsec/tfsec/pkg/result"
+	"github.com/tfsec/tfsec/pkg/rule"
+	"github.com/tfsec/tfsec/pkg/severity"
+)
+
+const DIGSpacesBucketVersioningEnabled = "DIG006"
+const DIGSpacesBucketVersioningEnabledDescription = "Spaces buckets should have versioning enabled"
+const DIGSpacesBucketVersioningEnabledImpact = "Deleted or modified data would not be recoverable"
+const DIGSpacesBucketVersioningEnabledResolution = "Enable versioning to protect against accidental or malicious removal or modification"
+const DIGSpacesBucketVersioningEnabledExplanation = `
+Versioning is a means of keeping multiple variants of an object in the same bucket. You can use the Spaces (S3) Versioning feature to preserve, retrieve, and restore every version of every object stored in your buckets. With versioning you can recover more easily from both unintended user actions and application failures.
+`
+const DIGSpacesBucketVersioningEnabledBadExample = `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "foobar"
+  region = "nyc3"
+}
+
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "foobar"
+  region = "nyc3"
+
+  versioning {
+	enabled = false	
+  }
+}
+`
+const DIGSpacesBucketVersioningEnabledGoodExample = `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "foobar"
+  region = "nyc3"
+
+  versioning {
+	enabled = true
+  }
+}
+`
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		ID: DIGSpacesBucketVersioningEnabled,
+		Documentation: rule.RuleDocumentation{
+			Summary:     DIGSpacesBucketVersioningEnabledDescription,
+			Explanation: DIGSpacesBucketVersioningEnabledExplanation,
+			Impact:      DIGSpacesBucketVersioningEnabledImpact,
+			Resolution:  DIGSpacesBucketVersioningEnabledResolution,
+			BadExample:  DIGSpacesBucketVersioningEnabledBadExample,
+			GoodExample: DIGSpacesBucketVersioningEnabledGoodExample,
+			Links: []string{
+				"https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs/resources/spaces_bucket#versioning",
+				"https://docs.aws.amazon.com/AmazonS3/latest/userguide/Versioning.html",
+			},
+		},
+		Provider:        provider.DigitalOceanProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"digitalocean_spaces_bucket"},
+		DefaultSeverity: severity.Warning, //TODO set the default severity
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			if resourceBlock.MissingChild("versioning") {
+				set.Add(result.New(resourceBlock).
+					WithDescription(fmt.Sprintf("Resource '%s' does not have versioning enabled.", resourceBlock.FullName())).
+					WithSeverity(severity.Warning).
+					WithRange(resourceBlock.Range()))
+
+				return
+			}
+
+			versioningBlock := resourceBlock.GetBlock("versioning")
+			enabledAttr := versioningBlock.GetAttribute("enabled")
+
+			if enabledAttr == nil || enabledAttr.IsFalse() {
+				set.Add(result.New(resourceBlock).
+					WithDescription(fmt.Sprintf("Resource '%s' has versioning specified, but it isn't enabled", resourceBlock.FullName())).
+					WithSeverity(severity.Warning).
+					WithAttributeAnnotation(enabledAttr).
+					WithRange(enabledAttr.Range()))
+
+			}
+
+		},
+	})
+}
diff --git a/internal/app/tfsec/rules/dig007.go b/internal/app/tfsec/rules/dig007.go
new file mode 100644
index 000000000..2f0994e6f
--- /dev/null
+++ b/internal/app/tfsec/rules/dig007.go
@@ -0,0 +1,68 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/block"
+	"github.com/tfsec/tfsec/internal/app/tfsec/hclcontext"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/tfsec/tfsec/pkg/provider"
+	"github.com/tfsec/tfsec/pkg/result"
+	"github.com/tfsec/tfsec/pkg/rule"
+	"github.com/tfsec/tfsec/pkg/severity"
+)
+
+const DIGForceDestroyEnabled = "DIG007"
+const DIGForceDestroyEnabledDescription = "Force destroy is enabled on Spaces bucket which is dangerous"
+const DIGForceDestroyEnabledImpact = "Accidental deletion of bucket objects"
+const DIGForceDestroyEnabledResolution = "Don't use force destroy on bucket configuration"
+const DIGForceDestroyEnabledExplanation = `
+Enabling force destroy on a Spaces bucket means that the bucket can be deleted without the additional check that it is empty. This risks important data being accidently deleted by a bucket removal process.
+`
+const DIGForceDestroyEnabledBadExample = `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   		= "foobar"
+  region 		= "nyc3"
+  force_destroy = true
+}
+`
+const DIGForceDestroyEnabledGoodExample = `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "foobar"
+  region = "nyc3"
+}
+`
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		ID: DIGForceDestroyEnabled,
+		Documentation: rule.RuleDocumentation{
+			Summary:     DIGForceDestroyEnabledDescription,
+			Explanation: DIGForceDestroyEnabledExplanation,
+			Impact:      DIGForceDestroyEnabledImpact,
+			Resolution:  DIGForceDestroyEnabledResolution,
+			BadExample:  DIGForceDestroyEnabledBadExample,
+			GoodExample: DIGForceDestroyEnabledGoodExample,
+			Links: []string{
+				"https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs/resources/spaces_bucket#force_destroy",
+				"",
+			},
+		},
+		Provider:        provider.DigitalOceanProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"digitalocean_spaces_bucket"},
+		DefaultSeverity: severity.Warning,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+			if resourceBlock.HasChild("force_destroy") {
+				forceDestroyAttr := resourceBlock.GetAttribute("force_destroy")
+				if forceDestroyAttr.IsTrue() {
+					set.Add(result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' has versioning specified, but it isn't enabled", resourceBlock.FullName())).
+						WithSeverity(severity.Warning).
+						WithAttributeAnnotation(forceDestroyAttr).
+						WithRange(forceDestroyAttr.Range()))
+				}
+			}
+		},
+	})
+}


Test patch:
diff --git a/internal/app/tfsec/test/dig006_test.go b/internal/app/tfsec/test/dig006_test.go
new file mode 100644
index 000000000..5e25041d2
--- /dev/null
+++ b/internal/app/tfsec/test/dig006_test.go
@@ -0,0 +1,64 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/rules"
+)
+
+func Test_DIGSpacesBucketVersioningEnabled(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "Spaces bucket with versioning actively disabled fails check",
+			source: `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "foobar"
+  region = "nyc3"
+
+  versioning {
+	enabled = false	
+  }
+}
+`,
+			mustIncludeResultCode: rules.DIGSpacesBucketVersioningEnabled,
+		},
+		{
+			name: "Spaces bucket with versioning left to default fails check",
+			source: `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "foobar"
+  region = "nyc3"
+}
+`,
+			mustIncludeResultCode: rules.DIGSpacesBucketVersioningEnabled,
+		},
+		{
+			name: "Spaces bucket with versioning enabled passes check",
+			source: `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "foobar"
+  region = "nyc3"
+
+  versioning {
+	enabled = true
+  }
+}
+`,
+			mustExcludeResultCode: rules.DIGSpacesBucketVersioningEnabled,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanHCL(test.source, t)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}
diff --git a/internal/app/tfsec/test/dig007_test.go b/internal/app/tfsec/test/dig007_test.go
new file mode 100644
index 000000000..6a1be0476
--- /dev/null
+++ b/internal/app/tfsec/test/dig007_test.go
@@ -0,0 +1,58 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/rules"
+)
+
+func Test_DIGForceDestroyEnabled(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "force destroy turned on fails check",
+			source: `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   		= "foobar"
+  region 		= "nyc3"
+  force_destroy = true
+}
+`,
+			mustIncludeResultCode: rules.DIGForceDestroyEnabled,
+		},
+		{
+			name: "force destroy left to default passes check",
+			source: `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "foobar"
+  region = "nyc3"
+}
+`,
+			mustExcludeResultCode: rules.DIGForceDestroyEnabled,
+		},
+		{
+			name: "force destroy explicitly turned off passes check",
+			source: `
+resource "digitalocean_spaces_bucket" "good_example" {
+	name   	 	  = "foobar"
+	region 		  = "nyc3"
+	force_destroy = false
+}
+`,
+			mustExcludeResultCode: rules.DIGForceDestroyEnabled,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanHCL(test.source, t)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}