Skip to content

Commit

Permalink
fix(integrations): don't send full env to subprocess (#3251)
Browse files Browse the repository at this point in the history
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.

---------

Co-authored-by: Kevin Michel <kevin.michel@aiven.io>
  • Loading branch information
sentrivana and kmichel-aiven authored Jul 8, 2024
1 parent 31efa62 commit 763e40a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
6 changes: 5 additions & 1 deletion sentry_sdk/integrations/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions tests/integrations/stdlib/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={}, universal_newlines=True)
assert "should_not_be_seen" not in output


def test_subprocess_invalid_args(sentry_init):
sentry_init(integrations=[StdlibIntegration()])

Expand Down

0 comments on commit 763e40a

Please sign in to comment.