Make delivery and integration failures explicit

Persist reminder presentations and retry state, atomically complete collapsed deliveries, fall back across away reaches, and block permanent failures visibly (V-715, V-678). Fail closed when enabled integrations lack credentials and keep remote arms explicitly dark (V-691). Give mavweb one sanitized, request-correlated error contract (V-689). Owner explicitly requested direct commits to master.
2026-08-13 02:50:59 +04:00
parent da9114b623
commit 35c6ff5a71
67 changed files with 3174 additions and 477 deletions
@@ -37,6 +37,12 @@ import (
 // the credential lives in the daemon's config (or a systemd credential),
 // never in the binary.
 type Config struct {
+	// Disabled keeps a written endpoint explicitly dark. This is distinct from
+	// an expanded-empty credential: the latter is a configuration error, while
+	// this flag records an operator decision to use another delivery reach until
+	// credentials are provisioned.
+	Disabled bool `json:"disabled,omitempty"`
+
 	// BaseURL — the ntfy server, no trailing path. Required.
 	BaseURL string `json:"base_url"`

@@ -71,23 +77,45 @@ type Sink struct {
 	hc  *http.Client
 }

-// New validates the config and builds the sink. BaseURL and Topic are
-// required; auth is optional (but deny-all servers reject unauthed publishes).
-func New(cfg Config) (*Sink, error) {
-	if cfg.BaseURL == "" {
-		return nil, fmt.Errorf("ntfysink: BaseURL is required")
+// Validate checks one configuration without constructing a client. A disabled
+// block is the only state in which credentials may be empty. Maven's ntfy
+// reach is private, and accepting an accidental anonymous configuration turns
+// a missing environment variable into an endless 403 retry loop.
+func Validate(cfg Config) error {
+	if cfg.Disabled {
+		return nil
+	}
+	if strings.TrimSpace(cfg.BaseURL) == "" {
+		return fmt.Errorf("ntfysink: BaseURL is required while enabled")
 	}
 	if _, err := url.Parse(cfg.BaseURL); err != nil {
-		return nil, fmt.Errorf("ntfysink: bad BaseURL: %w", err)
+		return fmt.Errorf("ntfysink: bad BaseURL: %w", err)
 	}
-	if cfg.Topic == "" {
-		return nil, fmt.Errorf("ntfysink: Topic is required")
+	if strings.TrimSpace(cfg.Topic) == "" {
+		return fmt.Errorf("ntfysink: Topic is required while enabled")
 	}
 	// Refuse rather than pick. Two credentials configured means someone
 	// intended one of them, and guessing which would send the other nowhere
 	// and leave a working config that is not the one they wrote.
-	if cfg.Token != "" && cfg.Username != "" {
-		return nil, fmt.Errorf("ntfysink: set Token or Username, not both")
+	if strings.TrimSpace(cfg.Token) != "" && (strings.TrimSpace(cfg.Username) != "" || cfg.Password != "") {
+		return fmt.Errorf("ntfysink: set Token or Username/Password, not both")
+	}
+	if strings.TrimSpace(cfg.Token) == "" {
+		if strings.TrimSpace(cfg.Username) == "" || cfg.Password == "" {
+			return fmt.Errorf("ntfysink: Token or Username/Password is required while enabled")
+		}
+	}
+	return nil
+}
+
+// New validates the config and builds the sink. Disabled configs belong at the
+// daemon wiring boundary and cannot accidentally become live sinks.
+func New(cfg Config) (*Sink, error) {
+	if cfg.Disabled {
+		return nil, fmt.Errorf("ntfysink: config is disabled")
+	}
+	if err := Validate(cfg); err != nil {
+		return nil, err
 	}
 	to := cfg.Timeout
 	if to == 0 {
@@ -127,6 +155,10 @@ func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error {
 	defer resp.Body.Close()
 	if resp.StatusCode/100 != 2 {
 		rb, _ := io.ReadAll(io.LimitReader(resp.Body, 512))
+		if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
+			return fmt.Errorf("%w: ntfysink: credentials rejected (%d): %s",
+				delivery.ErrPermanent, resp.StatusCode, strings.TrimSpace(string(rb)))
+		}
 		return fmt.Errorf("ntfysink: ntfy returned %d: %s", resp.StatusCode, strings.TrimSpace(string(rb)))
 	}
 	return nil
@@ -2,6 +2,7 @@ package ntfysink

 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -81,6 +82,10 @@ func reminderSendable(summary string) delivery.Sendable {
 	}
 }

+func tokenConfig(baseURL string) Config {
+	return Config{BaseURL: baseURL, Topic: "maven", Token: "scoped-write-token"}
+}
+
 // ----------------------------- config ---------------------------------------

 func TestNewRejectsEmptyBaseURL(t *testing.T) {
@@ -98,7 +103,7 @@ func TestNewRejectsEmptyTopic(t *testing.T) {
 }

 func TestNewDefaultTimeout(t *testing.T) {
-	s, err := New(Config{BaseURL: "http://localhost:8085", Topic: "maven"})
+	s, err := New(tokenConfig("http://localhost:8085"))
 	if err != nil {
 		t.Fatalf("New: %v", err)
 	}
@@ -107,6 +112,26 @@ func TestNewDefaultTimeout(t *testing.T) {
 	}
 }

+func TestNewRejectsAnonymousPublishing(t *testing.T) {
+	_, err := New(Config{BaseURL: "http://localhost:8085", Topic: "maven"})
+	if err == nil {
+		t.Fatal("New accepted a topic with no credential")
+	}
+	if !strings.Contains(err.Error(), "required while enabled") {
+		t.Fatalf("error should identify the enabled reach: %v", err)
+	}
+}
+
+func TestValidateAllowsEmptyCredentialOnlyWhenDisabled(t *testing.T) {
+	cfg := Config{Disabled: true, BaseURL: "http://localhost:8085", Topic: "maven"}
+	if err := Validate(cfg); err != nil {
+		t.Fatalf("Validate disabled config: %v", err)
+	}
+	if _, err := New(cfg); err == nil {
+		t.Fatal("New built a live sink from a disabled config")
+	}
+}
+
 // ----------------------------- send shape ----------------------------------

 func TestSendPostsToTopicPath(t *testing.T) {
@@ -114,7 +139,7 @@ func TestSendPostsToTopicPath(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, err := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, err := New(tokenConfig(srv.URL))
 	if err != nil {
 		t.Fatalf("New: %v", err)
 	}
@@ -137,7 +162,7 @@ func TestSendBodyIsSummaryNotFullBody(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	if err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "cert expiring soon")); err != nil {
 		t.Fatalf("Send: %v", err)
 	}
@@ -155,7 +180,7 @@ func TestSendNeverSendsTheBodyWhenSummaryEmpty(t *testing.T) {
 	defer srv.Close()

 	s := nudgeSendable(loop.Sev3, "")
-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	if err := sink.Send(context.Background(), s); err != nil {
 		t.Fatalf("Send: %v", err)
 	}
@@ -173,7 +198,7 @@ func TestSendNeverSendsAnEmptyMessage(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	s := nudgeSendable(loop.Sev3, "")
 	s.Body = ""
 	s.RuleName = ""
@@ -209,18 +234,18 @@ func TestSendSetsBasicAuth(t *testing.T) {
 	}
 }

-func TestSendNoAuthWhenUsernameEmpty(t *testing.T) {
+func TestTokenConfigSendsBearerAuth(t *testing.T) {
 	rs := newRecordingServer(t, 200, "")
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	if err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "down")); err != nil {
 		t.Fatalf("Send: %v", err)
 	}
 	_, _, _, auth, _, _ := rs.snapshot()
-	if auth != "" {
-		t.Fatalf("want no auth header, got %q", auth)
+	if auth != "Bearer scoped-write-token" {
+		t.Fatalf("want bearer auth header, got %q", auth)
 	}
 }

@@ -260,7 +285,7 @@ func TestSendTitleIsMaven(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	if err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "down")); err != nil {
 		t.Fatalf("Send: %v", err)
 	}
@@ -277,7 +302,7 @@ func TestPrioritySev3IsHigh(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	_ = sink.Send(context.Background(), nudgeSendable(loop.Sev3, "cert"))
 	_, _, _, _, _, prio := rs.snapshot()
 	if prio != "4" {
@@ -290,7 +315,7 @@ func TestPrioritySev4IsMax(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	_ = sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down"))
 	_, _, _, _, _, prio := rs.snapshot()
 	if prio != "5" {
@@ -303,7 +328,7 @@ func TestPriorityReminderIsHigh(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	_ = sink.Send(context.Background(), reminderSendable("wake up"))
 	_, _, _, _, _, prio := rs.snapshot()
 	if prio != "4" {
@@ -318,7 +343,7 @@ func TestSendReturnsErrorOnNon2xx(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "down"))
 	if err == nil {
 		t.Fatal("want error on 403")
@@ -326,6 +351,24 @@ func TestSendReturnsErrorOnNon2xx(t *testing.T) {
 	if !strings.Contains(err.Error(), "403") {
 		t.Fatalf("error should mention status 403, got: %v", err)
 	}
+	if !errors.Is(err, delivery.ErrPermanent) {
+		t.Fatalf("403 = %v; want delivery.ErrPermanent", err)
+	}
+}
+
+func TestSendServerFailureRemainsRetryable(t *testing.T) {
+	rs := newRecordingServer(t, http.StatusServiceUnavailable, `{"error":"temporarily unavailable"}`)
+	srv := httptest.NewServer(rs.handler())
+	defer srv.Close()
+
+	sink, _ := New(tokenConfig(srv.URL))
+	err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "down"))
+	if err == nil {
+		t.Fatal("want error on 503")
+	}
+	if errors.Is(err, delivery.ErrPermanent) {
+		t.Fatalf("503 was marked permanent: %v", err)
+	}
 }

 func TestSendContextCancelReturnsError(t *testing.T) {
@@ -333,7 +376,7 @@ func TestSendContextCancelReturnsError(t *testing.T) {
 	srv := httptest.NewServer(rs.handler())
 	defer srv.Close()

-	sink, _ := New(Config{BaseURL: srv.URL, Topic: "maven"})
+	sink, _ := New(tokenConfig(srv.URL))
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond)
 	defer cancel()
 	err := sink.Send(ctx, nudgeSendable(loop.Sev3, "down"))
@@ -343,7 +386,9 @@ func TestSendContextCancelReturnsError(t *testing.T) {
 }

 func TestSendConnectionRefusedReturnsError(t *testing.T) {
-	sink, _ := New(Config{BaseURL: "http://127.0.0.1:1", Topic: "maven", Timeout: time.Second})
+	cfg := tokenConfig("http://127.0.0.1:1")
+	cfg.Timeout = time.Second
+	sink, _ := New(cfg)
 	err := sink.Send(context.Background(), nudgeSendable(loop.Sev3, "down"))
 	if err == nil {
 		t.Fatal("want error on connection refused")