Disable risky memory debugging tools by default (#402)

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

diff --git a/README.md b/README.md
index 56d3bc5..2871f78 100644
--- a/README.md
+++ b/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)
 
diff --git a/lib/mini_profiler/config.rb b/lib/mini_profiler/config.rb
index c1f1022..53211db 100644
--- a/lib/mini_profiler/config.rb
+++ b/lib/mini_profiler/config.rb
@@ -34,9 +34,9 @@ module Rack
             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 @@ module Rack
 
       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
diff --git a/lib/mini_profiler/profiler.rb b/lib/mini_profiler/profiler.rb
index 527c359..2e9d1b7 100644
--- a/lib/mini_profiler/profiler.rb
+++ b/lib/mini_profiler/profiler.rb
@@ -62,6 +62,11 @@ module Rack
         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 @@ module Rack
       @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 @@ module Rack
 
       # 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 @@ module Rack
         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
diff --git a/lib/mini_profiler_rails/railtie.rb b/lib/mini_profiler_rails/railtie.rb
index e9f7b3c..2315643 100644
--- a/lib/mini_profiler_rails/railtie.rb
+++ b/lib/mini_profiler_rails/railtie.rb
@@ -64,6 +64,7 @@ module Rack::MiniProfilerRails
       ::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
 
diff --git a/spec/integration/middleware_spec.rb b/spec/integration/middleware_spec.rb
index 3a87abc..b2d7b35 100644
--- a/spec/integration/middleware_spec.rb
+++ b/spec/integration/middleware_spec.rb
@@ -24,6 +24,21 @@ describe Rack::MiniProfiler do
     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 @@ describe Rack::MiniProfiler do
       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
diff --git a/spec/integration/mini_profiler_spec.rb b/spec/integration/mini_profiler_spec.rb
index b620beb..05d1dc7 100644
--- a/spec/integration/mini_profiler_spec.rb
+++ b/spec/integration/mini_profiler_spec.rb
@@ -288,24 +288,13 @@ describe Rack::MiniProfiler do
       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

GitHub sha: 2b01bb6e

1 Like