open-policy-agent / opa

Open Policy Agent (OPA) is an open source, general-purpose policy engine.
https://www.openpolicyagent.org
Apache License 2.0
9.52k stars 1.32k forks source link

Upsert decision masking operation not supported with array paths #6883

Open charlieegan3 opened 1 month ago

charlieegan3 commented 1 month ago

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": "usr1@example.com" }, { "email": "usr2@example.com" } ]  }

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":"usr1@example.com"},{"email":"usr2@example.com"}]},"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":"usr1@example.com"},{}]},"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.

ashutosh-narkar commented 1 month ago

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

charlieegan3 commented 1 month ago

@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

stale[bot] commented 2 weeks ago

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.