NetRCInfo.netrc_info() doesn't work when run in a systemd service with DynamicUser=true

[tomchristie]

Discussed in #2526

^{Originally posted by djmattyg007 January 1, 2023}
The function in question, netrc_info(), iterates over three paths. Two of these paths refer to the home directory of the current process' user, by prefixing the paths with ~/. These paths are run through pathlib.Path.expanduser(), which raises an exception if the home directory can't be determined.

When the code is running in a systemd service with DynamicUser=true, there is no home directory. This means expanduser() will always raise an exception.

Fortunately, there's a potential override: set the NETRC environment variable to something. As long as it points to a file that exists, and doesn't need to know the current user's home directory, this is an acceptable workaround.

Unfortunately, there's an is_file() check performed on the Path object before attempting to use it. This means I can't just set NETRC=/dev/null as a simple way of working around this problem, even though netrc.netrc() will happily accept /dev/null.

Why does any of this matter? Because I'm not using httpx directly. I'm using the library python-telegram-bot, and have no control over how it uses httpx internally.

To summarise, there are two problems:

• The call to pathlib.Path.expanduser() can raise an exception that isn't currently being caught
• Setting NETRC=/dev/null doesn't work as expected, despite it being a perfectly suitable file to use in this context

I'm happy to raise an issue for this, and also to implement a PR to resolve this.

[tomchristie]

Okay, so we've got two outstanding netrc bugs...

I've taken a review over our netrc handling and I believe we ought to be making a behavioural change here...

• It looks to me like browser do not apply netrc authentication to outgoing requests.
• The curl command-line tool (which I always consider a good point of reference) does not automatically apply netrc authentication, although it can be explicitly enabled.

We currently have it built-in, because we're following requests, but I think we'd be doing better to follow browser behaviour here.

My suggestion is this...

We should drop the netrc authentication that's baked into the client instance, and instead provide a NetRCAuth() class, so that developers can explicitly enable netrc authentication. This would be a behavioural change, so it'd need to part of a 0.24 release, and called out clearly in the release notes.

I'm happy to raise an issue for this, and also to implement a PR to resolve this.

@djmattyg007 - That'd be neat, yup. If you're up for that I can provide review, and help us to land this.

Here's what I think is needed...

• Remove the netrc handling that's currently present in the client.
• Add a NetRCAuth([file]) class, implemented in our _auth.py module, and exposed as a new piece of public API.
• We currently try multiple lookup paths, and also inspect the NETRC environment variable. Let's not do that. If we've got an explicit NetRCAuth class, then let's just default to the same behaviour of the stdlib netrc.netrc() function. If the developer wants to use the NETRC environment variable, or to inspect and attempt more than one file location, then they're free to handle that logic in their own codebase.
• We currently handle "the netrc file may or may not exist". Again, let's not do that. If NetRCAuth is used, and the file doesn't exist, then we should allow netrc.netrc() to raise an exception at the point the NetRCAuth class is instantiated. This is more explicit behaviour, and will help ensure that if a developer is enabling netrc auth, then it won't just silently be ignored if they've got the environment setup incorrectly for it.

Implementing netrc handling as an auth class would also resolve #2088, since the auth logic would be applied on each new request.

[djmattyg007]

Sorry for any potential confusion - my offer to raise a PR was for a much more simplistic solution to directly address the two problems highlighted in my summary, within the existing architecture. I simply don't have the time to make more comprehensive changes than that.

[tomchristie]

@djmattyg007 Ah no worries, yup. 😅

I've opened a proposal #2535, which would remove the issue you're seeing since NetRCAuth would always be configured explicitly.

If that doesn't look like it's going to land anytime soon, then we could consider making the minor change that you've proposed, so that you're not hitting issues in the meantime.

[ShaoyiZhang]

Hi, would it be possible to reconsider this behavioral change? I think many people could be relying on netrc files to add a minimum authentication layer when they don't have much control over open source projects.

Could we consider adding an environment variable like IMPLICIT_NETRC, which would keep the netrc behavior prior to v0.24.0 if set to True? (Default should be False) With this approach, developers can choose to control netrc explicitly and the users can opt-in for traditional netrc behavior.

Thank you! 🙇