feat(bimi): reconcile the BIMI record to Cloudflare and keep it in step with the logo

The reconciler never wrote the BIMI record: only the console's verification knew about it, so a stored logo was never published to the zone. - SyncDnsRecordsInput carries the BIMI logo and certificate URLs; the record is created, updated (a= added or dropped) and, when the logo is removed, deleted. Only a record carrying this reconciler's comment is retired: one made by hand is left alone - bimi.Publish decides the record in one place for the timer sweep, the console's button and the panel's verification. A held certificate is advertised only while it is valid for the logo being served: replacing the logo ends the certificate, and a record that kept naming it makes every receiver reject BIMI, including those that need no certificate - saving or removing the logo or the certificate triggers the reconciliation in the background; the panel shows the record as the server writes it, says when a certificate is held back and why, and has a publish-now button - a read error is not 'no logo': the sync leaves the record alone instead of deleting it over a database hiccup Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-16 18:17:07 -03:00
1 parent 0ff9028927
commit 6e49ca00e2
@@ -29,10 +29,12 @@ type DnsSpecSource interface {
 // name already holding somebody else's value is not this server's to
 // overwrite - it may be the zone's mail, or a service nobody remembered.
 type ReconcileResult struct {
-	Domain    string   `json:"domain"`
-	Created   int      `json:"created"`
-	Updated   int      `json:"updated"`
-	Unchanged int      `json:"unchanged"`
+	Domain    string `json:"domain"`
+	Created   int    `json:"created"`
+	Updated   int    `json:"updated"`
+	Unchanged int    `json:"unchanged"`
+	// Deleted counts records retired because they are no longer wanted.
+	Deleted   int      `json:"deleted"`
 	Conflicts []string `json:"conflicts"`
 	Errors    []string `json:"errors"`
 }
@@ -13,6 +13,7 @@ import (
 	"time"

 	"lambdamail/protocols/internal/domain/bimi"
+	"lambdamail/protocols/internal/domain/entity"
 )

 // BimiStore holds the mark a domain publishes, and the certificate that
@@ -47,6 +48,25 @@ type bimiAPI struct {
 	now      func() time.Time
 	dns      bimi.Resolver
 	fetch    bimi.Fetcher
+	// publish is told when a domain's logo or certificate changed, so the DNS
+	// record can follow. Nil when there is no DNS provider to publish to.
+	publish func(domain string)
+}
+
+// SetBimiPublisher wires what happens after a logo or certificate changes.
+func (r *Router) SetBimiPublisher(publish func(domain string)) {
+	if r.bimi != nil {
+		r.bimi.publish = publish
+	}
+}
+
+// changed tells the publisher, in the background: a change is not slowed
+// down by a round trip to the DNS provider, and its outcome is the
+// reconciliation result the console shows, not part of the save.
+func (a *bimiAPI) changed(domain string) {
+	if a.publish != nil {
+		go a.publish(domain)
+	}
 }

 // SetBimiAPI wires the BIMI logo store.
@@ -168,6 +188,7 @@ func (r *Router) handleBimiAdmin(w http.ResponseWriter, req *http.Request) {
 			writeError(w, http.StatusInternalServerError, "DELETE_FAILED", "Could not remove the logo")
 			return
 		}
+		r.bimi.changed(domain)
 		writeJSON(w, http.StatusOK, map[string]any{"ok": true})
 	default:
 		writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "GET, PUT or DELETE")
@@ -228,6 +249,7 @@ func (r *Router) handleBimiCertificateAdmin(w http.ResponseWriter, req *http.Req
 			writeError(w, http.StatusInternalServerError, "DELETE_FAILED", "Could not remove the certificate")
 			return
 		}
+		r.bimi.changed(domain)
 		writeJSON(w, http.StatusOK, map[string]any{"ok": true})
 	default:
 		writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "PUT or DELETE")
