From 7bf7d3408df9abd02b7b4c0c9ec8c20602ed02cf Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Tue, 21 Jul 2026 10:12:00 +0100 Subject: [PATCH] Fix _is_inside_docker() false positive on systemd cgroups v2 hosts On modern systemd hosts using cgroups v2, /proc/1/cgroup reports PID 1 in "/init.scope" rather than "/". The previous heuristic treated any non-"/" cgroup path as "inside docker", so the auth-code callback server bound to 0.0.0.0 (all interfaces) instead of 127.0.0.1 loopback on ordinary Linux hosts, contrary to RFC 8252 section 8.3. Detect only genuine container markers (/.dockerenv, /run/.containerenv, or docker/containerd/kubepods/lxc/libpod in the PID 1 cgroup path). Real Docker/Podman/Kubernetes containers are still detected, so the docker port-forwarding scenario is preserved. Adds unit tests for _is_inside_docker(). Part of #886. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- msal/oauth2cli/authcode.py | 15 ++++++++-- tests/test_authcode.py | 56 +++++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/msal/oauth2cli/authcode.py b/msal/oauth2cli/authcode.py index 4c60fb4d..0a5f0689 100644 --- a/msal/oauth2cli/authcode.py +++ b/msal/oauth2cli/authcode.py @@ -41,17 +41,28 @@ def obtain_auth_code(listen_port, auth_uri=None): # Historically only used in t def _is_inside_docker(): + # Marker files created by the container runtimes themselves are the most + # reliable signal. "/.dockerenv" is created by Docker (including Docker + # Desktop on Mac); "/run/.containerenv" is created by Podman. + if os.path.exists("/.dockerenv") or os.path.exists("/run/.containerenv"): + return True try: with open("/proc/1/cgroup") as f: # https://stackoverflow.com/a/20012536/728675 # Search keyword "/proc/pid/cgroup" in this link for the file format # https://man7.org/linux/man-pages/man7/cgroups.7.html for line in f.readlines(): cgroup_path = line.split(":", 2)[2].strip() - if cgroup_path.strip() != "/": + # Only a recognized container cgroup counts as "inside a container". + # We must NOT treat any non-"/" path as a container: on a cgroups v2 + # host, systemd puts PID 1 in "/init.scope" (rather than "/"), which + # is not a container and would otherwise cause this server to bind to + # 0.0.0.0 (all interfaces) instead of loopback. See issue #886. + if any(marker in cgroup_path for marker in ( + "docker", "containerd", "kubepods", "lxc", "libpod")): return True except IOError: pass # We are probably not running on Linux - return os.path.exists("/.dockerenv") # Docker on Mac will run this line + return False def is_wsl(): diff --git a/tests/test_authcode.py b/tests/test_authcode.py index 8d7c9d03..178cf285 100644 --- a/tests/test_authcode.py +++ b/tests/test_authcode.py @@ -2,9 +2,14 @@ import socket import sys +try: # Python 3 + from unittest.mock import patch, mock_open +except ImportError: # Python 2 + from mock import patch, mock_open + import requests -from msal.oauth2cli.authcode import AuthCodeReceiver +from msal.oauth2cli.authcode import AuthCodeReceiver, _is_inside_docker class TestAuthCodeReceiver(unittest.TestCase): @@ -104,3 +109,52 @@ def test_post_request_state_mismatch(self): )] result = receiver.get_auth_response(timeout=3, state="expected_state") self.assertIsNone(result, "Should not receive auth response due to state mismatch") + + +class TestIsInsideDocker(unittest.TestCase): + """Guard the container-detection heuristic used to decide the bind address. + + A false positive makes the auth-code receiver bind to 0.0.0.0 (all + interfaces) instead of loopback, which is a security concern per + RFC 8252 Section 8.3. See issue #886. + """ + + def _run(self, cgroup_content, existing_files=()): + existing = set(existing_files) + if cgroup_content is None: + open_mock = mock_open() + open_mock.side_effect = IOError("no such file") # e.g. Windows/Mac + else: + open_mock = mock_open(read_data=cgroup_content) + with patch("msal.oauth2cli.authcode.open", open_mock, create=True), \ + patch( + "msal.oauth2cli.authcode.os.path.exists", + side_effect=lambda p: p in existing): + return _is_inside_docker() + + def test_systemd_cgroups_v2_host_is_not_docker(self): + # Modern systemd host on cgroups v2: PID 1 lives in /init.scope, not "/". + self.assertFalse(self._run("0::/init.scope\n")) + + def test_cgroups_v1_host_is_not_docker(self): + content = "12:pids:/\n11:hugetlb:/\n0::/\n" + self.assertFalse(self._run(content)) + + def test_dockerenv_marker_is_docker(self): + self.assertTrue(self._run("0::/\n", existing_files=("/.dockerenv",))) + + def test_containerenv_marker_is_container(self): + self.assertTrue( + self._run("0::/\n", existing_files=("/run/.containerenv",))) + + def test_docker_cgroup_path_is_docker(self): + content = "12:pids:/docker/abc123\n0::/docker/abc123\n" + self.assertTrue(self._run(content)) + + def test_kubepods_cgroup_path_is_container(self): + content = "0::/kubepods/besteffort/pod123/abc\n" + self.assertTrue(self._run(content)) + + def test_non_linux_without_marker_is_not_docker(self): + # No /proc/1/cgroup (open raises IOError) and no marker files. + self.assertFalse(self._run(None))