diff --git a/osism/commands/vault.py b/osism/commands/vault.py index 3557a0985..57d7b2ec1 100644 --- a/osism/commands/vault.py +++ b/osism/commands/vault.py @@ -67,6 +67,9 @@ def get_parser(self, prog_name): def take_action(self, parsed_args): path = parsed_args.path + if path is None: + logger.error("No path specified") + return 1 if not os.path.isabs(path): path = os.path.join("/opt/configuration", path) @@ -84,9 +87,7 @@ def take_action(self, parsed_args): return 1 if content.startswith(b"$ANSIBLE_VAULT"): - return subprocess.call( - f"/usr/local/bin/ansible-vault view {path}", shell=True - ) + return subprocess.call(["/usr/local/bin/ansible-vault", "view", path]) logger.warning(f"File is not vault-encrypted, showing plain content: {path}") sys.stdout.write(content.decode("utf-8", errors="replace")) @@ -102,9 +103,12 @@ def get_parser(self, prog_name): def take_action(self, parsed_args): path = parsed_args.path + if path is None: + logger.error("No path specified") + return 1 if not os.path.isabs(path): path = os.path.join("/opt/configuration", path) - subprocess.call(f"/usr/local/bin/ansible-vault decrypt {path}", shell=True) + return subprocess.call(["/usr/local/bin/ansible-vault", "decrypt", path]) # Well-known paths where secrets.yml files are typically found diff --git a/tests/unit/commands/test_vault.py b/tests/unit/commands/test_vault.py index c97caf825..de50a09e7 100644 --- a/tests/unit/commands/test_vault.py +++ b/tests/unit/commands/test_vault.py @@ -26,7 +26,7 @@ def test_view_invokes_ansible_vault_for_encrypted_file(tmp_path): _make_view().take_action(parsed_args) mock_call.assert_called_once_with( - f"/usr/local/bin/ansible-vault view {path}", shell=True + ["/usr/local/bin/ansible-vault", "view", str(path)] ) @@ -59,7 +59,7 @@ def test_view_resolves_relative_path_against_opt_configuration(): expected = "/opt/configuration/environments/openstack/secure.yml" open_mock.assert_called_once_with(expected, "rb") mock_call.assert_called_once_with( - f"/usr/local/bin/ansible-vault view {expected}", shell=True + ["/usr/local/bin/ansible-vault", "view", expected] ) @@ -108,5 +108,63 @@ def test_view_invokes_ansible_vault_for_vault_variants(tmp_path, header): _make_view().take_action(parsed_args) mock_call.assert_called_once_with( - f"/usr/local/bin/ansible-vault view {path}", shell=True + ["/usr/local/bin/ansible-vault", "view", str(path)] ) + + +def test_view_requires_path(loguru_logs): + parser = _make_view().get_parser("test") + parsed_args = parser.parse_args([]) + + with patch("osism.commands.vault.subprocess.call") as mock_call: + rc = _make_view().take_action(parsed_args) + + assert rc == 1 + mock_call.assert_not_called() + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("No path" in r["message"] for r in errors) + + +# --- Decrypt.take_action --- + + +def test_decrypt_requires_path(loguru_logs): + cmd = vault.Decrypt(MagicMock(), MagicMock()) + parser = cmd.get_parser("test") + parsed_args = parser.parse_args([]) + + with patch("osism.commands.vault.subprocess.call") as mock_call: + rc = cmd.take_action(parsed_args) + + assert rc == 1 + mock_call.assert_not_called() + errors = [r for r in loguru_logs if r["level"] == "ERROR"] + assert any("No path" in r["message"] for r in errors) + + +def test_decrypt_invokes_ansible_vault_without_shell(tmp_path): + path = tmp_path / "secrets.yml" + path.write_bytes(b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n") + cmd = vault.Decrypt(MagicMock(), MagicMock()) + parser = cmd.get_parser("test") + parsed_args = parser.parse_args([str(path)]) + + with patch("osism.commands.vault.subprocess.call") as mock_call: + cmd.take_action(parsed_args) + + mock_call.assert_called_once_with( + ["/usr/local/bin/ansible-vault", "decrypt", str(path)] + ) + + +def test_decrypt_propagates_exit_code(tmp_path): + path = tmp_path / "secrets.yml" + path.write_bytes(b"$ANSIBLE_VAULT;1.1;AES256\nciphertext\n") + cmd = vault.Decrypt(MagicMock(), MagicMock()) + parser = cmd.get_parser("test") + parsed_args = parser.parse_args([str(path)]) + + with patch("osism.commands.vault.subprocess.call", return_value=1): + rc = cmd.take_action(parsed_args) + + assert rc == 1