printercu/secure_credentials

encrypted:edit command fails when specifying a key file and the master key file exists

jimryan opened this issue · 4 comments

If a config/master.key exists, the rails encrypted:edit command will fail to generate the key file when one is specified but doesn't exist and will raise a ActiveSupport::EncryptedFile::MissingKeyError.

This happens because of how #key is defined in SecureCredentials::EncryptedFile.

It will fall back onto the master key if no env key or key file are specified. Compare this to how #key is defined in ActiveSupport::EncryptedFile. It will return nil if no env key or key file exist, but a master key does.

This comes into play when the encrypted command determines if it needs to generate the key or not. If EncryptedFile#key returns something, it will skip over that step. With the vanilla Rails implementation, this is nil if there is no env key or key file. However, with secure_credentials' implementation, EncryptedFile#key returns the master key, so that step is skipped. That key is eventually needed, resulting in a ActiveSupport::EncryptedFile::MissingKeyError being raised.

I could have submitted a quick PR to make secure_credentials' implementation match Rails' , but I figured there was a reason that it returned the master key, thus warranting a discussion and maybe further work.

Thanks for the awesome gem - has been a huge help and IMO should be part of rails core.

Hi! Thanks for reporting and investigation.

Yep, config/master.key can not be used as default value for argument, because we wan't to calculate it depending on file name (ex, secrets.staging.key for secrets.staging.yml.enc). But this calculated key_path should be able to fallback to original config/master.key if file is missing.

Looks like it's better to move this logic to Store#initialize and check for key file existence there with fallback to config/master.key. This will also simplify EncryptedFile.

I'll prepare fix soon.

Can you please try fix_fallback branch. I'll merge and release it if everything is ok.

@printercu So sorry for the slow reply - I must have missed this back in April.

Funnily enough, I ran into this issue again, headed over to the GitHub project, and found my own Issue (this one) - which I had forgotten all about.

Anyway, I tested the fix_fallback branch in this scenario and it works. Thanks a bunch.

Released 0.2.3 with this fix.