binaryseed / new_relic_absinthe

Absinthe Instrumentation for the New Relic Elixir Agent
https://hex.pm/packages/new_relic_absinthe
Apache License 2.0
19 stars 13 forks source link

Potential PII leak #14

Closed jamestelfer closed 4 years ago

jamestelfer commented 4 years ago

The default configuration for the middleware publishes the resolver arguments to New Relic, which may include PII. The information could be useful, but it would probably be better to be opt-in to avoid unexpected information leakage to a third party service.

https://github.com/binaryseed/new_relic_absinthe/blob/42e1cdbcfdeeed0fe55115b2a8c3a5b9ecb5d6d7/lib/new_relic/absinthe/middleware.ex#L50

A potential solution could be to make the addition of args to the parameters configurable.

What are your thoughts? Are you interested in a PR for this change?

sgtpepper43 commented 4 years ago

We had this same problem so we forked it and added a config similar to Absinthe Logger's config. We've been sitting on it for a couple of months but I just opened a PR for it today #15

jamestelfer commented 4 years ago

I left a comment on the PR; IMO to avoid PII leakage it shouldn't publish this information by default at all, instead making it opt-in.

binaryseed commented 4 years ago

Newer agent versions have a config function_argument_collection_enabled that I can leverage to determine if we should include the arguments...