-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
In README generated by bundle gem
, do not fill rubygems.org install commands with the gem name automatically
#6093
Conversation
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 |
In reference to this PR (rubygems/rubygems#6093) we need to protect the placeholder name from being brand jacked: `update_with_your_gem_name_prior_to_release_to_rubygems_org`
$ gem install <%= config[:name] %> | ||
$ gem install UPDATE_WITH_YOUR_GEM_NAME_PRIOR_TO_RELEASE_TO_RUBYGEMS_ORG | ||
|
||
TODO: Replace the `UPDATE_WITH_YOUR_GEM_NAME_PRIOR_TO_RELEASE_TO_RUBYGEMS_ORG` with your gem name before releasing it to RubyGems.org or delete this section if you do not plan to release to RubyGems.org. Please do not do it earlier due to security reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In next section TODO is at top of the section. Would it make sense to follow and do the same here (move TODO to top of this section)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this todo up as well as the one from the first section to align them
|
||
## Installation | ||
|
||
TODO: Replace the `UPDATE_WITH_YOUR_GEM_NAME_PRIOR_TO_RELEASE_TO_RUBYGEMS_ORG` with your gem name before releasing it to RubyGems.org or delete this section if you do not plan to release to RubyGems.org. Please do not do it earlier due to security reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The security concern is when changing the gem name, not when deleting the section. I guess it's obvious but the current wording feels ambiguous in that regard?
I also think we should link somewhere to try explain what the security concern is, or explain it ourselves. I believe users are more likely to follow advice they actually understand and I believe most users are not aware of this problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The security concern is when changing the gem name, not when deleting the section.
Not exactly. Even if you do not release someone can release it for you and brandjack. Which was happening.
I also think we should link somewhere to try explain what the security concern is, or explain it ourselves. I
Fair point. Do you want me to write a blog post for https://blog.rubygems.org/ about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, isn't the safe path the exact opposite to what we are recommending?
Replace the
UPDATE_WITH_YOUR_GEM_NAME_PRIOR_TO_RELEASE_TO_RUBYGEMS_ORG
with your gem name before releasing it to RubyGems.org
Shouldn't it be after rather than before, or maybe right after if we want to recommend doing it at that same-ish time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right after is a much better wording. Updating...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly. Even if you do not release someone can release it for you and brandjack. Which was happening.
Right, I meant it regarding the trailing "Please do not do it earlier due to security reasons" sentence. In the case of deleting the section because of not planning to release to rubygems.org, the timing doesn't really matter, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like after
was replaced with before
, but PRIOR_TO
was left unchanged.
I've opened #6338 to update this.
I also don't know if a blog post ended up being written, but if so, perhaps we can link to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to clarify wording a bit!
This PR comes as part of our security work. With @simi, @indirect, and @hsbt an increase in brandjacking attacks where the brand's origin comes from scanning GH for names defined in the README.md but not in RubyGems. It seems that often the default flow for people is to start their gems and upload them to GitHub without actually releasing them to RubyGems. In such cases or in cases where the author does not want to publish to RubyGems at all, the package name can be taken over and used for malicious activities, hoping someone will follow along with the recommendation from the README to install from RubyGems.
32eaa58
to
42bc471
Compare
bundle gem
, do not fill rubygems.org install commands with the gem name automatically
This PR comes as part of our security work. With @simi, @indirect, and @hsbt an increase in brandjacking attacks where the brand's origin comes from scanning GH for names defined in the README.md but not in RubyGems.
It seems that often the default flow for people is to start their gems and upload them to GitHub without actually releasing them to RubyGems. In such cases or in cases where the author does not want to publish to RubyGems at all, the package name can be taken over and used for malicious activities, hoping someone will follow along with the recommendation from the README to install from RubyGems.