Skip to content

gh-57684: Add the -p command line option #92361

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.

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

Closed
wants to merge 1 commit into from
Closed

gh-57684: Add the -p command line option #92361

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 6, 2022

Add the -p option to ignore the -P option and the PYTHONSAFEPATH
environment variable. The -p option takes precedence over -P and -I
options and the PYTHONSAFEPATH environment variable.

test_cmd_line, test_cmd_line_script, test_pdb, test_runpy and
test_tools now use the -p option to ignore the PYTHONSAFEPATH
environment variable.

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

Let me try to explain what problem is being solved by adding this new -p command line option.

Let's say that we have a Python module called hello:

$ echo "print('Hello World')" > hello.py

By default, Python 3.11 can import it:

$ python3.11 -c 'import hello'
Hello World

Good. Using the new -P option, the import fails as expected:

$ python3.11 -P -c 'import hello'
ModuleNotFoundError: No module named 'hello'

Problems arise when the PYTHONSAFEPATH=1 environment variable is set "globally":

$ export PYTHONSAFEPATH=1

$ python3.11 -c 'import hello'
ModuleNotFoundError: No module named 'hello'

$ python3.11
>>> import hello
ModuleNotFoundError: No module named 'hello'

Oh no! It's no longer possible to load hello.py which is in the current directory.

On Python 3.10, well, the environment variable is just ignored and it just works:

$ export PYTHONSAFEPATH=1

$ python3.10 -c 'import hello'
Hello World

$ python3.10
>>> import hello
Hello World

So the problem is that programs which rely on Python 3.10 default sys.path behavior no longer works with PYTHONSAFEPATH=1, but how can you update these programs for Python 3.11 (to still be able to run them with PYTHONSAFEPATH=1)?

Well, the first work around is to use -Ecommand line option:

$ export PYTHONSAFEPATH=1
$ python3.11 -E -c 'import hello'
Hello World

It works again! The problem is that the -E option ignores all PYTHON environment variables (PYTHONPATH, PYTHONWARNINGS, etc.), not only PYTHONSAFEPATH.

