* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' @ 2024-04-17 18:23 Augusto Stoffel 2024-04-17 19:13 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-17 18:23 UTC (permalink / raw) To: 70440 [-- Attachment #1: Type: text/plain, Size: 80 bytes --] Tags: patch The attachment should be self-explanatory, otherwise let me know. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Use-P-switch-when-calling-python-interpreter.patch --] [-- Type: text/patch, Size: 2551 bytes --] From 04db8a3fbb29f497fdc728f4413aea162650b30b Mon Sep 17 00:00:00 2001 From: Augusto Stoffel <arstoffel@gmail.com> Date: Wed, 17 Apr 2024 20:17:22 +0200 Subject: [PATCH] Use -P switch when calling 'python-interpreter' This excludes the current directory from Python's module load path, which can be unsafe. * lisp/progmodes/python.el (python--list-imports): Use -P switch (python--do-isort): Use -P switch (python-fix-imports): Use -P switch --- lisp/progmodes/python.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el index 85279d3e84b..180a8357aad 100644 --- a/lisp/progmodes/python.el +++ b/lisp/progmodes/python.el @@ -6805,7 +6805,7 @@ python--list-imports (append (split-string-shell-command python-interpreter-args) - `("-c" ,python--list-imports) + `("-Pc" ,python--list-imports) (list (or name ""))))) (with-current-buffer buffer (apply #'call-process @@ -6814,7 +6814,7 @@ python--list-imports (append (split-string-shell-command python-interpreter-args) - `("-c" ,python--list-imports) + `("-Pc" ,python--list-imports) (list (or name "")) (mapcar #'file-local-name source)))))) lines) @@ -6862,7 +6862,7 @@ python--do-isort (append (split-string-shell-command python-interpreter-args) - '("-m" "isort" "-") + '("-Pm" "isort" "-") args))) (tick (buffer-chars-modified-tick))) (unless (eq 0 status) @@ -6940,7 +6940,7 @@ python-fix-imports (append (split-string-shell-command python-interpreter-args) - '("-m" "pyflakes")))) + '("-Pm" "pyflakes")))) (goto-char (point-min)) (when (looking-at-p ".* No module named pyflakes$") (error "%s couldn't find pyflakes" python-interpreter)) -- 2.44.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-17 18:23 bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' Augusto Stoffel @ 2024-04-17 19:13 ` Eli Zaretskii 2024-04-18 15:25 ` kobarity 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-17 19:13 UTC (permalink / raw) To: Augusto Stoffel, kobarity; +Cc: 70440 > From: Augusto Stoffel <arstoffel@gmail.com> > Date: Wed, 17 Apr 2024 20:23:30 +0200 > > The attachment should be self-explanatory, otherwise let me know. > > > >From 04db8a3fbb29f497fdc728f4413aea162650b30b Mon Sep 17 00:00:00 2001 > From: Augusto Stoffel <arstoffel@gmail.com> > Date: Wed, 17 Apr 2024 20:17:22 +0200 > Subject: [PATCH] Use -P switch when calling 'python-interpreter' > > This excludes the current directory from Python's module load path, > which can be unsafe. > > * lisp/progmodes/python.el (python--list-imports): Use -P switch > (python--do-isort): Use -P switch > (python-fix-imports): Use -P switch > --- > lisp/progmodes/python.el | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el > index 85279d3e84b..180a8357aad 100644 > --- a/lisp/progmodes/python.el > +++ b/lisp/progmodes/python.el > @@ -6805,7 +6805,7 @@ python--list-imports > (append > (split-string-shell-command > python-interpreter-args) > - `("-c" ,python--list-imports) > + `("-Pc" ,python--list-imports) > (list (or name ""))))) > (with-current-buffer buffer > (apply #'call-process > @@ -6814,7 +6814,7 @@ python--list-imports > (append > (split-string-shell-command > python-interpreter-args) > - `("-c" ,python--list-imports) > + `("-Pc" ,python--list-imports) > (list (or name "")) > (mapcar #'file-local-name source)))))) > lines) > @@ -6862,7 +6862,7 @@ python--do-isort > (append > (split-string-shell-command > python-interpreter-args) > - '("-m" "isort" "-") > + '("-Pm" "isort" "-") > args))) > (tick (buffer-chars-modified-tick))) > (unless (eq 0 status) > @@ -6940,7 +6940,7 @@ python-fix-imports > (append > (split-string-shell-command > python-interpreter-args) > - '("-m" "pyflakes")))) > + '("-Pm" "pyflakes")))) > (goto-char (point-min)) > (when (looking-at-p ".* No module named pyflakes$") > (error "%s couldn't find pyflakes" python-interpreter)) > -- > 2.44.0 > Thanks. kobarity, any comments? ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-17 19:13 ` Eli Zaretskii @ 2024-04-18 15:25 ` kobarity 2024-04-18 15:52 ` Augusto Stoffel 2024-04-19 6:08 ` Augusto Stoffel 0 siblings, 2 replies; 17+ messages in thread From: kobarity @ 2024-04-18 15:25 UTC (permalink / raw) To: Augusto Stoffel, Eli Zaretskii; +Cc: 70440 Eli Zaretskii wrote: > > From: Augusto Stoffel <arstoffel@gmail.com> > > Date: Wed, 17 Apr 2024 20:23:30 +0200 > > > > The attachment should be self-explanatory, otherwise let me know. > > > > > > >From 04db8a3fbb29f497fdc728f4413aea162650b30b Mon Sep 17 00:00:00 2001 > > From: Augusto Stoffel <arstoffel@gmail.com> > > Date: Wed, 17 Apr 2024 20:17:22 +0200 > > Subject: [PATCH] Use -P switch when calling 'python-interpreter' > > > > This excludes the current directory from Python's module load path, > > which can be unsafe. > > > > * lisp/progmodes/python.el (python--list-imports): Use -P switch > > (python--do-isort): Use -P switch > > (python-fix-imports): Use -P switch > > --- > > lisp/progmodes/python.el | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el > > index 85279d3e84b..180a8357aad 100644 > > --- a/lisp/progmodes/python.el > > +++ b/lisp/progmodes/python.el > > @@ -6805,7 +6805,7 @@ python--list-imports > > (append > > (split-string-shell-command > > python-interpreter-args) > > - `("-c" ,python--list-imports) > > + `("-Pc" ,python--list-imports) > > (list (or name ""))))) > > (with-current-buffer buffer > > (apply #'call-process > > @@ -6814,7 +6814,7 @@ python--list-imports > > (append > > (split-string-shell-command > > python-interpreter-args) > > - `("-c" ,python--list-imports) > > + `("-Pc" ,python--list-imports) > > (list (or name "")) > > (mapcar #'file-local-name source)))))) > > lines) > > @@ -6862,7 +6862,7 @@ python--do-isort > > (append > > (split-string-shell-command > > python-interpreter-args) > > - '("-m" "isort" "-") > > + '("-Pm" "isort" "-") > > args))) > > (tick (buffer-chars-modified-tick))) > > (unless (eq 0 status) > > @@ -6940,7 +6940,7 @@ python-fix-imports > > (append > > (split-string-shell-command > > python-interpreter-args) > > - '("-m" "pyflakes")))) > > + '("-Pm" "pyflakes")))) > > (goto-char (point-min)) > > (when (looking-at-p ".* No module named pyflakes$") > > (error "%s couldn't find pyflakes" python-interpreter)) > > -- > > 2.44.0 > > > > Thanks. > > kobarity, any comments? The -P switch is new, introduced in CPython 3.11, so I don't think it can be added unconditionally. Furthermore, `python-interpreter' may not be CPython. Isn't it enough to customize `python-interpreter-args'? ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-18 15:25 ` kobarity @ 2024-04-18 15:52 ` Augusto Stoffel 2024-04-18 15:57 ` Eli Zaretskii 2024-04-19 6:08 ` Augusto Stoffel 1 sibling, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-18 15:52 UTC (permalink / raw) To: kobarity; +Cc: 70440, Eli Zaretskii On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > The -P switch is new, introduced in CPython 3.11, so I don't think it > can be added unconditionally. Furthermore, `python-interpreter' may > not be CPython. Isn't it enough to customize > `python-interpreter-args'? Ah, too bad. So let's send that option via environment variables. I'll make a new patch when I get the chance. This patch is a quite important fix and shouldn't be left to a customization. Without it, you need to trust the .py files in the current directory. Moreover any name clashes with the built-in library module names can crash the commands (just happened to me). ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-18 15:52 ` Augusto Stoffel @ 2024-04-18 15:57 ` Eli Zaretskii 2024-04-18 16:02 ` Augusto Stoffel 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-18 15:57 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org > Date: Thu, 18 Apr 2024 17:52:21 +0200 > > On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > > > The -P switch is new, introduced in CPython 3.11, so I don't think it > > can be added unconditionally. Furthermore, `python-interpreter' may > > not be CPython. Isn't it enough to customize > > `python-interpreter-args'? > > Ah, too bad. So let's send that option via environment variables. I'll > make a new patch when I get the chance. Maybe we should discuss this before you sit down to write and text the code. Pushing things into the environment has its downsides: those environment variables then affect all the subordinate processes, including their children, grandchildren etc. This is not always wanted. One alternative to environment variables would be detecting whether -P is supported before the first time we invoke Python. We do similar stuff for Grep and other programs we invoke. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-18 15:57 ` Eli Zaretskii @ 2024-04-18 16:02 ` Augusto Stoffel 2024-04-18 16:13 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-18 16:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70440, kobarity On Thu, 18 Apr 2024 at 18:57, Eli Zaretskii wrote: >> From: Augusto Stoffel <arstoffel@gmail.com> >> Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org >> Date: Thu, 18 Apr 2024 17:52:21 +0200 >> >> On Fri, 19 Apr 2024 at 00:25, kobarity wrote: >> >> > The -P switch is new, introduced in CPython 3.11, so I don't think it >> > can be added unconditionally. Furthermore, `python-interpreter' may >> > not be CPython. Isn't it enough to customize >> > `python-interpreter-args'? >> >> Ah, too bad. So let's send that option via environment variables. I'll >> make a new patch when I get the chance. > > Maybe we should discuss this before you sit down to write and text the > code. Pushing things into the environment has its downsides: those > environment variables then affect all the subordinate processes, > including their children, grandchildren etc. This is not always > wanted. In this case we would just let-bind the env variable for this specific subprocess call only. It's no different than passing a command-line switch. > One alternative to environment variables would be detecting whether -P > is supported before the first time we invoke Python. We do similar > stuff for Grep and other programs we invoke. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-18 16:02 ` Augusto Stoffel @ 2024-04-18 16:13 ` Eli Zaretskii 0 siblings, 0 replies; 17+ messages in thread From: Eli Zaretskii @ 2024-04-18 16:13 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: kobarity@gmail.com, 70440@debbugs.gnu.org > Date: Thu, 18 Apr 2024 18:02:34 +0200 > > On Thu, 18 Apr 2024 at 18:57, Eli Zaretskii wrote: > > >> From: Augusto Stoffel <arstoffel@gmail.com> > >> Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org > >> Date: Thu, 18 Apr 2024 17:52:21 +0200 > >> > >> On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > >> > >> > The -P switch is new, introduced in CPython 3.11, so I don't think it > >> > can be added unconditionally. Furthermore, `python-interpreter' may > >> > not be CPython. Isn't it enough to customize > >> > `python-interpreter-args'? > >> > >> Ah, too bad. So let's send that option via environment variables. I'll > >> make a new patch when I get the chance. > > > > Maybe we should discuss this before you sit down to write and text the > > code. Pushing things into the environment has its downsides: those > > environment variables then affect all the subordinate processes, > > including their children, grandchildren etc. This is not always > > wanted. > > In this case we would just let-bind the env variable for this specific > subprocess call only. It's no different than passing a command-line > switch. Don't then the child processes of that python subprocess inherit the same variable in their environments? That's what I meant by what I wrote above: all of the descendants of our sub-process will inherit the variable. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-18 15:25 ` kobarity 2024-04-18 15:52 ` Augusto Stoffel @ 2024-04-19 6:08 ` Augusto Stoffel 2024-04-19 7:15 ` Eli Zaretskii 1 sibling, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-19 6:08 UTC (permalink / raw) To: kobarity; +Cc: 70440, Eli Zaretskii [-- Attachment #1: Type: text/plain, Size: 1146 bytes --] On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > The -P switch is new, introduced in CPython 3.11, so I don't think it > can be added unconditionally. Furthermore, `python-interpreter' may > not be CPython. Isn't it enough to customize > `python-interpreter-args'? After sleeping on this, I recommend using -P anyway and simply failing if the installed Python is too old. The reason is that this has a security implication, similar to the recent Org mode Latex preview situation. Without -P the user is tacitly trusting the contents of the current directory. By tricking an user into downloading a malicious file with an intentional name clash (say via git pull), arbitrary code could in principle be executed on the user's machine. The -P switch completely removes this possibility, and conversely, without -P there seems to be no reasonable way to make Python safe. I've attached a new patch that informs the user why the commands failed when Python is too old, which is good enough in my opinion. Note also that this change only affects the Python import management commands, which is a very handy but by no means essential feature. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Use-P-switch-when-calling-python-interpreter.patch --] [-- Type: text/x-patch, Size: 3474 bytes --] From 2cca02440069a31546eff04c8cd6c00b171a85a2 Mon Sep 17 00:00:00 2001 From: Augusto Stoffel <arstoffel@gmail.com> Date: Wed, 17 Apr 2024 20:17:22 +0200 Subject: [PATCH] Use -P switch when calling 'python-interpreter' This excludes the current directory from Python's module load path, which can be unsafe. * lisp/progmodes/python.el (python--list-imports, python--do-isort), (python-fix-imports): Use -P switch (python--list-imports-check-status): Warn about old Python versions missing the -P switch. --- lisp/progmodes/python.el | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el index 85279d3e84b..304aa2d9d6e 100644 --- a/lisp/progmodes/python.el +++ b/lisp/progmodes/python.el @@ -6744,9 +6744,9 @@ python--list-imports try: from isort import find_imports_in_stream, find_imports_in_paths except ModuleNotFoundError: - exit(2) -except ImportError: exit(3) +except ImportError: + exit(4) query, files, result = argv[1] or None, argv[2:], {} @@ -6781,8 +6781,9 @@ python--list-imports-check-status (unless (eq 0 status) (let* ((details (cond - ((eq 2 status) " (maybe isort is missing?)") - ((eq 3 status) " (maybe isort version is older than 5.7.0?)") + ((eq 2 status) " (maybe Python version is older than 3.11?)") + ((eq 3 status) " (maybe isort is missing?)") + ((eq 4 status) " (maybe isort version is older than 5.7.0?)") (t ""))) (msg (concat "%s exited with status %s" details))) @@ -6805,7 +6806,7 @@ python--list-imports (append (split-string-shell-command python-interpreter-args) - `("-c" ,python--list-imports) + `("-Pc" ,python--list-imports) (list (or name ""))))) (with-current-buffer buffer (apply #'call-process @@ -6814,7 +6815,7 @@ python--list-imports (append (split-string-shell-command python-interpreter-args) - `("-c" ,python--list-imports) + `("-Pc" ,python--list-imports) (list (or name "")) (mapcar #'file-local-name source)))))) lines) @@ -6862,7 +6863,7 @@ python--do-isort (append (split-string-shell-command python-interpreter-args) - '("-m" "isort" "-") + '("-Pm" "isort" "-") args))) (tick (buffer-chars-modified-tick))) (unless (eq 0 status) @@ -6940,7 +6941,7 @@ python-fix-imports (append (split-string-shell-command python-interpreter-args) - '("-m" "pyflakes")))) + '("-Pm" "pyflakes")))) (goto-char (point-min)) (when (looking-at-p ".* No module named pyflakes$") (error "%s couldn't find pyflakes" python-interpreter)) -- 2.44.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 6:08 ` Augusto Stoffel @ 2024-04-19 7:15 ` Eli Zaretskii 2024-04-19 15:21 ` Augusto Stoffel 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-19 7:15 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org > Date: Fri, 19 Apr 2024 08:08:43 +0200 > > On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > > > The -P switch is new, introduced in CPython 3.11, so I don't think it > > can be added unconditionally. Furthermore, `python-interpreter' may > > not be CPython. Isn't it enough to customize > > `python-interpreter-args'? > > After sleeping on this, I recommend using -P anyway and simply failing > if the installed Python is too old. > > The reason is that this has a security implication, similar to the > recent Org mode Latex preview situation. Without -P the user is tacitly > trusting the contents of the current directory. By tricking an user > into downloading a malicious file with an intentional name clash (say > via git pull), arbitrary code could in principle be executed on the > user's machine. > > The -P switch completely removes this possibility, and conversely, > without -P there seems to be no reasonable way to make Python safe. > > I've attached a new patch that informs the user why the commands failed > when Python is too old, which is good enough in my opinion. Note also > that this change only affects the Python import management commands, > which is a very handy but by no means essential feature. Doing it this way would be an annoyance. Users could have less-than-the-latest Python (or non-CPython version) installed for any number of reasons, and it is not our business to annoy them because of this. Security of using Python is not our concern, it is the user's concern. So I'd prefer that the change probed the support for the -P switch when the relevant Emacs commands/functions are first invoked, and used that if -P is supported, without any annoying messages. Do you see any problems with such an approach? Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 7:15 ` Eli Zaretskii @ 2024-04-19 15:21 ` Augusto Stoffel 2024-04-19 15:40 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-19 15:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70440, kobarity On Fri, 19 Apr 2024 at 10:15, Eli Zaretskii wrote: >> From: Augusto Stoffel <arstoffel@gmail.com> >> Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org >> Date: Fri, 19 Apr 2024 08:08:43 +0200 >> >> On Fri, 19 Apr 2024 at 00:25, kobarity wrote: >> >> > The -P switch is new, introduced in CPython 3.11, so I don't think it >> > can be added unconditionally. Furthermore, `python-interpreter' may >> > not be CPython. Isn't it enough to customize >> > `python-interpreter-args'? >> >> After sleeping on this, I recommend using -P anyway and simply failing >> if the installed Python is too old. >> >> The reason is that this has a security implication, similar to the >> recent Org mode Latex preview situation. Without -P the user is tacitly >> trusting the contents of the current directory. By tricking an user >> into downloading a malicious file with an intentional name clash (say >> via git pull), arbitrary code could in principle be executed on the >> user's machine. >> >> The -P switch completely removes this possibility, and conversely, >> without -P there seems to be no reasonable way to make Python safe. >> >> I've attached a new patch that informs the user why the commands failed >> when Python is too old, which is good enough in my opinion. Note also >> that this change only affects the Python import management commands, >> which is a very handy but by no means essential feature. > > Doing it this way would be an annoyance. Users could have > less-than-the-latest Python (or non-CPython version) installed for any > number of reasons, and it is not our business to annoy them because of > this. Security of using Python is not our concern, it is the user's > concern. > > So I'd prefer that the change probed the support for the -P switch > when the relevant Emacs commands/functions are first invoked, and used > that if -P is supported, without any annoying messages. Do you see > any problems with such an approach? > > Thanks. Okay, you are the maintainer, but I hope I explained well that this is a security hole. (Apart from the security aspect, without -P the tool will just mysteriously stop working if a file with a name such as csv.py is added to the project; that's what happened to me. Perhaps outright not working and explaining why is not as bad as working fine until it doesn't anymore.) ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 15:21 ` Augusto Stoffel @ 2024-04-19 15:40 ` Eli Zaretskii 2024-04-19 15:55 ` Augusto Stoffel 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-19 15:40 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: kobarity@gmail.com, 70440@debbugs.gnu.org > Date: Fri, 19 Apr 2024 17:21:48 +0200 > > On Fri, 19 Apr 2024 at 10:15, Eli Zaretskii wrote: > > >> From: Augusto Stoffel <arstoffel@gmail.com> > >> Cc: Eli Zaretskii <eliz@gnu.org>, 70440@debbugs.gnu.org > >> Date: Fri, 19 Apr 2024 08:08:43 +0200 > >> > >> On Fri, 19 Apr 2024 at 00:25, kobarity wrote: > >> > >> > The -P switch is new, introduced in CPython 3.11, so I don't think it > >> > can be added unconditionally. Furthermore, `python-interpreter' may > >> > not be CPython. Isn't it enough to customize > >> > `python-interpreter-args'? > >> > >> After sleeping on this, I recommend using -P anyway and simply failing > >> if the installed Python is too old. > >> > >> The reason is that this has a security implication, similar to the > >> recent Org mode Latex preview situation. Without -P the user is tacitly > >> trusting the contents of the current directory. By tricking an user > >> into downloading a malicious file with an intentional name clash (say > >> via git pull), arbitrary code could in principle be executed on the > >> user's machine. > >> > >> The -P switch completely removes this possibility, and conversely, > >> without -P there seems to be no reasonable way to make Python safe. > >> > >> I've attached a new patch that informs the user why the commands failed > >> when Python is too old, which is good enough in my opinion. Note also > >> that this change only affects the Python import management commands, > >> which is a very handy but by no means essential feature. > > > > Doing it this way would be an annoyance. Users could have > > less-than-the-latest Python (or non-CPython version) installed for any > > number of reasons, and it is not our business to annoy them because of > > this. Security of using Python is not our concern, it is the user's > > concern. > > > > So I'd prefer that the change probed the support for the -P switch > > when the relevant Emacs commands/functions are first invoked, and used > > that if -P is supported, without any annoying messages. Do you see > > any problems with such an approach? > > > > Thanks. > > Okay, you are the maintainer, but I hope I explained well that this is a > security hole. I'm not sure I understand: if the user doesn't have a version of Python which supports this option, what else can we do? Refuse to use such a Python? That doesn't seem to be an option we can use. Yes, this is a security hole, but it's the user's security hole, not ours, if the user doesn't install the safer Python. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 15:40 ` Eli Zaretskii @ 2024-04-19 15:55 ` Augusto Stoffel 2024-04-19 17:31 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-19 15:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70440, kobarity On Fri, 19 Apr 2024 at 18:40, Eli Zaretskii wrote: > I'm not sure I understand: if the user doesn't have a version of > Python which supports this option, what else can we do? Refuse to > use such a Python? That doesn't seem to be an option we can use. Why not? Let me make sure we're on the same page that this affects only couple of handy but by no means essential commands that add or remove import statements. Nobody _needs_ this to write Python code. > Yes, this is a security hole, but it's the user's security hole, not > ours, if the user doesn't install the safer Python. I see it as _my_ security hole, since it was me who added a line to Emacs that calls 'python -c' in a random directory without removing the current directory from the module load path (as much as a find it a bad design choice in Python to do that by default.) ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 15:55 ` Augusto Stoffel @ 2024-04-19 17:31 ` Eli Zaretskii 2024-04-19 18:02 ` Augusto Stoffel 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-19 17:31 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: kobarity@gmail.com, 70440@debbugs.gnu.org > Date: Fri, 19 Apr 2024 17:55:51 +0200 > > On Fri, 19 Apr 2024 at 18:40, Eli Zaretskii wrote: > > > I'm not sure I understand: if the user doesn't have a version of > > Python which supports this option, what else can we do? Refuse to > > use such a Python? That doesn't seem to be an option we can use. > > Why not? Because it's unthinkable? > Let me make sure we're on the same page that this affects only > couple of handy but by no means essential commands that add or remove > import statements. Nobody _needs_ this to write Python code. That's not relevant. The important part is that if we accept your proposal, users who have Python that doesn't support -P will be unable to invoke Python even if their Python programs don't use the problematic features. > > Yes, this is a security hole, but it's the user's security hole, not > > ours, if the user doesn't install the safer Python. > > I see it as _my_ security hole, since it was me who added a line to > Emacs that calls 'python -c' in a random directory without removing the > current directory from the module load path (as much as a find it a bad > design choice in Python to do that by default.) Sorry, you lost me here. But if there's a way to close the hole without preventing users to use a subordinate Python, please describe it in more details. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 17:31 ` Eli Zaretskii @ 2024-04-19 18:02 ` Augusto Stoffel 2024-04-19 18:17 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-19 18:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70440, kobarity On Fri, 19 Apr 2024 at 20:31, Eli Zaretskii wrote: >> Let me make sure we're on the same page that this affects only >> couple of handy but by no means essential commands that add or remove >> import statements. Nobody _needs_ this to write Python code. > > That's not relevant. The important part is that if we accept your > proposal, users who have Python that doesn't support -P will be unable > to invoke Python even if their Python programs don't use the > problematic features. That's a misunderstanding. My patch only affects the following commands: python-sort-imports python-add-import python-fix-imports python-remove-import python-import-symbol-at-point There's no change in any customization option or in the way Python is invoked for any other purpose. Of course it would be unreasonable to change the way other Python subprocesses are invoked. ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 18:02 ` Augusto Stoffel @ 2024-04-19 18:17 ` Eli Zaretskii 2024-04-19 18:30 ` Augusto Stoffel 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2024-04-19 18:17 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: kobarity@gmail.com, 70440@debbugs.gnu.org > Date: Fri, 19 Apr 2024 20:02:16 +0200 > > On Fri, 19 Apr 2024 at 20:31, Eli Zaretskii wrote: > > >> Let me make sure we're on the same page that this affects only > >> couple of handy but by no means essential commands that add or remove > >> import statements. Nobody _needs_ this to write Python code. > > > > That's not relevant. The important part is that if we accept your > > proposal, users who have Python that doesn't support -P will be unable > > to invoke Python even if their Python programs don't use the > > problematic features. > > That's a misunderstanding. My patch only affects the following > commands: > > python-sort-imports > python-add-import > python-fix-imports > python-remove-import > python-import-symbol-at-point Affects in what way? AFAIU, with your patch these commands will not work if Python doesn't support -P. Is that right? ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 18:17 ` Eli Zaretskii @ 2024-04-19 18:30 ` Augusto Stoffel 2024-04-19 19:01 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Augusto Stoffel @ 2024-04-19 18:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70440, kobarity On Fri, 19 Apr 2024 at 21:17, Eli Zaretskii wrote: >> That's a misunderstanding. My patch only affects the following >> commands: >> >> python-sort-imports >> python-add-import >> python-fix-imports >> python-remove-import >> python-import-symbol-at-point > > Affects in what way? AFAIU, with your patch these commands will not > work if Python doesn't support -P. Is that right? Yes. (And again, that's not a gratuitous choice.) ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' 2024-04-19 18:30 ` Augusto Stoffel @ 2024-04-19 19:01 ` Eli Zaretskii 0 siblings, 0 replies; 17+ messages in thread From: Eli Zaretskii @ 2024-04-19 19:01 UTC (permalink / raw) To: Augusto Stoffel; +Cc: 70440, kobarity > From: Augusto Stoffel <arstoffel@gmail.com> > Cc: kobarity@gmail.com, 70440@debbugs.gnu.org > Date: Fri, 19 Apr 2024 20:30:27 +0200 > > On Fri, 19 Apr 2024 at 21:17, Eli Zaretskii wrote: > > >> That's a misunderstanding. My patch only affects the following > >> commands: > >> > >> python-sort-imports > >> python-add-import > >> python-fix-imports > >> python-remove-import > >> python-import-symbol-at-point > > > > Affects in what way? AFAIU, with your patch these commands will not > > work if Python doesn't support -P. Is that right? > > Yes. That's what IMO is unthinkable: we cannot possibly force the user's hand that much, especially since this used to work before. Users need to learn about the problem and install a newer Python, and then the security hole will be gone, at least as far as invoking Python from Emacs is concerned. The most we can do is urge the users to upgrade in the NEWS entry where we announce this new feature. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-04-19 19:01 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-17 18:23 bug#70440: [PATCH] Use -P switch when calling 'python-interpreter' Augusto Stoffel 2024-04-17 19:13 ` Eli Zaretskii 2024-04-18 15:25 ` kobarity 2024-04-18 15:52 ` Augusto Stoffel 2024-04-18 15:57 ` Eli Zaretskii 2024-04-18 16:02 ` Augusto Stoffel 2024-04-18 16:13 ` Eli Zaretskii 2024-04-19 6:08 ` Augusto Stoffel 2024-04-19 7:15 ` Eli Zaretskii 2024-04-19 15:21 ` Augusto Stoffel 2024-04-19 15:40 ` Eli Zaretskii 2024-04-19 15:55 ` Augusto Stoffel 2024-04-19 17:31 ` Eli Zaretskii 2024-04-19 18:02 ` Augusto Stoffel 2024-04-19 18:17 ` Eli Zaretskii 2024-04-19 18:30 ` Augusto Stoffel 2024-04-19 19:01 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).