chef-vault icon indicating copy to clipboard operation
chef-vault copied to clipboard

Fixed: Vault warns when valid JSON contains special character sequences

Open decoyjoe opened this issue 1 year ago • 4 comments

Description

Fixes an issue where Chef Vault writes a warning when a valid JSON document contains special character sequences, exposing sensitive vault data in the warning message.

Chef Vault should be agnostic to the content of the vault item JSON. It no longer inspects vault item contents, validation responsibility is left to the JSON parser to ensure the JSON data is valid.

Related Issue

Fixes #415 Chef Vault warns and leaks secrets when vault json contains properly encoded special characters · Issue #415 · chef/chef-vault

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • [x] I have read the CONTRIBUTING document.
  • [x] I have run the pre-merge tests locally and they pass.
  • [ ] I have updated the documentation accordingly.
  • [x] I have added tests to cover my changes.
  • [ ] If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • [x] All new and existing tests passed.
  • [x] All commits have been signed-off for the Developer Certificate of Origin.

decoyjoe avatar Nov 25 '24 22:11 decoyjoe

Hey @johnmccrae - I think this is related to powershell stuff?

jaymzh avatar Nov 26 '24 21:11 jaymzh

Just to keep people following this PR up-to-date, I followed up in Slack on this. The team is aware - but the pipelines here need some significant investigation to sort out. I've asked for a timeline estimate.

jaymzh avatar Feb 13 '25 17:02 jaymzh

https://github.com/chef/chef-vault/pull/435/files @jaymzh @decoyjoe fyi, I believe this is the work to fix the pipeline.

tpowell-progress avatar Mar 04 '25 17:03 tpowell-progress

Branch has been rebased on latest from main.

decoyjoe avatar May 06 '25 19:05 decoyjoe

@tpowell-progress @jaymzh it looks like the pipeline has been fixed. Could we get another review on this?

decoyjoe avatar May 13 '25 19:05 decoyjoe

Yeah, just approved workflows, if they pass, we'll merge it today.

jaymzh avatar May 13 '25 19:05 jaymzh

OH, we'll need workstation approvers. Will have THomas ping them in the meeting today.

jaymzh avatar May 13 '25 19:05 jaymzh

@decoyjoe It looks like the branch is out of date with the main. Could you please do a rebase with the main?

ashiqueps avatar May 14 '25 05:05 ashiqueps

@ashiqueps Rebased on main.

decoyjoe avatar May 14 '25 15:05 decoyjoe

@ashiqueps Rebased on main.

@decoyjoe , still I see this branch is out-of-date with the base branch

nikhil2611 avatar May 14 '25 16:05 nikhil2611

@nikhil2611 Oops, sorry. It's actually rebased now.

decoyjoe avatar May 14 '25 16:05 decoyjoe