Repository: tfsec/tfsec
Pull request #630 (2021-02-25 06:56:11)

Add support for negating predicateMatchSpec
### Problem
Currently, `matchSpec` actions are defined in such a way that you must create new `matchSpec` actions everytime you wish to negate a particular action. 

### Proposed Solution 
By introducing a `not` action, we can now support negating all existing actions (which may result in duplicates), but this makes adding new actions in the future much easier and also will simplify some of the custom check processing code. 

### Follow Up
For backwards compatibility, we have not removed existing actions that may be duplicated from the introduction of being able to use `not`, however if you would like, we can introduce a follow-up PR to add deprecation notices for actions that overlap with the introduced `not` functionality. 

(cc: @alloveras )


Patch:
diff --git a/docs-website/docs/config/custom_checks.md b/docs-website/docs/config/custom_checks.md
index 1c1d07edd..3155d5c92 100644
--- a/docs-website/docs/config/custom_checks.md
+++ b/docs-website/docs/config/custom_checks.md
@@ -540,6 +540,28 @@ matchSpec:
         - name: kernel_id
           action: isPresent
 ```
+##### not
+The `not` check action passes when the `predicateMatchSpec` evaluates to `false`.
+
+As an example, if you want to represent that a `resource` should not be included `inModule` you might use the following `matchSpec`:
+
+```
+"matchSpec": {
+  "action": "not",
+  "predicateMatchSpec": [
+    {
+        "action": "inModule" 
+    }
+  ]
+}
+```
+
+```yaml
+matchSpec:
+  action: not
+  predicateMatchSpec:
+    - action: inModule
+```
 
 ## How do I know my JSON is valid?
 We have provided the `tfsec-checkgen` binary which will validate your check file to ensure that it is valid for use with `tfsec`. 
diff --git a/internal/app/tfsec/custom/custom_check.go b/internal/app/tfsec/custom/custom_check.go
index 4cffff71f..8a1a82929 100644
--- a/internal/app/tfsec/custom/custom_check.go
+++ b/internal/app/tfsec/custom/custom_check.go
@@ -27,6 +27,7 @@ var ValidCheckActions = []CheckAction{
 	IsNone,
 	And,
 	Or,
+	Not,
 }
 
 // InModule checks that the block is part of a module
@@ -86,6 +87,9 @@ const And CheckAction = "and"
 // Or checks that at least one of the given predicateMatchSpec's evaluates to True
 const Or CheckAction = "or"
 
+// Not checks that the given predicateMatchSpec evaluates to False
+const Not CheckAction = "not"
+
 // MatchSpec specifies the checks that should be performed
 type MatchSpec struct {
 	Name               string      `json:"name,omitempty" yaml:"name,omitempty"`
diff --git a/internal/app/tfsec/custom/processing.go b/internal/app/tfsec/custom/processing.go
index 763e320cf..d12e1b1a4 100644
--- a/internal/app/tfsec/custom/processing.go
+++ b/internal/app/tfsec/custom/processing.go
@@ -153,6 +153,10 @@ func evalMatchSpec(block *parser.Block, spec *MatchSpec, ctx *scanner.Context) b
 		return resourceFound(spec, ctx)
 	}
 
+	if spec.Action == Not {
+		return !evalMatchSpec(block, &spec.PredicateMatchSpec[0], ctx)
+	}
+
 	// This And MatchSpec is only true if all childSpecs return true
 	if spec.Action == And {
 		for _, childSpec := range spec.PredicateMatchSpec {
diff --git a/internal/app/tfsec/custom/validate.go b/internal/app/tfsec/custom/validate.go
index 05f148992..fd21ffa3e 100644
--- a/internal/app/tfsec/custom/validate.go
+++ b/internal/app/tfsec/custom/validate.go
@@ -74,8 +74,8 @@ func validateMatchSpec(spec *MatchSpec, check *Check, checkErrors []error) []err
 	if !spec.Action.isValid() {
 		checkErrors = append(checkErrors, errors.New(fmt.Sprintf("matchSpec.Action[%s] is not a recognised option. Should be %s", spec.Action, ValidCheckActions)))
 	}
-	// if the check is one of `inModule`,`or`,`and`, no name is required
-	if len(spec.Name) == 0 && spec.Action != "inModule" && spec.Action != "or" && spec.Action != "and" {
+	// if the check is one of `inModule`,`or`,`and`, `not`, no name is required
+	if len(spec.Name) == 0 && spec.Action != "inModule" && spec.Action != "or" && spec.Action != "and"  && spec.Action != "not"{
 		checkErrors = append(checkErrors, errors.New("matchSpec.Name requires a value"))
 	}
 
@@ -86,6 +86,15 @@ func validateMatchSpec(spec *MatchSpec, check *Check, checkErrors []error) []err
 		}
 	}
 
+	// `not` specification can only have a single predicateMatchSpec associated, which must be valid
+	if spec.Action == "not" {
+		if len(spec.PredicateMatchSpec) == 1 {
+			checkErrors = append(validateMatchSpec(&spec.PredicateMatchSpec[0], check, checkErrors))
+		} else {
+			checkErrors = append(checkErrors, errors.New("`not` action must have a single predicate attached"))
+		}
+	}
+
 	if spec.SubMatch != nil {
 		return validateMatchSpec(spec.SubMatch, check, checkErrors)
 	}


Test patch:
diff --git a/internal/app/tfsec/custom/processing_test.go b/internal/app/tfsec/custom/processing_test.go
index d7bb6dbcd..ce68f8d4e 100644
--- a/internal/app/tfsec/custom/processing_test.go
+++ b/internal/app/tfsec/custom/processing_test.go
@@ -89,6 +89,16 @@ var testNestedMatchSpec = MatchSpec{
 		},
 	},
 }
+var testNotMatchSpec = MatchSpec{
+	Action: "not",
+	PredicateMatchSpec: []MatchSpec{
+		{
+			Name:       "virtualization_type",
+			Action:     "equals",
+			MatchValue: "paravirtual",
+		},
+	},
+}
 
 func TestRequiresPresenceWithResourcePresent(t *testing.T) {
 	scanResults := scanTerraform(t, `
@@ -268,6 +278,33 @@ resource "aws_ami" "example" {
 		})
 	}
 }
+
+func TestNotFunction(t *testing.T) {
+	var tests = []struct {
+		name               string
+		source             string
+		matchSpec          MatchSpec
+		expected           bool
+	}{
+		{
+			name: "check that not correctly inverts the outcome of a given predicateMatchSpec",
+			source: `
+resource "aws_ami" "example" {
+	virtualization_type = "paravirtual"
+}
+`,
+			matchSpec: testNotMatchSpec,
+			expected:           false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			blocks := createBlocksFromSource(test.source)[0]
+			result := evalMatchSpec(blocks, &test.matchSpec, nil)
+			assert.Equal(t, result, test.expected, "Not match functions evaluating incorrectly.")
+		})
+	}
+}
 func givenCheck(jsonContent string) {
 	var checksfile ChecksFile
 	err := json.NewDecoder(strings.NewReader(jsonContent)).Decode(&checksfile)