librato / librato-metrics

Ruby wrapper to make it easy to interact with Librato's API.
https://librato.com
Other
108 stars 51 forks source link

Pass only part of env to raised errors in middleware #126

Closed ys closed 7 years ago

ys commented 7 years ago

Passing everything means that if it raise to an error tracker you might end up leaking credentials.

Raising the whole env will also leak request headers like Authorization.

The fact that the first parameter is a string means a lot of the bug trackers won't filter and scrub it.

Librato::Metrics::ClientError: #<struct Faraday::Env method=:post,
body="{\"errors\":{\"params\":{\"measurements\":[\"No measurements
found\"]}},\"request_time\":1476218695}", url=#<URI::HTTPS
https://metrics-api.librato.com/v1/metrics>,
request=#<Faraday::RequestOptions timeout=30, open_timeout=20>,
request_headers={"User-Agent"=>"librato-metrics/1.6.1 (ruby; 2.3.1p112;
x86_64-linux) direct-faraday/0.9.2", "Content-Type"=>"application/json",
"Authorization"=>"Basic
NOT_REDACTED_IN_BUG_TRACKER=="},
ssl=#<Faraday::SSLOptions verify=true>, parallel_manager=nil,
params=nil, response=#<Faraday::Response:0x007f1454966fa8
@on_complete_callbacks=[], @env=#<Faraday::Env @method=:post
@body="{\"errors\":{\"params\":{\"measurements\":[\"No measurements
found\"]}},\"request_time\":1476218695}" @url=#<URI::HTTPS
https://metrics-api.librato.com/v1/metrics>
@request=#<Faraday::RequestOptions timeout=30, open_timeout=20>
@request_headers={"User-Agent"=>"librato-metrics/1.6.1 (ruby; 2.3.1p112;
x86_64-linux) direct-faraday/0.9.2", "Content-Type"=>"application/json",
"Authorization"=>"Basic
NOT_REDACTED_IN_BUG_TRACKER=="}
@ssl=#<Faraday::SSLOptions verify=true>
@response=#<Faraday::Response:0x007f1454966fa8 ...>
@response_headers={"content-type"=>"application/json", "date"=>"Tue, 11
Oct 2016 20:44:55 GMT", "server"=>"nginx", "content-length"=>"90",
"connection"=>"Close"} @status=400 @custom={:request_body=>"{}"}>>,
response_headers={"content-type"=>"application/json", "date"=>"Tue, 11
Oct 2016 20:44:55 GMT", "server"=>"nginx", "content-length"=>"90",
"connection"=>"Close"}, status=400>

Work here is simply taking https://github.com/lostisland/faraday/blob/master/lib/faraday/response/raise_error.rb and applying it here.

nextmat commented 7 years ago

@ys - thanks for raising this issue. I think we may need more than you include here for debugging purposes but we can definitely be more selective. Thanks for starting this.

ys commented 7 years ago

@nextmat As long as you do something quick to stop creds leaking to bug trackers, I am fine with any solution.

ys commented 7 years ago

Done in #130