From: Jacob Bachmeyer <jcb@gnu.org>
Date: Fri, 18 Nov 2022 02:43:51 +0000 (-0600)
Subject: Add checks to avoid removing backup files in scan_incoming
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=60dc492c5e5d8c4762a9550844114b6f678606fa;p=gatekeeper.git

Add checks to avoid removing backup files in scan_incoming

Also update internal documentation and adjust testsuite to properly cover
the new edge case.
---

diff --git a/gatekeeper.pl b/gatekeeper.pl
index 42ea8e6..62f85b4 100755
--- a/gatekeeper.pl
+++ b/gatekeeper.pl
@@ -1707,12 +1707,15 @@ sub scan_incoming {
  ENT: while (defined($_ = readdir INCOMING)) {
     next ENT if m/^[.]{1,2}$/;	# skip . and .. entries
     # require acceptable filenames
-    unless (length($_) <= MAX_FILE_NAME_LEN && /^($RE_filename_here)$/) {
+    unless (length($_) <= MAX_FILE_NAME_LEN && m/^($RE_filename_here)$/) {
       m/^(.*)$/;	# untaint the value
-      push @trash, File::Spec->catfile($directory, $1); $badname_count++;
       # This is safe for unlink (which is all we will do with @trash)
       # because the filename came from a directory entry, so it must be a
       # valid filename and cannot indicate directory traversal.
+      unless (m/^[.]$RE_filename_here$/) {
+	# do not unlink backup files left by cleanup and cleanup_dir
+	push @trash, File::Spec->catfile($directory, $1); $badname_count++;
+      }
       next ENT
     }
     my $ent = $1;	# if we get here, $RE_filename_here matched above
@@ -2599,8 +2602,7 @@ sub execute_commands {
 Remove all files older than one day from DIRECTORY.
 
 As a precaution against data loss, "removed" files are actually simply
-renamed to have a leading dot.  Note that, for the inbox directory,
-scan_incoming will remove those files on the next run.
+renamed to have a leading dot.
 
 =cut
 
@@ -2635,8 +2637,7 @@ sub cleanup_dir {
 Remove FILES from the inbox, scratch, and staging directories.
 
 As a precaution against data loss, "removed" files are actually simply
-renamed to have a leading dot.  Note that, for the inbox directory,
-scan_incoming will remove those files on the next run.
+renamed to have a leading dot.
 
 =cut
 
diff --git a/testsuite/gatekeeper.all/00_idle.exp b/testsuite/gatekeeper.all/00_idle.exp
index 4b3c5ee..d6223c0 100644
--- a/testsuite/gatekeeper.all/00_idle.exp
+++ b/testsuite/gatekeeper.all/00_idle.exp
@@ -58,11 +58,13 @@ foreach file {
 
 # files which are to be rejected, ignored, and removed
 foreach file {
-    x=x .abcfoobar -abcfoobar x;x \\~xax x*x x:x x?x ;xax _x +x
+    x=x .abcfoobar ..abcfoobar -abcfoobar x;x \\~xax x*x x:x x?x ;xax _x +x
 } {
     put_file [file join $tenv incoming ${file}] "bogus input $file\n"
     age_file [file join $tenv incoming ${file}] "3 minutes ago"
 }
+# ... except for .abcfoobar, which cleanup would produce when removing a
+#     file named "abcfoobar", which is an acceptable name
 
 start_test_services $tenv
 run_upload_batch_test
@@ -71,7 +73,7 @@ stop_test_services
 analyze_file_tree $tenv "idle processing: bogus files" {
     incoming
 } files {
-    bogus1 bogus2 bogus3
+    bogus1 bogus2 bogus3 .abcfoobar
     bo_gus bo+gus bo-gus bo~gus bo.gus
 }
 analyze_file_tree $tenv "idle processing: bogus files" {
@@ -95,6 +97,7 @@ analyze_log $tenv "idle processing: bogus files" {
 
     !scan,x=x		"ignored file: x=x"
     !scan,.abcfoobar	"ignored file: .abcfoobar"
+    !scan,..abcfoobar	"ignored file: ..abcfoobar"
     !scan,-abcfoobar	"ignored file: -abcfoobar"
     !scan,x;x		"ignored file: x;x"
     !scan,~xax		"ignored file: ~xax "