Never write credentials to lockfiles

[deivid-rodriguez]

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

Bundler will sometimes write credentials to lockfiles, which is NOT very secure since, for example, they may get pushed to a public repo by mistake.

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

Never write credentials to lockfiles.

Previously, if they were present, they'd be considered for lockfile expiration. I think it's fine to never consider them and never write them in the lockfile. If there are credentials present in the Gemfile (through ENV, for example), complete lockfile sources with those so that deployment mode (that only considers the lockfile) still works.

There was previously a setting to enable this behavior but I think it should be the default, so I removed the setting.

Fixes #1240.
Fixes #3234.
Closes #7185.

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[technicalpickles]

I'm still gathering information and getting a reproducible case, but I believe this change is causing problems during CI. The bundle install succeeds, but then we get missing gems when actually trying to activate, similar to the errors on #7185 ie

Could not find gem 'our-gem' in locally installed gems
Run `bundle install` to install missing gems.

Where our-gem uses a source with a credential in the gemfile.

[deivid-rodriguez]

Thanks letting me know so quickly. I will try to figure out on Monday what's happening here and and make a patch release with a revert if needed.

[gdubicki]

Hi @deivid-rodriguez!

Thanks for your work on this but I think we see an issue with it too: when using Bundler 2.5.10, although we DO use credentials in our Gemfile for some gems, with a syntax like:

source "https://<redacted>@gems.contribsys.com/" do
  gem "sidekiq-pro", "~> 7.2"
end

...in the generated Gemfile.lock the credentials were stripped. Update: so it works as intended and it doesn't cause any problem.

[deivid-rodriguez]

That's exactly the feature implemented by this PR. Did it cause any issues or is it that you actually want credentials written to the lockfile?

[gdubicki]

That's exactly the feature implemented by this PR. Did it cause any issues or is it that you actually want credentials written to the lockfile?

Oops, sorry! I actually just assumed that it would cause an issue. After actually testing it, I have none. So nvm!

[deivid-rodriguez]

@technicalpickles Since we haven't got any more reports, I'm actually holding on the revert, but I'll wait till you find out more and investigate then.

[mensfeld]

+1 on the issue of invalid assignment

[deivid-rodriguez]

