From e81151a4be545b4603875cf8ee7580a61cec0e18 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 5 Oct 2025 15:20:04 -0700 Subject: [PATCH 1/4] gh-139633: Run `netrc` file permission check only once per parse Change the `.netrc` security check to be run once per parse of the default file rather than once per line inside the file. --- Lib/netrc.py | 39 ++++++++++++++++++++++----------------- Lib/test/test_netrc.py | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/Lib/netrc.py b/Lib/netrc.py index 2f502c1d53364f..4347fbf182d3e2 100644 --- a/Lib/netrc.py +++ b/Lib/netrc.py @@ -152,23 +152,28 @@ def _parse(self, file, fp, default_netrc): else: raise NetrcParseError("bad follower token %r" % tt, file, lexer.lineno) - self._security_check(fp, default_netrc, self.hosts[entryname][0]) - - def _security_check(self, fp, default_netrc, login): - if _can_security_check() and default_netrc and login != "anonymous": - prop = os.fstat(fp.fileno()) - current_user_id = os.getuid() - if prop.st_uid != current_user_id: - fowner = _getpwuid(prop.st_uid) - user = _getpwuid(current_user_id) - raise NetrcParseError( - f"~/.netrc file owner ({fowner}) does not match" - f" current user ({user})") - if (prop.st_mode & (stat.S_IRWXG | stat.S_IRWXO)): - raise NetrcParseError( - "~/.netrc access too permissive: access" - " permissions must restrict access to only" - " the owner") + + if _can_security_check() and default_netrc: + for entry in self.hosts.values(): + if entry[0] != "anonymous": + # Raises on security issue + self._security_check(fp) + break + + def _security_check(self, fp): + prop = os.fstat(fp.fileno()) + current_user_id = os.getuid() + if prop.st_uid != current_user_id: + fowner = _getpwuid(prop.st_uid) + user = _getpwuid(current_user_id) + raise NetrcParseError( + f"~/.netrc file owner ({fowner}) does not match" + f" current user ({user})") + if (prop.st_mode & (stat.S_IRWXG | stat.S_IRWXO)): + raise NetrcParseError( + "~/.netrc access too permissive: access" + " permissions must restrict access to only" + " the owner") def authenticators(self, host): """Return a (user, account, password) tuple for given host.""" diff --git a/Lib/test/test_netrc.py b/Lib/test/test_netrc.py index 9d720f627102e3..1085281e0fb85d 100644 --- a/Lib/test/test_netrc.py +++ b/Lib/test/test_netrc.py @@ -1,6 +1,9 @@ import netrc, os, unittest, sys, textwrap +from pathlib import Path from test import support from test.support import os_helper +from unittest.mock import Mock + temp_filename = os_helper.TESTFN @@ -309,6 +312,29 @@ def test_security(self): self.assertEqual(nrc.hosts['foo.domain.com'], ('anonymous', '', 'pass')) + @unittest.skipUnless(os.name == 'posix', 'POSIX only test') + @unittest.skipUnless(hasattr(os, 'getuid'), "os.getuid is required") + @os_helper.skip_unless_working_chmod + def test_security_only_once(self): + # Make sure security check is only run once per parse when multiple + # entries are found. + check_called = netrc.netrc._security_check = Mock(return_value=True) + + # Parse a default netrc with more than one password line. + with os_helper.temp_dir() as tmp_dir: + netrc_path = Path(tmp_dir) / '.netrc' + netrc_path.write_text("""\ + machine foo.domain.com login bar password pass + machine bar.domain.com login foo password pass + """) + netrc_path.chmod(0o600) + with os_helper.EnvironmentVarGuard() as environ: + environ.set('HOME', tmp_dir) + netrc.netrc() + + check_called.assert_called_once() + del check_called + if __name__ == "__main__": unittest.main() From 6963ce94cf7a901a702e96d06000520556dcb877 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 5 Oct 2025 15:38:07 -0700 Subject: [PATCH 2/4] Add blurb --- .../next/Library/2025-10-05-15-38-02.gh-issue-139633.l3P839.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-10-05-15-38-02.gh-issue-139633.l3P839.rst diff --git a/Misc/NEWS.d/next/Library/2025-10-05-15-38-02.gh-issue-139633.l3P839.rst b/Misc/NEWS.d/next/Library/2025-10-05-15-38-02.gh-issue-139633.l3P839.rst new file mode 100644 index 00000000000000..94bd18074f8d2d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-10-05-15-38-02.gh-issue-139633.l3P839.rst @@ -0,0 +1,2 @@ +The :mod:`netrc` security check is now run once per parse rather than once +per entry. From 4a32d6d9aacdad3e6bba4736cb2de0cc8d5cb7d7 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 5 Oct 2025 18:41:20 -0700 Subject: [PATCH 3/4] Move to unittest.mock.patch --- Lib/test/test_netrc.py | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_netrc.py b/Lib/test/test_netrc.py index 1085281e0fb85d..354081e96213a6 100644 --- a/Lib/test/test_netrc.py +++ b/Lib/test/test_netrc.py @@ -2,7 +2,7 @@ from pathlib import Path from test import support from test.support import os_helper -from unittest.mock import Mock +from unittest.mock import patch temp_filename = os_helper.TESTFN @@ -318,22 +318,19 @@ def test_security(self): def test_security_only_once(self): # Make sure security check is only run once per parse when multiple # entries are found. - check_called = netrc.netrc._security_check = Mock(return_value=True) - - # Parse a default netrc with more than one password line. - with os_helper.temp_dir() as tmp_dir: - netrc_path = Path(tmp_dir) / '.netrc' - netrc_path.write_text("""\ - machine foo.domain.com login bar password pass - machine bar.domain.com login foo password pass - """) - netrc_path.chmod(0o600) - with os_helper.EnvironmentVarGuard() as environ: - environ.set('HOME', tmp_dir) - netrc.netrc() + with patch.object(netrc.netrc, "_security_check") as mock: + with os_helper.temp_dir() as tmp_dir: + netrc_path = Path(tmp_dir) / '.netrc' + netrc_path.write_text("""\ + machine foo.domain.com login bar password pass + machine bar.domain.com login foo password pass + """) + netrc_path.chmod(0o600) + with os_helper.EnvironmentVarGuard() as environ: + environ.set('HOME', tmp_dir) + netrc.netrc() - check_called.assert_called_once() - del check_called + mock.assert_called_once() if __name__ == "__main__": From 4549627d73ef2dcde94579bfc02feae3797df942 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 5 Oct 2025 18:44:13 -0700 Subject: [PATCH 4/4] fixup: Slightly better comment; return rather than break --- Lib/netrc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/netrc.py b/Lib/netrc.py index 4347fbf182d3e2..9671c6cd2a272a 100644 --- a/Lib/netrc.py +++ b/Lib/netrc.py @@ -156,9 +156,9 @@ def _parse(self, file, fp, default_netrc): if _can_security_check() and default_netrc: for entry in self.hosts.values(): if entry[0] != "anonymous": - # Raises on security issue + # Raises on security issue; once passed once can exit. self._security_check(fp) - break + return def _security_check(self, fp): prop = os.fstat(fp.fileno())