autorestore: can't modify frozen Hash

[bkeepers]

After perusing dependabot updates to dotenv 3.0, I'm seeing a handful of builds breaking with this error:

can't modify frozen Hash: {"SOME_ENV_VAR"=>"true"}
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `replace'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `block in restore'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications.rb:206:in `block in instrument'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications/instrumenter.rb:58:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications.rb:206:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:132:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `restore'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv/autorestore.rb:9:in `block (2 levels) in <top (required)>'

This is caused by autorestore and is happening because ENV is somehow frozen, but the head scratcher is that ENV.freeze raises cannot freeze ENV (TypeError).

Examples:

• https://github.com/ministryofjustice/Claim-for-Crown-Court-Defence/pull/6523 https://github.com/ministryofjustice/laa-assess-non-standard-magistrate-fee/pull/269
• https://github.com/OfficeForProductSafetyAndStandards/cosmetic-product-notifications/pull/3268
• https://github.com/opf/openproject/pull/14773

[dbackeus]

We're hitting this issue in a Rails 7 app for all specs using climate_control to modify ENV.

[bkeepers]

@dbackeus thanks, I'll work up a PR to disable autorestore if using climate_control or ice_age. In the mean time, do you have any idea how ENV is getting frozen? I can't figure out what is freezing it or how they're doing it, so any help with that would be appreciated.

[dbackeus]

I can confirm that your PR makes our specs green. But no idea about the frozen issue.

[rgarner]

We're not using Climate Control, but we are using stub_const in RSpec, e.g. before { stub_const('ENV', ENV.to_hash.merge('SOME_KEY' => 'some-value')) } and our dependabot build against dotenv-rails still breaks in this way.

We're using

  config.dotenv.autorestore = false

in config/environments/test.rb if it helps someone else.

(but it's better to just set the ENV var and use this autorestore instead, because that's the actual feature 🤦🏻 )

[kajetanowicz]

I can't figure out what is freezing it or how they're doing it, so any help with that would be appreciated.

Hi @bkeepers :wave: I wasn't able to fully figure out the exact chain of events but by trial and error it seems that the issue is caused by this bit https://github.com/bkeepers/dotenv/blob/main/lib/dotenv/diff.rb#L53. When Dotenv is used together with rspec's stub_const('ENV', 'foo' -> 'bar') it seems that the latter is doing it's own housekeeping after the test run and later when the autorestore kicks in it's trying to call replace on something that was previously frozen by the Diff class. One way to go around this issue is to remove the freeze part from Diff but it's not really solving the underlying problem.

[sk-]

@bkeepers as others have already mentioned this issue is not exclusive to certain CI environments, but rather to the fact that using rspec's stub_const messes with the autorestore functionality.

Would it be possible to reopen this issue and provide guidelines on the impact of disabling autorestore?

[bkeepers]

@rgarner, @kajetanowicz and @sk-, thanks for the stub_const pointers. I was able to make a test case that demonstrated the issue and fix it in #504.

The problem is happens when you replace ENV with an instance of Hash. ENV.to_h returns a copy, but Hash#to_h just returns itself, which was then getting frozen.

This should be fixed in 3.1.2, which will be out shortly.