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

Improve CLI program #681

Closed
6 tasks done
TheZ3ro opened this issue Jun 26, 2017 · 32 comments
Closed
6 tasks done

Improve CLI program #681

TheZ3ro opened this issue Jun 26, 2017 · 32 comments
Assignees
Milestone

Comments

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Jun 26, 2017

Listing here as a reminder some things that need to be improved in the cli program:

  • loading keyfile from command line argument
  • listing all the info about an entry (not only the password)
  • adding a linux man page
  • add entry manipulation commands (add, rm, edit, etc)
  • add clipboard clearing functionality.
  • add locate command
@TheZ3ro TheZ3ro changed the title Improving CLI program Improve CLI program Jun 26, 2017
@magkopian
Copy link
Contributor

Regarding the man page, I have already put the effort to write one for my package. You can find it here.

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Jun 26, 2017

Wow! Great work @magkopian

@droidmonkey
Copy link
Member

Also investigate use of cli from snap and appimage. It might just be easiest to deploy a separate snap entirely.

@phoerious
Copy link
Member

I was rather thinking about a CLI option for the main program which does an exec keepassxc-cli.

@olifre
Copy link

olifre commented Jun 28, 2017

Other things that would be helpful:

  • show does not support -g which means it can currently not be used with keyfiles.
  • optionally caching the password-tree (only structure, not passwords) would allow to offer shell-completion as e.g. pass does. Otherwise, manual usage is pretty cumbersome.
  • clip currently does not work for me - also, I guess clip does not yet auto-clear the clipboard after a while.

Apart from that, I would say loading keyfile from command line argument is the most crucial thing for me.
Also, it would be cool to have keepassxc-cli default to using the last DB and keyfile if no DB and keyfile are passed as parameters.

@louib
Copy link
Member

louib commented Jun 28, 2017

Thanks for the input @olifre !

show does not support -g which means it can currently not be used with keyfiles.

I'll make sure all the top-level commands have the -g available on the next iteration.

optionally caching the password-tree (only structure, not passwords) would allow to offer shell-completion as e.g. pass does. Otherwise, manual usage is pretty cumbersome.

I'm currently working on an interactive shell with readline support, with autocompletion. You can look at this PR for more details.

clip currently does not work for me - also, I guess clip does not yet auto-clear the clipboard after a while.

Which OS do you use? Right now the Clip command uses the QClipboard class, which expects a running GUI loop, which we do not have in the CLI. This might explain some of the problems you have with the clip command. I am also currently working on this (and on adding the timeout functionality)

@magkopian
Copy link
Contributor

magkopian commented Jun 28, 2017

I think it would also be nice to have a locate command for listing all the group paths, were entries with the same name appear. For example, by typing keepassxc-cli locate google passwords.kdbx to get an output similar to the following,

/work/mail/google
/personal/mail/google
/other/mail/google

This is especially useful, with commands like show and clip that you need to remember the group path in order to reference the right entry, when you have multiple with the same name in different groups. The list command can also be used for finding that information, but only with relatively small databases. Also, the list command lists the entries in a tree like structure, so you can't just copy the path to the entry and paste it to your show or clip command.

Another thing I'd really like, is by using a flag to have the ability to make list output a flattened version of the contents of the database, instead of a tree like one. This would be helpful for doing search operations using grep, counting the number of entries that match a particular pattern by piping the output to wc and all sorts of stuff. For example instead of,

work
    mail
        google
        zoho
    dev
        github
        bugsnag
personal
    mail
        google
    social
        twitter

display something like the following,

/work/mail/google
/work/mail/zoho
/work/dev/github
/work/dev/bugsnag
/personal/mail/google
/personal/social/twitter

@louib
Copy link
Member

louib commented Jun 29, 2017

@magkopian what about
find finds the entries in the db using the name, and returns a flat list of results

/work/mail/google
/personal/mail/google
/other/mail/google

search does a search on the entries fields, reusing the same code used right now by the search bar.

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Jun 29, 2017

I like the list suggestion by @magkopian

@magkopian
Copy link
Contributor

@louib Sorry I didn't know you have already implemented find and search, I guess in that case a locate command isn't really needed.

@louib
Copy link
Member

louib commented Jun 29, 2017

@magkopian they're not implemented yet, that was just a suggestion. They could be added pretty easily after #684 is merged though, if you're willing to jump in!

@magkopian
Copy link
Contributor

@louib Well, in that case, the actual name of the command is not an issue. The functionality is what it matters. I guess the reason why I said locate, was because it reminded me of the locate command on Linux, which looks up to its database in order to locate a file.

@olifre
Copy link

olifre commented Jul 2, 2017

Which OS do you use? Right now the Clip command uses the QClipboard class, which expects a running GUI loop, which we do not have in the CLI. This might explain some of the problems you have with the clip command. I am also currently working on this (and on adding the timeout functionality)

Perfect, thanks a lot for your reply!
I am using Gentoo Linux here. Really looking forward to the things to come ;-).

