Redact fields in Ecto.InvalidChangesetError and mixed keys CastError messages

lib/ecto/changeset.ex

@@ -960,7 +960,7 @@ defmodule Ecto.Changeset do
               value: params,
               message:
                 "expected params to be a map with atoms or string keys, " <>
-                  "got a map with mixed keys: #{inspect(params)}"
+                  "got a map with mixed keys: #{inspect(Map.keys(params))}"
           end
         end
 

lib/ecto/exceptions.ex

@@ -89,8 +89,11 @@ defmodule Ecto.InvalidChangesetError do
   """
   defexception [:action, :changeset]
 
+  @redacted "**redacted**"
+
   def message(%{action: action, changeset: changeset}) do
     changes = extract_changes(changeset)
+    params = redact_params(changeset.params, changeset.data)
     errors = Ecto.Changeset.traverse_errors(changeset, & &1)
 
     """
@@ -106,7 +109,7 @@ defmodule Ecto.InvalidChangesetError do
 
     Params
 
-    #{pretty(changeset.params)}
+    #{pretty(params)}
 
     Changeset
 
@@ -120,11 +123,14 @@ defmodule Ecto.InvalidChangesetError do
     |> Enum.map_join("\n", &("    " <> &1))
   end
 
-  defp extract_changes(%Ecto.Changeset{changes: changes}) do
+  defp extract_changes(%Ecto.Changeset{changes: changes, data: data}) do
+    redacted_fields = redacted_fields(data)
+
     Enum.reduce(changes, %{}, fn {key, value}, acc ->
-      case value do
-        %Ecto.Changeset{action: :delete} -> acc
-        _ -> Map.put(acc, key, extract_changes(value))
+      cond do
+        match?(%Ecto.Changeset{action: :delete}, value) -> acc
+        key in redacted_fields -> Map.put(acc, key, @redacted)
+        true -> Map.put(acc, key, extract_changes(value))
       end
     end)
   end
@@ -137,6 +143,62 @@ defmodule Ecto.InvalidChangesetError do
 
   defp extract_changes(other),
     do: other
+
+  # Params are keyed by strings, so we map them back to the schema fields
+  # in order to redact the ones marked as `redact: true`, recursing into
+  # the params of associations and embeds.
+  defp redact_params(params, %schema{}) when is_map(params),
+    do: redact_params(params, schema)
+
+  defp redact_params(params, schema) when is_map(params) and is_atom(schema) do
+    if schema?(schema) do
+      redacted_fields = schema.__schema__(:redact_fields)
+      types = schema.__changeset__()
+      fields = Map.new(types, fn {field, _type} -> {Atom.to_string(field), field} end)
+
+      Map.new(params, fn {key, value} ->
+        with {:ok, field} <- Map.fetch(fields, to_string(key)) do
+          cond do
+            field in redacted_fields ->
+              {key, @redacted}
+
+            match?({kind, %{related: _}} when kind in [:assoc, :embed], types[field]) ->
+              {_kind, %{related: related, cardinality: cardinality}} = types[field]
+              {key, redact_relation_params(value, cardinality, related)}
+
+            true ->
+              {key, value}
+          end
+        else
+          :error -> {key, value}
+        end
+      end)
+    else
+      params
+    end
+  end
+
+  defp redact_params(params, _data), do: params
+
+  defp redact_relation_params(params, :one, schema) when is_map(params),
+    do: redact_params(params, schema)
+
+  defp redact_relation_params(params, :many, schema) when is_list(params),
+    do: Enum.map(params, &redact_relation_params(&1, :one, schema))
+
+  defp redact_relation_params(params, :many, schema) when is_map(params),
+    do: Map.new(params, fn {key, value} -> {key, redact_relation_params(value, :one, schema)} end)
+
+  defp redact_relation_params(params, _cardinality, _schema), do: params
+
+  defp redacted_fields(%schema{}) do
+    if schema?(schema), do: schema.__schema__(:redact_fields), else: []
+  end
+
+  defp redacted_fields(_data), do: []
+
+  defp schema?(module),
+    do: Code.ensure_loaded?(module) and function_exported?(module, :__schema__, 1)
 end
 
 defmodule Ecto.CastError do

test/ecto/changeset_test.exs

@@ -543,6 +543,19 @@ defmodule Ecto.ChangesetTest do
     end
   end
 
