From fe86c5c5bfc70bd0f6551914f7bf1b2a60003c9a Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Tue, 5 May 2026 22:16:11 -0700 Subject: [PATCH] Fix: encrypt DB password on legacy YAML import (#31983) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `superset import_datasources` (and the v0 dispatcher path used by `superset import_datasources` for plain-YAML files) imported `databases.sqlalchemy_uri` via the model's raw `import_from_dict`, which uses `setattr` and never extracts the password. The result was a clear-text password sitting in `dbs.sqlalchemy_uri` and a `NULL` `password` column — the opposite of how the web UI stores it. Re-apply the URI through `set_sqlalchemy_uri` after the import so the password is encrypted into the `password` column and the stored URI is masked, matching the web-UI/v1-importer behavior. Co-Authored-By: Claude Opus 4.7 --- superset/commands/dataset/importers/v0.py | 10 +- .../commands/importers/v0/__init__.py | 16 ++++ .../commands/importers/v0/import_test.py | 94 +++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/datasets/commands/importers/v0/__init__.py create mode 100644 tests/unit_tests/datasets/commands/importers/v0/import_test.py diff --git a/superset/commands/dataset/importers/v0.py b/superset/commands/dataset/importers/v0.py index d6f7380cb5d..596905c334e 100644 --- a/superset/commands/dataset/importers/v0.py +++ b/superset/commands/dataset/importers/v0.py @@ -211,7 +211,15 @@ def import_from_dict(data: dict[str, Any], sync: Optional[list[str]] = None) -> if isinstance(data, dict): logger.info("Importing %d %s", len(data.get(DATABASES_KEY, [])), DATABASES_KEY) for database in data.get(DATABASES_KEY, []): - Database.import_from_dict(database, sync=sync) + # Re-apply ``sqlalchemy_uri`` via ``set_sqlalchemy_uri`` so that the + # password is extracted from the URI, encrypted into the ``password`` + # column, and the stored URI is masked — matching the web UI + # behavior. The raw ``setattr`` inside ``import_from_dict`` would + # otherwise leave the password readable in clear text. + sqlalchemy_uri = database.get("sqlalchemy_uri") + imported = Database.import_from_dict(database, sync=sync) + if sqlalchemy_uri is not None: + imported.set_sqlalchemy_uri(sqlalchemy_uri) else: logger.info("Supplied object is not a dictionary.") diff --git a/tests/unit_tests/datasets/commands/importers/v0/__init__.py b/tests/unit_tests/datasets/commands/importers/v0/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/tests/unit_tests/datasets/commands/importers/v0/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/unit_tests/datasets/commands/importers/v0/import_test.py b/tests/unit_tests/datasets/commands/importers/v0/import_test.py new file mode 100644 index 00000000000..d80fa5e253c --- /dev/null +++ b/tests/unit_tests/datasets/commands/importers/v0/import_test.py @@ -0,0 +1,94 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# pylint: disable=unused-argument, import-outside-toplevel, invalid-name + +from sqlalchemy.orm.session import Session + +from superset import db +from superset.constants import PASSWORD_MASK + + +def test_v0_import_from_dict_masks_password(session: Session) -> None: + """ + The legacy YAML import (used by ``superset import_datasources``) must + extract and encrypt the password from ``sqlalchemy_uri`` rather than + storing it as clear text. See GH#31983. + """ + from superset.commands.dataset.importers.v0 import import_from_dict + from superset.models.core import Database + + engine = db.session.get_bind() + Database.metadata.create_all(engine) # pylint: disable=no-member + + data = { + "databases": [ + { + "database_name": "Example", + "sqlalchemy_uri": ( + "postgresql://user:secret-password" + "@db.example.org:5432/superset_data" + ), + "cache_timeout": None, + "expose_in_sqllab": True, + "allow_run_async": False, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "extra": "{}", + } + ] + } + + import_from_dict(data) + + database = ( + db.session.query(Database).filter_by(database_name="Example").one() + ) + # The stored URI must have the password masked, not the clear-text value. + assert "secret-password" not in database.sqlalchemy_uri + assert PASSWORD_MASK in database.sqlalchemy_uri + # The actual password is stored separately in the encrypted column. + assert database.password == "secret-password" # noqa: S105 + + +def test_v0_import_from_dict_no_password(session: Session) -> None: + """ + A URI without a password is imported unchanged. + """ + from superset.commands.dataset.importers.v0 import import_from_dict + from superset.models.core import Database + + engine = db.session.get_bind() + Database.metadata.create_all(engine) # pylint: disable=no-member + + data = { + "databases": [ + { + "database_name": "NoPassword", + "sqlalchemy_uri": "sqlite:///example.db", + "extra": "{}", + } + ] + } + + import_from_dict(data) + + database = ( + db.session.query(Database).filter_by(database_name="NoPassword").one() + ) + assert database.sqlalchemy_uri == "sqlite:///example.db" + assert database.password is None