@@ -259,21 +281,21 @@ func (r *Router) handleBimiCheck(w http.ResponseWriter, req *http.Request) {
 	ctx, cancel := context.WithTimeout(req.Context(), 25*time.Second)
 	defer cancel()

+	// What the record should say is decided once (bimi.Publish), exactly as the
+	// sync writes it, so "the record is wrong" means it differs from what the
+	// reconciler would publish.
+	publication := bimi.Publish(logo.SVG, logo.VmcPEM, logo.VmcURL, domain,
+		BimiLogoURL(r.bimi.mailHost), BimiVmcURL(r.bimi.mailHost), r.bimi.now())
 	in := bimi.Inputs{
 		Domain:                 domain,
-		LogoURL:                BimiLogoURL(r.bimi.mailHost),
+		LogoURL:                publication.LogoURL,
+		CertificateURL:         publication.CertificateURL,
 		SVG:                    logo.SVG,
 		CertificatePEM:         logo.VmcPEM,
 		ExternalCertificateURL: logo.VmcURL,
+		Withheld:               publication.Withheld,
 		Now:                    r.bimi.now(),
 	}
-	// The record must name the certificate that is actually served
-	switch {
-	case len(logo.VmcPEM) > 0:
-		in.CertificateURL = BimiVmcURL(r.bimi.mailHost)
-	default:
-		in.CertificateURL = logo.VmcURL
-	}

 	results := bimi.RunChecks(ctx, in, r.bimi.dns, r.bimi.fetch)
 	checks := make([]map[string]string, 0, len(results))
@@ -298,15 +320,22 @@ func (a *bimiAPI) get(w http.ResponseWriter, req *http.Request, domain string) {
 		writeJSON(w, http.StatusOK, map[string]any{"configured": false})
 		return
 	}
+	publication := bimi.Publish(logo.SVG, logo.VmcPEM, logo.VmcURL, domain,
+		BimiLogoURL(a.mailHost), BimiVmcURL(a.mailHost), a.now())
 	writeJSON(w, http.StatusOK, map[string]any{
-		"configured":      true,
-		"etag":            logo.ETag,
-		"vmc_url":         logo.VmcURL,
-		"bytes":           len(logo.SVG),
-		"svg":             string(logo.SVG),
-		"logo_url":        BimiLogoURL(a.mailHost),
-		"certificate_url": BimiVmcURL(a.mailHost),
-		"certificate":     a.describeCertificate(domain, logo),
+		"configured": true,
+		"etag":       logo.ETag,
+		// The record exactly as the reconciler writes it, and the reason a held
+		// certificate is left out of it, if it is
+		"dns_record":           entity.BimiRecordValue(publication.LogoURL, publication.CertificateURL),
+		"certificate_withheld": publication.Withheld,
+		"dns_publishing":       a.publish != nil,
+		"vmc_url":              logo.VmcURL,
+		"bytes":                len(logo.SVG),
+		"svg":                  string(logo.SVG),
+		"logo_url":             BimiLogoURL(a.mailHost),
+		"certificate_url":      BimiVmcURL(a.mailHost),
+		"certificate":          a.describeCertificate(domain, logo),
 	})
 }

@@ -391,9 +420,11 @@ func (a *bimiAPI) putCertificate(w http.ResponseWriter, req *http.Request, domai
 		writeError(w, http.StatusConflict, "NO_LOGO", "Save the logo first")
 		return
 	}
+	a.changed(domain)
 	writeJSON(w, http.StatusOK, map[string]any{
-		"ok":          true,
-		"certificate": a.describeCertificate(domain, &BimiLogo{SVG: logo.SVG, VmcPEM: pem}),
+		"ok":             true,
+		"dns_publishing": a.publish != nil,
+		"certificate":    a.describeCertificate(domain, &BimiLogo{SVG: logo.SVG, VmcPEM: pem}),
 	})
 }

@@ -446,7 +477,8 @@ func (a *bimiAPI) put(w http.ResponseWriter, req *http.Request, domain string) {
 	}
 	// A certificate vouches for one exact SVG. Replacing the logo under a held
 	// certificate breaks it, which receivers do not report; say so now.
-	response := map[string]any{"ok": true, "etag": etag}
+	a.changed(domain)
+	response := map[string]any{"ok": true, "etag": etag, "dns_publishing": a.publish != nil}
 	if held, err := a.store.Load(req.Context(), domain); err == nil && held != nil && len(held.VmcPEM) > 0 {
 		if cert, _ := bimi.InspectCertificate(held.VmcPEM, domain, held.SVG, a.now()); cert != nil && !cert.MatchesLogo {
 			response["certificate_mismatch"] = true
@@ -335,3 +335,123 @@ func TestFetcherRefusesPrivateAndPlainHTTP(t *testing.T) {
 		}
 	}
 }
