rusackas commented on code in PR #44003:
URL: https://github.com/apache/superset/pull/44003#discussion_r3960099224
##########
superset/commands/database/update.py:
##########
@@ -180,3 +188,50 @@ def validate(self) -> None:
database_name,
):
raise
DatabaseInvalidError(exceptions=[DatabaseExistsValidationError()])
+
+ if self._model:
+ self._check_no_unsafe_secret_rebind()
+
+ def _check_no_unsafe_secret_rebind(self) -> None:
+ """
+ Refuse an update that changes the connection's effective destination
+ (URI host/port, `extra.engine_params`, or the SSH tunnel endpoint)
+ while leaving the corresponding stored secret masked.
+
+ Without this, an editor could silently redirect the real stored
+ password/encrypted_extra/SSH tunnel credential to a different
+ destination -- and since an update persists, every subsequent use of
+ the database (by any user) would send the real secret there, not
+ just the editor's own request.
+ """
+ model = self._model
+ assert model is not None
+
+ connection_identity_changed = False
+ submitted_password: str | None = None
+
+ if "sqlalchemy_uri" in self._properties:
+ submitted_uri = self._properties["sqlalchemy_uri"] or ""
+ connection_identity_changed = uri_identity_changed(
+ model.sqlalchemy_uri, submitted_uri
+ )
+ try:
+ submitted_password = make_url_safe(submitted_uri).password
+ except DatabaseInvalidError:
+ submitted_password = None
+
+ if "extra" in self._properties and engine_params_changed(
+ model.extra, self._properties["extra"]
+ ):
+ connection_identity_changed = True
+
+ if connection_identity_changed and submitted_password in (
+ None,
+ PASSWORD_MASK,
+ ):
+ raise
DatabaseInvalidError(exceptions=[DatabaseUpdateUnsafeRebindError()])
Review Comment:
Applied it (and refined the URI-password / encrypted_extra checks a bit
further, plus fixed the same field_name/ssh_tunnel-gating issues Bito's pass
caught nearby) in 72ccf98. Added regression tests for the false-positive and
the actual reuse gap, both verified to fail without the fix. Thanks for
catching this.
##########
superset/commands/database/update.py:
##########
@@ -180,3 +188,50 @@ def validate(self) -> None:
database_name,
):
raise
DatabaseInvalidError(exceptions=[DatabaseExistsValidationError()])
+
+ if self._model:
+ self._check_no_unsafe_secret_rebind()
+
+ def _check_no_unsafe_secret_rebind(self) -> None:
+ """
+ Refuse an update that changes the connection's effective destination
+ (URI host/port, `extra.engine_params`, or the SSH tunnel endpoint)
+ while leaving the corresponding stored secret masked.
+
+ Without this, an editor could silently redirect the real stored
+ password/encrypted_extra/SSH tunnel credential to a different
+ destination -- and since an update persists, every subsequent use of
+ the database (by any user) would send the real secret there, not
+ just the editor's own request.
+ """
+ model = self._model
+ assert model is not None
+
+ connection_identity_changed = False
+ submitted_password: str | None = None
+
+ if "sqlalchemy_uri" in self._properties:
+ submitted_uri = self._properties["sqlalchemy_uri"] or ""
+ connection_identity_changed = uri_identity_changed(
+ model.sqlalchemy_uri, submitted_uri
+ )
+ try:
+ submitted_password = make_url_safe(submitted_uri).password
+ except DatabaseInvalidError:
+ submitted_password = None
+
+ if "extra" in self._properties and engine_params_changed(
+ model.extra, self._properties["extra"]
+ ):
+ connection_identity_changed = True
+
+ if connection_identity_changed and submitted_password in (
+ None,
+ PASSWORD_MASK,
+ ):
+ raise
DatabaseInvalidError(exceptions=[DatabaseUpdateUnsafeRebindError()])
Review Comment:
Correct catch, this and sha174n's more detailed suggestion on the same lines
are the same underlying gap. Fixed in 72ccf98 (see reply on that thread for
detail).
--
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]
