validate patchJSON6902 (#4469)

* validate patchJSON6902

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>

* validate patchJSON6902

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>

* test: validateJSON6902 tests

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>

* validate patchJSON6902

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>

* test: validate patchJSON6902

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>

Signed-off-by: Shivansh-yadav13 <yadavshivansh@gmail.com>
Signed-off-by: Shivansh Yadav <yadavshivansh@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>

pkg/policy/validate.go

@@ -79,6 +79,36 @@ func validateJSONPatchPathForForwardSlash(patch string) error {
 	return nil
 }
 
+func validateJSONPatch(patch string, ruleIdx int) error {
+	patch = variables.ReplaceAllVars(patch, func(s string) string { return "kyvernojsonpatchvariable" })
+
+	jsonPatch, err := yaml.ToJSON([]byte(patch))
+	if err != nil {
+		return err
+	}
+
+	decodedPatch, err := jsonpatch.DecodePatch(jsonPatch)
+	if err != nil {
+		return err
+	}
+
+	for _, operation := range decodedPatch {
+		op := operation.Kind()
+		if op != "add" && op != "remove" && op != "replace" {
+			return fmt.Errorf("Unexpected kind: spec.rules[%d]: %s", ruleIdx, op)
+		}
+		v, _ := operation.ValueInterface()
+		if v != nil {
+			vs := fmt.Sprintf("%v", v)
+			if strings.ContainsAny(vs, `"`) || strings.ContainsAny(vs, `'`) {
+				return fmt.Errorf("missing quote around value: spec.rules[%d]: %s", ruleIdx, vs)
+			}
+		}
+	}
+
+	return nil
+}
+
 // Validate checks the policy and rules declarations for required configurations
 func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager) (*admissionv1.AdmissionResponse, error) {
 	namespaced := policy.IsNamespaced()
@@ -140,6 +170,9 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
 		if err := validateJSONPatchPathForForwardSlash(rule.Mutation.PatchesJSON6902); err != nil {
 			return nil, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err)
 		}
+		if err := validateJSONPatch(rule.Mutation.PatchesJSON6902, i); err != nil {
+			return nil, fmt.Errorf("%s", err)
+		}
 
 		if jsonPatchOnPod(rule) {
 			msg := "Pods managed by workload controllers should not be directly mutated using policies. " +

pkg/policy/validate_test.go

@@ -1599,6 +1599,38 @@ func Test_PodControllerAutoGenExclusion_None_Policy(t *testing.T) {
 	assert.NilError(t, err)
 }
 
+func Test_ValidateJSON6902(t *testing.T) {
+	var patch string = `- path: "/metadata/labels/img"
+  op: addition
+  value: "nginx"`
+	err := validateJSONPatch(patch, 0)
+	assert.Error(t, err, "Unexpected kind: spec.rules[0]: addition")
+
+	patch = `- path: "/metadata/labels/img"
+  op: add
+  value: "nginx"`
+	err = validateJSONPatch(patch, 0)
+	assert.NilError(t, err)
+
+	patch = `- path: "/metadata/labels/img"
+  op: add
+  value: nginx"`
+	err = validateJSONPatch(patch, 0)
+	assert.Error(t, err, `missing quote around value: spec.rules[0]: nginx"`)
+
+	patch = `- path: "/metadata/labels/img"
+  op: add
+  value: {"node.kubernetes.io/role": test"}`
+	err = validateJSONPatch(patch, 0)
+	assert.Error(t, err, `missing quote around value: spec.rules[0]: map[node.kubernetes.io/role:test"]`)
+
+	patch = `- path: "/metadata/labels/img"
+  op: add
+  value: "nginx"`
+	err = validateJSONPatch(patch, 0)
+	assert.NilError(t, err)
+}
+
 func Test_ValidateNamespace(t *testing.T) {
 	testcases := []struct {
 		description   string