From 569ab1386c2752d80739b74d2b54439b1cc65d1a Mon Sep 17 00:00:00 2001 From: Veinar Date: Sat, 23 Nov 2024 10:04:27 +0100 Subject: [PATCH] Fixed wrong not applied validation without --dry-run flag --- envcloak/cli.py | 87 +++++++++++++++++++-------------------- tests/test_cli.py | 102 +++++++++++++++++++++++----------------------- 2 files changed, 94 insertions(+), 95 deletions(-) diff --git a/envcloak/cli.py b/envcloak/cli.py index 48788cb..33573d5 100644 --- a/envcloak/cli.py +++ b/envcloak/cli.py @@ -14,8 +14,6 @@ validate_salt, ) from envcloak.exceptions import ( - KeyFileNotFoundException, - DirectoryEmptyException, OutputFileExistsException, DiskSpaceException, InvalidSaltException, @@ -30,7 +28,7 @@ def main(): """ EnvCloak: Securely manage encrypted environment variables. """ - pass + # No unnecessary pass here @click.command() @@ -60,25 +58,25 @@ def encrypt(input, directory, output, key_file, dry_run): Encrypt environment variables from a file or all files in a directory. """ try: + # Always perform validation if not input and not directory: raise click.UsageError("You must provide either --input or --directory.") if input and directory: raise click.UsageError( "You must provide either --input or --directory, not both." ) + if input: + check_file_exists(input) + check_permissions(input) + if directory: + check_directory_exists(directory) + check_directory_not_empty(directory) + check_file_exists(key_file) + check_permissions(key_file) + check_output_not_exists(output) + check_disk_space(output, required_space=1024 * 1024) if dry_run: - # Dry-run validation - if input: - check_file_exists(input) - check_permissions(input) - if directory: - check_directory_exists(directory) - check_directory_not_empty(directory) - check_file_exists(key_file) - check_permissions(key_file) - check_output_not_exists(output) - check_disk_space(output, required_space=1024 * 1024) click.echo("Dry-run checks passed successfully.") return @@ -103,8 +101,6 @@ def encrypt(input, directory, output, key_file, dry_run): f"File {file} encrypted -> {output_file} using key {key_file}" ) except ( - KeyFileNotFoundException, - DirectoryEmptyException, OutputFileExistsException, DiskSpaceException, FileEncryptionException, @@ -142,25 +138,25 @@ def decrypt(input, directory, output, key_file, dry_run): Decrypt environment variables from a file or all files in a directory. """ try: + # Always perform validation if not input and not directory: raise click.UsageError("You must provide either --input or --directory.") if input and directory: raise click.UsageError( "You must provide either --input or --directory, not both." ) + if input: + check_file_exists(input) + check_permissions(input) + if directory: + check_directory_exists(directory) + check_directory_not_empty(directory) + check_file_exists(key_file) + check_permissions(key_file) + check_output_not_exists(output) + check_disk_space(output, required_space=1024 * 1024) if dry_run: - # Dry-run validation - if input: - check_file_exists(input) - check_permissions(input) - if directory: - check_directory_exists(directory) - check_directory_not_empty(directory) - check_file_exists(key_file) - check_permissions(key_file) - check_output_not_exists(output) - check_disk_space(output, required_space=1024 * 1024) click.echo("Dry-run checks passed successfully.") return @@ -185,8 +181,6 @@ def decrypt(input, directory, output, key_file, dry_run): f"File {file} decrypted -> {output_file} using key {key_file}" ) except ( - KeyFileNotFoundException, - DirectoryEmptyException, OutputFileExistsException, DiskSpaceException, FileDecryptionException, @@ -209,9 +203,11 @@ def generate_key(output, no_gitignore, dry_run): Generate a new encryption key. """ try: + # Always perform validation + check_output_not_exists(output) + check_disk_space(output, required_space=32) + if dry_run: - check_output_not_exists(output) - check_disk_space(output, required_space=32) click.echo("Dry-run checks passed successfully.") return @@ -245,11 +241,13 @@ def generate_key_from_password(password, salt, output, no_gitignore, dry_run): Derive an encryption key from a password and salt. """ try: + # Always perform validation + check_output_not_exists(output) + check_disk_space(output, required_space=32) + if salt: + validate_salt(salt) + if dry_run: - check_output_not_exists(output) - check_disk_space(output, required_space=32) - if salt: - validate_salt(salt) click.echo("Dry-run checks passed successfully.") return @@ -281,15 +279,17 @@ def rotate_keys(input, old_key_file, new_key_file, output, dry_run): Rotate encryption keys by re-encrypting a file with a new key. """ try: + # Always perform validation + check_file_exists(input) + check_permissions(input) + check_file_exists(old_key_file) + check_permissions(old_key_file) + check_file_exists(new_key_file) + check_permissions(new_key_file) + check_output_not_exists(output) + check_disk_space(output, required_space=1024 * 1024) + if dry_run: - check_file_exists(input) - check_permissions(input) - check_file_exists(old_key_file) - check_permissions(old_key_file) - check_file_exists(new_key_file) - check_permissions(new_key_file) - check_output_not_exists(output) - check_disk_space(output, required_space=1024 * 1024) click.echo("Dry-run checks passed successfully.") return @@ -305,7 +305,6 @@ def rotate_keys(input, old_key_file, new_key_file, output, dry_run): os.remove(temp_decrypted) # Clean up temporary file click.echo(f"Keys rotated for {input} -> {output}") except ( - KeyFileNotFoundException, OutputFileExistsException, DiskSpaceException, FileDecryptionException, diff --git a/tests/test_cli.py b/tests/test_cli.py index 7074d5a..7dedf9d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -74,7 +74,7 @@ def test_encrypt(mock_encrypt_file, runner, isolated_mock_files): Test the `encrypt` CLI command. """ input_file = isolated_mock_files / "variables.env" - encrypted_file = isolated_mock_files / "variables.env.enc" + encrypted_file = isolated_mock_files / "variables.temp.enc" # Use unique temp file key_file = isolated_mock_files / "mykey.key" def mock_encrypt(input_path, output_path, key): @@ -111,6 +111,9 @@ def test_decrypt(mock_decrypt_file, runner, mock_files): """ _, encrypted_file, decrypted_file, key_file = mock_files + # Use a unique temporary output file + temp_decrypted_file = decrypted_file.with_name("variables.temp.decrypted") + def mock_decrypt(input_path, output_path, key): assert os.path.exists(input_path), "Encrypted file does not exist" with open(output_path, "w") as f: @@ -125,7 +128,7 @@ def mock_decrypt(input_path, output_path, key): "--input", str(encrypted_file), "--output", - str(decrypted_file), + str(temp_decrypted_file), "--key-file", str(key_file), ], @@ -134,9 +137,13 @@ def mock_decrypt(input_path, output_path, key): assert result.exit_code == 0 assert "File" in result.output mock_decrypt_file.assert_called_once_with( - str(encrypted_file), str(decrypted_file), key_file.read_bytes() + str(encrypted_file), str(temp_decrypted_file), key_file.read_bytes() ) + # Clean up: Remove temp decrypted file + if temp_decrypted_file.exists(): + temp_decrypted_file.unlink() + @patch("envcloak.cli.add_to_gitignore") @patch("envcloak.cli.generate_key_file") @@ -146,20 +153,19 @@ def test_generate_key_with_gitignore( """ Test the `generate-key` CLI command with default behavior (adds to .gitignore). """ - key_file = test_dir / "random.key" + # Use a unique temporary file for the key + key_file = test_dir / "temp_random.key" - # Simulate CLI behavior result = runner.invoke(main, ["generate-key", "--output", str(key_file)]) - # Assert CLI ran without errors assert result.exit_code == 0 - - # Ensure the `generate_key_file` was called mock_generate_key_file.assert_called_once_with(key_file) - - # Verify `add_to_gitignore` was called mock_add_to_gitignore.assert_called_once_with(key_file.parent, key_file.name) + # Cleanup + if key_file.exists(): + key_file.unlink() + @patch("envcloak.cli.add_to_gitignore") @patch("envcloak.cli.generate_key_file") @@ -169,22 +175,20 @@ def test_generate_key_no_gitignore( """ Test the `generate-key` CLI command with the `--no-gitignore` flag. """ - key_file = test_dir / "random.key" + key_file = test_dir / "temp_random.key" - # Simulate CLI behavior with `--no-gitignore` result = runner.invoke( main, ["generate-key", "--output", str(key_file), "--no-gitignore"] ) - # Assert CLI ran without errors assert result.exit_code == 0 - - # Verify the `generate_key_file` was called correctly mock_generate_key_file.assert_called_once_with(key_file) - - # Ensure `add_to_gitignore` was NOT called mock_add_to_gitignore.assert_not_called() + # Cleanup + if key_file.exists(): + key_file.unlink() + @patch("envcloak.cli.add_to_gitignore") @patch("envcloak.cli.generate_key_from_password_file") @@ -195,12 +199,11 @@ def test_generate_key_from_password_with_gitignore( Test the `generate-key-from-password` CLI command with default behavior (adds to .gitignore). """ _, _, _, key_file = mock_files + temp_key_file = key_file.with_name("temp_password_key.key") # Unique temp file password = "JustGiveItATry" salt = "e3a1c8b0d4f6e2c7a5b9d6f0c3e8f1a2" - # Simulate CLI behavior mock_generate_key_from_password_file.return_value = None - mock_add_to_gitignore.side_effect = lambda parent, name: None result = runner.invoke( main, @@ -211,20 +214,21 @@ def test_generate_key_from_password_with_gitignore( "--salt", salt, "--output", - str(key_file), + str(temp_key_file), ], ) - # Assert CLI ran without errors assert result.exit_code == 0 - - # Verify the `generate_key_from_password_file` was called correctly mock_generate_key_from_password_file.assert_called_once_with( - password, key_file, salt + password, temp_key_file, salt + ) + mock_add_to_gitignore.assert_called_once_with( + temp_key_file.parent, temp_key_file.name ) - # Verify `add_to_gitignore` was called - mock_add_to_gitignore.assert_called_once_with(key_file.parent, key_file.name) + # Cleanup + if temp_key_file.exists(): + temp_key_file.unlink() @patch("envcloak.cli.add_to_gitignore") @@ -236,6 +240,7 @@ def test_generate_key_from_password_no_gitignore( Test the `generate-key-from-password` CLI command with the `--no-gitignore` flag. """ _, _, _, key_file = mock_files + temp_key_file = key_file.with_name("temp_password_key.key") # Unique temp file password = "JustGiveItATry" salt = "e3a1c8b0d4f6e2c7a5b9d6f0c3e8f1a2" @@ -248,21 +253,21 @@ def test_generate_key_from_password_no_gitignore( "--salt", salt, "--output", - str(key_file), + str(temp_key_file), "--no-gitignore", ], ) assert result.exit_code == 0 - - # Check `generate_key_from_password_file` call mock_generate_key_from_password_file.assert_called_once_with( - password, key_file, salt + password, temp_key_file, salt ) - - # Ensure `add_to_gitignore` was NOT called mock_add_to_gitignore.assert_not_called() + # Cleanup + if temp_key_file.exists(): + temp_key_file.unlink() + @patch("envcloak.cli.decrypt_file") @patch("envcloak.cli.encrypt_file") @@ -270,19 +275,18 @@ def test_rotate_keys(mock_encrypt_file, mock_decrypt_file, runner, isolated_mock """ Test the `rotate-keys` CLI command. """ - # Use isolated copies of the mock files encrypted_file = isolated_mock_files / "variables.env.enc" - decrypted_file = isolated_mock_files / "variables.env.decrypted" + temp_decrypted_file = isolated_mock_files / "temp_variables.decrypted" key_file = isolated_mock_files / "mykey.key" - new_key_file = key_file.with_name("newkey.key") - new_key_file.write_bytes(os.urandom(32)) + temp_new_key_file = key_file.with_name("temp_newkey.key") + temp_new_key_file.write_bytes(os.urandom(32)) - tmp_file = str(decrypted_file) + ".tmp" + tmp_file = str(temp_decrypted_file) + ".tmp" def mock_decrypt(input_path, output_path, key): assert os.path.exists(input_path), "Encrypted file does not exist" with open(output_path, "w") as f: - f.write("Decrypted content") # Simulate decrypting the file + f.write("Decrypted content") def mock_encrypt(input_path, output_path, key): assert os.path.exists(input_path), "Decrypted file does not exist" @@ -292,7 +296,6 @@ def mock_encrypt(input_path, output_path, key): mock_decrypt_file.side_effect = mock_decrypt mock_encrypt_file.side_effect = mock_encrypt - # Simulate CLI behavior result = runner.invoke( main, [ @@ -302,28 +305,29 @@ def mock_encrypt(input_path, output_path, key): "--old-key-file", str(key_file), "--new-key-file", - str(new_key_file), + str(temp_new_key_file), "--output", - str(decrypted_file), + str(temp_decrypted_file), ], ) assert result.exit_code == 0 assert "Keys rotated" in result.output - - # Ensure `decrypt_file` was called to create the temporary file mock_decrypt_file.assert_called_once_with( str(encrypted_file), tmp_file, key_file.read_bytes() ) - - # Ensure `encrypt_file` was called with the temporary file mock_encrypt_file.assert_called_once_with( - tmp_file, str(decrypted_file), new_key_file.read_bytes() + tmp_file, str(temp_decrypted_file), temp_new_key_file.read_bytes() ) - # Confirm that the temporary file is deleted by the CLI assert not os.path.exists(tmp_file), f"Temporary file {tmp_file} was not deleted" + # Cleanup + if temp_decrypted_file.exists(): + temp_decrypted_file.unlink() + if temp_new_key_file.exists(): + temp_new_key_file.unlink() + def test_encrypt_with_mixed_input_and_directory(runner, mock_files): """ @@ -333,7 +337,6 @@ def test_encrypt_with_mixed_input_and_directory(runner, mock_files): directory = "mock_directory" output_path = "output_directory" - # Simulate mixed usage of `--input` and `--directory` result = runner.invoke( main, [ @@ -349,7 +352,6 @@ def test_encrypt_with_mixed_input_and_directory(runner, mock_files): ], ) - # Assert the CLI raises an appropriate error assert result.exit_code != 0 assert "You must provide either --input or --directory, not both." in result.output @@ -362,7 +364,6 @@ def test_decrypt_with_mixed_input_and_directory(runner, mock_files): directory = "mock_directory" output_path = "output_directory" - # Simulate mixed usage of `--input` and `--directory` result = runner.invoke( main, [ @@ -378,6 +379,5 @@ def test_decrypt_with_mixed_input_and_directory(runner, mock_files): ], ) - # Assert the CLI raises an appropriate error assert result.exit_code != 0 assert "You must provide either --input or --directory, not both." in result.output