WebSafe 3.7github.com
|
|
🏠
Skip to content

gh-127146: Skip test_netrc.test_security on Emscripten#136227

Closed
hoodmane wants to merge 1 commit intopython:mainfrom
hoodmane:emscripten-skip-netrc
Closed

gh-127146: Skip test_netrc.test_security on Emscripten#136227
hoodmane wants to merge 1 commit intopython:mainfrom
hoodmane:emscripten-skip-netrc

Conversation

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jul 3, 2025 •
edited by bedevere-app bot
Loading

Before #135816 it was skipped because pwd is missing on Emscripten. Now it isn't skipped because the test no longer requires pwd but it still doesn't work on Emscripten.
@hoodmane
pythongh-127146: Skip test_netrc.test_security on Emscripten
6b0c452 
Before python#135816 it was skipped because pwd is missing on Emscripten.
Now it isn't skipped because the test no longer requires pwd but it
still doesn't work on Emscripten.
@hoodmane hoodmane added tests Tests in the Lib/test dir skip news OS-emscripten labels Jul 3, 2025
@hoodmane hoodmane requested a review from freakboy3742 July 3, 2025 08:24
@bedevere-app bedevere-app bot mentioned this pull request Jul 3, 2025
Closed
@cmaloney
Copy link
Contributor

cmaloney commented Jul 3, 2025 •
edited
Loading

Is this failing because of https://github.com/python/cpython/blob/main/Lib/netrc.py#L10C5-L12 (from gh-135815)?

This test is validating the security check is working right (added in gh-59189). Worried about disabling a test for a security check rather than just disabling the module (or fixing the security check).
@freakboy3742
Copy link
Contributor

freakboy3742 commented Jul 9, 2025

Is this failing because of https://github.com/python/cpython/blob/main/Lib/netrc.py#L10C5-L12 (from gh-135815)?

This test is validating the security check is working right (added in gh-59189). Worried about disabling a test for a security check rather than just disabling the module (or fixing the security check).

I'm not 100% sure about this. Unlike WASI, Emscripten reliably has os.getuid(); but it doesn't have pwd. So - the netrc._can_security_check() will pass (which means the test added for WASI test will fail); but the default test is failing (because the check performed by that test doesn't hold for Emscripten):

======================================================================
ERROR: test_security (test.test_netrc.NetrcTestCase.test_security)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/emscripten/buildarea/3.x.rkm-emscripten.emscripten/build/Lib/test/test_netrc.py", line 292, in test_security
    nrc = netrc.netrc()
  File "/home/emscripten/buildarea/3.x.rkm-emscripten.emscripten/build/Lib/netrc.py", line 88, in __init__
    self._parse(file, fp, default_netrc)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/emscripten/buildarea/3.x.rkm-emscripten.emscripten/build/Lib/netrc.py", line 155, in _parse
    self._security_check(fp, default_netrc, self.hosts[entryname][0])
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/emscripten/buildarea/3.x.rkm-emscripten.emscripten/build/Lib/netrc.py", line 164, in _security_check
    raise NetrcParseError(
        f"~/.netrc file owner ({fowner}) does not match"
        f" current user ({user})")
netrc.NetrcParseError: ~/.netrc file owner (uid 1002) does not match current user (uid 0) (None, line None)

----------------------------------------------------------------------
Ran 23 tests in 0.037s

FAILED (errors=1, skipped=1)

My immediate guess is that the _can_security_check() method should be modified to catch the Emscripten behavior (maybe by adding and os.getuid() != 0, or maybe even an explicit and sys.platform != "emscripten". @hoodmane may have another idea.

That said - it's interesting that this failure is also occurring with a file mode, which is similar to the failure reported as #135783 (although it's a very different issue). That might be complete coincidence... but it's an interesting simliarity.
@hoodmane
Copy link
Contributor Author

hoodmane commented Jul 10, 2025

Closed in favor of #136509.
@hoodmane hoodmane closed this Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review OS-emscripten skip news tests Tests in the Lib/test dir

3 participants

@hoodmane @cmaloney @freakboy3742

Comments