gh-135815: skip `netrc` security checks if `os.getuid` is missing by picnixz · Pull Request #135816 · python/cpython · GitHub
Skip to content

gh-135815: skip netrc security checks if os.getuid is missing #135816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
picnixz merged 2 commits into from
Jun 22, 2025
+30 −18
Merged

gh-135815: skip netrc security checks if os.getuid is missing #135816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2a13f6
fix netrc security checks on WASI
picnixz Jun 22, 2025
cbe216a
avoid calling `os.getuid()` twice
picnixz Jun 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Doc/library/netrc.rst
View file
Open in desktop
29 changes: 18 additions & 11 deletions Lib/netrc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@
__all__ = ["netrc", "NetrcParseError"]


def _can_security_check():
# On WASI, getuid() is indicated as a stub but it may also be missing.
return os.name == 'posix' and hasattr(os, 'getuid')


def _getpwuid(uid):
try:
import pwd
return pwd.getpwuid(uid)[0]
except (ImportError, LookupError):
return f'uid {uid}'


class NetrcParseError(Exception):
"""Exception raised on syntax errors in the .netrc file."""
def __init__(self, msg, filename=None, lineno=None):
Expand Down Expand Up @@ -142,18 +155,12 @@ def _parse(self, file, fp, default_netrc):
self._security_check(fp, default_netrc, self.hosts[entryname][0])

def _security_check(self, fp, default_netrc, login):
if os.name == 'posix' and default_netrc and login != "anonymous":
if _can_security_check() and default_netrc and login != "anonymous":
prop = os.fstat(fp.fileno())
if prop.st_uid != os.getuid():
import pwd
try:
fowner = pwd.getpwuid(prop.st_uid)[0]
except KeyError:
fowner = 'uid %s' % prop.st_uid
try:
user = pwd.getpwuid(os.getuid())[0]
except KeyError:
user = 'uid %s' % os.getuid()
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}, {user}) does not match"
" current user"))
Comment on lines 165 to 166

@serhiy-storchaka serhiy-storchaka Jun 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message looks weird. Would not be better to rewrite in in the following way?

                    (f"~/.netrc file owner ({fowner}) does not match"
                     f" current user ({user})"))

@picnixz picnixz Jun 22, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change it in a follow-up PR so that only unstable branches are affected.

Expand Down
13 changes: 7 additions & 6 deletions Lib/test/test_netrc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import netrc, os, unittest, sys, textwrap
from test import support
from test.support import os_helper

try:
import pwd
except ImportError:
pwd = None

temp_filename = os_helper.TESTFN

class NetrcTestCase(unittest.TestCase):
Expand Down Expand Up @@ -269,9 +265,14 @@ def test_comment_at_end_of_machine_line_pass_has_hash(self):
machine bar.domain.com login foo password pass
""", '#pass')

@unittest.skipUnless(support.is_wasi, 'WASI only test')
def test_security_on_WASI(self):
self.assertFalse(netrc._can_security_check())
self.assertEqual(netrc._getpwuid(0), 'uid 0')
self.assertEqual(netrc._getpwuid(123456), 'uid 123456')

@unittest.skipUnless(os.name == 'posix', 'POSIX only test')
@unittest.skipIf(pwd is None, 'security check requires pwd module')
@unittest.skipUnless(hasattr(os, 'getuid'), "os.getuid is required")
@os_helper.skip_unless_working_chmod
def test_security(self):
# This test is incomplete since we are normally not run as root and
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2025-06-22-16-23-44.gh-issue-135815.0DandH.rst
View file
Open in desktop