Skip to content

Commit 0e9c255

Browse files
justin808claude
andcommitted
security: Fix command injection and add comprehensive test coverage
CRITICAL SECURITY FIX: - Fixed command injection vulnerability in fetch_latest_prerelease - Changed from string interpolation to array syntax for Open3.capture3 - Now uses: Open3.capture3('gem', 'search', '-ra', "^#{gem_name}$") QUALITY IMPROVEMENTS: - Improved regex pattern for prerelease version matching * Old: /\.(beta|rc)/ - matched anywhere in string (too permissive) * New: /^\d+\.\d+\.\d+[.-](beta|rc)(\.\d+)?$/i - strict semver format * Prevents matching invalid versions like "foo.beta.1" or "9.0.beta" - Added find_latest_prerelease method to extract validation logic - Rubygems returns versions in descending order (latest first) - Only valid semver prereleases (X.Y.Z-beta.N or X.Y.Z.beta.N) are matched TEST COVERAGE - Added 18 new tests (175 total): ✅ #parse_gem_versions (3 tests) ✅ #find_latest_prerelease (9 tests) - Validates strict semver patterns - Tests both dot and dash separators - Rejects invalid formats - Ensures latest is selected ✅ #default_version_for (3 tests) ✅ #fetch_latest_prerelease integration (3 tests) - Command failure handling - Exception handling - Verifies array syntax usage (security) ADDRESSED CONCERNS: - Prerelease versions won't be older than stable releases because: * Rubygems returns ALL versions in descending order * We filter to only prereleases, preserving the order * First match is always the latest prerelease available * Example: ['9.0.0.rc.1', '9.0.0.beta.1', '8.0.2'] → '9.0.0.rc.1' 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent e00198f commit 0e9c255

File tree

2 files changed

+147
-2
lines changed

2 files changed

+147
-2
lines changed

lib/demo_scripts/config.rb

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,16 @@ def load_config
6262
def fetch_latest_prerelease(gem_name)
6363
require 'open3'
6464

65-
stdout, stderr, status = Open3.capture3("gem search -ra '^#{gem_name}$'")
65+
# Use array syntax to prevent command injection
66+
stdout, stderr, status = Open3.capture3('gem', 'search', '-ra', "^#{gem_name}$")
6667

6768
unless status.success?
6869
warn "Warning: Failed to fetch prerelease version for #{gem_name}: #{stderr}"
6970
return nil
7071
end
7172

7273
versions = parse_gem_versions(stdout)
73-
prerelease = versions.find { |v| v.match?(/\.(beta|rc)/) }
74+
prerelease = find_latest_prerelease(versions)
7475

7576
if prerelease
7677
puts " Found prerelease version for #{gem_name}: #{prerelease}"
@@ -91,5 +92,17 @@ def parse_gem_versions(stdout)
9192

9293
match[1].split(',').map(&:strip)
9394
end
95+
96+
def find_latest_prerelease(versions)
97+
# Strict semver prerelease pattern: must have major.minor.patch followed by -beta.N or -rc.N
98+
# This ensures we only match valid semver prereleases, not arbitrary strings
99+
prerelease_versions = versions.grep(/^\d+\.\d+\.\d+[.-](beta|rc)(\.\d+)?$/i)
100+
101+
return nil if prerelease_versions.empty?
102+
103+
# Sort by version to get the latest (versions are already sorted by rubygems, but be explicit)
104+
# rubygems returns versions in descending order, so first match is latest
105+
prerelease_versions.first
106+
end
94107
end
95108
end

