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

unsafe_load secrets.yml with psych 4 #42687

Merged
merged 1 commit into from Jul 3, 2021

Conversation

NatMorcos
Copy link
Contributor

@NatMorcos NatMorcos commented Jul 3, 2021

Summary

Pysch 4 now defaults YAML.load to YAML.safe_load, which does not allow aliases unless an aliases flag is given:
ruby/psych#487
https://github.com/ruby/psych/blob/216f94fb9aacb0754f1dd2257b8d6a61b278a8b2/lib/psych.rb#L322

This PR switches secret loading to use unsafe_load. This should be fine because secrets.yml is in-repo, and therefore trusted. I have copied the implementation from #42257, which did the same thing for database.yml

This maintains backwards compatibility to allow developers to have a secrets.yml like

development: &default_settings
  thing_client_id: 'abcd'
  thing_client_secret: 'xyz'
test:
  <<: *default_settings
  
  #... etc.

Other Information

@rails-bot rails-bot bot added the railties label Jul 3, 2021
@NatMorcos NatMorcos force-pushed the psych_4_load_secrets_with_aliases branch from 1efc441 to b640bd2 Compare July 3, 2021 00:38
@kamipo
Copy link
Member

kamipo commented Jul 3, 2021

Can you copy the implementation from #42257 instead to follow the latest codebase?

@NatMorcos NatMorcos force-pushed the psych_4_load_secrets_with_aliases branch from b640bd2 to 0ebb708 Compare July 3, 2021 13:20
@NatMorcos NatMorcos changed the title Allow aliases in secrets.yml with psych 4 unsafe_load secrets.yml with psych 4 Jul 3, 2021
@kamipo kamipo merged commit 7179dcb into rails:main Jul 3, 2021
kamipo added a commit that referenced this pull request Jul 23, 2021
…iases

unsafe_load secrets.yml with psych 4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants