GNU bug report logs - #46609
Fix shell password prompt in minibuffer (bug 43302)

Previous Next

Package: emacs;

Reported by: Ryan Prior <rprior <at> protonmail.com>

Date: Thu, 18 Feb 2021 01:15:02 UTC

Severity: normal

Tags: fixed

Fixed in versions 28.1, 27.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46609 in the body.
You can then email your comments to 46609 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 01:15:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ryan Prior <rprior <at> protonmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 18 Feb 2021 01:15:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ryan Prior <rprior <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 01:13:58 +0000
The current comint-password-prompt-regexp does not tolerate newlines at
the end of the prompt, so a string like "Password:\n" will not be
recognized as a password prompt in shell-mode. Before Emacs 27
(74277b0e881) newlines were tolerated here, so this is a regression, and
as a result I would sometimes echo my password in plain text where
previously it would be hidden.

The first patch in this series updates the regexp to use the :space:
Unicode character class for trailing space characters instead of
:blank:, which includes /only/ horizontal whitespace.

The second patch updates comint-watch-for-password-prompt to trim
trailing newlines from the ~string~ argument, which avoids showing the
user a fat password prompt with newlines in the middle.

I am not a Unicode expert and don't know if there might be undesirable
side-effects from using :space: instead of :blank:. However, in my
manual testing these change give me the exact behavior I'm after.

I pair-programmed with Nick Drozd to diagnose and fix this issue. Thanks
Nick!


===File
/home/ryan/dev/emacs/patches/0001-lisp-comint.el-comint-password-prompt-regexp-Allow-e.patch===
From 6c9aa1f08b215abfb69f4f53f622289494588eea Mon Sep 17 00:00:00 2001
From: Ryan Prior <ryanprior <at> hey.com>
Subject: [PATCH 1/2] lisp/comint.el (comint-password-prompt-regexp): Allow
 ending newline.

---
 lisp/comint.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 57df6bfb19..0bc358bf51 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -378,7 +378,7 @@ This variable is buffer-local."
    "\\(?:" (regexp-opt password-word-equivalents) "\\|Response\\)"
    "\\(?:\\(?:, try\\)? *again\\| (empty for no passphrase)\\| (again)\\)?"
    ;; "[[:alpha:]]" used to be "for", which fails to match non-English.
-   "\\(?: [[:alpha:]]+ .+\\)?[[:blank:]]*[::៖][[:blank:]]*\\'")
+   "\\(?: [[:alpha:]]+ .+\\)?[[:blank:]]*[::៖][[:space:]]*\\'")
   "Regexp matching prompts for passwords in the inferior process.
 This is used by `comint-watch-for-password-prompt'."
   :version "27.1"
--
2.30.1

============================================================


===File
/home/ryan/dev/emacs/patches/0002-lisp-comint.el-comint-watch-for-password-prompt-Trim.patch===
From a9260384dfc0ced53d88380724c899f295ce0944 Mon Sep 17 00:00:00 2001
From: Ryan Prior <ryanprior <at> hey.com>
Subject: [PATCH 2/2] lisp/comint.el (comint-watch-for-password-prompt): Trim
 trailing newline(s)

---
 lisp/comint.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/comint.el b/lisp/comint.el
index 0bc358bf51..c2942a13da 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2432,6 +2432,8 @@ This function could be in the list `comint-output-filter-functions'."
                         (replace-regexp-in-string "\r" "" string)))
     (when (string-match "^[ \n\r\t\v\f\b\a]+" string)
       (setq string (replace-match "" t t string)))
+    (when (string-match "[\n]+$" string)
+      (setq string (replace-match "" t t string)))
     (let ((comint--prompt-recursion-depth (1+ comint--prompt-recursion-depth)))
       (if (> comint--prompt-recursion-depth 10)
           (message "Password prompt recursion too deep")
--
2.30.1

============================================================





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 11:45:02 GMT) Full text and rfc822 format available.

Message #8 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Ryan Prior <rprior <at> protonmail.com>
Cc: 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 12:44:32 +0100
Ryan Prior <rprior <at> protonmail.com> writes:

> The current comint-password-prompt-regexp does not tolerate newlines at
> the end of the prompt, so a string like "Password:\n" will not be
> recognized as a password prompt in shell-mode. Before Emacs 27
> (74277b0e881) newlines were tolerated here, so this is a regression, and
> as a result I would sometimes echo my password in plain text where
> previously it would be hidden.

Thanks; applied to Emacs 27 with one change:

> +    (when (string-match "[\n]+$" string)
> +      (setq string (replace-match "" t t string)))

This should probably be "\n+\\'", because we only want to remove
newlines from the end of the string, and not newlines from the middle of
the string, presumably?  ("$" means "match end of line", not "match end
of string".)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Added tag(s) fixed. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Thu, 18 Feb 2021 11:50:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.1, send any further explanations to 46609 <at> debbugs.gnu.org and Ryan Prior <rprior <at> protonmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Thu, 18 Feb 2021 11:50:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 27.1, send any further explanations to 46609 <at> debbugs.gnu.org and Ryan Prior <rprior <at> protonmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Thu, 18 Feb 2021 11:50:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 11:51:01 GMT) Full text and rfc822 format available.

Message #17 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Ryan Prior <rprior <at> protonmail.com>, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 12:50:47 +0100
On Feb 18 2021, Lars Ingebrigtsen wrote:

> Ryan Prior <rprior <at> protonmail.com> writes:
>
>> The current comint-password-prompt-regexp does not tolerate newlines at
>> the end of the prompt, so a string like "Password:\n" will not be
>> recognized as a password prompt in shell-mode. Before Emacs 27
>> (74277b0e881) newlines were tolerated here, so this is a regression, and
>> as a result I would sometimes echo my password in plain text where
>> previously it would be hidden.
>
> Thanks; applied to Emacs 27 with one change:
>
>> +    (when (string-match "[\n]+$" string)
>> +      (setq string (replace-match "" t t string)))
>
> This should probably be "\n+\\'", because we only want to remove
> newlines from the end of the string, and not newlines from the middle of
> the string, presumably?  ("$" means "match end of line", not "match end
> of string".)

The preceding line should probably use "\\`" as well.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 11:57:01 GMT) Full text and rfc822 format available.

Message #20 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Ryan Prior <rprior <at> protonmail.com>, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 12:56:37 +0100
Andreas Schwab <schwab <at> linux-m68k.org> writes:

> The preceding line should probably use "\\`" as well.

Yup, but that's not a regression, I think?  So it wouldn't be
appropriate to fix on the emacs-27 branch.  But we should fix it in
Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 14:40:01 GMT) Full text and rfc822 format available.

Message #23 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ryan Prior <rprior <at> protonmail.com>
Cc: 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 16:39:48 +0200
> Date: Thu, 18 Feb 2021 01:13:58 +0000
> From:  Ryan Prior via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> I am not a Unicode expert and don't know if there might be undesirable
> side-effects from using :space: instead of :blank:. However, in my
> manual testing these change give me the exact behavior I'm after.

I'm a bit worried by the [[:space:]]* part: it would match any number
of newlines and ^L characters, right?  Is that what we want here?

Perhaps it would be safer to just add a literal newline, and only
once?  Especially for the emacs-27 branch?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 14:43:01 GMT) Full text and rfc822 format available.

