[PATCH v3 3/3] notmuch-mutt: check that the search cache Maildir is not a real Maildir
This prevents data loss when users configure the search cache Maildir to be a
real Maildir containing their real mail data, since the search cache Maildir
is expected to contain only symlinks to the real mail data.
Prevents:
---
contrib/notmuch-mutt/notmuch-mutt | 46 +++
1 file changed, 46 insertions(+)
diff --git a/contrib/notmuch-mutt/notmuch-mutt
b/contrib/notmuch-mutt/notmuch-mutt
index 875fd032..b81252c8 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -13,6 +13,7 @@ use warnings;
use File::Path;
use File::Basename;
+use File::Find;
use Getopt::Long qw(:config no_getopt_compat);
use Mail::Header;
use Mail::Box::Maildir;
@@ -25,6 +26,50 @@ my $xdg_cache_dir = "$ENV{HOME}/.cache";
$xdg_cache_dir = $ENV{XDG_CACHE_HOME} if $ENV{XDG_CACHE_HOME};
my $cache_dir = "$xdg_cache_dir/notmuch/mutt";
+sub die_dir($$) {
+my ($maildir, $error) = @_;
+die "notmuch-mutt: search cache maildir $maildir $error\n".
+"Please ensure that the notmuch-mutt search cache Maildir\n".
+"contains no subfolders or real mail data, only symlinks to mail\n";
+}
+
+sub die_subdir($$$) {
+my ($maildir, $subdir, $error) = @_;
+die_dir($maildir, "subdir $subdir $error");
+}
+
+# check that the search cache maildir is that and not a real maildir
+# otherwise there could be data loss when the search cache is emptied
+sub check_search_cache_maildir($) {
+my ($maildir) = (@_);
+
+return unless -e $maildir;
+
+-d $maildir or die_dir($maildir, 'is not a directory');
+
+opendir(my $mdh, $maildir) or die_dir($maildir, "cannot be opened: $!");
+my @contents = grep { !/^\.\.?$/ } readdir $mdh;
+closedir $mdh;
+
+my @required = ('cur', 'new', 'tmp');
+foreach my $d (@required) {
+-l "$maildir/$d" and die_dir($maildir, "contains symlink $d");
+-e "$maildir/$d" or die_subdir($maildir, $d, 'is missing');
+-d "$maildir/$d" or die_subdir($maildir, $d, 'is not a directory');
+find(sub {
+$_ eq '.' and return;
+$_ eq '..' and return;
+-l $_ or die_subdir($maildir, $d, "contains non-symlink $_");
+}, "$maildir/$d");
+}
+
+my %required = map { $_ => 1 } @required;
+foreach my $d (@contents) {
+-l "$maildir/$d" and die_dir( $maildir, "contains symlink $d");
+-d "$maildir/$d" or die_dir( $maildir, "contains non-directory $d");
+exists($required{$d}) or die_dir( $maildir, "contains directory $d");
+}
+}
# create an empty search cache maildir (if missing) or empty existing one
sub empty_search_cache_maildir($) {
@@ -45,6 +90,7 @@ sub search($$$) {
push @args, "--duplicate=1" if $remove_dups;
push @args, $query;
+check_search_cache_maildir($maildir);
empty_search_cache_maildir($maildir);
open my $pipe, '-|', @args or die "Running @args failed: $!\n";
while (<$pipe>) {
--
2.40.1
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH v2 3/3] notmuch-mutt: check that the search cache Maildir is not a real Maildir
This prevents data loss when users configure the search cache Maildir to be a
real Maildir containing their real mail data, since the search cache Maildir
is expected to contain only symlinks to the real mail data.
Prevents:
---
contrib/notmuch-mutt/notmuch-mutt | 46 +++
1 file changed, 46 insertions(+)
diff --git a/contrib/notmuch-mutt/notmuch-mutt
b/contrib/notmuch-mutt/notmuch-mutt
index 875fd032..1ac68065 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -13,6 +13,7 @@ use warnings;
use File::Path;
use File::Basename;
+use File::Find;
use Getopt::Long qw(:config no_getopt_compat);
use Mail::Header;
use Mail::Box::Maildir;
@@ -25,6 +26,50 @@ my $xdg_cache_dir = "$ENV{HOME}/.cache";
$xdg_cache_dir = $ENV{XDG_CACHE_HOME} if $ENV{XDG_CACHE_HOME};
my $cache_dir = "$xdg_cache_dir/notmuch/mutt";
+sub die_dir($$) {
+my ($maildir, $error) = @_;
+die "notmuch-mutt: search cache maildir $maildir $error\n".
+"Please ensure that the notmuch-mutt search cache Maildir\n".
+"contains no subfolders or real mail data, only symlinks to mail\n";
+}
+
+sub die_subdir($$$) {
+my ($maildir, $subdir, $error) = @_;
+die_dir($maildir, "subdir $subdir $error");
+}
+
+# check that the search cache maildir is that and not a real maildir
+# otherwise there could be data loss when the search cache is emptied
+sub check_search_cache_maildir($) {
+my ($maildir) = (@_);
+
+return unless -e $maildir;
+
+-d $maildir or die_dir($maildir, 'is not a directory');
+
+opendir(my $mdh, $maildir) or die_dir($maildir, "cannot be opened: $!");
+my @contents = grep { !/^\.\.?$/ } readdir $mdh;
+closedir $mdh;
+
+my @required = ('cur', 'new', 'tmp');
+foreach my $d (@required) {
+-l "$maildir/$d" and die_dir($maildir, "contains symlink $d");
+-e "$maildir/$d" or die_subdir($maildir, $d, 'is missing');
+-d "$maildir/$d" or die_subdir($maildir, $d, 'is not a directory');
+find(sub {
+$_ eq '.' and return;
+$_ eq '..' and return;
+-l $_ or die_subdir($maildir, $d, "contains non-symlink $_");
+}, "$maildir/$d");
+}
+
+my %required = map { $_ => 1 } @required;
+foreach my $d (@contents) {
+-l "$maildir/$d" and die_dir( $maildir, "contains symlink $d");
+-d "$maildir/$d" or die_dir( $maildir, "contains non-directory $d");
+exists($required[$d]) or die_dir( $maildir, "contains directory $d");
+}
+}
# create an empty search cache maildir (if missing) or empty existing one
sub empty_search_cache_maildir($) {
@@ -45,6 +90,7 @@ sub search($$$) {
push @args, "--duplicate=1" if $remove_dups;
push @args, $query;
+check_search_cache_maildir($maildir);
empty_search_cache_maildir($maildir);
open my $pipe, '-|', @args or die "Running @args failed: $!\n";
while (<$pipe>) {
--
2.40.1
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH v2 1/3] notmuch-mutt: clarify the empty Maildir function operates on search caches
Rename the function and adjust the documentation comment.
---
contrib/notmuch-mutt/notmuch-mutt | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/notmuch-mutt/notmuch-mutt
b/contrib/notmuch-mutt/notmuch-mutt
index 01db6908..0f7379ac 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -26,8 +26,8 @@ $xdg_cache_dir = $ENV{XDG_CACHE_HOME} if $ENV{XDG_CACHE_HOME};
my $cache_dir = "$xdg_cache_dir/notmuch/mutt";
-# create an empty maildir (if missing) or empty an existing maildir"
-sub empty_maildir($) {
+# create an empty search cache maildir (if missing) or empty existing one
+sub empty_search_cache_maildir($) {
my ($maildir) = (@_);
rmtree($maildir) if (-d $maildir);
my $folder = new Mail::Box::Maildir(folder => $maildir,
@@ -45,7 +45,7 @@ sub search($$$) {
push @args, "--duplicate=1" if $remove_dups;
push @args, $query;
-empty_maildir($maildir);
+empty_search_cache_maildir($maildir);
open my $pipe, '-|', @args or die "Running @args failed: $!\n";
while (<$pipe>) {
chomp;
@@ -120,7 +120,7 @@ sub thread_action($$@) {
my $mid = get_message_id();
if (! defined $mid) {
- empty_maildir($results_dir);
+ empty_search_cache_maildir($results_dir);
die "notmuch-mutt: cannot find Message-Id, abort.\n";
}
--
2.40.1
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH v2 2/3] notmuch-mutt: do not clear search cache Maildir when nothing is found
The previous results might be useful to the user but
an empty directory definitely isn't useful.
---
contrib/notmuch-mutt/notmuch-mutt | 1 -
1 file changed, 1 deletion(-)
diff --git a/contrib/notmuch-mutt/notmuch-mutt
b/contrib/notmuch-mutt/notmuch-mutt
index 0f7379ac..875fd032 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -120,7 +120,6 @@ sub thread_action($$@) {
my $mid = get_message_id();
if (! defined $mid) {
- empty_search_cache_maildir($results_dir);
die "notmuch-mutt: cannot find Message-Id, abort.\n";
}
--
2.40.1
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH] notmuch-mutt: check that the search cache Maildir is not a real Maildir WAS: Re: Data loss
On Sat, 2023-05-27 at 21:10 -0300, David Bremner wrote:
> If possible, please use git-send-email for future series.
Will do, I forgot about the --cover-letter option.
> This seems to have introduced a warning, and I think there's a bug.
>
> % notmuch mutt search tag:inbox
> Argument "new" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
> line 70.
> Argument "cur" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
> line 70.
> Argument "tmp" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
> line 70.
>
> If I remember correctly you need to use {} for hash references in Perl.
Woops! Thanks, fixed in the v2 patch series.
> It might clearer to use the return value from (a possibly renamed
> check_search_cache (then the -d can be eliminated / folded into the new
> function).
When the checks fail, the Maildir creation and Maildir symlink
additions shouldn't happen either, otherwise Maildirs with real data
may get mail files overwritten with symlinks back to themselves. So
there needs to be a die() or return before the creation and the search
anyway and so checks may as well just die() when the first check fails.
I experimented with returning anyway and it makes things more complex:
File::Find cannot abort the search except via die() and so the calling
code would need to use eval in order to catch the die() and return it.
Threading return values all the way back up to search_action adds lots
of error checks to every function along the way.
The search_action interactive mode doesn't do more than one search
anyway so it would still just need to die() when there is an error.
If the code were to grow a multi-command interactive prompt, that could
use eval to catch and print the die() errors. Such a prompt would need
to do that anyway, because the non-search actions call die() already.
So I think that throwing exceptions via die() is the simpler method.
Since checking that the Maildir is suitable is really a separate action
to clearing the previous search results, I've moved the check function
from the empty function to the search function in the v2 patch series.
--
bye,
pabs
https://bonedaddy.net/pabs3/
signature.asc
Description: This is a digitally signed message part
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH] notmuch-mutt: check that the search cache Maildir is not a real Maildir WAS: Re: Data loss
Paul Wise writes:
> I think a better option is to detect if the search cache Maildir is
> actually a Maildir and doesn't contain any data that could be lost.
>
> I have attached three patches I created to improve notmuch-mutt safety.
If possible, please use git-send-email for future series.
> +my %required = map { $_ => 1 } @required;
> +foreach my $d (@contents) {
> +-l "$maildir/$d" and die_dir( $maildir, "contains symlink $d");
> +-d "$maildir/$d" or die_dir( $maildir, "contains non-directory $d");
> +exists($required[$d]) or die_dir( $maildir, "contains directory $d");
> +}
> +}
This seems to have introduced a warning, and I think there's a bug.
% notmuch mutt search tag:inbox
Argument "new" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
line 70.
Argument "cur" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
line 70.
Argument "tmp" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt
line 70.
If I remember correctly you need to use {} for hash references in Perl.
>
> # create an empty search cache maildir (if missing) or empty existing one
> sub empty_search_cache_maildir($) {
> my ($maildir) = (@_);
> +check_search_cache_maildir($maildir);
> rmtree($maildir) if (-d $maildir);
> my $folder = new Mail::Box::Maildir(folder => $maildir,
> create => 1);
It might clearer to use the return value from (a possibly renamed
check_search_cache (then the -d can be eliminated / folded into the new
function).
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v2 2/2] notmuch-mutt: replace extra command with notmuch thread search feature
Paul Wise writes: > This should be be slightly faster since it avoids forking a shell > and is less code in and less dependencies for the script. > > Since String::ShellQuote isn't used elsewhere, drop mention of it. applied to master. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH] test: add known broken test for message-id with embedded spaces.
According to my reading of RFC5322, there is an obsolete syntax for
Message-Id which permits folding whitespace (i.e. to be removed /
ignored by parsers). In [1] Paul Wise observed that notmuch removed
whitespace on indexing, but does not do any corresponding
normalization of queries. Mark the latter as a bug by adding a failing
test.
[1]: id:20230409044143.4041560-1-pa...@bonedaddy.net
---
test/T080-search.sh | 7 +++
1 file changed, 7 insertions(+)
diff --git a/test/T080-search.sh b/test/T080-search.sh
index a3f0dead..515eb88f 100755
--- a/test/T080-search.sh
+++ b/test/T080-search.sh
@@ -34,6 +34,12 @@ add_message '[subject]="search by id"' '[date]="Sat, 01 Jan
2000 12:00:00 -"
output=$(notmuch search id:${gen_msg_id} | notmuch_search_sanitize)
test_expect_equal "$output" "thread:XXX 2000-01-01 [1/1] Notmuch Test Suite;
search by id (inbox unread)"
+test_begin_subtest "Message-Id with spaces"
+test_subtest_known_broken
+add_message '[id]="message i...@example.com"' '[date]="Sat, 01 Jan 2000
12:00:00 -"'
+output=$(notmuch search --output=messages id:"message i...@example.com")
+test_expect_equal "$output" "messag...@example.com"
+
test_begin_subtest "Search by mid:"
add_message '[subject]="search by mid"' '[date]="Sat, 01 Jan 2000 12:00:00
-"'
output=$(notmuch search mid:${gen_msg_id} | notmuch_search_sanitize)
@@ -132,6 +138,7 @@ thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; search by
to (inbox unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; subjectsearchtest (inbox
unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; utf8-sübjéct (inbox unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; search by id (inbox unread)
+thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; Message-Id with spaces
(inbox unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; search by mid (inbox unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; search by tag (inbox
searchbytag unread)
thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; search by thread (inbox
unread)
--
2.39.2
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: T350-crypto T357-index-decryption: possible race condition?
David Bremner writes: > > I'm not sure if this is the main issue, but in commit bf8aa34324cc91a > that key was replaced by an ed255519 key > 9A3AFE6C60065A148FD4B58A7E6ABE924645CC60 > > Did the fedora sources get out of synch somehow? > > d > > PS. Currently spinning up a Fedora Rawhide VM to see if I can duplicate > the issue without the packaging infrastructure. Sorry, that's a silly question, that change is only in master so far. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: T350-crypto T357-index-decryption: possible race condition?
Michael J Gruber writes: > > Are all gpg related tests emacs based? Either gpg or emacs is the red > herring here, or both ... The issue (at least on rawhide) seems to be the interaction between gpg and emacs https://dev.gnupg.org/T6481 A fix seems to be in progress. Do you have problems with gpg versions other than 2.4.1? d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: T350-crypto T357-index-decryption: possible race condition?
Michael J Gruber writes: > David Bremner venit, vidit, dixit 2023-05-12 21:17:45: >> Michael J Gruber writes: >> >> > oh well, attachments ... >> > >> >> Can you encrypt to the key 6D92612D94E46381 interactively using an >> approriately simplified version of that command? > > Took me a while, sorry. In that chroot: > > ``` > sh-5.2# gpg --no-tty --import ./test/gnupg-secret-key.asc > gpg: directory '/builddir/.gnupg' created > gpg: /builddir/.gnupg/trustdb.gpg: trustdb created > gpg: key 6D92612D94E46381: public key "Notmuch Test Suite > (INSECURE!)" imported > gpg: key 6D92612D94E46381: secret key imported > gpg: Total number processed: 1 > gpg: imported: 1 > gpg: secret keys read: 1 > gpg: secret keys imported: 1 > > sh-5.2# echo supersecret | gpg -ear 6D92612D94E46381 > gpg: C44D36DEAD54AB16: There is no assurance this key belongs to the named > user > > sub rsa1024/C44D36DEAD54AB16 2011-02-05 Notmuch Test Suite > (INSECURE!) > Primary key fingerprint: 5AEA B11F 5E33 DCE8 75DD B75B 6D92 612D 94E4 6381 > Subkey fingerprint: 8987 5467 478A A81C EBD5 2E7E C44D 36DE AD54 AB16 > I'm not sure if this is the main issue, but in commit bf8aa34324cc91a that key was replaced by an ed255519 key 9A3AFE6C60065A148FD4B58A7E6ABE924645CC60 Did the fedora sources get out of synch somehow? d PS. Currently spinning up a Fedora Rawhide VM to see if I can duplicate the issue without the packaging infrastructure. ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
