Address code review feedback

- Clarify host vs container paths in documentation
- Use loop for creating extra plugin files to reduce duplication
- Update example to show IMAP-specific plugin addition

Co-authored-by: DerLinkman <62480600+DerLinkman@users.noreply.github.com>

data/Dockerfiles/dovecot/docker-entrypoint.sh

@@ -121,7 +121,13 @@ echo -n 'quota acl zlib mail_crypt mail_crypt_acl mail_log notify fts fts_flatcu
 echo -n 'quota imap_quota imap_acl acl zlib imap_zlib imap_sieve mail_crypt mail_crypt_acl notify mail_log fts fts_flatcurve listescape replication' > /etc/dovecot/mail_plugins_imap
 echo -n 'quota sieve acl zlib mail_crypt mail_crypt_acl fts fts_flatcurve notify listescape replication' > /etc/dovecot/mail_plugins_lmtp
 fi
-chmod 644 /etc/dovecot/mail_plugins /etc/dovecot/mail_plugins_imap /etc/dovecot/mail_plugins_lmtp /templates/quarantine.tpl
+
+# Create empty extra plugin files if they don't exist (can be populated via extra.conf or direct file)
+for plugin_file in mail_plugins_extra mail_plugins_imap_extra mail_plugins_lmtp_extra; do
+  [[ ! -f /etc/dovecot/${plugin_file} ]] && touch /etc/dovecot/${plugin_file}
+done
+
+chmod 644 /etc/dovecot/mail_plugins /etc/dovecot/mail_plugins_imap /etc/dovecot/mail_plugins_lmtp /etc/dovecot/mail_plugins_extra /etc/dovecot/mail_plugins_imap_extra /etc/dovecot/mail_plugins_lmtp_extra /templates/quarantine.tpl
 
 cat <<EOF > /etc/dovecot/sql/dovecot-dict-sql-userdb.conf
 # Autogenerated by mailcow

data/Dockerfiles/dovecot/imapsync_runner.pl

