Disable risky memory debugging tools by default (#402)

* Disable risky debugging tools by default

* Document the new option

* Remove disable_env_dump config option

* Improve new config description

* Enable when in development env

README.md

@@ -327,12 +327,12 @@ toggle_shortcut|Alt+P|Keyboard shortcut to toggle the mini_profiler's visibility
 start_hidden|`false`|`false` to make mini_profiler visible on page load.
 backtrace_threshold_ms|`0`|Minimum SQL query elapsed time before a backtrace is recorded.
 flamegraph_sample_rate|`0.5`|How often to capture stack traces for flamegraphs in milliseconds.
-disable_env_dump|`false`|`true` disables `?pp=env`, which prevents sending ENV vars over HTTP.
 base_url_path|`'/mini-profiler-resources/'`|Path for assets; added as a prefix when naming assets and sought when responding to requests.
 collapse_results|`true`|If multiple timing results exist in a single page, collapse them till clicked.
 max_traces_to_show|20|Maximum number of mini profiler timing blocks to show on one page
 html_container|`body`|The HTML container (as a jQuery selector) to inject the mini_profiler UI into
 show_total_sql_count|`false`|Displays the total number of SQL executions.
+enable_advanced_debugging_tools|`false`|Enables sensitive debugging tools that can be used via the UI. In production we recommend keeping this disabled as memory and environment debugging tools can expose contents of memory that may contain passwords.
 
 ### Custom middleware ordering (required if using `Rack::Deflate` with Rails)
 

lib/mini_profiler/config.rb

@@ -34,9 +34,9 @@ def self.default
             end
           end
           @enabled = true
-          @disable_env_dump = false
           @max_sql_param_length = 0 # disable sql parameter collection by default
           @skip_sql_param_names = /password/ # skips parameters with the name password by default
+          @enable_advanced_debugging_tools = false
 
           # ui parameters
           @autorized            = true
@@ -57,10 +57,10 @@ def self.default
 
       attr_accessor :authorization_mode, :auto_inject, :backtrace_ignores,
         :backtrace_includes, :backtrace_remove, :backtrace_threshold_ms,
-        :base_url_path, :disable_caching, :disable_env_dump, :enabled,
+        :base_url_path, :disable_caching, :enabled,
         :flamegraph_sample_rate, :logger, :pre_authorize_cb, :skip_paths,
         :skip_schema_queries, :storage, :storage_failure, :storage_instance,
-        :storage_options, :user_provider
+        :storage_options, :user_provider, :enable_advanced_debugging_tools
       attr_accessor :skip_sql_param_names, :suppress_encoding, :max_sql_param_length
 
       # ui accessors

lib/mini_profiler/profiler.rb

@@ -62,6 +62,11 @@ def request_authorized?
         Thread.current[:mp_authorized]
       end
 
+      def advanced_tools_message
+        <<~TEXT
+          This feature is disabled by default, to enable set the enable_advanced_debugging_tools option to true in Mini Profiler config.
+        TEXT
+      end
     end
 
     #
@@ -147,6 +152,14 @@ def config
       @config
     end
 
+    def advanced_debugging_enabled?
+      config.enable_advanced_debugging_tools
+    end
+
+    def tool_disabled_message(client_settings)
+      client_settings.handle_cookie(text_result(Rack::MiniProfiler.advanced_tools_message))
+    end
+
     def call(env)
 
       start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
@@ -195,12 +208,14 @@ def call(env)
 
       # profile gc
       if query_string =~ /pp=profile-gc/
+        return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
         current.measure = false if current
         return client_settings.handle_cookie(Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env))
       end
 
       # profile memory
       if query_string =~ /pp=profile-memory/
+        return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
         query_params = Rack::Utils.parse_nested_query(query_string)
         options = {
           ignore_files: query_params['memory_profiler_ignore_files'],
@@ -307,12 +322,14 @@ def call(env)
         return client_settings.handle_cookie(dump_exceptions exceptions)
       end
 
-      if query_string =~ /pp=env/ && !config.disable_env_dump
+      if query_string =~ /pp=env/
+        return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
         body.close if body.respond_to? :close
         return client_settings.handle_cookie(dump_env env)
       end
 
       if query_string =~ /pp=analyze-memory/
+        return tool_disabled_message(client_settings) if !advanced_debugging_enabled?
         body.close if body.respond_to? :close
         return client_settings.handle_cookie(analyze_memory)
       end

lib/mini_profiler_rails/railtie.rb

@@ -64,6 +64,7 @@ def self.initialize!(app)
       ::Rack::MiniProfiler.profile_method(ActionView::Template, :render) { |x, y| "Rendering: #{@virtual_path}" }
     end
 
+    c.enable_advanced_debugging_tools = Rails.env.development?
     @already_initialized = true
   end
 

spec/integration/middleware_spec.rb

@@ -24,6 +24,21 @@ def decompressed_response
     end
   end
 
+  describe 'when enable_advanced_debugging_tools is false' do
+    def app
+      Rack::Builder.new do
+        use Rack::MiniProfiler
+        run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
+      end
+    end
+    it 'advanced tools are disabled' do
+      %w{env analyze-memory profile-gc profile-memory}.each do |p|
+        do_get(pp: p)
+        expect(last_response.body).to eq(Rack::MiniProfiler.advanced_tools_message)
+      end
+    end
+  end
+
   describe 'with analyze-memory query' do
     def app
       Rack::Builder.new do
@@ -32,7 +47,8 @@ def app
       end
     end
 
-    it 'should return ObjectSpace statistics' do
+    it 'should return ObjectSpace statistics if advanced tools are enabled' do
+      Rack::MiniProfiler.config.enable_advanced_debugging_tools = true
       do_get(pp: 'analyze-memory')
       expect(last_response.body).to include('Largest strings:')
     end

spec/integration/mini_profiler_spec.rb

@@ -288,24 +288,13 @@ def load_prof(response)
       end
     end
 
-    describe 'disable_env_dump config option' do
-      context 'default (not configured' do
-        it 'allows env dump' do
-          get '/html?pp=env'
-
-          expect(last_response.body).to include('QUERY_STRING')
-          expect(last_response.body).to include('CONTENT_LENGTH')
-        end
-      end
-      context 'when enabled' do
-        it 'disables dumping the ENV over the web' do
-          Rack::MiniProfiler.config.disable_env_dump = true
-          get '/html?pp=env'
-
-          # Contains no ENV vars:
-          expect(last_response.body).not_to include('QUERY_STRING')
-          expect(last_response.body).not_to include('CONTENT_LENGTH')
-        end
+    describe 'env dump' do
+      it 'works when advanced tools are enabled' do
+        Rack::MiniProfiler.config.enable_advanced_debugging_tools = true
+        get '/html?pp=env'
+
+        expect(last_response.body).to include('QUERY_STRING')
+        expect(last_response.body).to include('CONTENT_LENGTH')
       end
     end
   end