From f636e268f97984d074e1bc2f031152c00afb3b73 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 08:59:05 +0000 Subject: [PATCH 1/2] Add a json template helper for webhook bodies text/template does no escaping of its own, so a custom webhook template whose body is JSON (e.g. `{"text": "{{.Message}}"}`) inserted a resource string raw. A disk alert's .Resource is a mountpoint read from /proc/mounts, which can contain a quote or backslash, producing a malformed request body. Register a `json` function on the template that marshals its argument and supplies its own surrounding quotes, so authors write `{{json .Message}}` instead of quoting the interpolation by hand. Existing templates that don't use `json` are unaffected. Update the shipped example config and the architecture doc to recommend the safe form. --- docs/ARCHITECTURE.md | 7 +++ internal/alert/notify.go | 22 ++++++- internal/alert/notify_test.go | 103 +++++++++++++++++++++++++++++++ packaging/pimonitor.example.yaml | 10 ++- 4 files changed, 139 insertions(+), 3 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 49850e6..ed4d813 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -206,6 +206,13 @@ 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. + **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..70005ee 100644 --- a/packaging/pimonitor.example.yaml +++ b/packaging/pimonitor.example.yaml @@ -107,8 +107,14 @@ 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. + # # Example Slack payload: + # # template: '{"text": {{json .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" From 3fe54a39cabc40346d70d41ff3fb2cc3417efcc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 09:42:05 +0000 Subject: [PATCH 2/2] Show how to compose literal text with the json template helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The example dropped the old "PiMonitor: " prefix, and neither it nor the ARCHITECTURE.md paragraph showed how to combine literal text with an escaped value. Someone restoring the prefix as "PiMonitor: {{json .Message}}" would get invalid JSON, since json supplies its own quotes and they nest inside the literal ones — reproducing exactly the bug this PR fixes. Show the printf composition form instead, which wraps the whole string value in json and round-trips correctly. --- docs/ARCHITECTURE.md | 5 ++++- packaging/pimonitor.example.yaml | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ed4d813..08fd1d0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -211,7 +211,10 @@ use the `json` template function (registered in `templateFuncs`) around every in 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. +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 diff --git a/packaging/pimonitor.example.yaml b/packaging/pimonitor.example.yaml index 70005ee..cd16e46 100644 --- a/packaging/pimonitor.example.yaml +++ b/packaging/pimonitor.example.yaml @@ -112,9 +112,10 @@ alerts: # # 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. - # # Example Slack payload: - # # template: '{"text": {{json .Message}}}' + # # 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"