@louib
Copy link
Member

louib commented Jul 31, 2017

@olifre key files are now supported for all the cli commands, and the clip command uses native programs, so this should solve your problem. You can test this from the develop branch.

@louib
Copy link
Member

louib commented Aug 5, 2017

@magkopian the locate command has been merged into develop, so you can start using/testing it.

I think at that point the 2 most important improvements would be the man page and basic entry manipulation commands (add, edit and remove). I won't have time to work on that in the near future, but if someone is willing to take care of it, I'll gladly help with code review and testing.

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Aug 6, 2017

Writing the manpage is pretty trivial and we already have a basic manpage in the unofficial debian package.

We should inclide them here in the main repository

@magkopian
Copy link
Contributor

@louib Thanks for letting me know, I've just started working on updating the manpage. By the way, have the --print-uuids and --gui-prompt options been removed? Because I just compiled the latest version from develop and they don't appear to work.

Also, the list command which is now called ls, doesn't appear to list all the entries of the database when the group option is omitted but only the contents of the root group. Is that the expected behavior?

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Aug 6, 2017

Yes --gui-prompt has been removed so now the cli application can run on headless server/os.

@magkopian
Copy link
Contributor

Ok, you can now find the updated version of the manpage here. Hopefully I haven't missed anything.

@louib
Copy link
Member

louib commented Aug 6, 2017

@magkopian the --print-uuids has indeed been removed (I was using it before I had functions to find entries by path / name) and yes, now the ls command only lists the top level elements if no group is specified.

Thanks for the contribution, the manpage appears to be up to date!

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Aug 12, 2017

@louib in your next cli PR, can you include the manpage? Thanks ;)

@edubxb
Copy link

edubxb commented Aug 27, 2017

I will really appreciate the ability to list all database entries flattened, like @magkopian suggests in his comment #681 (comment).

I'm trying to integrate the cli with fzf and currently is impossible because the tree format of the list command don't allow to use fzf to search entries.

@isamert
Copy link

isamert commented Sep 1, 2017

An optional password argument might be very useful -- if there isn't any already. A practical use of it may be redirecting the output to relevant field. For example, if I redirect my output to a file, it appends Insert the database password text at top of the file. Optional password argument may be overcome this. I know there is --gui-prompt option but it's not usable in every situation. This will be a generalized, easy solution.

Some example usage:

> keepassxc-cli show passwords.kdbx some_password_uuid --password db_password

@rikusilvola
Copy link

I'd be hesitant to adding a password argument, but would suggest supporting a password file instead.
When the password is passed as an argument, it is visible on the command history, and on the list of running processes. However, a password could be stored in a secured file instead.
e.g.

> keepassxc-cli show passwords.kdbx some_password_uuid --pwfile ./db_password

@TheZ3ro
Copy link
Contributor Author

TheZ3ro commented Sep 1, 2017

Password must be passed to stdin, I don't think a --password argoment is a suitable/secure option

@louib
Copy link
Member

louib commented Sep 1, 2017

@isamert would #831 fix your redirection issue?

@isamert
Copy link

isamert commented Sep 1, 2017

#831 seems like a good solution for my situation.

Adding a --password argument may seem insecure but it probably have some uses, like some automated personal scripts, doesn't really matter what it is. Other than that, a CLI user is probably aware of the security issues that mentioned here, so giving them this flexibility is not real security problem it's a matter of choice. I'm not insisting about this feature but a having a flexible CLI tool may help in general. And AFAIK KeePass CLI has this kind of thing.

@louib
Copy link
Member

louib commented Sep 6, 2017

@magkopian @edubxb created #925 to track your request for a flattened list of entries.

@edubxb
Copy link

edubxb commented Sep 6, 2017

@louib thanks!

@louib
Copy link
Member

louib commented Sep 9, 2017

Closing this one. Most of the points were addressed, and the main one remaining is tracked in another issue (#668).

@louib louib closed this as completed Sep 9, 2017
@TheZ3ro TheZ3ro added this to the v2.3.0 milestone Sep 9, 2017
@jagrg
Copy link

jagrg commented Apr 24, 2021

Thank you all for the input. I'm also trying to use keepassxc-cli from a script, but I'm not sure what to do with the password. For example, is it safe to use an encrypted password file? For example:

PASSWORD=$(gpg --decrypt $HOME/.passfile.gpg 2> /dev/null)
echo "$PASSWORD" | keepassxc-cli ls -q -R -f /path/to/db -k /path/to/keyfile | fzf

And is it safe/safer to use read with -s (no echo) option instead? For example:

IFS= read -s -p "Password: " PASSWORD
echo "$PASSWORD" | keepassxc-cli ls -q -R -f /path/to/db -k /path/to/keyfile | fzf

@droidmonkey
Copy link
Member

This PR is 4 years old. Make a new discussion topic if you need guidance please.

@keepassxreboot keepassxreboot locked and limited conversation to collaborators Apr 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants