plugins/logs: Add support for masking with array keys (#7246)

Fixes #6883

Signed-off-by: Charlie Egan <[email protected]>

docs/content/management-decision-logs.md

@@ -184,15 +184,17 @@ from the decision log event. The erased paths are recorded on the event itself:
 There are a few restrictions on the JSON Pointers that OPA will erase:
 
 * Pointers must be prefixed with `/input`, `/result`, or `/nd_builtin_cache`.
-* Pointers may be undefined. For example `/input/name/first` in the example
-  above would be undefined. Undefined pointers are ignored.
-* Pointers must refer to object keys. Pointers to array elements will be treated
-  as undefined. For example `/input/emails/0/value` is allowed but `/input/emails/0` is not.
+* Pointers may point to undefined data. For example `/input/name/first` in the
+  example above would be undefined. Masking operations on undefined pointers are
+  ignored.
+* Pointers can also refer to arrays both as part of the path and as the last
+  element in the path. For example, both `/input/users/0/name` and
+  `/input/users/0` would be valid.
 
 In order to **modify** the contents of an input field, the **mask** rule may utilize the following format.
 
 * `"op"` -- The operation to apply when masking. All operations are done at the
-  path specified.  Valid options include:
+  path specified. Valid options include:
 
 |  op | Description  |
 |-----|--------------|

v1/plugins/logs/mask.go

@@ -24,9 +24,7 @@ const (
 	partNDBCache = "nd_builtin_cache"
 )
 
-var (
-	errMaskInvalidObject = fmt.Errorf("mask upsert invalid object")
-)
+var errMaskInvalidObject = fmt.Errorf("mask upsert invalid object")
 
 type maskRule struct {
 	OP                maskOP      `json:"op"`
@@ -79,7 +77,7 @@ func newMaskRule(path string, opts ...maskRuleOption) (*maskRule, error) {
 		escapedParts[i] = url.PathEscape(parts[i])
 	}
 
-	modifyFullObj := false
+	var modifyFullObj bool
 	if len(escapedParts) == 1 {
 		modifyFullObj = true
 	}
@@ -126,7 +124,6 @@ func withFailUndefinedPath() maskRuleOption {
 }
 
 func (r maskRule) Mask(event *EventV1) error {
-
 	var maskObj *interface{}     // pointer to event Input|Result|NDBCache object
 	var maskObjPtr **interface{} // pointer to the event Input|Result|NDBCache pointer itself
 
@@ -167,28 +164,16 @@ func (r maskRule) Mask(event *EventV1) error {
 		if r.modifyFullObj {
 			*maskObjPtr = nil
 		} else {
-
-			parent, err := r.lookup(r.escapedParts[1:len(r.escapedParts)-1], *maskObj)
+			err := r.removeValue(r.escapedParts[1:], *maskObj)
 			if err != nil {
 				if err == errMaskInvalidObject && r.failUndefinedPath {
 					return err
 				}
-			}
-			parentObj, ok := parent.(map[string]interface{})
-			if !ok {
 				return nil
 			}
-
-			fld := r.escapedParts[len(r.escapedParts)-1]
-			if _, ok := parentObj[fld]; !ok {
-				return nil
-			}
-
-			delete(parentObj, fld)
-
 		}
-		event.Erased = append(event.Erased, r.String())
 
+		event.Erased = append(event.Erased, r.String())
 	case maskOPUpsert:
 		if r.modifyFullObj {
 			*maskObjPtr = &r.Value
@@ -206,71 +191,148 @@ func (r maskRule) Mask(event *EventV1) error {
 				return nil
 			}
 		}
-		event.Masked = append(event.Masked, r.String())
 
+		event.Masked = append(event.Masked, r.String())
 	default:
 		return fmt.Errorf("illegal mask op value: %s", r.OP)
 	}
 
 	return nil
-
 }
 
-func (r maskRule) lookup(p []string, node interface{}) (interface{}, error) {
-	for i := 0; i < len(p); i++ {
+func (r maskRule) removeValue(p []string, node interface{}) error {
+	if len(p) == 0 {
+		return nil
+	}
+
+	// the key or index to be removed
+	targetKey := p[len(p)-1]
+
+	// nodeParent stores the parent of the node to be modified during the
+	// removal, this is only needed when the node is a slice
+	var nodeParent interface{}
+	// nodeKey stores the key of the node to be modified relative to the parent
+	var nodeKey string
+
+	// Walk to the parent of the target to be removed, the nodeParent is cached
+	// support removing of slice values
+	for i := 0; i < len(p)-1; i++ {
 		switch v := node.(type) {
 		case map[string]interface{}:
-			var ok bool
-			if node, ok = v[p[i]]; !ok {
-				return nil, errMaskInvalidObject
+			child, ok := v[p[i]]
+			if !ok {
+				return errMaskInvalidObject
 			}
+			nodeParent = v
+			nodeKey = p[i]
+			node = child
+
 		case []interface{}:
-			idx, err := strconv.Atoi(p[i])
+			index, err := strconv.Atoi(p[i])
+			if err != nil || index < 0 || index >= len(v) {
+				return errMaskInvalidObject
+			}
+			nodeParent = v
+			nodeKey = p[i]
+			node = v[index]
+
+		default:
+			return errMaskInvalidObject
+		}
+	}
+
+	switch v := node.(type) {
+	case map[string]interface{}:
+		if _, ok := v[targetKey]; !ok {
+			return errMaskInvalidObject
+		}
+
+		delete(v, targetKey)
+
+	case []interface{}:
+		// first, check the targetKey is a valid index
+		targetIndex, err := strconv.Atoi(targetKey)
+		if err != nil || targetIndex < 0 || targetIndex >= len(v) {
+			return errMaskInvalidObject
+		}
+
+		switch nodeParent := nodeParent.(type) {
+		case []interface{}:
+			// update the target's grandparent slice with a new slice
+			index, err := strconv.Atoi(nodeKey)
 			if err != nil {
-				return nil, errMaskInvalidObject
-			} else if idx < 0 || idx >= len(v) {
-				return nil, errMaskInvalidObject
+				return errMaskInvalidObject
 			}
-			node = v[idx]
+
+			nodeParent[index] = append(v[:targetIndex], v[targetIndex+1:]...)
+
+		case map[string]interface{}:
+			nodeParent[nodeKey] = append(v[:targetIndex], v[targetIndex+1:]...)
+
 		default:
-			return nil, errMaskInvalidObject
+			return errMaskInvalidObject
 		}
+
+	default:
+		return errMaskInvalidObject
 	}
 
-	return node, nil
+	return nil
 }
 
-func (r maskRule) mkdirp(node map[string]interface{}, path []string, value interface{}) error {
+func (r maskRule) mkdirp(node interface{}, path []string, value interface{}) error {
 	if len(path) == 0 {
 		return nil
 	}
 
-	// create intermediate nodes
 	for i := 0; i < len(path)-1; i++ {
-		child, ok := node[path[i]]
+		switch v := node.(type) {
+		case map[string]interface{}:
+			child, ok := v[path[i]]
+			if !ok {
+				child = map[string]interface{}{}
+				v[path[i]] = child
+			}
 
-		if !ok {
-			child := map[string]interface{}{}
-			node[path[i]] = child
 			node = child
-			continue
-		}
 
-		switch obj := child.(type) {
-		case map[string]interface{}:
-			node = obj
+		case []interface{}:
+			idx, err := strconv.Atoi(path[i])
+			if err != nil || idx < 0 {
+				return errMaskInvalidObject
+			}
+
+			for len(v) <= idx {
+				v = append(v, nil)
+			}
+
+			node = v[idx]
+
 		default:
 			return errMaskInvalidObject
 		}
+	}
+
+	switch v := node.(type) {
+	case map[string]interface{}:
+		v[path[len(path)-1]] = value
+
+	case []interface{}:
+		idx, err := strconv.Atoi(path[len(path)-1])
+		if err != nil || idx < 0 || idx >= len(v) {
+			return errMaskInvalidObject
+		}
+		v[idx] = value
 
+	default:
+		return errMaskInvalidObject
 	}
 
-	node[path[len(path)-1]] = value
 	return nil
 }
 
 func newMaskRuleSet(rv interface{}, onRuleError func(*maskRule, error)) (*maskRuleSet, error) {
-	var mRuleSet = &maskRuleSet{
+	mRuleSet := &maskRuleSet{
 		OnRuleError: onRuleError,
 	}
 	rawRules, ok := rv.([]interface{})
@@ -279,7 +341,6 @@ func newMaskRuleSet(rv interface{}, onRuleError func(*maskRule, error)) (*maskRu
 	}
 
 	for _, iface := range rawRules {
-
 		switch v := iface.(type) {
 
 		case string:
@@ -310,12 +371,10 @@ func newMaskRuleSet(rv interface{}, onRuleError func(*maskRule, error)) (*maskRu
 
 			// use unmarshalled values to create new Mask Rule
 			rule, err := newMaskRule(rule.Path, withOP(rule.OP), withValue(rule.Value))
-
 			// TODO add withFailUndefinedPath() option based on
 			//   A) new syntax in user defined mask rule
 			//   B) passed in/global configuration option
 			//   rule precedence A>B
-
 			if err != nil {
 				return nil, err
 			}

v1/plugins/logs/mask_test.go

@@ -15,7 +15,6 @@ import (
 )
 
 func TestNewMaskRule(t *testing.T) {
-
 	tests := []struct {
 		note   string
 		input  *maskRule
@@ -181,7 +180,6 @@ func TestNewMaskRule(t *testing.T) {
 }
 
 func TestMaskRuleMask(t *testing.T) {
-
 	tests := []struct {
 		note   string
 		ptr    *maskRule
@@ -487,19 +485,68 @@ func TestMaskRuleMask(t *testing.T) {
 			exp:   `{"input": {"foo": [{"baz": 1}]}}`,
 		},
 		{
-			note: "erase: undefined array: remove element",
+			note: "erase: array: remove element",
 			ptr: &maskRule{
 				OP:   maskOPRemove,
 				Path: "/input/foo/0",
 			},
 			event: `{"input": {"foo": [1]}}`,
-			exp:   `{"input": {"foo": [1]}}`,
+			exp:   `{"input": {"foo": []}, "erased": ["/input/foo/0"]}`,
 		},
 		{
-			note: "upsert: unsupported nested object type (array) #2",
+			note: "erase: array: remove element with deeper nesting",
 			ptr: &maskRule{
-				OP:   maskOPUpsert,
-				Path: "/input/foo/0",
+				OP:   maskOPRemove,
+				Path: "/input/foo/0/bar/1",
+			},
+			event: `{"input": {"foo": [{"bar": [1, 2]}]}}`,
+			exp:   `{"input": {"foo": [{"bar": [1]}]}, "erased": ["/input/foo/0/bar/1"]}`,
+		},
+		{
+			note: "erase: array: remove element that does not exist",
+			ptr: &maskRule{
+				OP:   maskOPRemove,
+				Path: "/input/foo/0/bar/9",
+			},
+			event: `{"input": {"foo": [{"bar": [1, 2]}]}}`,
+			exp:   `{"input": {"foo": [{"bar": [1, 2]}]}}`,
+		},
+		{
+			note: "upsert: array: upsert element",
+			ptr: &maskRule{
+				OP:    maskOPUpsert,
+				Path:  "/input/foo/0",
+				Value: 2,
+			},
+			event: `{"input": {"foo": [1]}}`,
+			exp:   `{"input": {"foo": [2]}, "masked": ["/input/foo/0"]}`,
+		},
+		{
+			note: "upsert: array: upsert nested array element",
+			ptr: &maskRule{
+				OP:    maskOPUpsert,
+				Path:  "/input/foo/0/bar/0",
+				Value: 2,
+			},
+			event: `{"input": {"foo": [{"bar": [1]}]}}`,
+			exp:   `{"input": {"foo": [{"bar": [2]}]}, "masked": ["/input/foo/0/bar/0"]}`,
+		},
+		{
+			note: "upsert: array: upsert element in 2d array",
+			ptr: &maskRule{
+				OP:    maskOPUpsert,
+				Path:  "/input/foo/0/0",
+				Value: 2,
+			},
+			event: `{"input": {"foo": [[1]]}}`,
+			exp:   `{"input": {"foo": [[2]]}, "masked": ["/input/foo/0/0"]}`,
+		},
+		{
+			note: "upsert: array: upsert element that does not exist",
+			ptr: &maskRule{
+				OP:    maskOPUpsert,
+				Path:  "/input/foo/1",
+				Value: 2,
 			},
 			event: `{"input": {"foo": [1]}}`,
 			exp:   `{"input": {"foo": [1]}}`,
@@ -564,7 +611,6 @@ func TestMaskRuleMask(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.note, func(t *testing.T) {
-
 			ptr, err := newMaskRule(tc.ptr.Path, withOP(tc.ptr.OP), withValue(tc.ptr.Value))
 			if tc.ptr.failUndefinedPath {
 				_ = withFailUndefinedPath()(ptr)
@@ -641,7 +687,6 @@ func TestNewMaskRuleSet(t *testing.T) {
 }
 
 func TestMaskRuleSetMask(t *testing.T) {
-
 	tests := []struct {
 		note   string
 		rules  []*maskRule