Skip to content

Commit 37f5894

Browse files
justin808claude
andauthored
Security and reliability improvements for CI debugging tools (#2024)
## Summary This PR addresses the security and reliability improvements outlined in issue #1975. ## Changes Made ### High Priority - Security - **Remove eval from bin/ci-rerun-failures** - Replaced `eval` usage with a safe `run_command()` function using case statement - Commands are now explicitly defined in case branches, eliminating potential injection risks - Maintains same functionality while being more maintainable and secure ### Medium Priority - Documentation - **Document Ruby version requirement in bin/ci-switch-config** - Added comment explaining that script/convert uses current Ruby in PATH - Documents that version manager may not have reloaded yet - Specifies Ruby 2.6+ compatibility requirement ### Low Priority - Reliability - **Add bounds check for array access in bin/ci-run-failed-specs** - Added defensive check before accessing UNIQUE_SPECS[0] - Prevents potential errors if array is empty - **Improve git restore error handling in bin/ci-switch-config** - Replaced silent failure (2>/dev/null || true) with explicit warning - Users now see a clear message if files cannot be restored - Differentiates between success and partial failure ## Testing - All scripts validated with `bash -n` for syntax errors - RuboCop linting passed with zero offenses - Code formatted with Prettier Fixes #1975 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- Reviewable:start --> - - - This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/shakacode/react_on_rails/2024) <!-- Reviewable:end --> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3fe5033 commit 37f5894

File tree

3 files changed

+48
-19
lines changed

3 files changed

+48
-19
lines changed

bin/ci-rerun-failures

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,15 +173,41 @@ if [ -z "$FAILED_CHECKS" ]; then
173173
exit 0
174174
fi
175175

176-
# Map CI job names to local commands
176+
# Map CI job names to identifiers
177177
# NOTE: Version numbers below must match .github/workflows/main.yml matrix configuration
178178
declare -A JOB_MAP
179-
JOB_MAP["lint-js-and-ruby"]="bundle exec rubocop && yarn run eslint --report-unused-disable-directives && yarn start format.listDifferent"
180-
JOB_MAP["rspec-package-tests"]="bundle exec rake run_rspec:gem"
181-
JOB_MAP["package-js-tests"]="yarn test"
182-
JOB_MAP["dummy-app-integration-tests (3.4, 22, latest)"]="bundle exec rake run_rspec:all_dummy"
183-
JOB_MAP["dummy-app-integration-tests (3.2, 20, minimum)"]="bundle exec rake run_rspec:all_dummy"
184-
JOB_MAP["examples"]="bundle exec rake run_rspec:shakapacker_examples"
179+
JOB_MAP["lint-js-and-ruby"]="lint-js-and-ruby"
180+
JOB_MAP["rspec-package-tests"]="rspec-package-tests"
181+
JOB_MAP["package-js-tests"]="package-js-tests"
182+
JOB_MAP["dummy-app-integration-tests (3.4, 22, latest)"]="dummy-app-integration-tests"
183+
JOB_MAP["dummy-app-integration-tests (3.2, 20, minimum)"]="dummy-app-integration-tests"
184+
JOB_MAP["examples"]="examples"
185+
186+
# Function to execute commands without eval
187+
run_command() {
188+
local cmd_id="$1"
189+
case "$cmd_id" in
190+
"lint-js-and-ruby")
191+
bundle exec rubocop && yarn run eslint --report-unused-disable-directives && yarn start format.listDifferent
192+
;;
193+
"rspec-package-tests")
194+
bundle exec rake run_rspec:gem
195+
;;
196+
"package-js-tests")
197+
yarn test
198+
;;
199+
"dummy-app-integration-tests")
200+
bundle exec rake run_rspec:all_dummy
201+
;;
202+
"examples")
203+
bundle exec rake run_rspec:shakapacker_examples
204+
;;
205+
*)
206+
echo "Unknown command ID: $cmd_id"
207+
return 1
208+
;;
209+
esac
210+
}
185211

