Skip to content

Commit 25f5be6

Browse files
committed
Address security and portability issues in CI scripts
1. Security: Remove eval from bin/ci-run-failed-specs - Use bash array instead of string for RSPEC_CMD - Execute command directly with "${RSPEC_CMD[@]}" (safer) - Eliminates command injection risk from spec paths 2. Security: Document safe eval usage in bin/ci-rerun-failures - Add comment explaining eval is safe (commands from predefined JOB_MAP) - Fix REPLY variable to use explicit naming 3. Error handling: Add dependency checks - bin/ci-rerun-failures: Check for gh and jq - bin/ci-run-failed-specs: Check for bundle - Provide clear error messages with install instructions 4. Portability: Document cross-platform clipboard alternatives - Add Linux alternatives (xclip, wl-clipboard) in CLAUDE.md - Maintain pbpaste as primary example for macOS 5. Consistency: Fix REPLY variable handling - Explicitly name REPLY in all read commands - Use ${REPLY} with quotes for -u compatibility All scripts now have consistent error handling, better security, and clearer documentation for cross-platform usage.
1 parent efdd6fd commit 25f5be6

File tree

3 files changed

+52
-12
lines changed

3 files changed

+52
-12
lines changed

CLAUDE.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ When RSpec tests fail, run just those specific examples:
100100

101101
```bash
102102
# Copy failure output from GitHub Actions, then:
103-
pbpaste | bin/ci-run-failed-specs
103+
pbpaste | bin/ci-run-failed-specs # macOS
104+
# xclip -o | bin/ci-run-failed-specs # Linux (requires: apt install xclip)
105+
# wl-paste | bin/ci-run-failed-specs # Wayland (requires: apt install wl-clipboard)
104106

105107
# Or pass spec paths directly:
106108
bin/ci-run-failed-specs './spec/system/integration_spec.rb[1:1:1:1]'

bin/ci-rerun-failures

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,22 @@ done
7171
echo -e "${BLUE}=== CI Failure Re-runner ===${NC}"
7272
echo ""
7373

74+
# Check required dependencies
75+
MISSING_DEPS=()
76+
command -v gh >/dev/null 2>&1 || MISSING_DEPS+=("gh (GitHub CLI)")
77+
command -v jq >/dev/null 2>&1 || MISSING_DEPS+=("jq")
78+
79+
if [ ${#MISSING_DEPS[@]} -gt 0 ]; then
80+
echo -e "${RED}Error: Missing required dependencies:${NC}"
81+
for dep in "${MISSING_DEPS[@]}"; do
82+
echo " - $dep"
83+
done
84+
echo ""
85+
echo "Install with:"
86+
echo " brew install gh jq"
87+
exit 1
88+
fi
89+
7490
# Fetch PR info
7591
if [ -z "$PR_NUMBER" ]; then
7692
if ! gh pr view --json number,commits >/dev/null 2>&1; then
@@ -214,9 +230,9 @@ echo " 3. Run: pbpaste | bin/ci-run-failed-specs"
214230
echo ""
215231

216232
# Confirm before running
217-
read -p "Run these tests now? [Y/n] " -n 1 -r
233+
read -p "Run these tests now? [Y/n] " -n 1 -r REPLY
218234
echo
219-
if [[ ! $REPLY =~ ^[Yy]$ ]] && [[ ! -z $REPLY ]]; then
235+
if [[ ! "${REPLY}" =~ ^[Yy]$ ]] && [[ ! -z "${REPLY}" ]]; then
220236
echo "Cancelled."
221237
exit 0
222238
fi
@@ -239,6 +255,8 @@ for cmd in "${!COMMANDS_TO_RUN[@]}"; do
239255
echo -e "${BLUE}Command: $cmd${NC}"
240256
echo ""
241257

258+
# Note: Using eval here is safe because $cmd comes from predefined JOB_MAP,
259+
# not from user input. Commands may contain shell operators like && and ||.
242260
if eval "$cmd"; then
243261
echo -e "${GREEN}$job_name passed${NC}"
244262
echo ""

bin/ci-run-failed-specs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,21 @@ YELLOW='\033[1;33m'
1818
BLUE='\033[0;34m'
1919
NC='\033[0m' # No Color
2020

21+
# Check required dependencies
22+
MISSING_DEPS=()
23+
command -v bundle >/dev/null 2>&1 || MISSING_DEPS+=("bundle (Ruby bundler)")
24+
25+
if [ ${#MISSING_DEPS[@]} -gt 0 ]; then
26+
echo -e "${RED}Error: Missing required dependencies:${NC}"
27+
for dep in "${MISSING_DEPS[@]}"; do
28+
echo " - $dep"
29+
done
30+
echo ""
31+
echo "Install with:"
32+
echo " gem install bundler"
33+
exit 1
34+
fi
35+
2136
# Show help
2237
show_help() {
2338
cat << EOF
@@ -122,30 +137,35 @@ if [[ "${UNIQUE_SPECS[0]}" == *"spec/system"* ]] || [[ "${UNIQUE_SPECS[0]}" == *
122137
fi
123138
fi
124139

125-
# Build rspec command
126-
RSPEC_CMD="bundle exec rspec"
140+
# Build rspec command as array (safer than eval)
141+
RSPEC_CMD=("bundle" "exec" "rspec")
127142
for spec in "${UNIQUE_SPECS[@]}"; do
128-
RSPEC_CMD="$RSPEC_CMD '$spec'"
143+
RSPEC_CMD+=("$spec")
129144
done
130145

131-
echo -e "${BLUE}Command:${NC} cd $WORKING_DIR && $RSPEC_CMD"
146+
# Display command for user
147+
DISPLAY_CMD="bundle exec rspec"
148+
for spec in "${UNIQUE_SPECS[@]}"; do
149+
DISPLAY_CMD="$DISPLAY_CMD '$spec'"
150+
done
151+
echo -e "${BLUE}Command:${NC} cd $WORKING_DIR && $DISPLAY_CMD"
132152
echo ""
133153

134154
# Confirm (read from /dev/tty to handle piped input)
135155
if [ -t 0 ]; then
136-
read -p "Run these specs now? [Y/n] " -n 1 -r
156+
read -p "Run these specs now? [Y/n] " -n 1 -r REPLY
137157
else
138-
read -p "Run these specs now? [Y/n] " -n 1 -r < /dev/tty
158+
read -p "Run these specs now? [Y/n] " -n 1 -r REPLY < /dev/tty
139159
fi
140160
echo
141-
if [[ ! $REPLY =~ ^[Yy]$ ]] && [[ ! -z $REPLY ]]; then
161+
if [[ ! "${REPLY}" =~ ^[Yy]$ ]] && [[ ! -z "${REPLY}" ]]; then
142162
echo "Cancelled."
143163
exit 0
144164
fi
145165

146-
# Run the specs
166+
# Run the specs directly (no eval - safer)
147167
cd "$WORKING_DIR"
148-
eval "$RSPEC_CMD"
168+
"${RSPEC_CMD[@]}"
149169
RESULT=$?
150170

151171
echo ""

0 commit comments

Comments
 (0)