feat: redact middleware and adapter options when inspecting Tesla.Client - #939
BlueCollarChris wants to merge 3 commits into
Conversation
A %Tesla.Client{} holds every middleware with its options, and a
%Tesla.Env{} embeds the client as __client__. With the default Inspect,
`inspect(env)` therefore prints an Authorization header handed to
Tesla.Middleware.Headers, an API key in Tesla.Middleware.Query, or any
other secret a middleware was configured with — and the pattern
`Logger.error("request failed: #{inspect(env)}")` is common enough that
this reaches production log streams.
Add an Inspect implementation for Tesla.Client that renders middleware
and adapter module names but redacts their options:
#Tesla.Client<adapter: {Tesla.Adapter.Finch, :redacted},
middleware: [{Tesla.Middleware.Headers, :redacted}, Tesla.Middleware.JSON]>
`config :tesla, inspect: :full` opts back in to the previous rendering
for debugging. Tesla.Client.middleware/1 and adapter/1 are unchanged and
remain the way to read a client's configuration from code.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Adds a custom Documentation: new Reviewed by Cursor Bugbot for commit 0071e29. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
🟡 Changes recommended
The new test helper that temporarily sets :tesla, :inspect restores prior config using a truthiness check that can mis-restore falsy values, risking incorrect global config cleanup.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens Tesla’s developer-facing logging/inspection behavior by implementing a custom Inspect protocol for Tesla.Client that redacts middleware and adapter options by default, reducing the risk of credential leakage when %Tesla.Env{} (which embeds __client__) is inspected.
Changes:
- Add
Inspectimplementation forTesla.Clientthat prints module names but redacts options unlessconfig :tesla, inspect: :fullis set. - Add a new
@moduledoctoTesla.Clientdocumenting the redaction behavior and opt-in full inspection. - Add tests and guide documentation covering default redaction,
:fullmode, env embedding, andpostmiddleware rendering.
File summaries
| File | Description |
|---|---|
| lib/tesla/client.ex | Adds @moduledoc and a redacting Inspect implementation for Tesla.Client. |
| test/tesla/client_test.exs | Adds coverage for redacted vs full inspection, env embedding, and post middleware rendering. |
| guides/explanations/0.client.md | Documents the new default redaction behavior and how to opt into full inspect output. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The change is well-scoped, addresses a concrete secret-leak risk, and includes targeted tests and documentation to validate and explain the new behavior.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/tesla/client.ex:224
- The redaction logic currently inspects Tesla’s internal runtime tuples (e.g.
{Module, :call, [opts]}) and re-implementsunruntime/1inside theInspectimpl. This couplesInspectto internal representation details and duplicates the unruntime logic already maintained inTesla.Client.adapter/1andTesla.Client.middleware/1, increasing the risk that future changes to runtime stack representation will update one place but not the other.
You can avoid this by first converting adapter/middleware stacks back to the public “user form” via Tesla.Client.adapter/1 / Tesla.Client.middleware/1 (using a temporary %Tesla.Client{pre: stack} for post), and then applying redaction to those user-form entries.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…re/1
Build the rendering from the public user-form returned by
Tesla.Client.adapter/1 and Tesla.Client.middleware/1 instead of matching
the runtime {module, :call, [opts]} tuples and duplicating unruntime/1.
The Inspect impl no longer depends on the internal stack representation.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fa5e5f6 to
0071e29
Compare
|
@BlueCollarChris, first of all, I strongly agree with the intent here. However, after thinking about it for a while, I wouldn’t take this approach for a few reasons:
Instead, I would make secrecy explicit by introducing a value type such as: defmodule Tesla.SecretString do
@opaque t :: %__MODULE__{value: String.t()}
defstruct [:value]
def new(value) when is_binary(value) do
%__MODULE__{value: value}
end
end
defimpl Inspect, for: Tesla.SecretString do
def inspect(_val, _opts) do
"Tesla.SecretString<redacted>"
end
end
defimpl String.Chars, for: Tesla.SecretString do
def to_string(secret) do
secret.value
end
end
Tesla.client([
{Tesla.Middleware.Headers,
[
{"authorization", Tesla.SecretString.new("Bearer #{token}")}
]}
])Middleware could then wrap sensitive values in Something around those lines, |
|
Ill take a look at this approach a bit more. |
|
@BlueCollarChris any updates? |
Problem
A
%Tesla.Client{}carries every middleware together with its options, and a%Tesla.Env{}embeds the client as__client__. Both use the defaultInspect, so this:writes
Bearer s3cretto the log. The same applies to an API key inTesla.Middleware.Query, credentials in a custom middleware, or adapter options. TheLogger.error("...#{inspect(env)}")pattern is common in error handling, so this tends to surface as a credential in a production log aggregator rather than in a debugging session. We found it that way in a service that wraps a third-party API client built on Tesla: the provider API key appeared in the logs on every non-2xx response.Change
An
Inspectimplementation forTesla.Clientthat shows middleware and adapter modules but redacts their options by default:{Module, :redacted}; middleware without options render as the bare module; function middleware/adapters render as:fn.postmiddleware is shown only when present.__client__is rendered through this implementation,inspect(env)no longer prints the client's secrets either.config :tesla, inspect: :full. This follows the existingApplication.get_env(:tesla, ...)convention used for the adapter and the Logger middleware.Tesla.Client.middleware/1andTesla.Client.adapter/1are unchanged and remain the programmatic way to read a client's configuration.Docs: a
@moduledocforTesla.Client(it had none) and a short "Inspecting a client" section inguides/explanations/0.client.md.Why redact by default rather than opt in
inspect/1output is not a stable API and nothing in Tesla parses it. The leak is silent and lands in the place people look least (production logs), so the safer default seems right, with the previous rendering one config line away. Happy to flip it to opt-in if you'd prefer to keep current behaviour as the default.Not covered here
Request headers on the env itself (
env.headers) still print when an env is inspected before or after a request, since they are legitimately part of the env. Redacting those is a bigger behavioural question (which headers, and whetherTesla.Middleware.Logger's:filter_headersshould be shared with it), so I've kept this PR to the client, which is where the configuration secrets live.Tests
test/tesla/client_test.exsgains anInspectdescribe block: defaults redact header values, base URL and adapter options while keeping module names; an env embedding the client does not print the client's secrets;inspect: :fullrestores the original forms; empty client;postmiddleware. Doctests in the new moduledoc run via the existingdoctest Tesla.Client.