checker: harden HTTP collection and stabilize report ordering

Validate the federation tester URI placeholder, escape the domain, set
a client timeout, cap the response body, and ship CA certificates in
the scratch image so HTTPS calls succeed. Sort hosts, connection
reports, and errors when rendering so output is deterministic, and
deduplicate TLS problems. Drop the deprecated aggregate Rule() and add
tests for collection and rules.

Dockerfile

@@ -9,6 +9,7 @@ COPY . .
 RUN CGO_ENABLED=0 go build -tags standalone -ldflags "-X main.Version=${CHECKER_VERSION}" -o /checker-matrix .
 
 FROM scratch
+COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
 COPY --from=builder /checker-matrix /checker-matrix
 USER 65534:65534
 EXPOSE 8080

checker/collect.go

@@ -4,12 +4,23 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
+	"net/url"
 	"strings"
+	"time"
 
 	sdk "git.happydns.org/checker-sdk-go/checker"
 )
 
+const (
+	defaultTesterURI    = "https://federationtester.matrix.org/api/report?server_name=%s"
+	collectHTTPTimeout  = 30 * time.Second
+	maxResponseBodySize = 5 << 20 // 5 MiB
+)
+
+var collectHTTPClient = &http.Client{Timeout: collectHTTPTimeout}
+
 func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (any, error) {
 	domain, _ := opts["serviceDomain"].(string)
 	if domain == "" {
@@ -19,15 +30,20 @@ func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (
 
 	testerURI, _ := opts["federationTesterServer"].(string)
 	if testerURI == "" {
-		testerURI = "https://federationtester.matrix.org/api/report?server_name=%s"
+		testerURI = defaultTesterURI
+	}
+	if !strings.Contains(testerURI, "%s") {
+		return nil, fmt.Errorf("federationTesterServer must contain a %%s placeholder for the domain")
 	}
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf(testerURI, domain), nil)
+	reqURL := fmt.Sprintf(testerURI, url.QueryEscape(domain))
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
 	if err != nil {
 		return nil, fmt.Errorf("unable to build the request: %w", err)
 	}
 
-	resp, err := http.DefaultClient.Do(req)
+	resp, err := collectHTTPClient.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("unable to perform the test: %w", err)
 	}
@@ -38,7 +54,7 @@ func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (
 	}
 
 	var data MatrixFederationData
-	if err := json.NewDecoder(resp.Body).Decode(&data); err != nil {
+	if err := json.NewDecoder(io.LimitReader(resp.Body, maxResponseBodySize)).Decode(&data); err != nil {
 		return nil, fmt.Errorf("failed to decode federation tester response: %w", err)
 	}
 

checker/collect_test.go

@@ -0,0 +1,91 @@
+package checker
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	sdk "git.happydns.org/checker-sdk-go/checker"
+)
+
+func TestCollectMissingDomain(t *testing.T) {
+	p := &matrixProvider{}
+	if _, err := p.Collect(context.Background(), sdk.CheckerOptions{}); err == nil {
+		t.Fatal("expected error when serviceDomain is empty")
+	}
+}
+
+func TestCollectSuccess(t *testing.T) {
+	const body = `{
+		"WellKnownResult": {"m.server": "matrix.example.org:8448", "result": ""},
+		"DNSResult": {"SRVSkipped": false, "SRVRecords": [{"Target": "matrix.example.org.", "Port": 8448, "Priority": 10, "Weight": 5}]},
+		"ConnectionReports": {"1.2.3.4:8448": {"Checks": {"AllChecksOK": true, "MatchingServerName": true, "FutureValidUntilTS": true, "HasEd25519Key": true, "AllEd25519ChecksOK": true, "ValidCertificates": true}}},
+		"ConnectionErrors": {},
+		"Version": {"name": "Synapse", "version": "1.100.0"},
+		"FederationOK": true
+	}`
+
+	var gotURL string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotURL = r.URL.String()
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(body))
+	}))
+	defer srv.Close()
+
+	p := &matrixProvider{}
+	out, err := p.Collect(context.Background(), sdk.CheckerOptions{
+		"serviceDomain":          "example.org.",
+		"federationTesterServer": srv.URL + "/api/report?server_name=%s",
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !strings.Contains(gotURL, "server_name=example.org") {
+		t.Errorf("unexpected URL %q", gotURL)
+	}
+	data, ok := out.(*MatrixFederationData)
+	if !ok || data == nil {
+		t.Fatalf("expected *MatrixFederationData, got %T", out)
+	}
+	if !data.FederationOK {
+		t.Error("expected FederationOK=true")
+	}
+	if data.Version.Name != "Synapse" {
+		t.Errorf("unexpected version name %q", data.Version.Name)
+	}
+}
+
+func TestCollectNon2xx(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusBadGateway)
+	}))
+	defer srv.Close()
+
+	p := &matrixProvider{}
+	_, err := p.Collect(context.Background(), sdk.CheckerOptions{
+		"serviceDomain":          "example.org",
+		"federationTesterServer": srv.URL + "/?s=%s",
+	})
+	if err == nil {
+		t.Fatal("expected error on 502 response")
+	}
+}
+
+func TestCollectMalformedJSON(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Write([]byte("not json"))
+	}))
+	defer srv.Close()
+
+	p := &matrixProvider{}
+	_, err := p.Collect(context.Background(), sdk.CheckerOptions{
+		"serviceDomain":          "example.org",
+		"federationTesterServer": srv.URL + "/?s=%s",
+	})
+	if err == nil {
+		t.Fatal("expected decode error")
+	}
+}

