diff --git a/lib/httpigeon/log_redactor.rb b/lib/httpigeon/log_redactor.rb index 9536d32..716ff01 100644 --- a/lib/httpigeon/log_redactor.rb +++ b/lib/httpigeon/log_redactor.rb @@ -48,15 +48,21 @@ def redact_hash(data) if filter.present? replacement = filter.split('::')[1].presence - v = replacement.present? ? replacement : redact_value(v) - end - - if v.is_a?(Hash) - [k, redact_hash(v)] - elsif v.is_a?(Array) - [k, v.map { |val| redact(val) }] - else + # Replace collections wholesale; partial redaction leaks bytes of + # nested secrets. + v = if replacement.present? + replacement + elsif v.is_a?(Hash) || v.is_a?(Array) + HTTPigeon.redactor_string + else + redact_value(v) + end [k, v] + else + # Recurse through the dispatcher so nested values redact like + # top-level ones. This runs content/regex filters over string + # values too (e.g. secrets in JSON bodies). + [k, redact(v)] end end end @@ -67,13 +73,23 @@ def redact_string(data) next unless pattern.match?(%r{^/.*/([guysim]*)$}) + regex = regex_for(pattern) + data = if replacement.present? - data.gsub(regex_for(pattern), replacement) + # Block form keeps the replacement literal; the string form + # expands backreferences (\1, \&) and re-emits the secret. + data.gsub(regex) { replacement } else - data.gsub(regex_for(pattern)) do |sub| - captures = sub.match(regex_for(pattern))&.captures - - captures.present? ? captures[0] + redact_value(captures[1]) : sub + data.gsub(regex) do |sub| + captures = Regexp.last_match&.captures + + # A zero-width capture yields nil; skip it rather than + # concatenating nil and raising TypeError. + if captures.present? && !captures[1].nil? + captures[0] + redact_value(captures[1]) + else + sub + end end end end diff --git a/lib/httpigeon/request.rb b/lib/httpigeon/request.rb index 87b359f..3537f31 100644 --- a/lib/httpigeon/request.rb +++ b/lib/httpigeon/request.rb @@ -84,12 +84,12 @@ def run(method: :get, path: '/', payload: {}) raw_response = if HTTPigeon.mount_circuit_breaker fuse.execute(request_id: connection.headers[REQUEST_ID_HEADER]) do - connection.send(method, path, payload) do |request| + connection.send(sym_method, path, payload) do |request| yield(request) if block_given? end end else - connection.send(method, path, payload) do |request| + connection.send(sym_method, path, payload) do |request| yield(request) if block_given? end end diff --git a/spec/httpigeon/log_redactor_spec.rb b/spec/httpigeon/log_redactor_spec.rb index 983cdc1..f763722 100644 --- a/spec/httpigeon/log_redactor_spec.rb +++ b/spec/httpigeon/log_redactor_spec.rb @@ -201,6 +201,54 @@ end end + context 'when a filtered key holds a nested collection' do + it 'fully redacts a nested hash instead of leaking fragments' do + filters = %w[token] + data = { token: { access: 'SUPER_SECRET_ACCESS_TOKEN_123456', refresh: 'REFRESH_SECRET_999' } } + + expect(described_class.new(log_filters: filters).redact(data)).to eq({ token: '' }) + end + + it 'fully redacts a nested array instead of leaking fragments' do + filters = %w[credentials] + data = { credentials: %w[SUPER_SECRET_ONE SUPER_SECRET_TWO] } + + expect(described_class.new(log_filters: filters).redact(data)).to eq({ credentials: '' }) + end + end + + context 'when a content filter matches a string value inside a hash' do + it 'redacts the secret regardless of the key name' do + filters = [HTTPigeon::FilterPatterns::CLIENT_SECRET] + data = { error_uri: 'https://idp/cb?client_id=abc&client_secret=REALSECRET' } + + expect(described_class.new(log_filters: filters).redact(data)).to eq( + { error_uri: 'https://idp/cb?client_id=abc&client_secret=REA...' } + ) + end + end + + context 'when a replacement contains a gsub backreference' do + it 'treats the backreference literally instead of re-emitting the secret' do + filters = ['/(password=)([^&]+)/::password=\2'] + data = 'https://api.example.com/auth?password=S3cr3tP@ssw0rd&grant_type=code' + + expect(described_class.new(log_filters: filters).redact(data)).to eq( + 'https://api.example.com/auth?password=\2&grant_type=code' + ) + end + end + + context 'when a grouped filter matches an empty capture' do + it 'does not raise and leaves the empty value in place' do + filters = %w[/(client_id=)([0-9a-z]+)*/] + data = 'action=login&client_id=&next=home' + + expect { described_class.new(log_filters: filters).redact(data) }.not_to raise_error + expect(described_class.new(log_filters: filters).redact(data)).to eq('action=login&client_id=&next=home') + end + end + context 'when filters include an invalid regex' do it 'raises an error' do filters = %w[/key_3=[0-9a-z]*/im::key_3= key_4] diff --git a/spec/httpigeon/middleware/httpigeon_logger_spec.rb b/spec/httpigeon/middleware/httpigeon_logger_spec.rb new file mode 100644 index 0000000..79ef9b8 --- /dev/null +++ b/spec/httpigeon/middleware/httpigeon_logger_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe HTTPigeon::Middleware::HTTPigeonLogger do + subject(:middleware) { described_class.new(app, logger) } + + let(:app) { ->(env) { env } } + let(:logger) { instance_double(HTTPigeon::Logger, on_request_start: nil, on_request_finish: nil, log: nil) } + let(:env) { Faraday::Env.new } + + describe '#call' do + context 'when the request succeeds' do + it 'logs on completion and returns the response' do + allow(app).to receive(:call).and_return(Faraday::Response.new(env)) + + middleware.call(env) + + expect(logger).to have_received(:on_request_start) + expect(logger).to have_received(:on_request_finish) + expect(logger).to have_received(:log).with(env) + end + end + + context 'when the request raises' do + let(:error) { StandardError.new('boom') } + + before { allow(app).to receive(:call).and_raise(error) } + + it 'logs the error and re-raises it' do + expect { middleware.call(env) }.to raise_error(error) + + expect(logger).to have_received(:on_request_finish) + expect(logger).to have_received(:log).with(env, { error: error }) + end + end + end +end diff --git a/spec/httpigeon/request_spec.rb b/spec/httpigeon/request_spec.rb index d1c698f..a7d3617 100644 --- a/spec/httpigeon/request_spec.rb +++ b/spec/httpigeon/request_spec.rb @@ -223,6 +223,21 @@ it 'raises ArgumentError for method as string' do expect { request.run(method: 'system', path: '/users', payload: {}) }.to raise_error(ArgumentError, 'Invalid or unsupported HTTP method: system') end + + it 'dispatches the validated symbol, not a to_sym/to_str divergent object' do + clever_method = Class.new do + def to_sym = :get + def to_str = 'instance_eval' + end.new + + allow(request.connection).to receive(:instance_eval) + allow(request.connection).to receive(:get).and_call_original + + request.run(method: clever_method, path: '/users', payload: {}) + + expect(request.connection).to have_received(:get) + expect(request.connection).not_to have_received(:instance_eval) + end end end