Feature #22233
open
Warn when Net::HTTP#set_debug_output enables debug output
Description
Abstract¶
Net::HTTP#set_debug_output logs every request and response in plain text,
including Authorization and Cookie headers and message bodies. Its
documentation says never to use it in production, and nothing enforces that. I
propose writing a warning to $stderr when debug output is enabled, at most once
per thread.
Background¶
The method has carried this documentation for as long as it has existed:
The advice is sound and there is nothing behind it. The method behaves
identically in a console and in a production worker, so debug output committed by
accident, or enabled by hand during an incident and never reverted, keeps writing
credentials to whatever stream it was given. The credentials are then retained and indexed.
The only warning the method has today concerns call ordering, not safety:
Proposal¶
Warn when set_debug_output is enabled:
$ ruby -rnet/http -e 'Net::HTTP.new("example.com").set_debug_output($stdout)'
-e:1: warning: Net::HTTP#set_debug_output: every request and response, including Authorization and Cookie headers and message bodies, will be written to the given stream in plain text; never enable this in production
Nothing is written when:
$DEBUGis set. Ruby already has a flag meaning "this process is being
debugged", and enabling wire logging under it is deliberate.net/http's own
test harness reads it the same way, intest/net/http/utils.rb.outputisnil, which disables debug output rather than enabling it.- the warning has already been written on this thread.
The once-per-thread flag lives in thread-local storage. set_debug_output is a
configuration setter that net/http never calls itself, so a connection
configured once and reused pays nothing per request. But wrappers that build a
Net::HTTP per request and configure it each time would otherwise repeat an
identical message on every request and flood the log the warning is meant to draw
attention to. Thread-local storage rather than a class-level instance variable
because assigning the latter from a non-main Ractor raises
Ractor::IsolationError, and Thread#thread_variable_set has been available
since 2.0, so no version guard is needed.
Use cases¶
- A
set_debug_outputcall reaches production in a release. Today the first sign
is credentials in the log store. - Debug output is enabled while diagnosing a production incident and not
reverted. The next deploy carries it forward silently. - A gem enables debug output on a connection it owns. The application never sees
the call and today gets no indication that its requests are being logged.
Discussion¶
Why not detect production and warn only there? Because Ruby has no concept of
an application environment to consult. No file in lib/ reads RAILS_ENV,
RACK_ENV or APP_ENV; there is no RUBY_ENV; RbConfig describes the build,
not the deployment. Reading a framework's variable would put knowledge of that
framework into the standard library and force it to pick a side. Rails, which resolves
RAILS_ENV then RACK_ENV, and Sinatra, which resolves APP_ENV then
RACK_ENV, etc. It would also still miss the case most relevant to a standard
library: a script with no framework and no variables set. Warning unconditionally
and exempting $DEBUG avoids all of it and covers more.
Why not gate on $VERBOSE?$VERBOSE is false unless -w is passed,
which would hide this in exactly the unattended runs it exists to catch. The
existing "called after HTTP started" warning in this same method is not gated
either.
Why not a Warning category? Of the categories Ruby 4.0 defines
(:deprecated, :experimental, :performance, :strict_unused_block), only
:experimental is enabled by default, and none of them describes this. A
guardrail nobody has enabled does not guard anything. If ruby-core would prefer a
category, an always-on :security category has precedent in :experimental and
would be a reasonable separate proposal; I would rather not block this on
introducing one. The warning can already be intercepted by overriding
Warning.warn.
Why not deprecate the method? It is intended to exist and is useful.
Warning[:deprecated] is also off by default, so a deprecation would be
invisible in production. Feature #19630 used deprecation correctly because that
behavior genuinely was scheduled for removal.
Updated by