audit 2c: the rest the sources contradict, and the documents

Code and config: - C-M7: a DMARC report's explicit <pct>0</pct> stays 0; only an absent pct takes RFC 7489 § 6.3's default 100 (Pct *int). - C-D1: docs/development.md now tables the Debian versions the citations were read at (postfix 3.7.11-0+deb12u1, opendkim and libopendkim11 2.11.0~beta2-8+deb12u1, libmilter1.0.1 8.17.1.9-2+deb12u2, libsasl2-2 and sasl2-bin 2.1.28+dfsg-10, supervisor 4.2.5-1, logrotate 3.21.0-1 - read off the image with dpkg-query, 2026-10-15); the e2e stand's new checkCitedPackageVersions fails when the image it built differs. - C-E2: the e2e zone's SOA serial only grows (CoreDNS v1.14.6's file plugin reloads on a grown serial, not on mtime); Corefile comment says so; TestZoneSerialGrows. - C-K8, C-K9: the e2e compose and stage comments say how --project-directory and .env resolve. - D-arch:154: postfix-config.sh's "Postfix insists" comment says what TLS_README advises and that smtpd reads the key as root. - B-G7: inflight.go's reservationTTL comment: a dropped client does give ABORT (smtpd_proto() -> mail_reset() -> milter_abort()); the TTL is for smtpd dying or the milter connection breaking. - D-guide:301: the nginx fragment's first-certificate command now reaches certbot (--entrypoint certbot, --standalone, port 80 free), and the renewal loop passes --webroot -w /var/www/certbot - a standalone certificate would otherwise renew standalone while nginx holds port 80 (certbot docs, "Renewing certificates": the issuing plugin is used "unless you specify other plugins or options", and options of a successful renewal are saved). The register's proposal missed that. Documents: every D-arch, D-guide and D-sec claim the register's Answers E found false or partly true, rewritten - by three Sonnet agents, one per document, then reviewed: two proposals were obsolete after this stage's fixes and not used (a bounce's From: line - extract_from = "smtp" now; "Origin: null" - Referrer-Policy is same-origin now); a stand observation the agent had dated from a commit is now tied to the e2e test that checks it; the SQLite writers paragraph describes _txlock=immediate; the log tailer's rotation paragraph describes the minute's grace; the guide's nginx steps were put in working order (links and modes before `up`, no reconfigure step). "Deferred" is kept for Postfix's queue in the guide too. guide.md's CRLF from the agent normalised. Stand (2026-10-15, make e2e, exit 0, the version gate and zone serial test included); go vet and go test ./... pass on the server (24 packages). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-15 01:53:04 +03:00
parent 2e1544dac5
commit fbd0acd035
20 changed files with 401 additions and 141 deletions
@@ -25,9 +25,11 @@ services:
    restart: "no"
    build:
      # Resolved relative to --project-directory (.stage), NOT this file's own
-      # directory — compose build.context paths follow the project directory,
-      # unlike bind-mount volume paths, which is the same reason ./data and
-      # ./certs above need no path here at all.
+      # directory: with --project-directory, build.context and bind-mount
+      # volume paths alike resolve against it - which is why ./data, ./certs
+      # and the ./dns-stage and ./mail-stage below land in .stage
+      # (docs.docker.com/compose/how-tos/multiple-compose-files/merge/, "Paths
+      # are evaluated relative to the base file"; assumptions C-K8).
      context: ../../..
      dockerfile: build/Dockerfile
      args:
@@ -3,8 +3,10 @@
 # the panel nor Postfix ever needs to resolve anything outside this zone here.
 #
 # The `file` plugin's own `reload` sub-directive (NOT the top-level `reload`
-# plugin, which only watches the Corefile itself) polls db.zone's mtime/serial
-# and reloads with no signal needed, so the test harness can rewrite the zone
+# plugin, which only watches the Corefile itself) re-reads db.zone every
+# second and takes it when its SOA serial has grown - the serial, not the
+# mtime (CoreDNS v1.14.6, plugin/file/reload.go; dns_zone.go keeps it
+# growing) - with no signal needed, so the test harness can rewrite the zone
 # file (to publish a domain's DKIM record) and just poll until the new record
 # is visible — no container restart, no sleep.
 e2e.test {
@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 )

