astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.66k stars 1.09k forks source link

Respect `PYTHONSAFEPATH` if set #11928

Open thejcannon opened 4 months ago

thejcannon commented 4 months ago

Docs links:

Don’t prepend a potentially unsafe path to sys.path: python -m module command line: Don’t prepend the current working directory. python script.py command line: Don’t prepend the script’s directory. If it’s a symbolic link, resolve symbolic links. python -c code and python (REPL) command lines: Don’t prepend an empty string, which means the current working directory.

Reproducer with ruff (note foo is an implicit namespace package in vanilla Python-speak, but totally not something we expected to import)

$ mkdir foo
$ cat <<EOF > example.py
import pathlib

import pytest

import anthem  # other 1p
import foo

[pathlib, pytest, foo,, anthem]

EOF
$ export PYTHONSAFEPATH=1
$ ruff check --fix-only example.py

keeps foo with 1p.

but:

$ python -c 'import foo'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'foo'
zanieb commented 4 months ago

Hm. Is this always going to be set for all users of Ruff with your code? Otherwise it seems like we could flip-flop behaviors here depending on the local environment which seems less than ideal.

thejcannon commented 4 months ago

Yeah, we would set it as part of machine bringup

zanieb commented 4 months ago

After some internal discussion, we need to decide how best to support this use-case. We might want to use a dedicated setting instead of reading runtime Python variables.

thejcannon commented 4 months ago

(Not that you needed me to be, but) I'm cool with that as well 👍

zanieb commented 4 months ago

I think that's the most reasonable option and addresses my concern. Not entirely sure this is doable with our current module resolution handling, I'd need to look at it to refresh my understanding. If someone is interested in doing a proof of concept pull request, I'd review.

thejcannon commented 4 months ago

The issue I'm trying to solve is that sometimes folks have empty directories lying around and they get confused why their editor is reformatting in a way that CI then complains about and they don't know how to fix it (admittedly implicit namespace packages are not intuitive)

charliermarsh commented 4 months ago

Sort of hacky but we could check if foo is non-empty for this purpose, perhaps?

thejcannon commented 4 months ago

I honestly wonder if this might be made more general with a "never assume I use implicit namespace packages" config?

That's really what we want (but Python doesn't expose that, hence the env var about "safe"ty)