backport of #15477- fix(security): Replace YAML.load_stream to prevent deserialization attacks (CHEF-3854) #15466
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…(CHEF-3854) Replaced insecure YAML.load_stream calls with safe text pattern matching to prevent arbitrary code execution via YAML deserialization attacks. The vulnerability allowed malicious YAML files with deserialization gadgets Changes: - knife/lib/chef/knife/yaml_convert.rb: Count '---' separators with regex - lib/chef/recipe.rb: Count '---' separators with regex This maintains all existing functionality (multi-document detection, error messages) while eliminating the deserialization attack vector. The actual YAML parsing already uses YAML.safe_load() which is secure. Issue: CHEF-3854 Signed-off-by: nikhil2611 <nikhilgupta2102@gmail.com>
Signed-off-by: nikhil2611 <nikhilgupta2102@gmail.com>
|
jaymzh
approved these changes
Nov 25, 2025
tpowell-progress
approved these changes
Nov 25, 2025
jaymzh
requested changes
Nov 25, 2025
Collaborator
jaymzh
left a comment
jaymzh
left a comment
There was a problem hiding this comment.
Please make changes on main and backport, per standard policy. Do not change chef-18 directly unless the bug only affects Chef 18.
nikhil2611
changed the title
Collaborator
Author
Collaborator
Author
|
Hey @jaymzh , We added this security fix in chef-18 to get the Knife security update released, so we can ship the latest Knife version in Chef Workstation. |
jaymzh
approved these changes
Nov 26, 2025
Collaborator
jaymzh
left a comment
jaymzh
left a comment
There was a problem hiding this comment.
Thanks.
Would be really silly if we fixed a security issue that then regressed in 19. :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a security vulnerability (CHEF-3854) where
YAML.load_stream()was used to check for multi-document YAML files, allowing arbitrary code execution via deserialization attacks.Issue
CHEF-3854 - Chef/Knife YAML Insecure Deserialization through YAML.load_stream
Changes
knife/lib/chef/knife/yaml_convert.rb- ReplacedYAML.load_stream()with safe regex pattern matchinglib/chef/recipe.rb- ReplacedYAML.load_stream()with safe regex pattern matchingSecurity Impact
Fix: Replaced with
yaml_contents.scan(/^---\s*$/).lengthwhich simply counts document separator patterns without any deserialization, eliminating the attack vector.Tests & Coverage
Changed lines: 4; Existing tests verify multi-document detection still works correctly.
Functionality Preserved
---separators)Risk & Mitigations
Risk Classification: Low
Rationale: Localized change to YAML parsing validation logic. The actual parsing already used
YAML.safe_load()which is secure - we only fixed the pre-check that was vulnerable.Mitigation: Change only affects the multi-document detection mechanism, not the core YAML parsing functionality.
Rollback Strategy:
git revert 0cb36ea0ecAI Assistance
This work was completed with AI assistance following Progress AI policies.
DCO
All commits include Developer Certificate of Origin sign-off.