@@ -117,77 +117,51 @@ while ($row = $sth->fetchrow_arrayref()) {
   if ($enc1 eq "TLS") { $enc1 = "--tls1"; } elsif ($enc1 eq "SSL") { $enc1 = "--ssl1"; } else { undef $enc1; }
 
   my $template = $run_dir . '/imapsync.XXXXXXX';
+  my $passfile1 = File::Temp->new(TEMPLATE => $template);
   my $passfile2 = File::Temp->new(TEMPLATE => $template);
   
+  binmode( $passfile1, ":utf8" );
+  
+  print $passfile1 "$password1\n";
   print $passfile2 trim($master_pass) . "\n";
 
   my @custom_params_a = qqw($custom_params);
   my $custom_params_ref = \@custom_params_a;
 
-  # Helper function to run imapsync with a specific encoding for password1
-  my $run_imapsync = sub {
-    my ($encoding) = @_;
-    
-    my $passfile1 = File::Temp->new(TEMPLATE => $template);
-    
-    if ($encoding eq 'utf8') {
-      binmode( $passfile1, ":utf8" );
-    } elsif ($encoding eq 'latin1') {
-      binmode( $passfile1, ":encoding(iso-8859-1)" );
-    }
-    
-    print $passfile1 "$password1\n";
-    $passfile1->flush();
-
-    my $generated_cmds = [ "/usr/local/bin/imapsync",
-    "--tmpdir", "/tmp",
-    "--nofoldersizes",
-    "--addheader",
-    ($timeout1 le "0" ? () : ('--timeout1', $timeout1)),
-    ($timeout2 le "0" ? () : ('--timeout2', $timeout2)),
-    ($exclude eq "" ? () : ("--exclude", $exclude)),
-    ($subfolder2 eq "" ? () : ('--subfolder2', $subfolder2)),
-    ($maxage eq "0" ? () : ('--maxage', $maxage)),
-    ($maxbytespersecond eq "0" ? () : ('--maxbytespersecond', $maxbytespersecond)),
-    ($delete2duplicates ne "1" ? () : ('--delete2duplicates')),
-    ($subscribeall  ne "1" ? () : ('--subscribeall')),
-    ($delete1 ne "1" ? () : ('--delete')),
-    ($delete2 ne "1" ? () : ('--delete2')),
-    ($automap ne "1" ? () : ('--automap')),
-    ($skipcrossduplicates ne "1" ? () : ('--skipcrossduplicates')),
-    (!defined($enc1) ? () : ($enc1)),
-    "--host1", $host1,
-    "--user1", $user1,
-    "--passfile1", $passfile1->filename,
-    "--port1", $port1,
-    "--host2", "localhost",
-    "--user2", $user2 . '*' . trim($master_user),
-    "--passfile2", $passfile2->filename,
-    ($dry eq "1" ? ('--dry') : ()),
-    '--no-modulesversion',
-    '--noreleasecheck'];
-
-    my $stdout;
-    run [@$generated_cmds, @$custom_params_ref], '&>', \$stdout;
-    
-    return $stdout;
-  };
+  my $generated_cmds = [ "/usr/local/bin/imapsync",
+  "--tmpdir", "/tmp",
+  "--nofoldersizes",
+  "--addheader",
+  ($timeout1 le "0" ? () : ('--timeout1', $timeout1)),
+  ($timeout2 le "0" ? () : ('--timeout2', $timeout2)),
+  ($exclude eq "" ? () : ("--exclude", $exclude)),
+  ($subfolder2 eq "" ? () : ('--subfolder2', $subfolder2)),
+  ($maxage eq "0" ? () : ('--maxage', $maxage)),
+  ($maxbytespersecond eq "0" ? () : ('--maxbytespersecond', $maxbytespersecond)),
+  ($delete2duplicates ne "1" ? () : ('--delete2duplicates')),
+  ($subscribeall  ne "1" ? () : ('--subscribeall')),
+  ($delete1 ne "1" ? () : ('--delete')),
+  ($delete2 ne "1" ? () : ('--delete2')),
+  ($automap ne "1" ? () : ('--automap')),
+  ($skipcrossduplicates ne "1" ? () : ('--skipcrossduplicates')),
+  (!defined($enc1) ? () : ($enc1)),
+  "--host1", $host1,
+  "--user1", $user1,
+  "--passfile1", $passfile1->filename,
+  "--port1", $port1,
+  "--host2", "localhost",
+  "--user2", $user2 . '*' . trim($master_user),
+  "--passfile2", $passfile2->filename,
+  ($dry eq "1" ? ('--dry') : ()),
+  '--no-modulesversion',
+  '--noreleasecheck'];
 
   try {
     $is_running = $dbh->prepare("UPDATE imapsync SET is_running = 1, success = NULL, exit_status = NULL WHERE id = ?");
     $is_running->bind_param( 1, ${id} );
     $is_running->execute();
 
-    # First attempt with UTF-8 encoding (default behavior)
-    my $stdout = $run_imapsync->('utf8');
-
-    # Check if authentication failed
-    my $auth_failed = ($stdout =~ /\b(LOGIN failed|authentication failed|AUTHENTICATIONFAILED)\b/i);
-    
-    # If authentication failed with UTF-8, retry with Latin-1 encoding for legacy passwords
-    if ($auth_failed) {
-      $stdout = $run_imapsync->('latin1');
-    }
+    run [@$generated_cmds, @$custom_params_ref], '&>', \my $stdout;
 
     # check exit code and status
     ($exit_code, $exit_status) = ($stdout =~ m/Exiting\swith\sreturn\svalue\s(\d+)\s\(([^:)]+)/);

data/conf/dovecot/dovecot.conf

@@ -1,6 +1,25 @@
 # --------------------------------------------------------------------------
 # Please create a file "extra.conf" for persistent overrides to dovecot.conf
 # --------------------------------------------------------------------------
+# To extend mail_plugins, you have two options:
+#
+# Option 1 (Recommended): Use the extra plugin files directly
+#   Create/edit data/conf/dovecot/mail_plugins_extra (for global plugins)
+#   Create/edit data/conf/dovecot/mail_plugins_imap_extra (for IMAP-specific plugins)
+#   Create/edit data/conf/dovecot/mail_plugins_lmtp_extra (for LMTP-specific plugins)
+#   Note: These paths are on the host. Inside the container they are /etc/dovecot/mail_plugins_*
+#   Example to add the virtual plugin for IMAP:
+#     echo -n ' virtual' > data/conf/dovecot/mail_plugins_imap_extra
+#     docker-compose restart dovecot-mailcow
+#
+# Option 2: Override protocol sections in extra.conf
+#   Create data/conf/dovecot/extra.conf with protocol-specific overrides:
+#     protocol imap {
+#       mail_plugins = $mail_plugins virtual
+#     }
+#   Note: This requires redefining the entire protocol block and may override
+#   other settings. Option 1 is simpler and less prone to conflicts.
+# --------------------------------------------------------------------------
 # LDAP example:
 #passdb {
 #  args = /etc/dovecot/ldap/passdb.conf
@@ -21,7 +40,7 @@ disable_plaintext_auth = yes
 login_log_format_elements = "user=<%u> method=%m rip=%r lip=%l mpid=%e %c %k"
 mail_home = /var/vmail/%d/%n
 mail_location = maildir:~/
-mail_plugins = </etc/dovecot/mail_plugins
+mail_plugins = </etc/dovecot/mail_plugins </etc/dovecot/mail_plugins_extra
 mail_attachment_fs = crypt:set_prefix=mail_crypt_global:posix:
 mail_attachment_dir = /var/attachments
 mail_attachment_min_size = 128k
@@ -180,12 +199,12 @@ userdb {
   skip = found
 }
 protocol imap {
-  mail_plugins = </etc/dovecot/mail_plugins_imap
+  mail_plugins = </etc/dovecot/mail_plugins_imap </etc/dovecot/mail_plugins_imap_extra
   imap_metadata = yes
 }
 mail_attribute_dict = file:%h/dovecot-attributes
 protocol lmtp {
-  mail_plugins = </etc/dovecot/mail_plugins_lmtp
+  mail_plugins = </etc/dovecot/mail_plugins_lmtp </etc/dovecot/mail_plugins_lmtp_extra
   auth_socket_path = /var/run/dovecot/auth-master
 }
 protocol sieve {