Message #26 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: rprior <at> protonmail.com, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 16:42:42 +0200
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Thu, 18 Feb 2021 12:44:32 +0100
> Cc: 46609 <at> debbugs.gnu.org
> 
> Thanks; applied to Emacs 27 with one change:
> 
> > +    (when (string-match "[\n]+$" string)
> > +      (setq string (replace-match "" t t string)))
> 
> This should probably be "\n+\\'", because we only want to remove
> newlines from the end of the string, and not newlines from the middle of
> the string, presumably?  ("$" means "match end of line", not "match end
> of string".)

Do we want to remove more than one newline?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 14:48:01 GMT) Full text and rfc822 format available.

Message #29 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Ryan Prior <rprior <at> protonmail.com>, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 15:47:39 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> I am not a Unicode expert and don't know if there might be undesirable
>> side-effects from using :space: instead of :blank:. However, in my
>> manual testing these change give me the exact behavior I'm after.
>
> I'm a bit worried by the [[:space:]]* part: it would match any number
> of newlines and ^L characters, right?  Is that what we want here?

I think so -- matching "Password: \n\n" makes as much sense as matching
"Password: \n", I think?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 15:02:02 GMT) Full text and rfc822 format available.

Message #32 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: rprior <at> protonmail.com, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 17:01:53 +0200
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Ryan Prior <rprior <at> protonmail.com>,  46609 <at> debbugs.gnu.org
> Date: Thu, 18 Feb 2021 15:47:39 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> I am not a Unicode expert and don't know if there might be undesirable
> >> side-effects from using :space: instead of :blank:. However, in my
> >> manual testing these change give me the exact behavior I'm after.
> >
> > I'm a bit worried by the [[:space:]]* part: it would match any number
> > of newlines and ^L characters, right?  Is that what we want here?
> 
> I think so -- matching "Password: \n\n" makes as much sense as matching
> "Password: \n", I think?

It actually happens in Real Life?

What bothers me is that we could takes something unrelated as a
password prompt.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 15:28:01 GMT) Full text and rfc822 format available.

Message #35 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rprior <at> protonmail.com, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 16:26:53 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> I think so -- matching "Password: \n\n" makes as much sense as matching
>> "Password: \n", I think?
>
> It actually happens in Real Life?

I wouldn't have thought so, but then again, I wouldn't have thought that
anybody had even a single newline in the prompt, so my intuitions here
don't count for much.  

> What bothers me is that we could takes something unrelated as a
> password prompt.

If I understand the comint code correctly, it'll match this stuff if
it's the final thing that has been output.  So there should be little
danger in faulty matching here.  But I'm not overly confident in my
understanding of that code.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46609; Package emacs. (Thu, 18 Feb 2021 15:30:02 GMT) Full text and rfc822 format available.

Message #38 received at 46609 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rprior <at> protonmail.com, 46609 <at> debbugs.gnu.org
Subject: Re: bug#46609: Fix shell password prompt in minibuffer (bug 43302)
Date: Thu, 18 Feb 2021 16:29:40 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> > +    (when (string-match "[\n]+$" string)
>> > +      (setq string (replace-match "" t t string)))
>> 
>> This should probably be "\n+\\'", because we only want to remove
>> newlines from the end of the string, and not newlines from the middle of
>> the string, presumably?  ("$" means "match end of line", not "match end
>> of string".)
>
> Do we want to remove more than one newline?

It's just used as the prompt for a `read-string' call, so I think so?
Having a prompt end with a newline is just confusing.

(This (and the preceding two lines) should just be rewritten to use
`string-trim'.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 19 Mar 2021 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 9 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.