Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop publishing Gemfile in default gem template #6723

Merged
merged 1 commit into from Jun 8, 2023

Conversation

gareth
Copy link
Contributor

@gareth gareth commented Jun 6, 2023

Similarly to how the other ignored files are intended for local development and not for production, the Gemfile and Gemfile.lock files for a gem only relate to local development and aren't useful to people installing the gem.

What was the end-user or developer problem that led to this PR?

As part of our organisational security measures, internal systems with our Ruby projects deployed onto them are scanned by a vulnerability management tool. In our case, this tool identifies Ruby vulnerabilities by looking for Gemfile.lock files installed on the system and comparing the referenced library versions to the list of relevant vulnerabilities based on the CVE database.

In a broad sense, this is a reasonable check - as far as our deployed application is concerned, if its Gemfile.lock contains vulnerable libraries then we probably installed those libraries while deploying the app: if you bundle install in a project containing a file like this, of course you'll pull in the bad dependencies.

However, every gem chooses (either explicitly or implicitly) which of its source code's files get included and installed as part of its gem packaging. A large number of gems include their own development Gemfile.lock in that list. So that means our vulnerability scanner generates a number of false positives from all of the unused Gemfile.lock files that have been included in any of those gems.

False positives in security scans are bad because where they can't easily be worked around, people often get lazy and start getting used to warning messages and paying less attention to them.

This is not at all the fault of rubygems, or even a strong concern for gem authors, but it's a problem whose root cause is contributed to (in part) by the bundler gem defaults.

What is your fix for the problem, implemented in this PR?

So first up, this fix does not immediately and directly fix the problem I described. In general the approach taken by our scanning tool is overly broad, and we have successfully argued that just identifying Gemfile.lock files that reference vulnerable libraries is not enough to demonstrate a vulnerability. We would like to configure our tool to identify just the lock files that we're actively installing against.

However, after some research I wasn't able to find a good reason for Gemfile* files to get published as part of a gem at all. Just like with test files, they're only really useful for local development. If someone was looking to start working on a gem, they shouldn't be starting from the published bundle.

This PR stops the gemspec including files beginning with "Gemfile", meaning Gemfile and Gemfile.lock.

Even though not everyone uses bundle gem to get their gem started, by including a template that publishes Gemfile files, this project is suggesting that this is the correct behaviour. That will cascade down to other libraries, blogposts about the process, and so on.

Fixing the template also won't tidy up pre-existing gems that mistakenly publish these development files, but it might stop people from creating new ones, and at least provides a point of reference for anyone else who wonders about this.

Make sure the following tasks are checked

*The new test takes inspiration from the similar gemspec exclusion test. I wasn't able to test Gemfile.lock because it looks like similar tests in the same file don't actually run bundle install and I wasn't sure if that was necessary.

Similarly to how the other ignored files are intended for local
development and not for production, the Gemfile and Gemfile.lock files
for a gem only relate to local development and aren't useful to people
installing the gem.
@welcome
Copy link

welcome bot commented Jun 6, 2023

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks!

Copy link
Member

@simi simi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 This reminds me all those discussions about gem packaging (should be gem itself able to run tests, .., deps specifications in Gemfile/gemspec), ...

But since bin/spec/test/features directories are already on the ignore list, this makes sense.

@deivid-rodriguez
Copy link
Member

Thank you!

@deivid-rodriguez deivid-rodriguez merged commit 596d4ab into rubygems:master Jun 8, 2023
92 checks passed
deivid-rodriguez added a commit that referenced this pull request Jun 8, 2023
Stop publishing Gemfile in default gem template

(cherry picked from commit 596d4ab)
@gareth gareth deleted the dont-publish-gemfile branch June 8, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants