From 3084c37f3e1e5c1260fbc4a00082300ec0a7b0bd Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 9 Jul 2019 07:07:41 -0700 Subject: [PATCH] Perform more redactions in Redis performance bar traces HMSET and AUTH commands were not properly redacted. This commit does that and adds a test. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64309 --- lib/peek/views/redis.rb | 8 +++-- spec/lib/peek/views/redis_detailed_spec.rb | 36 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 spec/lib/peek/views/redis_detailed_spec.rb diff --git a/lib/peek/views/redis.rb b/lib/peek/views/redis.rb index ad3c3c9fe01..73de8672fa4 100644 --- a/lib/peek/views/redis.rb +++ b/lib/peek/views/redis.rb @@ -37,6 +37,8 @@ end module Peek module Views module RedisDetailed + REDACTED_MARKER = "" + def results super.merge(details: details) end @@ -57,10 +59,12 @@ module Peek end def format_command(cmd) + if cmd.length >= 2 && cmd.first =~ /^auth$/i + cmd[-1] = REDACTED_MARKER # Scrub out the value of the SET calls to avoid binary # data or large data from spilling into the view - if cmd.length >= 2 && cmd.first =~ /set/i - cmd[-1] = "" + elsif cmd.length >= 3 && cmd.first =~ /set/i + cmd[2..-1] = REDACTED_MARKER end cmd.join(' ') diff --git a/spec/lib/peek/views/redis_detailed_spec.rb b/spec/lib/peek/views/redis_detailed_spec.rb new file mode 100644 index 00000000000..da13b6df53b --- /dev/null +++ b/spec/lib/peek/views/redis_detailed_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Peek::Views::RedisDetailed do + let(:redis_detailed_class) do + Class.new do + include Peek::Views::RedisDetailed + end + end + + subject { redis_detailed_class.new } + + using RSpec::Parameterized::TableSyntax + + where(:cmd, :expected) do + [:auth, 'test'] | 'auth ' + [:set, 'key', 'value'] | 'set key ' + [:set, 'bad'] | 'set bad' + [:hmset, 'key1', 'value1', 'key2', 'value2'] | 'hmset key1 ' + [:get, 'key'] | 'get key' + end + + with_them do + it 'scrubs Redis commands', :request_store do + subject.detail_store << { cmd: cmd, duration: 1.second } + + expect(subject.details.count).to eq(1) + expect(subject.details.first) + .to eq({ + cmd: expected, + duration: 1000 + }) + end + end +end