checker/definition.go

@@ -17,7 +17,7 @@ import (
 var Version = "built-in"
 
 // Definition returns the CheckerDefinition for the matrix federation checker.
-func Definition() *sdk.CheckerDefinition {
+func (p *matrixProvider) Definition() *sdk.CheckerDefinition {
 	return &sdk.CheckerDefinition{
 		ID:      "matrixim",
 		Name:    "Matrix Federation Tester",

checker/interactive.go

@@ -35,7 +35,7 @@ func (p *matrixProvider) RenderForm() []sdk.CheckerOptionField {
 func (p *matrixProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) {
 	domain := strings.TrimSpace(r.FormValue("serviceDomain"))
 	if domain == "" {
-		return nil, errors.New("Matrix domain is required")
+		return nil, errors.New("matrix domain is required")
 	}
 
 	opts := sdk.CheckerOptions{

checker/provider.go

@@ -14,8 +14,3 @@ type matrixProvider struct{}
 func (p *matrixProvider) Key() sdk.ObservationKey {
 	return ObservationKeyMatrix
 }
-
-// Definition implements sdk.CheckerDefinitionProvider.
-func (p *matrixProvider) Definition() *sdk.CheckerDefinition {
-	return Definition()
-}

checker/report.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"html/template"
+	"sort"
 	"strings"
 
 	sdk "git.happydns.org/checker-sdk-go/checker"
@@ -326,7 +327,13 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) {
 	}
 
 	// Hosts
-	for name, h := range r.DNSResult.Hosts {
+	hostNames := make([]string, 0, len(r.DNSResult.Hosts))
+	for name := range r.DNSResult.Hosts {
+		hostNames = append(hostNames, name)
+	}
+	sort.Strings(hostNames)
+	for _, name := range hostNames {
+		h := r.DNSResult.Hosts[name]
 		data.Hosts = append(data.Hosts, matrixHostData{
 			Name:  name,
 			CName: h.CName,
@@ -335,7 +342,13 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) {
 	}
 
 	// Successful connections
-	for addr, cr := range r.ConnectionReports {
+	connAddrs := make([]string, 0, len(r.ConnectionReports))
+	for addr := range r.ConnectionReports {
+		connAddrs = append(connAddrs, addr)
+	}
+	sort.Strings(connAddrs)
+	for _, addr := range connAddrs {
+		cr := r.ConnectionReports[addr]
 		conn := matrixConnectionData{
 			Address:     addr,
 			TLSVersion:  cr.Cipher.Version,
@@ -363,10 +376,15 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) {
 	}
 
 	// Failed connections
-	for addr, ce := range r.ConnectionErrors {
+	errAddrs := make([]string, 0, len(r.ConnectionErrors))
+	for addr := range r.ConnectionErrors {
+		errAddrs = append(errAddrs, addr)
+	}
+	sort.Strings(errAddrs)
+	for _, addr := range errAddrs {
 		data.ConnectionErrors = append(data.ConnectionErrors, matrixConnErrData{
 			Address: addr,
-			Message: ce.Message,
+			Message: r.ConnectionErrors[addr].Message,
 		})
 	}
 

checker/rule.go

@@ -21,14 +21,6 @@ func Rules() []sdk.CheckRule {
 	}
 }
 
-// Rule returns the aggregate federation rule.
-//
-// Deprecated: prefer Rules() which exposes every concern individually. Kept
-// for backward compatibility with callers that embed a single rule.
-func Rule() sdk.CheckRule {
-	return &federationOKRule{}
-}
-
 // loadMatrixData fetches the Matrix observation. On error returns a
 // CheckState the caller should emit to short-circuit its rule.
 func loadMatrixData(ctx context.Context, obs sdk.ObservationGetter) (*MatrixFederationData, *sdk.CheckState) {

checker/rules_connection.go

@@ -3,6 +3,7 @@ package checker
 import (
 	"context"
 	"fmt"
+	"sort"
 
 	sdk "git.happydns.org/checker-sdk-go/checker"
 )
@@ -23,16 +24,22 @@ func (r *connectionReachableRule) Evaluate(ctx context.Context, obs sdk.Observat
 	}
 
 	if len(data.ConnectionErrors) == 0 && len(data.ConnectionReports) == 0 {
-		return []sdk.CheckState{infoState("matrix.connection_reachable.unknown", "No endpoint was probed by the federation tester.")}
+		return []sdk.CheckState{unknownState("matrix.connection_reachable.unknown", "No endpoint was probed by the federation tester.")}
 	}
 
 	if len(data.ConnectionErrors) == 0 {
 		return []sdk.CheckState{passState("matrix.connection_reachable.ok", fmt.Sprintf("All %d endpoint(s) accepted the connection.", len(data.ConnectionReports)))}
 	}
 
-	out := make([]sdk.CheckState, 0, len(data.ConnectionErrors))
-	for addr, cerr := range data.ConnectionErrors {
-		st := critState("matrix.connection_reachable.fail", cerr.Message)
+	addrs := make([]string, 0, len(data.ConnectionErrors))
+	for addr := range data.ConnectionErrors {
+		addrs = append(addrs, addr)
+	}
+	sort.Strings(addrs)
+
+	out := make([]sdk.CheckState, 0, len(addrs))
+	for _, addr := range addrs {
+		st := critState("matrix.connection_reachable.fail", data.ConnectionErrors[addr].Message)
 		st.Subject = addr
 		out = append(out, st)
 	}

checker/rules_federation.go

@@ -3,6 +3,7 @@ package checker
 import (
 	"context"
 	"fmt"
+	"sort"
 	"strings"
 
 	sdk "git.happydns.org/checker-sdk-go/checker"
@@ -41,16 +42,21 @@ func (r *federationOKRule) Evaluate(ctx context.Context, obs sdk.ObservationGett
 	var statusLine string
 	switch {
 	case data.DNSResult.SRVError != nil && data.WellKnownResult.Result != "":
-		statusLine = fmt.Sprintf("%s OR %s", data.DNSResult.SRVError.Message, data.WellKnownResult.Result)
+		statusLine = fmt.Sprintf("%s; %s", data.DNSResult.SRVError.Message, data.WellKnownResult.Result)
 	case len(data.ConnectionErrors) > 0:
+		srvs := make([]string, 0, len(data.ConnectionErrors))
+		for srv := range data.ConnectionErrors {
+			srvs = append(srvs, srv)
+		}
+		sort.Strings(srvs)
 		var msg strings.Builder
-		for srv, cerr := range data.ConnectionErrors {
+		for _, srv := range srvs {
 			if msg.Len() > 0 {
 				msg.WriteString("; ")
 			}
 			msg.WriteString(srv)
 			msg.WriteString(": ")
-			msg.WriteString(cerr.Message)
+			msg.WriteString(data.ConnectionErrors[srv].Message)
 		}
 		statusLine = fmt.Sprintf("Connection errors: %s", msg.String())
 	default:

checker/rules_test.go

@@ -0,0 +1,178 @@
+package checker
+
+import (
+	"context"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	sdk "git.happydns.org/checker-sdk-go/checker"
+)
+
+type stubObs struct {
+	data *MatrixFederationData
+	err  error
+}
+
+func (s stubObs) Get(_ context.Context, _ sdk.ObservationKey, dest any) error {
+	if s.err != nil {
+		return s.err
+	}
+	b, err := json.Marshal(s.data)
+	if err != nil {
+		return err
+	}
+	return json.Unmarshal(b, dest)
+}
+
+func (s stubObs) GetRelated(_ context.Context, _ sdk.ObservationKey) ([]sdk.RelatedObservation, error) {
+	return nil, nil
+}
+
+func eval(t *testing.T, rule sdk.CheckRule, data *MatrixFederationData, opts sdk.CheckerOptions) []sdk.CheckState {
+	t.Helper()
+	return rule.Evaluate(context.Background(), stubObs{data: data}, opts)
+}
+
+func TestFederationOKRulePass(t *testing.T) {
+	data := &MatrixFederationData{FederationOK: true}
+	data.Version.Name = "Synapse"
+	data.Version.Version = "1.100.0"
+	got := eval(t, &federationOKRule{}, data, sdk.CheckerOptions{"serviceDomain": "example.org."})
+	if len(got) != 1 || got[0].Status != sdk.StatusOK {
+		t.Fatalf("expected single OK state, got %+v", got)
+	}
+	if !strings.Contains(got[0].Message, "Synapse 1.100.0") {
+		t.Errorf("expected version in message, got %q", got[0].Message)
+	}
+}
+
+func TestFederationOKRuleFailDeterministicOrder(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.ConnectionErrors = map[string]struct {
+		Message string `json:"Message"`
+	}{
+		"z.example:8448": {Message: "boom z"},
+		"a.example:8448": {Message: "boom a"},
+		"m.example:8448": {Message: "boom m"},
+	}
+	first := eval(t, &federationOKRule{}, data, nil)[0].Message
+	for range 5 {
+		if eval(t, &federationOKRule{}, data, nil)[0].Message != first {
+			t.Fatal("federation_ok message not stable across runs")
+		}
+	}
+	idxA := strings.Index(first, "a.example")
+	idxM := strings.Index(first, "m.example")
+	idxZ := strings.Index(first, "z.example")
+	if !(idxA < idxM && idxM < idxZ) {
+		t.Errorf("expected sorted order a<m<z, got %q", first)
+	}
+}
+
+func TestConnectionReachableUnknownWhenNoEndpoints(t *testing.T) {
+	got := eval(t, &connectionReachableRule{}, &MatrixFederationData{}, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusUnknown {
+		t.Fatalf("expected single Unknown state, got %+v", got)
+	}
+}
+
+func TestConnectionReachableSortedFailures(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.ConnectionErrors = map[string]struct {
+		Message string `json:"Message"`
+	}{
+		"b:1": {Message: "b err"},
+		"a:1": {Message: "a err"},
+	}
+	got := eval(t, &connectionReachableRule{}, data, nil)
+	if len(got) != 2 {
+		t.Fatalf("expected 2 states, got %d", len(got))
+	}
+	if got[0].Subject != "a:1" || got[1].Subject != "b:1" {
+		t.Errorf("subjects not sorted: %q, %q", got[0].Subject, got[1].Subject)
+	}
+}
+
+func TestSRVRecordsSkipped(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.DNSResult.SRVSkipped = true
+	data.DNSResult.SRVCName = "matrix.example.org."
+	got := eval(t, &srvRecordsRule{}, data, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusUnknown {
+		t.Fatalf("expected Unknown, got %+v", got)
+	}
+}
+
+func TestVersionRulePass(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.Version.Name = "Dendrite"
+	data.Version.Version = "0.13.0"
+	got := eval(t, &versionRule{}, data, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusOK {
+		t.Fatalf("expected OK, got %+v", got)
+	}
+}
+
+func TestVersionRuleError(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.Version.Error = "connection refused"
+	got := eval(t, &versionRule{}, data, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusWarn {
+		t.Fatalf("expected Warn, got %+v", got)
+	}
+}
+
+func TestWellKnownAbsent(t *testing.T) {
+	got := eval(t, &wellKnownRule{}, &MatrixFederationData{}, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusInfo {
+		t.Fatalf("expected Info, got %+v", got)
+	}
+}
+
+func TestTLSChecksDedupAndSorted(t *testing.T) {
+	data := &MatrixFederationData{}
+	data.ConnectionReports = map[string]struct {
+		Certificates []struct {
+			SubjectCommonName string   `json:"SubjectCommonName"`
+			IssuerCommonName  string   `json:"IssuerCommonName"`
+			SHA256Fingerprint string   `json:"SHA256Fingerprint"`
+			DNSNames          []string `json:"DNSNames"`
+		} `json:"Certificates"`
+		Cipher struct {
+			Version     string `json:"Version"`
+			CipherSuite string `json:"CipherSuite"`
+		} `json:"Cipher"`
+		Checks struct {
+			AllChecksOK        bool `json:"AllChecksOK"`
+			MatchingServerName bool `json:"MatchingServerName"`
+			FutureValidUntilTS bool `json:"FutureValidUntilTS"`
+			HasEd25519Key      bool `json:"HasEd25519Key"`
+			AllEd25519ChecksOK bool `json:"AllEd25519ChecksOK"`
+			ValidCertificates  bool `json:"ValidCertificates"`
+		} `json:"Checks"`
+		Errors []string `json:"Errors"`
+	}{
+		"b:8448": {Errors: []string{"server name does not match certificate"}},
+		"a:8448": {Errors: []string{"server name does not match certificate", "server name does not match certificate"}},
+	}
+
+	got := eval(t, &tlsChecksRule{}, data, nil)
+	if len(got) != 2 {
+		t.Fatalf("expected 2 states, got %d", len(got))
+	}
+	if got[0].Subject != "a:8448" || got[1].Subject != "b:8448" {
+		t.Errorf("subjects not sorted: %q, %q", got[0].Subject, got[1].Subject)
+	}
+	if strings.Count(got[0].Message, "server name does not match certificate") != 1 {
+		t.Errorf("expected dedup, got %q", got[0].Message)
+	}
+}
+
+func TestLoadMatrixDataObservationError(t *testing.T) {
+	rule := &federationOKRule{}
+	got := rule.Evaluate(context.Background(), stubObs{err: context.Canceled}, nil)
+	if len(got) != 1 || got[0].Status != sdk.StatusError {
+		t.Fatalf("expected Error state, got %+v", got)
+	}
+}

checker/rules_tls.go

@@ -3,6 +3,7 @@ package checker
 import (
 	"context"
 	"fmt"
+	"sort"
 	"strings"
 
 	sdk "git.happydns.org/checker-sdk-go/checker"
@@ -30,28 +31,44 @@ func (r *tlsChecksRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter,
 		return []sdk.CheckState{unknownState("matrix.tls_checks.skipped", "No endpoint reached: TLS posture could not be assessed.")}
 	}
 
-	out := make([]sdk.CheckState, 0, len(data.ConnectionReports))
-	for addr, cr := range data.ConnectionReports {
+	addrs := make([]string, 0, len(data.ConnectionReports))
+	for addr := range data.ConnectionReports {
+		addrs = append(addrs, addr)
+	}
+	sort.Strings(addrs)
+
+	out := make([]sdk.CheckState, 0, len(addrs))
+	for _, addr := range addrs {
+		cr := data.ConnectionReports[addr]
 		var problems []string
+		seen := make(map[string]struct{})
+		add := func(p string) {
+			if p == "" {
+				return
+			}
+			if _, ok := seen[p]; ok {
+				return
+			}
+			seen[p] = struct{}{}
+			problems = append(problems, p)
+		}
 		if !cr.Checks.MatchingServerName {
-			problems = append(problems, "server name does not match certificate")
+			add("server name does not match certificate")
 		}
 		if !cr.Checks.FutureValidUntilTS {
-			problems = append(problems, "certificate expired or near expiry")
+			add("certificate expired or near expiry")
 		}
 		if !cr.Checks.ValidCertificates {
-			problems = append(problems, "certificate chain is invalid")
+			add("certificate chain is invalid")
 		}
 		if !cr.Checks.HasEd25519Key {
-			problems = append(problems, "no Ed25519 signing key advertised")
+			add("no Ed25519 signing key advertised")
 		}
 		if !cr.Checks.AllEd25519ChecksOK {
-			problems = append(problems, "Ed25519 key verification failed")
+			add("Ed25519 key verification failed")
 		}
 		for _, e := range cr.Errors {
-			if e != "" {
-				problems = append(problems, e)
-			}
+			add(e)
 		}
 
 		if len(problems) == 0 && cr.Checks.AllChecksOK {

plugin/plugin.go

@@ -20,5 +20,6 @@ var Version = "custom-build"
 // that the host will register in its global registries.
 func NewCheckerPlugin() (*sdk.CheckerDefinition, sdk.ObservationProvider, error) {
 	matrix.Version = Version
-	return matrix.Definition(), matrix.Provider(), nil
+	prvd := matrix.Provider()
+	return prvd.(sdk.CheckerDefinitionProvider).Definition(), prvd, nil
 }