Added "id" field support & policy validation tests

pkg/filters/filter-specs.go

@@ -19,6 +19,7 @@ package filters
 import (
 	"github.com/accurics/terrascan/pkg/policy"
 	"github.com/accurics/terrascan/pkg/utils"
+	"go.uber.org/zap"
 )
 
 // PolicyTypesFilterSpecification is policy type based Filter Spec
@@ -57,7 +58,14 @@ type RerefenceIDFilterSpecification struct {
 
 // IsSatisfied implementation for reference ID based Filter spec
 func (rs RerefenceIDFilterSpecification) IsSatisfied(r *policy.RegoMetadata) bool {
-	return rs.ReferenceID == r.ReferenceID
+	if rs.ReferenceID == r.ID {
+		return true
+	}
+	if rs.ReferenceID == r.ReferenceID {
+		zap.S().Warnf("Deprecation warning : Use 'id' (%s) instead of 'reference_id' (%s) to skip/scan rules", r.ID, r.ReferenceID)
+		return true
+	}
+	return false
 }
 
 // RerefenceIDsFilterSpecification is reference IDs based Filter Spec

pkg/policies/opa/rego/policy-test-util.go

@@ -0,0 +1,38 @@
+/*
+    Copyright (C) 2020 Accurics, Inc.
+
+	Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+		http://www.apache.org/licenses/LICENSE-2.0
+
+	Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+package rego
+
+import (
+	"encoding/json"
+	"io/ioutil"
+
+	"github.com/accurics/terrascan/pkg/policy"
+)
+
+// LoadRegoMetadata reads rego meta data file
+func LoadRegoMetadata(file string) (*policy.RegoMetadata, error) {
+	metadata, err := ioutil.ReadFile(file)
+	if err != nil {
+		return nil, err
+	}
+	// Read metadata into struct
+	regoMetadata := policy.RegoMetadata{}
+	if err = json.Unmarshal(metadata, &regoMetadata); err != nil {
+		return nil, err
+	}
+	return &regoMetadata, nil
+}

pkg/policies/opa/rego/policy_test.go

@@ -0,0 +1,125 @@
+/*
+    Copyright (C) 2020 Accurics, Inc.
+
+	Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+		http://www.apache.org/licenses/LICENSE-2.0
+
+	Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+package rego
+
+import (
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"regexp"
+	"testing"
+
+	"github.com/accurics/terrascan/pkg/policy"
+	"github.com/accurics/terrascan/pkg/utils"
+)
+
+var metaDataReferenceIDPattern = regexp.MustCompile(fmt.Sprintf("(%s|%s)", utils.MetaDataReferenceIDRegex, utils.MetaDataIDRegex))
+var metaDataIDPattern = regexp.MustCompile(utils.MetaDataIDRegex)
+
+func TestPolicyValidation(t *testing.T) {
+
+	// The root directory for reading policies
+	path := "."
+
+	dirList, err := utils.FindAllDirectories(path)
+	if err != nil {
+		t.Errorf("Error while walking the policy directories : %s", err)
+		return
+	}
+
+	for _, dir := range dirList {
+		t.Run("Policy Validation", func(t *testing.T) {
+			Validate(dir, t)
+		})
+	}
+
+}
+
+func Validate(dir string, t *testing.T) {
+
+	var fileInfo []os.FileInfo
+	fileInfo, err := ioutil.ReadDir(dir)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	metadataFiles := utils.FilterFileInfoBySuffix(&fileInfo, []string{".json"})
+
+	for j := range metadataFiles {
+		filePath := filepath.Join(dir, *metadataFiles[j])
+		var regoMetadata *policy.RegoMetadata
+		regoMetadata, err = LoadRegoMetadata(filePath)
+		if err != nil {
+			t.Errorf("Error while reading %s : %s", filePath, err)
+			continue
+		}
+
+		validateRequiredFields(regoMetadata, filePath, t)
+
+		if !metaDataReferenceIDPattern.MatchString(regoMetadata.ReferenceID) {
+			t.Errorf("%s invalid reference_id pattern", filePath)
+		}
+
+		if !metaDataIDPattern.MatchString(regoMetadata.ID) {
+			t.Errorf("%s invalid id pattern", filePath)
+		}
+
+		if _, err := os.Stat(filepath.Join(dir, regoMetadata.File)); errors.Is(err, os.ErrNotExist) {
+			t.Errorf("%s doesn't exist", filepath.Join(dir, regoMetadata.File))
+		}
+
+	}
+
+}
+
+func validateRequiredFields(regoMetadata *policy.RegoMetadata, filepath string, t *testing.T) {
+
+	if regoMetadata.Name == "" {
+		validationErrorLogger("name", filepath, t)
+	}
+	if regoMetadata.File == "" {
+		validationErrorLogger("file", filepath, t)
+	}
+	if regoMetadata.PolicyType == "" {
+		validationErrorLogger("policy_type", filepath, t)
+	}
+	if regoMetadata.ResourceType == "" {
+		validationErrorLogger("resource_type", filepath, t)
+	}
+	if regoMetadata.Severity == "" {
+		validationErrorLogger("severity", filepath, t)
+	}
+	if regoMetadata.Description == "" {
+		validationErrorLogger("description", filepath, t)
+	}
+	if regoMetadata.Category == "" {
+		validationErrorLogger("category", filepath, t)
+	}
+	if regoMetadata.Version == 0 {
+		validationErrorLogger("version", filepath, t)
+	}
+	if regoMetadata.ID == "" {
+		validationErrorLogger("id", filepath, t)
+	}
+
+}
+
+func validationErrorLogger(field string, filepath string, t *testing.T) {
+	t.Errorf("Required Field missing in %s : \"%s\"", filepath, field)
+}

pkg/policy/opa/engine.go

@@ -294,7 +294,7 @@ func (e *Engine) reportViolation(regoData *policy.RegoData, resource *output.Res
 	violation := results.Violation{
 		RuleName:     regoData.Metadata.Name,
 		Description:  regoData.Metadata.Description,
-		RuleID:       regoData.Metadata.ReferenceID,
+		RuleID:       regoData.Metadata.ID,
 		Severity:     regoData.Metadata.Severity,
 		Category:     regoData.Metadata.Category,
 		RuleFile:     regoData.Metadata.File,
@@ -343,7 +343,7 @@ func (e *Engine) reportPassed(regoData *policy.RegoData) {
 	passedRule := results.PassedRule{
 		RuleName:    regoData.Metadata.Name,
 		Description: regoData.Metadata.Description,
-		RuleID:      regoData.Metadata.ReferenceID,
+		RuleID:      regoData.Metadata.ID,
 		Severity:    regoData.Metadata.Severity,
 		Category:    regoData.Metadata.Category,
 	}
@@ -431,9 +431,15 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput, filter policy.PreScanF
 					found := false
 					var skipComment string
 					for _, rule := range resource.SkipRules {
+						if strings.EqualFold(e.regoDataMap[k].Metadata.ID, rule.Rule) {
+							found = true
+							skipComment = rule.Comment
+							break
+						}
 						if strings.EqualFold(k, rule.Rule) {
 							found = true
 							skipComment = rule.Comment
+							zap.S().Warnf("Deprecation warning : Use 'id' (%s) instead of 'reference_id' (%s) to skip rules", e.regoDataMap[k].Metadata.ID, k)
 							break
 						}
 					}

pkg/policy/types.go

@@ -58,6 +58,7 @@ type RegoMetadata struct {
 	ReferenceID  string                 `json:"reference_id"`
 	Category     string                 `json:"category"`
 	Version      int                    `json:"version"`
+	ID           string                 `json:"id"`
 }
 
 // RegoData Stores all information needed to evaluate and report on a rego rule

pkg/utils/skip_rules.go

@@ -35,15 +35,18 @@ const (
 	TerrascanSkipComment = "comment"
 	// SkipRulesPrefix used to identify and trim the skipping rule patterns
 	SkipRulesPrefix = "#ts:skip="
-	// RuleIDRegex used to match the reference_id string
-	RuleIDRegex = `((([ A-Za-z0-9]+[.-]{1})){2,5}([\d]+)){1}`
+	// MetaDataIDRegex pattern to match Rego Metadata ID
+	MetaDataIDRegex = `(AC_)(AWS|AZURE|GCP|K8S|GITHUB)[_]([\d]{4})`
+	// MetaDataReferenceIDRegex pattern to match Rego Metadata ReferenceID
+	MetaDataReferenceIDRegex = `(([ A-Za-z0-9]+[.-]{1}){2,5})([\d]+)`
 	// SkipRuleCommentRegex used to detect comments in skipped rule
 	SkipRuleCommentRegex = `([ \t]+.*){0,1}`
 )
 
 var (
-	ruleIDPattern                  = regexp.MustCompile(RuleIDRegex)
-	skipRulesPattern               = regexp.MustCompile(fmt.Sprintf("(%s%s%s)", SkipRulesPrefix, RuleIDRegex, SkipRuleCommentRegex))
+	ruleIDRegex                    = fmt.Sprintf("(%s|%s)", MetaDataReferenceIDRegex, MetaDataIDRegex)
+	ruleIDPattern                  = regexp.MustCompile(ruleIDRegex)
+	skipRulesPattern               = regexp.MustCompile(fmt.Sprintf("(%s%s%s)", SkipRulesPrefix, ruleIDRegex, SkipRuleCommentRegex))
 	infileInstructionNotPresentLog = "%s not present for resource: %s"
 )
 

pkg/utils/skip_rules_test.go

@@ -30,6 +30,11 @@ func TestGetSkipRules(t *testing.T) {
 	testRuleAWS3 := "AWS.S3 Bucket.DS.High.1041"
 	testRuleAWS4 := "AWS.S3 Bucket DS.High.1041"
 	testRuleAWS5 := "AWS.S3 Bucket DS .High.1041"
+	testRuleAWS6 := "AC_AWS_1111"
+	testRuleAZURE1 := "AC_AZURE_1111"
+	testRuleGCP1 := "AC_GCP_1111"
+	testRuleK8S1 := "AC_K8S_1111"
+	testRuleGITHUB1 := "AC_GITHUB_1111"
 	testRuleAWSwithHyphen := "AC-AWS-NS-IN-M-1172"
 	testRuleAzure := "accurics.azure.NS.147"
 	testRuleKubernetesWithHyphen := "AC-K8-DS-PO-M-0143"
@@ -181,6 +186,109 @@ func TestGetSkipRules(t *testing.T) {
 				},
 			},
 		},
+		{
+			// skipping rule by ID field
+			name:  "skipping rule using ID field",
+			input: "#ts:skip=AC_AWS_1111",
+			expected: []output.SkipRule{
+				{Rule: testRuleAWS6},
+			},
+		},
+		{
+			// skipping rule by ID field and comment
+			name:  "skipping rule using ID field and comment",
+			input: "#ts:skip=AC_AWS_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{
+					Rule:    testRuleAWS6,
+					Comment: "skip rule by ID",
+				},
+			},
+		},
+		{
+			// skipping AZURE rule by ID field
+			name:  "skipping AZURE rule using ID field",
+			input: "#ts:skip=AC_AZURE_1111",
+			expected: []output.SkipRule{
+				{Rule: testRuleAZURE1},
+			},
+		},
+		{
+			// skipping AZURE rule by ID field and comment
+			name:  "skipping AZURE rule using ID field and comment",
+			input: "#ts:skip=AC_AZURE_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{
+					Rule:    testRuleAZURE1,
+					Comment: "skip rule by ID",
+				},
+			},
+		},
+		{
+			// skipping GCP rule by ID field
+			name:  "skipping GCP rule using ID field",
+			input: "#ts:skip=AC_GCP_1111",
+			expected: []output.SkipRule{
+				{Rule: testRuleGCP1},
+			},
+		},
+		{
+			// skipping GCP rule by ID field and comment
+			name:  "skipping GCP rule using ID field and comment",
+			input: "#ts:skip=AC_GCP_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{
+					Rule:    testRuleGCP1,
+					Comment: "skip rule by ID",
+				},
+			},
+		},
+		{
+			// skipping K8S rule by ID field
+			name:  "skipping K8S rule using ID field ",
+			input: "#ts:skip=AC_K8S_1111",
+			expected: []output.SkipRule{
+				{Rule: testRuleK8S1},
+			},
+		},
+		{
+			// skipping K8S rule by ID field and comment
+			name:  "skipping K8S rule using ID field and comment",
+			input: "#ts:skip=AC_K8S_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{
+					Rule:    testRuleK8S1,
+					Comment: "skip rule by ID",
+				},
+			},
+		},
+		{
+			// skipping GITHUB rule by ID field
+			name:  "skipping GITHUB rule using ID field ",
+			input: "#ts:skip=AC_GITHUB_1111",
+			expected: []output.SkipRule{
+				{Rule: testRuleGITHUB1},
+			},
+		},
+		{
+			// skipping K8S rule by ID field and comment
+			name:  "skipping GITHUB rule using ID field and comment",
+			input: "#ts:skip=AC_GITHUB_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{
+					Rule:    testRuleGITHUB1,
+					Comment: "skip rule by ID",
+				},
+			},
+		},
+		{
+			// skipping rule by ID field and comment
+			name:  "skipping rule using ID field and comment repeated name",
+			input: "#ts:skip=AC_AWS_1111AC_AWS_1111 skip rule by ID",
+			expected: []output.SkipRule{
+				{Rule: testRuleAWS6},
+			},
+		},
 	}
 
 	for _, tt := range table {

test/e2e/scan/golden/k8s_scans/k8s/kubernetes_ingress_violations/kubernetes_ingress_human_verbose.txt

@@ -5,7 +5,7 @@ Violation Details -
 	Line           :	1
 	Severity       :	HIGH
 	Rule Name      :	noHttps
-	Rule ID        :	AC-K8-NS-IN-H-0020
+	Rule ID        :	AC_K8S_0001
 	Resource Name  :	ingress-demo-disallowed
 	Resource Type  :	kubernetes_ingress
 	Category       :	Network Security

test/e2e/scan/golden/k8s_scans/k8s/kubernetes_ingress_violations/kubernetes_ingress_json.txt

@@ -4,7 +4,7 @@
       {
         "rule_name": "noHttps",
         "description": "TLS disabled can affect the confidentiality of the data in transit",
-        "rule_id": "AC-K8-NS-IN-H-0020",
+        "rule_id": "AC_K8S_0001",
         "severity": "HIGH",
         "category": "Network Security",
         "resource_name": "ingress-demo-disallowed",