The purpose of the -p option is to run a single program (or command) with Python 3.10 sys.path behavior, and only that (don't change anything else):

$ python3.11 -p -c 'import hello'
Hello World

The assumption is that only a minority of Python programs will need the -p option. Most programs use modules installed system-wide or in the user site directory. In that case, PYTHONSAFEPATH=1 and -P option have no effect: these programs continue to work.


A new feature of this option is the ability to use the isolated mode but still load code from the current working directory (CWD).

$ export PYTHONWARNINGS=default

By default, PYTHONWARNINGS is read and import from CWD works:

$ python3.11 -c 'import hello; import sys; print(sys.warnoptions)'
Hello World
['default']

The isolated mode excludes CWD from sys.path and so the import fails:

$ .python3.11 -I -c 'import hello; import sys; print(sys.warnoptions)'
ModuleNotFoundError: No module named 'hello'

Using the new -p option, isolated mode features are still in place (ignore PYTHONWARNINGS env var), but importing from CWD is now possible:

# Isolated mode + -p option: CWD is included => import hello works, but other isolated mode features are still in place
$ python3.11 -I -p -c 'import hello; import sys; print(sys.warnoptions)'
Hello World
[]

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

Without this -p option, setting PYTHONSAFEPATH=1 system-wide globally (ex: to increase the security of the system) is going to be a pain in practice.

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

@gpshead: Here is a concrete implementation add the -p option. So you can play with it and see the benefits. If possible, I would prefer to add it to Python 3.11 to make the overall "safe path" feature more complete and usable.

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

(copy of my comment on #31542)

@vstinner:

Maybe we should consider adding -p option which is the opposite of -P: add the unsafe path.

@gpshead:

I think that should be left for the future. Lets see if anyone actually wants it first. If we do go the route of aiming for a default flip in the future it might make sense.

Flipping the default can now be done by setting PYTHONSAFEPATH=1 system wide (or in the current shell).

The problem is that right now, if PYTHONSAFEPATH=1 is set, it's no longer possible to run programs which rely on the Python 3.10 behavior to load modules in the current directory (as Python freeze.py tool), without having to modify the source code of these programs.

IMO command line options must have the priority over environment variables. Python does not provide command line options to disable the effect of all Python environment variables. For example, PYTHONUNBUFFERED=1 cannot be "undone" by a command line option. But the effect of this variable is limited: I'm not aware of any program broken by it.

The Python UTF-8 Mode is way more likely to cause compatibility issues and so -X utf8=0 can disable it on the command line: ignore PYTHONUTF8=1 env var.

We don't have to add -p, but it makes PYTHONSAFEPATH=1 harder to use. The current workaround is to use -E to ignore environment variables, or -I (isolated mode) but this option has more effects (like not adding user site directories).

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Thanks, @vstinner.

For whatever its worth, I'm not a highly experienced core dev like yourself and others, but the rationale for including -p as a counterpart to -P outlined here and in the previous PR makes eminent sense to me.

I do have a handful of grammar/punctuation fixes and phrasing clarifications for the docs and UI text, if that's alright.

Comment on lines 336 to 338
The :option:`-p` option takes precedence over the :option:`-P` and
:option:`-I` (isolated mode) options and the :envvar:`PYTHONSAFEPATH`
environment variable.
Copy link
Member

Choose a reason for hiding this comment

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

I'd be concerned this might not be entirely clear that this doesn't override all of -I, just the -P it implies. What about something like this?

Suggested change
The :option:`-p` option takes precedence over the :option:`-P` and
:option:`-I` (isolated mode) options and the :envvar:`PYTHONSAFEPATH`
environment variable.
The :option:`-p` option takes precedence over the :option:`-P` option
(either passed explicitly or implied by :option:`-I`, isolated mode)
and the :envvar:`PYTHONSAFEPATH` environment variable.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good, but agree with @CAM-Gerlach's wording suggestion. We may want to merge early so we get this in before the feature freeze.

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

@CAM-Gerlach: It was really hard for me to explain the subtle differences between the default Python behavior and the usage of this option which is a corner case. Thanks for your review, I enhanced the documentation to take your review in account.

@vstinner
Copy link
Member Author

vstinner commented May 6, 2022

PR rebased to get the Docs CI fix: #92395

@gpshead
Copy link
Member

gpshead commented May 6, 2022

If possible, I would prefer to add it to Python 3.11 to make the overall "safe path" feature more complete and usable.

3.11 is closed to features at this point so you'd need a release manager exception.

I think the nicer world is one in which python doesn't put the the main.py path in sys.path (what PYTHONSAFEPATH now provides to those who opt-in). I'm not suggesting we ever get to flip to that default.

But I think adding an opt-out flag to prevent cases where someone may wants to explicitly avoid that nicer world isn't super compelling. At least not any more so than someone who is capable of updating their #! line to have a 3.12+ -p or python command line invoked from elsewhere to have a -p instead adding this to the same .py file:

import sys
sys.path.insert(os.path.dirname(sys.argv[0]), 0)

which is basically the same thing and works regardless of Python version. (adjust as needed based on if they wanted $CWD or script directory or "" to be added - the point being the logic that -P aka PYTHONSAFEPATH=1 now disables can be duplicated in a couple lines and is more obvious to a reader to understand than a single letter case sensitive command line flag)

@python python deleted a comment May 6, 2022
@CAM-Gerlach
Copy link
Member

But I think adding an opt-out flag to prevent cases where someone may wants to explicitly avoid that nicer world isn't super compelling. At least not any more so than someone who is capable of updating their #! line to have a 3.12+ -p or python command line invoked from elsewhere to have a -p instead adding this to the same .py file:

Yes, but many potential use cases don't necessarily involve running scripts that the user wrote themself, as opposed to, for example, invocations of a test runner or other tools, or modules, scripts or other entrypoints the user does not directly control. Furthermore, they may wish to run the same script/tool/module in different contexts, or for testing, with and without the CWD on the path without manually editing its code or adding a whole CLI of its own to trigger hacking sys.path.

It would seem to me that this opt-out option is, perhaps counterintuitively at first, actually a path closer to that better world of a safer default, because it allows users/sysadmins to just set PYTHONSAFEPATH=1 as a global default on their system with much less friction, knowing that opting out temporarily just for a particular case that needs it doesn't demand mucking about with manual sys.path hacking or require modifying the code being run.

@vstinner
Copy link
Member Author

vstinner commented May 9, 2022

@gpshead:

(...) is more obvious to a reader to understand than a single letter case sensitive command line flag

About "case sensitive": it's a deliberate choice to use -P to mean "EXCLUDE something", my initial plan was to add its companion "-p" to "INCLUDE something". Example of Unix grep using a similar command line interface:

  • grep -L = --files-without-match
  • grep -l = --files-with-matches

@vstinner
Copy link
Member Author

vstinner commented May 9, 2022

At least not any more so than someone who is capable of updating their #! line to have a 3.12+ -p or python command line invoked from elsewhere to have a -p instead adding this to the same .py file

The problem is when you simply cannot modify the source code of an application impacted by PYTHONSAFEPATH=1. For example, on Linux, a regular user cannot modify Python applications in /usr/bin/.

The current workaround is to manually unset PYTHONSAFEPATH to run these applications. Replace:

subprocess.run(["/usr/bin/app"])

with:

unsafe_env = dict(os.environ)
unsafe_env.pop("PYTHONSAFEPATH", "")
subprocess.run(["/usr/bin/app"], env=unsafe_env)

I propose instead:

subprocess.run([sys.executable, "-p", "/usr/bin/app"])

The -p option also gives the ability to put the option on the shebang. IMO it's simpler and more reliable than having to modify sys.path manually.

Add the -p option to ignore the -P option and the PYTHONSAFEPATH
environment variable. The -p option takes precedence over -P and -I
options and the PYTHONSAFEPATH environment variable.

test_cmd_line, test_cmd_line_script, test_pdb, test_runpy and
test_tools now use the -p option to ignore the PYTHONSAFEPATH
environment variable.
@vstinner
Copy link
Member Author

The purpose of the -p option is similar to the -R option which ignores the PYTHONHASHSEED environment variable: the -p option ignores PYTHONSAFEPATH environment variable.

@vstinner
Copy link
Member Author

I rebased my PR and changed it to target Python 3.12.

@vstinner
Copy link
Member Author

vstinner commented May 25, 2022

For Fedora, @hroncok and me plan to add -P to Python program shebangs: see https://fedoraproject.org/wiki/Changes/PythonSafePath

For the Fedora use case, the PYTHONSAFEPATH environment variable is not needed.

I would prefer to have PYTHONSAFEPATH env var with the -p option. Either have both in Python 3.11 (add -p option), or have none in Python 3.11 (remove PYTHONSAFEPATH env var).

@hroncok @pitrou @gpshead: What do you think? Is it better to postpone PYTHONSAFEPATH to Python 3.12 and also add -p to Python 3.12, or request Python 3.11 release manager to get an exception to add the -p option?

IMO having PYTHONSAFEPATH=1 without -p makes PYTHONSAFEPATH=1 barely usable. They are still too many programs which rely on Python 3.10 default behavior: add unsafe paths to sys.path.

@hroncok
Copy link
Contributor

hroncok commented May 27, 2022

Are you proposing reverting just the environment variable or -P as well? Honestly, for my use case in Fedora I don't care that much about the environment variable.

I get the idea that scripts that would be broken in an environment with PYTHONSAFEPATH set could use some way to communicate that in their shebang. But OTOH there are not that many scripts that could simply add a flag that is backward-incompatible with older Python versions. So if the use case is "this script imports from its directory and it must work even when PYTHONSAFEPATH is set", I guess the way to achieve that is sys.path.insert(0, os.path.dirname(__file__)) which is more explicit anyway.

So, no, I don't think that not having -p should be a blocker for including PYTHONSAFEPATH support.

As a reference, I think there are many ways scripts can get broken by PYTHON... env vars already and making such scripts immune to it by applying an alphabet soup to the shebang is not practical (and not always possible). The only way to do it is to pass -E (or other options implying -E) and that option already exists.

@vstinner
Copy link
Member Author

Are you proposing reverting just the environment variable or -P as well?

I'm happy with -P in Python 3.11, I don't want to change it :-)

The only way to do it is to pass -E (or other options implying -E) and that option already exists.

The world is complicated :-) There are use cases where you want to accept most PYTHON env vars, but not be affected by PYTHONSAFEPATH.

