diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 65f7977d4d2..7f79fbc3b0d 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -654,7 +654,24 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods logger.warning(bottom_banner) def check_secret_key(self) -> None: - if self.config["SECRET_KEY"] == CHANGE_ME_SECRET_KEY: + secret_key = self.config["SECRET_KEY"] + # A missing/empty SECRET_KEY is as insecure as the well-known default + # placeholder: both are treated as insecure and handled identically. An + # empty key is reachable when a deployment explicitly sets SECRET_KEY to + # an empty value (the env fallback only substitutes the placeholder). + if secret_key and secret_key != CHANGE_ME_SECRET_KEY: + return + if not secret_key: + warning = ( + "An empty SECRET_KEY was detected, please use superset_config.py " + "to set a non-empty value.\n" + "Use a strong complex alphanumeric string and use a tool to help" + " you generate \n" + "a sufficiently random sequence, ex: openssl rand -base64 42 \n" + "For more info, see: https://superset.apache.org/docs/" + "configuration/configuring-superset#specifying-a-secret_key" + ) + else: warning = ( "A Default SECRET_KEY was detected, please use superset_config.py " "to override it.\n" @@ -664,17 +681,13 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods "For more info, see: https://superset.apache.org/docs/" "configuration/configuring-superset#specifying-a-secret_key" ) - if ( - self.superset_app.debug - or self.superset_app.config["TESTING"] - or is_test() - ): - logger.warning("Debug mode identified with default secret key") - self._log_config_warning(warning) - return + if self.superset_app.debug or self.superset_app.config["TESTING"] or is_test(): + logger.warning("Debug mode identified with insecure secret key") self._log_config_warning(warning) - logger.error("Refusing to start due to insecure SECRET_KEY") - sys.exit(1) + return + self._log_config_warning(warning) + logger.error("Refusing to start due to insecure SECRET_KEY") + sys.exit(1) def check_guest_token_secret(self) -> None: """Refuse to start with default guest JWT secret when embedding is enabled.""" diff --git a/tests/unit_tests/test_check_secret_key.py b/tests/unit_tests/test_check_secret_key.py new file mode 100644 index 00000000000..e1416276925 --- /dev/null +++ b/tests/unit_tests/test_check_secret_key.py @@ -0,0 +1,74 @@ +# 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. +"""Tests for SupersetAppInitializer.check_secret_key.""" + +from typing import Optional +from unittest.mock import MagicMock, patch + +import pytest + +from superset.constants import CHANGE_ME_SECRET_KEY +from superset.initialization import SupersetAppInitializer + + +def _make_initializer( + secret_key: Optional[str], + *, + debug: bool = False, + testing: bool = False, +) -> SupersetAppInitializer: + """Build a bare initializer with just the attributes check_secret_key needs.""" + init = object.__new__(SupersetAppInitializer) + init.config = {"SECRET_KEY": secret_key} + app = MagicMock() + app.debug = debug + app.config = {"TESTING": testing} + init.superset_app = app + return init + + +@pytest.mark.parametrize("secret_key", ["", None, CHANGE_ME_SECRET_KEY]) +def test_check_secret_key_refuses_to_start_when_insecure( + secret_key: Optional[str], +) -> None: + """An empty/missing or placeholder key fails closed in non-debug mode.""" + initializer = _make_initializer(secret_key) + with patch("superset.initialization.is_test", return_value=False): + with pytest.raises(SystemExit): + initializer.check_secret_key() + + +@pytest.mark.parametrize("secret_key", ["", None, CHANGE_ME_SECRET_KEY]) +def test_check_secret_key_warns_but_starts_in_debug( + secret_key: Optional[str], +) -> None: + """In debug/testing mode an insecure key warns but does not exit.""" + initializer = _make_initializer(secret_key, debug=True) + with patch("superset.initialization.is_test", return_value=False): + # Should not raise SystemExit. + initializer.check_secret_key() + + +def test_check_secret_key_accepts_strong_key() -> None: + """A non-empty, non-placeholder key starts without warning or exit.""" + initializer = _make_initializer("a-strong-random-secret-key") + with ( + patch("superset.initialization.is_test", return_value=False), + patch.object(initializer, "_log_config_warning") as log_warning, + ): + initializer.check_secret_key() + log_warning.assert_not_called()