diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 49850e6..08fd1d0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -206,6 +206,16 @@ Per-webhook behavior: a severity filter (`min_level`), an optional exponential backoff up to `notify_max_retries`, and a per-`(metric, resource)` rate limiter (`notify_min_interval_seconds`) that coalesces repeated firings. +`text/template` does no escaping of its own, so a custom template whose body is JSON must +use the `json` template function (registered in `templateFuncs`) around every interpolated +value — e.g. `{"text": {{json .Message}}}` — rather than embedding it inside a quoted string +literal. `json` marshals the value and supplies its own surrounding quotes, so a resource +string containing a quote or backslash (e.g. an unusual mountpoint) can't produce a +malformed request body. Because `json` supplies its own quotes, it must wrap the *whole* +string value: literal text has to be composed into the value with `printf` +(`{{json (printf "PiMonitor: %s" .Message)}}`) rather than concatenated around the +`json` action, or the literal quotes and the helper's quotes nest into invalid JSON. + **Only retryable failures consume the retry budget.** Transport errors (DNS, refused connections, timeouts) and 5xx responses are transient, as are `408 Request Timeout` and `429 Too Many Requests`, which explicitly invite a retry. Every other 4xx is a permanent diff --git a/internal/alert/notify.go b/internal/alert/notify.go index e481305..bf23fa5 100644 --- a/internal/alert/notify.go +++ b/internal/alert/notify.go @@ -39,6 +39,24 @@ const ( defaultNotifyContentType = "application/json" ) +// templateFuncs are the helpers available to a webhook body template. +// text/template does no escaping of its own, so a template rendering a JSON +// body must be able to emit a correctly quoted and escaped JSON value — +// otherwise a resource string containing a quote or backslash produces a +// malformed request body. +var templateFuncs = template.FuncMap{ + // json renders v as a complete JSON value, including surrounding quotes + // for strings. Use it as {"text": {{json .Message}}} — note there are no + // quotes around the action, json supplies them. + "json": func(v any) (string, error) { + b, err := json.Marshal(v) + if err != nil { + return "", err + } + return string(b), nil + }, +} + // webhook is a single resolved delivery destination: the config values with // the template pre-parsed and defaults applied. type webhook struct { @@ -104,7 +122,9 @@ func NewNotifier(cfg config.Alerts, log *slog.Logger) (*Notifier, error) { wh.contentType = defaultNotifyContentType } if w.Template != "" { - tmpl, err := template.New(fmt.Sprintf("webhook[%d]", i)).Parse(w.Template) + tmpl, err := template.New(fmt.Sprintf("webhook[%d]", i)). + Funcs(templateFuncs). + Parse(w.Template) if err != nil { return nil, fmt.Errorf("alerts.webhooks[%d].template: %w", i, err) } diff --git a/internal/alert/notify_test.go b/internal/alert/notify_test.go index 421362a..f60eaf7 100644 --- a/internal/alert/notify_test.go +++ b/internal/alert/notify_test.go @@ -118,6 +118,109 @@ func TestNotifier_RendersTemplate(t *testing.T) { } } +// The json template helper escapes a hostile resource string into a valid +// JSON value, so a mountpoint containing a quote or backslash cannot break +// the payload (regression test for #109). +func TestNotifier_JSONHelperEscapesHostileResource(t *testing.T) { + srv, ch := newCapturingServer(t, http.StatusOK) + + n, err := NewNotifier(config.Alerts{ + Webhooks: []config.Webhook{{ + URL: srv.URL, + Template: `{"text": {{json .Message}}}`, + }}, + }, nil) + if err != nil { + t.Fatalf("NewNotifier: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + n.Start(ctx) + + ev := Event{ + Metric: "disk", Resource: `/mnt/we"ird\path`, + Kind: KindFired, From: LevelOK, To: LevelCrit, Value: 98, At: time.Now(), + } + n.Notify([]Event{ev}) + + req := waitForRequest(t, ch) + var payload map[string]any + if err := json.Unmarshal(req.body, &payload); err != nil { + t.Fatalf("payload is not valid JSON: %v (body=%s)", err, req.body) + } + want := formatMessage(ev) + if payload["text"] != want { + t.Errorf("payload[text] = %q, want %q", payload["text"], want) + } +} + +// The same hostile resource, rendered through the unescaped form the json +// helper replaces, demonstrably produces invalid JSON. This documents why the +// helper exists and would catch a future change that silently starts +// escaping template output on its own. +func TestNotifier_UnescapedTemplateProducesInvalidJSON(t *testing.T) { + srv, ch := newCapturingServer(t, http.StatusOK) + + n, err := NewNotifier(config.Alerts{ + Webhooks: []config.Webhook{{ + URL: srv.URL, + Template: `{"text": "{{.Message}}"}`, + }}, + }, nil) + if err != nil { + t.Fatalf("NewNotifier: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + n.Start(ctx) + + ev := Event{ + Metric: "disk", Resource: `/mnt/we"ird\path`, + Kind: KindFired, From: LevelOK, To: LevelCrit, Value: 98, At: time.Now(), + } + n.Notify([]Event{ev}) + + req := waitForRequest(t, ch) + var payload map[string]any + if err := json.Unmarshal(req.body, &payload); err == nil { + t.Fatalf("expected invalid JSON from the unescaped template, got valid payload: %s", req.body) + } +} + +// The json helper also handles non-string values, rendering a float64 as a +// bare JSON number and a time.Time as a quoted RFC 3339 string. +func TestNotifier_JSONHelperHandlesNonStringValues(t *testing.T) { + srv, ch := newCapturingServer(t, http.StatusOK) + + n, err := NewNotifier(config.Alerts{ + Webhooks: []config.Webhook{{ + URL: srv.URL, + Template: `{"value": {{json .Value}}, "at": {{json .At}}}`, + }}, + }, nil) + if err != nil { + t.Fatalf("NewNotifier: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + n.Start(ctx) + + at := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + n.Notify([]Event{firedEvent(at)}) + + req := waitForRequest(t, ch) + var payload map[string]any + if err := json.Unmarshal(req.body, &payload); err != nil { + t.Fatalf("payload is not valid JSON: %v (body=%s)", err, req.body) + } + if payload["value"] != float64(98) { + t.Errorf("payload[value] = %v, want 98", payload["value"]) + } + if payload["at"] != at.Format(time.RFC3339) { + t.Errorf("payload[at] = %v, want %v", payload["at"], at.Format(time.RFC3339)) + } +} + // A custom content_type overrides the default, so a plain-text template body // isn't mislabeled as JSON. func TestNotifier_CustomContentType(t *testing.T) { diff --git a/packaging/pimonitor.example.yaml b/packaging/pimonitor.example.yaml index 4769a47..cd16e46 100644 --- a/packaging/pimonitor.example.yaml +++ b/packaging/pimonitor.example.yaml @@ -107,8 +107,15 @@ alerts: # # Optional Go text/template rendered into the request body. When # # omitted, a default JSON object describing the event is sent. The # # template sees: .Metric, .Resource, .Kind ("fired"/"cleared"), - # # .From, .To, .Value, .At, .Message. Example Slack payload: - # # template: '{"text": "PiMonitor: {{.Message}}"}' + # # .From, .To, .Value, .At, .Message. + # # + # # When the body is JSON, wrap every interpolated value in the `json` + # # helper — it emits a correctly quoted and escaped JSON value, so a + # # mountpoint containing a quote or backslash cannot break the payload. + # # Note that `json` supplies the surrounding quotes itself, so it must + # # wrap the *entire* string value — combine literal text into the value + # # with `printf` rather than concatenating it around the action, e.g. + # # template: '{"text": {{json (printf "PiMonitor: %s" .Message)}}}' # # Content-Type header (default "application/json"). Override it if a # # custom template renders a non-JSON body, e.g. "text/plain". # content_type: "application/json"