Skip to content

Update netrc.py from 3.13.6 and make pwd accesible on Android #6083

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update netrc.py from 3.13.6 and make pwd accesible on Android #6083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 19 additions & 12 deletions Lib/netrc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,24 @@

# Module and documentation by Eric S. Raymond, 21 Dec 1998

import os, shlex, stat
import os, stat

__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"))
Expand Down
19 changes: 9 additions & 10 deletions Lib/test/test_netrc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import netrc, os, unittest, sys, textwrap
from test.support import os_helper, run_unittest

try:
import pwd
except ImportError:
pwd = None
from test import support
from test.support import os_helper

temp_filename = os_helper.TESTFN

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 Expand Up @@ -308,8 +309,6 @@ def test_security(self):
self.assertEqual(nrc.hosts['foo.domain.com'],
('anonymous', '', 'pass'))

def test_main():
run_unittest(NetrcTestCase)

if __name__ == "__main__":
test_main()
unittest.main()
12 changes: 10 additions & 2 deletions vm/src/stdlib/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,13 @@ pub mod posix;
mod ctypes;
#[cfg(windows)]
pub(crate) mod msvcrt;
#[cfg(all(unix, not(any(target_os = "android", target_os = "redox"))))]

#[cfg(all(
unix,
not(any(target_os = "ios", target_os = "wasi", target_os = "redox"))
))]
mod pwd;

pub(crate) mod signal;
pub mod sys;
#[cfg(windows)]
Expand Down Expand Up @@ -120,7 +125,10 @@ pub fn get_module_inits() -> StdlibMap {
"_thread" => thread::make_module,
}
// Unix-only
#[cfg(all(unix, not(any(target_os = "android", target_os = "redox"))))]
#[cfg(all(
unix,
not(any(target_os = "ios", target_os = "wasi", target_os = "redox"))
))]
{
"pwd" => pwd::make_module,
}
Expand Down
3 changes: 3 additions & 0 deletions vm/src/stdlib/pwd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

pub(crate) use pwd::make_module;

#[allow(unused)]
Copy link
Member

Choose a reason for hiding this comment

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

Because the conditions in stdlib/mod.rs must be aligned, this allow will not be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only being used on android.
I can do what coderabbitai is suggesting if that's ok

#[pymodule]
mod pwd {
Comment on lines +5 to 7
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Narrow the lint scope; avoid blanket allow(unused).

Use a targeted allow only on Android or guard the specific import instead of silencing all unused warnings for the whole module.

Suggested change:

-#[allow(unused)]
+#[cfg_attr(target_os = "android", allow(unused_imports))]

Additionally, consider guarding or moving the NonNull import so you can drop the allow entirely on Android:

Option A (guard the import):

#[cfg(not(target_os = "android"))]
use std::ptr::NonNull;

Option B (move into the function and fully-qualify):

// remove the module-level `use std::ptr::NonNull;`
// inside getpwall:
while let Some(ptr) = std::ptr::NonNull::new(unsafe { libc::getpwent() }) {
    // ...
}

This keeps clippy happy and avoids hiding unrelated issues.

🤖 Prompt for AI Agents 
In vm/src/stdlib/pwd.rs around lines 5 to 7, the use of a blanket
#[allow(unused)] attribute on the entire module is too broad and hides unrelated
warnings. To fix this, remove the module-level #[allow(unused)] and instead
apply a more targeted approach by either guarding the import of
std::ptr::NonNull with #[cfg(not(target_os = "android"))] or by removing the
import entirely and fully qualifying std::ptr::NonNull inside the function where
it is used. This will narrow the lint scope and keep clippy warnings relevant.

use crate::{
Expand All @@ -26,6 +27,7 @@ mod pwd {
pw_dir: String,
pw_shell: String,
}

#[pyclass(with(PyStructSequence))]
impl Passwd {}

Expand Down Expand Up @@ -91,6 +93,7 @@ mod pwd {
}

// TODO: maybe merge this functionality into nix?
#[cfg(not(target_os = "android"))]
#[pyfunction]
fn getpwall(vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
// setpwent, getpwent, etc are not thread safe. Could use fgetpwent_r, but this is easier
Expand Down
Loading