Skip to content

Commit 5f78e6f

Browse files
justin808claude
andcommitted
Enhance security documentation and nil safety
Security improvements: - Clarify that Open3.capture3 doesn't invoke shell for simple commands - Update all security warnings to explain shell metacharacters won't work - Document recommended .gitignore approach (commit .example, gitignore actual) - Add execution order documentation (services → hook → Procfile) - Enhanced security note in docs with best practices Code robustness: - Add safe navigation operator for nil check: output&.include?(expected_output) - Prevents NoMethodError if output is nil - Add ArgumentError rescue for invalid command formats - Improved inline documentation about command execution Documentation improvements: - Expanded security note with IMPORTANT callout - Added recommended approach section - Added execution order section - Clarified that shell features will fail (not just "avoid") All tests passing (14/14). Zero RuboCop offenses. Addresses security and robustness feedback from PR review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 0f54088 commit 5f78e6f

File tree

4 files changed

+35
-6
lines changed

4 files changed

+35
-6
lines changed

docs/building-features/process-managers.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,24 @@ redis
144144
145145
#### Security Note
146146
147-
⚠️ Commands in `.dev-services.yml` are executed during `bin/dev` startup. Only add commands from trusted sources. Consider adding `.dev-services.yml` to `.gitignore` if it contains machine-specific paths or sensitive information.
147+
⚠️ **IMPORTANT**: Commands in `.dev-services.yml` are executed during `bin/dev` startup without shell expansion for safety. However, you should still:
148+
149+
- **Only add commands from trusted sources**
150+
- **Avoid shell metacharacters** (&&, ||, ;, |, $, etc.) - they won't work and indicate an anti-pattern
151+
- **Review changes carefully** if .dev-services.yml is committed to version control
152+
- **Consider adding to .gitignore** if it contains machine-specific paths or sensitive information
153+
154+
**Recommended approach:**
155+
156+
- Commit `.dev-services.yml.example` to version control (safe, documentation)
157+
- Add `.dev-services.yml` to `.gitignore` (developers copy from example)
158+
- This prevents accidental execution of untrusted commands from compromised dependencies
159+
160+
**Execution order:**
161+
162+
1. Service dependency checks (`.dev-services.yml`)
163+
2. Precompile hook (if configured in `config/shakapacker.yml`)
164+
3. Process manager starts processes from Procfile
148165
149166
## Installing a Process Manager
150167

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
# adding .dev-services.yml to .gitignore if it contains machine-specific config.
1515
#
1616
# Security best practices:
17-
# - Use simple commands without shell metacharacters when possible
18-
# - Avoid complex shell scripts or chained commands (&&, ||, |, etc.)
17+
# - Commands are executed without shell expansion (shell metacharacters won't work)
18+
# - Use simple, single commands (e.g., "redis-cli ping", "pg_isready")
19+
# - Do NOT use shell features: &&, ||, |, $, ;, backticks, etc. will fail
1920
# - Only include services you trust
2021
# - Keep commands simple and focused on service health checks
22+
# - Consider adding .dev-services.yml to .gitignore (commit .example instead)
2123
#
2224
# Example configuration:
2325
#

lib/react_on_rails/dev/service_checker.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,25 @@ def check_service(_name, config)
110110

111111
return status.success? if expected_output.nil?
112112

113-
status.success? && output.include?(expected_output)
113+
# Safe nil check for output before calling include?
114+
status.success? && output&.include?(expected_output)
114115
end
115116

116117
def run_check_command(command)
117118
require "open3"
119+
# Execute command as-is. Commands are from local .dev-services.yml config file
120+
# which should be trusted. Shell metacharacters won't work as expected since
121+
# Open3.capture3 doesn't invoke a shell by default for simple command strings.
118122
stdout, stderr, status = Open3.capture3(command, err: %i[child out])
119123
output = stdout + stderr
120124
[output, status]
121125
rescue Errno::ENOENT
122126
# Command not found - service is not available
123127
["", nil]
128+
rescue ArgumentError => e
129+
# Invalid command format
130+
warn "Invalid command format: #{e.message}" if ENV["DEBUG"]
131+
["", nil]
124132
rescue StandardError => e
125133
# Log unexpected errors for debugging
126134
warn "Unexpected error checking service: #{e.message}" if ENV["DEBUG"]

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
# adding .dev-services.yml to .gitignore if it contains machine-specific config.
1515
#
1616
# Security best practices:
17-
# - Use simple commands without shell metacharacters when possible
18-
# - Avoid complex shell scripts or chained commands (&&, ||, |, etc.)
17+
# - Commands are executed without shell expansion (shell metacharacters won't work)
18+
# - Use simple, single commands (e.g., "redis-cli ping", "pg_isready")
19+
# - Do NOT use shell features: &&, ||, |, $, ;, backticks, etc. will fail
1920
# - Only include services you trust
2021
# - Keep commands simple and focused on service health checks
22+
# - Consider adding .dev-services.yml to .gitignore (commit .example instead)
2123
#
2224
# For the React on Rails dummy app, we typically don't require external services,
2325
# but here are some common examples you might use in a real application:

0 commit comments

Comments
 (0)