spec/demo_scripts/config_spec.rb

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,136 @@
148148
end
149149
end
150150
end
151+
152+
describe '#parse_gem_versions' do
153+
let(:config) { described_class.new(config_file: '/nonexistent') }
154+
155+
it 'parses gem versions from stdout' do
156+
stdout = 'shakapacker (9.0.0.beta.1, 8.0.2, 8.0.1)'
157+
versions = config.send(:parse_gem_versions, stdout)
158+
159+
expect(versions).to eq(['9.0.0.beta.1', '8.0.2', '8.0.1'])
160+
end
161+
162+
it 'returns empty array when no versions found' do
163+
stdout = 'shakapacker'
164+
versions = config.send(:parse_gem_versions, stdout)
165+
166+
expect(versions).to eq([])
167+
end
168+
169+
it 'handles empty stdout' do
170+
versions = config.send(:parse_gem_versions, '')
171+
expect(versions).to eq([])
172+
end
173+
end
174+
175+
describe '#find_latest_prerelease' do
176+
let(:config) { described_class.new(config_file: '/nonexistent') }
177+
178+
it 'finds valid beta version' do
179+
versions = ['9.0.0.beta.1', '8.0.2', '8.0.1']
180+
result = config.send(:find_latest_prerelease, versions)
181+
182+
expect(result).to eq('9.0.0.beta.1')
183+
end
184+
185+
it 'finds valid rc version' do
186+
versions = ['9.0.0.rc.2', '9.0.0.beta.1', '8.0.2']
187+
result = config.send(:find_latest_prerelease, versions)
188+
189+
expect(result).to eq('9.0.0.rc.2')
190+
end
191+
192+
it 'accepts versions with dot separator (e.g., 9.0.0.beta.1)' do
193+
versions = ['9.0.0.beta.1', '8.0.2']
194+
result = config.send(:find_latest_prerelease, versions)
195+
196+
expect(result).to eq('9.0.0.beta.1')
197+
end
198+
199+
it 'accepts versions with dash separator (e.g., 9.0.0-beta.1)' do
200+
versions = ['9.0.0-beta.1', '8.0.2']
201+
result = config.send(:find_latest_prerelease, versions)
202+
203+
expect(result).to eq('9.0.0-beta.1')
204+
end
205+
206+
it 'rejects invalid formats' do
207+
versions = ['9.0.beta', '8.0.2', 'foo.beta.1']
208+
result = config.send(:find_latest_prerelease, versions)
209+
210+
expect(result).to be_nil
211+
end
212+
213+
it 'rejects versions with beta/rc not following semver' do
214+
versions = ['beta.9.0.0', '8.0.2']
215+
result = config.send(:find_latest_prerelease, versions)
216+
217+
expect(result).to be_nil
218+
end
219+
220+
it 'returns nil when no prerelease versions exist' do
221+
versions = ['8.0.2', '8.0.1', '7.9.0']
222+
result = config.send(:find_latest_prerelease, versions)
223+
224+
expect(result).to be_nil
225+
end
226+
227+
it 'returns first prerelease (latest) when multiple exist' do
228+
# rubygems returns versions in descending order
229+
versions = ['9.0.0.beta.2', '9.0.0.beta.1', '8.0.2']
230+
result = config.send(:find_latest_prerelease, versions)
231+
232+
expect(result).to eq('9.0.0.beta.2')
233+
end
234+
235+
it 'handles empty array' do
236+
result = config.send(:find_latest_prerelease, [])
237+
expect(result).to be_nil
238+
end
239+
end
240+
241+
describe '#default_version_for' do
242+
let(:config) { described_class.new(config_file: '/nonexistent') }
243+
244+
it 'returns default shakapacker version' do
245+
expect(config.send(:default_version_for, 'shakapacker')).to eq('~> 8.0')
246+
end
247+
248+
it 'returns default react_on_rails version' do
249+
expect(config.send(:default_version_for, 'react_on_rails')).to eq('~> 16.0')
250+
end
251+
252+
it 'raises ArgumentError for unknown gem' do
253+
expect do
254+
config.send(:default_version_for, 'unknown_gem')
255+
end.to raise_error(ArgumentError, 'Unknown gem: unknown_gem')
256+
end
257+
end
258+
259+
describe '#fetch_latest_prerelease integration' do
260+
let(:config) { described_class.new(config_file: '/nonexistent') }
261+
262+
it 'returns nil on command failure' do
263+
allow(Open3).to receive(:capture3).and_return(['', 'error', double(success?: false)])
264+
265+
result = config.send(:fetch_latest_prerelease, 'shakapacker')
266+
expect(result).to be_nil
267+
end
268+
269+
it 'returns nil when exception occurs' do
270+
allow(Open3).to receive(:capture3).and_raise(StandardError, 'test error')
271+
272+
result = config.send(:fetch_latest_prerelease, 'shakapacker')
273+
expect(result).to be_nil
274+
end
275+
276+
it 'uses array syntax for gem search command (security)' do
277+
expect(Open3).to receive(:capture3).with('gem', 'search', '-ra', '^shakapacker$')
278+
.and_return(['shakapacker (8.0.0)', '', double(success?: true)])
279+
280+
config.send(:fetch_latest_prerelease, 'shakapacker')
281+
end
282+
end
151283
end

0 commit comments

Comments
 (0)