check.milter: fail_open does not cover a milter session-dial failure

Open

#866 opened on Sep 10, 2026 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty: 2/5
Estimated time: 1-3 hours
Newbie friendliness: 25/100

Issue type: Bug
Clarity: Clearly specified
Activity status: Stale
Tech stack: go
Domain: backend

Research direction

Read internal/check/milter/milter.go, especially CheckStateForMsg and the existing ioError handling. Review the regression test mentioned in PR #865 and verify that an unreachable milter with fail_open enabled accepts the message unchecked while the test passes.

Written by the indexing model from the issue text.

Description

Summary

check.milter's fail_open directive is documented to let a message through when the milter can't be consulted, but it silently doesn't cover one specific failure mode: the milter being completely unreachable when maddy tries to establish a session.

Root cause

CheckStateForMsg (internal/check/milter/milter.go) dials the milter backend via c.cl.Session() and returns any error unconditionally. This happens before a *state exists, so the failure never reaches ioError() — the only place fail_open is actually checked. Result: with fail_open set, a message is still hard-rejected (a real SMTP-level error handed back to the original sender) if the milter is simply down — exactly the case fail_open exists to guard against.

Reproduction

Configure check.milter with fail_open: true pointing at a milter address that's genuinely unreachable (e.g. nothing listening on that port), then send a message through. Expected: message accepted unchecked, per fail_open. Actual: a hard SMTP-level rejection.

Fix

I've prepared and tested a fix — handles the dial failure the same way ioError() already handles a later I/O failure — and opened it as #865, with a regression test that fails against unpatched code and passes with the fix.

Apologies for the PR arriving before this issue — I realize CONTRIBUTING.md's process is to raise the issue first. Filing this now so there's a proper place to discuss the bug alongside the PR.

Dominant language: Go
Stars: 6.1k
Forks: 327
PR merge metrics: No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

Read the whole issue, then the project's contributing guide.
Comment on the issue to say you are picking it up — it saves two people doing the same work.
Fork the repository and make your change on a branch.
Open a pull request that references the issue number.

Similar issues

[GEP-28] `gardenadm restore`: Add developer docs for the local setup Open

area/dev-productivity area/disaster-recovery area/ipcei kind/enhancement

Difficulty 2/5 1-3 hours Newbie friendliness 70/100

gardener/gardener#15788 · Opened Sep 23, 2026
Codex skills: `git rev-parse --verify --` rejects every explicit ref Open

Difficulty 1/5 Under an hour Newbie friendliness 85/100

kenn-io/roborev#1229 · Opened Sep 23, 2026
download_media returns a relative path that breaks when the bridge and MCP server run from different working directories Open

Difficulty 2/5 1-3 hours Newbie friendliness 75/100

FelixIsaac/whatsapp-mcp-extended#85 · Opened Sep 23, 2026
`docker logout docker.io` does not remove Docker Hub credentials Open

kind/bug status/0-triage

Difficulty 2/5 1-3 hours Newbie friendliness 75/100

docker/cli#7326 · Opened Sep 23, 2026
Truncation of tekton task names causes conflict even though names are different Open

🤔 refinement needed

Difficulty 2/5 1-3 hours Newbie friendliness 75/100

equinor/radix-operator#1979 · Opened Sep 23, 2026

More Go issues