@technicalpickles Can you check if the lockfile is correct after bundle install (same as before, except it's removing credentials from it) or if it is somehow mis-assigning gems to incorrect sources?

[ngan]

Hey @deivid-rodriguez, our Gemfile.lock diff only contains removal of credentials and bundler version change.

I spent a few minutes trying to reproduce this locally but couldn’t. The only difference between local and CI is that we use deployment mode. I’m going to give that a try next when I get a chance.

[nertzy]

@deivid-rodriguez I have reproduced the issue.

Our application uses Karafka Pro, following the setup from @mensfeld in the Karafka docs:

source 'https://USERNAME:PASSWORD@gems.karafka.io' do
  gem 'karafka-license', 'LICENSE_ID'
end

We upgraded our Bundler version via bundle update --bundler and checked in the resulting diff to the Gemfile.lock that updated the Bundler version and removed those credentials.

When deploying to Heroku, we see:

-----> Installing dependencies using bundler 2.5.6
       Running: BUNDLE_WITHOUT='development:test' BUNDLE_PATH=vendor/bundle BUNDLE_BIN=vendor/bundle/bin BUNDLE_DEPLOYMENT=1 bundle install -j4
       The list of sources changed, but the lockfile can't be updated because frozen
       mode is set
       
       You have changed in the Gemfile:
       * karafka-license from `rubygems repository [https://gems.karafka.io/`](https://gems.karafka.io/%60) to
       `rubygems repository ***gems.karafka.io/`
       
       Run `bundle install` elsewhere and add the updated Gemfile to version control.
       If this is a development machine, remove the Gemfile.lock freeze by running
       `bundle config set frozen false`.
       Bundler Output: The list of sources changed, but the lockfile can't be updated because frozen
       mode is set
       
       You have changed in the Gemfile:
       * karafka-license from `rubygems repository [https://gems.karafka.io/`](https://gems.karafka.io/%60) to
       `rubygems repository ***gems.karafka.io/`
       
       Run `bundle install` elsewhere and add the updated Gemfile to version control.
       If this is a development machine, remove the Gemfile.lock freeze by running
       `bundle config set frozen false`.

 !
 !     Failed to install gems via Bundler.
 !
 !     Push rejected, failed to compile Ruby app.

 !     Push failed
 ▸    Build failed

For now I will roll back to our previous Bundler version as a workaround.

[nertzy]

I noticed there is a backtick getting escaped to %60 in the output https://gems.karafka.io/%60 so I wonder if that is related to the underlying issue?

[deivid-rodriguez]

Oh your logs made me realize.

Normally a lockfile generated with Bundle x.y.z will be bundled with Bundler x.y.z. Bundler has builtin behavior to ensure that. If that happens things work fine.

However, Heroku is special and will forcefully use its own version (2.5.6 at the moment, apparently) and also does not provide any options to customize that. If Bundler older than 2.5.10 bundles a 2.5.10 lockfile that stripped credentials, and does that in "frozen mode", which Heroku also enables by default, then indeed things won't work.

If you run bundle config --local allow_deployment_source_credential_changes true and commit the resulting configuration, does that make things work again?

[deivid-rodriguez]

Or if that's easier, set the BUNDLE_ALLOW_DEPLOYMENT_SOURCE_CREDENTIAL_CHANGES env variable to true.

[nertzy]

Or if that's easier, set the BUNDLE_ALLOW_DEPLOYMENT_SOURCE_CREDENTIAL_CHANGES env variable to true.

That didn't work, because that variable only has an effect in Bundler 2.5.10 and later, and we're still given 2.5.6 by the Heroku buildpack, if I understand correctly.

I think another workaround would be to explicitly set the credentials via BUNDLE_GEMS__KARAFKA__IO (or its equivalent).

[deivid-rodriguez]

That didn't work, because that variable only has an effect in Bundler 2.5.10 and later, and we're still given 2.5.6 by the Heroku buildpack, if I understand correctly.

Oh, too bad. That variable was present in 2.5.6 and I suggested it because I thought the reporter of #7185 suggested that it had the effect of preventing credentials to be added, but now I see it's actually the other way around (it forced credentials to be added when credentials were specified in configuration).

I think another workaround would be to explicitly set the credentials via BUNDLE_GEMS__KARAFKA__IO (or its equivalent).

Yes, that'd be the preferred workaround, don't reference credentials directly in Gemfile either 👍.

[veganstraightedge]

Hi 👋🏻 while I understand and support the idea of keeping credentials out of checked in lockfiles, it feels to me that changing the behavior and format of the lockfile is a major change. A contract breaking change.

IMO, this kind of change should be warned about with deprecation warnings in a minor version first. Then in only in a major version, make this change.

This change broke all Dependabot PRs with private gems using this pattern:

# Gemfile
source "https:/USERNAME:PASSWORD@example.com" do
  gem "private_gem"
end

For example, Sidekiq Enterprise.

Can this change be urgently reverted please?
And then rolled out with deprecation warnings and major version release please?

🖤

cc: @deivid-rodriguez

[deivid-rodriguez]

Hello. Thanks for the ping.

This was supposed to change only a lockfile detail, but I was not expecting it to break anything other than that. I don't consider a somewhat surprising diff in a lockfile to be a breaking change.

That said, it sounds like this has caused real problems, as reported in #7715. That's of course not ok. I will investigate that issue and decide what to do next.

[broksonic21]

As a user, my thoughts:

in #7715, they are running an older bundler vs a 2.5.11 generated lockfile. Like the dependabot related tickets, that's ultimately what I am noticing too -> anytime you use a 2.5.10+ bundler (whether locally or via a tool like dependabot), it writes out a syntax that doesn't work if you also use a lower-bundler for any reason (heroku, vendor'd, etc)

In that sense, this is a breaking change, but I also think there's a bit of user (or tooling issues) - when you upgrade bundler in one spot, you should update in all.

That's hard, I admit, if:

• Heroku won't respect the bundler version in the Gemfile.lock - rubygems has code to autoupdate you locally on recent rubygems, so unfortunate Heroku doesn't
• Dependabot like tools should respect the Gemfile.lock listed bundler version. I know dependabot just updated to use 2.5.11 always, which helps my case since we want to use 2.5.10+ bundlers with the security fix + without constant lockfile churn by dependabot putting creds back in if older bundler and local devs taking it back out with the newer bundler, but probably harms the reverse case

[deivid-rodriguez]

Thanks for the helpful feedback @broksonic21. I agree. I thought only issues were with Heroku and Dependabot, which in my opinion should eventually respect auto-switching. I also thought issues were only about constant lockfile churn from Dependabot/Heroku and local Bundler competing to add & remove credentials, and I expected those to be gone once both tools upgrade their internal version of Bundler.

However, after thinking more about this today and looking at the new issue opened, I realize there are more cases where Bundler won't autoswitch (expectedly, or unexpectedly), and things can break in that case. Also I realize there are way more people putting credentials in their Gemfiles than I thought 😅.

So I think we should restore a better default behavior that keeps credentials in lockfile if they are already there. Working on it!

[deivid-rodriguez]

I created two PRs to alleviate these issues:

• Auto switch to locked bundler version even when using binstubs #7719 will make Bundler switch versions properly when using binstubs with a local Bundler path, preventing Can't bundle sidekiq-ent with latest Bundler #7715 from happening.
• Keep credentials in lockfile if they are already there #7720 keeps credentials in lockfiles that already had credentials in them, allowing them to keep working on older versions of Bundler.

As a follow up change, I will add deprecation warnings when writing lockfiles with credentials, so that people can gradually remove credentials from them.