From 3c8b46caae40b841465e81f587fcb325e95976e9 Mon Sep 17 00:00:00 2001 From: Kevin Michel Date: Fri, 5 Jul 2024 07:47:15 +0200 Subject: [PATCH 1/2] fix(integrations): don't send full env to subprocess During the arguments modification to `subprocess.Popen.__init__`, an explicitly empty environment of `{}` is incorrectly confused with a `None` environment. This causes sentry to pass the entire environment of the parent process instead of sending just the injected environment variables. Fix it by only replacing the environment with `os.environ` if the variable is None, and not just falsy. --- sentry_sdk/integrations/stdlib.py | 6 +++++- tests/integrations/stdlib/test_subprocess.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 58e561d4b2..e0b4d06794 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -207,7 +207,11 @@ def sentry_patched_popen_init(self, *a, **kw): ): if env is None: env = _init_argument( - a, kw, "env", 10, lambda x: dict(x or os.environ) + a, + kw, + "env", + 10, + lambda x: dict(x if x is not None else os.environ), ) env["SUBPROCESS_" + k.upper().replace("-", "_")] = v diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index 1e0d63149b..d4a0576419 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -174,6 +174,19 @@ def test_subprocess_basic( assert sys.executable + " -c" in subprocess_init_span["description"] +def test_subprocess_empty_env(sentry_init, monkeypatch): + monkeypatch.setenv("TEST_MARKER", "should_not_be_seen") + sentry_init(integrations=[StdlibIntegration()], traces_sample_rate=1.0) + with start_transaction(name="foo"): + args = [ + sys.executable, + "-c", + "import os; print(os.environ.get('TEST_MARKER', None))", + ] + output = subprocess.check_output(args, env={}, text=True) + assert "should_not_be_seen" not in output + + def test_subprocess_invalid_args(sentry_init): sentry_init(integrations=[StdlibIntegration()]) From cf7b459362fbed313acc4448adf2f7c94066949f Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Fri, 5 Jul 2024 20:11:59 +0200 Subject: [PATCH 2/2] 3.6 compat --- tests/integrations/stdlib/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index d4a0576419..593ef8a0dc 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -183,7 +183,7 @@ def test_subprocess_empty_env(sentry_init, monkeypatch): "-c", "import os; print(os.environ.get('TEST_MARKER', None))", ] - output = subprocess.check_output(args, env={}, text=True) + output = subprocess.check_output(args, env={}, universal_newlines=True) assert "should_not_be_seen" not in output