Proposal: Add Req.secret to Req

[venkatd]

I notice you have a private method where you redact secrets in Req like so:

    defp redact(string) do
      len = String.length(string)

      if len < 4 do
        String.duplicate("*", len)
      else
        String.slice(string, 0, 3) <> String.duplicate("*", len - 3)
      end
    end

I was wondering if exposing something like Req.secret or Req.Secret.new to wrap secrets (for example to avoid mistakenly printing them out in livebook) could be a general purpose solution.

In our project we have something like this:

defmodule Ex.Req.Secret do
  defstruct [:value]

  def new(value) do
    %__MODULE__{value: value}
  end

  defimpl Inspect do
    def inspect(%Ex.Req.Secret{value: value}, _opts) do
      len = String.length(value)

      if len < 4 do
        "#Req.Secret<" <> String.duplicate("*", len) <> ">"
      else
        "#Req.Secret<" <> String.slice(value, 0, 3) <> String.duplicate("*", len - 3) <> ">"
      end
    end
  end

  defimpl String.Chars do
    def to_string(%Ex.Req.Secret{value: value}) do
      value
    end
  end
end

Then if I print out a Req client in livebook, the options will show up like this:

...
        access_key_id: #Ex.Req.Secret<abc*****************************>,
        secret_access_key: #Ex.Req.Secret<def*************************************************************>,
...

Thoughts?

[wojtekmach]

Thank you for starting the discussion, I think this is pretty clever! Right now I maintain an allowlist of what headers or options are redacted. I was thinking about an extensibility mechanism along the lines of #282. If I'm understanding your proposal vs #282 it would be the following. Say we need to set an x-foo-auth header and a bar.baz option for some other step:

# #461
iex> Req.new(
...>    headers: [x_foo_auth: Req.secret("foobar")],
...>    bar: [baz: Req.secret("barbar")]]
...> )
%Req.Request{
  headers: %{
    "x-foo-auth": #Req.Secret<foo***>
  },
  options: %{
    bar: [baz: #Req.Secret<bar***>]
  }
}

# #282
iex> Req.new(
...>  redact_headers: [:x_foo_auth],
...>  redact_options: [bar: :baz],
...>  headers: [x_foo_auth: "foobar"],
...>  bar: [baz: "barbar"]]
...>)
%Req.Request{
  headers: %{
    "x-foo-auth": "foo***"
  },
  options: %{
    bar: [baz: "bar***"]
  }
}

Did I get that comparison close enough? If so can you help me drawing up pros and cons of either approach?

[venkatd]

Yes that's pretty close!

For 461:
The main con I see with Req.secret is that it would somehow need to get unwrapped or have to_string called in all the places it could be used.

For example, if you were to access the Req struct directly to get options/headers directly, you might not think of handling a Req.secret scenario and assume all header values are strings.

The main advantage I think is the ease of use. You (or the implementor of a Req step) just have to know to wrap a secret with Req.secret whenever you use something and that's it.

Then for 282:
One downside is that you have to keep the keys in sync with the actual headers while with having a secret struct, it's all encapsulated there.

There's also the issue of having to deal with all the permutations of where the secrets can live. For example - some APIs (unfortunately!) have an api key as a query param or perhaps it could be nested inside of options. So then for headers, query params, and options, you'd have to start adding more fields redact_X fields to the Req struct.

Anything that you feel like I'm missing?

[a3kov]

Just wanted to add, sometimes even URL itself may contain sensitive info. Yes, it's dumb, but when you deal with a third-party service you have no way to undo their mistakes.