rails
rails copied to clipboard
YAML serialization options
Summary
Allow setting YAML serialization options on a per attribute basis – along with application wide setting.
- Permit YAML classes per attribute (
serialize :content, yaml: { permitted_classes: [Symbol] }
) - Permit YAML unsafe load per attribute (
serialize :content, yaml: { unsafe_load: true }
)
Other Information
Rails' versions 7.0.3.1, 6.1.6.1, 6.0.5.1, and 5.2.8.1 introduced application wide setting to allow unsafe YAML load, along with a global permitted classes setting. This PR intends to add such options on a per attribute basis, so updating to the new behavior can be done incrementally, while also providing more flexibility when allowing YAML to load classes.
In case this is accepted, would it be fine to backport/release this to 5.x and 6.x?
I'm having the same issue
Having the same issue here too. Thanks @xjunior for this PR. You are shining like the sun rising.
Had the same issue here. Thanks @xjunior for this PR. You saved the day here buddy.
would it be fine to backport/release this to 5.x and 6.x?
No. We don't backport new features to stable releases.
would it be fine to backport/release this to 5.x and 6.x?
No. We don't backport new features to stable releases.
I get it, but in this case, this was a security patch that introduced a feature creating problems on older releases. Wouldn't that be acceptable?
We have 30 models using the YAML serializer in the application I work. We're working on getting this app up to rails 6, then 6.1, and finally 7. Meanwhile, we've disabled safe loads – which is alright since it has always been unsafe.
We want to work toward enabling safe loads. Ideally, that could be done model by model, allowing specific classes for specific cases or sometimes allowing models to use unsafe load. We'd love to do that before we get to rails 7.
For anyone with this issue, this script looks good to find what you serialize: https://gist.github.com/crawler/47a1e66ee2c2ea37f56f9c0c2aac071a
Actually, even Rails 7.0 would not get this change. The security fix didn't introduce any problem on existing application. Existing application can ,on their own risk, disable the security fix or allow all the classes that they want to allow globally.
This isn't a bug fix for the security fix, which we could consider backporting. This is a new feature, not necessary to apply the security fix, neither to upgrade the applications to the latest version.
@rafaelfranca I guess it makes sense. Thanks!
We're using the aforementioned script to figure out the classes so we can properly fix this CVE in our app. If you're running a rails 5, replace the pairs method with a hardcoded collection of Class/method pairs.
Merged in 4a075537dc0592753718b0f2d264ba56297ee4d3. Thanks