Fix #1754 Invalid variable validation (#1770)

Signed-off-by: Max Goncharenko <kacejot@fex.net>

pkg/engine/variables/vars.go

@@ -81,6 +81,37 @@ func substituteReferences(log logr.Logger, rule interface{}) (interface{}, error
 	return jsonUtils.NewTraversal(rule, substituteReferencesIfAny(log)).TraverseJSON()
 }
 
+// ValidateBackgroundModeVars validates variables against the specified context,
+// which contains a list of allowed JMESPath queries in background processing,
+// and throws an error if the variable is not allowed.
+func ValidateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface, rule interface{}) (interface{}, error) {
+	return jsonUtils.NewTraversal(rule, validateBackgroundModeVars(log, ctx)).TraverseJSON()
+}
+
+func validateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface) jsonUtils.Action {
+	return jsonUtils.OnlyForLeafs(func(data *jsonUtils.ActionData) (interface{}, error) {
+		value, ok := data.Element.(string)
+		if !ok {
+			return data.Element, nil
+		}
+		vars := regexVariables.FindAllString(value, -1)
+		for _, v := range vars {
+			variable := replaceBracesAndTrimSpaces(v)
+
+			_, err := ctx.Query(variable)
+			if err != nil {
+				switch err.(type) {
+				case context.InvalidVariableErr:
+					return nil, err
+				default:
+					return nil, fmt.Errorf("failed to resolve %v at path %s", variable, data.Path)
+				}
+			}
+		}
+		return nil, nil
+	})
+}
+
 // NotFoundVariableErr is returned when it is impossible to resolve the variable
 type NotFoundVariableErr struct {
 	variable string
@@ -151,9 +182,7 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt
 		vars := regexVariables.FindAllString(value, -1)
 		for len(vars) > 0 {
 			for _, v := range vars {
-				variable := strings.ReplaceAll(v, "{{", "")
-				variable = strings.ReplaceAll(variable, "}}", "")
-				variable = strings.TrimSpace(variable)
+				variable := replaceBracesAndTrimSpaces(v)
 
 				operation, err := ctx.Query("request.operation")
 				if err == nil && operation == "DELETE" {
@@ -199,6 +228,13 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt
 	})
 }
 
+func replaceBracesAndTrimSpaces(v string) string {
+	variable := strings.ReplaceAll(v, "{{", "")
+	variable = strings.ReplaceAll(variable, "}}", "")
+	variable = strings.TrimSpace(variable)
+	return variable
+}
+
 func resolveReference(log logr.Logger, fullDocument interface{}, reference, absolutePath string) (interface{}, error) {
 	var foundValue interface{}
 

pkg/policy/background.go

@@ -36,19 +36,26 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error {
 				ctx.AddBuiltInVars(contextEntry.Name)
 			}
 		}
+		err = validateBackgroundModeVars(ctx, rule)
+		if err != nil {
+			return err
+		}
+		if rule, err = variables.SubstituteAllInRule(log.Log, ctx, rule); !checkNotFoundErr(err) {
+			return fmt.Errorf("variable substitution failed for rule %s: %s", rule.Name, err.Error())
+		}
 
 		if rule, err = variables.SubstituteAllInRule(log.Log, ctx, rule); !checkNotFoundErr(err) {
 			return fmt.Errorf("variable substitution failed for rule %s: %s", rule.Name, err.Error())
 		}
 
 		if rule.AnyAllConditions != nil {
-			if err = validatePreConditions(idx, ctx, rule.AnyAllConditions); err != nil {
+			if err = validatePreConditions(idx, ctx, rule.AnyAllConditions); !checkNotFoundErr(err) {
 				return err
 			}
 		}
 
 		if rule.Validation.Deny != nil {
-			if err = validateDenyConditions(idx, ctx, rule.Validation.Deny.AnyAllConditions); err != nil {
+			if err = validateDenyConditions(idx, ctx, rule.Validation.Deny.AnyAllConditions); !checkNotFoundErr(err) {
 				return err
 			}
 		}
@@ -118,6 +125,21 @@ func userInfoDefined(ui kyverno.UserInfo) string {
 	return ""
 }
 
+func validateBackgroundModeVars(ctx context.EvalInterface, document apiextensions.JSON) error {
+	jsonByte, err := json.Marshal(document)
+	if err != nil {
+		return err
+	}
+
+	var jsonInterface interface{}
+	err = json.Unmarshal(jsonByte, &jsonInterface)
+	if err != nil {
+		return err
+	}
+	_, err = variables.ValidateBackgroundModeVars(log.Log, ctx, jsonInterface)
+	return err
+}
+
 func substituteVarsInJSON(ctx context.EvalInterface, document apiextensions.JSON) (apiextensions.JSON, error) {
 	jsonByte, err := json.Marshal(document)
 	if err != nil {

pkg/policy/background_test.go

@@ -0,0 +1,137 @@
+package policy
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
+	"gotest.tools/assert"
+)
+
+func Test_Validation_valid_backgroundPolicy(t *testing.T) {
+	rawPolicy := []byte(`
+		{
+			"apiVersion": "kyverno.io/v1",
+			"kind": "ClusterPolicy",
+			"metadata": {
+			  "name": "test-gen",
+			  "annotations": {
+				"policies.kyverno.io/category": "Best Practices"
+			  }
+			},
+			"spec": {
+			  "rules": [
+				{
+				  "match": {
+					"resources": {
+					  "kinds": [
+						"Namespace"
+					  ]
+					}
+				  },
+				  "name": "test-gen",
+				  "preconditions": {
+					"all": [
+					  {
+						"key": "{{request.object.metadata.name}}",
+						"operator": "NotEquals",
+						"value": ""
+					  }
+					]
+				  },
+				  "context": [
+					{
+					  "name": "mycm",
+					  "configMap": {
+						"name": "config-name",
+						"namespace": "default"
+					  }
+					}
+				  ],
+				  "generate": {
+					"kind": "ConfigMap",
+					"name": "{{request.object.metadata.name}}-config-name",
+					"namespace": "{{request.object.metadata.name}}",
+					"data": {
+					  "data": {
+						"new": "{{ mycm.data.foo }}"
+					  }
+					}
+				  }
+				}
+			  ]
+			}
+		  }
+		`)
+
+	var policy kyverno.ClusterPolicy
+	err := json.Unmarshal(rawPolicy, &policy)
+	assert.NilError(t, err)
+
+	err = ContainsVariablesOtherThanObject(policy)
+	assert.NilError(t, err)
+}
+
+func Test_Validation_invalid_backgroundPolicy(t *testing.T) {
+	rawPolicy := []byte(`
+		{
+			"apiVersion": "kyverno.io/v1",
+			"kind": "ClusterPolicy",
+			"metadata": {
+			  "name": "test-gen",
+			  "annotations": {
+				"policies.kyverno.io/category": "Best Practices"
+			  }
+			},
+			"spec": {
+			  "rules": [
+				{
+				  "match": {
+					"resources": {
+					  "kinds": [
+						"Namespace"
+					  ]
+					}
+				  },
+				  "name": "test-gen",
+				  "preconditions": {
+					"all": [
+					  {
+						"key": "{{request.object.metadata.name}}",
+						"operator": "NotEquals",
+						"value": ""
+					  }
+					]
+				  },
+				  "context": [
+					{
+					  "name": "mycm",
+					  "configMap": {
+						"name": "config-name",
+						"namespace": "default"
+					  }
+					}
+				  ],
+				  "generate": {
+					"kind": "ConfigMap",
+					"name": "{{serviceAccountName}}-config-name",
+					"namespace": "{{serviceAccountName}}",
+					"data": {
+					  "data": {
+						"new": "{{ mycm.data.foo }}"
+					  }
+					}
+				  }
+				}
+			  ]
+			}
+		  }
+		`)
+
+	var policy kyverno.ClusterPolicy
+	err := json.Unmarshal(rawPolicy, &policy)
+	assert.NilError(t, err)
+	err = ContainsVariablesOtherThanObject(policy)
+	assert.Assert(t, strings.Contains(err.Error(), "variable serviceAccountName cannot be used, allowed variables: [request.object request.namespace mycm]"))
+}