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

Biscuit does not export yaml, so don't yaml-parse it #4

Merged
merged 7 commits into from
May 16, 2018

Conversation

nevinera
Copy link
Contributor

@nevinera nevinera commented May 16, 2018

Motivation

I discovered in an Uploader PR (to move stuff from CF to biscuit) that LEGACY_INITIALIZATION_VECTOR had some kind of problem after I moved it. Investigation showed that, when I biscuit put -f FILE LEGACY_INITIALIZATION_VECTOR 1,2,3,4,5, the actual envar being supplied back by biscuit in staging was "12345".

When I dug into the details, this turned out to be because biscuit export is printing out its data in a raw KEY: VALUE format, which is not actually YAML, but resembles it strongly. We YAML-parse the output of that file, which results in getting an Array value for that key, and then cast it to a string in the library.

While my initial reaction was to just switch to using some other separator that YAML doesn't care about, I think this is a gotcha that will catch others, especially since biscuit doesn't actually get used locally.

Changes

  • Refactored the SecretsDecrypter specs around a set of shared examples
  • Added a few examples, including two that fail (numeric keys/values, and array value)
  • Rewrite the SecretsDecrypter to avoid using YAML.load (use raw string-splitting instead)
  • Quote the path to the secrets file in the bash command - this would only cause issues to someone that has oddities in the full-path to their source directories, but it could happen!
  • Bump the version to 0.1.1 - this is sufficiently backwards-compatible in my book. If anyone is relying on this behavior, they're being foolish.

Copy link
Member

@jerrod jerrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - what a great find! I could see this causing all manner of chaos.


context "when the values look like arrays" do
let(:exported_data) { "foo: 1,2,3,4,5" }
let(:expected_hash) { Hash["foo" => "1,2,3,4,5"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Copy link

@cdimartino cdimartino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@nevinera nevinera merged commit 10a310d into master May 16, 2018
@nevinera nevinera deleted the sp--biscuit-does-not-export-yaml branch May 16, 2018 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants