Files
selfpost/internal/supervisor/rpc_test.go
T
mixandClaude Opus 5.5 9eddd49602
test / test (push) Canceled after 0s
audit 2c: findings of the two reviews
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>
2026-10-15 09:17:33 +03:00

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)
}
}