Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: redis sandboxing #50

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
POSTGRES_USER: "circleci"
POSTGRES_DB: "safer_rails_console_test"
POSTGRES_HOST_AUTH_METHOD: "trust"
- image: cimg/redis:6.2.6
working_directory: ~/safer_rails_console
steps:
- checkout
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
[![Build Status](https://circleci.com/gh/salsify/safer_rails_console.svg?style=svg)](https://circleci.com/gh/salsify/safer_rails_console)
[![Gem Version](https://badge.fury.io/rb/safer_rails_console.svg)](https://badge.fury.io/rb/safer_rails_console)

This gem makes Rails console sessions less dangerous in specified environments by warning, color-coding, and auto-sandboxing PostgreSQL connections. In the future we'd like to extend this to make other external connections read-only too (e.g. disable job queueing, non-GET HTTP requests, etc.)
This gem makes Rails console sessions less dangerous in specified environments by warning, color-coding, and auto-sandboxing PostgreSQL and Redis connections. In the future we'd like to extend this to make other external connections read-only too (e.g. disable job queueing, non-GET HTTP requests, etc.)

## Installation

Expand Down
67 changes: 67 additions & 0 deletions lib/safer_rails_console/patches/sandbox/redis_readonly.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# frozen_string_literal: true

module SaferRailsConsole
module Patches
module Sandbox
module RedisReadonly
READ_COMMANDS = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm vaguely worried about this list falling out of date as new commands are added, but I think it's fine since we're whitelisting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a nit, can we alphabetize these?

'hvals', 'bitcount', 'zscan', 'hget', 'smembers', 'hrandfield', 'zrevrange', 'bitpos', 'hlen', 'xlen', 'post',
'zscore', 'dbsize', 'get', 'hstrlen', 'zrangebylex', 'scan', 'georadiusbymember_ro', 'zmscore', 'smismember',
'zcount', 'lrange', 'stralgo', 'zrank', 'pttl', 'lpos', 'geopos', 'ttl', 'zrangebyscore', 'sdiff', 'llen',
'sismember', 'zrevrangebyscore', 'zdiff', 'zrandmember', 'geodist', 'hexists', 'zrange', 'hmget', 'lindex',
'zrevrangebylex', 'sunion', 'randomkey', 'zrevrank', 'xrange', 'xpending', 'hgetall', 'getrange', 'exists',
'keys', 'georadius_ro', 'lolwut', 'hscan', 'object', 'zlexcount', 'type', 'geohash', 'touch', 'hkeys',
'strlen', 'scard', 'substr', 'zinter', 'srandmember', 'mget', 'xinfo', 'geosearch', 'zunion', 'xread',
'pfcount', 'xrevrange', 'sscan', 'memory', 'bitfield_ro', 'dump', 'host:', 'sinter', 'getbit', 'zcard'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is host: correct here?

].freeze

def self.raise_exception_on_write_command(command, service = ::Redis)
unless READ_COMMANDS.include?(command.downcase.to_s)
raise service.to_s.constantize::CommandError.new(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why service.to_s.constantize?

"Write commands are not allowed in readonly mode: #{command}"
)
end
end

def self.handle_and_reraise_exception(error)
if error.message.include?('Write commands are not allowed')
puts SaferRailsConsole::Colors.color_text( # rubocop:disable Rails/Output
'An operation could not be completed due to read-only mode.',
SaferRailsConsole::Colors::RED
)
end

raise error
end

module RedisPatch
def process(commands)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work for pipelined/multi commands? I think this may only catch this first command being executed.

begin
command = commands.flatten.first
SaferRailsConsole::Patches::Sandbox::RedisReadonly.raise_exception_on_write_command(command)
rescue Redis::CommandError => e
SaferRailsConsole::Patches::Sandbox::RedisReadonly.handle_and_reraise_exception(e)
end
super
end
end

module RedisClientPatch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redis-client has great middleware support. It might be easier to implement this via a middleware (in the config object).

def call(commands, redis_config)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise I don't think this covers pipelines or multis?

command = commands.first
begin
SaferRailsConsole::Patches::Sandbox::RedisReadonly.raise_exception_on_write_command(command,
::RedisClient)
rescue RedisClient::CommandError => e
SaferRailsConsole::Patches::Sandbox::RedisReadonly.handle_and_reraise_exception(e)
end
super
end
end

::Redis::Client.prepend(RedisPatch) if defined?(::Redis::Client)
::RedisClient.register(RedisClientPatch) if defined?(::RedisClient)
end
end
end
end
2 changes: 2 additions & 0 deletions safer_rails_console.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'overcommit', '~> 0.39.0'
spec.add_development_dependency 'pg', '~> 1.1'
spec.add_development_dependency 'rake', '~> 12.0'
spec.add_development_dependency 'redis', '~> 4.7'
spec.add_development_dependency 'redis-client', '~> 0.11'
spec.add_development_dependency 'rspec', '~> 3.6'
spec.add_development_dependency 'rspec_junit_formatter'
spec.add_development_dependency 'salsify_rubocop', '~> 1.27.0'
Expand Down
35 changes: 35 additions & 0 deletions spec/integration/patches/sandbox_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,41 @@
end
end

context "redis readonly" do
it "enforces readonly commands" do
# Run a console session that makes some redis changes
run_console_commands('Redis.new.set("test", "value")')

# Run a new console session to ensure the redis changes were not saved
result = run_console_commands('puts "Redis.get(\"test\").nil? = #{Redis.new.get(\'test\').nil?}"') # rubocop:disable Lint/InterpolationCheck Layout/LineLength
expect(result.stdout).to include('Redis.get("test").nil? = true')
end

it "lets the user know that an operation could not be completed" do
result = run_console_commands('Redis.new.set("test", "value")')
expect(result.stdout).to include('An operation could not be completed due to read-only mode.')
end
end

context "redis-client readonly" do
it "enforces readonly commands" do
# Run a console session that makes some redis changes
run_console_commands('RedisClient.new.call("SET", "test", "value")')

# Run a new console session to ensure the redis changes were not saved
result = run_console_commands(
'puts "RedisClient.new.call(\"GET\", \"test\").nil? = ' \
'#{RedisClient.new.call(\'GET\', \'test\').nil?}"' # rubocop:disable Lint/InterpolationCheck
)
expect(result.stdout).to include('RedisClient.new.call("GET", "test").nil? = true')
end

it "lets the user know that an operation could not be completed" do
result = run_console_commands('RedisClient.new.call("SET", "test", "value")')
expect(result.stdout).to include('An operation could not be completed due to read-only mode.')
end
end

def run_console_commands(*commands)
commands += ['exit']
run_console('--sandbox', input: commands.join("\n"))
Expand Down
2 changes: 2 additions & 0 deletions spec/internal/rails_6_0/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ source 'https://rubygems.org'

gem 'pg'
gem 'rails', '~> 6.0.0'
gem 'redis', '~> 4.0.0'
gem 'redis-client', '~> 0.11'

gem 'safer_rails_console', path: '../../../'
2 changes: 2 additions & 0 deletions spec/internal/rails_6_1/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ source 'https://rubygems.org'

gem 'pg'
gem 'rails', '~> 6.1.0'
gem 'redis', '~> 4.0.0'
gem 'redis-client', '~> 0.11'

gem 'safer_rails_console', path: '../../../'
2 changes: 2 additions & 0 deletions spec/internal/rails_7_0/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ source 'https://rubygems.org'

gem 'pg'
gem 'rails', '~> 7.0.0'
gem 'redis', '~> 4.0.0'
gem 'redis-client', '~> 0.11'

gem 'safer_rails_console', path: '../../../'