protect against inadvertent use of potentially dangerous redis commands

test plan:
 * try Rails.cache.instance_variable_get(:@data).keys, or any other way
   you can think of to try and get all keys (or flushdb or other scary
   stuff) from redis
 * it should raise an error
 * Shackles.activate!(:deploy)
 * try again
 * it should work

Change-Id: I57772df3851fd14b6a46a56c9cd8ef6ddce015e3
Reviewed-on: https://gerrit.instructure.com/73940
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

lib/canvas.rb

@@ -18,6 +18,8 @@ module Canvas
   def self.redis
     raise "Redis is not enabled for this install" unless Canvas.redis_enabled?
     @redis ||= begin
+      Bundler.require 'redis'
+      Canvas::Redis.patch
       settings = ConfigFile.load('redis')
       Canvas::RedisConfig.from_settings(settings).redis
     end

lib/canvas/redis.rb

@@ -71,35 +71,100 @@ module Canvas::Redis
     end
   end
 
-  def self.patch
-    return if @redis_patched
-    Redis::Client.class_eval do
-      def process_with_conn_error(commands, *a, &b)
-        # try to return the type of value the command would expect, for some
-        # specific commands that we know can cause problems if we just return
-        # nil
-        #
-        # this isn't fool-proof, and in some situations it would probably
-        # actually be nicer to raise the exception and let the app handle it,
-        # but it's what we're going with for now
-        #
-        # for instance, Rails.cache.delete_matched will error out if the 'keys' command returns nil instead of []
-        last_command = commands.try(:last)
-        failure_val = case (last_command.respond_to?(:first) ? last_command.first : last_command).to_s
-          when 'keys', 'hmget'
-            []
-          when 'del'
-            0
-          else
-            nil
-        end
+  class UnsupportedRedisMethod < RuntimeError
+  end
 
-        Canvas::Redis.handle_redis_failure(failure_val, self.location) do
-          process_without_conn_error(commands, *a, &b)
-        end
+  module Client
+    def process(commands, *a, &b)
+      # try to return the type of value the command would expect, for some
+      # specific commands that we know can cause problems if we just return
+      # nil
+      #
+      # this isn't fool-proof, and in some situations it would probably
+      # actually be nicer to raise the exception and let the app handle it,
+      # but it's what we're going with for now
+      #
+      # for instance, Rails.cache.delete_matched will error out if the 'keys' command returns nil instead of []
+      last_command = commands.try(:last)
+      failure_val = case (last_command.respond_to?(:first) ? last_command.first : last_command).to_s
+                    when 'keys', 'hmget'
+                      []
+                    when 'del'
+                      0
+                    end
+
+      Canvas::Redis.handle_redis_failure(failure_val, self.location) do
+        super
       end
-      alias_method_chain :process, :conn_error
     end
-    @redis_patched = true
+
+    def write(command)
+      if UNSUPPORTED_METHODS.include?(command.first.to_s)
+        raise(UnsupportedRedisMethod, "Redis method `#{command.first}` is not supported by Twemproxy, and so shouldn't be used in Canvas")
+      end
+      if ALLOWED_UNSUPPORTED.include?(command.first.to_s) && Shackles.environment != :deploy
+        raise(UnsupportedRedisMethod, "Redis method `#{command.first}` is potentially dangerous, and should only be called from console, and only if you fully understand the consequences. If you're sure, retry after running Shackles.activate!(:deploy)")
+      end
+      super
+    end
+
+    UNSUPPORTED_METHODS = %w[
+      migrate
+      move
+      object
+      randomkey
+      rename
+      renamenx
+      scan
+      bitop
+      msetnx
+      blpop
+      brpop
+      brpoplpush
+      psubscribe
+      publish
+      punsubscribe
+      subscribe
+      unsubscribe
+      discard
+      exec
+      multi
+      unwatch
+      watch
+      script
+      echo
+      ping
+    ].freeze
+
+    # There are some methods that are not supported by twemproxy, but which we
+    # don't block, because they are viewed as maintenance-type commands that
+    # wouldn't be run as part of normal code, but could be useful to run
+    # one-off in script/console if you aren't using twemproxy, or in specs:
+    ALLOWED_UNSUPPORTED = %w[
+      keys
+      auth
+      quit
+      flushall
+      flushdb
+      info
+      bgrewriteaof
+      bgsave
+      client
+      config
+      dbsize
+      debug
+      lastsave
+      monitor
+      save
+      shutdown
+      slaveof
+      slowlog
+      sync
+      time
+    ].freeze
+  end
+
+  def self.patch
+    Redis::Client.prepend(Client)
   end
 end

lib/canvas/redis_wrapper.rb

@@ -1,70 +1,5 @@
 module Canvas
   class RedisWrapper < SimpleDelegator
-    UNSUPPORTED_METHODS = %w[
-      keys
-      migrate
-      move
-      object
-      randomkey
-      rename
-      renamenx
-      scan
-      bitop
-      msetnx
-      blpop
-      brpop
-      brpoplpush
-      psubscribe
-      publish
-      punsubscribe
-      subscribe
-      unsubscribe
-      discard
-      exec
-      multi
-      unwatch
-      watch
-      script
-      echo
-      ping
-    ]
-
-    # There are some methods that are not supported by twemproxy, but which we
-    # don't block, because they are viewed as maintenance-type commands that
-    # wouldn't be run as part of normal code, but could be useful to run
-    # one-off in script/console if you aren't using twemproxy, or in specs:
-    ALLOWED_UNSUPPORTED = %w[
-      auth
-      quit
-      select
-      flushall
-      flushdb
-      info
-      bgrewriteaof
-      bgsave
-      client
-      config
-      dbsize
-      debug
-      lastsave
-      monitor
-      save
-      shutdown
-      slaveof
-      slowlog
-      sync
-      time
-    ]
-
-    class UnsupportedRedisMethod < RuntimeError
-    end
-
-    UNSUPPORTED_METHODS.each do |method|
-      define_method(method) { |*a|
-        raise(UnsupportedRedisMethod, "Redis method `#{method}` is not supported by Twemproxy, and so shouldn't be used in Canvas")
-      }
-    end
-
     # We don't marshal for the data wrapper
     def set(key, value, options = nil)
       options ||= {}

spec/lib/canvas/redis_spec.rb

@@ -168,7 +168,7 @@ describe "Canvas::Redis" do
     end
 
     it "should raise on unsupported commands" do
-      expect { Canvas.redis.keys }.to raise_error(Canvas::RedisWrapper::UnsupportedRedisMethod)
+      expect { Canvas.redis.keys }.to raise_error(Canvas::Redis::UnsupportedRedisMethod)
     end
   end
 end

spec/spec_helper.rb

@@ -489,7 +489,8 @@ RSpec.configure do |config|
   end
   config.before :each do
     if Canvas.redis_enabled? && Canvas.redis_used
-      Canvas.redis.flushdb
+      # yes, we really mean to run this dangerous redis command
+      Shackles.activate(:deploy) { Canvas.redis.flushdb }
     end
     Canvas.redis_used = false
   end