diff --git a/lib/ecto/changeset.ex b/lib/ecto/changeset.ex index 0f7257279e..0acd233e8c 100644 --- a/lib/ecto/changeset.ex +++ b/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 diff --git a/lib/ecto/exceptions.ex b/lib/ecto/exceptions.ex index 22b096375f..f8ccfd7334 100644 --- a/lib/ecto/exceptions.ex +++ b/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 diff --git a/test/ecto/changeset_test.exs b/test/ecto/changeset_test.exs index cc561882d3..a96eb440f3 100644 --- a/test/ecto/changeset_test.exs +++ b/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