Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove-omissions not working #98

Closed
eoogbe opened this issue Nov 19, 2022 · 1 comment
Closed

remove-omissions not working #98

eoogbe opened this issue Nov 19, 2022 · 1 comment

Comments

@eoogbe
Copy link

eoogbe commented Nov 19, 2022

remove-omissions does not actually remove sensitive keys because of 2 bugs in its implementation.

First, look at what gets passed in:

(defn remove-omissions
  [config tx] ; This takes in a config
  (let [sensitive-keys (get-in config [::config :sensitive-keys] #{})]
    ; ...
    ))

(defn- log! [env msg value]
  ; ...
  (pr-str (remove-omissions config value))
  ; ...
  )

(defn log-request! [{:keys [env tx] :as req}]
  (let [config         (:config env)
        sensitive-keys (conj
                         (get-in config [::config :sensitive-keys] #{})
                         :com.wsscode.pathom/trace)]
    (log! env "transaction: " (remove-omissions sensitive-keys tx)) ; This is sending sensitive-keys
    req))

(defn log-response!
  [{:keys [config] :as env} response]
  (let [sensitive-keys (conj
                         (get-in config [::config :sensitive-keys] #{})
                         :com.wsscode.pathom/trace)]
    (log! env "response: " (remove-omissions sensitive-keys response)) ; This is sending sensitive-keys
    response))

log-request! and log-response! are parsing the sensitive keys to send into remove-omissions even though remove-omissions repeats this actions. That means what reaches remove-omissions is nil.

The second bug is that the config passed into new-parser is never actually put into the env. So {:keys [config] :as env} will always get nil as the config.

@awkay
Copy link
Member

awkay commented Nov 21, 2022

To be honest, I'd rather just delete that whole mess than fix it. Be better for the user to just add their own plugin to accomplish it, which is the solution I'd suggest to you.

@awkay awkay closed this as completed Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants