opa icon indicating copy to clipboard operation
opa copied to clipboard

Upsert decision masking operation not supported with array paths

Open charlieegan3 opened this issue 1 year ago • 3 comments

From the docs, this should be supported:

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.

However, I am unable to get this to work. Reported here and in Slack.

input.json

{ "emails": [ { "email": "[email protected]" }, { "email": "[email protected]" } ]  }

policy.rego

package example

import rego.v1

default allow := false

allow if input.foo

system.rego

package system.log

import rego.v1

mask contains {"op": "upsert", "path": "/input/emails/0/email", "value": "foo" } 
# tab 1
opa run -s --set=decision_logs.console=true -b .
# tab 2
watch -n 1 curl localhost:8181/v0/data --data-binary @input.json

I see this output (not redacted):

{"bundles":{".":{}},"decision_id":"dba41d54-3e1b-46e6-a9b3-00d925c1f892","input":{"emails":[{"email":"[email protected]"},{"email":"[email protected]"}]},"labels":{"id":"49ffa192-3ad9-49c0-a3da-9ac0296c8dfd","version":"0.66.0"},"level":"info","metrics":{"counter_server_query_cache_hit":1,"timer_rego_external_resolve_ns":1375,"timer_rego_query_eval_ns":151708,"timer_server_handler_ns":262584},"msg":"Decision Log","req_id":6,"requested_by":"[::1]:58080","result":{"example":{"allow":false}},"time":"2024-07-23T11:24:28+01:00","timestamp":"2024-07-23T10:24:28.257819Z","type":"openpolicyagent.org/decision_logs"}

updating system.rego to:

package system.log

import rego.v1

mask contains {"op": "upsert", "path": "/input/emails", "value": "foo" } 

I see this output: (field upserted to foo)

{"bundles":{".":{}},"decision_id":"95c176e8-84db-44cd-a105-20dadd46f083","input":{"emails":"foo"},"labels":{"id":"8cb81d60-8e9b-4f92-8d36-4b7b1f3093ce","version":"0.66.0"},"level":"info","masked":["/input/emails"],"metrics":{"counter_server_query_cache_hit":1,"timer_rego_external_resolve_ns":917,"timer_rego_query_eval_ns":122250,"timer_server_handler_ns":215583},"msg":"Decision Log","req_id":4,"requested_by":"[::1]:58209","result":{"example":{"allow":false}},"time":"2024-07-23T11:25:26+01:00","timestamp":"2024-07-23T10:25:26.259216Z","type":"openpolicyagent.org/decision_logs"}

updating system.rego to

package system.log

import rego.v1

mask contains {"op": "remove", "path": "/input/emails/1/email" }

I see this output: (field removed)

{"bundles":{".":{}},"decision_id":"584c9e8b-1fc1-4ec3-b672-e0b7258f377b","erased":["/input/emails/1/email"],"input":{"emails":[{"email":"[email protected]"},{}]},"labels":{"id":"27628ec9-def6-4ac8-819d-9d556d02195a","version":"0.66.0"},"level":"info","metrics":{"counter_server_query_cache_hit":1,"timer_rego_external_resolve_ns":1292,"timer_rego_query_eval_ns":1470250,"timer_server_handler_ns":1591125},"msg":"Decision Log","req_id":2,"requested_by":"[::1]:58380","result":{"example":{"allow":false}},"time":"2024-07-23T11:26:50+01:00","timestamp":"2024-07-23T10:26:50.26311Z","type":"openpolicyagent.org/decision_logs"}

So in summary, it looks like upsert is not working when there is an array element in the path.

In mask_test.rego I can't see any tests for upserting when there's an array element in the path.

charlieegan3 avatar Jul 23 '24 10:07 charlieegan3

Seems like a bug. I would have expected /input/emails/0/email to be upserted.

ashutosh-narkar avatar Jul 23 '24 20:07 ashutosh-narkar

@RohitRox reported that there is a case I'd missed that does cover this case, but with the assertion that it's a no-op. https://github.com/open-policy-agent/opa/blob/main/plugins/logs/mask_test.go#L481C1-L482C1

So since that's not consistent with the docs, then one needs to change. I think we should be able to support this case as it seems valid to me.

We also support more obscure things like injecting values into bodies:

upsert: undefined object: missing key, provided value

charlieegan3 avatar Jul 25 '24 08:07 charlieegan3

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

stale[bot] avatar Aug 25 '24 10:08 stale[bot]