182 lines
6.2 KiB
Diff
182 lines
6.2 KiB
Diff
|
From 027cbef223616f5ab6c73e60bcaa9f9e81a6ce67 Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Daniel Mach <daniel.mach@suse.com>
|
|||
|
|
Date: Wed, 28 Jun 2023 16:39:42 +0200
|
|||
|
|
Subject: [PATCH] Fix utf8 handling in 'pass' renderer and make it more
|
|||
|
|
robust (#579)
|
|||
|
|
|
|||
|
|
* Migrate string formatting in 'pass' renderer to a f-string
|
|||
|
|
|
|||
|
|
* Fix utf8 handling in 'pass' renderer and make it more robust
|
|||
|
|
---
|
|||
|
|
changelog/64300.fixed.md | 1 +
|
|||
|
|
salt/renderers/pass.py | 12 +--
|
|||
|
|
tests/pytests/unit/renderers/test_pass.py | 99 +++++++++++++++++++++++
|
|||
|
|
3 files changed, 103 insertions(+), 9 deletions(-)
|
|||
|
|
create mode 100644 changelog/64300.fixed.md
|
|||
|
|
|
|||
|
|
diff --git a/changelog/64300.fixed.md b/changelog/64300.fixed.md
|
|||
|
|
new file mode 100644
|
|||
|
|
index 0000000000..4418db1d04
|
|||
|
|
--- /dev/null
|
|||
|
|
+++ b/changelog/64300.fixed.md
|
|||
|
|
@@ -0,0 +1 @@
|
|||
|
|
+Fix utf8 handling in 'pass' renderer
|
|||
|
|
diff --git a/salt/renderers/pass.py b/salt/renderers/pass.py
|
|||
|
|
index ba0f152c23..ae75bba443 100644
|
|||
|
|
--- a/salt/renderers/pass.py
|
|||
|
|
+++ b/salt/renderers/pass.py
|
|||
|
|
@@ -145,23 +145,17 @@ def _fetch_secret(pass_path):
|
|||
|
|
env["GNUPGHOME"] = pass_gnupghome
|
|||
|
|
|
|||
|
|
try:
|
|||
|
|
- proc = Popen(cmd, stdout=PIPE, stderr=PIPE, env=env)
|
|||
|
|
+ proc = Popen(cmd, stdout=PIPE, stderr=PIPE, env=env, encoding="utf-8")
|
|||
|
|
pass_data, pass_error = proc.communicate()
|
|||
|
|
pass_returncode = proc.returncode
|
|||
|
|
- except OSError as e:
|
|||
|
|
+ except (OSError, UnicodeDecodeError) as e:
|
|||
|
|
pass_data, pass_error = "", str(e)
|
|||
|
|
pass_returncode = 1
|
|||
|
|
|
|||
|
|
# The version of pass used during development sent output to
|
|||
|
|
# stdout instead of stderr even though its returncode was non zero.
|
|||
|
|
if pass_returncode or not pass_data:
|
|||
|
|
- try:
|
|||
|
|
- pass_error = pass_error.decode("utf-8")
|
|||
|
|
- except (AttributeError, ValueError):
|
|||
|
|
- pass
|
|||
|
|
- msg = "Could not fetch secret '{}' from the password store: {}".format(
|
|||
|
|
- pass_path, pass_error
|
|||
|
|
- )
|
|||
|
|
+ msg = f"Could not fetch secret '{pass_path}' from the password store: {pass_error}"
|
|||
|
|
if pass_strict_fetch:
|
|||
|
|
raise SaltRenderError(msg)
|
|||
|
|
else:
|
|||
|
|
diff --git a/tests/pytests/unit/renderers/test_pass.py b/tests/pytests/unit/renderers/test_pass.py
|
|||
|
|
index 1e2ebb7ea8..f7c79e1fe1 100644
|
|||
|
|
--- a/tests/pytests/unit/renderers/test_pass.py
|
|||
|
|
+++ b/tests/pytests/unit/renderers/test_pass.py
|
|||
|
|
@@ -1,8 +1,12 @@
|
|||
|
|
import importlib
|
|||
|
|
+import os
|
|||
|
|
+import shutil
|
|||
|
|
+import tempfile
|
|||
|
|
|
|||
|
|
import pytest
|
|||
|
|
|
|||
|
|
import salt.exceptions
|
|||
|
|
+import salt.utils.files
|
|||
|
|
from tests.support.mock import MagicMock, patch
|
|||
|
|
|
|||
|
|
# "pass" is a reserved keyword, we need to import it differently
|
|||
|
|
@@ -19,6 +23,47 @@ def configure_loader_modules(master_opts):
|
|||
|
|
}
|
|||
|
|
|
|||
|
|
|
|||
|
|
+@pytest.fixture()
|
|||
|
|
+def pass_executable(request):
|
|||
|
|
+ tmp_dir = tempfile.mkdtemp(prefix="salt_pass_")
|
|||
|
|
+ pass_path = os.path.join(tmp_dir, "pass")
|
|||
|
|
+ with salt.utils.files.fopen(pass_path, "w") as f:
|
|||
|
|
+ f.write("#!/bin/sh\n")
|
|||
|
|
+ # return path path wrapped into unicode characters
|
|||
|
|
+ # pass args ($1, $2) are ("show", <pass_path>)
|
|||
|
|
+ f.write('echo "α>>> $2 <<<β"\n')
|
|||
|
|
+ os.chmod(pass_path, 0o755)
|
|||
|
|
+ yield pass_path
|
|||
|
|
+ shutil.rmtree(tmp_dir)
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
+@pytest.fixture()
|
|||
|
|
+def pass_executable_error(request):
|
|||
|
|
+ tmp_dir = tempfile.mkdtemp(prefix="salt_pass_")
|
|||
|
|
+ pass_path = os.path.join(tmp_dir, "pass")
|
|||
|
|
+ with salt.utils.files.fopen(pass_path, "w") as f:
|
|||
|
|
+ f.write("#!/bin/sh\n")
|
|||
|
|
+ # return error message with unicode characters
|
|||
|
|
+ f.write('echo "ERROR: αβγ" >&2\n')
|
|||
|
|
+ f.write("exit 1\n")
|
|||
|
|
+ os.chmod(pass_path, 0o755)
|
|||
|
|
+ yield pass_path
|
|||
|
|
+ shutil.rmtree(tmp_dir)
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
+@pytest.fixture()
|
|||
|
|
+def pass_executable_invalid_utf8(request):
|
|||
|
|
+ tmp_dir = tempfile.mkdtemp(prefix="salt_pass_")
|
|||
|
|
+ pass_path = os.path.join(tmp_dir, "pass")
|
|||
|
|
+ with salt.utils.files.fopen(pass_path, "wb") as f:
|
|||
|
|
+ f.write(b"#!/bin/sh\n")
|
|||
|
|
+ # return invalid utf-8 sequence
|
|||
|
|
+ f.write(b'echo "\x80\x81"\n')
|
|||
|
|
+ os.chmod(pass_path, 0o755)
|
|||
|
|
+ yield pass_path
|
|||
|
|
+ shutil.rmtree(tmp_dir)
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
# The default behavior is that if fetching a secret from pass fails,
|
|||
|
|
# the value is passed through. Even the trailing newlines are preserved.
|
|||
|
|
def test_passthrough():
|
|||
|
|
@@ -161,3 +206,57 @@ def test_env():
|
|||
|
|
call_args, call_kwargs = popen_mock.call_args_list[0]
|
|||
|
|
assert call_kwargs["env"]["GNUPGHOME"] == config["pass_gnupghome"]
|
|||
|
|
assert call_kwargs["env"]["PASSWORD_STORE_DIR"] == config["pass_dir"]
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
+@pytest.mark.skip_on_windows(reason="Not supported on Windows")
|
|||
|
|
+def test_utf8(pass_executable):
|
|||
|
|
+ config = {
|
|||
|
|
+ "pass_variable_prefix": "pass:",
|
|||
|
|
+ "pass_strict_fetch": True,
|
|||
|
|
+ }
|
|||
|
|
+ mocks = {
|
|||
|
|
+ "_get_pass_exec": MagicMock(return_value=pass_executable),
|
|||
|
|
+ }
|
|||
|
|
+
|
|||
|
|
+ pass_path = "pass:secret"
|
|||
|
|
+ with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks):
|
|||
|
|
+ result = pass_.render(pass_path)
|
|||
|
|
+ assert result == "α>>> secret <<<β"
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
+@pytest.mark.skip_on_windows(reason="Not supported on Windows")
|
|||
|
|
+def test_utf8_error(pass_executable_error):
|
|||
|
|
+ config = {
|
|||
|
|
+ "pass_variable_prefix": "pass:",
|
|||
|
|
+ "pass_strict_fetch": True,
|
|||
|
|
+ }
|
|||
|
|
+ mocks = {
|
|||
|
|
+ "_get_pass_exec": MagicMock(return_value=pass_executable_error),
|
|||
|
|
+ }
|
|||
|
|
+
|
|||
|
|
+ pass_path = "pass:secret"
|
|||
|
|
+ with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks):
|
|||
|
|
+ with pytest.raises(
|
|||
|
|
+ salt.exceptions.SaltRenderError,
|
|||
|
|
+ match=r"Could not fetch secret 'secret' from the password store: ERROR: αβγ",
|
|||
|
|
+ ):
|
|||
|
|
+ result = pass_.render(pass_path)
|
|||
|
|
+
|
|||
|
|
+
|
|||
|
|
+@pytest.mark.skip_on_windows(reason="Not supported on Windows")
|
|||
|
|
+def test_invalid_utf8(pass_executable_invalid_utf8):
|
|||
|
|
+ config = {
|
|||
|
|
+ "pass_variable_prefix": "pass:",
|
|||
|
|
+ "pass_strict_fetch": True,
|
|||
|
|
+ }
|
|||
|
|
+ mocks = {
|
|||
|
|
+ "_get_pass_exec": MagicMock(return_value=pass_executable_invalid_utf8),
|
|||
|
|
+ }
|
|||
|
|
+
|
|||
|
|
+ pass_path = "pass:secret"
|
|||
|
|
+ with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks):
|
|||
|
|
+ with pytest.raises(
|
|||
|
|
+ salt.exceptions.SaltRenderError,
|
|||
|
|
+ match=r"Could not fetch secret 'secret' from the password store: 'utf-8' codec can't decode byte 0x80 in position 0: invalid start byte",
|
|||
|
|
+ ):
|
|||
|
|
+ result = pass_.render(pass_path)
|
|||
|
|
--
|
|||
|
|
2.41.0
|
|||
|
|
|
|||
|
|
|