Skip to content

Commit 1db6101

Browse files
justin808claude
andcommitted
Fix security and stability issues from PR review
- Add require 'fileutils' to server.rb - Refactor shell command execution to prevent injection attacks - Use argument arrays instead of string interpolation - Pass commands as arrays to spawn() and system() - Improve database state reset safety - Move after_state_reset hook to run after cleanup - Add support for disable_referential_integrity when available - Use proper table name quoting with quote_table_name - Use File.expand_path for folder detection These changes address security concerns about command injection and improve compatibility across different Rails environments. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1790254 commit 1db6101

File tree

2 files changed

+44
-30
lines changed

2 files changed

+44
-30
lines changed

lib/cypress_on_rails/server.rb

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
require 'socket'
22
require 'timeout'
3+
require 'fileutils'
34
require 'cypress_on_rails/configuration'
45

56
module CypressOnRails
@@ -25,7 +26,7 @@ def open
2526

2627
def run
2728
start_server do
28-
result = run_command(run_command_str, "Running #{framework} tests")
29+
result = run_command(run_command_args, "Running #{framework} tests")
2930
exit(result ? 0 : 1)
3031
end
3132
end
@@ -40,7 +41,7 @@ def init
4041
def detect_install_folder
4142
# Check common locations for cypress/playwright installation
4243
possible_folders = ['e2e', 'spec/e2e', 'spec/cypress', 'spec/playwright', 'cypress', 'playwright']
43-
folder = possible_folders.find { |f| File.exist?(f) }
44+
folder = possible_folders.find { |f| File.exist?(File.expand_path(f)) }
4445
folder || 'e2e'
4546
end
4647

@@ -94,17 +95,17 @@ def start_server(&block)
9495
end
9596

9697
def spawn_server
97-
rails_command = if File.exist?('bin/rails')
98-
'bin/rails'
98+
rails_args = if File.exist?('bin/rails')
99+
['bin/rails']
99100
else
100-
'bundle exec rails'
101+
['bundle', 'exec', 'rails']
101102
end
102103

103-
server_command = "#{rails_command} server -p #{port} -b #{host}"
104+
server_args = rails_args + ['server', '-p', port.to_s, '-b', host]
104105

105-
puts "Starting Rails server: #{server_command}"
106+
puts "Starting Rails server: #{server_args.join(' ')}"
106107

107-
spawn(server_command, out: $stdout, err: $stderr)
108+
spawn(*server_args, out: $stdout, err: $stderr)
108109
end
109110

110111
def wait_for_server(timeout = 30)
@@ -140,47 +141,47 @@ def open_command
140141
case framework
141142
when :cypress
142143
if command_exists?('yarn')
143-
"yarn cypress open --project #{install_folder} --config baseUrl=#{base_url}"
144+
['yarn', 'cypress', 'open', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
144145
elsif command_exists?('npx')
145-
"npx cypress open --project #{install_folder} --config baseUrl=#{base_url}"
146+
['npx', 'cypress', 'open', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
146147
else
147-
"cypress open --project #{install_folder} --config baseUrl=#{base_url}"
148+
['cypress', 'open', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
148149
end
149150
when :playwright
150151
if command_exists?('yarn')
151-
"yarn playwright test --ui"
152+
['yarn', 'playwright', 'test', '--ui']
152153
elsif command_exists?('npx')
153-
"npx playwright test --ui"
154+
['npx', 'playwright', 'test', '--ui']
154155
else
155-
"playwright test --ui"
156+
['playwright', 'test', '--ui']
156157
end
157158
end
158159
end
159160

160-
def run_command_str
161+
def run_command_args
161162
case framework
162163
when :cypress
163164
if command_exists?('yarn')
164-
"yarn cypress run --project #{install_folder} --config baseUrl=#{base_url}"
165+
['yarn', 'cypress', 'run', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
165166
elsif command_exists?('npx')
166-
"npx cypress run --project #{install_folder} --config baseUrl=#{base_url}"
167+
['npx', 'cypress', 'run', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
167168
else
168-
"cypress run --project #{install_folder} --config baseUrl=#{base_url}"
169+
['cypress', 'run', '--project', install_folder, '--config', "baseUrl=#{base_url}"]
169170
end
170171
when :playwright
171172
if command_exists?('yarn')
172-
"yarn playwright test"
173+
['yarn', 'playwright', 'test']
173174
elsif command_exists?('npx')
174-
"npx playwright test"
175+
['npx', 'playwright', 'test']
175176
else
176-
"playwright test"
177+
['playwright', 'test']
177178
end
178179
end
179180
end
180181

181-
def run_command(command, description)
182-
puts "#{description}: #{command}"
183-
system(command)
182+
def run_command(command_args, description)
183+
puts "#{description}: #{command_args.join(' ')}"
184+
system(*command_args)
184185
end
185186

186187
def command_exists?(command)

lib/cypress_on_rails/state_reset_middleware.rb

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,26 @@ def call(env)
1818
def reset_application_state
1919
config = CypressOnRails.configuration
2020

21-
# Run after_state_reset hook if configured
22-
run_hook(config.after_state_reset)
23-
2421
# Default state reset actions
2522
if defined?(DatabaseCleaner)
2623
DatabaseCleaner.clean_with(:truncation)
2724
elsif defined?(ActiveRecord::Base)
28-
ActiveRecord::Base.connection.tables.each do |table|
29-
next if table == 'schema_migrations' || table == 'ar_internal_metadata'
30-
ActiveRecord::Base.connection.execute("DELETE FROM #{table}")
25+
connection = ActiveRecord::Base.connection
26+
27+
# Use disable_referential_integrity if available for safer table clearing
28+
if connection.respond_to?(:disable_referential_integrity)
29+
connection.disable_referential_integrity do
30+
connection.tables.each do |table|
31+
next if table == 'schema_migrations' || table == 'ar_internal_metadata'
32+
connection.execute("DELETE FROM #{connection.quote_table_name(table)}")
33+
end
34+
end
35+
else
36+
# Fallback to regular deletion with proper table name quoting
37+
connection.tables.each do |table|
38+
next if table == 'schema_migrations' || table == 'ar_internal_metadata'
39+
connection.execute("DELETE FROM #{connection.quote_table_name(table)}")
40+
end
3141
end
3242
end
3343

@@ -36,6 +46,9 @@ def reset_application_state
3646

3747
# Reset any class-level state
3848
ActiveSupport::Dependencies.clear if defined?(ActiveSupport::Dependencies)
49+
50+
# Run after_state_reset hook after cleanup is complete
51+
run_hook(config.after_state_reset)
3952
end
4053

4154
def run_hook(hook)

0 commit comments

Comments
 (0)