run_heal can destroy a healthy proxy container on a single failed docker inspect

[rvalitov]

Summary

run_heal() (mtproxymax.sh:7781) ends with a recovery branch that removes and recreates the proxy container whenever a liveness probe says it is not running. The probe is a single docker inspect, and a failed inspect is treated as proof of absence — so a transient hiccup on a healthy, serving proxy turns into a real outage.

The reaction is also disproportionate: docker rm -f runs unconditionally, even though start_proxy_container already guards against starting a running container.

How it triggers

# mtproxymax.sh:7829-7833, the tail of run_heal()
if ! is_proxy_running; then
    log_warn "Proxy container is not running — attempting recovery start..."
    docker rm -f "$CONTAINER_NAME" 2>/dev/null || true
    start_proxy_container 2>/dev/null || true
fi

and the entire detector (:9259):

is_proxy_running() {
    [ "$(docker inspect -f '{{.State.Running}}' "${CONTAINER_NAME:-mtproxymax}" 2>/dev/null)" = "true" ]
}

Two things follow from reading these together.

1. "I could not tell" is read as "it is not running." 2>/dev/null discards the reason, so a failed inspect (Docker daemon busy, socket or permission fault, host under load) and an inspect that legitimately reported false both collapse to the same empty string. Failure to observe is not evidence of absence.

2. The removal is not needed to start it. start_proxy_container already does:

if is_proxy_running; then
    log_info "Proxy is already running"
    return 0
fi

That guard would make this safe on its own — but run_heal destroys the container first, so by the time start_proxy_container runs there is nothing left to detect.

Reproduction

Tested on Alpine 3.24.1, Docker 29.5.3, substituting only is_proxy_running and stubbing start_proxy_container so no real start was attempted:

CONTROL (real predicate):   "Active Users Impacted:  0 (Zero Disruption)"
                            after: 26a66929dd1c... true     <- unchanged

FORCED  (predicate=false):  "Active Users Impacted:  0 (Zero Disruption)"
                            [!] Proxy container is not running — attempting recovery start...
                            after: CONTAINER GONE
                            keeper replaced it: 26e20798d79a... true   <- different ID

Two details worth noting from that output:

• the banner at :7826 prints Active Users Impacted: 0 (Zero Disruption) immediately before the branch that destroys the container users depend on;
• the destruction is docker rm -f — a removal, not a restart.

A false negative is reachable in practice, not just in theory. Running the same probe as an unprivileged user with no Docker socket access:

as root:      is_proxy_running = TRUE
as test-user:  raw probe output: []   is_proxy_running = FALSE   (rc=1)

Any permission error, socket fault or dockerd hiccup produces FALSE while the container is healthy.

The timing makes it more likely

The daemon initialises its periodic timers to 0, so they all fire on the first loop pass after a restart rather than after their interval — which is exactly when Docker is busy starting the container and a probe is most likely to miss:

_last_report=0
_last_health=0        # health tick fires on pass 1
_last_enforcement=0   # sweep -> run_heal fires on pass 1

Scope — and one correction

AUTO_HEAL_ENABLED defaults to false (:167), so this only affects hosts that opted in. Worth knowing before anyone reads the above as universal.

The daemon's health tick is not directly destructive, since it is easy to conflate the two paths. It calls:

"${INSTALL_DIR}/mtproxymax" start &>/dev/null

which routes through start_proxy_container and therefore re-checks liveness and returns early if the container came back. It is additionally gated on TELEGRAM_ALERTS_ENABLED. The unconditional docker rm -f exists only in run_heal. Both paths do act on the same probe result on the first pass after a restart, though, which is the overlap worth addressing.

run_heal is also invoked as run_heal >/dev/null 2>&1 (:15100), so the user sees nothing at all — the container simply vanishes and reappears.

Suggested fixes

The first two are small and clearly correct while the last two change recovery behaviour and therefore require your approval strategically.

Behaviour-preserving:

1. Make the detector distinguish "inspect failed" from "inspect reported not-running":

   is_proxy_running() {
       local _state
       _state=$(docker inspect -f '{{.State.Running}}' "${CONTAINER_NAME:-mtproxymax}" 2>/dev/null) || return 0
       [ "$_state" = "true" ]
   }

   An unreadable inspect can then never be read as evidence of death, while a genuine false still reports correctly.

2. Drop the unconditional docker rm -f from run_heal and let start_proxy_container's existing guard do its job.

Changes recovery semantics:

3. Require two consecutive failed probes before acting.
4. Stagger the sweep and the health tick so they cannot both fire on the same pass.

I can make a PR implementing the first 2 points only or all of them. What's your call?