diff --git a/docs/content/management-decision-logs.md b/docs/content/management-decision-logs.md index 6dee5a948b..efedb21184 100644 --- a/docs/content/management-decision-logs.md +++ b/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 | |-----|--------------| diff --git a/v1/plugins/logs/mask.go b/v1/plugins/logs/mask.go index 81bb52af39..0acc4a7ed7 100644 --- a/v1/plugins/logs/mask.go +++ b/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 } diff --git a/v1/plugins/logs/mask_test.go b/v1/plugins/logs/mask_test.go index c4bfb4dbb7..0837bfd4eb 100644 --- a/v1/plugins/logs/mask_test.go +++ b/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