[CLI] Add and Edit attributes of an entry

[olszeww0]

…attributes in cli for command add or edit

Lack of add or edit attribute in the entry

Testing strategy

some password for a new database

read -s PK

create a new database

printf "%s\n%s\n" "$PW" "$PW" | keepassxc-cli db-create -p test.kdbx

create group

printf "%s\n" "$PW" | keepassxc-cli mkdir -q test.kdbx a

create entry 'e1' in group 'a' with empty unprotected attribute 'custom'

printf "%s\n%s\n" "$PW" "$PW" | keepassxc-cli add -q -u test --url 'http://example.net' -p -a custom test.kdbx a/e1

create entry 'e2' in group 'a' with unprotected attribute 'custom' with attribute's value 'custom value'

printf "%s\n%s\n" "$PW" "$PW" | keepassxc-cli add -q -u test --url 'http://example.net' -p -a custom -A $( echo -n "custom value" | base64 -w 0) test.kdbx a/e2

create entry 'e3' in group 'a' with protected attribute 'custom' with attribute's value 'custom value'

printf "%s\n%s\n" "$PW" "$PW" | keepassxc-cli add -q -u test --url 'http://example.net' -p -a custom -A $( echo -n "custom value" | base64 -w 0) -P test.kdbx a/e3

create empty unprotected attribute 'custom2' in a/e1

printf "%s\n" "$PW" | keepassxc-cli edit -q -a custom2 test.kdbx a/e1

create empty protected attribute 'custom3' in a/e1

printf "%s\n" "$PW" | keepassxc-cli edit -q -a custom3 -P test.kdbx a/e1

set attribute 'custom3' in a/e1 to be unprotected

printf "%s\n" "$PW" | keepassxc-cli edit -q -a custom3 --unprotect test.kdbx a/e1

set value of attribute 'custom3' in a/e1

printf "%s\n" "$PW" | keepassxc-cli edit -q -a custom3 -A $( echo -n "custom value" | base64 -w 0) test.kdbx a/e1

show things

printf "%s\n" "$PW" | keepassxc-cli ls -q test.kdbx a printf "%s\n" "$PW" | keepassxc-cli show -q -s -a custom test.kdbx a/e1 printf "%s\n" "$PW" | keepassxc-cli show -q -s -a custom test.kdbx a/e2 printf "%s\n" "$PW" | keepassxc-cli show -q -s -a custom test.kdbx a/e3 printf "%s\n" "$PW" | keepassxc-cli show -q -a custom2 test.kdbx a/e1 printf "%s\n" "$PW" | keepassxc-cli show -q -a custom3 test.kdbx a/e1 In edit command if atribute exists and some option like -P,--unprotect or -A is not used, appropriately attribute's protection and attribute's value remain unchanged. So It is possible to use -a and any option from ( -A, -P, --unprotect ) in any combination. Add command hasn't option --unprotect, because default action is unprotect - you only set protection with -P option. Of course, -P and --unprotect cann't bu used together. Only one attribute can be added or edited at once. If you have more than one attributes and entry does not exists, you can(not must) add first with add command, then use edit command to add rest. If entry exist, use edit command to add all attributes.

[codecov-commenter]

Codecov Report

Merging #7462 (87d25de) into develop (4f07103) will decrease coverage by 0.03%. The diff coverage is 38.00%.

@@             Coverage Diff             @@
##           develop    #7462      +/-   ##
===========================================
- Coverage    64.29%   64.26%   -0.03%     
===========================================
  Files          339      339              
  Lines        43364    43413      +49     
===========================================
+ Hits         27879    27898      +19     
- Misses       15485    15515      +30     

