rusackas commented on code in PR #43393:
URL: https://github.com/apache/superset/pull/43393#discussion_r3832248128
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -637,9 +661,12 @@ def load_data(data_uri: str, dataset: SqlaTable, database:
Database) -> None:
handlers.extend([_PeerValidatingHTTPHandler,
_PeerValidatingHTTPSHandler])
opener = request.build_opener(*handlers)
data = opener.open(data_uri) # pylint: disable=consider-using-with #
noqa: S310
+ # Cap the bytes materialized from the download, before and after gzip
+ # decompression (same per-file knob as ZIP bundle uploads).
+ max_bytes = app.config["ZIPPED_FILE_MAX_SIZE"]
if data_uri.endswith(".gz"):
data = gzip.open(data)
- df = pd.read_csv(data, encoding="utf-8")
+ df = pd.read_csv(_read_bounded(data, max_bytes), encoding="utf-8")
Review Comment:
Good catch, fixed. Now bounding the raw compressed download too, before
decompression, not just the decompressed output.
##########
superset/commands/importers/v1/utils.py:
##########
@@ -152,21 +196,47 @@ def load_configs(
try:
config = load_yaml(file_name, content)
+ # Stored secrets are only reusable when the incoming config
+ # still points at the same endpoint as the stored one; a UUID
+ # match alone must never rebind stored credentials to a new
+ # host (see database_connection_identity_unchanged).
+ db_secrets_reusable = (
+ prefix == "databases"
+ and database_connection_identity_unchanged(
+ db_sqlalchemy_uris.get(str(config.get("uuid"))),
+ config.get("sqlalchemy_uri"),
+ )
+ )
+ incoming_tunnel = config.get("ssh_tunnel") or {}
+ stored_tunnel_server = db_ssh_tunnel_servers.get(
+ str(config.get("uuid"))
+ )
Review Comment:
Good catch, fixed. A non-mapping top-level YAML now raises a ValidationError
instead of hitting .get() on it.
##########
superset/commands/database/importers/v1/utils.py:
##########
@@ -37,6 +38,63 @@
logger = logging.getLogger(__name__)
+def _connection_identity_changed(existing: Database, config: dict[str, Any])
-> bool:
+ """Whether the import points the database at a different endpoint."""
+ try:
+ stored = make_url_safe(existing.sqlalchemy_uri)._replace(password=None)
+ incoming =
make_url_safe(config["sqlalchemy_uri"])._replace(password=None)
+ except Exception: # pylint: disable=broad-except
+ # An unparseable URI cannot be compared: treat it as a change so
+ # stored secrets never survive onto it.
+ return True
Review Comment:
Good catch, fixed, though not with the suggested ValueError: make_url_safe()
wraps parse failures in DatabaseInvalidError, so that's what's caught now.
##########
superset/commands/database/importers/v1/utils.py:
##########
@@ -37,6 +38,63 @@
logger = logging.getLogger(__name__)
+def _connection_identity_changed(existing: Database, config: dict[str, Any])
-> bool:
+ """Whether the import points the database at a different endpoint."""
+ try:
+ stored = make_url_safe(existing.sqlalchemy_uri)._replace(password=None)
+ incoming =
make_url_safe(config["sqlalchemy_uri"])._replace(password=None)
+ except Exception: # pylint: disable=broad-except
+ # An unparseable URI cannot be compared: treat it as a change so
+ # stored secrets never survive onto it.
+ return True
+ return stored != incoming
+
+
+def _refuse_stored_secret_reuse(existing: Database, config: dict[str, Any]) ->
None:
+ """
+ Refuse an overwrite that changes the connection endpoint without fresh
+ credentials.
+
+ Database UUIDs are not secrets -- they appear in every exported bundle --
+ so an import must not be able to repoint an existing connection at a new
+ host while the stored password (or SSH tunnel key) is silently kept: the
+ next connection would hand the real credential to the new endpoint.
+ """
+ if _connection_identity_changed(existing, config):
+ try:
+ uri_password = make_url_safe(config["sqlalchemy_uri"]).password
+ except Exception: # pylint: disable=broad-except
+ uri_password = None
Review Comment:
Good catch, fixed, same DatabaseInvalidError as the other one, since that's
what make_url_safe() actually raises.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
