test / test (push) Canceled after 0s
Technical review (Opus, 11 findings, 2 medium) and security review (Fable, no high or medium) of 3c86280..HEAD. Both confirm the port gives dkim_mail_parse()'s result on every input it parses, and that no line the check passes is signed with another domain's key. - F1 (medium): OpenDKIM's parser takes lines no mail program reads as one address - "ceo@bank.com:me@mine.com", "Name <ceo@bank.com:me@mine.com>", "<b@evil.com, a@mine.com>", "ceo@bank.com <me@mine.com>", "ceo@bank.com@mine.com" - and signs for the last while a lenient reader shows the first. The line must now also be one address in RFC 5322's grammar (net/mail, display names in any charset passed through), at the domain OpenDKIM signs for. A quoted display name may still hold anything. - F2 (medium): the bindings were asked about OpenDKIM's form of the address, quotes and all, so "x"@mine.com passed for an application holding @mine.com while x@mine.com belonged to another. They are now asked about the reader's address, its local part unquoted, as Postfix holds the envelope's. - F3 / security (low): Bindings.Allowed and DomainOf fold case in ASCII only: Postfix folds as Unicode (casefold.c, ICU), but every key and login the panel writes is ASCII, and the Unicode fold let the Kelvin sign pass for "k" (net/mail then refuses such a line on the domain test). - F4 (low): a TERM between "postfix start-fg &" and "child=$!" is no longer lost: the wrapper stops Postfix as soon as the pid is known. - F5 (low): for startsecs after a start any failed reload passed as "not running". postfix-reload is now build/postfix-reload.sh: exit 75 when master -t says no master runs, else exec postfix reload; RunOnce returns EndError with the status, and only 75 is taken as done. supervisor.State goes. - F6 (low): crashexit spares postfix-reload too - a reload that could not be spawned is reported to whoever asked, not a reason to stop the container. - F7 (low): a certs/ that is itself a link is resolved before the walk (filepath.WalkDir does not follow its root). - F8 (low): at shutdown the tailer reads what is left of the rotated file before saving its place. Not changed: a refusal line still waiting on the old file at shutdown is not kept - the panel writes refused rows itself, only a kill loses one, and that window is the minute's grace. - F9 (low): pct read as text - empty or unreadable is 100, the rest held to 0..100. - F10 (low): the directory is synced after sasldb2's rename. - F11 (low): POSTFIX_STARTSECS by expr, so "010" or "08" in MILTER_WAIT_TIMEOUT are decimal, not octal or an error. - Security (low): pagehide also empties defaultValue - the input's value attribute and the textarea's text - not only .value. A sasldb2.new left by a killed panel is removed at start (entrypoint) and never archived. security.md says that links under certs/ are followed and by whom they can be placed. Tests: sender check cases for F1 (four lines), F2 (a bound address quoted, for its owner and for another), F3; EndError's state and status; pct cases. architecture.md and CHANGELOG describe the reader rule. Stand (2026-10-15, make e2e, exit 0); go vet and go test ./... pass on the server (24 packages). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
162 lines
5.4 KiB
Go
162 lines
5.4 KiB
Go
package supervisor
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"net"
|
|
"net/http"
|
|
"os"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strings"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// fakeSupervisord answers startProcess and getProcessInfo the way supervisor
|
|
// 4.2.5 does over XML-RPC (Python's xmlrpc.client.dumps with methodresponse:
|
|
// one <param>, or a <fault> struct), from a script of states.
|
|
type fakeSupervisord struct {
|
|
mu sync.Mutex
|
|
// alreadyStarted is how many startProcess calls get fault 60 first.
|
|
alreadyStarted int
|
|
// states are the (state, statename, exitstatus) getProcessInfo returns,
|
|
// one per call; the last one repeats.
|
|
states []fakeState
|
|
starts int
|
|
asked int
|
|
}
|
|
|
|
type fakeState struct {
|
|
state int
|
|
name string
|
|
exitStatus int
|
|
}
|
|
|
|
var methodRe = regexp.MustCompile(`<methodName>([^<]+)</methodName>`)
|
|
|
|
func (f *fakeSupervisord) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
|
body, _ := io.ReadAll(r.Body)
|
|
m := methodRe.FindSubmatch(body)
|
|
if r.URL.Path != "/RPC2" || m == nil {
|
|
http.Error(w, "no", http.StatusNotFound)
|
|
return
|
|
}
|
|
f.mu.Lock()
|
|
defer f.mu.Unlock()
|
|
w.Header().Set("Content-Type", "text/xml")
|
|
switch string(m[1]) {
|
|
case "supervisor.startProcess":
|
|
f.starts++
|
|
if !strings.Contains(string(body), "<boolean>0</boolean>") {
|
|
http.Error(w, "start must not wait", http.StatusBadRequest)
|
|
return
|
|
}
|
|
if f.alreadyStarted > 0 {
|
|
f.alreadyStarted--
|
|
io.WriteString(w, "<?xml version='1.0'?>\n<methodResponse>\n<fault>\n<value><struct>\n"+
|
|
"<member>\n<name>faultCode</name>\n<value><int>60</int></value>\n</member>\n"+
|
|
"<member>\n<name>faultString</name>\n<value><string>ALREADY_STARTED: postfix-reload</string></value>\n</member>\n"+
|
|
"</struct></value>\n</fault>\n</methodResponse>\n")
|
|
return
|
|
}
|
|
io.WriteString(w, "<?xml version='1.0'?>\n<methodResponse>\n<params>\n<param>\n<value><boolean>1</boolean></value>\n</param>\n</params>\n</methodResponse>\n")
|
|
case "supervisor.getProcessInfo":
|
|
s := f.states[min(f.asked, len(f.states)-1)]
|
|
f.asked++
|
|
fmt.Fprintf(w, "<?xml version='1.0'?>\n<methodResponse>\n<params>\n<param>\n<value><struct>\n"+
|
|
"<member>\n<name>name</name>\n<value><string>postfix-reload</string></value>\n</member>\n"+
|
|
"<member>\n<name>state</name>\n<value><int>%d</int></value>\n</member>\n"+
|
|
"<member>\n<name>statename</name>\n<value><string>%s</string></value>\n</member>\n"+
|
|
"<member>\n<name>exitstatus</name>\n<value><int>%d</int></value>\n</member>\n"+
|
|
"<member>\n<name>spawnerr</name>\n<value><string></string></value>\n</member>\n"+
|
|
"</struct></value>\n</param>\n</params>\n</methodResponse>\n", s.state, s.name, s.exitStatus)
|
|
default:
|
|
http.Error(w, "unknown method", http.StatusBadRequest)
|
|
}
|
|
}
|
|
|
|
func serveFake(t *testing.T, f *fakeSupervisord) *rpc {
|
|
t.Helper()
|
|
// A short directory: a unix socket's path is limited to about 100 bytes.
|
|
dir, err := os.MkdirTemp("", "sv")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
t.Cleanup(func() { os.RemoveAll(dir) })
|
|
sock := filepath.Join(dir, "s")
|
|
ln, err := net.Listen("unix", sock)
|
|
if err != nil {
|
|
t.Skipf("unix sockets: %v", err)
|
|
}
|
|
srv := &http.Server{Handler: f}
|
|
go srv.Serve(ln)
|
|
t.Cleanup(func() { srv.Close() })
|
|
old := pollEvery
|
|
pollEvery = time.Millisecond
|
|
t.Cleanup(func() { pollEvery = old })
|
|
return newRPC(sock)
|
|
}
|
|
|
|
var (
|
|
running = fakeState{20, "RUNNING", 0}
|
|
exited0 = fakeState{100, "EXITED", 0}
|
|
)
|
|
|
|
func TestRunOnceWaitsForTheEndAndItsStatus(t *testing.T) {
|
|
f := &fakeSupervisord{states: []fakeState{running, running, exited0}}
|
|
if err := serveFake(t, f).runOnce("postfix-reload"); err != nil {
|
|
t.Fatalf("runOnce: %v", err)
|
|
}
|
|
if f.starts != 1 || f.asked != 3 {
|
|
t.Fatalf("starts %d, asked %d; want 1 start and asked until EXITED (3)", f.starts, f.asked)
|
|
}
|
|
}
|
|
|
|
func TestRunOnceReportsAFailedRun(t *testing.T) {
|
|
f := &fakeSupervisord{states: []fakeState{running, {100, "EXITED", 1}}}
|
|
err := serveFake(t, f).runOnce("postfix-reload")
|
|
if err == nil || !strings.Contains(err.Error(), "ended EXITED, exit status 1") {
|
|
t.Fatalf("runOnce: %v; want the exit status 1", err)
|
|
}
|
|
var end *EndError
|
|
if !errors.As(err, &end) || end.State != "EXITED" || end.Status != 1 {
|
|
t.Fatalf("runOnce: %#v; want an EndError, EXITED, status 1", err)
|
|
}
|
|
}
|
|
|
|
// A run in progress may have read the files before the caller wrote them:
|
|
// it is waited out and a run of the caller's own follows.
|
|
func TestRunOnceStartsItsOwnAfterARunInProgress(t *testing.T) {
|
|
f := &fakeSupervisord{alreadyStarted: 1, states: []fakeState{running, exited0, running, exited0}}
|
|
if err := serveFake(t, f).runOnce("postfix-reload"); err != nil {
|
|
t.Fatalf("runOnce: %v", err)
|
|
}
|
|
if f.starts != 2 || f.asked != 4 {
|
|
t.Fatalf("starts %d, asked %d; want the running one waited out, then a start of its own (2 starts, 4 asks)", f.starts, f.asked)
|
|
}
|
|
}
|
|
|
|
func TestRunOnceGivesUpOnARunThatDoesNotEnd(t *testing.T) {
|
|
f := &fakeSupervisord{states: []fakeState{running}}
|
|
c := serveFake(t, f)
|
|
old := runOnceFor
|
|
runOnceFor = 20 * time.Millisecond
|
|
t.Cleanup(func() { runOnceFor = old })
|
|
err := c.runOnce("postfix-reload")
|
|
if err == nil || !strings.Contains(err.Error(), "still RUNNING") {
|
|
t.Fatalf("runOnce: %v; want still RUNNING", err)
|
|
}
|
|
}
|
|
|
|
func TestFaultIsReadFromTheAnswer(t *testing.T) {
|
|
f := &fakeSupervisord{alreadyStarted: 1, states: []fakeState{exited0}}
|
|
_, err := serveFake(t, f).call("supervisor.startProcess", "postfix-reload", false)
|
|
fault, ok := err.(*Fault)
|
|
if !ok || fault.Code != faultAlreadyStarted || fault.String != "ALREADY_STARTED: postfix-reload" {
|
|
t.Fatalf("call: %v; want fault 60 ALREADY_STARTED", err)
|
|
}
|
|
}
|