fix: Prevent logs leaking into config files (`FETCHMAIL_PARALLEL=1`) by lkwg82 · Pull Request #4586 · docker-mailserver/docker-mailserver

lkwg82 · 2025-10-05T13:53:59Z

Description

fixes fetchmail in parallel mode, when logging is added as first line

root@mail:/etc/fetchmailrc.d# head fetchmail-1.rc 
2025-10-05 15:16:55+02:00 INFO  start-mailserver.sh: File '/etc/fetchmailrc' was missing a final newline - this has been fixed
set daemon 300
set syslog
poll posteo.de with proto IMAP
...

results in

mailserver_1  | 2025-10-05 15:16:55+02:00 INFO  start-mailserver.sh: Starting daemons
mailserver_1  | 2025-10-05 15:16:56,859 WARN exited: fetchmail-1 (exit status 5; not expected)
mailserver_1  | 2025-10-05 15:16:57,066 WARN exited: fetchmail-2 (exit status 5; not expected)
mailserver_1  | 2025-10-05 15:16:57+02:00 INFO  start-mailserver.sh: mail.example.com is up and running

explanation:

this reads from STDOUT and adds the log as well to the rc file

target/scripts/startup/setup.d/fetchmail.sh (line >54)

   while read -r LINE; do
        if [[ ${LINE} =~ poll ]]; then
         ...
          # Just the server settings that need to be added to the specific rc.d file
          echo "${LINE}" >>"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc"
        fi
      done < <(_get_valid_lines_from_file "${FETCHMAILRC}")

target/scripts/helpers/utils.sh

# Returns input after filtering out lines that are:
# empty, white-space, comments (`#` as the first non-whitespace character)
function _get_valid_lines_from_file() {
  _convert_crlf_to_lf_if_necessary "${1}"
  _append_final_newline_if_missing "${1}"

  grep --extended-regexp --invert-match "^\s*$|^\s*#" "${1}" || true
}

below

# This is to sanitize configs from users that unknowingly removed the end-of-file LF:
function _append_final_newline_if_missing() {
   ....
      _log 'info' "File '${1}' was missing a final newline - this has been fixed"
  ....

}

because of

target/scripts/helpers/logs.sh

function _log() {
  ...
  if [[ ${1} =~ ^(warn|error)$ ]]; then
    echo -e "${MESSAGE}" >&2
  else
    echo -e "${MESSAGE}"  # <----- 
  fi
}

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (non-breaking change that does improve existing functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (README.md or the documentation under docs/)
If necessary, I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have added information about changes made in this PR to CHANGELOG.md

interferes with reading from STDOUT ;)

lkwg82 · 2025-10-05T14:35:43Z

I tried to run the tests locally, but I use podman and even after adjusting Dockerfile the testsuite fails because of sth with eth0 (I do have eno1)

polarathene

I am in support of DMS emitting all logs it emits to STDERR 👍

This particular issue could be resolved however by ensuring your config file you supply has a final new line present.

The scripts could also resolve it with:

Calling the helper prior to the while loop, then reading the file contents in afterwards.
Adjusting the single info log from the helper to be a warn level log.

The proposed change is sufficient, so as long as tests pass I'm good with that :)

I'll apply the feedback suggestions so we can check all is good in the test suite.

Reference - Reproduction

services:
  dms:
    image: ghcr.io/docker-mailserver/docker-mailserver:15.1
    hostname: mail.example.test
    environment:
      ENABLE_FETCHMAIL: 1
      FETCHMAIL_PARALLEL: 1
    configs:
      - source: dms-accounts
        target: /tmp/docker-mailserver/postfix-accounts.cf
      - source: fetchmail
        target: /tmp/docker-mailserver/fetchmail.cf

configs:
  fetchmail:
    # `|-` instead of `|` will prevent appending a new line to the file content:
    content: |-
      poll 'mail.remote.test' proto imap
        user '[email protected]'
        pass 'secret'
        is '[email protected]'
        no sslcertck

  dms-accounts:
    content: |
      [email protected]|{SHA512-CRYPT}$$6$$sbgFRCmQ.KWS5ryb$$EsWrlYosiadgdUOxCBHY0DQ3qFbeudDhNMqHs6jZt.8gmxUwiLVy738knqkHD4zj4amkb296HFqQ3yDq4UXt8.

# No final new line at user provided config:
$ docker compose exec dms bash -c 'tail -c1 /tmp/docker-mailserver/fetchmail.cf | wc -l'
0

