From 6ba463eb01f05c1b9b70542d625c668cf8e8cc5e Mon Sep 17 00:00:00 2001 From: Colin Dean Date: Thu, 27 Jun 2024 14:22:20 -0400 Subject: [PATCH 1/3] fix: Prevent error when using quotation marks in BuildMessage When a PR title or commit summary contains quotation marks, they were not escaped when injected into the JSON attachment. This caused a message like unable to unmarshal webhook message: invalid character 't' after object key:value pair when used inside of a template containing this test: ```json { "text": "*Status*: FAILURE\n*Repo*: {{ .RepositoryName }} | *Branch*: {{ .BuildBranch }} | *Author*: {{ .BuildAuthor }}\n*Build*: {{ .BuildLink }}\n*Message*: {{ .BuildMessage }}" } ``` This change safens this use case. Other typical fields _shouldn't_ have quotation marks in them, but it might be prudent down the line to cleanse _all_ fields as they are injected into the JSON or provide a method usable inside of the template that escapes correctly. --- cmd/vela-slack/plugin.go | 9 ++++++++- cmd/vela-slack/plugin_test.go | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/cmd/vela-slack/plugin.go b/cmd/vela-slack/plugin.go index 03f0243..2b6985c 100644 --- a/cmd/vela-slack/plugin.go +++ b/cmd/vela-slack/plugin.go @@ -91,7 +91,7 @@ func (p *Plugin) Exec() error { // clean up newlines that could invalidate JSON // BuildMessage is the only field that can have newlines; // typically when the commit contains a title and body message - p.Env.BuildMessage = strings.Replace(p.Env.BuildMessage, "\n", "\\n", -1) + p.Env.BuildMessage = cleanBuildMessage(p.Env.BuildMessage) // create message struct file Slack msg := slack.WebhookMessage{ @@ -187,6 +187,13 @@ func (p *Plugin) Exec() error { return nil } +func cleanBuildMessage(buildMessage string) string { + subject := buildMessage + subject = strings.Replace(subject, "\n", "\\n", -1) + subject = strings.Replace(subject, "\"", "\\\"", -1) + return subject +} + // Validate function to validate plugin configuration. func (p *Plugin) Validate() error { logrus.Debug("validating plugin configuration") diff --git a/cmd/vela-slack/plugin_test.go b/cmd/vela-slack/plugin_test.go index 3bd588f..883890e 100644 --- a/cmd/vela-slack/plugin_test.go +++ b/cmd/vela-slack/plugin_test.go @@ -328,6 +328,29 @@ Newlines`, } } +func TestSlack_Plugin_Exec_Quote(t *testing.T) { + // setup types + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "ok") + })) + defer ts.Close() + + p := &Plugin{ + Webhook: ts.URL, + Env: &Env{ + BuildMessage: `This message has "quotes"`, + }, + Path: "testdata/slack_attachment.json", + WebhookMsg: &slack.WebhookMessage{}, + Remote: false, + } + + err := p.Exec() + if err != nil { + t.Errorf("Exec returned err: %v", err) + } +} + func TestSlack_Plugin_Exec_Newline_Embedded(t *testing.T) { // setup types ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From e6a55fd953c929a52b670a09866bce146b564e4f Mon Sep 17 00:00:00 2001 From: Colin Dean Date: Thu, 27 Jun 2024 14:51:07 -0400 Subject: [PATCH 2/3] fix: Use ReplaceAll instead of Replace with -1 --- cmd/vela-slack/plugin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/vela-slack/plugin.go b/cmd/vela-slack/plugin.go index 2b6985c..bb3dc79 100644 --- a/cmd/vela-slack/plugin.go +++ b/cmd/vela-slack/plugin.go @@ -189,8 +189,8 @@ func (p *Plugin) Exec() error { func cleanBuildMessage(buildMessage string) string { subject := buildMessage - subject = strings.Replace(subject, "\n", "\\n", -1) - subject = strings.Replace(subject, "\"", "\\\"", -1) + subject = strings.ReplaceAll(subject, "\n", "\\n") + subject = strings.ReplaceAll(subject, "\"", "\\\"") return subject } From 17816a074433a9139ce8d2fb710aa60e9bcea7a1 Mon Sep 17 00:00:00 2001 From: Colin Dean Date: Fri, 28 Jun 2024 13:58:40 -0400 Subject: [PATCH 3/3] fix: Address cuddling comment --- cmd/vela-slack/plugin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/vela-slack/plugin.go b/cmd/vela-slack/plugin.go index bb3dc79..d923485 100644 --- a/cmd/vela-slack/plugin.go +++ b/cmd/vela-slack/plugin.go @@ -191,6 +191,7 @@ func cleanBuildMessage(buildMessage string) string { subject := buildMessage subject = strings.ReplaceAll(subject, "\n", "\\n") subject = strings.ReplaceAll(subject, "\"", "\\\"") + return subject }