Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions api/pkg/emails/event_payload_formatter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package emails

import (
"bytes"
"encoding/json"
"html"
"html/template"
"strings"
)

const (
eventPayloadCodeBlockStyle = "margin:0;padding:12px;border:1px solid #D0D7DE;border-radius:6px;background:#F6F8FA;color:#24292F;font-family:Consolas,Monaco,'Courier New',monospace;font-size:13px;line-height:1.5;white-space:pre-wrap;word-break:break-word;overflow-wrap:anywhere;"
jsonKeyStyle = "color:#0550AE;font-weight:600;"
jsonStringStyle = "color:#0A3069;"
jsonNumberStyle = "color:#953800;"
jsonLiteralStyle = "color:#8250DF;"
)

func formatEventPayload(payload string) (string, template.HTML) {
formattedPayload, isJSON := indentEventPayloadJSON(payload)
content := html.EscapeString(formattedPayload)
if isJSON {
content = highlightEventPayloadJSON(formattedPayload)
}

// Every payload token is escaped before this trusted wrapper is constructed.
richPayload := template.HTML(`<pre style="` + eventPayloadCodeBlockStyle + `">` + content + `</pre>`)

Check failure on line 27 in api/pkg/emails/event_payload_formatter.go

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

api/pkg/emails/event_payload_formatter.go#L27

Found a formatted template string passed to 'template.HTML()'. 'template.HTML()' does not escape contents.
return formattedPayload, richPayload
}

func indentEventPayloadJSON(payload string) (string, bool) {
var formatted bytes.Buffer
if err := json.Indent(&formatted, []byte(payload), "", " "); err != nil {
return payload, false
}

return formatted.String(), true
}

func highlightEventPayloadJSON(payload string) string {
var highlighted strings.Builder
highlighted.Grow(len(payload))

for index := 0; index < len(payload); {
switch {
case payload[index] == '"':
end := eventPayloadJSONStringEnd(payload, index)
style := jsonStringStyle
if eventPayloadNextNonSpace(payload, end) == ':' {
style = jsonKeyStyle
}
writeEventPayloadToken(&highlighted, style, payload[index:end])
index = end
case payload[index] == '-' || isEventPayloadDigit(payload[index]):
end := index + 1
// json.Indent already validated this as JSON, so this continuation set only sees JSON number bytes.
for end < len(payload) && isEventPayloadNumberCharacter(payload[end]) {
end++
}
writeEventPayloadToken(&highlighted, jsonNumberStyle, payload[index:end])
index = end
case strings.HasPrefix(payload[index:], "true"):
writeEventPayloadToken(&highlighted, jsonLiteralStyle, "true")
index += len("true")
case strings.HasPrefix(payload[index:], "false"):
writeEventPayloadToken(&highlighted, jsonLiteralStyle, "false")
index += len("false")
case strings.HasPrefix(payload[index:], "null"):
writeEventPayloadToken(&highlighted, jsonLiteralStyle, "null")
index += len("null")
default:
highlighted.WriteString(html.EscapeString(payload[index : index+1]))
index++
}
}

return highlighted.String()
}

func eventPayloadJSONStringEnd(payload string, start int) int {
escaped := false
for index := start + 1; index < len(payload); index++ {
switch {
case escaped:
escaped = false
case payload[index] == '\\':
escaped = true
case payload[index] == '"':
return index + 1
}
}

return len(payload)
}

func eventPayloadNextNonSpace(payload string, start int) byte {
for index := start; index < len(payload); index++ {
switch payload[index] {
case ' ', '\n', '\r', '\t':
continue
default:
return payload[index]
}
}

return 0
}

func isEventPayloadDigit(value byte) bool {
return value >= '0' && value <= '9'
}

func isEventPayloadNumberCharacter(value byte) bool {
return isEventPayloadDigit(value) ||
value == '-' ||
value == '+' ||
value == '.' ||
value == 'e' ||
value == 'E'
}