# Internal copy differs (includes globals/defaults and final new line):
$ docker compose exec dms bash -c 'tail -c1 /etc/fetchmail | wc -l'
1

# Log line stored as first line of each split config file generated:
$ docker compose exec dms bash -c 'cat /etc/fetchmailrc.d/*'

2025-10-06 03:32:59+00:00 INFO  start-mailserver.sh: File '/etc/fetchmailrc' was missing a final newline - this has been fixed
set daemon 300
set syslog
poll 'mail.remote.test' proto imap
user '[email protected]'
pass 'secret'
is '[email protected]'
no sslcertck

# Original unused config:
$ docker compose exec dms bash -c 'cat /etc/fetchmailrc'
# General options

set daemon 300
set syslog

# Fetch rules

poll 'mail.remote.test' proto imap
  user '[email protected]'
  pass 'secret'
  is '[email protected]'
  no sslcertck

The internal copy is fine as we don't do a direct copy, this adds a new line for us implicitly:

docker-mailserver/target/scripts/startup/setup.d/fetchmail.sh

Lines 9 to 16 in d0a915b

    
           CONFIGURATION='/tmp/docker-mailserver/fetchmail.cf' 
        
           FETCHMAILRC='/etc/fetchmailrc' 
        
           if [[ -f ${CONFIGURATION} ]]; then 
        
             cat /etc/fetchmailrc_general "${CONFIGURATION}" >"${FETCHMAILRC}" 
        
           else 
        
             cat /etc/fetchmailrc_general >"${FETCHMAILRC}" 
        
           fi

While Parallel feature does something similar, but only after we've processed their user provided config (NOTE: The commentary is a bit inaccurate for the DEFAULT_FILE var as well mind you):

docker-mailserver/target/scripts/startup/setup.d/fetchmail.sh

Lines 30 to 44 in d0a915b

    
           # Split the content of /etc/fetchmailrc into 
        
           # smaller fetchmailrc files per server [poll] entries. Each 
        
           # separate fetchmailrc file is stored in /etc/fetchmailrc.d 
        
           # 
        
           # The sole purpose for this is to work around what is known 
        
           # as the Fetchmail IMAP idle issue. 
        
           function _fetchmailrc_split() { 
        
             local FETCHMAILRC='/etc/fetchmailrc' 
        
             local FETCHMAILRCD='/etc/fetchmailrc.d' 
        
             local DEFAULT_FILE="${FETCHMAILRCD}/defaults" 
        
             if [[ ! -r ${FETCHMAILRC} ]]; then 
        
               _log 'warn' "File '${FETCHMAILRC}' not found" 
        
               return 1 
        
             fi

docker-mailserver/target/scripts/startup/setup.d/fetchmail.sh

Lines 55 to 62 in d0a915b

    
           if [[ ${LINE} =~ poll ]]; then 
        
             # If we read "poll" then we reached a new server definition 
        
             # We need to create a new file with fetchmail defaults from 
        
             # /etc/fetcmailrc 
        
             COUNTER=$(( COUNTER + 1 )) 
        
             SERVER=1 
        
             cat "${DEFAULT_FILE}" >"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc" 
        
             echo "${LINE}" >>"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc"

polarathene · 2025-10-06T04:00:11Z

NOTE: I have reverted your unrelated change for a separate fetchmail cache per instance, unless you can provide more information to justify that being necessary it seems like it'd not provide much good?

the testsuite fails because of sth with eth0 (I do have eno1)

NETWORK_INTERFACE=eno1 is probably what you'd want, although I don't think our test suite allows for adjusting that 😓

polarathene

LGTM 👍 Thanks for spotting this issue and contributing a fix ❤️

georglauterbach

LGTM

casperklein · 2025-10-06T17:15:45Z

Not related to this PR:

From the naming, I wouldn't expect a _get_*() function to alter anything.

lkwg82 · 2025-10-06T17:31:43Z

@casperklein it is BASH, the language which is making use almost only of sideeffects 😁

lkwg82 added 5 commits October 5, 2025 15:52

adds failing tests

4b63420

cleanup

b47b3be

prevents logging to STDOUT

9abd72b

interferes with reading from STDOUT ;)

adds tiny changelog entry

60bbf87

avoid cache sharing for UIDs

573fdc8

lkwg82 force-pushed the fix_parallel_fetchmail branch from 4682838 to 573fdc8 Compare October 5, 2025 14:24