@@ -38,12 +39,16 @@ type txtRecord struct {

 // writeZone (re)writes the whole authoritative db.zone for e2e.test: the fixed
 // records CoreDNS/sink need to be reachable, plus whatever TXT records the
-// test has published so far. CoreDNS's `reload 2s` (see dns/Corefile) picks up
-// the new mtime with no signal needed.
+// test has published so far. The file plugin's `reload 1s` (see dns/Corefile)
+// reloads the zone when its SOA serial has grown - not on a new mtime:
+// CoreDNS v1.14.6, plugin/file/reload.go, Reload() re-parses with the current
+// serial and file.go, Parse(), refuses a zone whose serial did not change
+// (coredns.io/plugins/file, "reload"; assumptions C-E2). So the serial only
+// ever grows, by one at least, even for two writes within a second.
 func writeZone(stageDir string, extra []txtRecord) error {
 	var b strings.Builder
 	fmt.Fprintf(&b, "$ORIGIN e2e.test.\n$TTL 300\n")
-	fmt.Fprintf(&b, "@       IN SOA  ns.e2e.test. admin.e2e.test. ( %d 3600 900 604800 300 )\n", time.Now().Unix())
+	fmt.Fprintf(&b, "@       IN SOA  ns.e2e.test. admin.e2e.test. ( %d 3600 900 604800 300 )\n", nextZoneSerial())
 	fmt.Fprintf(&b, "@       IN NS   ns.e2e.test.\n")
 	fmt.Fprintf(&b, "ns      IN A    10.77.0.10\n")
 	// The sink-MX: Postfix's outbound delivery for the recipient domain used by
@@ -58,6 +63,20 @@ func writeZone(stageDir string, extra []txtRecord) error {
 	return os.WriteFile(filepath.Join(stageDir, "dns-stage", "db.zone"), []byte(b.String()), 0o644)
 }

+var zoneSerial struct {
+	sync.Mutex
+	last int64
+}
+
+// nextZoneSerial is the time in seconds, or one more than the last serial
+// when that is not later.
+func nextZoneSerial() int64 {
+	zoneSerial.Lock()
+	defer zoneSerial.Unlock()
+	zoneSerial.last = max(zoneSerial.last+1, time.Now().Unix())
+	return zoneSerial.last
+}
+
 // chunkTXT splits a TXT value into <255-byte quoted strings (the DNS
 // <character-string> limit, RFC 1035 § 3.3), which a DKIM verifier
 // concatenates back into one value (RFC 6376 § 3.6.2.2).
@@ -0,0 +1,39 @@
+package e2e
+
+import (
+	"os"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"testing"
+)
+
+// Two zones written within one second still get two serials, the second
+// larger: CoreDNS reloads a zone only when its serial grows (C-E2).
+func TestZoneSerialGrows(t *testing.T) {
+	stage := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(stage, "dns-stage"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	soa := regexp.MustCompile(`IN SOA  ns\.e2e\.test\. admin\.e2e\.test\. \( (\d+) `)
+	serial := func() int64 {
+		t.Helper()
+		if err := writeZone(stage, nil); err != nil {
+			t.Fatal(err)
+		}
+		b, err := os.ReadFile(filepath.Join(stage, "dns-stage", "db.zone"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		m := soa.FindSubmatch(b)
+		if m == nil {
+			t.Fatalf("no SOA serial in\n%s", b)
+		}
+		n, _ := strconv.ParseInt(string(m[1]), 10, 64)
+		return n
+	}
+	first, second := serial(), serial()
+	if second <= first {
+		t.Fatalf("serials %d then %d: the second write would not be served", first, second)
+	}
+}
@@ -116,6 +116,9 @@ func TestE2E(t *testing.T) {
 		if err := checkInboundRelayOff(h); err != nil {
 			t.Fatal(err)
 		}
+		if err := checkCitedPackageVersions(h); err != nil {
+			t.Fatal(err)
+		}
 		if err := checkPort25ConfigCombinations(h); err != nil {
 			t.Fatal(err)
 		}
@@ -2,6 +2,9 @@ package e2e

 import (
 	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 )
@@ -83,3 +86,46 @@ func checkInboundRelayOff(s *stack) error {
 	}
 	return nil
 }
+
+// checkCitedPackageVersions compares the Debian packages the image runs with
+// the versions their sources were read at (docs/development.md, the table
+// between the cited-package-versions markers). The image installs unpinned
+// from a moving bookworm, so a point release changes them on a rebuild
+// without anyone reading what changed (assumptions C-D1).
+func checkCitedPackageVersions(s *stack) error {
+	doc, err := os.ReadFile(filepath.Join(s.repoRoot, "docs", "development.md"))
+	if err != nil {
+		return err
+	}
+	text := string(doc)
+	start := strings.Index(text, "<!-- cited-package-versions -->")
+	end := strings.Index(text, "<!-- /cited-package-versions -->")
+	if start < 0 || end < start {
+		return fmt.Errorf("docs/development.md has no cited-package-versions table")
+	}
+	row := regexp.MustCompile("(?m)^[|] `([^`]+)` [|] `([^`]+)` [|]$")
+	want := map[string]string{}
+	var names []string
+	for _, m := range row.FindAllStringSubmatch(text[start:end], -1) {
+		want[m[1]] = m[2]
+		names = append(names, m[1])
+	}
+	if len(names) == 0 {
+		return fmt.Errorf("the cited-package-versions table lists no package")
+	}
+	out, err := s.execIn("selfpost", append([]string{"dpkg-query", "-W", "-f", "${Package} ${Version}\n"}, names...)...)
+	if err != nil {
+		return fmt.Errorf("dpkg-query: %v\n%s", err, out)
+	}
+	var diffs []string
+	for _, line := range strings.Split(strings.TrimSpace(out), "\n") {
+		f := strings.Fields(line)
+		if len(f) == 2 && want[f[0]] != f[1] {
+			diffs = append(diffs, fmt.Sprintf("%s: the image has %s, the citations were read at %s", f[0], f[1], want[f[0]]))
+		}
+	}
+	if len(diffs) > 0 {
+		return fmt.Errorf("packages changed under the citations - re-read them (docs/development.md):\n%s", strings.Join(diffs, "\n"))
+	}
+	return nil
+}
@@ -63,10 +63,11 @@ func (s *stack) compose(args ...string) ([]byte, error) {
 	}, args...)
 	cmd := exec.Command("docker", full...)
 	// deploy/docker-compose.yml interpolates ${SELFPOST_HOSTNAME:?...} at
-	// compose-file parse time, straight from the process environment — that
-	// happens before the override file's `environment:` mapping is even
-	// looked at (that mapping only reaches the container, not compose's own
-	// variable interpolation), so it has to be set here too.
+	// compose-file parse time, from the shell's environment or the project
+	// directory's .env, the shell first (docs.docker.com/compose/how-tos/
+	// environment-variables/variable-interpolation/; assumptions C-K9) - the
+	// override file's `environment:` mapping only reaches the container, not
+	// compose's own interpolation - so it is set here, where it wins.
 	cmd.Env = append(os.Environ(), "SELFPOST_HOSTNAME="+selfpostHostname)
 	cmd.Env = append(cmd.Env, s.env...)
 	var out bytes.Buffer
@@ -55,8 +55,11 @@ func prepareStage(s *stack) error {
 // writeDeployFiles puts docker-compose.yml and .env into the stage directory.
 // The stage directory is what the container sees as /selfpost-deploy, the
 // operator's project directory, and a full backup refuses to run unless both
-// files are there to be archived. compose reads neither: the stand names its
-// files with -f, and the .env repeats what compose is given anyway.
+// files are there to be archived. compose reads the .env too - without
+// --env-file it takes .env from --project-directory - but the shell's value
+// set by stack.compose takes precedence, and the two agree anyway
+// (docs.docker.com/compose/how-tos/environment-variables/variable-interpolation/;
+// assumptions C-K9). The compose file itself is named with -f.
 func writeDeployFiles(s *stack) error {
 	compose, err := os.ReadFile(s.baseComposeFile)
 	if err != nil {