Impacted Files	Coverage Δ
src/cli/Add.cpp	75.82% <25.00%> (-14.32%)	:arrow_down:
src/cli/Edit.cpp	78.63% <46.67%> (-11.14%)	:arrow_down:
src/core/FileWatcher.cpp	86.75% <0.00%> (+1.20%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4f07103...87d25de. Read the comment docs.

[louib]

@olszeww0 thanks for the PR.

From what I understand, only 1 attribute can be added or edited at a time. Is that correct?

Regarding the testing strategy, all the cases you outlined should be translated into unit tests. See TestCli.cpp

[olszeww0]

From what I understand, only 1 attribute can be added or edited at a time. Is that correct?

Yes, only 1 attribute can be added or edited at a time.

[droidmonkey]

We should allow more than one, just allow multiple instances of the flag(s).

[olszeww0]

We should allow more than one, just allow multiple instances of the flag(s).

I can see the following disadvantages of this solution:

1. it will complicate in my opinion the present simple code
2. it will be difficult for a common user to use
3. the command length limit in the shell or operating system may quickly be reached

For the advanced user, yes, this can be convenient.

[louib]

@droidmonkey

We should allow more than one, just allow multiple instances of the flag(s).

I'm not sure that would work with the current implementation, because the attribute name is specified separately from the attribute value.

I thought about that a bit more, and I can't think of an elegant solution to address the problem. Maybe we could allow specifying the attribute name and value together, as such:

keepassxc-cli edit -a my-attribute=value passwords.kdbx entry/path

That would allow editing multiple attributes using a single edit invocation. However, I'm not sure how to handle the protected/unprotected option with that implementation. Maybe the --protect option could accept a parameter with a format similar to the -a option. For example:

keepassxc-cli edit -a my-attribute=value --protect my-attribute=true passwords.kdbx entry/path

@olszeww0

the command length limit in the shell or operating system may quickly be reached

Fair point, but I'm not sure this is relevant in our case. I doubt that a significant number of attributes will be edited in a single invocation, and users can always make multiple calls to edit if that's the case. Also, we'll probably have that problem anyway once we allow editing the notes field from the CLI.

[louib]

@olszeww0 I'm planning to implement the attribute-name=value proposition described in my previous comment. I will base my work on your branch so that your contribution is included.

[droidmonkey]

@louib do we still want to merge this one?

[olszeww0]

After about a year of creating this PR, my thinking is that the proposed solution may not be optimal for various reasons.

What do you think about using only one attribute for example "-a" with the following syntax:

keepassxc-cli add|edit -a attribute-name{SEPARATOR1}attribute-value{SEPARATOR2}attribute-flag database.kdbx entry/path

attribute-name - mandatory attribute name attribute-value - optional attribute value as base64 text, default empty attribute-flag - optional attribute flag (protected or unprotected), default unprotected

{SEPARATOR1} and {SEPARATOR2} - always required, to distinguish between the name, value, and protection flag of an attribute. I don't know if {SEPARATOR1} and {SEPARATOR2} should be the same character. However, it seems to me that {SEPARATOR1} should be a character that cannot be used in the attribute name, do you have any suggestions? In the case of {SEPARATOR2} there is no such problem because it is enough that it is outside the range of base64 characters.

Or maybe change the order:

keepassxc-cli add|edit -a attribute-value{SEPARATOR}attribute-flag{SEPARATOR}attribute-name database.kdbx entry/path

and in this case {SEPARATOR} is also always required and it is enough that it is outside the range of base64 characters. What do you think?

I also see that the add and edit options are redundant, so you could only opt for the "edit" option. In this case, if the argument didn't exist, it would be created. However, there is a risk here that a user who would make a mistake in the name of an existing attribute would create a new unwanted attribute. Maybe it would be better to tell the user that the attribute doesn't exist when he thinks it does and return an error. There could be solutions like this:

1. use a different but similar option, e.g. -A

keepassxc-cli edit -A attribute-value{SEPARATOR}attribute-flag{SEPARATOR}attribute-name database.kdbx entry/path

-A means edit existing attribute or create new if it not exists. -a means edit existing attribute, return error if attribute is not exists.

2. expand the meaning of the attribute-flag. If the create flag is used, edit the attribute if it exists, add a new one if it does not exist. If the create flag is not used, return an error if the attribute does not exist, or an edit if it does. Create flag is optional and can be used in combination with protected flag.

[droidmonkey]

Generally speaking, the CLI is not fault tolerant to perceived user errors, I wouldn't worry too much about that. It is not meant for daily/sole use and very much meant for scripted use cases. I like your idea to collapse to just an edit command. If you really wanted to you could add a flag like --onlynew or similar which would prevent overwriting an existing attribute.

Additionally, for the multiple attributes you could just accept a json string which has it's own escaping rules.

keepassxc-cli edit --A "{key1 : \"value1\", key2 : \"value2\"}"

[bricewge]

Would you be so kind to support a way other that passing attributes' value from a CLI option (or a environment variable) , to avoid leaking secret (such as TOTP secrets) in the shell's history or the OS process table?

A solution would be to be able to specify such value from a pipe or a prompt or a file.

[Bonjour123]

Any insight on when it will be available in releases ?

[droidmonkey]

It has some challenges so those need to be resolved first.