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.

Sign up for GitHub

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

Add --env-prefix to encrypt-file command #697

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add --env-prefix to encrypt-file command #697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5678b8f
Add --env-prefix to encrypt-file command
mlocati Nov 14, 2019
45e5efe
Update syntax of encrypt-file in README.md
mlocati Nov 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ There are two ways the client can treat existing values:
-R, --store-repo SLUG like --repo, but remembers value for current directory
-K, --key KEY encryption key to be used (randomly generated otherwise)
--iv IV encryption IV to be used (randomly generated otherwise)
--env-prefix ENV_PREFIX prefix of the names of the encryption key/IV environment variables (randomly generated otherwise)
-d, --decrypt decrypt the file instead of encrypting it, requires key and iv
-f, --force override output file if it exists
-p, --print-key print (possibly generated) key and iv
Expand Down
1 change: 1 addition & 0 deletions lib/travis/cli/encrypt_file.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class EncryptFile < RepoCommand
description 'encrypts a file and adds decryption steps to .travis.yml'
on '-K', '--key KEY', 'encryption key to be used (randomly generated otherwise)'
on '--iv IV', 'encryption IV to be used (randomly generated otherwise)'
on '--env-prefix ENV_PREFIX', 'prefix of the names of the encryption key/IV environment variables (randomly generated otherwise)'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, there. Thanks for the PR.

I have a couple of questions:

  1. What does this do that the existing --key and --iv flags do not?
  2. The code defines the variable, but it doesn't actually do anything. Did I miss something?

Copy link
Author

Choose a reason for hiding this comment

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

I'd need to know the names of the two variables in advance.
At the moment, what we have before _key and _iv is really hard to be determined before running the command (their prefix is determined here).

If we let users specify this --env-prefix option, the env_prefix variable will be set to the value of it, and

@env_prefix ||= "encrypted_#{Digest.hexencode(Digest::SHA1.digest(input_path)[0..5])}"

won't do anything (since env_prefix is already defined, it won't be assigned the value of what we have after the =).

Copy link
Author

Choose a reason for hiding this comment

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

@BanzaiMan Is there something unclear with my answer?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, there isn't. I understand the intent now. One thing still a little uneasy is the name. env_prefix seems a little vague. Maybe arg_prefix, env_var_prefix or something like that. Maybe a short flag name would help as well.

Additionally, the help text should indicate how this would interact with -K/--key and --iv.

Copy link
Author

Choose a reason for hiding this comment

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

One thing still a little uneasy is the name. env_prefix seems a little vague. Maybe arg_prefix, env_var_prefix or something like that.

  • if users don't specify this new parameter, the name of the two variables containing the IV/key pair will be prefixed by encrypted_ followed by a SHA-1 hash (for example: encrypted_abcdef012345_iv / encrypted_abcdef012345_key)
  • if users use this new parameter, they'll be able to specify what should be used instead of encrypted_abcdef012345.

So, what about enc_var_prefix ?

Maybe a short flag name would help as well.

Well, I think it'll be a rarely used argument, I don't know if it's worth reserving a short flag just for this tiny feature.

Additionally, the help text should indicate how this would interact with -K/--key and --iv.

Well, there's no interaction with the value of the generated variables, just with their names.
And to me the current text is quite clear about that:
prefix of the names of the encryption key/IV environment variables (randomly generated otherwise)
Do you have a better alternative to it?

Copy link
Contributor

Choose a reason for hiding this comment

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

enc_var_prefix seems acceptable.

By "interaction", I mean "what would happen if both -K and this new flag is given?" (as strange as it may sound). I believe this flag overrides --key and --iv, and that should be stated.

Copy link
Author

@mlocati mlocati Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

  • the --key (aka -K) sets the value of the encryption key
  • the --iv sets the value of the encryption IV
  • this --enc-var-prefix sets the prefix of the names of the two above values

Examples:

  • travis encrypt-file file.txt
    generates two environment variables:
    • encrypted_abcdef012345_iv with a value generated automatically
    • encrypted_abcdef012345_key with a value generated automatically
  • travis encrypt-file --enc-var-prefix myprefix file.txt
    generates two environment variables:
    • myprefix_iv with a value generated automatically
    • myprefix_key with a value generated automatically
  • travis encrypt-file --iv myiv --key mykey file.txt
    generates two environment variables:
    • encrypted_abcdef012345_iv with the "myiv" value
    • encrypted_abcdef012345_key with the "mykey" value
  • travis encrypt-file --enc-var-prefix myprefix --iv myiv --key mykey file.txt
    generates two environment variables:
    • myprefix_iv with the "myiv" value
    • myprefix_key with the "mykey" value

As you can see, there's no interaction between --key/-K/--iv and --enc-var-prefix.

And that's quite clear to me from the description of the options:

  • --key: encryption key to be used (randomly generated otherwise)
  • --iv: encryption IV to be used (randomly generated otherwise)
  • --enc-var-prefix: prefix of the names of the encryption key/IV environment variables (randomly generated otherwise)

on '-d', '--decrypt', 'decrypt the file instead of encrypting it, requires key and iv'
on '-f', '--force', 'override output file if it exists'
on '-p', '--print-key', 'print (possibly generated) key and iv'
Expand Down