You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/09/04 22:06:21 UTC
(superset) branch master updated: fix(importers): catch JSONDecodeError in load_configs masked_encrypted_extra merge (#43261)
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new fb12d4ae120 fix(importers): catch JSONDecodeError in load_configs masked_encrypted_extra merge (#43261)
fb12d4ae120 is described below
commit fb12d4ae1201b5258a571f5c5c7c1c732140ac7e
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Sep 4 15:06:14 2026 -0700
fix(importers): catch JSONDecodeError in load_configs masked_encrypted_extra merge (#43261)
Co-authored-by: Claude Opus 4.8 <no...@anthropic.com>
---
superset/commands/importers/v1/utils.py | 14 +++
.../unit_tests/commands/importers/v1/utils_test.py | 102 +++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/superset/commands/importers/v1/utils.py b/superset/commands/importers/v1/utils.py
index bf599ffe4f1..b8272dfafde 100644
--- a/superset/commands/importers/v1/utils.py
+++ b/superset/commands/importers/v1/utils.py
@@ -325,6 +325,20 @@ def load_configs(
)
exc.messages = {file_name: exc.messages}
exceptions.append(exc)
+ except json.JSONDecodeError as exc:
+ # masked_encrypted_extra comes straight from the imported YAML
+ # (before schema validation) and may not be valid JSON. Convert
+ # the raw decode error into a ValidationError so it flows into
+ # the aggregated CommandInvalidError like every other per-file
+ # validation failure, instead of escaping as an opaque 500.
+ logger.error(
+ "Invalid JSON in masked_encrypted_extra for %s: %s",
+ file_name,
+ exc,
+ )
+ exceptions.append(
+ ValidationError({file_name: {"masked_encrypted_extra": [str(exc)]}})
+ )
return configs
diff --git a/tests/unit_tests/commands/importers/v1/utils_test.py b/tests/unit_tests/commands/importers/v1/utils_test.py
index 38e1a470800..ee92395c6ea 100644
--- a/tests/unit_tests/commands/importers/v1/utils_test.py
+++ b/tests/unit_tests/commands/importers/v1/utils_test.py
@@ -166,6 +166,108 @@ class TestLoadYaml:
load_yaml("test.yaml", 'key: "unterminated string')
+class TestLoadConfigs:
+ """
+ load_configs() merges caller-supplied ``encrypted_extra_secrets`` into the
+ ``masked_encrypted_extra`` field of each config, which comes straight from
+ the imported YAML (before schema validation). A malformed value there used
+ to raise a raw simplejson.JSONDecodeError that escaped uncaught (opaque
+ 500); it must instead be collected as a ValidationError like every other
+ per-file failure.
+ """
+
+ @staticmethod
+ def _trivial_schema():
+ from marshmallow import EXCLUDE, Schema
+
+ class TrivialSchema(Schema):
+ class Meta:
+ unknown = EXCLUDE
+
+ return TrivialSchema()
+
+ @patch("superset.commands.importers.v1.utils.db")
+ def test_invalid_json_in_masked_encrypted_extra_is_collected(
+ self, mock_db: object
+ ) -> None:
+ """A non-JSON ``masked_encrypted_extra`` is converted into a
+ ValidationError appended to ``exceptions`` rather than raising."""
+ from marshmallow.exceptions import ValidationError
+
+ from superset.commands.importers.v1.utils import load_configs
+
+ # No existing databases / ssh tunnels in the (mocked) metadata DB.
+ mock_db.session.query.return_value.all.return_value = [] # type: ignore[attr-defined]
+
+ file_name = "databases/db.yaml"
+ contents = {
+ file_name: (
+ "uuid: abc-123\n"
+ "password: secret\n"
+ "masked_encrypted_extra: not valid json\n"
+ )
+ }
+ exceptions: list[ValidationError] = []
+
+ configs = load_configs(
+ contents=contents,
+ schemas={"databases/": self._trivial_schema()},
+ passwords={},
+ exceptions=exceptions,
+ ssh_tunnel_passwords={},
+ ssh_tunnel_private_keys={},
+ ssh_tunnel_priv_key_passwords={},
+ encrypted_extra_secrets={file_name: {"$.foo": "actual_secret"}},
+ )
+
+ # The bad file is not added to configs, and a structured error is
+ # collected instead of a raw JSONDecodeError propagating out.
+ assert file_name not in configs
+ assert len(exceptions) == 1
+ assert isinstance(exceptions[0], ValidationError)
+ assert file_name in exceptions[0].messages
+ assert "masked_encrypted_extra" in exceptions[0].messages[file_name]
+
+ @patch("superset.commands.importers.v1.utils.db")
+ def test_valid_json_in_masked_encrypted_extra_still_merges(
+ self, mock_db: object
+ ) -> None:
+ """Control: valid JSON in ``masked_encrypted_extra`` still has the
+ secrets merged in and produces no exceptions."""
+ from marshmallow.exceptions import ValidationError
+
+ from superset.commands.importers.v1.utils import load_configs
+ from superset.utils import json
+
+ mock_db.session.query.return_value.all.return_value = [] # type: ignore[attr-defined]
+
+ file_name = "databases/db.yaml"
+ contents = {
+ file_name: (
+ "uuid: abc-123\n"
+ "password: secret\n"
+ 'masked_encrypted_extra: \'{"foo": "XXXXXXXXXX"}\'\n'
+ )
+ }
+ exceptions: list[ValidationError] = []
+
+ configs = load_configs(
+ contents=contents,
+ schemas={"databases/": self._trivial_schema()},
+ passwords={},
+ exceptions=exceptions,
+ ssh_tunnel_passwords={},
+ ssh_tunnel_private_keys={},
+ ssh_tunnel_priv_key_passwords={},
+ encrypted_extra_secrets={file_name: {"$.foo": "actual_secret"}},
+ )
+
+ assert exceptions == []
+ assert file_name in configs
+ merged = json.loads(configs[file_name]["masked_encrypted_extra"])
+ assert merged == {"foo": "actual_secret"}
+
+
class TestLoadConfigsNonMappingYaml:
"""A syntactically valid YAML document whose top-level value is a
scalar or list (not a mapping) must be reported as a schema validation