func writeEventPayloadToken(builder *strings.Builder, style string, token string) {
builder.WriteString(`<span style="`)
builder.WriteString(style)
builder.WriteString(`">`)
builder.WriteString(html.EscapeString(token))
builder.WriteString(`</span>`)
}
113 changes: 113 additions & 0 deletions api/pkg/emails/event_payload_formatter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package emails

import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func TestFormatEventPayloadIndentsAndHighlightsJSON(t *testing.T) {
payload := `{"message":"hello","count":2,"ratio":1.5,"enabled":true,"disabled":false,"missing":null,"nested":{"value":"ok"}}`

plain, rich := formatEventPayload(payload)
html := string(rich)

assert.Equal(t, `{
"message": "hello",
"count": 2,
"ratio": 1.5,
"enabled": true,
"disabled": false,
"missing": null,
"nested": {
"value": "ok"
}
}`, plain)
assert.Contains(t, html, `<span style="color:#0550AE;font-weight:600;">&#34;message&#34;</span>`)
assert.Contains(t, html, `<span style="color:#0A3069;">&#34;hello&#34;</span>`)
assert.Contains(t, html, `<span style="color:#953800;">2</span>`)
assert.Contains(t, html, `<span style="color:#953800;">1.5</span>`)
assert.Contains(t, html, `<span style="color:#8250DF;">true</span>`)
assert.Contains(t, html, `<span style="color:#8250DF;">false</span>`)
assert.Contains(t, html, `<span style="color:#8250DF;">null</span>`)
assert.Contains(t, html, `white-space:pre-wrap`)
}

func TestFormatEventPayloadEscapesPayloadHTML(t *testing.T) {
plain, rich := formatEventPayload(`{"message":"<script>alert(\"x\")</script>&"}`)
html := string(rich)

assert.Contains(t, plain, `<script>alert`)
assert.NotContains(t, html, `<script>`)
assert.Contains(t, html, `&lt;script&gt;`)
assert.Contains(t, html, `&amp;`)
}

func TestFormatEventPayloadPreservesInvalidJSONWithoutHighlighting(t *testing.T) {
payload := "line one\n <strong>line two</strong>"

plain, rich := formatEventPayload(payload)
html := string(rich)

assert.Equal(t, payload, plain)
assert.Contains(t, html, "line one\n &lt;strong&gt;line two&lt;/strong&gt;")
assert.NotContains(t, html, `<strong>`)
assert.NotContains(t, html, `<span style="color:`)
assert.Equal(t, 1, strings.Count(html, `<pre style=`))
}

func TestFormatEventPayloadHandlesTopLevelPayloadShapes(t *testing.T) {
tests := []struct {
name string
payload string
wantPlain string
wantHTMLContains []string
wantHTMLNotContains []string
}{
{
name: "empty payload falls back to unhighlighted block",
payload: "",
wantPlain: "",
wantHTMLContains: []string{`<pre style="`, `</pre>`},
wantHTMLNotContains: []string{`<span style="color:`},
},
{
name: "top-level number stays valid JSON",
payload: "42",
wantPlain: "42",
wantHTMLContains: []string{`<span style="color:#953800;">42</span>`},
wantHTMLNotContains: []string{`color:#0550AE`},
},
{
name: "json array stays readable and escaped",
payload: `[{"message":"<b>safe</b>"},true,null,3]`,
wantPlain: "[\n {\n \"message\": \"<b>safe</b>\"\n },\n true,\n null,\n 3\n]",
wantHTMLContains: []string{
"[\n {",
`&#34;message&#34;`,
`&lt;b&gt;safe&lt;/b&gt;`,
`<span style="color:#953800;">3</span>`,
},
wantHTMLNotContains: []string{`<b>safe</b>`},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
plain, rich := formatEventPayload(tt.payload)
html := string(rich)

assert.Equal(t, tt.wantPlain, plain)
assert.Equal(t, 1, strings.Count(html, `<pre style=`))

for _, want := range tt.wantHTMLContains {
assert.Contains(t, html, want)
}

for _, unwanted := range tt.wantHTMLNotContains {
assert.NotContains(t, html, unwanted)
}
})
}
}
22 changes: 21 additions & 1 deletion api/pkg/emails/hermes_notification_email_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package emails