lkwg82 added 2 commits October 5, 2025 16:25

fix indentation

9fab97d

adds comment in hard to understand areas

5c8fe0a

polarathene requested changes Oct 6, 2025

View reviewed changes

Comment thread target/scripts/helpers/log.sh

Comment thread target/scripts/startup/setup.d/fetchmail.sh Outdated

Comment thread test/tests/parallel/set1/fetchmail.bats Outdated

Comment thread CHANGELOG.md Outdated

Comment thread target/scripts/helpers/log.sh Outdated

Apply suggestions from code review

e13ec97

polarathene added area/scripts kind/improvement Improve an existing feature, configuration file or the documentation area/configuration (file) service/fetchmail kind/bug/fix A fix (PR) for a confirmed bug bug/confirmed A bug report whose bug is confirmed labels Oct 6, 2025

polarathene added this to the v16.0.0 milestone Oct 6, 2025

polarathene assigned lkwg82 Oct 6, 2025

polarathene changed the title ~~fix failing fetchmail in parallel mode~~ fix: Prevent logs leaking into config files (FETCHMAIL_PARALLEL=1) Oct 6, 2025

polarathene approved these changes Oct 6, 2025

View reviewed changes

polarathene requested review from casperklein and georglauterbach October 6, 2025 04:04

georglauterbach approved these changes Oct 6, 2025

View reviewed changes

casperklein approved these changes Oct 6, 2025

View reviewed changes

georglauterbach merged commit 9eb3fa7 into docker-mailserver:master Oct 6, 2025
7 checks passed

polarathene mentioned this pull request Oct 7, 2025

chore: Revise inline docs for Fetchmail #4587

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Prevent logs leaking into config files (`FETCHMAIL_PARALLEL=1`)#4586

fix: Prevent logs leaking into config files (`FETCHMAIL_PARALLEL=1`)#4586
georglauterbach merged 8 commits intodocker-mailserver:masterfrom
lkwg82:fix_parallel_fetchmail

lkwg82 commented Oct 5, 2025 •

edited

Loading

Uh oh!

lkwg82 commented Oct 5, 2025

Uh oh!

polarathene left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

polarathene commented Oct 6, 2025

Uh oh!

polarathene left a comment

Uh oh!

georglauterbach left a comment

Uh oh!

casperklein commented Oct 6, 2025

Uh oh!

lkwg82 commented Oct 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	CONFIGURATION='/tmp/docker-mailserver/fetchmail.cf'
	FETCHMAILRC='/etc/fetchmailrc'

	if [[ -f ${CONFIGURATION} ]]; then
	cat /etc/fetchmailrc_general "${CONFIGURATION}" >"${FETCHMAILRC}"
	else
	cat /etc/fetchmailrc_general >"${FETCHMAILRC}"
	fi

	# Split the content of /etc/fetchmailrc into
	# smaller fetchmailrc files per server [poll] entries. Each
	# separate fetchmailrc file is stored in /etc/fetchmailrc.d
	#
	# The sole purpose for this is to work around what is known
	# as the Fetchmail IMAP idle issue.
	function _fetchmailrc_split() {
	local FETCHMAILRC='/etc/fetchmailrc'
	local FETCHMAILRCD='/etc/fetchmailrc.d'
	local DEFAULT_FILE="${FETCHMAILRCD}/defaults"

	if [[ ! -r ${FETCHMAILRC} ]]; then
	_log 'warn' "File '${FETCHMAILRC}' not found"
	return 1
	fi

	if [[ ${LINE} =~ poll ]]; then
	# If we read "poll" then we reached a new server definition
	# We need to create a new file with fetchmail defaults from
	# /etc/fetcmailrc
	COUNTER=$(( COUNTER + 1 ))
	SERVER=1
	cat "${DEFAULT_FILE}" >"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc"
	echo "${LINE}" >>"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc"

Uh oh!

Conversation

lkwg82 commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

explanation:

Type of change

Checklist

Uh oh!

lkwg82 commented Oct 5, 2025

Uh oh!

polarathene left a comment

Choose a reason for hiding this comment

Reference - Reproduction

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

polarathene commented Oct 6, 2025

Uh oh!

polarathene left a comment

Choose a reason for hiding this comment

Uh oh!

georglauterbach left a comment

Choose a reason for hiding this comment

Uh oh!

casperklein commented Oct 6, 2025

Uh oh!

lkwg82 commented Oct 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lkwg82 commented Oct 5, 2025 •

edited

Loading