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

credentials:edit/show commands ignores custom content_path and key_path #37631

Closed
MatthieuBarthel opened this issue Nov 4, 2019 · 5 comments · Fixed by #47107
Closed

credentials:edit/show commands ignores custom content_path and key_path #37631

MatthieuBarthel opened this issue Nov 4, 2019 · 5 comments · Fixed by #47107

Comments

@MatthieuBarthel
Copy link

Steps to reproduce

In application.rb, configure custom credentials content_path and key_path:

config.credentials.content_path = Rails.root.join('config', "credentials.custom.yml.enc")
config.credentials.key_path = Rails.root.join('config', "master.custom.key")

Rename your existing credentials files to the new names we've just configured.
rails s and rails c should work fine with your existing credentials.

Expected behavior

rails credentials:edit and rails credentials:show should work with the custom paths, but it doesn't.

Actual behavior

rails credentials:edit will generate new credentials files (credentials.yml.enc and credentials.yml.enc) instead of editing our current credentials with custom paths.

rails credentials:show also ignores custom paths.

System configuration

Rails version: 6.0.0

Ruby version: 2.6.5

@xenyal
Copy link

xenyal commented Nov 4, 2019

I'd like to validate my interpretation of the issue at hand before taking a stab at it (I've yet to put up a PR to rails yet but I'm looking for opportunities) 😃

In the railties subdirectory, within application.rb we're given the option of overriding config.credentials.content_path and config.credentials.key_path where credentials is being defined initially. However, in Rails::Command::CredentialsCommand the content_path being used by rails credentials:show and rails credentials:edit seems to be hardcoded to the evaluation of options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc". So, any overrides passed into Rails::Application will get ignored.

Would an adequate solution be to add more guards inside of Rails::Command::CredentialsCommand that will first check against config.credentials.content_path and config.credentials.key_path before trying environment.yml.enc and credentials.yml.enc, and then adding the corresponding unit tests for those cases?

@xenyal
Copy link

xenyal commented Nov 4, 2019

It looks to me that the design made in Rails::Command::CredentialsCommand is specific towards only supporting config/credentials/:environment.yml.enc and config/credentials/:environment.key if an environment option is given, otherwise defaulting to the static path of config/credentials.yml.enc. I'd like to double check that overriding those paths with a custom one on the application configuration level is actually supported before starting.

@MatthieuBarthel
Copy link
Author

Thanks for your help, here are some links about the paths override :
https://edgeapi.rubyonrails.org/classes/Rails/Application.html#method-i-credentials
#34535 (comment)

@brianthoman
Copy link

brianthoman commented Nov 19, 2019

Hi, I hope you don't mind if I add a bit to this issue, as I would expect that the paths can also be set on a per-environment basis in config/environments/<environment>.rb, as with most/all other configuration settings.

@rails-bot
Copy link

rails-bot bot commented Feb 17, 2020

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-0-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Feb 17, 2020
@rails-bot rails-bot bot closed this as completed Feb 24, 2020
@ghiculescu ghiculescu reopened this Jan 22, 2023
@rails-bot rails-bot bot removed the stale label Jan 22, 2023
jonathanhefner added a commit to jonathanhefner/rails that referenced this issue Jan 25, 2023
This commit changes the credentials commands (e.g. `bin/rails
credentials:edit`) to respect `config.credentials.content_path` and
`config.credentials.key_path` when set in `config/application.rb`.

Before this commit:

* Unlike other `bin/rails` commands, `bin/rails credentials:edit`
  ignored `RAILS_ENV`, and would always edit `config/credentials.yml.enc`.

* `bin/rails credentials:edit --environment foo` would create and edit
  `config/credentials/foo.yml.enc`.

* If `config.credentials.content_path` or `config.credentials.key_path`
  was set, `bin/rails credentials:edit` could not be used to edit the
  credentials.  Editing credentials required using `bin/rails
  encrypted:edit path/to/credentials --key path/to/key`.

After this commit:

* `bin/rails credentials:edit` will edit the credentials file that the
  app would load for the current `RAILS_ENV`.

* `bin/rails credentials:edit` respects `config.credentials.content_path`
  and `config.credentials.key_path` when set in `config/application.rb`.
  Using `RAILS_ENV`, environment-specific paths can be set, such as:

    ```ruby
    # config/application.rb
    module MyCoolApp
      class Application < Rails::Application
        config.credentials.content_path = "my_credentials/#{Rails.env}.yml.enc"

        config.credentials.key_path = "path/to/production.key" if Rails.env.production?
      end
    end
    ```

* `bin/rails credentials:edit --environment foo` will create and edit
  `config/credentials/foo.yml.enc` _if_ `config.credentials.content_path`
  has not been set for the `foo` environment.  Ultimately, it will edit
  the credentials file that the app would load for the `foo` environment.

Note that the credentials commands do not run initializers.  Therefore,
they will not, for example, attempt to connect to the database of the
specified environment.

Fixes rails#37631.
Fixes rails#41830.
Fixes rails#46884.
Closes rails#46904.

Co-authored-by: Alex Ghiculescu <alex@tanda.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants