GNU bug report logs - #30190
27.0.50; term run in line mode shows user passwords

Previous Next

Full log

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 30190 <at> debbugs.gnu.org,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#30190: 27.0.50; term run in line mode shows user passwords
Date: Wed, 18 Jul 2018 10:24:14 -0400

Thanks Noam, looks good.
Couldn't help send you some nitpicks, tho,


        Stefan


> @@ -2288,7 +2289,8 @@ term-send-invisible
>  \\[view-lossage]."
>    (interactive "P") ; Defeat snooping via C-x esc
>    (when (not (stringp str))
> -    (setq str (term-read-noecho "Non-echoed text: " t)))
> +    (let ((read-hide-char ?*))
> +      (setq str (read-passwd "Non-echoed text: "))))
>    (when (not proc)
>      (setq proc (get-buffer-process (current-buffer))))
>    (if (not proc) (error "Current buffer has no process")

Why do we need to bind `read-hide-char` here?
More specifically, shouldn't `read-passwd` do that for us (hence if it
doesn't yet, then the right patch is to add this let-binding to
`read-passwd`)?

> @@ -2297,6 +2299,17 @@ term-send-invisible
>      (term-send-string proc str)
>      (term-send-string proc "\n")))
>  
> +;; TODO: Maybe combine this with `comint-watch-for-password-prompt'.

Would be nice, yes.

> +(defun term-watch-for-password-prompt (string)
> +  "Prompt in the minibuffer for password and send without echoing.
> +This function uses `term-send-invisible' to read and send a password to the buffer's
> +process if STRING contains a password prompt defined by
> +`comint-password-prompt-regexp'."

"... uses `term-send-invisible' to read [...] a password ..." is
incorrect, since the password is read by `read-passwd` rather than by
term-send-invisible.  But in any case I don't see any reason to document
in the docstring what internal mechanism is used [ I just fixed the comint
version of the function accordingly.  ]

> @@ -3152,6 +3165,9 @@ term-emulate-terminal
>  	  (term-handle-deferred-scroll))
>  
>  	(set-marker (process-mark proc) (point))
> +        (when (stringp decoded-substring)
> +          (term-watch-for-password-prompt (prog1 decoded-substring
> +                                            (setq decoded-substring nil))))

I suggest you add a comment explaining why we set decoded-substring to nil.

Previous Next

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