Merge the mavpoll sweep: four flag and body-cap defects (#253)

Four defects in cmd/mavpoll/main.go, all found by sweeping the file:
-interval 0 panicked NewTicker, -timeout 0 removed the HTTP deadline
entirely, -wg-cmd "" panicked on fields[0], and get() silently truncated
an oversized body so every monitor past the cut read as "unknown" and
overwrote live services. run() now refuses the three flag values and
get() errors on overflow, leaving the previous facts in place. pollWg
appends ExitError stderr so a missing CAP_NET_ADMIN says so.

(V-611)

cmd/mavpoll/main.go

@@ -77,6 +77,21 @@ func run(args []string) error {
 	if *netdataURL == "" && *kumaURL == "" && *wgIface == "" && *zenTokenFile == "" {
 		return fmt.Errorf("nothing to poll: set -netdata, -kuma, -wg and/or -zenmoney-token-file")
 	}
+	// A bad duration or an empty -wg-cmd used to get past start and kill the
+	// poller on the first tick — time.NewTicker panics on a non-positive
+	// interval, and pollWg indexed field 0 of an empty command. A zero -timeout
+	// is worse than a crash: http.Client reads it as "no deadline", so one
+	// wedged source stalls every other source behind it forever. Refuse all
+	// three here, where the operator sees the message.
+	if *interval <= 0 {
+		return fmt.Errorf("-interval must be positive, got %s", *interval)
+	}
+	if *timeout <= 0 {
+		return fmt.Errorf("-timeout must be positive, got %s", *timeout)
+	}
+	if *wgIface != "" && strings.TrimSpace(*wgCmd) == "" {
+		return fmt.Errorf("-wg-cmd is empty but -wg is set")
+	}
 
 	zen, err := newZenClient(*zenTokenFile, *zenURL, *timeout)
 	if err != nil {
@@ -283,9 +298,19 @@ const (
 // `wg show` needs CAP_NET_ADMIN; run mavpoll with the cap or set -wg-cmd "sudo wg".
 func (p *poller) pollWg(ctx context.Context) error {
 	fields := strings.Fields(p.wgCmd)
+	if len(fields) == 0 {
+		return fmt.Errorf("wg command is empty")
+	}
 	args := append(fields[1:], "show", p.wgIface, "latest-handshakes")
 	out, err := exec.CommandContext(ctx, fields[0], args...).Output()
 	if err != nil {
+		// wg says why it refused on stderr — usually a missing CAP_NET_ADMIN or
+		// an interface that does not exist. Output() drops that, leaving a log
+		// line that reads "exit status 1" and diagnoses nothing.
+		var ee *exec.ExitError
+		if errors.As(err, &ee) && len(ee.Stderr) > 0 {
+			return fmt.Errorf("run %s: %w: %s", p.wgCmd, err, strings.TrimSpace(string(ee.Stderr)))
+		}
 		return fmt.Errorf("run %s: %w", p.wgCmd, err)
 	}
 	maxTs := parseMaxHandshake(string(out))
@@ -552,6 +577,11 @@ func isNoFact(err error) bool {
 
 // maxBodyBytes caps what a source can make the poller hold. Kuma's whole
 // metrics page is a few hundred kilobytes, so 4 MiB is slack, not a budget.
+//
+// Hitting the cap is an error, not a shorter body. A truncated kuma page parses
+// cleanly right up to the cut, and every monitor past it reads as deleted — the
+// poller would write "unknown" over live services and the down-rule would go
+// quiet. Reading one byte past the cap is how we tell full from truncated.
 const maxBodyBytes = 4 << 20
 
 func (p *poller) get(ctx context.Context, url, basicUser string) ([]byte, error) {
@@ -567,12 +597,15 @@ func (p *poller) get(ctx context.Context, url, basicUser string) ([]byte, error)
 		return nil, err
 	}
 	defer resp.Body.Close()
-	body, err := io.ReadAll(io.LimitReader(resp.Body, maxBodyBytes))
+	body, err := io.ReadAll(io.LimitReader(resp.Body, maxBodyBytes+1))
 	if err != nil {
 		return nil, err
 	}
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("GET %s: %s", url, resp.Status)
 	}
+	if len(body) > maxBodyBytes {
+		return nil, fmt.Errorf("GET %s: body over %d bytes", url, maxBodyBytes)
+	}
 	return body, nil
 }

cmd/mavpoll/main_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -212,3 +213,59 @@ func TestRunRequiresSomethingToPoll(t *testing.T) {
 		t.Errorf("err = %v, want a 'nothing to poll' refusal", err)
 	}
 }
+
+// A flag value that would kill the poller later is refused at start, before it
+// dials core: a non-positive interval panics time.NewTicker on the first tick, a
+// zero timeout means http.Client waits forever, and an empty wg command used to
+// index field 0 of an empty slice.
+func TestRunRefusesFlagsThatCrashLater(t *testing.T) {
+	cases := []struct {
+		name string
+		args []string
+		want string
+	}{
+		{"zero interval", []string{"-interval", "0"}, "-interval must be positive"},
+		{"negative interval", []string{"-interval", "-5s"}, "-interval must be positive"},
+		{"zero timeout", []string{"-timeout", "0"}, "-timeout must be positive"},
+		{"empty wg command", []string{"-wg", "wg0", "-wg-cmd", " "}, "-wg-cmd is empty"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			args := append([]string{"-socket", "/tmp/nope.sock"}, c.args...)
+			err := run(args)
+			if err == nil || !strings.Contains(err.Error(), c.want) {
+				t.Errorf("err = %v, want %q", err, c.want)
+			}
+		})
+	}
+}
+
+// pollWg refuses an empty command rather than panicking on fields[0].
+func TestPollWgEmptyCommand(t *testing.T) {
+	p := &poller{core: &factCore{}, wgIface: "wg0", wgCmd: ""}
+	if err := p.pollWg(context.Background()); err == nil {
+		t.Error("want an error, got a poll that ran something")
+	}
+}
+
+// A body at the cap is a truncated body, and a truncated kuma page reads as
+// "every monitor past the cut was deleted". Refuse it instead of parsing it.
+func TestGetRefusesTruncatedBody(t *testing.T) {
+	big := strings.Repeat("x", maxBodyBytes+64)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		io.WriteString(w, big)
+	}))
+	defer srv.Close()
+	p := &poller{http: srv.Client()}
+	if _, err := p.get(context.Background(), srv.URL, ""); err == nil {
+		t.Error("want an over-size refusal, got a silently truncated body")
+	}
+	small := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		io.WriteString(w, "ok")
+	}))
+	defer small.Close()
+	body, err := p.get(context.Background(), small.URL, "")
+	if err != nil || string(body) != "ok" {
+		t.Errorf("get = %q, %v; want the whole small body", body, err)
+	}
+}