bug#74218: [PATCH] Ask confirmation before sending region to search engine.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

* lisp/net/eww.el (eww-search-confirm-send-region,
eww-search-words): With 'eww-search-words' (by default bound to 'M-s
M-w') a user can type in some search terms and get back the results
of a web search from a predefined search engine. If a region is
selected, 'eww-search-words' will use that for the web search
instead of prompting the user.

In its current form, 'eww-search-words' presents a security and
usability problem. It is relatively too easy to mistakenly launch
the function and, if a region of text is selected, have potentially
sensitive data sent out to a third-party service.

This commit changes the search function's default behaviour so that
explicit confirmation is required before a region is sent to a
search engine. The behaviour can be adjusted via the
newly-introduced 'eww-search-confirm-send-region' variable, which is
set to true by default.
---
Hiya,

This is to change the default behaviour of the 'eww-search-words' function. The
provided commit message provides some context around why I think the change is
necessary.

I tentatively marked 'eww-search-confirm-send-region' as introduced in 30.0. Let
me know if and when you think it makes sense to merge this and therefore whether
30.0 should be changed to any later number.

I hope the commit looks alright but should any change be needed, please just let
me know. This is my first commit to Emacs - any feedback is more than welcome!

Thanks, best wishes, Fabio.


 lisp/net/eww.el | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 2d351dff88f..8f503757f68 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -52,6 +52,15 @@
   :group 'eww
   :type 'string)
 
+(defcustom eww-search-confirm-send-region t
+  "Non-nil if Emacs should confirm sending the selected region to
+the configured search engine.  This is the default to mitigate the
+risk of accidental data leak.  Set this variable to nil to send
+the region to the search engine straightaway."
+  :version "30.0"
+  :group 'eww
+  :type 'boolean)
+
 (defcustom eww-search-prefix "https://duckduckgo.com/html/?q="
   "Prefix URL to search engine."
   :version "24.4"
@@ -603,10 +612,15 @@ user for a search string.  See the variable `eww-search-prefix'
 for the search engine used."
   (interactive)
   (if (use-region-p)
-      (let ((region-string (buffer-substring (region-beginning) (region-end))))
-        (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
-            (eww region-string)
-          (call-interactively #'eww)))
+      (when (or (not eww-search-confirm-send-region)
+                (yes-or-no-p
+                 (format-message
+                  "Send region to the configured search engine? ")))
+        (let ((region-string (buffer-substring (region-beginning)
+                                               (region-end))))
+          (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
+              (eww region-string)
+            (call-interactively #'eww))))
     (call-interactively #'eww)))
 
 (defun eww--open-url-in-new-buffer (url)
-- 
2.46.0

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> Cc: Fabio Natali <me@fabionatali.com>
> Date: Wed,  6 Nov 2024 00:46:46 +0000
> From:  Fabio Natali via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> * lisp/net/eww.el (eww-search-confirm-send-region,
> eww-search-words): With 'eww-search-words' (by default bound to 'M-s
> M-w') a user can type in some search terms and get back the results
> of a web search from a predefined search engine. If a region is
> selected, 'eww-search-words' will use that for the web search
> instead of prompting the user.

This should be reformatted according to our conventions, see
CONTRIBUTE.

> +(defcustom eww-search-confirm-send-region t
> +  "Non-nil if Emacs should confirm sending the selected region to
> +the configured search engine.  This is the default to mitigate the

The first line of a doc string should be a single complete sentence,
and should attempt to summarize what the function/variable does,
because some "apropos" commands show only the first line of each doc
string.

> +risk of accidental data leak.  Set this variable to nil to send
> +the region to the search engine straightaway."
> +  :version "30.0"

This should be "31.1".

> +      (when (or (not eww-search-confirm-send-region)
> +                (yes-or-no-p
> +                 (format-message
> +                  "Send region to the configured search engine? ")))

IMO, this should somehow try to indicate the problematic aspect of
doing this.  For example, maybe it should say

       Really send the entire region to the search engine?

It is also possible that short regions should be sent without any need
for confirmation.  In which case perhaps the variable should allow
integer values, not just nil and t.

In addition, I don't see any need to ask for confirmation when we are
not going to send anything to the search engine, so I think the test
for white-space region should be before the confirmation prompt, and
only if the region is going to be sent.

Last, but not least: this contribution almost exhausts the amount of
changes we can accept from you without a copyright assignment.  Would
you like to start at this time your legal paperwork of assigning the
copyright to the FSF, so that we could accept your future
contributions without limitations?  If so, I will send you the form to
fill and the instructions to go with it.

Thanks.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 2024-11-06, 14:34 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> Last, but not least: this contribution almost exhausts the amount of
> changes we can accept from you without a copyright assignment.  Would
> you like to start at this time your legal paperwork of assigning the
> copyright to the FSF, so that we could accept your future
> contributions without limitations?  If so, I will send you the form to
> fill and the instructions to go with it.

Hi Eli,

Thanks for your quick and thorough reply.

All points that you mention make sense to me, I'll work towards a v2
that addresses all of them.

In the meanwhile, I'd be very glad to fill out the copyright assignment
paperwork, would you be able to send me the form and/or point me to any
relevant instructions?

Thanks, have a lovely day, Fabio.


-- 
Fabio Natali
https://fabionatali.com

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Fabio Natali <me@fabionatali.com>
> Cc: 74218@debbugs.gnu.org
> Date: Wed, 06 Nov 2024 13:18:27 +0000
> 
> On 2024-11-06, 14:34 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> > Last, but not least: this contribution almost exhausts the amount of
> > changes we can accept from you without a copyright assignment.  Would
> > you like to start at this time your legal paperwork of assigning the
> > copyright to the FSF, so that we could accept your future
> > contributions without limitations?  If so, I will send you the form to
> > fill and the instructions to go with it.
> 
> Hi Eli,
> 
> Thanks for your quick and thorough reply.
> 
> All points that you mention make sense to me, I'll work towards a v2
> that addresses all of them.
> 
> In the meanwhile, I'd be very glad to fill out the copyright assignment
> paperwork, would you be able to send me the form and/or point me to any
> relevant instructions?

Thanks, form sent off-list.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]

Hi Eli,

Please find attached a v2 that - hopefully - addresses the points
mentioned in your email. Please see my further comments inline below.

Thanks for all the help, cheers, Fabio.

On 2024-11-06, 14:34 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> * lisp/net/eww.el (eww-search-confirm-send-region,
>> eww-search-words): With 'eww-search-words' (by default bound to 'M-s
>> M-w') a user can type in some search terms and get back the results
>> of a web search from a predefined search engine. If a region is
>> selected, 'eww-search-words' will use that for the web search
>> instead of prompting the user.
>
> This should be reformatted according to our conventions, see
> CONTRIBUTE.

Ok, here's what I've changed:

- Set max line length to 63 chars.
- Slightly reordered the text so that some broader explanation comes
  first and the ChangeLog entries later.
- Micro-improvements to the ChangeLog entries.

I hope it looks better now - but I'm still a little unsure. If there's
anything else that's left to fix, please let me know.

> The first line of a doc string should be a single complete sentence,
> and should attempt to summarize what the function/variable does,
> because some "apropos" commands show only the first line of each doc
> string.

Ha! True, sorry, that's also fixed now.

>> +  :version "30.0"
>
> This should be "31.1".

Fixed.

>> +                 (format-message
>> +                  "Send region to the configured search engine? ")))
>
> IMO, this should somehow try to indicate the problematic aspect of
> doing this.  For example, maybe it should say
>
>        Really send the entire region to the search engine?

Good one, fixed.

> It is also possible that short regions should be sent without any need
> for confirmation.  In which case perhaps the variable should allow
> integer values, not just nil and t.

I think I disagree on this one.

The functionality you suggest is a superset of what I implemented and it
goes in the direction of giving more freedom to the user. On the other
hand, however, I don't see a strong correlation between the sensitivity
of a piece of information and its length.

For the sake of simplicity, I'd have a preference to maintain the
boolean logic as per my original patch.

> In addition, I don't see any need to ask for confirmation when we are
> not going to send anything to the search engine, so I think the test
> for white-space region should be before the confirmation prompt, and
> only if the region is going to be sent.

Ha, another good one! Thanks, fixed.

> Would you like to start at this time your legal paperwork of assigning
> the copyright to the FSF, so that we could accept your future
> contributions without limitations?

Sent separately, thanks.


-- 
Fabio Natali
https://fabionatali.com



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Ask-confirmation-before-sending-region-to-search-.patch --]
[-- Type: text/x-patch, Size: 2654 bytes --]

From cdd17053befac8298a04d0cdfc4cafe5a410166b Mon Sep 17 00:00:00 2001
From: Fabio Natali <me@fabionatali.com>
Date: Tue, 5 Nov 2024 23:52:30 +0000
Subject: [PATCH v2] Ask confirmation before sending region to search engine

With 'eww-search-words' (by default bound to 'M-s M-w') a user
can type in some search terms and get back the results of a web
search from a predefined search engine. If a region is selected,
'eww-search-words' will use that for the web search instead of
prompting the user.

In its current form, 'eww-search-words' presents a security and
usability problem. It is relatively too easy to mistakenly
launch the function and, if a region of text is selected, have
potentially sensitive data sent out to a third-party service.

This commit changes the search function's default behaviour so
that explicit confirmation is required before a region is sent
to a search engine. The behaviour can be adjusted via the
newly-introduced 'eww-search-confirm-send-region' variable,
which is set to true by default.

* lisp/net/eww.el (eww-search-confirm-send-region): Add.
(eww-search-words): Update default 'eww-search-words' behaviour
so as to ask confirmation before sending a region to a search
engine.
---
 lisp/net/eww.el | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 2d351dff88f..cbf989f4a6a 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -52,6 +52,17 @@
   :group 'eww
   :type 'string)
 
+(defcustom eww-search-confirm-send-region t
+  "Whether to confirm before sending a region to a search engine.
+Non-nil if EWW should ask confirmation before sending the
+selected region to the configured search engine.  This is the
+default to mitigate the risk of accidental data leak.  Set this
+variable to nil to send the region to the search engine
+straightaway."
+  :version "31.1"
+  :group 'eww
+  :type 'boolean)
+
 (defcustom eww-search-prefix "https://duckduckgo.com/html/?q="
   "Prefix URL to search engine."
   :version "24.4"
@@ -605,7 +616,12 @@ for the search engine used."
   (if (use-region-p)
       (let ((region-string (buffer-substring (region-beginning) (region-end))))
         (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
-            (eww region-string)
+            (when
+                (or (not eww-search-confirm-send-region)
+                    (yes-or-no-p
+                     (format-message
+                      "Really send the entire region to the search engine? ")))
+              (eww region-string))
           (call-interactively #'eww)))
     (call-interactively #'eww)))
 
-- 
2.46.0

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Stefan Kangas]

Fabio Natali via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> From cdd17053befac8298a04d0cdfc4cafe5a410166b Mon Sep 17 00:00:00 2001
> From: Fabio Natali <me@fabionatali.com>
> Date: Tue, 5 Nov 2024 23:52:30 +0000
> Subject: [PATCH v2] Ask confirmation before sending region to search engine
>
> With 'eww-search-words' (by default bound to 'M-s M-w') a user
> can type in some search terms and get back the results of a web
> search from a predefined search engine. If a region is selected,
> 'eww-search-words' will use that for the web search instead of
> prompting the user.
>
> In its current form, 'eww-search-words' presents a security and
> usability problem. It is relatively too easy to mistakenly
> launch the function and, if a region of text is selected, have
> potentially sensitive data sent out to a third-party service.
>
> This commit changes the search function's default behaviour so
> that explicit confirmation is required before a region is sent
> to a search engine. The behaviour can be adjusted via the
> newly-introduced 'eww-search-confirm-send-region' variable,
> which is set to true by default.

This is a good addition, thanks.

I think it should be announced in etc/NEWS, too.

> * lisp/net/eww.el (eww-search-confirm-send-region): Add.
> (eww-search-words): Update default 'eww-search-words' behaviour
> so as to ask confirmation before sending a region to a search
> engine.
> ---
>  lisp/net/eww.el | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/net/eww.el b/lisp/net/eww.el
> index 2d351dff88f..cbf989f4a6a 100644
> --- a/lisp/net/eww.el
> +++ b/lisp/net/eww.el
> @@ -52,6 +52,17 @@
>    :group 'eww
>    :type 'string)
>
> +(defcustom eww-search-confirm-send-region t
> +  "Whether to confirm before sending a region to a search engine.

We avoid the word "Whether" in the beginning of the docstring of a
defcustom, since it doesn't make clear which value means what.

So this should read something like:

    If non-nil, prompt before sending region to a search engine.

> +Non-nil if EWW should ask confirmation before sending the
> +selected region to the configured search engine.  This is the
> +default to mitigate the risk of accidental data leak.  Set this
> +variable to nil to send the region to the search engine
> +straightaway."

I suggest reformulating this like so:

    This user option mitigates the risk of accidental data leak.  Set
    this variable to nil to send the region to a search engine without
    prompting.

Note that the first sentence in that paragraph now just repeats the
first line, and so can be removed.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

Stefan Kangas <stefankangas@gmail.com> writes:

> Fabio Natali writes:
>
>> From cdd17053befac8298a04d0cdfc4cafe5a410166b Mon Sep 17 00:00:00 2001
>> From: Fabio Natali <me@fabionatali.com>
>> Date: Tue, 5 Nov 2024 23:52:30 +0000
>> Subject: [PATCH v2] Ask confirmation before sending region to search engine
>>
>> With 'eww-search-words' (by default bound to 'M-s M-w') a user
>> can type in some search terms and get back the results of a web
>> search from a predefined search engine. If a region is selected,
>> 'eww-search-words' will use that for the web search instead of
>> prompting the user.
>>
>> In its current form, 'eww-search-words' presents a security and
>> usability problem. It is relatively too easy to mistakenly
>> launch the function and, if a region of text is selected, have
>> potentially sensitive data sent out to a third-party service.
>>
>> This commit changes the search function's default behaviour so
>> that explicit confirmation is required before a region is sent
>> to a search engine. The behaviour can be adjusted via the
>> newly-introduced 'eww-search-confirm-send-region' variable,
>> which is set to true by default.
>
> This is a good addition, thanks.

I too agree that it's a good idea to optionally require confirmation.
However, I suspect that a yes/no question is not the best interface in
this case.  Instead, it's better to simply prepopulate the minibuffer
with the contents of the region.  Then you confirm with RET and cancel
with C-g.  In addition, this lets you examine and edit your input.

Namely, we can implement eww-search-words along the following lines:

--8<---------------cut here---------------start------------->8---
(defun eww-search-words ()
  "..."
  (interactive)
  (eww (eww-read-url-or-search-string
        (and (use-region-p)
             (string-trim (buffer-substring-no-properties (point) (mark)))))))
--8<---------------cut here---------------end--------------->8---

Where eww-read-url-or-search-string is a new function extracted from the
interactive spec of eww:

--8<---------------cut here---------------start------------->8---
(defun eww-read-url-or-search-string (&optional initial-input)
  (let ((uris (eww-suggested-uris)))
    (completing-read (format-prompt "Enter URL or keywords" uris)
                     (seq-uniq (append eww-prompt-history uris))
                     nil nil initial-input 'eww-prompt-history uris)))
--8<---------------cut here---------------end--------------->8---


Just my 2c,

Eshel

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: Fabio Natali <me@fabionatali.com>,  Eli Zaretskii <eliz@gnu.org>,
>   74218@debbugs.gnu.org
> Date: Thu, 07 Nov 2024 09:42:29 +0100
> 
> I too agree that it's a good idea to optionally require confirmation.
> However, I suspect that a yes/no question is not the best interface in
> this case.  Instead, it's better to simply prepopulate the minibuffer
> with the contents of the region.  Then you confirm with RET and cancel
> with C-g.  In addition, this lets you examine and edit your input.

Why copy the region into the mini-window when it is already shown in
the current buffer's window?  By default, it will be highlighted, but
if not (e.g., transient-mark-mode was disabled), we could forcibly
highlight it.  Why is that not enough?

Copying stuff into the minibuffer has the disadvantage of resizing the
mini-window, and then it could hit the limits on such resizes, which
will prevent the user from seeing large portions of the text, if the
region is large.

Also, does anyone have an opinion about asking for confirmation only
for regions that are large enough?  E.g., when the region is a single
word, do we want to ask for confirmation anyway?

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Robert Pluim]

>>>>> On Thu, 07 Nov 2024 10:53:23 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> Also, does anyone have an opinion about asking for confirmation only
    Eli> for regions that are large enough?  E.g., when the region is a single
    Eli> word, do we want to ask for confirmation anyway?

The default for sending stuff to remote servers should be not to do it
unless explicitly authorized, even if the amount of data is small: the
submission itself provides data about your machine, IP, location etc.

Robert
-- 

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eshel Yaron <me@eshelyaron.com>,  74218@debbugs.gnu.org,
>   stefankangas@gmail.com,  me@fabionatali.com
> Date: Thu, 07 Nov 2024 10:02:00 +0100
> 
> >>>>> On Thu, 07 Nov 2024 10:53:23 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> Also, does anyone have an opinion about asking for confirmation only
>     Eli> for regions that are large enough?  E.g., when the region is a single
>     Eli> word, do we want to ask for confirmation anyway?
> 
> The default for sending stuff to remote servers should be not to do it
> unless explicitly authorized, even if the amount of data is small: the
> submission itself provides data about your machine, IP, location etc.

We are talking about a command which is document as follows:

  (eww-search-words)

  Search the web for the text in the region.
  If region is active (and not whitespace), search the web for
  the text between region beginning and end.  Else, prompt the
  user for a search string.  See the variable ‘eww-search-prefix’
  for the search engine used.

It should be clear from this that a Web search engine is used, and
that the word or the region are sent to it.  Since the user invokes
this command, how is it reasonable not to do what the user requested?
If the user doesn't want to reveal details to the Internet, the user
can avoid invoking the command in the first place.

I feel that I'm missing something here.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Robert Pluim]

>>>>> On Thu, 07 Nov 2024 12:49:47 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Eshel Yaron <me@eshelyaron.com>,  74218@debbugs.gnu.org,
    >> stefankangas@gmail.com,  me@fabionatali.com
    >> Date: Thu, 07 Nov 2024 10:02:00 +0100
    >> 
    >> >>>>> On Thu, 07 Nov 2024 10:53:23 +0200, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    Eli> Also, does anyone have an opinion about asking for confirmation only
    Eli> for regions that are large enough?  E.g., when the region is a single
    Eli> word, do we want to ask for confirmation anyway?
    >> 
    >> The default for sending stuff to remote servers should be not to do it
    >> unless explicitly authorized, even if the amount of data is small: the
    >> submission itself provides data about your machine, IP, location etc.

    Eli> We are talking about a command which is document as follows:

    Eli>   (eww-search-words)

    Eli>   Search the web for the text in the region.
    Eli>   If region is active (and not whitespace), search the web for
    Eli>   the text between region beginning and end.  Else, prompt the
    Eli>   user for a search string.  See the variable ‘eww-search-prefix’
    Eli>   for the search engine used.

    Eli> It should be clear from this that a Web search engine is used, and
    Eli> that the word or the region are sent to it.  Since the user invokes
    Eli> this command, how is it reasonable not to do what the user requested?
    Eli> If the user doesn't want to reveal details to the Internet, the user
    Eli> can avoid invoking the command in the first place.

    Eli> I feel that I'm missing something here.

And so am I. Why are we discussing adding a confirmation to an
explicit request from the user? Or is the intent to leave it as 'off',
but allow customizing it to 'ask'?

Robert
-- 

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: me@eshelyaron.com,  74218@debbugs.gnu.org,  stefankangas@gmail.com,
>   me@fabionatali.com
> Date: Thu, 07 Nov 2024 12:03:20 +0100
> 
> >>>>> On Thu, 07 Nov 2024 12:49:47 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Cc: Eshel Yaron <me@eshelyaron.com>,  74218@debbugs.gnu.org,
>     >> stefankangas@gmail.com,  me@fabionatali.com
>     >> Date: Thu, 07 Nov 2024 10:02:00 +0100
>     >> 
>     >> >>>>> On Thu, 07 Nov 2024 10:53:23 +0200, Eli Zaretskii <eliz@gnu.org> said:
>     >> 
>     Eli> Also, does anyone have an opinion about asking for confirmation only
>     Eli> for regions that are large enough?  E.g., when the region is a single
>     Eli> word, do we want to ask for confirmation anyway?
>     >> 
>     >> The default for sending stuff to remote servers should be not to do it
>     >> unless explicitly authorized, even if the amount of data is small: the
>     >> submission itself provides data about your machine, IP, location etc.
> 
>     Eli> We are talking about a command which is document as follows:
> 
>     Eli>   (eww-search-words)
> 
>     Eli>   Search the web for the text in the region.
>     Eli>   If region is active (and not whitespace), search the web for
>     Eli>   the text between region beginning and end.  Else, prompt the
>     Eli>   user for a search string.  See the variable ‘eww-search-prefix’
>     Eli>   for the search engine used.
> 
>     Eli> It should be clear from this that a Web search engine is used, and
>     Eli> that the word or the region are sent to it.  Since the user invokes
>     Eli> this command, how is it reasonable not to do what the user requested?
>     Eli> If the user doesn't want to reveal details to the Internet, the user
>     Eli> can avoid invoking the command in the first place.
> 
>     Eli> I feel that I'm missing something here.
> 
> And so am I. Why are we discussing adding a confirmation to an
> explicit request from the user? Or is the intent to leave it as 'off',
> but allow customizing it to 'ask'?

My take on it is that the user might not realize that the region is
very large and includes parts she didn't intend to send.  IOW, a
cockpit error.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Robert Pluim]

>>>>> On Thu, 07 Nov 2024 13:05:43 +0200, Eli Zaretskii <eliz@gnu.org> said:
    >> And so am I. Why are we discussing adding a confirmation to an
    >> explicit request from the user? Or is the intent to leave it as 'off',
    >> but allow customizing it to 'ask'?

    Eli> My take on it is that the user might not realize that the region is
    Eli> very large and includes parts she didn't intend to send.  IOW, a
    Eli> cockpit error.

Hmm, ok. As long as it defaults to off.

Robert
-- 

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 2024-11-07, 13:05 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> My take on it is that the user might not realize that the region is
> very large and includes parts she didn't intend to send.  IOW, a
> cockpit error.

It's not only that. Commands can be typed by mistake. The fact that the
command's docstring warns about its effects is not enough.

By default, 'eww-search-words' is bound to 'M-s M-w'. The probability of
accidentally mistyping that combination is not at all negligible. I did
discover the command's beheaviour via view-lossage after mistyping 'M-s
M-w', for example.

One might argue that, no matter how long, all sequences of keys and
commands could be mistyped, but that'd be a bit misleading. I think that
adding a warning and a yes-or-no confirmation request would make
'eww-search-words' sufficiently safe, that's the assumption behind my
patch.

As I said above, I don't think that the sensitivity of a block of text
is a function of its length. Case in point, a password, an address, any
piece of Personally Identifiable Information.

Users can always override the default and might decide to customise
'eww-search-words' as they like - but I still think it's important to
provide a safe default, something safer than what we have today.

Just my 2 cents. Thanks for giving this patch attention.

Have a lovely day, cheers,

Fabio.


-- 
Fabio Natali
https://fabionatali.com

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Fabio Natali <me@fabionatali.com>
> Cc: me@eshelyaron.com, 74218@debbugs.gnu.org, stefankangas@gmail.com
> Date: Thu, 07 Nov 2024 11:29:37 +0000
> 
> On 2024-11-07, 13:05 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> > My take on it is that the user might not realize that the region is
> > very large and includes parts she didn't intend to send.  IOW, a
> > cockpit error.
> 
> It's not only that. Commands can be typed by mistake. The fact that the
> command's docstring warns about its effects is not enough.
> 
> By default, 'eww-search-words' is bound to 'M-s M-w'. The probability of
> accidentally mistyping that combination is not at all negligible. I did
> discover the command's beheaviour via view-lossage after mistyping 'M-s
> M-w', for example.

Those are still "cockpit errors", aren't they?

Did it happen to you that you typed incorrect phrase into a browser's
search window?  Does a browser always unconditionally ask you whether
you really meant that?

> One might argue that, no matter how long, all sequences of keys and
> commands could be mistyped, but that'd be a bit misleading. I think that
> adding a warning and a yes-or-no confirmation request would make
> 'eww-search-words' sufficiently safe, that's the assumption behind my
> patch.

You ask a valid question, but don't answer it.  Indeed, why would we
treat this particular command differently from others?  "Would be
misleading" doesn't provide an answer to the question; instead, it
seems to claim that the question itself is invalid.  Why is it?

> As I said above, I don't think that the sensitivity of a block of text
> is a function of its length. Case in point, a password, an address, any
> piece of Personally Identifiable Information.

Is this the only command which sends user-typed text to the Internet?
I don't think so: the first example I could think about is sending
email.  Do we ask the user for confirmation each time the user types
the command to send a message?  Why not, and how is this command
different, in the general sense?

> Users can always override the default and might decide to customise
> 'eww-search-words' as they like - but I still think it's important to
> provide a safe default, something safer than what we have today.

I'm asking why requesting a confirmation in every case is a reasonable
default.  It is safe, I agree, but it is also annoying in many cases.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Eli,

Thanks for getting back to me.

On 2024-11-07, 13:56 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Fabio Natali <me@fabionatali.com>
>> Cc: me@eshelyaron.com, 74218@debbugs.gnu.org, stefankangas@gmail.com
>> Date: Thu, 07 Nov 2024 11:29:37 +0000
>> 
>> On 2024-11-07, 13:05 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> > My take on it is that the user might not realize that the region is
>> > very large and includes parts she didn't intend to send.  IOW, a
>> > cockpit error.
>> 
>> It's not only that. Commands can be typed by mistake. The fact that the
>> command's docstring warns about its effects is not enough.
>> 
>> By default, 'eww-search-words' is bound to 'M-s M-w'. The probability of
>> accidentally mistyping that combination is not at all negligible. I did
>> discover the command's beheaviour via view-lossage after mistyping 'M-s
>> M-w', for example.
>
> Those are still "cockpit errors", aren't they?

True, you're right. What I meant is that there are at least two
scenarios that might lead to an involuntary data leak.

- I deliberately type 'M-x eww-search-words', it's just that I haven't
  read how the function behaves, I haven't taken the time to read its
  docstring.

- I clumsily mistype 'M-s M-w' while I wanted to do something else.

I suppose they might both fall under the cockpit error umbrella, but
they're somehow different. I'm particularly worried about the latter
scenario. (Which is what happened to me by the way, so I know this *can*
happen.)

> Did it happen to you that you typed incorrect phrase into a browser's
> search window?  Does a browser always unconditionally ask you whether
> you really meant that?

As I said, there's always a chance to mistype a series of keys, steps,
or commands, no matter how long/complicated the combination is. Yes,
you're right, I might have copy-and-paste'd sensitive information in my
browser's URL bar at some point.

However, I think that the data leak risk associated with
'eww-search-words', in its current implementation, is higher that
similar other examples and that this should be fixed.

I suppose the correct way of going at this would be to involve a
security and usability expert to assess the severity of this particular
scenario and to compare it to others. I'm not a usability expert, but I
do have first-hand experience of fumbling up a 'M-s M-w'! :)

>> One might argue that, no matter how long, all sequences of keys and
>> commands could be mistyped, but that'd be a bit misleading. I think
>> that adding a warning and a yes-or-no confirmation request would make
>> 'eww-search-words' sufficiently safe, that's the assumption behind my
>> patch.
>
> You ask a valid question, but don't answer it.  Indeed, why would we
> treat this particular command differently from others?  "Would be
> misleading" doesn't provide an answer to the question; instead, it
> seems to claim that the question itself is invalid.  Why is it?

The answer is: because this scenario is more risky. It's easier to
mistype 'M-s M-w' as opposed to other commands and the consequences of
such mistake are more serious than other commands. It's the very
definition of risk, i.e. likelihood times severity.

>> As I said above, I don't think that the sensitivity of a block of
>> text is a function of its length. Case in point, a password, an
>> address, any piece of Personally Identifiable Information.
>
> Is this the only command which sends user-typed text to the Internet?
> I don't think so: the first example I could think about is sending
> email.  Do we ask the user for confirmation each time the user types
> the command to send a message?  Why not, and how is this command
> different, in the general sense?

The way my email client is configured, it takes more steps to mistakenly
leak sensitive information. For the sake of argument, if I type 'M-x
notmuch-mua-new-mail' when a region is selected, that doesn't lead to
that region being sent straightaway to the first contact in my email
address book!

However, should there be cases similar to 'eww-search-words' I'd be
definitely up for having them fixed. You're orders of magnitude more
familiar with Emacs than I am, but 'eww-search-words' is the first
command that struck me as so risky - we're only a selected region and a
'M-s M-w' away from sending data to a third-party.

>> Users can always override the default and might decide to customise
>> 'eww-search-words' as they like - but I still think it's important to
>> provide a safe default, something safer than what we have today.
>
> I'm asking why requesting a confirmation in every case is a reasonable
> default.  It is safe, I agree, but it is also annoying in many cases.

If the user makes heavy use of 'eww-search-words', they can still
permanently or temporarily disable the confirmation step. But I think
that the default should be the safer alternative, not the more
convenient (but risky!) one.

I hope this brings further context and clarifies my point of view.

Thanks, cheers, Fabio.


-- 
Fabio Natali
https://fabionatali.com

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi All,

I was wondering if there was any further feedback on this and what the
general feeling was with regard to my v2 patch of 6th Nov.

If there's anything that you think I should adjust in the patch, let me
know and I'll be glad to look into it.

Thanks, cheers, Fabio.


-- 
Fabio Natali
https://fabionatali.com

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Fabio Natali <me@fabionatali.com>
> Cc: rpluim@gmail.com, me@eshelyaron.com, 74218@debbugs.gnu.org,
>  stefankangas@gmail.com
> Date: Tue, 26 Nov 2024 12:48:12 +0000
> 
> Hi All,
> 
> I was wondering if there was any further feedback on this and what the
> general feeling was with regard to my v2 patch of 6th Nov.
> 
> If there's anything that you think I should adjust in the patch, let me
> know and I'll be glad to look into it.

The latest patch LGTM, but your legal paperwork of copyright
assignment is not yet complete, is it?  So we are waiting for it to be
completed, and will install at that time.

Thanks.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 2024-11-30, 12:48 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> The latest patch LGTM, but your legal paperwork of copyright
> assignment is not yet complete, is it?  So we are waiting for it to be
> completed, and will install at that time.

Hi Eli, you're right, sorry, I hadn't acted on that yet. It's all done
now, although I guess it might take a few days for things to be
processed and for you to see the assignment (or the assignment-related
A-OK) at your end.

Unless I hear anything, I'll follow-up here in a couple of weeks (say
early Jan?) to make sure the assignment has been received and the patch
can be merged.

Thanks, have a lovely day, Fabio.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 2024-12-04, 13:08 +0000, Fabio Natali <me@fabionatali.com> wrote:
> On 2024-11-30, 12:48 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> The latest patch LGTM, but your legal paperwork of copyright
>> assignment is not yet complete, is it? So we are waiting for it to be
>> completed, and will install at that time.

(No rush, but I wanted to share that the copyright assignment process
has now been completed, as confirmed to me by the FSF
copyright-clerk@fsf.org. Cheers, Fabio.)

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Fabio Natali <me@fabionatali.com>
> Cc: rpluim@gmail.com, me@eshelyaron.com, 74218@debbugs.gnu.org,
>  stefankangas@gmail.com
> Date: Wed, 18 Dec 2024 17:51:24 +0000
> 
> On 2024-12-04, 13:08 +0000, Fabio Natali <me@fabionatali.com> wrote:
> > On 2024-11-30, 12:48 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
> >> The latest patch LGTM, but your legal paperwork of copyright
> >> assignment is not yet complete, is it? So we are waiting for it to be
> >> completed, and will install at that time.
> 
> (No rush, but I wanted to share that the copyright assignment process
> has now been completed, as confirmed to me by the FSF
> copyright-clerk@fsf.org. Cheers, Fabio.)

Thanks, I've now installed the changes on the master branch, and I'm
closing this bug.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: Fabio Natali <me@fabionatali.com>,  Eli Zaretskii <eliz@gnu.org>,
>>   74218@debbugs.gnu.org
>> Date: Thu, 07 Nov 2024 09:42:29 +0100
>> 
>> I too agree that it's a good idea to optionally require confirmation.
>> However, I suspect that a yes/no question is not the best interface in
>> this case.  Instead, it's better to simply prepopulate the minibuffer
>> with the contents of the region.  Then you confirm with RET and cancel
>> with C-g.  In addition, this lets you examine and edit your input.
>
> Why copy the region into the mini-window when it is already shown in
> the current buffer's window?  By default, it will be highlighted, but
> if not (e.g., transient-mark-mode was disabled), we could forcibly
> highlight it.  Why is that not enough?

While point is always visible, mark can be out of view, so the region
need not be fully visible in the selected window.  But more importantly,
using the minibuffer provides a smoother and more consistent UX compared
to an additional yes/no question, IMO.

> Copying stuff into the minibuffer has the disadvantage of resizing the
> mini-window, and then it could hit the limits on such resizes, which
> will prevent the user from seeing large portions of the text, if the
> region is large.
>
> Also, does anyone have an opinion about asking for confirmation only
> for regions that are large enough?  E.g., when the region is a single
> word, do we want to ask for confirmation anyway?

I think it makes sense to have an option that is sensitive to the size
of the region, although personally I'd probably stick to "always ask",
especially if the prompt for confirmation isn't too obtrusive.

bug#74218: [PATCH] Ask confirmation before sending region to search engine.

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: 74218@debbugs.gnu.org,  stefankangas@gmail.com,  me@fabionatali.com
> Date: Thu, 07 Nov 2024 10:12:53 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Eshel Yaron <me@eshelyaron.com>
> >> Cc: Fabio Natali <me@fabionatali.com>,  Eli Zaretskii <eliz@gnu.org>,
> >>   74218@debbugs.gnu.org
> >> Date: Thu, 07 Nov 2024 09:42:29 +0100
> >> 
> >> I too agree that it's a good idea to optionally require confirmation.
> >> However, I suspect that a yes/no question is not the best interface in
> >> this case.  Instead, it's better to simply prepopulate the minibuffer
> >> with the contents of the region.  Then you confirm with RET and cancel
> >> with C-g.  In addition, this lets you examine and edit your input.
> >
> > Why copy the region into the mini-window when it is already shown in
> > the current buffer's window?  By default, it will be highlighted, but
> > if not (e.g., transient-mark-mode was disabled), we could forcibly
> > highlight it.  Why is that not enough?
> 
> While point is always visible, mark can be out of view, so the region
> need not be fully visible in the selected window.  But more importantly,
> using the minibuffer provides a smoother and more consistent UX compared
> to an additional yes/no question, IMO.

Not all the region is always visible, but I'm sure you will agree that
in most cases _more_ of it will be visible in its buffer than if
copied to minibuffer.  To say nothing of the fact that resizing the
mini-window has adverse effect on visibility of other windows, and
thus on the window where the current buffer is displayed.

> > Also, does anyone have an opinion about asking for confirmation only
> > for regions that are large enough?  E.g., when the region is a single
> > word, do we want to ask for confirmation anyway?
> 
> I think it makes sense to have an option that is sensitive to the size
> of the region, although personally I'd probably stick to "always ask",
> especially if the prompt for confirmation isn't too obtrusive.

We can argue about defaults later, but personally I fail to see how
asking for confirmation when a single word is sent would be TRT.