From ee70175e3392993e19d1fffd9fff6b67395d6128 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 13 Sep 2022 14:58:21 -0700 Subject: [PATCH] Fix and deprecate env whitelist handling --- jupyter_server/gateway/gateway_client.py | 58 ++++++++++++++++++++---- jupyter_server/gateway/managers.py | 11 +++-- tests/test_gateway.py | 18 +++++++- 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/jupyter_server/gateway/gateway_client.py b/jupyter_server/gateway/gateway_client.py index dc780ac4f7..6faee7135c 100644 --- a/jupyter_server/gateway/gateway_client.py +++ b/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) diff --git a/jupyter_server/gateway/managers.py b/jupyter_server/gateway/managers.py index 33c079e348..e826670b08 100644 --- a/jupyter_server/gateway/managers.py +++ b/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"] diff --git a/tests/test_gateway.py b/tests/test_gateway.py index e48d2d9483..1a29df70e8 100644 --- a/tests/test_gateway.py +++ b/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