import (
"fmt"
"strings"
"time"

"github.com/NdoleStudio/httpsms/pkg/events"
Expand All @@ -17,6 +18,8 @@ type hermesNotificationEmailFactory struct {
generator hermes.Hermes
}

const webhookSendFailedEventPayloadPlaceholder = "__HTTPSMS_WEBHOOK_SEND_FAILED_EVENT_PAYLOAD__"

// NewHermesNotificationEmailFactory creates a new instance of the UserEmailFactory
func NewHermesNotificationEmailFactory(config *HermesGeneratorConfig) NotificationEmailFactory {
return &hermesNotificationEmailFactory{
Expand Down Expand Up @@ -76,6 +79,8 @@ func (factory *hermesNotificationEmailFactory) DiscordSendFailed(user *entities.
}

func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.User, payload *events.WebhookSendFailedPayload) (*Email, error) {
formattedPayload, formattedPayloadHTML := formatEventPayload(payload.EventPayload)

email := hermes.Email{
Body: hermes.Body{
Title: "Hello",
Expand All @@ -89,7 +94,11 @@ func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.
{Key: "Phone Number", Value: factory.formatPhoneNumber(payload.Owner)},
{Key: "HTTP Response Code", Value: factory.formatHTTPResponseCode(payload.HTTPResponseStatusCode)},
{Key: "Error Message / HTTP Response", Value: payload.ErrorMessage},
{Key: "Event Payload", Value: payload.EventPayload},
{
Key: "Event Payload",
Value: webhookSendFailedEventPayloadPlaceholder,
UnsafeValue: formattedPayloadHTML,
},
},
Actions: []hermes.Action{
{
Expand Down Expand Up @@ -118,6 +127,8 @@ func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.
if err != nil {
return nil, stacktrace.Propagate(err, "cannot generate text email")
}
// Hermes/html2text collapses dictionary whitespace, so restore the payload after plain-text generation.
text = replaceWebhookSendFailedEventPayloadPlaceholder(text, formattedPayload)

return &Email{
ToEmail: user.Email,
Expand All @@ -127,6 +138,15 @@ func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.
}, nil
}

func replaceWebhookSendFailedEventPayloadPlaceholder(text string, formattedPayload string) string {
before, after, found := strings.Cut(text, webhookSendFailedEventPayloadPlaceholder)
if !found {
return text
}

return before + formattedPayload + after
}
Comment on lines +141 to +148

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent plain-text fallback exposes placeholder string

replaceWebhookSendFailedEventPayloadPlaceholder returns text unchanged when the placeholder is not found. At that point text is the hermes-generated plain-text output, which still contains __HTTPSMS_WEBHOOK_SEND_FAILED_EVENT_PAYLOAD__ as the literal Value of the dictionary entry. If hermes or the underlying html2text converter ever transforms or trims this token (unlikely today but possible on a dependency bump), users would receive a webhook failure email whose "Event Payload" line reads __HTTPSMS_WEBHOOK_SEND_FAILED_EVENT_PAYLOAD__ instead of the actual payload. An alternative would be to set Value to formattedPayload directly and accept that html2text may collapse its whitespace, or to emit an observable signal (log/metric) when !found.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9fa5b76. If Hermes no longer preserves the sentinel, plain-text generation now retries with the actual formatted payload instead of exposing the internal token.


func (factory *hermesNotificationEmailFactory) MessageExpired(user *entities.User, payload *events.MessageSendExpiredPayload) (*Email, error) {
email := hermes.Email{
Body: hermes.Body{
Expand Down
Loading
Loading