+
+// publishRecorder collects the domains the console asked to have published.
+type publishRecorder struct{ domains chan string }
+
+func newPublishRecorder() *publishRecorder { return &publishRecorder{domains: make(chan string, 8)} }
+
+func (p *publishRecorder) record(domain string) { p.domains <- domain }
+
+func (p *publishRecorder) expect(t *testing.T, what string) {
+	t.Helper()
+	select {
+	case got := <-p.domains:
+		if got != "ebay.com" {
+			t.Errorf("%s published %q", what, got)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatalf("%s did not ask for the DNS record to be published", what)
+	}
+}
+
+func (p *publishRecorder) expectNothing(t *testing.T, what string) {
+	t.Helper()
+	select {
+	case got := <-p.domains:
+		t.Fatalf("%s published %q though it changed nothing", what, got)
+	case <-time.After(150 * time.Millisecond):
+	}
+}
+
+// The record in the zone follows the picture: every change to the logo or the
+// certificate asks for the DNS to be reconciled, and nothing else does.
+func TestBimiChangesAreHandedToThePublisher(t *testing.T) {
+	store := &memoryBimiStore{logos: map[string]*BimiLogo{}}
+	r := newBimiRouter(store)
+	rec := newPublishRecorder()
+	r.bimi.publish = rec.record
+
+	putLogo(t, r, fixture(t, "ebay.svg"))
+	rec.expect(t, "saving the logo")
+
+	putCertificate(r, fixture(t, "ebay.pem"))
+	rec.expect(t, "saving the certificate")
+
+	// Reads and refused uploads change nothing and publish nothing
+	bimiCall(r, http.MethodGet, "/api/v1/admin/bimi?domain=ebay.com", "", true)
+	putCertificate(r, []byte("-----BEGIN PRIVATE KEY-----\nQUJD\n-----END PRIVATE KEY-----\n"))
+	putLogo(t, r, []byte("<svg/>"))
+	rec.expectNothing(t, "a read or a refused upload")
+
+	bimiCall(r, http.MethodDelete, "/api/v1/admin/bimi/certificate?domain=ebay.com", "", true)
+	rec.expect(t, "removing the certificate")
+
+	bimiCall(r, http.MethodDelete, "/api/v1/admin/bimi?domain=ebay.com", "", true)
+	rec.expect(t, "removing the logo")
+}
+
+func TestBimiWithoutAPublisherStillWorks(t *testing.T) {
+	r := newBimiRouter(&memoryBimiStore{logos: map[string]*BimiLogo{}})
+	resp := jsonBody(t, putLogo(t, r, fixture(t, "ebay.svg")))
+	if resp["ok"] != true || resp["dns_publishing"] != false {
+		t.Fatalf("%v", resp)
+	}
+}
+
+// The check expects what the reconciler writes: with a certificate that no
+// longer matches the logo that is a record without a=, and that is correct.
+func TestBimiCheckExpectsWhatTheReconcilerPublishes(t *testing.T) {
+	store := &memoryBimiStore{logos: map[string]*BimiLogo{}}
+	r := newBimiRouter(store)
+	putLogo(t, r, fixture(t, "cnn.svg"))
+	store.logos["ebay.com"].VmcPEM = fixture(t, "ebay.pem") // a certificate for another picture
+
+	r.bimi.dns = fakeBimiDNS{
+		"_dmarc.ebay.com":        {"v=DMARC1; p=reject"},
+		"default._bimi.ebay.com": {"v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg;"},
+	}
+	body := jsonBody(t, bimiCall(r, http.MethodGet, "/api/v1/admin/bimi/check?domain=ebay.com", "", true))
+
+	status := map[string]string{}
+	detail := map[string]string{}
+	for _, c := range body["checks"].([]any) {
+		m := c.(map[string]any)
+		status[m["id"].(string)] = m["status"].(string)
+		detail[m["id"].(string)] = m["detail"].(string)
+	}
+	if status["record"] != "ok" {
+		t.Errorf("the record without a= is what the reconciler writes here, but the check said %s: %s", status["record"], detail["record"])
+	}
+	if status["certificate"] != "fail" || !strings.Contains(detail["certificate"], "NOT published") {
+		t.Errorf("the certificate check must say it is held back: %s %s", status["certificate"], detail["certificate"])
+	}
+}
+
+// The console shows the record the reconciler writes, taken from the server
+// rather than put together again in the browser.
+func TestBimiGetReportsTheRecordItWouldPublish(t *testing.T) {
+	store := &memoryBimiStore{logos: map[string]*BimiLogo{}}
+	r := newBimiRouter(store)
+	putLogo(t, r, fixture(t, "ebay.svg"))
+	read := func() map[string]any {
+		return jsonBody(t, bimiCall(r, http.MethodGet, "/api/v1/admin/bimi?domain=ebay.com", "", true))
+	}
+
+	got := read()
+	if got["dns_record"] != "v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg;" || got["certificate_withheld"] != "" {
+		t.Fatalf("logo only: %v", got)
+	}
+
+	putCertificate(r, fixture(t, "ebay.pem"))
+	if got := read(); got["dns_record"] != "v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg; a=https://mail.ebay.com/.well-known/bimi/vmc.pem;" {
+		t.Fatalf("with its certificate: %v", got["dns_record"])
+	}
+
+	// The logo is replaced: the certificate is held but no longer advertised
+	putLogo(t, r, fixture(t, "cnn.svg"))
+	got = read()
+	if got["certificate_withheld"] != "logoMismatch" || strings.Contains(got["dns_record"].(string), "a=") {
+		t.Fatalf("after replacing the logo: %v / %v", got["dns_record"], got["certificate_withheld"])
+	}
+}