Add support for Enpass - a password manager for secrets #1189
Conversation
lib/kamal/secrets/adapters/enpass.rb
Outdated
| end | ||
|
|
||
| def fetch_secret_titles(secrets) | ||
| secrets.reduce(Set.new) do |acc, secret| |
There was a problem hiding this comment.
Accumulator. Common naming in Elixir. What would you recommend?
There was a problem hiding this comment.
I changed it to secrets_with_passwords
lib/kamal/secrets/adapters/enpass.rb
Outdated
| unparsed_result.split("\n").reduce({}) do |acc, line| | ||
| title = line[/title:\s{1}(\w+)/, 1] | ||
| label = line[/label:\s{1}(.*?)\s{2}/, 1] | ||
| password = line[/password:\s{1}([^"]+)/, 1] |
There was a problem hiding this comment.
This parsing seems very fragile - what about titles with spaces, or labels with two spaces?
I think this is really just a function of Enpass's output format though - it would be nice if it offered JSON.
Maybe we should just ask for each password one by one to avoid ambiguity or misparsing of results? How fast is a single enpasscli pass call?
There was a problem hiding this comment.
Right now they don't offer JSON, but I have brought it up. If it gets added - then I will change the current implementation.
As to getting the password one by one - it won't be a good experience, because there is no concept of a session. So you would have to enter the master password for each item.
There was a problem hiding this comment.
Created a PR for JSON support hazcod/enpass-cli#142
There was a problem hiding this comment.
@djmb The PR is merged and I implemented the logic with JSON in mind.
lib/kamal/secrets/adapters/enpass.rb
Outdated
| secrets_titles = fetch_secret_titles(secrets) | ||
|
|
||
| # Enpass outputs result as stderr, I did not find a way to stub backticks and output to stderr. Open3 did the job. | ||
| _stdout, stderr, status = Open3.capture3("enpass-cli -vault #{account.shellescape} show #{secrets_titles.map(&:shellescape).join(" ")}") |
There was a problem hiding this comment.
There no account here, so we probably:
- Avoid passing an account at all
- Use the from field to specify the vault
The base adapter just appends the from field and secrets together and passes them into fetch_secrets.
That's not going to work here since the separator it uses is a /, so I think it should be ok to replace the fetch method for this adapter and have it pass the from field separately.
We can refactor that to be neater later once all the other new adapter PRs have landed to avoid messing with them.
There was a problem hiding this comment.
Yes, if I could override the fetch, that would be cleaner.
There was a problem hiding this comment.
@djmb The -account option is marked as required in Kamal::Cli::Secrets (option :account, type: :string, required: true). If I want to not pass it at all, I need to make it optional. Is this a good idea to do it, just because of the way Enpass works? For other password managers it does make sense to have it required.
I think it's more practical to treat the Vault path as account for Enpass. What do you think?
Another option is to pass some dummy value as account and just not use it, but I'm also not sure about it.
There was a problem hiding this comment.
I went ahead and make --account optional and passed vault in --from 327d716
I hope the maintainers would like to add support for one more password manager - Enpass. It is a well known app, and I would be thrilled to use it together with Kamal. I also promise to fix any future issues with it.
Given a saved secret like this:
Usage with Kamal would be: