GNU bug report logs - #74218
[PATCH] Ask confirmation before sending region to search engine.

Previous Next

Full log

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

From: Fabio Natali <me <at> fabionatali.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74218 <at> debbugs.gnu.org
Subject: Re: bug#74218: [PATCH] Ask confirmation before sending region to
 search engine.
Date: Wed, 06 Nov 2024 15:27:04 +0000

[Message part 1 (text/plain, inline)]

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 <at> 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

[v2-0001-Ask-confirmation-before-sending-region-to-search-.patch (text/x-patch, inline)]

From cdd17053befac8298a04d0cdfc4cafe5a410166b Mon Sep 17 00:00:00 2001
From: Fabio Natali <me <at> 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

Previous Next

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