|
| 1 | +# v1.3.34.1 -- Telegram notifications: HTML default |
| 2 | + |
| 3 | +A focused notifications-channel patch on top of v1.3.34. Switches |
| 4 | +the default Telegram parse_mode from MarkdownV2 to HTML so event |
| 5 | +types like `config_change` (with their underscores) stop tripping |
| 6 | +the Telegram parser. Closes the ninth strike in the upstream- |
| 7 | +behaviour pattern. |
| 8 | + |
| 9 | +## Why |
| 10 | + |
| 11 | +Through v1.3.34 the default Telegram template emitted |
| 12 | +`*{{ .Type }}*` (bold) without piping `.Type` through `escapeMD`. |
| 13 | +For event types containing underscores -- `config_change`, |
| 14 | +`threat_ip_banned`, `cert_renewal_failed`, etc. -- Telegram's |
| 15 | +MarkdownV2 parser reads the unescaped `_` as "begin italic" and |
| 16 | +fails the request: |
| 17 | + |
| 18 | +``` |
| 19 | +{"ok":false,"error_code":400,"description":"can't parse entities: |
| 20 | + Character '_' is reserved and must be escaped with the preceding |
| 21 | + '\\'"} |
| 22 | +``` |
| 23 | + |
| 24 | +Operator dogfood revealed every config_change / |
| 25 | +threat_ip_banned / cert_* delivery silently logging as `failed` |
| 26 | +in the `notification_deliveries` table. The Telegram channel |
| 27 | +was effectively silent for the 12 most common event types. |
| 28 | + |
| 29 | +The MarkdownV2 escape set has 18 reserved chars |
| 30 | +(`_*[]()~\`>#+-=|{}.!`); HTML has 3 (`<`, `>`, `&`). Switching |
| 31 | +the default to HTML makes the parser an order of magnitude |
| 32 | +more forgiving for arbitrary operator data ending up in event |
| 33 | +fields, with no expressivity loss for the default template's |
| 34 | +bold-Type + code-HostDomain shape. |
| 35 | + |
| 36 | +## What ships |
| 37 | + |
| 38 | +### Two Go-source changes |
| 39 | + |
| 40 | +`backend/internal/notifications/templates.go`: |
| 41 | + |
| 42 | +- New `escapeHTML` template function. Wraps `html.EscapeString` |
| 43 | + from the Go stdlib (covers `<`, `>`, `&`, plus quotes for |
| 44 | + defence-in-depth). Accepts `any` so callers can pipe |
| 45 | + string-typed aliases like `EventType` through it without |
| 46 | + `printf` coercion. |
| 47 | +- `escapeMD` widened to also accept `any` (was `string`-only) |
| 48 | + for the same reason -- regression-safe since Sprintf("%v", s) |
| 49 | + on a `string` returns it unchanged. |
| 50 | +- Default Telegram template rewritten as HTML: |
| 51 | + |
| 52 | + ```text |
| 53 | + {{ .Severity | severityEmoji }} <b>{{ .Type | escapeHTML }}</b> |
| 54 | + {{ if .HostDomain }}host: <code>{{ .HostDomain | escapeHTML }}</code>{{ end }} |
| 55 | + {{ .Message | escapeHTML }} |
| 56 | + ``` |
| 57 | + |
| 58 | +`backend/internal/notifications/senders/telegram.go`: |
| 59 | + |
| 60 | +- Empty-`parse_mode` fallback flipped from `MarkdownV2` to |
| 61 | + `HTML`. Channels with `parse_mode` explicitly set keep their |
| 62 | + setting (no forced migration). |
| 63 | + |
| 64 | +### Unit tests |
| 65 | + |
| 66 | +`backend/internal/notifications/templates_test.go` (new): |
| 67 | + |
| 68 | +- `TestTelegramDefaultTemplateRendersValidHTML` -- default |
| 69 | + output contains `<b>config_change</b>` and |
| 70 | + `<code>host_with_under.example.com</code>`, no MarkdownV2 |
| 71 | + backslash-escapes, and the Message's `<`/`>`/`&` survive as |
| 72 | + `<`/`>`/`&`. |
| 73 | +- `TestEscapeHTMLOnDynamicFields` -- `<script>alert(1)</script>` |
| 74 | + in HostDomain is rendered as `<script>...</script>`, |
| 75 | + no raw markup leaks. |
| 76 | +- `TestEscapeMDStillWorks` -- regression: a custom MarkdownV2 |
| 77 | + template using `{{ ... | escapeMD }}` keeps producing the |
| 78 | + v1.3.21-era `\_`, `\(`, `\)`, `\>` escapes for operators with |
| 79 | + pinned MarkdownV2 channels. |
| 80 | + |
| 81 | +`backend/internal/notifications/senders/telegram_test.go` (new): |
| 82 | + |
| 83 | +- `TestTelegramSenderDefaultsToHTMLParseMode` -- mock Bot API |
| 84 | + server captures the form body; asserts `parse_mode=HTML` |
| 85 | + when the channel config omits the field. |
| 86 | +- `TestTelegramSenderHonoursExplicitMarkdownV2` -- the same |
| 87 | + mock server asserts `parse_mode=MarkdownV2` is honoured when |
| 88 | + set in channel config; no forced migration of pre-v1.3.34.1 |
| 89 | + channels. |
| 90 | +- `TestTelegramSenderSurfacesAPIError` -- 400 with a |
| 91 | + `description: "can't parse entities..."` body produces a |
| 92 | + wrapped Go error containing both the status code and the |
| 93 | + Telegram description, so the worker's `notification_deliveries` |
| 94 | + row carries a useful error_message. |
| 95 | + |
| 96 | +All five new tests + the three existing rate-limit tests pass: |
| 97 | +`go test ./internal/notifications/...` is green. |
| 98 | + |
| 99 | +### Documentation |
| 100 | + |
| 101 | +`docs/features/notifications.md` -- new "parse_mode: HTML |
| 102 | +(default) vs MarkdownV2" subsection under the telegram channel |
| 103 | +config docs. Lists the trade-offs (3 reserved chars vs 18), |
| 104 | +quotes the default HTML template, links to |
| 105 | +[Telegram Bot API HTML Style](https://core.telegram.org/bots/api#html-style), |
| 106 | +documents the no-forced-migration policy for existing channels |
| 107 | +that pinned `parse_mode: "MarkdownV2"`. |
| 108 | + |
| 109 | +## Why a four-component version (and no version-string bump) |
| 110 | + |
| 111 | +This release ships behavioural code change but the operator |
| 112 | +chose to keep `argosVersion` and `frontend/package.json` at |
| 113 | +`1.3.33` for the same reason v1.3.34 did: a coherent |
| 114 | +"v1.3.33-binary-line" identifier through the doc-refresh + |
| 115 | +notification-fix patch range. The next behavioural release |
| 116 | +that warrants a version-string bump (a feature, schema |
| 117 | +migration, or non-trivial behavior change) will resume the |
| 118 | +three-component sequence at `v1.3.35`. |
| 119 | + |
| 120 | +The four-component `v1.3.34.1` tag exists for git history / |
| 121 | +GitHub Releases (so the docs portal lists it next to v1.3.34 |
| 122 | +under release notes), and the panel **does** require a |
| 123 | +rebuild to pick up the Go source changes -- it's not a |
| 124 | +tag-without-rebuild release like v1.3.27.1 / v1.3.34 were. |
| 125 | + |
| 126 | +`scripts/check-no-personal-data.sh` clean. |
| 127 | + |
| 128 | +## Mid-impl gotcha (caught + fixed pre-tag) |
| 129 | + |
| 130 | +**Go template type strictness.** Initial `escapeHTML` |
| 131 | +implementation was `func(s string) string`. Tests immediately |
| 132 | +failed: |
| 133 | + |
| 134 | +``` |
| 135 | +template: tmpl:1:46: executing "tmpl" at <escapeHTML>: |
| 136 | + wrong type for value; expected string; |
| 137 | + got notifications.EventType |
| 138 | +``` |
| 139 | + |
| 140 | +`text/template` does not auto-convert string-typed aliases |
| 141 | +(like `type EventType string`) into `string` for FuncMap call |
| 142 | +sites. The pre-v1.3.34.1 default template never piped `.Type` |
| 143 | +through any function -- it just emitted `*{{ .Type }}*` raw, |
| 144 | +which is exactly the bug. Fix: `escapeHTML` and `escapeMD` now |
| 145 | +both accept `any` and stringify via `fmt.Sprintf("%v", v)`. No |
| 146 | +behaviour change for existing string callers. |
| 147 | + |
| 148 | +## Smoke gate |
| 149 | + |
| 150 | +The smoke for this release is operator-mediated against a real |
| 151 | +Telegram channel: |
| 152 | + |
| 153 | +1. After `make sync-prod` + binary rebuild + `make deploy-prod`, |
| 154 | + open the panel's notifications settings. |
| 155 | +2. Create a fresh Telegram channel pointing at an operator- |
| 156 | + owned bot + chat (do NOT include `parse_mode` in the channel |
| 157 | + config -- let the v1.3.34.1 default kick in). |
| 158 | +3. Click "Send test" on the channel. |
| 159 | +4. Verify Telegram receives the test message with the event |
| 160 | + type rendered as bold (e.g. **threat_ip_banned**), the host |
| 161 | + in monospace, and no 400/failed delivery in the |
| 162 | + `notification_deliveries` audit table. |
| 163 | +5. (Optional) Repeat with a custom template that contains |
| 164 | + characters like `<script>` in the message body; verify |
| 165 | + Telegram displays them as literal text, not as parsed HTML. |
| 166 | + |
| 167 | +A unit-tested mock-server smoke (the three sender tests above) |
| 168 | +already verifies the request-path shape; the operator-mediated |
| 169 | +smoke is the EFFECT gate. |
| 170 | + |
| 171 | +## Files changed |
| 172 | + |
| 173 | +- `backend/internal/notifications/templates.go` (escapeHTML + |
| 174 | + default template + escapeMD signature widening) |
| 175 | +- `backend/internal/notifications/senders/telegram.go` |
| 176 | + (parse_mode default flipped to HTML) |
| 177 | +- `backend/internal/notifications/templates_test.go` (new) |
| 178 | +- `backend/internal/notifications/senders/telegram_test.go` |
| 179 | + (new) |
| 180 | +- `docs/features/notifications.md` (parse_mode subsection) |
| 181 | +- `docs/release-notes/v1.3.34.1.md` (this file) |
| 182 | +- `CHANGELOG.md`, `mkdocs.yml` |
| 183 | + |
| 184 | +**NOT changed**: `backend/cmd/argos/main.go` `argosVersion` |
| 185 | +stays at `1.3.33`; `frontend/package.json` `version` stays at |
| 186 | +`1.3.33`; no migrations; no frontend behavior; no smokes |
| 187 | +under `scripts/smoke/`. |
| 188 | + |
| 189 | +## Upgrade |
| 190 | + |
| 191 | +```bash |
| 192 | +cd ~/argos-edge |
| 193 | +git pull |
| 194 | +make sync-prod # picks up Go source change + docs |
| 195 | +make deploy-prod # rebuilds the panel binary; required |
| 196 | + # because templates.go + telegram.go |
| 197 | + # ship new Go code |
| 198 | +``` |
| 199 | + |
| 200 | +After `deploy-prod` finishes, the operator-mediated smoke gate |
| 201 | +above replaces a scripted smoke for this release. |
| 202 | + |
| 203 | +For operators with an existing Telegram channel that has been |
| 204 | +silently failing on `config_change` / `threat_ip_banned` |
| 205 | +deliveries since v1.3.21 -- those events will start being |
| 206 | +delivered correctly within seconds of the new binary serving |
| 207 | +traffic. No channel reconfiguration is required for channels |
| 208 | +without a pinned `parse_mode`. Channels with |
| 209 | +`parse_mode: "MarkdownV2"` set explicitly are honoured as-is. |
| 210 | + |
| 211 | +## Ninth-strike entry in the upstream-behaviour pattern |
| 212 | + |
| 213 | +The eight-strike pattern documented in |
| 214 | +`memory/project_four_strike_upstream_pattern.md` becomes |
| 215 | +**nine** with this release. The new strike's lesson: |
| 216 | + |
| 217 | +> When a third-party parser offers multiple syntaxes, prefer |
| 218 | +> the variant with the smallest reserved-char set. HTML's three |
| 219 | +> reserved chars are an order of magnitude more forgiving than |
| 220 | +> MarkdownV2's eighteen for arbitrary operator data ending up |
| 221 | +> in event fields. |
| 222 | +
|
| 223 | +This applies prospectively to future Slack / Discord / other |
| 224 | +chat-sender additions: start from plain or HTML, never from the |
| 225 | +richest markup available. |
0 commit comments