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

Add capability to generate secret values #7

Merged
merged 2 commits into from
Aug 18, 2021

Conversation

sk3w
Copy link
Contributor

@sk3w sk3w commented Aug 17, 2021

I added a quick feature to automatically generate new secret values and add them to the config. Is this something you'd find useful?

> amber generate KEY1
Your new secret value is KEY1: awUHt_Z9UKmYzDp2HKPz8A==

❯ cat amber.yaml
---
file_format_version: 1
public_key: <redacted>
secrets:
  - name: KEY1
    sha256: dba4b63c27dc8827b465594894f6e5c6901a75799ca7b5c00c1b7740128237a8
    cipher: <redacted>

Copy link
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

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

Looks like a great addition, thanks! Can you also include a line in the changelog about the new subcommand?

@@ -56,6 +57,15 @@ fn encrypt(opt: cli::Opt, key: String, value: String) -> Result<()> {
config.save(&opt.amber_yaml)
}

fn generate(opt: cli::Opt, key: String) -> Result<()> {
let value = sodiumoxide::randombytes::randombytes(16);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should include a command line flag to control how many bytes of entropy are included here. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was leaning towards the hardcoded value for simplicity and footgun-avoidance, but I guess there is a potential use-case when secrets of a speicific size are required? (Though the cases I can think of at the moment would also have their own preferred tools for generating & formatting the required values.)

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, we can leave as-is until there's a specific requirement.

Copy link
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

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

Thanks!

@snoyberg snoyberg merged commit 465610e into fpco:main Aug 18, 2021
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.

2 participants