unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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).