Escape glob metacharacters in install paths when globbing#9687
Open
hsbt wants to merge 1 commit into
Open
Conversation
Directory names may legally contain glob metacharacters like "[1]", and interpolating such a path into Dir[...] makes the glob silently match nothing. This made gem plugins and Gem.find_files come up empty, skipped gemspec serialization when installing git gems, and made `bundle clean` and `bundle cache` lose track of the files they manage. Glob with Gem::Util.glob_files_in_dir so the directory is not treated as part of the pattern, like 7fe7e7b did for the path source.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens RubyGems/Bundler file discovery against install paths containing glob metacharacters (e.g., dir[1]) by avoiding interpolated Dir[...] calls and instead globbing with a fixed base: directory.
Changes:
- Switch several path-sensitive
Dir[...]usages toGem::Util.glob_files_in_dir(..., base_path)to prevent metacharacters in the base path from being treated as part of the glob. - Refactor
Gem::BasicSpecification#matches_for_globto glob relative tofull_gem_path. - Add regression tests covering bundle paths / gem dirs containing glob metacharacters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/rubygems/test_gem_stub_specification.rb | Adds a regression test ensuring matches_for_glob works when the gem base dir contains []. |
| spec/install/gemfile/git_spec.rb | Adds coverage to ensure git gemspec serialization works when bundle path contains glob metacharacters. |
| spec/commands/clean_spec.rb | Adds coverage to ensure bundle clean works when bundle path contains glob metacharacters. |
| lib/rubygems/util.rb | Clarifies intent of glob_files_in_dir (glob relative to base: so base-path metacharacters aren’t interpreted). |
| lib/rubygems/basic_specification.rb | Updates matches_for_glob to use Gem::Util.glob_files_in_dir and factors require_paths glob string building into lib_dirs. |
| lib/bundler/source/rubygems.rb | Uses Gem::Util.glob_files_in_dir when enumerating cached .gem files. |
| lib/bundler/source/git.rb | Uses Gem::Util.glob_files_in_dir for gemspec serialization discovery under a destination dir. |
| lib/bundler/runtime.rb | Uses Gem::Util.glob_files_in_dir for cache/clean/prune directory enumeration and switches one manpath check to Dir.glob(base:). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if raw_require_paths.size > 1 | ||
| "{#{raw_require_paths.join(",")}}" | ||
| else | ||
| raw_require_paths.first |
Comment on lines
78
to
+81
| # Globs for files matching +pattern+ inside of +directory+, | ||
| # returning absolute paths to the matching files. | ||
| # returning absolute paths to the matching files. Unlike a plain | ||
| # Dir.glob with an interpolated path, glob metacharacters in | ||
| # +base_path+ are not treated as part of the pattern. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Install paths interpolated directly into
Dir[...]silently match nothing when the path contains glob metacharacters such as[,],{or}, which are legal in directory names. On Windows this happens in practice with user names likefoo[1], which browsers produce when deduplicating download names. The affected sites wereGem::BasicSpecification#matches_for_glob, which made gem plugins silently fail to load andGem.find_filesreturn nothing,Bundler::Source::Git#serialize_gemspecs_in, which skipped gemspec serialization for git gems, and the globs behindbundle cleanandbundle cache, which lost track of the files they manage.This applies the same fix as 7fe7e7b, which addressed the identical problem for the
gemspecDSL and the path source: glob throughGem::Util.glob_files_in_dir, which passes the directory as thebase:argument ofDir.globso only the intended pattern part is treated as a glob. The returned paths keep the same absolute form as before because bothGem.dirandfull_gem_pathare already expanded.Each of the three paths is covered by a new test using a
dir[1]directory, and all of them fail before the fix. A few remaining interpolated globs in secondary commands (gem contents,gem stale,gem setup, the certificate trust dir,bundle doctor) are left for a follow-up since they are not on the install path.