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 3 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
53 changes: 53 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,53 @@
# frozen_string_literal: true

module SaferRailsConsole
module Patches
module Sandbox
module RedisReadonly
# rubocop:disable Style/WordArray
mileszim marked this conversation as resolved.
Show resolved Hide resolved
WRITE_COMMANDS = %w{
bitop restore pfdebug lpushx incr move getex decrby renamenx flushdb setex
setnx linsert rpush bzpopmax hset del copy xsetid georadiusbymember setrange blmove set rpop
lset xtrim zrangestore psetex xclaim append georadius incrbyfloat bitfield expire sort hsetnx
sadd zincrby lpop spop sunionstore getdel lrem zunionstore sdiffstore zremrangebyscore ltrim
bzpopmin xack pfadd unlink swapdb sinterstore zrem xgroup hdel zdiffstore xautoclaim xdel hmset
zpopmax zremrangebyrank setbit pexpireat mset hincrbyfloat incrby blpop getset expireat xreadgroup
hincrby migrate lmove pexpire flushall smove msetnx decr persist rpushx pfmerge xadd zremrangebylex
restore-asking geoadd rpoplpush zadd lpush srem brpoplpush zpopmin brpop geosearchstore zinterstore rename
}.freeze
# rubocop:enable Style/WordArray

def self.raise_exception_on_write_command(command)
mileszim marked this conversation as resolved.
Show resolved Hide resolved
if WRITE_COMMANDS.include?(command.to_s)
raise ::Redis::CommandError.new("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.

commands.flatten.each do |command|
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

::Redis::Client.prepend(RedisPatch) if defined?(::Redis::Client)
end
end
end
end
1 change: 1 addition & 0 deletions safer_rails_console.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ 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 'rspec', '~> 3.6'
spec.add_development_dependency 'rspec_junit_formatter'
spec.add_development_dependency 'salsify_rubocop', '~> 1.27.0'
Expand Down
16 changes: 16 additions & 0 deletions spec/integration/patches/sandbox_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,22 @@
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

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

gem 'pg'
gem 'rails', '~> 6.0.0'
gem 'redis', '~> 4.0.0'

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

gem 'pg'
gem 'rails', '~> 6.1.0'
gem 'redis', '~> 4.0.0'

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

gem 'pg'
gem 'rails', '~> 7.0.0'
gem 'redis', '~> 4.0.0'

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