Skip to content

Commit a5e32cb

Browse files
justin808claude
andcommitted
Improve code quality and security in ServiceChecker
Code improvements: - Extract configuration keys as CONFIG_KEYS constant for maintainability - Use constants throughout instead of magic strings - More specific exception handling (Errno::ENOENT vs StandardError) - Add debug logging for unexpected errors (enabled with DEBUG env var) Security improvements: - Enhanced security warnings in .dev-services.yml.example files - Added best practices section about avoiding shell metacharacters - Documented risks of complex shell scripts and chained commands - Emphasized keeping commands simple and focused All tests passing (14/14). Zero RuboCop offenses. Addresses feedback from PR review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent c02505f commit a5e32cb

File tree

3 files changed

+41
-13
lines changed

3 files changed

+41
-13
lines changed

lib/generators/react_on_rails/templates/base/base/.dev-services.yml.example

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
# sensitive information or custom paths specific to your machine. Consider
1414
# adding .dev-services.yml to .gitignore if it contains machine-specific config.
1515
#
16+
# Security best practices:
17+
# - Use simple commands without shell metacharacters when possible
18+
# - Avoid complex shell scripts or chained commands (&&, ||, |, etc.)
19+
# - Only include services you trust
20+
# - Keep commands simple and focused on service health checks
21+
#
1622
# Example configuration:
1723
#
1824
# services:

lib/react_on_rails/dev/service_checker.rb

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,19 @@ module Dev
2424
# description: "PostgreSQL database"
2525
#
2626
class ServiceChecker
27+
# Configuration file keys
28+
CONFIG_KEYS = {
29+
services: "services",
30+
check_command: "check_command",
31+
expected_output: "expected_output",
32+
start_command: "start_command",
33+
install_hint: "install_hint",
34+
description: "description"
35+
}.freeze
36+
2737
class << self
2838
# Check all required services and provide helpful output
39+
#
2940
# @param config_path [String] Path to .dev-services.yml (default: ./.dev-services.yml)
3041
# @return [Boolean] true if all services are running or no config exists
3142
def check_services(config_path: ".dev-services.yml")
@@ -40,13 +51,13 @@ def check_services(config_path: ".dev-services.yml")
4051
private
4152

4253
def config_has_services?(config)
43-
config && config["services"] && !config["services"].empty?
54+
config && config[CONFIG_KEYS[:services]] && !config[CONFIG_KEYS[:services]].empty?
4455
end
4556

4657
def check_and_report_services(config, config_path)
4758
print_services_header(config_path)
4859

49-
failures = collect_service_failures(config["services"])
60+
failures = collect_service_failures(config[CONFIG_KEYS[:services]])
5061

5162
report_results(failures)
5263
end
@@ -56,10 +67,10 @@ def collect_service_failures(services)
5667

5768
services.each do |name, service_config|
5869
if check_service(name, service_config)
59-
print_service_ok(name, service_config["description"])
70+
print_service_ok(name, service_config[CONFIG_KEYS[:description]])
6071
else
6172
failures << { name: name, config: service_config }
62-
print_service_failed(name, service_config["description"])
73+
print_service_failed(name, service_config[CONFIG_KEYS[:description]])
6374
end
6475
end
6576

@@ -86,8 +97,8 @@ def load_config(config_path)
8697
end
8798

8899
def check_service(_name, config)
89-
check_command = config["check_command"]
90-
expected_output = config["expected_output"]
100+
check_command = config[CONFIG_KEYS[:check_command]]
101+
expected_output = config[CONFIG_KEYS[:expected_output]]
91102

92103
return false if check_command.nil?
93104

@@ -105,7 +116,12 @@ def run_check_command(command)
105116
stdout, stderr, status = Open3.capture3(command, err: %i[child out])
106117
output = stdout + stderr
107118
[output, status]
108-
rescue StandardError
119+
rescue Errno::ENOENT
120+
# Command not found - service is not available
121+
["", nil]
122+
rescue StandardError => e
123+
# Log unexpected errors for debugging
124+
warn "Unexpected error checking service: #{e.message}" if ENV["DEBUG"]
109125
["", nil]
110126
end
111127

@@ -142,20 +158,20 @@ def print_failures_summary(failures)
142158
failures.each do |failure|
143159
name = failure[:name]
144160
config = failure[:config]
145-
description = config["description"] || name
161+
description = config[CONFIG_KEYS[:description]] || name
146162

147163
puts Rainbow(name.to_s).cyan.bold
148-
puts " #{description}" if config["description"]
164+
puts " #{description}" if config[CONFIG_KEYS[:description]]
149165

150-
if config["start_command"]
166+
if config[CONFIG_KEYS[:start_command]]
151167
puts ""
152168
puts " #{Rainbow('To start:').yellow}"
153-
puts " #{Rainbow(config['start_command']).green}"
169+
puts " #{Rainbow(config[CONFIG_KEYS[:start_command]]).green}"
154170
end
155171

156-
if config["install_hint"]
172+
if config[CONFIG_KEYS[:install_hint]]
157173
puts ""
158-
puts " #{Rainbow('Not installed?').yellow} #{config['install_hint']}"
174+
puts " #{Rainbow('Not installed?').yellow} #{config[CONFIG_KEYS[:install_hint]]}"
159175
end
160176

161177
puts ""

spec/dummy/.dev-services.yml.example

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
# sensitive information or custom paths specific to your machine. Consider
1414
# adding .dev-services.yml to .gitignore if it contains machine-specific config.
1515
#
16+
# Security best practices:
17+
# - Use simple commands without shell metacharacters when possible
18+
# - Avoid complex shell scripts or chained commands (&&, ||, |, etc.)
19+
# - Only include services you trust
20+
# - Keep commands simple and focused on service health checks
21+
#
1622
# For the React on Rails dummy app, we typically don't require external services,
1723
# but here are some common examples you might use in a real application:
1824

0 commit comments

Comments
 (0)