Skip to content

Commit 690f7f2

Browse files
justin808claude
andcommitted
Address code quality and security issues
Security Fixes: - Replace backticks with Open3.capture2 in detect_default_branch - Eliminates command injection risk while maintaining safety - Follows existing codebase pattern (already using Open3) Code Quality Improvements: - Document nil ref behavior in process_gem_value - Refactor --github option to use shared parse_github_spec_without_shorthand - Eliminates code duplication between --github and gem-specific flags - Add detailed comment explaining network call during validation Design Notes: - Auto-detection during validation is intentional for fail-fast behavior - git ls-remote is lightweight and happens once per unique repo - Repo validation occurs before any network calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent b978345 commit 690f7f2

File tree

2 files changed

+36
-27
lines changed

2 files changed

+36
-27
lines changed

lib/demo_scripts/gem_swapper.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -627,13 +627,21 @@ def validate_github_repos(repos)
627627
end
628628
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength
629629

630+
# Auto-detect the default branch for a GitHub repository
631+
# Called during initialization/validation phase for repos without explicit branch
632+
# Note: This makes a network call during object construction, but it's intentional because:
633+
# 1. The call is fast (git ls-remote with --symref is lightweight)
634+
# 2. The repo is already validated before calling this
635+
# 3. It happens once per unique repo, not per demo
636+
# 4. Failing early during validation is better than failing later during clone
630637
def detect_default_branch(repo)
631638
puts " 🔍 Detecting default branch for #{repo}..."
632639

633640
# Use git ls-remote to find the default branch without cloning
634-
output = `git ls-remote --symref https://github.com/#{repo}.git HEAD 2>/dev/null`
641+
# Use array form to avoid shell injection (repo is already validated by validate_github_repo)
642+
output, status = Open3.capture2('git', 'ls-remote', '--symref', "https://github.com/#{repo}.git", 'HEAD')
635643

636-
if $CHILD_STATUS.success? && output =~ %r{ref: refs/heads/(\S+)\s+HEAD}
644+
if status.success? && output =~ %r{ref: refs/heads/(\S+)\s+HEAD}
637645
branch = ::Regexp.last_match(1)
638646
puts " Detected: #{branch}"
639647
branch

lib/demo_scripts/swap_deps_cli.rb

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ def process_gem_value(gem_name, value)
9393
elsif value.start_with?('#', '@') || value.include?('/')
9494
# It's a GitHub spec
9595
repo, ref, ref_type = parse_github_spec(gem_name, value)
96+
# NOTE: ref can be nil for auto-detection; validate_github_repos will detect default branch later
9697
@github_repos[gem_name] = { repo: repo, branch: ref, ref_type: ref_type }
9798
else
9899
# Default to local path (for simple names without paths or GitHub markers)
@@ -101,7 +102,27 @@ def process_gem_value(gem_name, value)
101102
end
102103
# rubocop:enable Lint/DuplicateBranch
103104

104-
# rubocop:disable Metrics/MethodLength
105+
# Parse GitHub spec without shorthand expansion (for --github flag)
106+
# Returns: [repo, ref, ref_type]
107+
def parse_github_spec_without_shorthand(spec)
108+
if spec.include?('@')
109+
# Full: user/repo@tag
110+
repo, ref = spec.split('@', 2)
111+
ref_type = :tag
112+
elsif spec.include?('#')
113+
# Full: user/repo#branch
114+
repo, ref = spec.split('#', 2)
115+
ref_type = :branch
116+
else
117+
# Just repo name: user/repo
118+
repo = spec
119+
ref = nil # Will auto-detect default branch
120+
ref_type = :branch
121+
end
122+
123+
[repo, ref, ref_type]
124+
end
125+
105126
def parse_github_spec(gem_name, spec)
106127
# Handle shorthand formats:
107128
# - #branch -> shakacode/gem#branch
@@ -124,24 +145,13 @@ def parse_github_spec(gem_name, spec)
124145

125146
ref = spec[1..]
126147
ref_type = :tag
127-
elsif spec.include?('@')
128-
# Full: user/repo@tag
129-
repo, ref = spec.split('@', 2)
130-
ref_type = :tag
131-
elsif spec.include?('#')
132-
# Full: user/repo#branch
133-
repo, ref = spec.split('#', 2)
134-
ref_type = :branch
135148
else
136-
# Just repo name: user/repo
137-
repo = spec
138-
ref = nil # Will auto-detect default branch
139-
ref_type = :branch
149+
# Delegate to shared parser for full specs (user/repo, user/repo#branch, user/repo@tag)
150+
repo, ref, ref_type = parse_github_spec_without_shorthand(spec)
140151
end
141152

142153
[repo, ref, ref_type]
143154
end
144-
# rubocop:enable Metrics/MethodLength
145155

146156
def detect_context!
147157
# Check if we're in a demo directory by looking for Gemfile and presence of ../../.swap-deps.yml
@@ -194,17 +204,8 @@ def parse_options!
194204
opts.on('--github REPO[#BRANCH|@TAG]',
195205
'GitHub repository (e.g., user/repo, user/repo#branch, or user/repo@tag)',
196206
'Note: In zsh, quote values with # or @ (e.g., \'user/repo#main\')') do |value|
197-
if value.include?('@')
198-
repo, ref = value.split('@', 2)
199-
ref_type = :tag
200-
elsif value.include?('#')
201-
repo, ref = value.split('#', 2)
202-
ref_type = :branch
203-
else
204-
repo = value
205-
ref = nil # Auto-detect default branch
206-
ref_type = :branch
207-
end
207+
# Parse GitHub spec and infer gem name from repo
208+
repo, ref, ref_type = parse_github_spec_without_shorthand(value)
208209
gem_name = infer_gem_from_repo(repo)
209210
@github_repos[gem_name] = { repo: repo, branch: ref, ref_type: ref_type }
210211
end

0 commit comments

Comments
 (0)