From 7980622883f24be8389d1b9056645523f377caa5 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 11 Jul 2025 09:39:20 +1200 Subject: [PATCH 1/3] fix: don't unnecessarily strip valid characters from secret data --- juju/secrets.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/juju/secrets.py b/juju/secrets.py index 5a47f1ef9..0994615c3 100644 --- a/juju/secrets.py +++ b/juju/secrets.py @@ -27,15 +27,10 @@ def create_secret_data(args): """ data = {} for val in args: - # Remove any base64 padding ("=") before splitting the key=value. - stripped = val.rstrip(base64.b64encode(b"=").decode("utf-8")) - idx = stripped.find("=") - if idx < 1: + key, _, value = val.partition('=') + if not key or not value: raise ValueError(f"Invalid key value {val}") - key = stripped[0:idx] - value = stripped[idx + 1 :] - # If the key doesn't have the #file suffix, then add it to the bag and continue. if not key.endswith(file_suffix): data[key] = value From fa79bf48c3ba004af429b747796d8ec32a09488b Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 11 Jul 2025 10:08:39 +1200 Subject: [PATCH 2/3] test: add extra test cases for create_secret_data --- tests/unit/test_secrets.py | 46 ++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py index f22217fd8..5de7e23ba 100644 --- a/tests/unit/test_secrets.py +++ b/tests/unit/test_secrets.py @@ -11,18 +11,29 @@ from juju.secrets import create_secret_data, read_secret_data -class TestCreateSecretData(unittest.TestCase): - def test_bad_key(self): +class TestCreateSecretData: + @pytest.mark.parametrize("keyval", ["foo", "=bar", "baz=", "f=bar", "fo=bar", "foo_bar=baz"]) + def test_bad_key(self, keyval: str): with pytest.raises(ValueError): - create_secret_data(["foo"]) - with pytest.raises(ValueError): - create_secret_data(["=bar"]) + create_secret_data([keyval]) + + + @pytest.mark.parametrize( + "keyval,expected_key,expected_val", + [ + ("foo=bar", "foo", "YmFy"), + ("hello=world", "hello", "d29ybGQ="), + ("goodbye#base64=world", "goodbye", "world"), + ("equalsign==", "equalsign", "PQ=="), + ("equalsign#base64=PQ==", "equalsign", "PQ=="), + ("pq-identity-theorem=P===Q", "pq-identity-theorem", "UD09PVE="), + ] + ) + def test_goo_key_values(self, keyval: str, expected_key: str, expected_val: str): + actual = create_secret_data([keyval]) + expected = {expected_key: expected_val} + assert actual == expected - def test_good_key_values(self): - self.assertDictEqual( - create_secret_data(["foo=bar", "hello=world", "goodbye#base64=world"]), - {"foo": "YmFy", "hello": "d29ybGQ=", "goodbye": "world"}, - ) def test_key_content_too_large(self): with pytest.raises(ValueError): @@ -50,15 +61,12 @@ def test_secret_key_from_file(self): data = create_secret_data(["key1=value1", f"key2#file={file_path}"]) - self.assertDictEqual( - data, - { - "key1": "dmFsdWUx", - "key2": ( - "ICAgICAgICAgIC0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLQogICAgICAgICAgTUlJRllqQ0NBMHFnQXdJQkFnSVFLYVBORDlZZ2dJRzYrak9jZ21wazNEQU5CZ2txaGtpRzl3MEJBUXNGQURBegogICAgICAgICAgTVJ3d0dnWURWUVFLRXhOc2FXNTFlR052Ym5SaGFXNWxjbk11YjNKbk1STXdFUVlEVlFRRERBcDBhVzFBWld4MwogICAgICAgICAgLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQ==" - ), - }, - ) + assert data == { + "key1": "dmFsdWUx", + "key2": ( + "ICAgICAgICAgIC0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLQogICAgICAgICAgTUlJRllqQ0NBMHFnQXdJQkFnSVFLYVBORDlZZ2dJRzYrak9jZ21wazNEQU5CZ2txaGtpRzl3MEJBUXNGQURBegogICAgICAgICAgTVJ3d0dnWURWUVFLRXhOc2FXNTFlR052Ym5SaGFXNWxjbk11YjNKbk1STXdFUVlEVlFRRERBcDBhVzFBWld4MwogICAgICAgICAgLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQ==" + ), + } class TestReadSecretData(unittest.TestCase): From 154c7c21ca3a80c735ec962e32fc0f3cb1f8a37d Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 11 Jul 2025 10:16:26 +1200 Subject: [PATCH 3/3] style: linting --- juju/secrets.py | 2 +- tests/unit/test_secrets.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/juju/secrets.py b/juju/secrets.py index 0994615c3..bbec09b83 100644 --- a/juju/secrets.py +++ b/juju/secrets.py @@ -27,7 +27,7 @@ def create_secret_data(args): """ data = {} for val in args: - key, _, value = val.partition('=') + key, _, value = val.partition("=") if not key or not value: raise ValueError(f"Invalid key value {val}") diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py index 5de7e23ba..7052a1b8c 100644 --- a/tests/unit/test_secrets.py +++ b/tests/unit/test_secrets.py @@ -12,12 +12,13 @@ class TestCreateSecretData: - @pytest.mark.parametrize("keyval", ["foo", "=bar", "baz=", "f=bar", "fo=bar", "foo_bar=baz"]) + @pytest.mark.parametrize( + "keyval", ["foo", "=bar", "baz=", "f=bar", "ff=bar", "foo_bar=baz"] + ) def test_bad_key(self, keyval: str): with pytest.raises(ValueError): create_secret_data([keyval]) - @pytest.mark.parametrize( "keyval,expected_key,expected_val", [ @@ -27,14 +28,13 @@ def test_bad_key(self, keyval: str): ("equalsign==", "equalsign", "PQ=="), ("equalsign#base64=PQ==", "equalsign", "PQ=="), ("pq-identity-theorem=P===Q", "pq-identity-theorem", "UD09PVE="), - ] + ], ) def test_goo_key_values(self, keyval: str, expected_key: str, expected_val: str): actual = create_secret_data([keyval]) expected = {expected_key: expected_val} assert actual == expected - def test_key_content_too_large(self): with pytest.raises(ValueError): create_secret_data(["foo=" + ("a" * 8 * 1024)])