[Gateway] Fix and deprecate env whitelist handling (#979)

Author: kevin-bates

jupyter_server/gateway/gateway_client.py

@@ -12,7 +12,7 @@
 
 from tornado import web
 from tornado.httpclient import AsyncHTTPClient, HTTPClientError, HTTPResponse
-from traitlets import Bool, Float, Int, TraitError, Unicode, default, validate
+from traitlets import Bool, Float, Int, TraitError, Unicode, default, observe, validate
 from traitlets.config import SingletonConfigurable
 
 
@@ -282,20 +282,29 @@ def __init__(self, **kwargs):
         # store of cookies with store time
         self._cookies = {}  # type: ty.Dict[str, ty.Tuple[Morsel, datetime]]
 
-    env_whitelist_default_value = ""
-    env_whitelist_env = "JUPYTER_GATEWAY_ENV_WHITELIST"
-    env_whitelist = Unicode(
-        default_value=env_whitelist_default_value,
+    allowed_envs_default_value = ""
+    allowed_envs_env = "JUPYTER_GATEWAY_ALLOWED_ENVS"
+    allowed_envs = Unicode(
+        default_value=allowed_envs_default_value,
         config=True,
         help="""A comma-separated list of environment variable names that will be included, along with
-         their values, in the kernel startup request.  The corresponding `env_whitelist` configuration
+         their values, in the kernel startup request.  The corresponding `allowed_envs` configuration
          value must also be set on the Gateway server - since that configuration value indicates which
-         environmental values to make available to the kernel. (JUPYTER_GATEWAY_ENV_WHITELIST env var)""",
+         environmental values to make available to the kernel. (JUPYTER_GATEWAY_ALLOWED_ENVS env var)""",
     )
 
-    @default("env_whitelist")
-    def _env_whitelist_default(self):
-        return os.environ.get(self.env_whitelist_env, self.env_whitelist_default_value)
+    @default("allowed_envs")
+    def _allowed_envs_default(self):
+        return os.environ.get(
+            "JUPYTER_GATEWAY_ENV_WHITELIST",
+            os.environ.get(self.allowed_envs_env, self.allowed_envs_default_value),
+        )
+
+    env_whitelist = Unicode(
+        default_value=allowed_envs_default_value,
+        config=True,
+        help="""Deprecated, use `GatewayClient.allowed_envs`""",
+    )
 
     gateway_retry_interval_default_value = 1.0
     gateway_retry_interval_env = "JUPYTER_GATEWAY_RETRY_INTERVAL"
@@ -386,6 +395,35 @@ def accept_cookies_default(self):
             not in ["no", "false"]
         )
 
+    _deprecated_traits = {
+        "env_whitelist": ("allowed_envs", "2.0"),
+    }
+
+    # Method copied from
+    # https://github.com/jupyterhub/jupyterhub/blob/d1a85e53dccfc7b1dd81b0c1985d158cc6b61820/jupyterhub/auth.py#L143-L161
+    @observe(*list(_deprecated_traits))
+    def _deprecated_trait(self, change):
+        """observer for deprecated traits"""
+        old_attr = change.name
+        new_attr, version = self._deprecated_traits[old_attr]
+        new_value = getattr(self, new_attr)
+        if new_value != change.new:
+            # only warn if different
+            # protects backward-compatible config from warnings
+            # if they set the same value under both names
+            self.log.warning(
+                (
+                    "{cls}.{old} is deprecated in jupyter_server "
+                    "{version}, use {cls}.{new} instead"
+                ).format(
+                    cls=self.__class__.__name__,
+                    old=old_attr,
+                    new=new_attr,
+                    version=version,
+                )
+            )
+            setattr(self, new_attr, change.new)
+
     @property
     def gateway_enabled(self):
         return bool(self.url is not None and len(self.url) > 0)

jupyter_server/gateway/managers.py

@@ -421,15 +421,16 @@ async def start_kernel(self, **kwargs):
             if os.environ.get("KERNEL_USERNAME") is None and GatewayClient.instance().http_user:
                 os.environ["KERNEL_USERNAME"] = GatewayClient.instance().http_user
 
+            payload_envs = os.environ.copy()
+            payload_envs.update(kwargs.get("env", {}))  # Add any env entries in this request
+
+            # Build the actual env payload, filtering allowed_envs and those starting with 'KERNEL_'
             kernel_env = {
                 k: v
-                for (k, v) in dict(os.environ).items()
-                if k.startswith("KERNEL_") or k in GatewayClient.instance().env_whitelist.split(",")
+                for (k, v) in payload_envs.items()
+                if k.startswith("KERNEL_") or k in GatewayClient.instance().allowed_envs.split(",")
             }
 
-            # Add any env entries in this request
-            kernel_env.update(kwargs.get("env", {}))
-
             # Convey the full path to where this notebook file is located.
             if kwargs.get("cwd") is not None and kernel_env.get("KERNEL_WORKING_DIR") is None:
                 kernel_env["KERNEL_WORKING_DIR"] = kwargs["cwd"]

tests/test_gateway.py

@@ -104,6 +104,10 @@ async def mock_gateway_request(url, **kwargs):
         env = json_body.get("env")
         kspec_name = env.get("KERNEL_KSPEC_NAME")
         assert name == kspec_name  # Ensure that KERNEL_ env values get propagated
+        # Verify env propagation is well-behaved...
+        assert "FOO" in env
+        assert "BAR" in env
+        assert "BAZ" not in env
         model = generate_model(name)
         running_kernels[model.get("id")] = model  # Register model as a running kernel
         response_buf = BytesIO(json.dumps(model).encode("utf-8"))
@@ -190,6 +194,10 @@ def init_gateway(monkeypatch):
     monkeypatch.setenv("JUPYTER_GATEWAY_CONNECT_TIMEOUT", "44.4")
     monkeypatch.setenv("JUPYTER_GATEWAY_LAUNCH_TIMEOUT_PAD", "1.1")
     monkeypatch.setenv("JUPYTER_GATEWAY_ACCEPT_COOKIES", "false")
+    monkeypatch.setenv("JUPYTER_GATEWAY_ENV_WHITELIST", "FOO,BAR")
+    monkeypatch.setenv("FOO", "foo")
+    monkeypatch.setenv("BAR", "bar")
+    monkeypatch.setenv("BAZ", "baz")
     yield
     GatewayClient.clear_instance()
 
@@ -204,18 +212,20 @@ async def test_gateway_env_options(init_gateway, jp_serverapp):
     assert jp_serverapp.gateway_config.connect_timeout == 44.4
     assert jp_serverapp.gateway_config.launch_timeout_pad == 1.1
     assert jp_serverapp.gateway_config.accept_cookies is False
+    assert jp_serverapp.gateway_config.allowed_envs == "FOO,BAR"
 
     GatewayClient.instance().init_static_args()
     assert GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == 43
 
 
-async def test_gateway_cli_options(jp_configurable_serverapp):
+async def test_gateway_cli_options(jp_configurable_serverapp, capsys):
     argv = [
         "--gateway-url=" + mock_gateway_url,
         "--GatewayClient.http_user=" + mock_http_user,
         "--GatewayClient.connect_timeout=44.4",
         "--GatewayClient.request_timeout=96.0",
         "--GatewayClient.launch_timeout_pad=5.1",
+        "--GatewayClient.env_whitelist=FOO,BAR",
     ]
 
     GatewayClient.clear_instance()
@@ -227,6 +237,12 @@ async def test_gateway_cli_options(jp_configurable_serverapp):
     assert app.gateway_config.connect_timeout == 44.4
     assert app.gateway_config.request_timeout == 96.0
     assert app.gateway_config.launch_timeout_pad == 5.1
+    assert app.gateway_config.allowed_envs == "FOO,BAR"
+    captured = capsys.readouterr()
+    assert (
+        "env_whitelist is deprecated in jupyter_server 2.0, use GatewayClient.allowed_envs"
+        in captured.err
+    )
     GatewayClient.instance().init_static_args()
     assert (
         GatewayClient.instance().KERNEL_LAUNCH_TIMEOUT == 90