From f7e4d38762df534857817d00d02ed52579322d01 Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Sun, 30 Mar 2025 14:02:56 +0000 Subject: [PATCH 1/8] First Draft --- borgmatic/hooks/credential/keepassxc.py | 26 ++++---- tests/unit/hooks/credential/test_keepassxc.py | 65 +++++++++++++++++++ 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/borgmatic/hooks/credential/keepassxc.py b/borgmatic/hooks/credential/keepassxc.py index 7e6ac9134..1a0da572b 100644 --- a/borgmatic/hooks/credential/keepassxc.py +++ b/borgmatic/hooks/credential/keepassxc.py @@ -11,27 +11,23 @@ def load_credential(hook_config, config, credential_parameters): ''' Given the hook configuration dict, the configuration dict, and a credential parameters tuple containing a KeePassXC database path and an attribute name to load, run keepassxc-cli to fetch - the corresponidng KeePassXC credential and return it. + the corresponding KeePassXC credential and return it. Raise ValueError if keepassxc-cli can't retrieve the credential. ''' try: - (database_path, attribute_name) = credential_parameters + database_path, attribute_name = credential_parameters[:2] + extra_args = credential_parameters[2:] # Handle additional arguments like --key-file or --yubikey except ValueError: - path_and_name = ' '.join(credential_parameters) - - raise ValueError( - f'Cannot load credential with invalid KeePassXC database path and attribute name: "{path_and_name}"' - ) + raise ValueError( f'Invalid KeePassXC credential parameters: {credential_parameters}') expanded_database_path = os.path.expanduser(database_path) if not os.path.exists(expanded_database_path): - raise ValueError( - f'Cannot load credential because KeePassXC database path does not exist: {database_path}' - ) + raise ValueError( f'KeePassXC database path does not exist: {database_path}') - return borgmatic.execute.execute_command_and_capture_output( + # Build the keepassxc-cli command + command = ( tuple(shlex.split((hook_config or {}).get('keepassxc_cli_command', 'keepassxc-cli'))) + ( 'show', @@ -41,4 +37,10 @@ def load_credential(hook_config, config, credential_parameters): expanded_database_path, attribute_name, ) - ).rstrip(os.linesep) + + tuple(extra_args) # Append extra arguments + ) + + try: + return borgmatic.execute.execute_command_and_capture_output(command).rstrip(os.linesep) + except Exception as e: + raise ValueError(f'Failed to retrieve credential: {e}') diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index 0c460e233..9fddf1331 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -116,3 +116,68 @@ def test_load_credential_with_expanded_directory_with_present_database_fetches_p ) == 'password' ) + + +def test_load_credential_with_key_file(): + flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return( + 'database.kdbx' + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args( + ( + 'keepassxc-cli', + 'show', + '--show-protected', + '--attributes', + 'Password', + 'database.kdbx', + 'mypassword', + '--key-file', + '/path/to/keyfile', + ) + ).and_return( + 'password' + ).once() + + assert ( + module.load_credential( + hook_config={}, + config={}, + credential_parameters=('database.kdbx', 'mypassword', '--key-file', '/path/to/keyfile'), + ) + == 'password' + ) + + +def test_load_credential_with_yubikey(): + flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return( + 'database.kdbx' + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args( + ( + 'keepassxc-cli', + 'show', + '--show-protected', + '--attributes', + 'Password', + 'database.kdbx', + 'mypassword', + '--yubikey', + ) + ).and_return( + 'password' + ).once() + + assert ( + module.load_credential( + hook_config={}, + config={}, + credential_parameters=('database.kdbx', 'mypassword', '--yubikey'), + ) + == 'password' + ) From 863c95414429f29d1959a15b3cd68ed66ace46a9 Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Sun, 30 Mar 2025 15:57:42 +0000 Subject: [PATCH 2/8] added schema.yaml --- borgmatic/config/schema.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 757820acd..33f0580c1 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2683,6 +2683,16 @@ properties: description: | Command to use instead of "keepassxc-cli". example: /usr/local/bin/keepassxc-cli + key_file: + type: string + description: | + Path to a key file for unlocking the KeePassXC database. + example: /path/to/keyfile + yubikey: + type: boolean + description: | + Whether to use a YubiKey for unlocking the KeePassXC database. + example: true description: | Configuration for integration with the KeePassXC password manager. default_actions: From 92ebc77597fc7504ebfaa1775ef3f603cf8e77e7 Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Sun, 30 Mar 2025 16:19:56 +0000 Subject: [PATCH 3/8] 2nd Draft --- borgmatic/config/schema.yaml | 2 +- tests/unit/hooks/credential/test_keepassxc.py | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 33f0580c1..15c04ee3f 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2683,7 +2683,7 @@ properties: description: | Command to use instead of "keepassxc-cli". example: /usr/local/bin/keepassxc-cli - key_file: + key-file: type: string description: | Path to a key file for unlocking the KeePassXC database. diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index 9fddf1331..c7c2306f1 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -181,3 +181,43 @@ def test_load_credential_with_yubikey(): ) == 'password' ) + + +def test_load_credential_with_key_file_and_yubikey(): + flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return( + 'database.kdbx' + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args( + ( + 'keepassxc-cli', + 'show', + '--show-protected', + '--attributes', + 'Password', + 'database.kdbx', + 'mypassword', + '--key-file', + '/path/to/keyfile', + '--yubikey', + ) + ).and_return( + 'password' + ).once() + + assert ( + module.load_credential( + hook_config={}, + config={}, + credential_parameters=( + 'database.kdbx', + 'mypassword', + '--key-file', + '/path/to/keyfile', + '--yubikey', + ), + ) + == 'password' + ) From 7a0c56878bdc3963b4cc815111bf9ee7ede7032d Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Wed, 2 Apr 2025 10:47:35 +0000 Subject: [PATCH 4/8] Applied changes --- borgmatic/config/schema.yaml | 6 +++--- borgmatic/hooks/credential/keepassxc.py | 14 ++++++++++--- tests/unit/hooks/credential/test_keepassxc.py | 20 ++++++++----------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 15c04ee3f..9c12f6c9f 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2689,10 +2689,10 @@ properties: Path to a key file for unlocking the KeePassXC database. example: /path/to/keyfile yubikey: - type: boolean + type: string description: | - Whether to use a YubiKey for unlocking the KeePassXC database. - example: true + Path or identifier for the YubiKey to use for unlocking the KeePassXC database. + example: /path/to/yubikey description: | Configuration for integration with the KeePassXC password manager. default_actions: diff --git a/borgmatic/hooks/credential/keepassxc.py b/borgmatic/hooks/credential/keepassxc.py index 1a0da572b..0cb9ad12d 100644 --- a/borgmatic/hooks/credential/keepassxc.py +++ b/borgmatic/hooks/credential/keepassxc.py @@ -17,7 +17,6 @@ def load_credential(hook_config, config, credential_parameters): ''' try: database_path, attribute_name = credential_parameters[:2] - extra_args = credential_parameters[2:] # Handle additional arguments like --key-file or --yubikey except ValueError: raise ValueError( f'Invalid KeePassXC credential parameters: {credential_parameters}') @@ -25,7 +24,11 @@ def load_credential(hook_config, config, credential_parameters): if not os.path.exists(expanded_database_path): raise ValueError( f'KeePassXC database path does not exist: {database_path}') - + + # Retrieve key file and Yubikey options from config + key_file = hook_config.get('key_file') + yubikey = hook_config.get('yubikey') + # Build the keepassxc-cli command command = ( tuple(shlex.split((hook_config or {}).get('keepassxc_cli_command', 'keepassxc-cli'))) @@ -37,8 +40,13 @@ def load_credential(hook_config, config, credential_parameters): expanded_database_path, attribute_name, ) - + tuple(extra_args) # Append extra arguments ) + + if key_file: + command += ('--key-file', key_file) + + if yubikey: + command += ('--yubikey', yubikey) try: return borgmatic.execute.execute_command_and_capture_output(command).rstrip(os.linesep) diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index c7c2306f1..8b21cf08a 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -143,9 +143,9 @@ def test_load_credential_with_key_file(): assert ( module.load_credential( - hook_config={}, + hook_config={'key_file': '/path/to/keyfile'}, config={}, - credential_parameters=('database.kdbx', 'mypassword', '--key-file', '/path/to/keyfile'), + credential_parameters=('database.kdbx', 'mypassword'), ) == 'password' ) @@ -168,6 +168,7 @@ def test_load_credential_with_yubikey(): 'database.kdbx', 'mypassword', '--yubikey', + '/path/to/yubikey', ) ).and_return( 'password' @@ -175,9 +176,9 @@ def test_load_credential_with_yubikey(): assert ( module.load_credential( - hook_config={}, + hook_config={'yubikey': '/path/to/yubikey'}, config={}, - credential_parameters=('database.kdbx', 'mypassword', '--yubikey'), + credential_parameters=('database.kdbx', 'mypassword'), ) == 'password' ) @@ -202,6 +203,7 @@ def test_load_credential_with_key_file_and_yubikey(): '--key-file', '/path/to/keyfile', '--yubikey', + '/path/to/yubikey', ) ).and_return( 'password' @@ -209,15 +211,9 @@ def test_load_credential_with_key_file_and_yubikey(): assert ( module.load_credential( - hook_config={}, + hook_config={'key_file': '/path/to/keyfile', 'yubikey': '/path/to/yubikey'}, config={}, - credential_parameters=( - 'database.kdbx', - 'mypassword', - '--key-file', - '/path/to/keyfile', - '--yubikey', - ), + credential_parameters=('database.kdbx', 'mypassword'), ) == 'password' ) From 96ec66de799191bbf09cb1ab5dc5c5578d89bd4f Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Wed, 2 Apr 2025 10:50:25 +0000 Subject: [PATCH 5/8] Applied changes --- borgmatic/config/schema.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 9c12f6c9f..85fcb9d0e 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2683,7 +2683,7 @@ properties: description: | Command to use instead of "keepassxc-cli". example: /usr/local/bin/keepassxc-cli - key-file: + key_file: type: string description: | Path to a key file for unlocking the KeePassXC database. From 4e555472353b0eee7e97494947f6e6d833745126 Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Wed, 2 Apr 2025 15:35:12 +0000 Subject: [PATCH 6/8] Command Restructuring --- borgmatic/hooks/credential/keepassxc.py | 23 ++++--------------- tests/unit/hooks/credential/test_keepassxc.py | 14 +++++------ 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/borgmatic/hooks/credential/keepassxc.py b/borgmatic/hooks/credential/keepassxc.py index 0cb9ad12d..e3799da96 100644 --- a/borgmatic/hooks/credential/keepassxc.py +++ b/borgmatic/hooks/credential/keepassxc.py @@ -25,29 +25,16 @@ def load_credential(hook_config, config, credential_parameters): if not os.path.exists(expanded_database_path): raise ValueError( f'KeePassXC database path does not exist: {database_path}') - # Retrieve key file and Yubikey options from config - key_file = hook_config.get('key_file') - yubikey = hook_config.get('yubikey') - + # Build the keepassxc-cli command command = ( tuple(shlex.split((hook_config or {}).get('keepassxc_cli_command', 'keepassxc-cli'))) - + ( - 'show', - '--show-protected', - '--attributes', - 'Password', - expanded_database_path, - attribute_name, - ) + + ('show', '--show-protected', '--attributes', 'Password') + + (('--key-file', hook_config['key_file']) if 'key_file' in hook_config else ()) + + (('--yubikey', hook_config['yubikey']) if 'yubikey' in hook_config else ()) + + (expanded_database_path, attribute_name) # Ensure database & entry are last ) - if key_file: - command += ('--key-file', key_file) - - if yubikey: - command += ('--yubikey', yubikey) - try: return borgmatic.execute.execute_command_and_capture_output(command).rstrip(os.linesep) except Exception as e: diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index 8b21cf08a..b3d013667 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -132,10 +132,10 @@ def test_load_credential_with_key_file(): '--show-protected', '--attributes', 'Password', - 'database.kdbx', - 'mypassword', '--key-file', '/path/to/keyfile', + 'database.kdbx', + 'mypassword', ) ).and_return( 'password' @@ -165,10 +165,10 @@ def test_load_credential_with_yubikey(): '--show-protected', '--attributes', 'Password', - 'database.kdbx', - 'mypassword', '--yubikey', '/path/to/yubikey', + 'database.kdbx', + 'mypassword', ) ).and_return( 'password' @@ -198,12 +198,12 @@ def test_load_credential_with_key_file_and_yubikey(): '--show-protected', '--attributes', 'Password', - 'database.kdbx', - 'mypassword', '--key-file', '/path/to/keyfile', '--yubikey', '/path/to/yubikey', + 'database.kdbx', + 'mypassword', ) ).and_return( 'password' @@ -216,4 +216,4 @@ def test_load_credential_with_key_file_and_yubikey(): credential_parameters=('database.kdbx', 'mypassword'), ) == 'password' - ) + ) \ No newline at end of file From 248999c23effc077d703f203880994030a1193b4 Mon Sep 17 00:00:00 2001 From: Gautam Aggarwal Date: Thu, 3 Apr 2025 17:10:52 +0000 Subject: [PATCH 7/8] Final --- borgmatic/config/schema.yaml | 8 ++++++-- borgmatic/hooks/credential/keepassxc.py | 2 +- tests/unit/hooks/credential/test_keepassxc.py | 8 ++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 85fcb9d0e..991b072dc 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2691,8 +2691,12 @@ properties: yubikey: type: string description: | - Path or identifier for the YubiKey to use for unlocking the KeePassXC database. - example: /path/to/yubikey + YubiKey slot and optional serial number used to access the KeePassXC database. + Format: "", where: + - is the YubiKey slot number (e.g., `1` or `2`). + - (optional) is the YubiKey's serial number (e.g., `1:7370001`). + example: "1:7370001" + description: | Configuration for integration with the KeePassXC password manager. default_actions: diff --git a/borgmatic/hooks/credential/keepassxc.py b/borgmatic/hooks/credential/keepassxc.py index e3799da96..acaeb449f 100644 --- a/borgmatic/hooks/credential/keepassxc.py +++ b/borgmatic/hooks/credential/keepassxc.py @@ -16,7 +16,7 @@ def load_credential(hook_config, config, credential_parameters): Raise ValueError if keepassxc-cli can't retrieve the credential. ''' try: - database_path, attribute_name = credential_parameters[:2] + (database_path, attribute_name) = credential_parameters except ValueError: raise ValueError( f'Invalid KeePassXC credential parameters: {credential_parameters}') diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index b3d013667..d34b72325 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -166,7 +166,7 @@ def test_load_credential_with_yubikey(): '--attributes', 'Password', '--yubikey', - '/path/to/yubikey', + '1:7370001', 'database.kdbx', 'mypassword', ) @@ -176,7 +176,7 @@ def test_load_credential_with_yubikey(): assert ( module.load_credential( - hook_config={'yubikey': '/path/to/yubikey'}, + hook_config={'yubikey': '1:7370001'}, config={}, credential_parameters=('database.kdbx', 'mypassword'), ) @@ -201,7 +201,7 @@ def test_load_credential_with_key_file_and_yubikey(): '--key-file', '/path/to/keyfile', '--yubikey', - '/path/to/yubikey', + '2', 'database.kdbx', 'mypassword', ) @@ -211,7 +211,7 @@ def test_load_credential_with_key_file_and_yubikey(): assert ( module.load_credential( - hook_config={'key_file': '/path/to/keyfile', 'yubikey': '/path/to/yubikey'}, + hook_config={'key_file': '/path/to/keyfile', 'yubikey': '2'}, config={}, credential_parameters=('database.kdbx', 'mypassword'), ) From e8542f361388b635378fc6cee225346ea636a7b2 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 3 Apr 2025 11:41:58 -0700 Subject: [PATCH 8/8] Fix KeePassXC error when "keepassxc:" option is not present, add new options to NEWS (#1047). --- NEWS | 1 + borgmatic/config/schema.yaml | 10 +++--- borgmatic/hooks/credential/keepassxc.py | 32 +++++++++++-------- tests/unit/hooks/credential/test_keepassxc.py | 4 +-- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 817dc8b5d..7cbaa6793 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,7 @@ "working_directory" are used. * #1044: Fix an error in the systemd credential hook when the credential name contains a "." character. + * #1047: Add "key-file" and "yubikey" options to the KeePassXC credential hook. * #1048: Fix a "no such file or directory" error in ZFS, Btrfs, and LVM hooks with nested directories that reside on separate devices/filesystems. * #1050: Fix a failure in the "spot" check when the archive contains a symlink. diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 991b072dc..98923e905 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -2691,12 +2691,12 @@ properties: yubikey: type: string description: | - YubiKey slot and optional serial number used to access the KeePassXC database. - Format: "", where: - - is the YubiKey slot number (e.g., `1` or `2`). - - (optional) is the YubiKey's serial number (e.g., `1:7370001`). + YubiKey slot and optional serial number used to access the + KeePassXC database. The format is "", where: + * is the YubiKey slot number (e.g., `1` or `2`). + * (optional) is the YubiKey's serial number (e.g., + `7370001`). example: "1:7370001" - description: | Configuration for integration with the KeePassXC password manager. default_actions: diff --git a/borgmatic/hooks/credential/keepassxc.py b/borgmatic/hooks/credential/keepassxc.py index acaeb449f..c3605fcfc 100644 --- a/borgmatic/hooks/credential/keepassxc.py +++ b/borgmatic/hooks/credential/keepassxc.py @@ -18,24 +18,28 @@ def load_credential(hook_config, config, credential_parameters): try: (database_path, attribute_name) = credential_parameters except ValueError: - raise ValueError( f'Invalid KeePassXC credential parameters: {credential_parameters}') + raise ValueError(f'Invalid KeePassXC credential parameters: {credential_parameters}') expanded_database_path = os.path.expanduser(database_path) if not os.path.exists(expanded_database_path): - raise ValueError( f'KeePassXC database path does not exist: {database_path}') - - - # Build the keepassxc-cli command + raise ValueError(f'KeePassXC database path does not exist: {database_path}') + + # Build the keepassxc-cli command. command = ( tuple(shlex.split((hook_config or {}).get('keepassxc_cli_command', 'keepassxc-cli'))) - + ('show', '--show-protected', '--attributes', 'Password') - + (('--key-file', hook_config['key_file']) if 'key_file' in hook_config else ()) - + (('--yubikey', hook_config['yubikey']) if 'yubikey' in hook_config else ()) - + (expanded_database_path, attribute_name) # Ensure database & entry are last + + ('show', '--show-protected', '--attributes', 'Password') + + ( + ('--key-file', hook_config['key_file']) + if hook_config and hook_config.get('key_file') + else () + ) + + ( + ('--yubikey', hook_config['yubikey']) + if hook_config and hook_config.get('yubikey') + else () + ) + + (expanded_database_path, attribute_name) # Ensure database and entry are last. ) - - try: - return borgmatic.execute.execute_command_and_capture_output(command).rstrip(os.linesep) - except Exception as e: - raise ValueError(f'Failed to retrieve credential: {e}') + + return borgmatic.execute.execute_command_and_capture_output(command).rstrip(os.linesep) diff --git a/tests/unit/hooks/credential/test_keepassxc.py b/tests/unit/hooks/credential/test_keepassxc.py index d34b72325..ccb9173b5 100644 --- a/tests/unit/hooks/credential/test_keepassxc.py +++ b/tests/unit/hooks/credential/test_keepassxc.py @@ -135,7 +135,7 @@ def test_load_credential_with_key_file(): '--key-file', '/path/to/keyfile', 'database.kdbx', - 'mypassword', + 'mypassword', ) ).and_return( 'password' @@ -216,4 +216,4 @@ def test_load_credential_with_key_file_and_yubikey(): credential_parameters=('database.kdbx', 'mypassword'), ) == 'password' - ) \ No newline at end of file + )