From f48ec58abe0aad8db0da1b599a1ea52f4fd236a5 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Mon, 22 Jun 2026 16:58:26 -0700 Subject: [PATCH] fix(database): mask SSH tunnel credentials explicitly on read paths The SSH tunnel credential fields (password, private_key, private_key_password) are masked on the write paths (POST/PUT) via mask_password_info(), but the read paths (GET / and GET //connection) attached SSHTunnel.data directly, relying solely on the model property to mask. Apply mask_password_info() on the read paths too so the masking contract is enforced consistently at the API boundary and is robust to future changes in SSHTunnel.data. Adds an integration test asserting the three credential fields are masked on both read paths while the stored values remain intact. Co-Authored-By: Claude Fable 5 --- superset/databases/api.py | 14 +++- .../integration_tests/databases/api_tests.py | 71 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/superset/databases/api.py b/superset/databases/api.py index d6a7b4692dc..abfb99827b2 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -358,7 +358,12 @@ class DatabaseRestApi(BaseSupersetModelRestApi): } try: if database and database.ssh_tunnel: - response["result"]["ssh_tunnel"] = database.ssh_tunnel.data + # Mask credential fields explicitly at the API boundary so read + # responses apply the same masking contract as the write paths + # (POST/PUT), rather than relying solely on SSHTunnel.data. + response["result"]["ssh_tunnel"] = mask_password_info( + database.ssh_tunnel.data + ) return self.response(200, **response) except SupersetException as ex: return self.response(ex.status, message=ex.message) @@ -398,7 +403,12 @@ class DatabaseRestApi(BaseSupersetModelRestApi): database = DatabaseDAO.find_by_id(pk) if database and database.ssh_tunnel: payload = data.json - payload["result"]["ssh_tunnel"] = database.ssh_tunnel.data + # Mask credential fields explicitly at the API boundary so read + # responses apply the same masking contract as the write paths + # (POST/PUT), rather than relying solely on SSHTunnel.data. + payload["result"]["ssh_tunnel"] = mask_password_info( + database.ssh_tunnel.data + ) return payload return data except SupersetException as ex: diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index 03482ecbfd3..e324f70dd0d 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -337,6 +337,77 @@ class TestDatabaseApi(SupersetTestCase): db.session.delete(model) db.session.commit() + @with_feature_flags(SSH_TUNNELING=True) + @mock.patch( + "superset.commands.database.test_connection.TestConnectionDatabaseCommand.run", + ) + @mock.patch("superset.models.core.Database.get_all_catalog_names") + @mock.patch("superset.models.core.Database.get_all_schema_names") + def test_get_database_ssh_tunnel_credentials_are_masked( + self, + mock_get_all_schema_names, + mock_get_all_catalog_names, + mock_test_connection_database_command_run, + ): + """ + Database API: SSH tunnel credentials are masked on the read paths + (GET / and GET //connection), consistently with create/update. + """ + self.login(ADMIN_USERNAME) + example_db = get_example_database() + if example_db.backend == "sqlite": + return + ssh_tunnel_properties = { + "server_address": "123.132.123.1", + "server_port": 8080, + "username": "foo", + "password": "bar", + "private_key": "secret-key-material", + "private_key_password": "secret-key-password", + } + database_data = { + "database_name": "test-db-with-ssh-tunnel-read-masking", + "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted, + "ssh_tunnel": ssh_tunnel_properties, + } + rv = self.client.post("api/v1/database/", json=database_data) + response = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 201 + database_id = response.get("id") + + masked_fields = ("password", "private_key", "private_key_password") + + # GET //connection + rv = self.client.get(f"api/v1/database/{database_id}/connection") + assert rv.status_code == 200 + connection_tunnel = json.loads(rv.data.decode("utf-8"))["result"]["ssh_tunnel"] + for field in masked_fields: + assert connection_tunnel[field] == "XXXXXXXXXX" # noqa: S105 + + # GET / + rv = self.client.get(f"api/v1/database/{database_id}") + assert rv.status_code == 200 + get_tunnel = json.loads(rv.data.decode("utf-8"))["result"]["ssh_tunnel"] + for field in masked_fields: + assert get_tunnel[field] == "XXXXXXXXXX" # noqa: S105 + + # The stored credentials remain intact (only the response is masked). + model_ssh_tunnel = ( + db.session.query(SSHTunnel) + .filter(SSHTunnel.database_id == database_id) + .one() + ) + assert model_ssh_tunnel.password == "bar" # noqa: S105 + assert model_ssh_tunnel.private_key == "secret-key-material" # noqa: S105 + assert ( + model_ssh_tunnel.private_key_password == "secret-key-password" # noqa: S105 + ) + + # Cleanup + model = db.session.query(Database).get(database_id) + db.session.delete(model) + db.session.commit() + @with_feature_flags(SSH_TUNNELING=True) @mock.patch( "superset.commands.database.test_connection.TestConnectionDatabaseCommand.run",