+  test "cast/4: does not print param values on mixed keys" do
+    exception =
+      assert_raise Ecto.CastError, fn ->
+        cast(%Post{}, %{"title" => "foo", body: "s3cret"}, ~w())
+      end
+
+    message = Exception.message(exception)
+
+    assert message =~ ~s(got a map with mixed keys: [:body, "title"])
+
+    refute message =~ "s3cret"
+  end
+
   test "cast/4: protects against atom injection" do
     assert_raise ArgumentError, fn ->
       cast(%Post{}, %{}, ~w(surely_never_saw_this_atom_before)a)
@@ -3634,6 +3647,17 @@ defmodule Ecto.ChangesetTest do
     end
   end
 
+  defmodule RedactedParentSchema do
+    use Ecto.Schema
+
+    schema "redacted_parent_schema" do
+      field :api_key, :string, redact: true
+      field :name, :string
+      embeds_one :credential, RedactedEmbeddedSchema
+      embeds_many :credentials, RedactedEmbeddedSchema
+    end
+  end
+
   describe "inspect" do
     test "reveals relevant data" do
       assert inspect(%Ecto.Changeset{}) ==
@@ -3713,4 +3737,91 @@ defmodule Ecto.ChangesetTest do
       assert inspect(changeset) =~ "**redacted**"
     end
   end
+
+  describe "Ecto.InvalidChangesetError" do
+    test "redacts fields marked redact: true in changes and params" do
+      changeset =
+        %RedactedSchema{}
+        |> cast(%{"username" => "hunter", "password" => "hunter2"}, [:username, :password])
+        |> validate_length(:username, min: 10)
+
+      exception =
+        assert_raise Ecto.InvalidChangesetError, fn -> apply_action!(changeset, :insert) end
+
+      message = Exception.message(exception)
+
+      assert message =~ "could not perform insert because changeset is invalid"
+      assert message =~ ~s(%{password: "**redacted**", username: "hunter"})
+      assert message =~ ~s(%{"password" => "**redacted**", "username" => "hunter"})
+      refute message =~ "hunter2"
+    end
+
+    test "redacts fields marked redact: true in nested embeds" do
+      params = %{
+        "api_key" => "sk-live-123",
+        "name" => "acme",
+        "credential" => %{"username" => "one", "password" => "one-secret"},
+        "credentials" => [
+          %{"username" => "two", "password" => "two-secret"},
+          %{"username" => "three", "password" => "three-secret"}
+        ]
+      }
+
+      changeset =
+        %RedactedParentSchema{}
+        |> cast(params, [:api_key, :name])
+        |> cast_embed(:credential, with: &cast(&1, &2, [:username, :password]))
+        |> cast_embed(:credentials, with: &cast(&1, &2, [:username, :password]))
+        |> validate_length(:name, min: 10)
+
+      exception =
+        assert_raise Ecto.InvalidChangesetError, fn -> apply_action!(changeset, :insert) end
+
+      message = Exception.message(exception)
+
+      assert message =~ "**redacted**"
+      refute message =~ "sk-live-123"
+      refute message =~ "one-secret"
+      refute message =~ "two-secret"
+      refute message =~ "three-secret"
+      assert message =~ "acme"
+      assert message =~ ~s("username" => "two")
+    end
+
+    test "redacts params of embeds_many given as an indexed map" do
+      params = %{
+        "name" => "acme",
+        "credentials" => %{"0" => %{"username" => "two", "password" => "two-secret"}}
+      }
+
+      changeset =
+        %RedactedParentSchema{}
+        |> cast(params, [:name])
+        |> cast_embed(:credentials, with: &cast(&1, &2, [:username, :password]))
+        |> validate_length(:name, min: 10)
+
+      exception =
+        assert_raise Ecto.InvalidChangesetError, fn -> apply_action!(changeset, :insert) end
+
+      message = Exception.message(exception)
+
+      refute message =~ "two-secret"
+      assert message =~ ~s("username" => "two")
+    end
+
+    test "keeps changes and params for schemaless changesets" do
+      changeset =
+        {%{}, %{title: :string}}
+        |> cast(%{"title" => "hello"}, [:title])
+        |> validate_length(:title, min: 10)
+
+      exception =
+        assert_raise Ecto.InvalidChangesetError, fn -> apply_action!(changeset, :insert) end
+
+      message = Exception.message(exception)
+
+      assert message =~ ~s(%{title: "hello"})
+      assert message =~ ~s(%{"title" => "hello"})
+    end
+  end
 end