186212
# Map CI job names to human-readable versions (matches SWITCHING_CI_CONFIGS.md)
187213
declare -A JOB_VERSION_MAP
@@ -236,10 +262,10 @@ if [ "$NUM_COMMANDS" -eq 0 ]; then
236262
fi
237263

238264
echo -e "${BLUE}Will run the following commands:${NC}"
239-
for cmd in "${!COMMANDS_TO_RUN[@]}"; do
240-
job_name="${COMMANDS_TO_RUN[$cmd]}"
265+
for cmd_id in "${!COMMANDS_TO_RUN[@]}"; do
266+
job_name="${COMMANDS_TO_RUN[$cmd_id]}"
241267
version_info=$(get_version_info "$job_name")
242-
echo -e "${BLUE}$job_name${version_info}:${NC} $cmd"
268+
echo -e "${BLUE}$job_name${version_info}${NC}"
243269
done
244270
echo ""
245271

@@ -270,17 +296,14 @@ fi
270296
# Run commands
271297
FAILED_COMMANDS=()
272298

273-
for cmd in "${!COMMANDS_TO_RUN[@]}"; do
274-
job_name="${COMMANDS_TO_RUN[$cmd]}"
299+
for cmd_id in "${!COMMANDS_TO_RUN[@]}"; do
300+
job_name="${COMMANDS_TO_RUN[$cmd_id]}"
275301
version_info=$(get_version_info "$job_name")
276302

277303
echo -e "${BLUE}▶ Running: $job_name${version_info}${NC}"
278-
echo -e "${BLUE}Command: $cmd${NC}"
279304
echo ""
280305

281-
# Note: Using eval here is safe because $cmd comes from predefined JOB_MAP,
282-
# not from user input. Commands may contain shell operators like && and ||.
283-
if eval "$cmd"; then
306+
if run_command "$cmd_id"; then
284307
echo -e "${GREEN}$job_name${version_info} passed${NC}"
285308
echo ""
286309
else

bin/ci-run-failed-specs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ echo ""
139139

140140
# Determine the working directory (check if we need to be in spec/dummy)
141141
WORKING_DIR="."
142-
if [[ "${UNIQUE_SPECS[0]}" == *"spec/system"* ]] || [[ "${UNIQUE_SPECS[0]}" == *"spec/helpers"* ]]; then
142+
if [ ${#UNIQUE_SPECS[@]} -gt 0 ] && ([[ "${UNIQUE_SPECS[0]}" == *"spec/system"* ]] || [[ "${UNIQUE_SPECS[0]}" == *"spec/helpers"* ]]); then
143143
if [ -d "spec/dummy" ]; then
144144
WORKING_DIR="spec/dummy"
145145
echo -e "${BLUE}Running from spec/dummy directory${NC}"

bin/ci-switch-config

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ EOF
255255
set_node_version "20.18.1" "$VERSION_MANAGER"
256256

257257
# Run conversion script
258+
# NOTE: This uses whatever 'ruby' is in PATH after version manager updates above.
259+
# The version manager may not have reloaded yet, so ensure your current Ruby is
260+
# compatible with script/convert (Ruby 2.6+ should work).
258261
print_header "Running script/convert to downgrade dependencies"
259262
cd "$PROJECT_ROOT"
260263
ruby script/convert
@@ -395,8 +398,11 @@ EOF
395398
# Restore files from git
396399
print_header "Restoring dependency files from git"
397400
cd "$PROJECT_ROOT"
398-
git restore Gemfile.development_dependencies package.json spec/dummy/package.json packages/react-on-rails-pro/package.json 2>/dev/null || true
399-
print_success "Files restored from git"
401+
if ! git restore Gemfile.development_dependencies package.json spec/dummy/package.json packages/react-on-rails-pro/package.json 2>/dev/null; then
402+
print_warning "Some files could not be restored (may not exist in git)"
403+
else
404+
print_success "Files restored from git"
405+
fi
400406

401407
# Clean and reinstall
402408
print_header "Cleaning node_modules and reinstalling"

0 commit comments

Comments
 (0)