I guess the way to achieve that is sys.path.insert(0, os.path.dirname(file)) which is more explicit anyway.

Sometimes, you cannot modify the source code. For example, to run a /usr/bin/ application as a regular user.

The existing workaround is to run a program in its own environment and drop PYTHONSAFEPATH from this environment.

I dislike modifying sys.path manually. It's fragile. I don't know if it was already modified by something else. And it can be modified later by something else.

As an user, I would like to have a very simple way to opt-in for Python 3.10 sys.path behavior: get the "unsafe" path.

@hroncok
Copy link
Contributor

hroncok commented May 27, 2022

Sometimes, you cannot modify the source code.

If you cannot modify the source, you cannot change the shebang either. What is it that you except the user will do exactly if the the -p flag exists?

As an user, I would like to have a very simple way to opt-in for Python 3.10 sys.path behavior: get the "unsafe" path.

Like this?

$ PYTHONSAFEPATH=0 command-name-here

That however doesn't work, PYTHONSAFEPATH can be set to anything non-empty apparently, which is a tad confusing.

This works (but is not very intuitive):

$ PYTHONSAFEPATH= command-name-here

@vstinner
Copy link
Member Author

If you cannot modify the source, you cannot change the shebang either. What is it that you except the user will do exactly if the the -p flag exists?

See the PR, especially changes in tests. The -p is added to the command line to run programs implemented in Python. For example, I have no idea how the FREEZE program works and I really don't want to modify it. All I need is to add -p to its command line. No need to change its shebang.

For Tools/freeze/test/freeze.py, a shebang wouldn't help since the program is run directly with "python (...)".

That however doesn't work, PYTHONSAFEPATH can be set to anything non-empty apparently, which is a tad confusing.

Yeah, it's annoying but I chose that to mimick the design of all other PYTHON env vars. I agree that it's annoying to define an environment variable to an empty value, and sometimes it's not possible.

@vstinner
Copy link
Member Author

It seems like there is no strong requirement for adding a -p option if the Python default sys.path behavior doesn't change. We can reconsider adding -p option if the default changes tomorrow.

@vstinner vstinner closed this Jun 17, 2022
@vstinner vstinner deleted the safe_path_p_option branch June 17, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants